Skip to content

Fail CI on Rust advisories, and on a skipped Rust check - #450

Merged
btsouth merged 1 commit into
mainfrom
ci/rust-advisory-audit
Sep 18, 2026
Merged

btsouth merged 1 commit into
mainfrom
ci/rust-advisory-audit

Conversation

@btsouth

@btsouth btsouth commented Sep 18, 2026

Copy link
Copy Markdown
Owner

RUSTSEC-2026-0285 (rustls) and RUSTSEC-2026-0258 (h2) both shipped in a released build and neither was visible from CI. Nothing here ran cargo audit, so main stayed green while the lockfile carried them, and they were only found by looking by hand. This adds the gate.

What changes

New audit job (Rust / advisory audit, ubuntu, 15 minute timeout). It installs cargo-audit through taiki-e/install-action pinned to a full commit SHA with the version comment, as the other third-party actions here are, and runs cargo audit --file Cargo.lock at the workspace root, which covers both manifests from the single root lockfile.

Only vulnerabilities fail it. The advisory categories that are informational (unmaintained, unsound, yanked) do not, so the 11 currently open against this graph keep reporting without blocking a pull request: fxhash, proc-macro-error, the unic-* set, event-listener, glib, rand 0.7. A real vulnerability has to be bumped, or waived with an explicit --ignore next to a comment saying why, which is the shape a genuine lockfile-only entry would take.

The Rust aggregate now includes it, and fails instead of skipping. It was needs: [rust-shared, rust-desktop] with a bare echo. A job that is skipped because a job it needs failed reports as skipped rather than failed, so a red Rust / shared or Rust / desktop left the required Rust check looking satisfied. It now runs if: always() and fails unless all three jobs, including the audit, report success.

Rust is already a required status check on main, so the audit blocks merges without any repository settings change.

Verification

cargo audit --file Cargo.lock        exit 0, warning: 11 allowed warnings found
cargo audit --file Cargo.lock        (the same command the job runs, from the repo root)
python3 yaml.safe_load(ci.yml)       jobs: frontend, rust-shared, rust-desktop, audit, rust
                                     audit steps: checkout, install-action, cargo audit
                                     rust needs: [rust-shared, rust-desktop, audit], if: always()

The aggregate's shell logic was exercised both ways: all-success passes, and one failure result exits 1 with the error annotation, so the required check goes red rather than quiet.

Expect this to bite once

The first time a vulnerability is published against a crate this repo already pins, main goes red and stays red until it is bumped or waived. That is the point, and it is the difference between this and what happened with rustls and h2.

Note

Add cargo audit job to CI and fail Rust aggregate status on skipped checks

  • Adds an audit job to the CI workflow that runs cargo audit --file Cargo.lock with a 15-minute timeout, failing on unignored vulnerability advisories.
  • Updates the Rust aggregate-status job in ci.yml to run with if: always(), depend on rust-shared, rust-desktop, and audit, and fail unless all three report success.
  • Replaces the previous unconditional success message with a shell check that logs each job's result and exits non-zero on any non-success result.
  • Behavioral Change: the Rust check now fails when cargo audit finds advisories or when any required Rust job is skipped; previously it always reported success.

Macroscope summarized d20dcd2.

Summary by CodeRabbit

  • Chores
    • Added automated vulnerability auditing for Rust dependencies.
    • Updated Rust validation checks to fail when dependency auditing or other required Rust checks fail.
    • Added clearer enforcement of successful completion across all Rust checks.

@coderabbitai

coderabbitai Bot commented Sep 18, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: bc88af15-40c9-44e1-ab3d-b9109ff6e725

📥 Commits

Reviewing files that changed from the base of the PR and between a08d438 and d20dcd2.

📒 Files selected for processing (1)
  • .github/workflows/ci.yml

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The workflow adds a Rust advisory audit job. The aggregate Rust job now waits for the shared, desktop, and audit jobs and fails unless all three succeed.

Changes

Rust CI validation

Layer / File(s) Summary
Advisory audit job
.github/workflows/ci.yml
Adds a job that installs cargo-audit and runs cargo audit --file Cargo.lock.
Rust result enforcement
.github/workflows/ci.yml
Updates the aggregate rust job to run after all Rust jobs and fail unless each job result is success.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Feature

Suggested reviewers: finesssee

Sequence Diagram(s)

sequenceDiagram
  participant RustShared
  participant RustDesktop
  participant AuditJob
  participant RustAggregate
  RustShared-->>RustAggregate: provide result
  RustDesktop-->>RustAggregate: provide result
  AuditJob-->>RustAggregate: provide result
  RustAggregate->>RustAggregate: fail unless all results are success
Loading

Merge Risk: ⚪ Minimal · up to d20dc

The Rust advisory audit and aggregate check should correctly report dependency failures, skips, and cancellations under normal workflow execution.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes both primary changes: failing CI for Rust advisories and failing the aggregate check when a Rust job is skipped.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cloudflare-workers-and-pages

Copy link
Copy Markdown
Contributor

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
ceiling d20dcd2 Commit Preview URL

Branch Preview URL
Sep 18 2026, 12:49 AM

@btsouth
btsouth merged commit 186f155 into main Sep 18, 2026
11 checks passed
@btsouth
btsouth deleted the ci/rust-advisory-audit branch September 18, 2026 00:58
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