ci(supply-chain): yanks warn, not block — stop dep drift gating unrelated PRs (#82) - #122
Merged
Conversation
…ated PRs (#82) cargo-deny's advisory check re-reads the live RustSec DB + crates.io yank status on every run, so a yank or fresh CVE could turn an UNRELATED PR red with no change on our side (PR #81 ate a bitcoin_hashes yank). Split the gate by determinism: deterministic bans/licenses/sources still gate PRs; a bare crates.io yank is now warn-level so outside-world drift can't block work that never touched that dep. Real RUSTSEC advisories still block: on PRs, at release, and in the daily scan. - deny.toml: yanked = "deny" -> "warn" (with rationale) - audit.yml: open/refresh a tracking issue on a failed daily scan (gh CLI, no new action) - ci.yml: advisories-job comment reflects the yank policy - docs/AGENTIC-ENGINEERING.md section 4: document the PR-vs-release advisory policy The daily watch + the release-boundary gate are the safety net, not the PR gate. Mainstream Rust-OSS posture; never ship a vulnerable build.
…review on #82) Cross-model code review (Codex + subagents) on PR #122 found that applying yanked=warn uniformly let a release ship a yanked dependency, and the new audit.yml alert step had shell/robustness gaps. Adopt variant D2 and harden it. D2 (release boundary): deny.toml keeps `yanked = "deny"`; only the PR-time cargo-deny-advisories job downgrades yanks to a warning (`--warn yanked`, gated on pull_request). Releases, push-to-main, and the nightly audit still block yanks. Real RUSTSEC advisories still block everywhere. audit.yml alert hardening: - gate on `steps.deny.outcome == 'failure'` so a checkout/infra failure no longer files a misleading "advisory landed" issue - tolerate a transient `gh issue list` failure (|| true) so `set -e` can't swallow the alert - exact-title de-dup (post-filter; gh search is fuzzy) + a concurrency group so overlapping runs can't open duplicate trackers - pass `-R "$REPO"` explicitly to the gh calls Also: fix a stale deny.toml header that called the supply-chain gate "non-blocking" (it is required + blocking), annotate the ci.yml summary diagram, and stop the docs claiming a security yank always carries a RUSTSEC advisory.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #82.
Problem
cargo-deny's advisory check re-reads the live RustSec advisory DB + crates.io yank status onevery run, so a dependency yank or a freshly-published CVE can turn an unrelated PR red with
no change on our side. PR #81 (a swap/refresh feature) was blocked when
bitcoin_hashes 0.14.100was yanked — code was fine, the outside world changed.
Approach (chosen via /autoplan review)
Split the supply-chain gate by determinism, and within advisories, by kind:
bans/licenses/sourcesonlychange when we change dependencies. Unchanged.
vulnerability stops the line until fixed or given a justified
ignore. Gating logic untouched.deny.toml:yanked = "deny"→"warn"). A yank isusually a pulled publish, not a vulnerability; if it is a security pull it also carries a RUSTSEC
advisory and still blocks. So yanks stay visible without gating unrelated PRs.
This is the mainstream Rust-OSS posture (schedule the non-deterministic check, gate the deterministic
ones), tuned so "we take security seriously" means never ship a vulnerable build, not block every
contributor on outside-world drift. The daily watch (
audit.yml, already present) + therelease-boundary gate (
ci.ymlreused byrelease.yml) are the safety net, not the PR gate.Why not
continue-on-error: that reports a green check even on a live CVE, hiding the signal.Rejected for visibility. Approach D keeps the gating logic intact, so the release gate is unchanged
by construction.
Changes
deny.toml:yanked = "warn"+ rationale..github/workflows/audit.yml: open/refresh a tracking issue on a failed daily scan (uses thepreinstalled
ghCLI, no third-party action added) so a new advisory is loud, not just an email..github/workflows/ci.yml: advisories-job comment reflects the yank policy.docs/AGENTIC-ENGINEERING.mdsection 4: document the PR-vs-release advisory policy.Verification
cargo deny check advisories→advisories ok(exit 0) withyanked = "warn".untouched; only deny.toml's yank level changed, which applies uniformly).
Follow-ups (separate)
build/test/clippyare not required status checks today (onlythe two cargo-deny jobs gate merges).