From 5bdabe9ce826766817f1802d07ebceb79dbaf175 Mon Sep 17 00:00:00 2001 From: t Date: Tue, 15 Sep 2026 23:31:13 +0530 Subject: [PATCH] Fail a pull request for the advisories it introduces, not the world's `dependency-security.yml` ran only the unconditional `cargo audit`, which answers "does this lockfile contain an accepted-severity advisory" and has no concept of before and after. On 2026-09-15 `RUSTSEC-2026-0285` was published against `rustls` (transitively via `reqwest`) and that step went red on an unchanged master, turning all nine then-open pull requests red at once -- including ones touching neither crate. `scripts/check-advisory-delta.mjs` was written for exactly this, with contract tests, and wired into nothing: its only reference anywhere under `.github/` or `scripts/` was its own test file. `docs/proposed-dependency-policy.md` proposed the wiring and deliberately stopped short of applying it. This applies section 1 of that proposal, and nothing else. The differential step is **additional, never a replacement**. The unconditional audit still runs and still fails on a real advisory whoever introduced it -- a pull request opened while master is genuinely broken must still be visibly blocked by something. What the new step adds is the question a pull request's author can act on: did *this* change introduce one. It runs on pull requests only, because a `push: master` run has no other side to diff against. `actions/checkout` gains `fetch-depth: 0`: the default shallow clone has no merge base to resolve, and fetching only the pull request ref would not bring the base branch with it. One thing the proposal predates. It was written while this workflow ran only for pull requests targeting master, which #401 changed. The script already handles it: `resolveBaseCommit` prefers `origin/$GITHUB_BASE_REF` before falling back to `origin/master`, so a stacked pull request diffs against its own base rather than against master. Verified rather than assumed -- `GITHUB_BASE_REF=claude/voucher-presence` against #308's head resolves `via origin/claude/voucher-presence` to 64b8666b, that branch's tip. Driven both directions on the real commit pair for RUSTSEC-2026-0285 rather than asserted. Base 9558a316 to head 8b17f95c reports the advisory introduced and exits 1; reversed, it reports the advisory resolved and exits 0. On this branch the default invocation resolves via `origin/master` and reports the lockfile byte-identical. The failing path exits non-zero with a complete, actionable message carried on a thrown Error, which produces a stack trace. That is left alone deliberately: every sibling check script in `scripts/` signals failure the same way and none calls `process.exit`, so changing this one would make it the odd one out. Not applied: section 2's scheduled daily audit of master, which is a new workflow file and a separate decision. Without it, master's own drift is still only discovered when someone opens a pull request. `dependency-security.yml` is pinned, so surface and matrix are resealed: one hash changed, pin set set-compared against master and unchanged at 212. `check-advisory-delta.mjs` is not pinned and is unmodified. Verified with every exit status read directly: contract tests `check-advisory-delta.test.mjs` pass; `reseal.sh --verify` exit 0; `check-ci-workflow-consistency.mjs` exit 0. The workflow parses: seven steps, the new one last and gated on `github.event_name == 'pull_request'`. Co-Authored-By: Claude Opus 5 --- .github/workflows/dependency-security.yml | 24 +++++++++++++++++++ .../compatibility/compatibility-matrix.json | 2 +- .../compatibility/compatibility-surface.json | 4 ++-- 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/.github/workflows/dependency-security.yml b/.github/workflows/dependency-security.yml index 9fba20b8..9b562cd5 100644 --- a/.github/workflows/dependency-security.yml +++ b/.github/workflows/dependency-security.yml @@ -30,6 +30,11 @@ jobs: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 with: persist-credentials: false + # The differential step below resolves a merge base against the pull + # request's own base branch. The default shallow clone has no merge + # base to find, and fetching only the PR ref would not bring the base + # branch with it. + fetch-depth: 0 - uses: pnpm/action-setup@ea17c68df8912ef543352723c149a84f56e3d413 # v6.1.0 - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v6 with: @@ -42,3 +47,22 @@ jobs: fallback: none - name: Audit locked Rust dependencies run: cargo audit --file src-tauri/Cargo.lock + # Deliberately additional, not a replacement. The step above answers "does + # this lockfile contain an accepted-severity advisory" and must keep + # failing when it does, whoever introduced it. This one answers the + # different question "did THIS pull request introduce one", which is the + # question a pull request's author can actually act on. + # + # On 2026-09-15 `RUSTSEC-2026-0285` was published against `rustls`, and + # the step above went red on an unchanged master, turning all nine then + # open pull requests red at once -- including ones touching neither + # rustls nor reqwest. Keeping both steps means that event still blocks + # master, while a pull request that introduced nothing is told so by name. + # + # Pull requests only: a `push: master` run has no other side to diff + # against. The script resolves its base from GITHUB_BASE_REF first, so a + # pull request stacked on a non-master branch diffs against that branch + # rather than against master. + - name: Fail only on advisories this pull request introduces + if: github.event_name == 'pull_request' + run: node scripts/check-advisory-delta.mjs diff --git a/docs/tally/compatibility/compatibility-matrix.json b/docs/tally/compatibility/compatibility-matrix.json index 3273e529..087dac65 100644 --- a/docs/tally/compatibility/compatibility-matrix.json +++ b/docs/tally/compatibility/compatibility-matrix.json @@ -1,7 +1,7 @@ { "schema_version": 1, "bridge_commit_sha": "be1c20cc3fd66fa1ece196505c69f26e555e4b8e", - "compatibility_surface_sha256": "ad4e2866217ec8778d76a3eb50c951b892d47bd9f786f38cfd3bd1e07e72bd95", + "compatibility_surface_sha256": "c47a6909b4f2ba087221303e611c0d6cfe89fec0fb9e4c9ccb339335d514dc13", "claims": [ { "claim_id": "erp9-6-6-3-windows-education-xml-one-company", diff --git a/docs/tally/compatibility/compatibility-surface.json b/docs/tally/compatibility/compatibility-surface.json index 037a1b20..fb9f8e03 100644 --- a/docs/tally/compatibility/compatibility-surface.json +++ b/docs/tally/compatibility/compatibility-surface.json @@ -7,7 +7,7 @@ }, { "path": ".github/workflows/dependency-security.yml", - "sha256": "fd70b9a317677100d6b760a369495f9e3c6a083d7cdbf8f1916267d97eaa250f" + "sha256": "4f5420aeeeddf9326aad5b3d671468d292a10ca8466fa43125c7f7b735a41282" }, { "path": "docs/adr/0004-tally-write-safety.md", @@ -850,5 +850,5 @@ "sha256": "a8ac2714fecf51947f2822c8c46d7ce2e8602c732780ff60566a7771f0836f9a" } ], - "manifest_sha256": "ad4e2866217ec8778d76a3eb50c951b892d47bd9f786f38cfd3bd1e07e72bd95" + "manifest_sha256": "c47a6909b4f2ba087221303e611c0d6cfe89fec0fb9e4c9ccb339335d514dc13" } \ No newline at end of file