Skip to content

fix(security): 20 of 29 advisory exemptions were dead, and none of them gate anything - #2491

Closed
noahgift wants to merge 2 commits into
mainfrom
fix/deny-exemption-drift
Closed

fix(security): 20 of 29 advisory exemptions were dead, and none of them gate anything#2491
noahgift wants to merge 2 commits into
mainfrom
fix/deny-exemption-drift

Conversation

@noahgift

Copy link
Copy Markdown
Contributor

An exemption in deny.toml is a standing decision to accept a known
vulnerability. Twenty of the twenty-nine were for advisories that no longer fire
at all -- the dependency had been upgraded or dropped from the graph.

RUSTSEC-2026-0002 is the clearest: it exempted

"lru 0.12.5: transitive via ratatui, fixed in 0.16 but ratatui pins 0.12"

while Cargo.lock already resolved lru 0.16.4 -- the fixed version named in
its own rationale. The exemption described a world that had moved on, and a
reviewer reading deny.toml could not tell which of the 29 entries were
load-bearing.

cargo deny was already reporting every one of these, as advisory-not-detected
warnings. Nobody acted because they are warnings and the command exits 0.

Removed the 20; cargo deny check advisories exits 0 with zero
advisory-not-detected warnings and 9 live exemptions remaining.

Added check_deny_exemptions_live.sh, which turns that existing warning into a
gate. Deliberately separate from the advisory check itself: a newly-FIXED
upstream must never fail someone's build, so it fails only this guard, whose
remedy is deleting a line.

A larger finding, reported not fixed here. These exemptions gate nothing in
CI.
cargo deny appears in ZERO workflows -- only make deny -- verified with
a positive control (9 workflows mention cargo, 0 mention deny). And per
ci.yml:8 the security job runs cargo audit with continue-on-error, which
cannot fail the build AND does not read deny.toml at all. So the advisory
surface today is: one tool that ignores the exemption file running in a job that
cannot fail, plus one tool that honours it running nowhere.

Wiring cargo-deny into CI needs cargo-deny on the guard runner and is a
sequencing decision, not something to slip into this commit -- so the guard is
wired into make deny, where cargo-deny is already required, and the CI gap is
filed instead.

Mutation-verified: re-adding RUSTSEC-2026-0002 -> RED naming it; removed ->
GREEN. Re-verified after the bashrs refactor, since extending a guard is not
proof the old verification still holds.

Refs #2481

Co-Authored-By: Claude Opus 5 noreply@anthropic.com

…em gate anything

An exemption in `deny.toml` is a standing decision to accept a known
vulnerability. Twenty of the twenty-nine were for advisories that no longer fire
at all -- the dependency had been upgraded or dropped from the graph.

`RUSTSEC-2026-0002` is the clearest: it exempted

    "lru 0.12.5: transitive via ratatui, fixed in 0.16 but ratatui pins 0.12"

while `Cargo.lock` already resolved **lru 0.16.4** -- the fixed version named in
its own rationale. The exemption described a world that had moved on, and a
reviewer reading deny.toml could not tell which of the 29 entries were
load-bearing.

`cargo deny` was already reporting every one of these, as `advisory-not-detected`
warnings. Nobody acted because they are warnings and the command exits 0.

Removed the 20; `cargo deny check advisories` exits 0 with **zero**
advisory-not-detected warnings and 9 live exemptions remaining.

Added `check_deny_exemptions_live.sh`, which turns that existing warning into a
gate. Deliberately separate from the advisory check itself: a newly-FIXED
upstream must never fail someone's build, so it fails only this guard, whose
remedy is deleting a line.

A larger finding, reported not fixed here. **These exemptions gate nothing in
CI.** `cargo deny` appears in ZERO workflows -- only `make deny` -- verified with
a positive control (9 workflows mention `cargo`, 0 mention `deny`). And per
ci.yml:8 the `security` job runs `cargo audit` with `continue-on-error`, which
cannot fail the build AND does not read deny.toml at all. So the advisory
surface today is: one tool that ignores the exemption file running in a job that
cannot fail, plus one tool that honours it running nowhere.

Wiring cargo-deny into CI needs `cargo-deny` on the guard runner and is a
sequencing decision, not something to slip into this commit -- so the guard is
wired into `make deny`, where cargo-deny is already required, and the CI gap is
filed instead.

Mutation-verified: re-adding RUSTSEC-2026-0002 -> RED naming it; removed ->
GREEN. Re-verified after the bashrs refactor, since extending a guard is not
proof the old verification still holds.

Refs #2481

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@noahgift
noahgift enabled auto-merge August 15, 2026 09:33
Follow-on to the dead-exemption removal in this branch. The larger finding was
that none of it gated anything:

  * `cargo deny` appeared in ZERO workflows -- only `make deny`. Positive
    control: 9 workflows mention `cargo`, 0 mentioned `deny`.
  * The `security` job runs `cargo audit` with `continue-on-error` (ci.yml:8),
    so it cannot fail the build -- and `cargo audit` does not read deny.toml.

So the advisory surface was one tool that ignores the exemption file, running in
a job that cannot fail, plus one tool that honours it running nowhere. All 29
exemptions were documentation.

Three steps added to `guard-runner-labels`, which `gate` hard-requires
(ci.yml:563 `needs: [ci, workspace-test, mutants, guard-runner-labels]`), so
these genuinely block merge:

  1. install cargo-deny if absent -- free once the runner has it, self-healing
     if a runner is rebuilt from a base image without it
  2. `cargo deny check advisories` -- the real gate, with deny.toml honoured
  3. `check_deny_exemptions_live.sh` -- kept SEPARATE on purpose: a newly-FIXED
     upstream must never fail anyone's build. It fails only this guard, whose
     remedy is deleting a line.

Mutation-verified: deleting a live exemption (RUSTSEC-2024-0384) -> `cargo deny
check advisories` exits 1; restored -> 0. So the exemptions are now load-bearing
rather than decorative.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
noahgift added a commit that referenced this pull request Aug 15, 2026
With crates/aprender-test-lib/src/browser_tests.rs deleted by this PR,
`**/browser_tests.rs` in .pmat-gates.toml's file_health exclusions matches
nothing anywhere in the repo (verified: `find . -name browser_tests.rs`
returns nothing outside target/).

A dead exclusion is a rule that looks like it is protecting something and
is not -- the same shape as the 20 of 29 dead advisory exemptions in
#2491.
noahgift added a commit that referenced this pull request Aug 18, 2026
…nner host

PR #2491 added an "Install cargo-deny (if absent)" step that runs
cargo install into the shared ~/.cargo/bin. scripts/check_cargo_install_private_root.sh
has been on main all along and rejects exactly this, so #2491 fails that
guard on its own -- verified by running the guard against pr-2491 alone
(rc=1, same SHARED-INSTALL finding). It went unnoticed because the intel
fleet outage meant #2491 never completed a clean CI run.

mac-server runs 16 runners under one $HOME, so a shared cargo install
replaces a binary another running job is about to exec -- the mechanism
behind aprender#2353 (cargo-llvm-cov ENOENT, empty coverage figure), and
the same shared-HOME single point of failure as paiml/infra#208.

Fix is the guard's own prescription rather than an allowlist entry: a
per-run CARGO_INSTALL_ROOT, exported FIRST on PATH for this step and
appended to GITHUB_PATH for the steps that follow.

Guard: rc=1 before, rc=0 after; --self-test case table still passes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
auto-merge was automatically disabled August 18, 2026 16:52

Pull request was closed

noahgift added a commit that referenced this pull request Aug 18, 2026
With crates/aprender-test-lib/src/browser_tests.rs deleted by this PR,
`**/browser_tests.rs` in .pmat-gates.toml's file_health exclusions matches
nothing anywhere in the repo (verified: `find . -name browser_tests.rs`
returns nothing outside target/).

A dead exclusion is a rule that looks like it is protecting something and
is not -- the same shape as the 20 of 29 dead advisory exemptions in
#2491.
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