From 5a3fedd4cee10a9acbd22b5c881eda45e41e733d Mon Sep 17 00:00:00 2001 From: Jim Schreckengast Date: Tue, 10 Nov 2026 22:54:05 -0700 Subject: [PATCH] Check advisories with cargo-deny instead of the audit-check action The `Dependency advisories` job ran rustsec/audit-check with its own copy of the four ignored RUSTSEC ids and a comment asking whoever touched either list to keep them in step. Reading the action at its pinned SHA turned up two limits that no configuration of it fixes: - On a `schedule` event it reports by filing an issue and returns without ever failing the job, so a new advisory produced a green weekly run and an issue, never a red build. - It hard-codes a skip for yanked crates, logging "was yanked, but no issue will be reported about it" and carrying on. A yanked dependency produced no issue, no failure, and one buried log line. The second gap was live: async-executor 1.14.0 was yanked and the 2026-11-09 sweep passed green and silent on it. `cargo deny check advisories` has neither limit. The evaluation behind the swap, against cargo-deny 0.20.2 -- both what is installed here and the newest published version, so what CI's `cargo install cargo-deny --locked` gets: - Advisories fail the build. With the ignore list emptied the check exits 1 and names exactly the four advisories the action was ignoring; with the list intact it exits 0. - Yanked crates fail once `yanked = "deny"` is set. The default is only a warning. Run against the pre-bump lockfile, the config committed here exits 1 on async-executor 1.14.0 -- the sweep that went green. - A stale ignore is reported as `advisory-not-detected`, so the list now says when an entry is ready to be pruned instead of leaning on someone to recheck upstream. - Scope defaults needed pinning, and this is the one way the swap could have quietly weakened things. `unmaintained` defaults to "all", which is what the four transitive ignores need, but `unsound` defaults to "workspace" -- which here means nothing is checked at all, since every advisory we ever see is in a dependency. Both are now explicit. - A registry index that cannot be queried is a warning, not an error, so the stricter yanked policy cannot redden CI on a registry hiccup. The ignore list now lives in deny.toml alone, with the reasoning beside each id, and nothing was added to it. The job needs no token permissions, because it reports by exiting non-zero rather than filing anything: the `checks: write` and the `issues: write` that a2d464b added both go away with the action, leaving the workflow-wide `contents: read`. Licenses and advisories now share one job, a step each, since both are cargo-deny and the slowest thing in either is building cargo-deny itself. The advisories step runs even when the license step fails, so one run still reports both. The weekly cron is unchanged and still reaches this job -- no job in the workflow filters on event name -- but a new advisory now fails it instead of filing an issue and going green. --- .github/workflows/ci.yml | 43 ++++++++++++++-------------------------- deny.toml | 27 ++++++++++++++++++++++++- 2 files changed, 41 insertions(+), 29 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6835b9f..7deaf67 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,7 +6,8 @@ on: pull_request: branches: [main] schedule: - # Weekly advisory sweep so new RustSec advisories surface without a push. + # Weekly advisory sweep so a new RustSec advisory fails the build without + # waiting for someone to push. - cron: "0 6 * * 1" permissions: @@ -161,7 +162,12 @@ jobs: run: cargo check --workspace --locked --release deny: - name: License policy + # Licences and advisories are both cargo-deny checks, and the slowest + # thing in either one is building cargo-deny itself, so they share a job + # and one install. The advisories step still runs when the licence step + # fails, so a single run reports both rather than hiding the second + # behind the first. + name: Licenses and advisories runs-on: ubuntu-latest steps: - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 @@ -173,29 +179,10 @@ jobs: run: cargo install cargo-deny --locked - name: Check dependency licenses run: cargo deny check licenses - - audit: - name: Dependency advisories - runs-on: ubuntu-latest - permissions: - contents: read - checks: write # audit-check posts its report as a check run - # Only on a schedule event does the action report by filing an issue - # (one per advisory, skipped when its id is already named in an issue - # or PR) instead of a check run; without this the sweep dies on a 403. - issues: write - steps: - - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - - uses: rustsec/audit-check@69366f33c96575abad1ee0dba8212993eecbe998 # v2.0.0 - with: - token: ${{ secrets.GITHUB_TOKEN }} - # Mirrors deny.toml -- keep both lists in step. ttf-parser is - # unmaintained but reaches us only through winit's Wayland titlebar - # path; the three zbus-family crates were folded into zbus 6.0 and - # reach us only through accesskit_unix and ashpd, neither of which - # has released a zbus 6 upgrade. No fix exists for any of them yet. - # Keeping the list current still matters, in the other direction: - # the scheduled run files an un-ignored warning as an issue and - # still exits green, so that issue is the only signal a genuinely - # new advisory has appeared. - ignore: RUSTSEC-2026-0192,RUSTSEC-2026-0370,RUSTSEC-2026-0371,RUSTSEC-2026-0372 + - name: Check dependency advisories + # Reads its ignore list from deny.toml, where the reasoning for each + # id lives, so there is no second copy of that list here for a human + # to keep in step. Reports by exiting non-zero, which is why this job + # needs no token permissions beyond the workflow-wide contents: read. + if: ${{ !cancelled() }} + run: cargo deny check advisories diff --git a/deny.toml b/deny.toml index cbb85af..d5b5db2 100644 --- a/deny.toml +++ b/deny.toml @@ -1,5 +1,7 @@ # cargo-deny configuration (https://embarkstudios.github.io/cargo-deny/). -# Checked in CI: every license in the dependency graph must be on this list. +# Both sections are checked in CI, in one job, a step each: every license in +# the dependency graph must be on the list below, and no crate may carry an +# advisory or be yanked unless it is named here with the reason why. [licenses] allow = [ @@ -22,6 +24,29 @@ allow = [ ] [advisories] +# A new advisory has to turn the build red. That is the whole point of the +# weekly sweep, and it is the thing the rustsec/audit-check action could not +# do: on a `schedule` event it reported by filing an issue and then returned +# without failing, so a new advisory produced a green run plus an issue and +# never a red build. This check just exits non-zero. +# +# Both scopes are set explicitly rather than left to cargo-deny's defaults. +# `unmaintained` already defaults to "all", but every id ignored below is a +# transitive unmaintained advisory, so the value is load-bearing here and +# should not be able to change under us in a cargo-deny release. `unsound` +# defaults to "workspace", which would quietly narrow the check to our own +# crates -- that is, to nothing, since everything we ever see is in a +# dependency. +unmaintained = "all" +unsound = "all" +# Yanked dependencies fail too. audit-check hard-coded a skip for these +# ("was yanked, but no issue will be reported about it"), which is how +# async-executor 1.14.0 sat in the lockfile through a green weekly sweep. +# When the registry index cannot be queried cargo-deny downgrades this to a +# warning and stays green, so a registry hiccup cannot redden CI on its own. +yanked = "deny" +# Every id below is reported as `advisory-not-detected` once it stops matching +# anything in the tree, so this list says when it is ready to be pruned. ignore = [ # ttf-parser is unmaintained; it reaches us only through # winit → sctk-adwaita → ab_glyph (Wayland titlebar rendering).