fix(ci): actually run the advisories check, and clear the dead ignores (REQ-347) - #934
Merged
Conversation
…s (REQ-347) The Cargo Deny job ran `cargo deny check bans licenses sources`. The advisories check was not run at all, so deny.toml's entire [advisories] section — every reasoned RUSTSEC ignore — was configuration the job never consulted. Advisory checking did exist as a separate cargo-audit job, but that one is not among CI Gate's eleven needs, so a tree failing `cargo deny check advisories` merged clean. It fails today: three unmaintained crates, none of them ignored. The job now runs `advisories bans licenses sources` and is renamed to Cargo Deny (advisories, licenses, bans, sources). A job whose name omits what it checks is the same defect one level up, and the old name was honest — the gap was that nothing else gated advisories either. `deny` IS in CI Gate's needs, so advisory checking is now blocking rather than absent. THE VERSION SKEW WAS THE ACTUAL RISK. CI pinned cargo-deny 0.16.4 while this policy was verified on 0.20.2, which treats `unmaintained` as an ERROR by default. Adding the check while pinning a version whose behaviour was never tested is how a gate passes vacuously, so CI is bumped to 0.20.2 — the version the behaviour was measured on. Root cause of all three failures is one chain: gluesql-core -> im -> sized-chunks -> bitmaps, reached through the default-on `sql` feature that ships `rivet sql`. REQ-202's minimal --no-default-features build excludes it entirely. Accepted rather than fixed, with the reason written into deny.toml: gluesql has not moved off im, and swapping the SQL engine is far larger than the exposure warrants. TWO THINGS FOUND WHILE DOING IT. The RUSTSEC ids were GUESSED on the first attempt and were wrong. cargo-deny answered with advisory-not-encountered warnings while the three errors persisted, so the ids were read from the tool's own output instead: 2026-0247 bitmaps, 0248 im, 0251 sized-chunks. An invented identifier in an ignore list silences nothing while appearing to, which is the failure this change is about. Larger: TWELVE OF THE SIXTEEN EXISTING IGNORES WERE DEAD. RUSTSEC-2026-0085 through 0096 and 0103 all matched nothing in the tree — resolved by the wasmtime 44->45 bump and by salsa moving off the affected thin-vec. Their rationale comments still read "upgrade to wasmtime >=43 when available" and "remove when salsa >= 0.27 lands": instructions for work already done. So the section was dead in a second sense beyond not being run — three quarters of it recorded history rather than decisions. All twelve removed, leaving four live entries each with a current reason. Confirmed with cargo deny check advisories exit 0, zero errors and zero advisory-not-encountered warnings; the full four-check command exit 0 reporting advisories ok, bans ok, licenses ok, sources ok. Negative-controlled: removing the im ignore reddens with exit 1 and the unmaintained error, restoring it returns exit 0. Also files REQ-351 — the liveness probe has fired 95 times, tripping nightly on the 16-shard mutation matrix against 4 lean-mem runners. #933 confirms REQ-342 and REQ-343 both working in production: the alert now says the runner list could not be read and claims no cause, where the old code would have announced the pool OFFLINE while it was online=12 busy=11. Confirmed with fmt 0, clippy 1.97.0 --all-targets -D warnings 0, cargo test --workspace 0 (2368 passed), cargo test -p rivet-cli --test cli_commands 0 (213 passed), rivet validate 0, rivet docs check 0, yamllint 0, diagnose_test.sh 0, cargo deny full 0. Fixes: REQ-347
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
📐 Rivet artifact delta
Graphgraph LR
REQ_347["REQ-347"]:::modified
REQ_351["REQ-351"]:::added
classDef added fill:#d4edda,stroke:#28a745,color:#155724
classDef removed fill:#f8d7da,stroke:#dc3545,color:#721c24
classDef modified fill:#fff3cd,stroke:#ffc107,color:#856404
classDef overflow fill:#e2e3e5,stroke:#6c757d,color:#495057,stroke-dasharray: 3 3
Added
Modified
Posted by |
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
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.
The Cargo Deny job ran
cargo deny check bans licenses sources. The advisories check was not run at all — so deny.toml's entire[advisories]section, every reasoned RUSTSEC ignore, was configuration the job never consulted.cargo-auditexists but sits outsideCI Gate's needs, so a tree failingcargo deny check advisoriesmerged clean.It fails today: three unmaintained crates, none ignored.
Change
Now runs
advisories bans licenses sources, renamed to Cargo Deny (advisories, licenses, bans, sources) — a job whose name omits what it checks is the same defect one level up.denyis inCI Gate's needs, so advisory checking is now blocking.The version skew was the actual risk. CI pinned
0.16.4; the policy was verified on0.20.2, which treatsunmaintainedas an error by default. Adding the check while pinning an untested version is how a gate passes vacuously — bumped to 0.20.2.Root cause is one chain:
gluesql-core → im → sized-chunks → bitmaps, via the default-onsqlfeature. Accepted with the reason in deny.toml;--no-default-features(REQ-202) excludes it entirely.Two things found doing it
The RUSTSEC ids were guessed and wrong. cargo-deny answered
advisory-not-encounteredwhile the errors persisted; the real ids came from the tool:2026-0247/0248/0251. An invented id silences nothing while appearing to — the failure this change is about.12 of the 16 existing ignores were dead.
RUSTSEC-2026-0085…0096and0103matched nothing — resolved by the wasmtime 44→45 bump and salsa moving off thin-vec. Their comments still read "upgrade to wasmtime ≥43 when available" and "remove when salsa ≥0.27 lands": instructions for work already done. Three-quarters of the section recorded history, not decisions. Removed; four live entries remain, each with a current reason.Verification
Negative control: removing the
imignore → exit 1 with the unmaintained error; restoring → exit 0.fmt/clippy -D warnings(1.97.0)cargo test --workspacecargo test -p rivet-cli --test cli_commandsvalidate/docs check/yamllint/diagnose_test.shAlso files REQ-351 — the probe has fired 95 times, tripping nightly on the 16-shard matrix against 4
lean-memrunners. #933 confirms REQ-342 and REQ-343 working in production: the alert now says the runner list could not be read and claims no cause, where the old code would have announced the pool OFFLINE while it wasonline=12 busy=11.Fixes: REQ-347