diff --git a/.github/scripts/audit_regressions.sh b/.github/scripts/audit_regressions.sh new file mode 100755 index 00000000..55a236e4 --- /dev/null +++ b/.github/scripts/audit_regressions.sh @@ -0,0 +1,106 @@ +#!/usr/bin/env bash +# Fail if a lockfile has a RustSec vulnerability that its base lockfile doesn't. +# +# 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 +# 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. +# +# 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 + +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. + (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" + 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" +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 + 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_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 + 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..d4156fdd 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,91 @@ 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" + # 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 # 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: