From 7681313161fd8b6e8c05b734505608d2b5a905d0 Mon Sep 17 00:00:00 2001 From: Jaro-c <75870284+Jaro-c@users.noreply.github.com> Date: Mon, 12 Oct 2026 17:25:53 -0500 Subject: [PATCH 1/8] ci: audit main and develop daily, accept RUSTSEC-2024-0449, and open an issue when the scheduled audit fails Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> --- .cargo/audit.toml | 7 + .github/workflows/audit.yml | 109 ++++++-- .github/workflows/ci.yml | 5 +- .github/workflows/reusable-rust-audit.yml | 17 +- deny.toml | 10 +- tests/audit_ignore_lists.rs | 295 ++++++++++++++++++++++ 6 files changed, 419 insertions(+), 24 deletions(-) create mode 100644 .cargo/audit.toml create mode 100644 tests/audit_ignore_lists.rs diff --git a/.cargo/audit.toml b/.cargo/audit.toml new file mode 100644 index 00000000..7a8a6fe6 --- /dev/null +++ b/.cargo/audit.toml @@ -0,0 +1,7 @@ +# cargo-audit's ignore list. cargo-deny reads deny.toml; cargo-audit reads +# this file, relative to the directory it runs in, and release.yml runs it +# from the repository root with --deny warnings. The reason for each entry +# is written in deny.toml. tests/audit_ignore_lists.rs keeps the two equal. + +[advisories] +ignore = ["RUSTSEC-2024-0449"] diff --git a/.github/workflows/audit.yml b/.github/workflows/audit.yml index aa1bd4bb..0ef6b9cc 100644 --- a/.github/workflows/audit.yml +++ b/.github/workflows/audit.yml @@ -1,31 +1,37 @@ name: Security audit -# Two triggers answering two different questions, and the split is the whole -# point rather than a convenience. +# Three triggers, three questions, and the split is the whole point. # -# The weekly cron audits the WHOLE tree against an advisory database that moves -# on somebody else's schedule. A finding there belongs to a third party: RustSec -# published something today about a crate nobody here touched. That must not be -# able to block a pull request, which is why this workflow was cron-only, and -# `standards/testing` says so in as many words: "a finding that belongs to a -# third party must NOT fail the run, or someone else's disclosure timing blocks -# every pull request in the organisation". +# The daily schedule audits the WHOLE tree on both branches against an +# advisory database that moves on somebody else's schedule. A finding +# there belongs to a third party: RustSec published something today +# about a crate nobody here touched. That must not be able to block a +# pull request, which is why this workflow kept pull requests off the +# third-party-timing lane, and `standards/testing` says so in as many +# words: "a finding that belongs to a third party must NOT fail the run, +# or someone else's disclosure timing blocks every pull request in the +# organisation". # -# The pull-request trigger asks the other question: does THIS change introduce a -# dependency with a known advisory? That finding belongs to the pull request, and -# blocking is the right answer. Scoping it to the manifest and the lockfile is -# what keeps the two apart: a pull request that touches neither cannot introduce -# a dependency, so upstream's timing reaches it through the cron and not through -# a red check it did not cause. +# The pull-request trigger asks the other question: does THIS change +# introduce a dependency with a known advisory? That finding belongs to +# the pull request, and blocking is the right answer. Scoping it to the +# manifest, the lockfile and the two ignore lists is what keeps the two +# apart: a pull request that touches none of them cannot introduce a +# dependency, so upstream's timing reaches it through the cron and not +# through a red check it did not cause. # -# The residue, stated rather than discovered: a pull request that does change the -# lockfile on the day an unrelated advisory lands is blocked by that advisory. -# That is the narrow case where the two questions overlap, and it lands on the -# pull request that is changing dependencies, which is where someone is already -# looking at them. +# The residue, stated rather than discovered: a pull request that does +# change the lockfile on the day an unrelated advisory lands is blocked +# by that advisory. That is the narrow case where the two questions +# overlap, and it lands on the pull request that is changing +# dependencies, which is where someone is already looking at them. +# +# The schedule runs daily on main and on develop (#2034). On 2026-10-12 +# the weekly run on main went red and nothing said so; the report job +# below opens an issue when a scheduled audit fails. on: schedule: - - cron: "0 6 * * 1" # Every Monday at 06:00 UTC + - cron: "0 6 * * *" # Daily at 06:00 UTC workflow_dispatch: pull_request: branches: @@ -35,6 +41,7 @@ on: - "Cargo.toml" - "Cargo.lock" - "deny.toml" + - ".cargo/audit.toml" - ".github/workflows/audit.yml" - ".github/workflows/reusable-rust-audit.yml" @@ -42,5 +49,65 @@ permissions: contents: read jobs: + # Kept as audit so its check names stay the same. audit: uses: ./.github/workflows/reusable-rust-audit.yml + + # A schedule only runs on the default branch, so without this develop + # is never audited on a schedule. Pull requests audit their own tree + # through the job above. + audit-develop: + if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' + uses: ./.github/workflows/reusable-rust-audit.yml + with: + ref: develop + + # Runs only when a scheduled audit failed. issues: write is granted + # to this job alone; nothing here checks out code or installs tools, + # and every value reaches the script through env. + report: + name: open an issue when the scheduled audit fails + runs-on: ubuntu-latest + needs: [audit, audit-develop] + if: always() && github.event_name == 'schedule' && (needs.audit.result == 'failure' || needs.audit-develop.result == 'failure') + permissions: + issues: write + # Bound: no runs measured yet, so the ten-minute floor every job + # here uses. + timeout-minutes: 10 + env: + GH_TOKEN: ${{ github.token }} + GH_REPO: ${{ github.repository }} + AUDIT_RESULT: ${{ needs.audit.result }} + AUDIT_DEVELOP_RESULT: ${{ needs.audit-develop.result }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + ISSUE_TITLE: Scheduled dependency audit failed + steps: + - name: Open a single tracking issue and update it on each failure + run: | + set -euo pipefail + NUMBER=$(gh issue list --state open --search "in:title \"$ISSUE_TITLE\"" --json number,title \ + --jq 'map(select(.title == env.ISSUE_TITLE)) | .[0].number // empty') + DATE=$(date -u +%Y-%m-%dT%H:%M:%SZ) + BRANCHES="" + if [ "$AUDIT_RESULT" = "failure" ]; then + BRANCHES="${BRANCHES}main, " + fi + if [ "$AUDIT_DEVELOP_RESULT" = "failure" ]; then + BRANCHES="${BRANCHES}develop, " + fi + BRANCHES="${BRANCHES%, }" + if [ -n "$NUMBER" ]; then + gh issue comment "$NUMBER" --body "Failed again on ${DATE} on: ${BRANCHES}. Run: ${RUN_URL}" + echo "Commented on #${NUMBER}." + exit 0 + fi + gh issue create \ + --title "$ISSUE_TITLE" \ + --label "type:security" --label "prio:P1" --label "area:ci" --label "status:triage" \ + --body "The scheduled dependency audit failed on ${DATE} on: ${BRANCHES}. + + Run: ${RUN_URL} + + Close this once the scheduled audit is green on both branches. While this stays open, each new failure adds a comment here instead of opening another issue." + echo "Opened new issue." diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2261bd55..80f4a041 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -120,7 +120,10 @@ jobs: uses: ./.github/workflows/reusable-schedule-freshness.yml with: workflow: audit.yml - max-age-days: 15 + # audit.yml runs daily. GitHub's cron has been measured arriving + # between 2h26 and 5h35 late, so three days absorbs a late or a + # skipped firing without hiding a schedule that stopped. + max-age-days: 3 freshness-fuzz: if: github.event_name != 'merge_group' diff --git a/.github/workflows/reusable-rust-audit.yml b/.github/workflows/reusable-rust-audit.yml index 30c66de0..d8106dce 100644 --- a/.github/workflows/reusable-rust-audit.yml +++ b/.github/workflows/reusable-rust-audit.yml @@ -25,12 +25,23 @@ on: description: Exact version of cargo-deny to install; bumped by editing this file type: string default: "0.20.2" + # Empty means actions/checkout's default, the event's ref. + # audit.yml passes develop so the daily schedule, which always + # runs on main, audits develop as well. + ref: + description: Branch or tag to check out; empty means actions/checkout's own default (the event's ref). + type: string + default: "" permissions: contents: read concurrency: - group: rust-audit-${{ github.workflow }}-${{ github.ref }}-${{ inputs.working-directory }} + # inputs.ref is part of the key: a schedule carries refs/heads/main + # as github.ref whichever branch a caller audits, so without it the + # main and develop audits of one run would share a group and cancel + # each other. + group: rust-audit-${{ github.workflow }}-${{ github.ref }}-${{ inputs.ref }}-${{ inputs.working-directory }} cancel-in-progress: true jobs: @@ -49,6 +60,8 @@ jobs: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false + # See the ref input. + ref: ${{ inputs.ref }} - name: Install toolchain env: @@ -95,6 +108,8 @@ jobs: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false + # See the ref input. + ref: ${{ inputs.ref }} - name: Install toolchain env: diff --git a/deny.toml b/deny.toml index be4b3b90..0746e0c5 100644 --- a/deny.toml +++ b/deny.toml @@ -10,7 +10,15 @@ all-features = true [advisories] version = 2 yanked = "deny" -ignore = [] +# RUSTSEC-2024-0449: unsafe-libyaml 0.2.11 is unmaintained. It is not a +# known vulnerability. It comes in through serde_yaml_ng 0.10.0, the latest +# release, so no bump clears it. What it parses is the compose file of the +# person running podup. Moving off it is #2035, and that pull request +# removes this entry and the one in .cargo/audit.toml. +# tests/audit_ignore_lists.rs keeps the two lists equal. +ignore = [ + { id = "RUSTSEC-2024-0449", reason = "unmaintained, not a known vulnerability; pulled by serde_yaml_ng 0.10.0, the latest release; see #2035" }, +] [licenses] version = 2 diff --git a/tests/audit_ignore_lists.rs b/tests/audit_ignore_lists.rs new file mode 100644 index 00000000..8f168da2 --- /dev/null +++ b/tests/audit_ignore_lists.rs @@ -0,0 +1,295 @@ +//! `cargo audit` and `cargo deny` agree on which RUSTSEC ids are ignored. +//! +//! `deny.toml` carries the cargo-deny ignore list and `.cargo/audit.toml` +//! carries the cargo-audit one. cargo-deny reads `deny.toml` and cargo-audit +//! reads `.cargo/audit.toml`, relative to the directory it runs in. The +//! release job (release.yml) runs `cargo audit --deny warnings` from the +//! repository root, so an ignore that lives only in deny.toml does not help +//! the release gate: the next release would fail on the same advisory that +//! turned the scheduled audit red on 2026-10-12 (#2034). The two lists must +//! be equal. +//! +//! No new crate dependency is taken on for this. `toml` is not in the +//! workspace's lock today, and pulling it just to compare two short string +//! lists is more weight than the assertion needs. The parser is a hand-rolled +//! scan for `id = "..."` (deny.toml's table form) and `ignore = [ "..." ]` +//! (cargo-audit's list form); both shapes are stable per the input files +//! they target. + +use std::fs; +use std::path::Path; + +fn read_repo(rel: &str) -> String { + let path = Path::new(env!("CARGO_MANIFEST_DIR")).join(rel); + fs::read_to_string(&path).unwrap_or_else(|e| panic!("{rel} is readable: {e}")) +} + +/// Extract the set of advisory ids in `body` whose shape is a deny.toml +/// table entry: `id = "RUSTSEC-..."`. The line may be a full inline +/// table (`{ id = "X", reason = "..." }`) or just the field (`id = "X"`), +/// so the parser scans for `id = "..."` anywhere on the line. Comment +/// lines and lines without the field are skipped; a future refactor +/// that introduces an unrelated `id = "..."` field on a different key +/// would also be rejected only when its value starts with `RUSTSEC-`, +/// which is the convention the file's own entries follow. +fn deny_toml_ids(body: &str) -> Vec { + let mut out = Vec::new(); + for line in body.lines() { + let trimmed = line.trim_start(); + // Comment and blank lines: skip. Comment lines mention ids in + // prose and an inline comment that just happens to mention + // one would otherwise be picked up. Same shape as the parsers + // in `tests/workflow_audit_strictness.rs`. + if trimmed.is_empty() || trimmed.starts_with('#') { + continue; + } + // Find every `id = "RUSTSEC-..."` token on the line. The line + // may also be the inline-table form `{ id = "X", ... }`, in + // which case the scan still starts at `id` and reads forward + // past the `=` to the quoted string. + let bytes = trimmed.as_bytes(); + let mut i = 0; + while i < bytes.len() { + if bytes[i] != b'i' { + i += 1; + continue; + } + if !trimmed[i..].starts_with("id") { + i += 1; + continue; + } + let after_id_idx = i + 2; + if after_id_idx < bytes.len() && !matches!(bytes[after_id_idx], b' ' | b'=') { + i += 1; + continue; + } + let mut j = after_id_idx; + while j < bytes.len() && bytes[j] == b' ' { + j += 1; + } + if j >= bytes.len() || bytes[j] != b'=' { + i += 1; + continue; + } + j += 1; + while j < bytes.len() && bytes[j] == b' ' { + j += 1; + } + if j >= bytes.len() { + break; + } + let quote = bytes[j]; + if quote != b'"' && quote != b'\'' { + i += 1; + continue; + } + j += 1; + let start = j; + while j < bytes.len() && bytes[j] != quote { + j += 1; + } + if j >= bytes.len() { + break; + } + let token = &trimmed[start..j]; + if token.starts_with("RUSTSEC-") { + out.push(token.to_string()); + } + i = j + 1; + } + } + out +} + +/// Extract the set of advisory ids in `body` whose shape is the +/// cargo-audit list form: `ignore = [ "RUSTSEC-...", "..." ]`. Strings +/// inside `[]` are matched one at a time; commas and trailing whitespace +/// are tolerated because that is the form `.cargo/audit.toml` already +/// uses. Lines outside the `ignore = [...]` declaration are not parsed +/// here either: the file is small and a stray RUSTSEC id elsewhere would +/// still belong in `ignore`, so the question "is X in the ignore list?" +/// either way points at the same place. +/// +/// The inline single-line form (`ignore = ["X", "Y"]`) is read in one +/// pass: the `]` may appear on the same line that holds the entries, +/// so the parser cannot rely on a line-by-line open/close. It tracks +/// bracket nesting instead: an `[` that is not inside a string opens +/// the list, a matching `]` closes it, and quoted strings inside the +/// brackets are not counted as brackets. Multi-line lists behave the +/// same way, with the `[` on one line and the matching `]` later. +fn audit_toml_ids(body: &str) -> Vec { + let mut out = Vec::new(); + let mut depth: i32 = 0; + for line in body.lines() { + // Walk the line character by character. Outside the list + // (`depth == 0`) we skip everything except a `[` that opens + // the ignore list; inside it (depth > 0) we collect every + // quoted `RUSTSEC-...` token. + let bytes = line.as_bytes(); + let mut i = 0; + while i < bytes.len() { + let c = bytes[i]; + // A quoted token. Single or double quotes; the inside of + // a quoted string is opaque, so brackets inside a string + // do not count as brackets, and `=` inside a string does + // not count as the assignment. + if c == b'"' || c == b'\'' { + let quote = c; + let start = i + 1; + let mut j = start; + while j < bytes.len() && bytes[j] != quote { + j += 1; + } + if j < bytes.len() { + if depth > 0 { + let token = &line[start..j]; + if token.starts_with("RUSTSEC-") { + out.push(token.to_string()); + } + } + i = j + 1; + continue; + } + } + // A `[` inside or outside the list changes the bracket + // nesting: outside, an opening `[` enters the list only + // when the line was `ignore = [ ... ]` (i.e. depth was + // zero and we are tracking `ignore`-list entries). To + // keep the parser robust against the multiline form + // (`ignore = [\n "X",\n "Y",\n]`) the scan tracks + // `depth` rather than a boolean: a `[` already inside an + // `ignore = [...]` increments the depth, and a `]` + // decrements it back to zero. + if c == b'[' { + depth += 1; + i += 1; + continue; + } + if c == b']' { + if depth > 0 { + depth -= 1; + } + i += 1; + continue; + } + i += 1; + } + } + out +} + +/// `audit_toml_ids` once used a `first_id_token` helper for the +/// single-line form. The rewritten parser above is bracket-depth aware +/// and reads every quoted id on a line in one pass, so the helper is +/// gone: keep this comment so a future reader who lands on a `git +/// log -p` and wonders why a private helper existed finds the answer. + +#[test] +fn the_two_ignore_lists_agree_on_advisory_ids() { + let deny_ids: Vec = deny_toml_ids(&read_repo("deny.toml")); + let audit_ids: Vec = audit_toml_ids(&read_repo(".cargo/audit.toml")); + // Order does not matter: the contract is the SET of ids each tool + // accepts. `BTreeSet` also makes the failure message stable. + let deny_set: std::collections::BTreeSet<_> = deny_ids.iter().collect(); + let audit_set: std::collections::BTreeSet<_> = audit_ids.iter().collect(); + assert_eq!( + deny_set, audit_set, + "deny.toml and .cargo/audit.toml must list the same set of ignored \ + advisories. deny.toml: {:?}, .cargo/audit.toml: {:?}. The release \ + gate (release.yml) runs cargo-audit, which reads .cargo/audit.toml \ + only; a difference here is a difference in what each tool approves.", + deny_ids, audit_ids + ); +} + +/// Pin the parser on input it must accept. Today the file format is +/// `{ id = "X", reason = "..." }` for deny.toml and `ignore = ["X", "Y"]` +/// for .cargo/audit.toml; the parser is what every other test in this +/// file trusts, and a parser that always returned one canonical id would +/// satisfy the assertions above against a tree where the lists had +/// drifted. +#[test] +fn the_parser_reads_what_it_claims() { + // deny.toml table form: one id, exactly one entry, no comments. + let deny_body = "\ +[advisories] +ignore = [ + { id = \"RUSTSEC-0000-0000\", reason = \"x\" }, +] +"; + let deny_ids = deny_toml_ids(deny_body); + assert_eq!( + deny_ids, + vec!["RUSTSEC-0000-0000".to_string()], + "deny.toml table form: exactly one id, got {deny_ids:?}" + ); + + // A comment above the entry that mentions the id must not be counted. + let deny_with_comment = "\ +[advisories] +# ignore = [{ id = \"RUSTSEC-9999-9999\", reason = \"comment only\" }] +ignore = [{ id = \"RUSTSEC-0000-0000\", reason = \"real\" }] +"; + let ids = deny_toml_ids(deny_with_comment); + assert_eq!( + ids, + vec!["RUSTSEC-0000-0000".to_string()], + "deny.toml parser picked up an id from a comment line, got {ids:?}" + ); + + // cargo-audit list form: a multi-line array with more than one id. + let audit_body = "\ +[advisories] +ignore = [ + \"RUSTSEC-0000-0000\", + \"RUSTSEC-1111-2222\", +] +"; + let audit_ids = audit_toml_ids(audit_body); + assert_eq!( + audit_ids, + vec![ + "RUSTSEC-0000-0000".to_string(), + "RUSTSEC-1111-2222".to_string() + ], + "cargo-audit multi-line list form: both ids parsed, got {audit_ids:?}" + ); + + // cargo-audit single-line list form: both ids on the same line. + let audit_one_line = "\ +[advisories] +ignore = [\"RUSTSEC-0000-0000\", \"RUSTSEC-1111-2222\"] +"; + let ids_inline = audit_toml_ids(audit_one_line); + assert_eq!( + ids_inline, + vec![ + "RUSTSEC-0000-0000".to_string(), + "RUSTSEC-1111-2222".to_string() + ], + "cargo-audit single-line form: both ids parsed, got {ids_inline:?}" + ); + + // The negative control: a deny.toml fixture where the two parsers + // disagree. The parser pair, not the input set, is what the + // agreement test depends on: a parser that hardcoded the + // canonical set would pass every assertion above against any + // well-formed file, including a drifted one. + let deny_differs = "\ +[advisories] +ignore = [{ id = \"RUSTSEC-9999-9999\", reason = \"x\" }] +"; + let audit_differs = "\ +[advisories] +ignore = [\"RUSTSEC-0000-0000\"] +"; + let d: std::collections::BTreeSet<_> = deny_toml_ids(deny_differs).into_iter().collect(); + let a: std::collections::BTreeSet<_> = audit_toml_ids(audit_differs).into_iter().collect(); + assert_ne!( + d, a, + "the negative fixture did not disagree between parsers; the \ + agreement test would have passed against drifted lists, which \ + is the failure mode the inline case is here to prove the \ + parsers can catch. deny: {d:?}, audit: {a:?}" + ); +} From 6fd43bb0d0825f36cd23c0569a2b323b65715bff Mon Sep 17 00:00:00 2001 From: Jaro-c <75870284+Jaro-c@users.noreply.github.com> Date: Mon, 12 Oct 2026 17:26:52 -0500 Subject: [PATCH 2/8] ci: report a cancelled or timed-out scheduled audit too Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> --- .github/workflows/audit.yml | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/.github/workflows/audit.yml b/.github/workflows/audit.yml index 0ef6b9cc..df0228ee 100644 --- a/.github/workflows/audit.yml +++ b/.github/workflows/audit.yml @@ -62,14 +62,16 @@ jobs: with: ref: develop - # Runs only when a scheduled audit failed. issues: write is granted + # Runs when a scheduled audit did not succeed: a failure, and also a + # cancelled or timed-out run, which would otherwise pass in silence. + # issues: write is granted # to this job alone; nothing here checks out code or installs tools, # and every value reaches the script through env. report: name: open an issue when the scheduled audit fails runs-on: ubuntu-latest needs: [audit, audit-develop] - if: always() && github.event_name == 'schedule' && (needs.audit.result == 'failure' || needs.audit-develop.result == 'failure') + if: always() && github.event_name == 'schedule' && (needs.audit.result != 'success' || needs.audit-develop.result != 'success') permissions: issues: write # Bound: no runs measured yet, so the ten-minute floor every job @@ -90,11 +92,11 @@ jobs: --jq 'map(select(.title == env.ISSUE_TITLE)) | .[0].number // empty') DATE=$(date -u +%Y-%m-%dT%H:%M:%SZ) BRANCHES="" - if [ "$AUDIT_RESULT" = "failure" ]; then - BRANCHES="${BRANCHES}main, " + if [ "$AUDIT_RESULT" != "success" ]; then + BRANCHES="${BRANCHES}main (${AUDIT_RESULT}), " fi - if [ "$AUDIT_DEVELOP_RESULT" = "failure" ]; then - BRANCHES="${BRANCHES}develop, " + if [ "$AUDIT_DEVELOP_RESULT" != "success" ]; then + BRANCHES="${BRANCHES}develop (${AUDIT_DEVELOP_RESULT}), " fi BRANCHES="${BRANCHES%, }" if [ -n "$NUMBER" ]; then From 5a94f0f4a7a0c88de3beee631d5c5ab9d33b294b Mon Sep 17 00:00:00 2001 From: Jaro-c <75870284+Jaro-c@users.noreply.github.com> Date: Mon, 12 Oct 2026 17:45:25 -0500 Subject: [PATCH 3/8] ci: keep the cache out of audits of another ref, and leave the freshness bound at 15 days Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> --- .github/workflows/ci.yml | 5 +---- .github/workflows/reusable-rust-audit.yml | 13 +++++++++++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 80f4a041..2261bd55 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -120,10 +120,7 @@ jobs: uses: ./.github/workflows/reusable-schedule-freshness.yml with: workflow: audit.yml - # audit.yml runs daily. GitHub's cron has been measured arriving - # between 2h26 and 5h35 late, so three days absorbs a late or a - # skipped firing without hiding a schedule that stopped. - max-age-days: 3 + max-age-days: 15 freshness-fuzz: if: github.event_name != 'merge_group' diff --git a/.github/workflows/reusable-rust-audit.yml b/.github/workflows/reusable-rust-audit.yml index d8106dce..7c0695cb 100644 --- a/.github/workflows/reusable-rust-audit.yml +++ b/.github/workflows/reusable-rust-audit.yml @@ -70,7 +70,14 @@ jobs: rustup toolchain install "$TOOLCHAIN" --profile minimal rustup default "$TOOLCHAIN" - - uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + # No cache when the caller picks another ref. The schedule runs in + # main's cache scope, which every branch reads, and cargo here runs + # over the other ref's tree (cargo deny calls cargo metadata, which + # honours that tree's .cargo/config.toml). Installing from scratch + # costs minutes once a day; a cache written from that tree would be + # read by every later run. CodeQL flagged it on #2036. + - if: inputs.ref == '' + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: path: | ~/.cargo/bin @@ -118,7 +125,9 @@ jobs: rustup toolchain install "$TOOLCHAIN" --profile minimal rustup default "$TOOLCHAIN" - - uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + # Same no-cache rule as the cargo-audit job above. + - if: inputs.ref == '' + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: path: | ~/.cargo/bin From 4e84a722e9934b68c525676ea952519b5d613c22 Mon Sep 17 00:00:00 2001 From: Jaro-c <75870284+Jaro-c@users.noreply.github.com> Date: Mon, 12 Oct 2026 18:00:30 -0500 Subject: [PATCH 4/8] ci: audit develop in its own run instead of checking it out from main's Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> --- .github/workflows/audit.yml | 52 ++++++++++++++--------- .github/workflows/reusable-rust-audit.yml | 30 ++----------- 2 files changed, 35 insertions(+), 47 deletions(-) diff --git a/.github/workflows/audit.yml b/.github/workflows/audit.yml index df0228ee..d2ef198b 100644 --- a/.github/workflows/audit.yml +++ b/.github/workflows/audit.yml @@ -26,9 +26,9 @@ name: Security audit # overlap, and it lands on the pull request that is changing # dependencies, which is where someone is already looking at them. # -# The schedule runs daily on main and on develop (#2034). On 2026-10-12 -# the weekly run on main went red and nothing said so; the report job -# below opens an issue when a scheduled audit fails. +# The schedule audits main and starts a run of this workflow on develop; +# the report job below opens an issue when either does not succeed +# (#2034). on: schedule: - cron: "0 6 * * *" # Daily at 06:00 UTC @@ -53,25 +53,36 @@ jobs: audit: uses: ./.github/workflows/reusable-rust-audit.yml - # A schedule only runs on the default branch, so without this develop - # is never audited on a schedule. Pull requests audit their own tree - # through the job above. + # A schedule only runs on the default branch. Checking develop out + # inside this run would execute develop's tree in main's context, + # where any step can write the cache every branch reads (CodeQL, + # #2036). So the schedule starts a separate run of this workflow on + # develop, which audits develop in develop's own context. audit-develop: - if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' - uses: ./.github/workflows/reusable-rust-audit.yml - with: - ref: develop + name: start the develop audit + if: github.event_name == 'schedule' + runs-on: ubuntu-latest + permissions: + actions: write + # Bound: no runs measured yet, so the ten-minute floor every job here uses. + timeout-minutes: 10 + env: + GH_TOKEN: ${{ github.token }} + GH_REPO: ${{ github.repository }} + steps: + - name: Dispatch audit.yml on develop + run: gh workflow run audit.yml --ref develop - # Runs when a scheduled audit did not succeed: a failure, and also a - # cancelled or timed-out run, which would otherwise pass in silence. - # issues: write is granted - # to this job alone; nothing here checks out code or installs tools, - # and every value reaches the script through env. + # Runs when this run's own audit did not succeed (a failure, a + # cancellation or a timeout), on either a schedule or a dispatch, and + # also when the schedule could not start the develop audit. issues: + # write is granted to this job alone; nothing here checks out code or + # installs tools, and every value reaches the script through env. report: name: open an issue when the scheduled audit fails runs-on: ubuntu-latest needs: [audit, audit-develop] - if: always() && github.event_name == 'schedule' && (needs.audit.result != 'success' || needs.audit-develop.result != 'success') + if: always() && ((github.event_name == 'schedule' && (needs.audit.result != 'success' || needs.audit-develop.result != 'success')) || (github.event_name == 'workflow_dispatch' && needs.audit.result != 'success')) permissions: issues: write # Bound: no runs measured yet, so the ten-minute floor every job @@ -80,8 +91,9 @@ jobs: env: GH_TOKEN: ${{ github.token }} GH_REPO: ${{ github.repository }} + REF_NAME: ${{ github.ref_name }} AUDIT_RESULT: ${{ needs.audit.result }} - AUDIT_DEVELOP_RESULT: ${{ needs.audit-develop.result }} + DISPATCH_RESULT: ${{ needs.audit-develop.result }} RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} ISSUE_TITLE: Scheduled dependency audit failed steps: @@ -93,10 +105,10 @@ jobs: DATE=$(date -u +%Y-%m-%dT%H:%M:%SZ) BRANCHES="" if [ "$AUDIT_RESULT" != "success" ]; then - BRANCHES="${BRANCHES}main (${AUDIT_RESULT}), " + BRANCHES="${BRANCHES}${REF_NAME} (${AUDIT_RESULT}), " fi - if [ "$AUDIT_DEVELOP_RESULT" != "success" ]; then - BRANCHES="${BRANCHES}develop (${AUDIT_DEVELOP_RESULT}), " + if [ "$DISPATCH_RESULT" != "success" ] && [ "$DISPATCH_RESULT" != "skipped" ]; then + BRANCHES="${BRANCHES}develop (could not start its audit: ${DISPATCH_RESULT}), " fi BRANCHES="${BRANCHES%, }" if [ -n "$NUMBER" ]; then diff --git a/.github/workflows/reusable-rust-audit.yml b/.github/workflows/reusable-rust-audit.yml index 7c0695cb..30c66de0 100644 --- a/.github/workflows/reusable-rust-audit.yml +++ b/.github/workflows/reusable-rust-audit.yml @@ -25,23 +25,12 @@ on: description: Exact version of cargo-deny to install; bumped by editing this file type: string default: "0.20.2" - # Empty means actions/checkout's default, the event's ref. - # audit.yml passes develop so the daily schedule, which always - # runs on main, audits develop as well. - ref: - description: Branch or tag to check out; empty means actions/checkout's own default (the event's ref). - type: string - default: "" permissions: contents: read concurrency: - # inputs.ref is part of the key: a schedule carries refs/heads/main - # as github.ref whichever branch a caller audits, so without it the - # main and develop audits of one run would share a group and cancel - # each other. - group: rust-audit-${{ github.workflow }}-${{ github.ref }}-${{ inputs.ref }}-${{ inputs.working-directory }} + group: rust-audit-${{ github.workflow }}-${{ github.ref }}-${{ inputs.working-directory }} cancel-in-progress: true jobs: @@ -60,8 +49,6 @@ jobs: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false - # See the ref input. - ref: ${{ inputs.ref }} - name: Install toolchain env: @@ -70,14 +57,7 @@ jobs: rustup toolchain install "$TOOLCHAIN" --profile minimal rustup default "$TOOLCHAIN" - # No cache when the caller picks another ref. The schedule runs in - # main's cache scope, which every branch reads, and cargo here runs - # over the other ref's tree (cargo deny calls cargo metadata, which - # honours that tree's .cargo/config.toml). Installing from scratch - # costs minutes once a day; a cache written from that tree would be - # read by every later run. CodeQL flagged it on #2036. - - if: inputs.ref == '' - uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + - uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: path: | ~/.cargo/bin @@ -115,8 +95,6 @@ jobs: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false - # See the ref input. - ref: ${{ inputs.ref }} - name: Install toolchain env: @@ -125,9 +103,7 @@ jobs: rustup toolchain install "$TOOLCHAIN" --profile minimal rustup default "$TOOLCHAIN" - # Same no-cache rule as the cargo-audit job above. - - if: inputs.ref == '' - uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + - uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: path: | ~/.cargo/bin From bd84220f9785907d22b77e5e785cfe72152ddb6f Mon Sep 17 00:00:00 2001 From: Jaro-c <75870284+Jaro-c@users.noreply.github.com> Date: Mon, 12 Oct 2026 18:29:46 -0500 Subject: [PATCH 5/8] ci: wait for the develop audit, report once per failure, and read both ignore list forms Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> --- .github/workflows/audit.yml | 62 ++++- tests/audit_ignore_lists.rs | 443 ++++++++++++++++++++---------------- 2 files changed, 292 insertions(+), 213 deletions(-) diff --git a/.github/workflows/audit.yml b/.github/workflows/audit.yml index d2ef198b..af9973b8 100644 --- a/.github/workflows/audit.yml +++ b/.github/workflows/audit.yml @@ -33,6 +33,11 @@ on: schedule: - cron: "0 6 * * *" # Daily at 06:00 UTC workflow_dispatch: + inputs: + from-schedule: + description: Set by the scheduled run on main when it starts the develop audit; that run reports for both branches + type: boolean + default: false pull_request: branches: - main @@ -57,32 +62,65 @@ jobs: # inside this run would execute develop's tree in main's context, # where any step can write the cache every branch reads (CodeQL, # #2036). So the schedule starts a separate run of this workflow on - # develop, which audits develop in develop's own context. + # develop, waits for it, and fails when that run does not succeed or + # never appears at all: a "queued forever" dispatch and a dispatch + # that returned a run id but exited non-zero both leave this job + # non-success, and the report job below then opens an issue for the + # develop branch instead of declaring it green by omission. audit-develop: name: start the develop audit if: github.event_name == 'schedule' runs-on: ubuntu-latest permissions: actions: write - # Bound: no runs measured yet, so the ten-minute floor every job here uses. - timeout-minutes: 10 + # Bound: the dispatched audit installs its tools and runs both + # checks; forty minutes covers that plus a queue, and no run has + # been measured yet. + timeout-minutes: 40 env: GH_TOKEN: ${{ github.token }} GH_REPO: ${{ github.repository }} steps: - - name: Dispatch audit.yml on develop - run: gh workflow run audit.yml --ref develop + - name: Dispatch audit.yml on develop and wait for it + run: | + set -euo pipefail + SINCE=$(date -u +%Y-%m-%dT%H:%M:%SZ) + gh workflow run audit.yml --ref develop -f from-schedule=true + ID= + for _ in $(seq 1 10); do + ID=$(gh run list --workflow audit.yml --branch develop --event workflow_dispatch --limit 5 --json databaseId,createdAt --jq "map(select(.createdAt >= \"$SINCE\")) | sort_by(.createdAt) | .[0].databaseId // empty") + if [ -n "$ID" ]; then + break + fi + sleep 15 + done + if [ -z "$ID" ]; then + echo "::error::the develop audit was dispatched but its run never appeared" + exit 1 + fi + if ! gh run watch "$ID" --exit-status --interval 30; then + echo "::error::the develop audit run $ID did not succeed; see $GITHUB_SERVER_URL/$GH_REPO/actions/runs/$ID" + exit 1 + fi + echo "Develop audit run: $GITHUB_SERVER_URL/$GH_REPO/actions/runs/$ID" - # Runs when this run's own audit did not succeed (a failure, a - # cancellation or a timeout), on either a schedule or a dispatch, and - # also when the schedule could not start the develop audit. issues: - # write is granted to this job alone; nothing here checks out code or - # installs tools, and every value reaches the script through env. + # Runs when this run's audit did not succeed (failure, cancellation or + # timeout). On the schedule it reports both branches, because the job + # above fails when develop's run fails or never appears. A develop run + # started by the schedule passes from-schedule and does not report + # itself; a manual dispatch reports its own branch. issues: write is + # granted to this job alone; nothing here checks out code or installs + # tools, and every value reaches the script through env. report: name: open an issue when the scheduled audit fails runs-on: ubuntu-latest needs: [audit, audit-develop] - if: always() && ((github.event_name == 'schedule' && (needs.audit.result != 'success' || needs.audit-develop.result != 'success')) || (github.event_name == 'workflow_dispatch' && needs.audit.result != 'success')) + # One reporter at a time across runs, so two of them cannot both + # find no open issue and both create one. + concurrency: + group: audit-report + cancel-in-progress: false + if: always() && ((github.event_name == 'schedule' && (needs.audit.result != 'success' || needs.audit-develop.result != 'success')) || (github.event_name == 'workflow_dispatch' && !inputs.from-schedule && needs.audit.result != 'success')) permissions: issues: write # Bound: no runs measured yet, so the ten-minute floor every job @@ -108,7 +146,7 @@ jobs: BRANCHES="${BRANCHES}${REF_NAME} (${AUDIT_RESULT}), " fi if [ "$DISPATCH_RESULT" != "success" ] && [ "$DISPATCH_RESULT" != "skipped" ]; then - BRANCHES="${BRANCHES}develop (could not start its audit: ${DISPATCH_RESULT}), " + BRANCHES="${BRANCHES}develop (${DISPATCH_RESULT}; see the start the develop audit job for the develop run), " fi BRANCHES="${BRANCHES%, }" if [ -n "$NUMBER" ]; then diff --git a/tests/audit_ignore_lists.rs b/tests/audit_ignore_lists.rs index 8f168da2..4deead15 100644 --- a/tests/audit_ignore_lists.rs +++ b/tests/audit_ignore_lists.rs @@ -1,20 +1,23 @@ //! `cargo audit` and `cargo deny` agree on which RUSTSEC ids are ignored. //! //! `deny.toml` carries the cargo-deny ignore list and `.cargo/audit.toml` -//! carries the cargo-audit one. cargo-deny reads `deny.toml` and cargo-audit -//! reads `.cargo/audit.toml`, relative to the directory it runs in. The -//! release job (release.yml) runs `cargo audit --deny warnings` from the -//! repository root, so an ignore that lives only in deny.toml does not help -//! the release gate: the next release would fail on the same advisory that -//! turned the scheduled audit red on 2026-10-12 (#2034). The two lists must -//! be equal. +//! carries the cargo-audit one. `cargo-deny` reads `deny.toml`; +//! `cargo-audit` reads `.cargo/audit.toml`, relative to its working +//! directory. The release job (release.yml) runs `cargo audit --deny +//! warnings` from the repository root, so an ignore that lives only in +//! deny.toml does not help the release gate: the next release would fail +//! on the same advisory that turned the scheduled audit red on +//! 2026-10-12 (#2034). The two lists must be equal. //! -//! No new crate dependency is taken on for this. `toml` is not in the -//! workspace's lock today, and pulling it just to compare two short string -//! lists is more weight than the assertion needs. The parser is a hand-rolled -//! scan for `id = "..."` (deny.toml's table form) and `ignore = [ "..." ]` -//! (cargo-audit's list form); both shapes are stable per the input files -//! they target. +//! One parser reads both files. It strips `#`-style comments (only +//! outside double-quoted strings, so `reason = "see #2035"` survives), +//! walks until the `[advisories]` table header, takes everything between +//! its `ignore = [` and the matching `]`, and collects every +//! double-quoted string whose WHOLE value matches +//! `^RUSTSEC-[0-9]{4}-[0-9]{4}$`. The two shapes cargo-deny accepts, +//! `{ id = "X", reason = "..." }` and the plain string `"X"`, resolve to +//! the same set; an id that only appears inside a longer string is not +//! collected. use std::fs; use std::path::Path; @@ -24,170 +27,221 @@ fn read_repo(rel: &str) -> String { fs::read_to_string(&path).unwrap_or_else(|e| panic!("{rel} is readable: {e}")) } -/// Extract the set of advisory ids in `body` whose shape is a deny.toml -/// table entry: `id = "RUSTSEC-..."`. The line may be a full inline -/// table (`{ id = "X", reason = "..." }`) or just the field (`id = "X"`), -/// so the parser scans for `id = "..."` anywhere on the line. Comment -/// lines and lines without the field are skipped; a future refactor -/// that introduces an unrelated `id = "..."` field on a different key -/// would also be rejected only when its value starts with `RUSTSEC-`, -/// which is the convention the file's own entries follow. -fn deny_toml_ids(body: &str) -> Vec { - let mut out = Vec::new(); - for line in body.lines() { - let trimmed = line.trim_start(); - // Comment and blank lines: skip. Comment lines mention ids in - // prose and an inline comment that just happens to mention - // one would otherwise be picked up. Same shape as the parsers - // in `tests/workflow_audit_strictness.rs`. - if trimmed.is_empty() || trimmed.starts_with('#') { - continue; - } - // Find every `id = "RUSTSEC-..."` token on the line. The line - // may also be the inline-table form `{ id = "X", ... }`, in - // which case the scan still starts at `id` and reads forward - // past the `=` to the quoted string. - let bytes = trimmed.as_bytes(); - let mut i = 0; - while i < bytes.len() { - if bytes[i] != b'i' { +/// Strip `#`-prefixed comments from a TOML-like file, leaving any `#` +/// that lives inside a `"..."` string alone. Each returned line still +/// contains its indentation and any trailing content, so the parser that +/// consumes this output does not lose column positions. +fn strip_comments(body: &str) -> String { + let mut out = String::with_capacity(body.len()); + let bytes = body.as_bytes(); + let mut i = 0; + let mut at_line_start = true; + while i < bytes.len() { + let c = bytes[i]; + if c == b'"' { + out.push('"'); + i += 1; + while i < bytes.len() && bytes[i] != b'"' { + out.push(bytes[i] as char); i += 1; - continue; } - if !trimmed[i..].starts_with("id") { + if i < bytes.len() { + out.push('"'); i += 1; - continue; } - let after_id_idx = i + 2; - if after_id_idx < bytes.len() && !matches!(bytes[after_id_idx], b' ' | b'=') { + at_line_start = false; + continue; + } + if at_line_start && c == b'#' { + while i < bytes.len() && bytes[i] != b'\n' { i += 1; - continue; } - let mut j = after_id_idx; - while j < bytes.len() && bytes[j] == b' ' { - j += 1; - } - if j >= bytes.len() || bytes[j] != b'=' { - i += 1; + at_line_start = true; + continue; + } + if c == b'\n' { + at_line_start = true; + } else { + at_line_start = false; + } + out.push(c as char); + i += 1; + } + out +} + +/// Slice the body to the bytes that live inside the `[advisories]` +/// table, i.e. from the line that starts with `[advisories]` up to the +/// next line that begins with `[` (a new table header). Returns the +/// raw bytes so the caller can keep the same byte offsets throughout. +/// An empty slice means the section was not found. +fn advisories_section(body: &str) -> &[u8] { + let bytes = body.as_bytes(); + let header = b"[advisories]"; + let header_len = header.len(); + let mut start: Option = None; + let mut end: Option = None; + let mut i = 0; + let mut at_line_start = true; + while i < bytes.len() { + let c = bytes[i]; + if at_line_start + && c == b'[' + && i + header_len <= bytes.len() + && &bytes[i..i + header_len] == header + { + if start.is_none() { + start = Some(i); + i += header_len; + at_line_start = false; continue; + } else { + end = Some(i); + break; } - j += 1; - while j < bytes.len() && bytes[j] == b' ' { + } + if c == b'\n' { + at_line_start = true; + } else if c != b' ' && c != b'\t' && c != b'\r' { + at_line_start = false; + } + i += 1; + } + let start = match start { + Some(s) => s, + None => return &[], + }; + let end = end.unwrap_or(bytes.len()); + &bytes[start..end] +} + +/// Find the `ignore = [ ... ]` array inside the `[advisories]` section +/// and return the bytes inside the brackets. Returns an empty slice if +/// the `ignore = [` opener is never seen. Nested `{ ... }` tables inside +/// the array are opened and closed correctly; quoted strings are opaque +/// to the bracket counter, so a `[` or `{` inside a string is not +/// counted. The closing `]` of `ignore = [...]` is the first one that +/// brings the bracket depth back to zero after the opener. +fn ignore_array(body: &[u8]) -> Vec { + let mut open_at: Option = None; + let mut i = 0; + while i + 11 < body.len() { + // Look for `ignore = [` at any column. Anything after a + // `\n` is fair game; the section has been narrowed to the + // [advisories] table already, so we won't pick up a stray + // `ignore` somewhere else. + if &body[i..i + 6] == b"ignore" { + let mut j = i + 6; + while j < body.len() && (body[j] == b' ' || body[j] == b'\t') { j += 1; } - if j >= bytes.len() { + if j + 2 < body.len() && body[j] == b'=' && body[j + 1] == b' ' && body[j + 2] == b'[' { + open_at = Some(j + 3); break; } - let quote = bytes[j]; - if quote != b'"' && quote != b'\'' { - i += 1; - continue; + } + i += 1; + } + let open_at = match open_at { + Some(o) => o, + None => return Vec::new(), + }; + let mut depth: i32 = 0; + let mut k = open_at; + while k < body.len() { + let c = body[k]; + if c == b'"' { + k += 1; + while k < body.len() && body[k] != b'"' { + k += 1; } - j += 1; - let start = j; - while j < bytes.len() && bytes[j] != quote { - j += 1; + if k < body.len() { + k += 1; } - if j >= bytes.len() { - break; + continue; + } + if c == b'[' || c == b'{' { + depth += 1; + k += 1; + continue; + } + if c == b']' { + if depth == 0 { + return body[open_at..k].to_vec(); } - let token = &trimmed[start..j]; - if token.starts_with("RUSTSEC-") { - out.push(token.to_string()); + depth -= 1; + k += 1; + continue; + } + if c == b'}' { + if depth > 0 { + depth -= 1; } - i = j + 1; + k += 1; + continue; } + k += 1; } - out + Vec::new() } -/// Extract the set of advisory ids in `body` whose shape is the -/// cargo-audit list form: `ignore = [ "RUSTSEC-...", "..." ]`. Strings -/// inside `[]` are matched one at a time; commas and trailing whitespace -/// are tolerated because that is the form `.cargo/audit.toml` already -/// uses. Lines outside the `ignore = [...]` declaration are not parsed -/// here either: the file is small and a stray RUSTSEC id elsewhere would -/// still belong in `ignore`, so the question "is X in the ignore list?" -/// either way points at the same place. -/// -/// The inline single-line form (`ignore = ["X", "Y"]`) is read in one -/// pass: the `]` may appear on the same line that holds the entries, -/// so the parser cannot rely on a line-by-line open/close. It tracks -/// bracket nesting instead: an `[` that is not inside a string opens -/// the list, a matching `]` closes it, and quoted strings inside the -/// brackets are not counted as brackets. Multi-line lists behave the -/// same way, with the `[` on one line and the matching `]` later. -fn audit_toml_ids(body: &str) -> Vec { +/// Collect every `"X"` inside `bytes` whose whole value matches +/// `^RUSTSEC-[0-9]{4}-[0-9]{4}$`. The whole-value test is what stops an +/// id embedded in a longer string (e.g. `reason = "see RUSTSEC-..."`) +/// from being read as if it were a list entry. +fn extract_ids(bytes: &[u8]) -> Vec { let mut out = Vec::new(); - let mut depth: i32 = 0; - for line in body.lines() { - // Walk the line character by character. Outside the list - // (`depth == 0`) we skip everything except a `[` that opens - // the ignore list; inside it (depth > 0) we collect every - // quoted `RUSTSEC-...` token. - let bytes = line.as_bytes(); - let mut i = 0; - while i < bytes.len() { - let c = bytes[i]; - // A quoted token. Single or double quotes; the inside of - // a quoted string is opaque, so brackets inside a string - // do not count as brackets, and `=` inside a string does - // not count as the assignment. - if c == b'"' || c == b'\'' { - let quote = c; - let start = i + 1; - let mut j = start; - while j < bytes.len() && bytes[j] != quote { - j += 1; - } - if j < bytes.len() { - if depth > 0 { - let token = &line[start..j]; - if token.starts_with("RUSTSEC-") { - out.push(token.to_string()); - } - } - i = j + 1; - continue; - } + let mut i = 0; + while i < bytes.len() { + let c = bytes[i]; + if c == b'"' { + let start = i + 1; + let mut j = start; + while j < bytes.len() && bytes[j] != b'"' { + j += 1; } - // A `[` inside or outside the list changes the bracket - // nesting: outside, an opening `[` enters the list only - // when the line was `ignore = [ ... ]` (i.e. depth was - // zero and we are tracking `ignore`-list entries). To - // keep the parser robust against the multiline form - // (`ignore = [\n "X",\n "Y",\n]`) the scan tracks - // `depth` rather than a boolean: a `[` already inside an - // `ignore = [...]` increments the depth, and a `]` - // decrements it back to zero. - if c == b'[' { - depth += 1; - i += 1; - continue; + if j >= bytes.len() { + return out; } - if c == b']' { - if depth > 0 { - depth -= 1; - } - i += 1; - continue; + let token = &bytes[start..j]; + if is_rustsec_id(token) { + out.push(std::str::from_utf8(token).unwrap_or("").to_string()); } - i += 1; + i = j + 1; + continue; } + i += 1; } out } -/// `audit_toml_ids` once used a `first_id_token` helper for the -/// single-line form. The rewritten parser above is bracket-depth aware -/// and reads every quoted id on a line in one pass, so the helper is -/// gone: keep this comment so a future reader who lands on a `git -/// log -p` and wonders why a private helper existed finds the answer. +fn is_rustsec_id(token: &[u8]) -> bool { + let prefix = b"RUSTSEC-"; + if token.len() != 17 || !token.starts_with(prefix) { + return false; + } + let bytes = &token[8..]; + if bytes[4] != b'-' { + return false; + } + bytes[..4].iter().all(|b| b.is_ascii_digit()) && bytes[5..].iter().all(|b| b.is_ascii_digit()) +} + +/// One parser for both deny.toml and `.cargo/audit.toml`. Strips +/// comments, narrows to the `[advisories]` section, finds the +/// `ignore = [...]` array, and collects every quoted RUSTSEC id whose +/// value is exactly `RUSTSEC-YYYY-NNNN`. +fn ignored_ids(body: &str) -> Vec { + let stripped = strip_comments(body); + let section = advisories_section(&stripped); + let array = ignore_array(section); + extract_ids(&array) +} #[test] fn the_two_ignore_lists_agree_on_advisory_ids() { - let deny_ids: Vec = deny_toml_ids(&read_repo("deny.toml")); - let audit_ids: Vec = audit_toml_ids(&read_repo(".cargo/audit.toml")); + let deny_ids: Vec = ignored_ids(&read_repo("deny.toml")); + let audit_ids: Vec = ignored_ids(&read_repo(".cargo/audit.toml")); // Order does not matter: the contract is the SET of ids each tool // accepts. `BTreeSet` also makes the failure message stable. let deny_set: std::collections::BTreeSet<_> = deny_ids.iter().collect(); @@ -202,94 +256,81 @@ fn the_two_ignore_lists_agree_on_advisory_ids() { ); } -/// Pin the parser on input it must accept. Today the file format is -/// `{ id = "X", reason = "..." }` for deny.toml and `ignore = ["X", "Y"]` -/// for .cargo/audit.toml; the parser is what every other test in this -/// file trusts, and a parser that always returned one canonical id would -/// satisfy the assertions above against a tree where the lists had -/// drifted. #[test] fn the_parser_reads_what_it_claims() { - // deny.toml table form: one id, exactly one entry, no comments. + // (a) Table form and plain-string form of the same id give the + // same set, in either file. cargo-deny accepts both forms in the + // same `[advisories].ignore` array. let deny_body = "\ [advisories] ignore = [ { id = \"RUSTSEC-0000-0000\", reason = \"x\" }, ] "; - let deny_ids = deny_toml_ids(deny_body); + let plain_body = "\ +[advisories] +ignore = [\"RUSTSEC-0000-0000\"] +"; + let deny_set: std::collections::BTreeSet<_> = ignored_ids(deny_body).into_iter().collect(); + let plain_set: std::collections::BTreeSet<_> = ignored_ids(plain_body).into_iter().collect(); assert_eq!( - deny_ids, - vec!["RUSTSEC-0000-0000".to_string()], - "deny.toml table form: exactly one id, got {deny_ids:?}" + deny_set, plain_set, + "table form and plain-string form of the same id must agree" + ); + assert_eq!( + deny_set, + ["RUSTSEC-0000-0000".to_string()].into_iter().collect() ); - // A comment above the entry that mentions the id must not be counted. - let deny_with_comment = "\ + // (b) A commented-out ignore line does not contribute to the set. + let commented = "\ [advisories] # ignore = [{ id = \"RUSTSEC-9999-9999\", reason = \"comment only\" }] ignore = [{ id = \"RUSTSEC-0000-0000\", reason = \"real\" }] "; - let ids = deny_toml_ids(deny_with_comment); + let ids: std::collections::BTreeSet<_> = ignored_ids(commented).into_iter().collect(); assert_eq!( ids, - vec!["RUSTSEC-0000-0000".to_string()], - "deny.toml parser picked up an id from a comment line, got {ids:?}" + ["RUSTSEC-0000-0000".to_string()].into_iter().collect(), + "commented-out ignore line must not contribute to the set, got {ids:?}" ); - // cargo-audit list form: a multi-line array with more than one id. - let audit_body = "\ + // (c) A `reason = "..."` string that mentions another RUSTSEC id + // does not add it: the whole-value test rejects it. + let reason_mentions = "\ [advisories] ignore = [ - \"RUSTSEC-0000-0000\", - \"RUSTSEC-1111-2222\", + { id = \"RUSTSEC-0000-0000\", reason = \"see RUSTSEC-1111-2222\" }, ] "; - let audit_ids = audit_toml_ids(audit_body); - assert_eq!( - audit_ids, - vec![ - "RUSTSEC-0000-0000".to_string(), - "RUSTSEC-1111-2222".to_string() - ], - "cargo-audit multi-line list form: both ids parsed, got {audit_ids:?}" - ); - - // cargo-audit single-line list form: both ids on the same line. - let audit_one_line = "\ -[advisories] -ignore = [\"RUSTSEC-0000-0000\", \"RUSTSEC-1111-2222\"] -"; - let ids_inline = audit_toml_ids(audit_one_line); + let ids: std::collections::BTreeSet<_> = ignored_ids(reason_mentions).into_iter().collect(); assert_eq!( - ids_inline, - vec![ - "RUSTSEC-0000-0000".to_string(), - "RUSTSEC-1111-2222".to_string() - ], - "cargo-audit single-line form: both ids parsed, got {ids_inline:?}" + ids, + ["RUSTSEC-0000-0000".to_string()].into_iter().collect(), + "a reason string mentioning another RUSTSEC id must not add that \ + id to the set, got {ids:?}" ); - // The negative control: a deny.toml fixture where the two parsers - // disagree. The parser pair, not the input set, is what the - // agreement test depends on: a parser that hardcoded the - // canonical set would pass every assertion above against any - // well-formed file, including a drifted one. - let deny_differs = "\ + // (d) Negative fixture: a deliberate mismatch between two + // fixtures makes the parser pair disagree. A parser that + // hardcoded the canonical set would still pass every assertion + // above against any well-formed input; this fixture proves the + // parsers actually read the bytes. + let a = "\ [advisories] ignore = [{ id = \"RUSTSEC-9999-9999\", reason = \"x\" }] "; - let audit_differs = "\ + let b = "\ [advisories] ignore = [\"RUSTSEC-0000-0000\"] "; - let d: std::collections::BTreeSet<_> = deny_toml_ids(deny_differs).into_iter().collect(); - let a: std::collections::BTreeSet<_> = audit_toml_ids(audit_differs).into_iter().collect(); + let sa: std::collections::BTreeSet<_> = ignored_ids(a).into_iter().collect(); + let sb: std::collections::BTreeSet<_> = ignored_ids(b).into_iter().collect(); assert_ne!( - d, a, - "the negative fixture did not disagree between parsers; the \ - agreement test would have passed against drifted lists, which \ - is the failure mode the inline case is here to prove the \ - parsers can catch. deny: {d:?}, audit: {a:?}" + sa, sb, + "the negative fixture did not disagree between parses; the \ + agreement test would have passed against drifted lists, \ + which is the failure mode this case is here to prove the \ + parser can catch. deny: {sa:?}, audit: {sb:?}" ); } From 5c0c55e9ae99d01fbfd5ff8624e44328babce336 Mon Sep 17 00:00:00 2001 From: Jaro-c <75870284+Jaro-c@users.noreply.github.com> Date: Mon, 12 Oct 2026 18:41:55 -0500 Subject: [PATCH 6/8] ci: let the scheduled audit find its own develop run, and drop the reporter concurrency group Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> --- .github/workflows/audit.yml | 40 +++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/.github/workflows/audit.yml b/.github/workflows/audit.yml index af9973b8..036b5ca0 100644 --- a/.github/workflows/audit.yml +++ b/.github/workflows/audit.yml @@ -34,10 +34,10 @@ on: - cron: "0 6 * * *" # Daily at 06:00 UTC workflow_dispatch: inputs: - from-schedule: - description: Set by the scheduled run on main when it starts the develop audit; that run reports for both branches - type: boolean - default: false + parent-run: + description: Set by the scheduled run on main to its own run id when it starts the develop audit; empty for a manual dispatch + type: string + default: "" pull_request: branches: - main @@ -53,6 +53,12 @@ on: permissions: contents: read +# On a schedule or pull_request event `inputs.parent-run` is empty and +# the run is named after the workflow. On a workflow_dispatch started by +# the schedule the input carries the parent's run id and the run is +# named so the parent can find its own child by displayTitle. +run-name: ${{ inputs.parent-run && format('Security audit of develop for run {0}', inputs.parent-run) || github.workflow }} + jobs: # Kept as audit so its check names stay the same. audit: @@ -84,11 +90,10 @@ jobs: - name: Dispatch audit.yml on develop and wait for it run: | set -euo pipefail - SINCE=$(date -u +%Y-%m-%dT%H:%M:%SZ) - gh workflow run audit.yml --ref develop -f from-schedule=true + gh workflow run audit.yml --ref develop -f parent-run="$GITHUB_RUN_ID" ID= for _ in $(seq 1 10); do - ID=$(gh run list --workflow audit.yml --branch develop --event workflow_dispatch --limit 5 --json databaseId,createdAt --jq "map(select(.createdAt >= \"$SINCE\")) | sort_by(.createdAt) | .[0].databaseId // empty") + ID=$(gh run list --workflow audit.yml --branch develop --event workflow_dispatch --limit 20 --json databaseId,displayTitle --jq "map(select(.displayTitle == \"Security audit of develop for run $GITHUB_RUN_ID\")) | .[0].databaseId // empty") if [ -n "$ID" ]; then break fi @@ -107,20 +112,21 @@ jobs: # Runs when this run's audit did not succeed (failure, cancellation or # timeout). On the schedule it reports both branches, because the job # above fails when develop's run fails or never appears. A develop run - # started by the schedule passes from-schedule and does not report - # itself; a manual dispatch reports its own branch. issues: write is - # granted to this job alone; nothing here checks out code or installs - # tools, and every value reaches the script through env. + # started by the schedule passes parent-run and does not report itself; + # a manual dispatch reports its own branch. issues: write is granted + # to this job alone; nothing here checks out code or installs tools, + # and every value reaches the script through env. + # + # Two reporters running at once can open two issues; that only happens + # when a manual dispatch fails at the same moment as the schedule, and + # a duplicate issue is cheaper than a lost report, which is what a + # concurrency group would risk (GitHub keeps one pending job and + # cancels the rest). report: name: open an issue when the scheduled audit fails runs-on: ubuntu-latest needs: [audit, audit-develop] - # One reporter at a time across runs, so two of them cannot both - # find no open issue and both create one. - concurrency: - group: audit-report - cancel-in-progress: false - if: always() && ((github.event_name == 'schedule' && (needs.audit.result != 'success' || needs.audit-develop.result != 'success')) || (github.event_name == 'workflow_dispatch' && !inputs.from-schedule && needs.audit.result != 'success')) + if: always() && ((github.event_name == 'schedule' && (needs.audit.result != 'success' || needs.audit-develop.result != 'success')) || (github.event_name == 'workflow_dispatch' && !inputs.parent-run && needs.audit.result != 'success')) permissions: issues: write # Bound: no runs measured yet, so the ten-minute floor every job From 84f60305f51060a32a8f7aa68b4c86fc4fa3b8cd Mon Sep 17 00:00:00 2001 From: Jaro-c <75870284+Jaro-c@users.noreply.github.com> Date: Mon, 12 Oct 2026 19:17:28 -0500 Subject: [PATCH 7/8] test: read both ignore lists in one canonical layout and refuse any other Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> --- .cargo/audit.toml | 4 +- tests/audit_ignore_lists.rs | 420 ++++++++++-------------------------- 2 files changed, 115 insertions(+), 309 deletions(-) diff --git a/.cargo/audit.toml b/.cargo/audit.toml index 7a8a6fe6..7aec5de8 100644 --- a/.cargo/audit.toml +++ b/.cargo/audit.toml @@ -4,4 +4,6 @@ # is written in deny.toml. tests/audit_ignore_lists.rs keeps the two equal. [advisories] -ignore = ["RUSTSEC-2024-0449"] +ignore = [ + "RUSTSEC-2024-0449", +] diff --git a/tests/audit_ignore_lists.rs b/tests/audit_ignore_lists.rs index 4deead15..fe2dd0bc 100644 --- a/tests/audit_ignore_lists.rs +++ b/tests/audit_ignore_lists.rs @@ -1,336 +1,140 @@ -//! `cargo audit` and `cargo deny` agree on which RUSTSEC ids are ignored. -//! -//! `deny.toml` carries the cargo-deny ignore list and `.cargo/audit.toml` -//! carries the cargo-audit one. `cargo-deny` reads `deny.toml`; -//! `cargo-audit` reads `.cargo/audit.toml`, relative to its working -//! directory. The release job (release.yml) runs `cargo audit --deny -//! warnings` from the repository root, so an ignore that lives only in -//! deny.toml does not help the release gate: the next release would fail -//! on the same advisory that turned the scheduled audit red on -//! 2026-10-12 (#2034). The two lists must be equal. -//! -//! One parser reads both files. It strips `#`-style comments (only -//! outside double-quoted strings, so `reason = "see #2035"` survives), -//! walks until the `[advisories]` table header, takes everything between -//! its `ignore = [` and the matching `]`, and collects every -//! double-quoted string whose WHOLE value matches -//! `^RUSTSEC-[0-9]{4}-[0-9]{4}$`. The two shapes cargo-deny accepts, -//! `{ id = "X", reason = "..." }` and the plain string `"X"`, resolve to -//! the same set; an id that only appears inside a longer string is not -//! collected. - +//! `cargo audit` and `cargo deny` read two different files but must agree +//! on which RUSTSEC ids are ignored. `deny.toml` carries the cargo-deny +//! list and `.cargo/audit.toml` carries the cargo-audit one. The release +//! job (`release.yml`) runs `cargo audit --deny warnings` from the +//! repository root, so an ignore that lives only in deny.toml does not +//! help the release gate. The two lists must be equal. This test only +//! accepts one canonical layout per file and refuses the rest. use std::fs; use std::path::Path; - fn read_repo(rel: &str) -> String { - let path = Path::new(env!("CARGO_MANIFEST_DIR")).join(rel); - fs::read_to_string(&path).unwrap_or_else(|e| panic!("{rel} is readable: {e}")) + fs::read_to_string(Path::new(env!("CARGO_MANIFEST_DIR")).join(rel)) + .unwrap_or_else(|e| panic!("{rel} readable: {e}")) } - -/// Strip `#`-prefixed comments from a TOML-like file, leaving any `#` -/// that lives inside a `"..."` string alone. Each returned line still -/// contains its indentation and any trailing content, so the parser that -/// consumes this output does not lose column positions. -fn strip_comments(body: &str) -> String { - let mut out = String::with_capacity(body.len()); - let bytes = body.as_bytes(); - let mut i = 0; - let mut at_line_start = true; - while i < bytes.len() { - let c = bytes[i]; - if c == b'"' { - out.push('"'); - i += 1; - while i < bytes.len() && bytes[i] != b'"' { - out.push(bytes[i] as char); - i += 1; - } - if i < bytes.len() { - out.push('"'); - i += 1; - } - at_line_start = false; - continue; - } - if at_line_start && c == b'#' { - while i < bytes.len() && bytes[i] != b'\n' { - i += 1; - } - at_line_start = true; - continue; - } - if c == b'\n' { - at_line_start = true; - } else { - at_line_start = false; - } - out.push(c as char); - i += 1; - } - out +fn is_rustsec(s: &str) -> Option<&str> { + let b = s.as_bytes(); + let d = |r: std::ops::Range| b[r].iter().all(u8::is_ascii_digit); + (b.len() == 17 && b.starts_with(b"RUSTSEC-") && b[12] == b'-' && d(8..12) && d(13..17)) + .then_some(s) } - -/// Slice the body to the bytes that live inside the `[advisories]` -/// table, i.e. from the line that starts with `[advisories]` up to the -/// next line that begins with `[` (a new table header). Returns the -/// raw bytes so the caller can keep the same byte offsets throughout. -/// An empty slice means the section was not found. -fn advisories_section(body: &str) -> &[u8] { - let bytes = body.as_bytes(); - let header = b"[advisories]"; - let header_len = header.len(); - let mut start: Option = None; - let mut end: Option = None; - let mut i = 0; - let mut at_line_start = true; - while i < bytes.len() { - let c = bytes[i]; - if at_line_start - && c == b'[' - && i + header_len <= bytes.len() - && &bytes[i..i + header_len] == header - { - if start.is_none() { - start = Some(i); - i += header_len; - at_line_start = false; - continue; - } else { - end = Some(i); - break; - } - } - if c == b'\n' { - at_line_start = true; - } else if c != b' ' && c != b'\t' && c != b'\r' { - at_line_start = false; - } - i += 1; +const DENY_SHAPE: &str = "`{ id = \"RUSTSEC-YYYY-NNNN\", reason = \"...\" },`"; +fn parse_entry(file: &str, l: &str) -> Option { + let s = l.strip_suffix(',')?.trim(); + if file.ends_with("/audit.toml") { + is_rustsec(s.strip_prefix('"')?.strip_suffix('"')?).map(String::from) + } else { + let inner = s.strip_prefix('{')?.strip_suffix('}')?.trim(); + let (left, right) = inner.split_once(',')?; + let id = left + .trim() + .strip_prefix("id = ")? + .strip_prefix('"')? + .strip_suffix('"')?; + let rv = right + .trim() + .strip_prefix("reason = ")? + .strip_prefix('"')? + .strip_suffix('"')?; + (!rv.contains('"')).then_some(())?; + is_rustsec(id).map(String::from) } - let start = match start { - Some(s) => s, - None => return &[], - }; - let end = end.unwrap_or(bytes.len()); - &bytes[start..end] } - -/// Find the `ignore = [ ... ]` array inside the `[advisories]` section -/// and return the bytes inside the brackets. Returns an empty slice if -/// the `ignore = [` opener is never seen. Nested `{ ... }` tables inside -/// the array are opened and closed correctly; quoted strings are opaque -/// to the bracket counter, so a `[` or `{` inside a string is not -/// counted. The closing `]` of `ignore = [...]` is the first one that -/// brings the bracket depth back to zero after the opener. -fn ignore_array(body: &[u8]) -> Vec { - let mut open_at: Option = None; - let mut i = 0; - while i + 11 < body.len() { - // Look for `ignore = [` at any column. Anything after a - // `\n` is fair game; the section has been narrowed to the - // [advisories] table already, so we won't pick up a stray - // `ignore` somewhere else. - if &body[i..i + 6] == b"ignore" { - let mut j = i + 6; - while j < body.len() && (body[j] == b' ' || body[j] == b'\t') { - j += 1; - } - if j + 2 < body.len() && body[j] == b'=' && body[j + 1] == b' ' && body[j + 2] == b'[' { - open_at = Some(j + 3); - break; - } - } - i += 1; - } - let open_at = match open_at { - Some(o) => o, - None => return Vec::new(), - }; - let mut depth: i32 = 0; - let mut k = open_at; - while k < body.len() { - let c = body[k]; - if c == b'"' { - k += 1; - while k < body.len() && body[k] != b'"' { - k += 1; - } - if k < body.len() { - k += 1; - } - continue; - } - if c == b'[' || c == b'{' { - depth += 1; - k += 1; - continue; - } - if c == b']' { - if depth == 0 { - return body[open_at..k].to_vec(); - } - depth -= 1; - k += 1; +fn ignored(file: &str, body: &str, shape: &str) -> Vec { + macro_rules! bad { ($l:expr) => { panic!("{file}: write the ignore list in the canonical form, one entry per line: {shape}; found: {}", $l) } } + let dup = || panic!("{file}: expected exactly one ignore key"); + let mut in_adv = false; + let mut in_arr = false; + let mut seen = false; + let mut out = Vec::new(); + for line in body.lines() { + let l = line.trim(); + in_adv = in_adv || l == "[advisories]"; + if !in_adv { continue; } - if c == b'}' { - if depth > 0 { - depth -= 1; + let ig = l.starts_with("ignore =") || l.starts_with("ignore="); + match (in_arr, ig, l) { + (false, _, l) if l.is_empty() || l.starts_with('#') => continue, + (false, _, l) if l.starts_with('[') && l.ends_with(']') && l != "[advisories]" => break, + (false, false, _) => continue, + (false, true, l) => { + if seen { + dup(); + } + seen = true; + match l { + "ignore = []" => return Vec::new(), + "ignore = [" => in_arr = true, + _ => bad!(l), + } } - k += 1; - continue; + (true, true, _) => dup(), + (true, false, "]") => return out, + (true, false, l) => match parse_entry(file, l) { + Some(id) => out.push(id), + None => bad!(l), + }, } - k += 1; } - Vec::new() -} - -/// Collect every `"X"` inside `bytes` whose whole value matches -/// `^RUSTSEC-[0-9]{4}-[0-9]{4}$`. The whole-value test is what stops an -/// id embedded in a longer string (e.g. `reason = "see RUSTSEC-..."`) -/// from being read as if it were a list entry. -fn extract_ids(bytes: &[u8]) -> Vec { - let mut out = Vec::new(); - let mut i = 0; - while i < bytes.len() { - let c = bytes[i]; - if c == b'"' { - let start = i + 1; - let mut j = start; - while j < bytes.len() && bytes[j] != b'"' { - j += 1; - } - if j >= bytes.len() { - return out; - } - let token = &bytes[start..j]; - if is_rustsec_id(token) { - out.push(std::str::from_utf8(token).unwrap_or("").to_string()); - } - i = j + 1; - continue; - } - i += 1; + if !seen { + dup(); } - out + panic!("{file}: missing closing `]` for ignore array"); } - -fn is_rustsec_id(token: &[u8]) -> bool { - let prefix = b"RUSTSEC-"; - if token.len() != 17 || !token.starts_with(prefix) { - return false; - } - let bytes = &token[8..]; - if bytes[4] != b'-' { - return false; - } - bytes[..4].iter().all(|b| b.is_ascii_digit()) && bytes[5..].iter().all(|b| b.is_ascii_digit()) +fn cargo_ids(body: &str) -> Vec { + ignored(".cargo/audit.toml", body, "`\"RUSTSEC-YYYY-NNNN\",`") } - -/// One parser for both deny.toml and `.cargo/audit.toml`. Strips -/// comments, narrows to the `[advisories]` section, finds the -/// `ignore = [...]` array, and collects every quoted RUSTSEC id whose -/// value is exactly `RUSTSEC-YYYY-NNNN`. -fn ignored_ids(body: &str) -> Vec { - let stripped = strip_comments(body); - let section = advisories_section(&stripped); - let array = ignore_array(section); - extract_ids(&array) +fn deny_ids(body: &str) -> Vec { + ignored("deny.toml", body, DENY_SHAPE) } - #[test] fn the_two_ignore_lists_agree_on_advisory_ids() { - let deny_ids: Vec = ignored_ids(&read_repo("deny.toml")); - let audit_ids: Vec = ignored_ids(&read_repo(".cargo/audit.toml")); - // Order does not matter: the contract is the SET of ids each tool - // accepts. `BTreeSet` also makes the failure message stable. - let deny_set: std::collections::BTreeSet<_> = deny_ids.iter().collect(); - let audit_set: std::collections::BTreeSet<_> = audit_ids.iter().collect(); + let mut deny = deny_ids(&read_repo("deny.toml")); + let mut audit = cargo_ids(&read_repo(".cargo/audit.toml")); + deny.sort(); + audit.sort(); assert_eq!( - deny_set, audit_set, - "deny.toml and .cargo/audit.toml must list the same set of ignored \ - advisories. deny.toml: {:?}, .cargo/audit.toml: {:?}. The release \ - gate (release.yml) runs cargo-audit, which reads .cargo/audit.toml \ - only; a difference here is a difference in what each tool approves.", - deny_ids, audit_ids + deny, audit, + "deny.toml and .cargo/audit.toml disagree. deny: {deny:?}, audit: {audit:?}" ); } - #[test] -fn the_parser_reads_what_it_claims() { - // (a) Table form and plain-string form of the same id give the - // same set, in either file. cargo-deny accepts both forms in the - // same `[advisories].ignore` array. - let deny_body = "\ -[advisories] -ignore = [ - { id = \"RUSTSEC-0000-0000\", reason = \"x\" }, -] -"; - let plain_body = "\ -[advisories] -ignore = [\"RUSTSEC-0000-0000\"] -"; - let deny_set: std::collections::BTreeSet<_> = ignored_ids(deny_body).into_iter().collect(); - let plain_set: std::collections::BTreeSet<_> = ignored_ids(plain_body).into_iter().collect(); - assert_eq!( - deny_set, plain_set, - "table form and plain-string form of the same id must agree" - ); +fn empty_ignore_array_yields_empty() { + assert!(cargo_ids("[advisories]\nignore = []\n").is_empty()); +} +#[test] +fn canonical_two_entries_read_in_order() { assert_eq!( - deny_set, - ["RUSTSEC-0000-0000".to_string()].into_iter().collect() + cargo_ids( + "[advisories]\nignore = [\n \"RUSTSEC-2024-0001\",\n \"RUSTSEC-2024-0002\",\n]\n" + ), + vec![ + "RUSTSEC-2024-0001".to_string(), + "RUSTSEC-2024-0002".to_string() + ] ); - - // (b) A commented-out ignore line does not contribute to the set. - let commented = "\ -[advisories] -# ignore = [{ id = \"RUSTSEC-9999-9999\", reason = \"comment only\" }] -ignore = [{ id = \"RUSTSEC-0000-0000\", reason = \"real\" }] -"; - let ids: std::collections::BTreeSet<_> = ignored_ids(commented).into_iter().collect(); - assert_eq!( - ids, - ["RUSTSEC-0000-0000".to_string()].into_iter().collect(), - "commented-out ignore line must not contribute to the set, got {ids:?}" +} +#[test] +#[should_panic(expected = "write the ignore list in the canonical form")] +fn commented_out_entry_panics() { + let _ = cargo_ids( + "[advisories]\nignore = [\n # \"RUSTSEC-2024-0001\",\n \"RUSTSEC-2024-0002\",\n]\n", ); - - // (c) A `reason = "..."` string that mentions another RUSTSEC id - // does not add it: the whole-value test rejects it. - let reason_mentions = "\ -[advisories] -ignore = [ - { id = \"RUSTSEC-0000-0000\", reason = \"see RUSTSEC-1111-2222\" }, -] -"; - let ids: std::collections::BTreeSet<_> = ignored_ids(reason_mentions).into_iter().collect(); +} +#[test] +#[should_panic(expected = "write the ignore list in the canonical form")] +fn one_line_array_panics() { + let _ = cargo_ids("[advisories]\nignore = [\"RUSTSEC-0000-0000\"]\n"); +} +#[test] +#[should_panic(expected = "expected exactly one ignore key")] +fn second_ignore_key_panics() { + let _ = cargo_ids("[advisories]\nignore = [\nignore = [],\n]\n"); +} +#[test] +fn deny_entry_with_reason_mentioning_id_yields_only_id() { assert_eq!( - ids, - ["RUSTSEC-0000-0000".to_string()].into_iter().collect(), - "a reason string mentioning another RUSTSEC id must not add that \ - id to the set, got {ids:?}" - ); - - // (d) Negative fixture: a deliberate mismatch between two - // fixtures makes the parser pair disagree. A parser that - // hardcoded the canonical set would still pass every assertion - // above against any well-formed input; this fixture proves the - // parsers actually read the bytes. - let a = "\ -[advisories] -ignore = [{ id = \"RUSTSEC-9999-9999\", reason = \"x\" }] -"; - let b = "\ -[advisories] -ignore = [\"RUSTSEC-0000-0000\"] -"; - let sa: std::collections::BTreeSet<_> = ignored_ids(a).into_iter().collect(); - let sb: std::collections::BTreeSet<_> = ignored_ids(b).into_iter().collect(); - assert_ne!( - sa, sb, - "the negative fixture did not disagree between parses; the \ - agreement test would have passed against drifted lists, \ - which is the failure mode this case is here to prove the \ - parser can catch. deny: {sa:?}, audit: {sb:?}" + deny_ids("[advisories]\nignore = [\n { id = \"RUSTSEC-2024-0001\", reason = \"see RUSTSEC-9999-9999\" },\n]\n"), + vec!["RUSTSEC-2024-0001".to_string()] ); } From f3252b33cc949a0b0afe384fddee4cb92e3015dd Mon Sep 17 00:00:00 2001 From: Jaro-c <75870284+Jaro-c@users.noreply.github.com> Date: Mon, 12 Oct 2026 19:28:02 -0500 Subject: [PATCH 8/8] ci: tell reruns apart, and refuse odd spacing, trailing duplicate keys and escapes in the ignore lists Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> --- .github/workflows/audit.yml | 4 ++-- tests/audit_ignore_lists.rs | 30 +++++++++++++++++++++++++----- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/.github/workflows/audit.yml b/.github/workflows/audit.yml index 036b5ca0..5de4ea6b 100644 --- a/.github/workflows/audit.yml +++ b/.github/workflows/audit.yml @@ -90,10 +90,10 @@ jobs: - name: Dispatch audit.yml on develop and wait for it run: | set -euo pipefail - gh workflow run audit.yml --ref develop -f parent-run="$GITHUB_RUN_ID" + gh workflow run audit.yml --ref develop -f parent-run="$GITHUB_RUN_ID-$GITHUB_RUN_ATTEMPT" ID= for _ in $(seq 1 10); do - ID=$(gh run list --workflow audit.yml --branch develop --event workflow_dispatch --limit 20 --json databaseId,displayTitle --jq "map(select(.displayTitle == \"Security audit of develop for run $GITHUB_RUN_ID\")) | .[0].databaseId // empty") + ID=$(gh run list --workflow audit.yml --branch develop --event workflow_dispatch --limit 20 --json databaseId,displayTitle --jq "map(select(.displayTitle == \"Security audit of develop for run $GITHUB_RUN_ID-$GITHUB_RUN_ATTEMPT\")) | .[0].databaseId // empty") if [ -n "$ID" ]; then break fi diff --git a/tests/audit_ignore_lists.rs b/tests/audit_ignore_lists.rs index fe2dd0bc..ecedfe55 100644 --- a/tests/audit_ignore_lists.rs +++ b/tests/audit_ignore_lists.rs @@ -35,7 +35,7 @@ fn parse_entry(file: &str, l: &str) -> Option { .strip_prefix("reason = ")? .strip_prefix('"')? .strip_suffix('"')?; - (!rv.contains('"')).then_some(())?; + (!rv.contains('"') && !rv.contains('\\')).then_some(())?; is_rustsec(id).map(String::from) } } @@ -52,7 +52,7 @@ fn ignored(file: &str, body: &str, shape: &str) -> Vec { if !in_adv { continue; } - let ig = l.starts_with("ignore =") || l.starts_with("ignore="); + let ig = l.split_once('=').is_some_and(|(k, _)| k.trim() == "ignore"); match (in_arr, ig, l) { (false, _, l) if l.is_empty() || l.starts_with('#') => continue, (false, _, l) if l.starts_with('[') && l.ends_with(']') && l != "[advisories]" => break, @@ -63,13 +63,13 @@ fn ignored(file: &str, body: &str, shape: &str) -> Vec { } seen = true; match l { - "ignore = []" => return Vec::new(), + "ignore = []" => {} "ignore = [" => in_arr = true, _ => bad!(l), } } (true, true, _) => dup(), - (true, false, "]") => return out, + (true, false, "]") => in_arr = false, (true, false, l) => match parse_entry(file, l) { Some(id) => out.push(id), None => bad!(l), @@ -79,7 +79,10 @@ fn ignored(file: &str, body: &str, shape: &str) -> Vec { if !seen { dup(); } - panic!("{file}: missing closing `]` for ignore array"); + if in_arr { + panic!("{file}: missing closing `]` for ignore array"); + } + out } fn cargo_ids(body: &str) -> Vec { ignored(".cargo/audit.toml", body, "`\"RUSTSEC-YYYY-NNNN\",`") @@ -138,3 +141,20 @@ fn deny_entry_with_reason_mentioning_id_yields_only_id() { vec!["RUSTSEC-2024-0001".to_string()] ); } +#[test] +#[should_panic(expected = "expected exactly one ignore key")] +fn second_ignore_key_after_the_array_panics() { + let _ = cargo_ids("[advisories]\nignore = []\nignore = []\n"); +} +#[test] +#[should_panic(expected = "write the ignore list in the canonical form")] +fn ignore_key_with_other_spacing_panics() { + let _ = cargo_ids("[advisories]\nignore = [\n]\n"); +} +#[test] +#[should_panic(expected = "write the ignore list in the canonical form")] +fn deny_reason_with_a_backslash_panics() { + let _ = deny_ids( + "[advisories]\nignore = [\n { id = \"RUSTSEC-2024-0001\", reason = \"C:\\work\" },\n]\n", + ); +}