Skip to content

fix(security): deny.toml ran in no workflow and had drifted until it failed on main - #2523

Merged
noahgift merged 1 commit into
mainfrom
fix/deny-unenforced
Aug 16, 2026
Merged

fix(security): deny.toml ran in no workflow and had drifted until it failed on main#2523
noahgift merged 1 commit into
mainfrom
fix/deny-unenforced

Conversation

@noahgift

Copy link
Copy Markdown
Contributor

cargo deny check on origin/main:

advisories ok, bans FAILED, licenses FAILED, sources ok      (rc=6)

Why nothing noticed

cargo-deny is invoked from exactly one place in this repo: the deny target in the Makefile. That target is not a prerequisite of tier1, tier2, tier3, or tier4 — so nothing but a human typing make deny ever ran it.

Meanwhile cargo-deny 0.20.2 is already installed in the CI image (/usr/local/cargo/bin/cargo-deny). The tool shipped, the 7.2 KB policy shipped, and no workflow connected them.

ci / security from the reusable sovereign-ci workflow runs cargo audit, which does not read deny.toml — so the licence allow-list, banned-crate rules, and source allow-list were enforced by nothing at all.

CLAUDE.md compounds this: its CI/CD section documents security.yml ("cargo-audit, cargo-deny, cargo-outdated (weekly)"), benchmark.yml, and release.yml. None of those three files exist.

Both failures were config drift, not defects

1 — bans/wildcards. All 10 flagged deps are kind = dev sibling path-deps with no version:

aprender-core     -> aprender-train, aprender-profile
aprender-compute  -> aprender-core, aprender-simulate, aprender-cuda-edge,
                     aprender-solve, aprender-sparse
aprender-serve, aprender-orchestrate -> aprender-cuda-edge
aprender-simulate -> aprender-present-test

That is the deliberate mechanism for breaking the crates.io publish cycle — cargo publish strips dev-dependencies, so a path-only dev-dep can't create a circular version requirement. Adding version would reintroduce the cycle. cargo-deny reports a versionless path dep as a wildcard, so it flagged ten load-bearing choices as banned.

allow-wildcard-paths = true. wildcards = "deny" still applies to real dependencies, which is the case worth banning.

2 — licences. libfuzzer-sys is (MIT OR Apache-2.0) AND NCSA, reached via:

libfuzzer-sys <- rav1e <- ravif <- image <- aprender-test-lib

i.e. through an AV1 encoder pulled in by image, not through anything this project chose. NCSA is OSI-approved and FSF Free/Libre.

Added as a scoped [[licenses.exceptions]] for libfuzzer-sys rather than putting NCSA in the global allow list — so a future NCSA dependency arriving by a different path is still flagged for a decision instead of being silently admitted. Flagging that as the one judgement call in this PR.

After: advisories ok, bans ok, licenses ok, sources ok

Wired so it can't drift again

  • ci.yml guard-runner-labels (in gate.needs) runs cargo deny check inside the image, where cargo-deny already lives — no install step needed.
  • make tier3 runs it too, so pre-push catches it locally.

Mutation: reverting deny.toml reproduces bans FAILED, licenses FAILED; restoring gives four greens. Verified both directions.

bashrs make lint: 1 error, identical to origin/main (pre-existing).

Release-readiness, checked alongside

cargo publish --dry-run --no-verify across all 70 publishable crates: 70 ok, 0 FAIL — against the corrected lockfile from #2518. It fails outright without that fix.

Refs #2503

… and had drifted until it failed on main

`cargo deny check` on origin/main:

    advisories ok, bans FAILED, licenses FAILED, sources ok      (rc=6)

WHY NOTHING NOTICED

`cargo-deny` is invoked from exactly one place in this repo: the `deny` target
in the Makefile. That target is not a prerequisite of tier1, tier2, tier3 or
tier4, so nothing but a human typing `make deny` ever ran it.

Meanwhile cargo-deny 0.20.2 is ALREADY INSTALLED in the CI image
(/usr/local/cargo/bin/cargo-deny). The tool shipped, the policy shipped, and no
workflow connected them.

`ci / security` from the reusable sovereign-ci workflow runs `cargo audit`,
which does NOT read deny.toml -- so the licence allow-list, the banned-crate
rules and the source allow-list were enforced by nothing at all.

