From 167bb22ccddea2cb5fe2f5fe9572d263d0652703 Mon Sep 17 00:00:00 2001 From: Nic-dorman Date: Wed, 4 Nov 2026 14:29:07 +0000 Subject: [PATCH 1/2] ci(security): fail the audit only on advisories a PR introduces, and gate on it A plain cargo audit fails every PR the day an advisory is published against a dependency main already has; that made Security Audit red on 43 of 50 PRs (RUSTSEC-2026-0387) and kept it out of CI Gate. Audit the base and PR lockfiles against the same advisory database and fail only on (advisory, crate) pairs the PR adds; existing ones are warnings. A PR that doesn't change Cargo.lock passes without auditing. cargo-audit is pinned to 0.22.2 with --locked, and the job joins CI Gate. New advisories against main are the org sweep's job (V2-1767). Closes V2-1800 Co-Authored-By: Claude Opus 5.5 --- .github/scripts/audit_regressions.sh | 86 ++++++++++++++++++++++++++++ .github/workflows/ci.yml | 64 ++++++++++++++++++--- 2 files changed, 143 insertions(+), 7 deletions(-) create mode 100755 .github/scripts/audit_regressions.sh diff --git a/.github/scripts/audit_regressions.sh b/.github/scripts/audit_regressions.sh new file mode 100755 index 00000000..439a7a01 --- /dev/null +++ b/.github/scripts/audit_regressions.sh @@ -0,0 +1,86 @@ +#!/usr/bin/env bash +# Fail if a lockfile has a RustSec vulnerability that its base lockfile doesn't. +# +# audit_regressions.sh HEAD_LOCK [BASE_LOCK] +# +# A PR's Security Audit should fail on an advisory the PR brings in, and not on +# one that was published against a dependency main already had: that one would +# red every open PR the day it lands, whatever the PR changes (V2-1800). So +# this audits both lockfiles against the same advisory database and fails only +# on (advisory, crate) pairs that are in HEAD_LOCK and not in BASE_LOCK. Moving +# an already-vulnerable crate to another vulnerable version is not new. The +# vulnerabilities both share are listed as warnings, so they stay visible. +# +# With no BASE_LOCK every vulnerability in HEAD_LOCK counts as new. Ignores in +# .cargo/audit.toml in the working directory apply to both audits. +set -euo pipefail + +head_lock=$1 +base_lock=${2:-} +work=$(mktemp -d) +trap 'rm -rf "$work"' EXIT + +# audit LOCKFILE REPORT [cargo audit args] +audit() { + # cargo audit exits non-zero when it finds a vulnerability, so its status + # can't tell a finding from a crash. A run that didn't produce a report fails + # the check on the jq test below instead. + cargo audit --json --file "$1" "${@:3}" > "$2" || true + if ! jq -e '.vulnerabilities.list | type == "array"' "$2" > /dev/null 2>&1; then + echo "::error::cargo audit produced no report for $1" + cat "$2" + exit 1 + fi +} + +# One line per vulnerability: "ID CRATE VERSION: title". The first two fields +# are what "new" is judged on. +vulnerabilities() { + jq -r '.vulnerabilities.list[] | "\(.advisory.id) \(.package.name) \(.package.version): \(.advisory.title)"' "$1" | sort -u +} + +# One line per informational warning (unmaintained, unsound, yanked). +warnings() { + jq -r '.warnings // {} | to_entries[] | .value[] + | "\(.kind) \(.advisory.id // "-") \(.package.name) \(.package.version)"' "$1" | sort -u +} + +# The first audit fetches the advisory database; the second reuses that copy, +# so both lockfiles are judged against the same advisories. +audit "$head_lock" "$work/head.json" +vulnerabilities "$work/head.json" > "$work/head.txt" +warnings "$work/head.json" > "$work/head-warnings.txt" +if [ -n "$base_lock" ]; then + audit "$base_lock" "$work/base.json" --no-fetch + vulnerabilities "$work/base.json" > "$work/base.txt" + warnings "$work/base.json" > "$work/base-warnings.txt" +else + : > "$work/base.txt" + : > "$work/base-warnings.txt" +fi + +# Split HEAD's vulnerabilities by whether BASE has the same advisory for the +# same crate. (FILENAME, not NR == FNR: that is also true on the second file +# when the first is empty.) +awk 'FILENAME == ARGV[1] { seen[$1 " " $2] = 1; next } !(($1 " " $2) in seen)' \ + "$work/base.txt" "$work/head.txt" > "$work/new.txt" +awk 'FILENAME == ARGV[1] { seen[$1 " " $2] = 1; next } ($1 " " $2) in seen' \ + "$work/base.txt" "$work/head.txt" > "$work/existing.txt" +# Informational warnings are never fatal; the new ones get an annotation. +comm -13 "$work/base-warnings.txt" "$work/head-warnings.txt" > "$work/new-warnings.txt" + +while IFS= read -r line; do + echo "::warning::already in the base lockfile, not introduced here: $line" +done < "$work/existing.txt" +while IFS= read -r line; do + echo "::warning::new informational advisory: $line" +done < "$work/new-warnings.txt" + +echo "vulnerabilities: $(wc -l < "$work/head.txt") in $head_lock," \ + "$(wc -l < "$work/existing.txt") already in the base, $(wc -l < "$work/new.txt") new" +if [ -s "$work/new.txt" ]; then + while IFS= read -r line; do + echo "::error::introduced by this change: $line" + done < "$work/new.txt" + exit 1 +fi diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 58bcb0b4..77508d93 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -25,6 +25,7 @@ env: # tests without default features # filesystems the storage tests on ext4, XFS and btrfs # build-release the shipped Linux binary, built exactly as release.yml does +# security RustSec vulnerabilities the PR adds to Cargo.lock jobs: quick-checks: name: Quick checks @@ -258,27 +259,76 @@ jobs: fi echo "no failpoint in the release build" - # Kept outside CI Gate until V2-1800 settles how advisories are handled: an - # advisory comes from the RustSec database, not from the PR, so today this - # reds every PR (RUSTSEC-2026-0387, V2-1743). + # Fails on a RustSec vulnerability that the PR brings into Cargo.lock (V2-1800). + # + # A plain `cargo audit` fails on every advisory in the lockfile. A new advisory + # comes from the RustSec database, not from a PR, so the day one is published + # against a dependency main already has, every open PR goes red. That made + # this job red on 43 of 50 PRs (RUSTSEC-2026-0387, V2-1743), and the red + # stopped meaning anything. So the job audits the base branch's lockfile and + # the PR's against the same database, and fails only on what the PR adds. + # Advisories already on main are printed as warnings. A PR that doesn't + # change Cargo.lock passes without auditing, since it can't add anything. + # + # Finding advisories newly published against main is the daily org-wide sweep + # (V2-1767), not this job. security: name: Security Audit runs-on: ubuntu-latest timeout-minutes: 20 steps: - uses: actions/checkout@v4 + with: + # The PR's merge commit and its first parent, the base branch tip. + fetch-depth: 2 + - name: Does this PR change the lockfile or the audit? + id: inputs + shell: bash + run: | + set -euo pipefail + # Fail safe: anything other than a merge commit with its base parent + # present audits the whole lockfile, with nothing counted as existing. + if ! git rev-parse --verify -q 'HEAD^2' > /dev/null \ + || ! git rev-parse --verify -q 'HEAD^1' > /dev/null; then + echo "not a pull request merge commit; auditing everything" + echo "audit=true" >> "$GITHUB_OUTPUT" + echo "base=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + changed=$(git diff --name-only 'HEAD^1' HEAD -- \ + Cargo.lock .cargo/audit.toml .github/scripts/audit_regressions.sh .github/workflows/ci.yml) + if [ -n "$changed" ]; then + echo "inputs changed:" + echo "$changed" + echo "audit=true" >> "$GITHUB_OUTPUT" + else + echo "::notice::No change to Cargo.lock, .cargo/audit.toml or the audit itself; nothing new to audit" + echo "audit=false" >> "$GITHUB_OUTPUT" + fi + echo "base=true" >> "$GITHUB_OUTPUT" - uses: dtolnay/rust-toolchain@stable + if: steps.inputs.outputs.audit == 'true' - name: Install cargo-audit - run: cargo install cargo-audit - - name: Run security audit - run: cargo audit + if: steps.inputs.outputs.audit == 'true' + run: cargo install cargo-audit --locked --version 0.22.2 + - name: Audit for advisories this PR introduces + if: steps.inputs.outputs.audit == 'true' + shell: bash + run: | + set -euo pipefail + if [ "${{ steps.inputs.outputs.base }}" = true ]; then + git show 'HEAD^1:Cargo.lock' > "$RUNNER_TEMP/base.lock" + bash .github/scripts/audit_regressions.sh Cargo.lock "$RUNNER_TEMP/base.lock" + else + bash .github/scripts/audit_regressions.sh Cargo.lock + fi # The single check that says whether a PR is green. It fails unless every job # it needs succeeded; a skipped or cancelled job counts as a failure. gate: name: CI Gate if: always() - needs: [quick-checks, msrv, notices, test, filesystems, build-release] + needs: [quick-checks, msrv, notices, test, filesystems, build-release, security] runs-on: ubuntu-latest timeout-minutes: 5 steps: From 5cae04903605e76a3f5b8547c196bdfee98f9e91 Mon Sep 17 00:00:00 2001 From: Nic-dorman Date: Thu, 5 Nov 2026 12:45:19 +0000 Subject: [PATCH 2/2] ci(security): audit with the base branch's ignores, not the PR's Hermes round 1: cargo audit reads .cargo/audit.toml from its working directory, so both audits ran with the PR's ignores. A PR could add an ignore for the advisory it introduces and hide it from both reports, and the gate passed. Reproduced: a lockfile with rustls-webpki 0.103.15 (RUSTSEC-2026-0382) plus an ignore for it passed with "0 new". audit_regressions.sh now takes --config AUDIT_TOML and runs cargo audit from a scratch directory holding only that file, so the checkout's config is never read. (cargo audit 0.22 reads only ./.cargo/audit.toml and $CARGO_HOME/audit.toml, checked on dev2.) CI passes the base branch's copy, or an empty file when the base has none, and warns when the PR changes .cargo/audit.toml: its ignores apply once it merges. The log lists the ignores that were applied. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/audit_regressions.sh | 34 ++++++++++++++++++++++------ .github/workflows/ci.yml | 17 +++++++++++++- 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/.github/scripts/audit_regressions.sh b/.github/scripts/audit_regressions.sh index 439a7a01..55a236e4 100755 --- a/.github/scripts/audit_regressions.sh +++ b/.github/scripts/audit_regressions.sh @@ -1,7 +1,7 @@ #!/usr/bin/env bash # Fail if a lockfile has a RustSec vulnerability that its base lockfile doesn't. # -# audit_regressions.sh HEAD_LOCK [BASE_LOCK] +# audit_regressions.sh [--config AUDIT_TOML] HEAD_LOCK [BASE_LOCK] # # A PR's Security Audit should fail on an advisory the PR brings in, and not on # one that was published against a dependency main already had: that one would @@ -11,21 +11,40 @@ # an already-vulnerable crate to another vulnerable version is not new. The # vulnerabilities both share are listed as warnings, so they stay visible. # -# With no BASE_LOCK every vulnerability in HEAD_LOCK counts as new. Ignores in -# .cargo/audit.toml in the working directory apply to both audits. +# With no BASE_LOCK every vulnerability in HEAD_LOCK counts as new. +# +# Only the ignores in AUDIT_TOML apply, to both audits; with no --config, none +# do. cargo audit reads .cargo/audit.toml from its working directory, so it runs +# from a scratch directory holding only AUDIT_TOML. CI passes the base branch's +# copy: if a PR's own ignores applied, the PR could add an ignore for the +# advisory it introduces and hide it from both audits. (cargo audit also reads +# $CARGO_HOME/audit.toml, which belongs to the runner, not the PR.) set -euo pipefail -head_lock=$1 -base_lock=${2:-} +config= +if [ "${1:-}" = --config ]; then + config=$2 + shift 2 + [ -f "$config" ] || { echo "::error::no audit config at $config"; exit 1; } +fi +# Absolute, since cargo audit runs from the scratch directory. +head_name=$1 +absolute() { case $1 in /*) echo "$1" ;; *) echo "$PWD/$1" ;; esac; } +head_lock=$(absolute "$1") +base_lock=${2:+$(absolute "$2")} work=$(mktemp -d) trap 'rm -rf "$work"' EXIT +mkdir -p "$work/cwd/.cargo" +if [ -n "$config" ]; then + cp "$config" "$work/cwd/.cargo/audit.toml" +fi # audit LOCKFILE REPORT [cargo audit args] audit() { # cargo audit exits non-zero when it finds a vulnerability, so its status # can't tell a finding from a crash. A run that didn't produce a report fails # the check on the jq test below instead. - cargo audit --json --file "$1" "${@:3}" > "$2" || true + (cd "$work/cwd" && cargo audit --json --file "$1" "${@:3}") > "$2" || true if ! jq -e '.vulnerabilities.list | type == "array"' "$2" > /dev/null 2>&1; then echo "::error::cargo audit produced no report for $1" cat "$2" @@ -48,6 +67,7 @@ warnings() { # The first audit fetches the advisory database; the second reuses that copy, # so both lockfiles are judged against the same advisories. audit "$head_lock" "$work/head.json" +echo "ignores applied: $(jq -r '.settings.ignore | if length == 0 then "none" else join(", ") end' "$work/head.json")" vulnerabilities "$work/head.json" > "$work/head.txt" warnings "$work/head.json" > "$work/head-warnings.txt" if [ -n "$base_lock" ]; then @@ -76,7 +96,7 @@ while IFS= read -r line; do echo "::warning::new informational advisory: $line" done < "$work/new-warnings.txt" -echo "vulnerabilities: $(wc -l < "$work/head.txt") in $head_lock," \ +echo "vulnerabilities: $(wc -l < "$work/head.txt") in $head_name," \ "$(wc -l < "$work/existing.txt") already in the base, $(wc -l < "$work/new.txt") new" if [ -s "$work/new.txt" ]; then while IFS= read -r line; do diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 77508d93..d4156fdd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -318,7 +318,22 @@ jobs: set -euo pipefail if [ "${{ steps.inputs.outputs.base }}" = true ]; then git show 'HEAD^1:Cargo.lock' > "$RUNNER_TEMP/base.lock" - bash .github/scripts/audit_regressions.sh Cargo.lock "$RUNNER_TEMP/base.lock" + # The base branch's ignores, not the PR's: an ignore the PR adds + # would hide the advisory it is meant to catch from both audits. + # A change to them takes effect once it is on the base branch. + if git cat-file -e 'HEAD^1:.cargo/audit.toml' 2> /dev/null; then + git show 'HEAD^1:.cargo/audit.toml' > "$RUNNER_TEMP/base-audit.toml" + else + : > "$RUNNER_TEMP/base-audit.toml" + fi + if ! git diff --quiet 'HEAD^1' HEAD -- .cargo/audit.toml; then + echo "::warning::This PR changes .cargo/audit.toml. The audit applies the base branch's ignores; the PR's apply once it merges." + git diff 'HEAD^1' HEAD -- .cargo/audit.toml + fi + bash .github/scripts/audit_regressions.sh --config "$RUNNER_TEMP/base-audit.toml" \ + Cargo.lock "$RUNNER_TEMP/base.lock" + elif [ -f .cargo/audit.toml ]; then + bash .github/scripts/audit_regressions.sh --config .cargo/audit.toml Cargo.lock else bash .github/scripts/audit_regressions.sh Cargo.lock fi