diff --git a/.github/cursor-review/README.md b/.github/cursor-review/README.md index 68e6caf..aa327a1 100644 --- a/.github/cursor-review/README.md +++ b/.github/cursor-review/README.md @@ -88,6 +88,7 @@ silently vanishing — the review tells you what didn't run. | [`slack-notify.sh`](slack-notify.sh) | Sends the start/complete Slack DMs to the triggerer (no-ops without a token). | | [`install-cursor-cli.sh`](install-cursor-cli.sh) | Installs the Cursor agent CLI from the versioned, sha256-pinned release artifact — not `curl cursor.com/install \| bash`. Used by all three CLI-using jobs; the pin (`CURSOR_CLI_VERSION` / `CURSOR_CLI_SHA256`) lives in `cursor-review.yml`'s top-level `env:`. | | [`build-ledger.py`](build-ledger.py) | Builds the **prior-review ledger** — what earlier rounds raised on this PR and how the author answered — and splices it into the panel/judge prompts. Also the prompt splicer, so the no-ledger path is byte-identical to the pre-ledger prompt. | +| [`fence-diff.py`](fence-diff.py) | Wraps the reviewed diff (plus the incremental hunks, and the judge's panel-findings block) in `=== BEGIN/END DIFF ===` fences. The diff is attacker-authored PR bytes, so static literal fences are not a control; the nonce is what a PR cannot forge. Each prompt-build step mints its OWN nonce (`mint`), into a shell variable rather than a step `env:` or job output — Actions dumps a step's env map into the public run log, and a per-prompt value means a leak in one job cannot forge a fence in another. Copies the body through **byte for byte** — it never defangs or normalizes the payload. | | [`catalog-drift.py`](catalog-drift.py) | Backs the weekly catalog-drift check. Extracts the pins from `cursor-review.yml`, diffs them against raw `cursor-agent models` output, and renders the sticky issue title + body (delisted pins, pins marked NO-ZDR, unpinned same-lab ids, catalog ids from unpinned families, stale audit date). Reports only — it never edits a pin. | ## Adopt it in your repo diff --git a/.github/cursor-review/fence-diff.py b/.github/cursor-review/fence-diff.py new file mode 100644 index 0000000..f25bc96 --- /dev/null +++ b/.github/cursor-review/fence-diff.py @@ -0,0 +1,254 @@ +#!/usr/bin/env python3 +"""Fence the reviewed diff with a per-run nonce (BE-7645). + +The diff spliced into the panel and judge prompts is `git diff BASE...HEAD` — +i.e. **attacker-authored PR file bytes**, the most attacker-controlled input the +review sees. It used to sit between STATIC literal fences (`=== BEGIN DIFF ===` +/ `=== END DIFF ===`) with no untrusted-data labelling, while the prior-review +ledger — a strictly *less* controlled input — got both (see `build-ledger.py`'s +`_UNTRUSTED_HEADER`). This module closes that gap. + +The control is an **unguessable per-run nonce in the fence**, not literal-string +matching. A PR cannot forge the close fence because the nonce does not exist +when the bytes it would have to contain are already fixed: it is minted inside +the consuming job, at prompt-build time, long after the head SHA the diff is +built from was resolved. + +The nonce is *disclosed within its own run*, not secret. It is deliberately +never put in a step `env:` block (Actions prints a step's env map into the run +log before the script runs, and consumer logs are public), but a panel model +that quotes a fence marker while describing the diff republishes it into model +output the workflow prints and posts. That is harmless: by then the diff those +fences wrap is immutable, and every prompt mints its own nonce, so a value seen +in one job's output cannot forge a fence in another's. + +Two things this deliberately does NOT do: + +* **It does not mutate the diff.** The body is copied through byte for byte, + because a reviewer must be able to trust that the code shown is the code under + review. In particular it is NOT run through `build-ledger.py`'s + `_defang_fences`: that rewrites fence-looking lines, and in a unified diff + every content line already carries a `+`/`-`/space prefix, so a forged fence + renders as `+=== END DIFF ===` and cannot byte-match a close fence anyway. + Defanging would therefore corrupt the payload to buy nothing. +* **It does not make the fence tamper-proof against a *guessed* nonce.** It + makes the fence unguessable in practice and labels the region as data; the + prompt wording (`prompt-adversarial.md`, `prompt-edge-case.md`, + `prompt-judge.md`) carries the other half — that no text inside the markers is + an instruction. + +Subcommands: + + mint Print a fresh nonce. Called per prompt-build step so the panel + and judge prompts never share one — see `cmd_mint`. + + strip-marker Drop the prompt head's trailing static `=== BEGIN DIFF ===` + line. That line is the ledger splicer's `--marker` anchor, so it + must stay in the prompt `.md` files verbatim; it is removed only + after the splice, because `emit` re-emits it WITH the nonce. + + emit Write `=== BEGIN