From fe02f2e498493f327ec639ccceddb6121a3bcd19 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Mon, 31 Aug 2026 06:27:39 +1000 Subject: [PATCH 1/2] ci(security): tamper-evidence for deny.toml and security.yml (LAB-1151) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The supply-chain gate read its policy and its own definition from the PR head, so one commit could violate a ban and remove the ban. Deletion already fails closed (required context, no bypass actors); this adds the modification half: a tripwire step that diffs deny.toml and security.yml against the PR base and fails the required check unless the PR body carries [gate-change-approved]. Residual (documented in-step and in README): the tripwire itself lives in the PR head; removing it in the same commit is deliberate evasion, only closable by an org-ruleset workflows rule — escalated, not self-served. --- .github/workflows/security.yml | 45 ++++++++++++++++++++++++++++++++++ Makefile | 5 ++-- README.md | 40 ++++++++++++++++++++++++++++-- 3 files changed, 86 insertions(+), 4 deletions(-) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index da24f7e..730f9dc 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -93,6 +93,11 @@ jobs: # This job never pushes; don't leave the token on a self-hosted # runner workspace (zizmor: artipacked). persist-credentials: false + # Depth 2 so the tamper check below can diff HEAD^1 (the base tip + # the merge commit was computed against) without a live network + # fetch — a live base tip drifts under queued re-runs and would + # red-flag innocent PRs for gate changes that landed on main. + fetch-depth: 2 - name: cargo deny (advisories + bans + licenses + sources) # --all-features is load-bearing, not tidiness. cargo-deny builds a @@ -126,3 +131,43 @@ jobs: # Runs even when the step above failed so one run shows both verdicts. if: ${{ !cancelled() }} run: cargo audit + + - name: Gate tamper check (deny.toml / security.yml vs base) + # This job reads its own policy (deny.toml) and its own definition + # (this file) from the PR head, so the PR being gated can weaken the + # gate while keeping the required `supply-chain` context green — drop + # --all-features, append `|| true`, or delete the [bans] entries the + # command faithfully enforces (LAB-1151). Outright deletion fails + # closed (a required context that never reports blocks merge, with no + # bypass actors on ruleset 17788230) — UNLESS the PR ships a + # replacement check with the same name, which is why the second diff + # below also trips on any other changed workflow file that mentions + # supply-chain. This step makes modification fail closed: gate-file + # diffs turn this required check red without the approval marker in + # the PR body (see README "Gate tamper-evidence" for the marker and + # for what this deliberately does not defend against). + # + # HEAD is the PR merge commit, so HEAD^1 is the base tip it was + # computed against — exact PR effect, no network, no base-drift + # false positives (checkout fetch-depth: 2 makes it resolvable). + # PR_BODY enters via env only and is matched by grep as data; it is + # never interpolated into this shell. + if: ${{ github.event_name == 'pull_request' && !cancelled() }} + env: + PR_BODY: ${{ github.event.pull_request.body }} + run: | + set -o pipefail + changed=$(git diff --name-only HEAD^1 HEAD -- deny.toml .github/workflows/security.yml | tr '\n' ' ') + shadow="" + while IFS= read -r f; do + [ -n "$f" ] || continue + if git grep -qe supply-chain HEAD -- "$f"; then shadow="$shadow$f "; fi + done < <(git diff --name-only HEAD^1 HEAD -- '.github/workflows/' ':!.github/workflows/security.yml') + if [ -z "$changed$shadow" ]; then + echo "Gate files unchanged vs PR base." + elif grep -qF -- '[gate-change-approved]' <<<"$PR_BODY"; then + echo "::warning title=Supply-chain gate files changed::${changed}${shadow}differ from the PR base; approval marker present in PR body — confirm the human sign-off in review." + else + echo "::error title=Supply-chain gate tampering::This PR changes ${changed}${shadow}— files that define this gate. If intentional: get human sign-off, add the approval marker documented in README section 'Gate tamper-evidence' to the PR body, then push a commit (empty is fine — body edits alone do not re-trigger, and re-runs reuse the old event payload)." + exit 1 + fi diff --git a/Makefile b/Makefile index 4290544..e231e02 100644 --- a/Makefile +++ b/Makefile @@ -26,9 +26,10 @@ test-wasm: CARGO_TARGET_WASM32_UNKNOWN_UNKNOWN_RUNNER=wasm-bindgen-test-runner \ $(CARGO) test -p cachekit-rs --target wasm32-unknown-unknown --no-default-features --features workers,cachekitio,encryption,macros --test wasm_session_tests -# Supply-chain gate — the same commands CI runs in +# Supply-chain gate — the same enforcement commands CI runs in # .github/workflows/security.yml, so a local pass means a CI pass. (CI runs the -# audit step even when deny fails; make stops at the first failure.) +# audit step even when deny fails, plus a PR-only gate tamper check with no +# local equivalent; make stops at the first failure.) # Kept out of `quick-check`: both tools fetch the RustSec advisory database over # the network, which does not belong in a per-commit loop. # Why both tools, and why --all-features: see the table in README.md. diff --git a/README.md b/README.md index a771728..d2bc207 100644 --- a/README.md +++ b/README.md @@ -505,8 +505,10 @@ make build # cargo build --release make build-wasm # wasm32-unknown-unknown (workers feature) ``` -`make security` runs the same two commands as the `supply-chain` job in -`.github/workflows/security.yml`, so a local pass means a CI pass. It needs +`make security` runs the same two enforcement commands as the `supply-chain` +job in `.github/workflows/security.yml`, so a local pass means a CI pass +(the job's final step, the gate tamper check below, is PR-context-only and +has no local equivalent). It needs `cargo-deny` and `cargo-audit` installed, and it reaches the network to refresh the RustSec advisory database — which is why it is not folded into `quick-check`. @@ -529,6 +531,40 @@ reintroduced behind an optional feature passes a bare `cargo deny check`. and `toxiproxy_rust`, because this SDK is rustls-only. Run `make deny` before adding or bumping a dependency. +### Gate tamper-evidence + +The `supply-chain` check reads both its policy (`deny.toml`) and its own +definition (`security.yml`) from the PR head, so a PR could weaken the gate it +is being graded by — delete a `[bans]` entry, or drop `--all-features` while +keeping the job name green. Two properties defend against that: + +- **Deletion fails closed.** `supply-chain` is a required status check with no + bypass actors; a PR that deletes the workflow leaves the context unreported + and the PR permanently unmergeable. The one deletion variant that would not + fail closed — shipping a replacement check under the same name — is why the + wire below also trips on any other changed workflow file mentioning + `supply-chain`, and why the required check is pinned to the GitHub Actions + app, so an API-posted commit status cannot impersonate it. +- **Modification trips a wire.** The job's final step diffs `deny.toml` and + `security.yml` against the PR's base and fails the required check on any + change, unless the PR body contains the exact, case-sensitive string + `[gate-change-approved]` (add it after human sign-off, *then* push a commit + — the marker is read from the push-time event, so a body edit alone does + not re-trigger). Legitimate policy updates therefore stay possible, but + only as a conscious, loudly-marked act. + +What this does **not** defend against: a PR that edits the tamper-check step +itself out in the same commit; an author who self-serves the marker without +sign-off; and a marker hidden inside an HTML comment, which satisfies the +check but is invisible in the rendered body — when reviewing a gate-file +diff, check the raw PR body, not just the rendered view. All are deliberate +evasion, not the lazy path — each leaves an explicit trail in a reviewed +diff or the PR body source. Closing the first mechanically requires an +org-ruleset `workflows` rule pinning `security.yml` to an out-of-tree ref +(an org-scope decision, tracked on LAB-1151), which would still not protect +`deny.toml` — the wire above remains the only guard on the policy file +itself. + ## Minimum Supported Rust Version **Rust 1.85** or later (Edition 2021). From 73e2955be2d6aebaa98e3ec4167553e08a25024c Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Mon, 31 Aug 2026 06:39:51 +1000 Subject: [PATCH 2/2] =?UTF-8?q?test(ci):=20drop=20--all-features=20from=20?= =?UTF-8?q?supply-chain=20job=20=E2=80=94=20LAB-1151=20negative=20proof=20?= =?UTF-8?q?2,=20DO=20NOT=20MERGE?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/security.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 730f9dc..a1b8085 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -112,7 +112,7 @@ jobs: # silently regenerate Cargo.lock when Cargo.toml has drifted. Without # this, deny grades an uncommitted graph while audit grades the # committed one. - run: cargo deny --locked --all-features check + run: cargo deny --locked check - name: cargo audit (lockfile advisories) # Complements cargo deny rather than duplicating it — see the table in