Skip to content

Check advisories with cargo-deny instead of the audit-check action - #3

Merged
schreck61 merged 1 commit into
mainfrom
ci-advisories-via-cargo-deny
Nov 11, 2026
Merged

schreck61 merged 1 commit into
mainfrom
ci-advisories-via-cargo-deny

Conversation

@schreck61

Copy link
Copy Markdown
Owner

What does this change?

Replaces the rustsec/audit-check action in the advisories job with a plain
cargo deny check advisories step, and makes a yanked dependency fail the build.

Reading audit-check 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 returning without
    failing the job
    , so a new advisory produced a green weekly run plus 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. (That bump landed separately in d45a0f0,
already on main.)

Evaluation

Measured against cargo-deny 0.20.2, which is both the newest published version
and what CI's cargo install cargo-deny --locked resolves to — so the tests below
reflect what CI will actually run:

Behavior audit-check cargo deny check advisories
Ignore list duplicated in ci.yml, human-synced deny.toml only
Token permissions checks: write + issues: write none
New advisory on schedule files issue, exits 0 exits 1
Yanked crate never reported exits 1 with yanked = "deny"
Stale/unused ignore no signal advisory-not-detected warning
Index unqueryable n/a warning, stays green

Proofs run locally:

  • Ignore list emptied against the real tree → exit 1, naming exactly the four
    advisories the action was ignoring. The list is load-bearing, not decorative.
  • The config in this PR against the pre-bump lockfile → exit 1 on
    async-executor 1.14.0, i.e. it catches precisely what the 2026-11-09 sweep
    passed green on. This is the end-to-end proof that the gap is closed.
  • Post-bump tree, final config → advisories ok, exit 0, no warnings.
  • cargo deny check licenses still ok.

One finding a reviewer should know

[advisories] scope defaults in cargo-deny are asymmetric, and this is the one
way the swap could have quietly weakened things:

  • unmaintained defaults to "all" — fine, and load-bearing here, since all four
    ignored ids are transitive unmaintained advisories.
  • unsound defaults to "workspace" — narrower than cargo-audit. For this repo
    that means nothing is checked at all, because every advisory we ever see is in a
    dependency.

Both are now pinned explicitly in deny.toml. Please don't simplify them away later.

Relatedly, IndexFailure ("unable to check for yanked crates") is Severity::Warning
in cargo-deny, so the stricter yanked policy cannot redden CI on a registry
hiccup
. The flip side is that a yanked crate could slip by on a warning if the
index is unreachable — still strictly more than audit-check, which never reports
yanked at all.

Job layout

Licenses and advisories now share one job (Licenses and advisories), a step each,
since both are cargo-deny and the slowest thing in either is building cargo-deny
itself. The advisories step carries if: ${{ !cancelled() }} so a license failure
doesn't hide it — that was the only real cost of folding. Renaming the job is safe
because main has no branch protection, so there are no required-status-check names
to preserve.

The weekly cron is unchanged and still reaches this job (no job filters on event
name), but a new advisory now fails it instead of filing an issue and going green.
The checks: write and issues: write added in a2d464b both go away with the
action, leaving the workflow-wide contents: read.

The four documented ignores are unchanged and nothing was added to them.

Checklist

  • cargo fmt is clean
  • cargo clippy --all-targets --release is warning-free
  • cargo test --release passes
  • Engine changes come with a test — n/a, no engine changes: this PR touches
    only .github/workflows/ci.yml and deny.toml, no Rust source

Note on the checklist: clippy and the release test suite were run locally on macOS
and both pass, but this PR changes no Rust code, so CI on Linux is the signal that
matters here — in particular the renamed Licenses and advisories job.

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.
@schreck61
schreck61 merged commit 3bb06d1 into main Nov 11, 2026
8 checks passed
@schreck61
schreck61 deleted the ci-advisories-via-cargo-deny branch November 11, 2026 14:15
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