CLAUDE.md compounds this: its CI/CD section documents `security.yml`
("cargo-audit, cargo-deny, cargo-outdated (weekly)"), `benchmark.yml` and
`release.yml`. None of those three files exist.

BOTH FAILURES WERE CONFIG DRIFT, NOT DEFECTS

1. bans/wildcards. All 10 flagged deps are `kind = dev` sibling path-deps with
   no `version` field:

       aprender-core    -> aprender-train, aprender-profile
       aprender-compute -> aprender-core, aprender-simulate, aprender-cuda-edge,
                           aprender-solve, aprender-sparse
       aprender-serve, aprender-orchestrate -> aprender-cuda-edge
       aprender-simulate -> aprender-present-test

   That is the deliberate mechanism for breaking the crates.io publish cycle:
   `cargo publish` strips dev-dependencies, so a path-only dev-dep cannot create
   a circular version requirement. Adding `version` would REINTRODUCE the cycle.
   cargo-deny reports a versionless path dep as a wildcard, so it flagged ten
   load-bearing choices. `allow-wildcard-paths = true` -- `wildcards = "deny"`
   still applies to real dependencies, which is the case worth banning.

2. licences. libfuzzer-sys is `(MIT OR Apache-2.0) AND NCSA`, reached via

       libfuzzer-sys <- rav1e <- ravif <- image <- aprender-test-lib

   i.e. through an AV1 encoder pulled in by `image`, not through anything this
   project chose. NCSA is OSI-approved and FSF Free/Libre.

   Added as a SCOPED `[[licenses.exceptions]]` for libfuzzer-sys rather than
   putting NCSA in the global `allow` list, so a future NCSA dependency arriving
   by a different path is still flagged for a decision instead of being silently
   admitted.

AFTER: advisories ok, bans ok, licenses ok, sources ok

WIRED, so it cannot drift unnoticed again:
  * ci.yml guard-runner-labels (in gate.needs) runs `cargo deny check` inside
    the image, where cargo-deny already lives.
  * `make tier3` now runs it too, so pre-push catches it locally.

MUTATION: reverting deny.toml reproduces `bans FAILED, licenses FAILED`;
restoring it gives four greens. Verified both directions.

bashrs make lint: 1 error, identical to origin/main (pre-existing).

Release-readiness checks run alongside this, both green:
  * `cargo publish --dry-run --no-verify` on all 70 publishable crates: 70 ok, 0 FAIL
    (against the corrected lockfile from #2518 -- it fails outright without it)

Refs #2503
@noahgift

Copy link
Copy Markdown
Contributor Author

This PR is the blocker for every merge on the repo. Traced while diagnosing why #2502 had been evicted from the merge queue four times:

merge_group run 31961990382
  ci / lint   FAILED  ->  step "Supply chain audit (cargo deny)"
  ci / gate   FAILED  ->  #2502 evicted

The reusable sovereign-ci workflow runs cargo deny check advisories licenses sources inside ci / lint. On origin/main:

advisories ok, licenses FAILED, sources ok

with this PR:

advisories ok, licenses ok, sources ok

So main has been unmergeable all day on the licence rejection, and nothing else can land until this does.

Two corrections to the PR description above

  1. It says cargo-deny "ran in no workflow". The file security.yml genuinely doesn't exist, but the reusable workflow does run it inside ci / lint. The description overstates the gap — licences were gated, and were failing.

  2. CI checks advisories licenses sourcesnot bans. So the allow-wildcard-paths half of this PR is real but not what unblocks anything. The scoped NCSA exception is the load-bearing change, which makes the judgement call flagged in the description the one that actually matters. Worth your explicit sign-off rather than my inference.

Known overlap, deliberately not fixed here

The step this PR adds to guard-runner-labels runs cargo deny check (all four gates), while the reusable workflow already covers three. The genuinely new coverage is bans; the other three are duplicated (~10s).

Not fixing that here on purpose: concurrency.cancel-in-progress: true means a push would cancel this PR's checks, and right now this run is the only thing on the fleet and the whole repo is waiting on it. Narrowing the step to cargo deny check bans belongs in a follow-up.

@noahgift
noahgift enabled auto-merge August 16, 2026 19:09
@noahgift
noahgift added this pull request to the merge queue Aug 16, 2026
Merged via the queue into main with commit bc7120d Aug 16, 2026
27 of 34 checks passed
@noahgift
noahgift deleted the fix/deny-unenforced branch August 16, 2026 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant