diff --git a/docs/decisions/records/ci/shared-pr-file-enumeration.md b/docs/decisions/records/ci/shared-pr-file-enumeration.md index 0903fbc22..196627dcc 100644 --- a/docs/decisions/records/ci/shared-pr-file-enumeration.md +++ b/docs/decisions/records/ci/shared-pr-file-enumeration.md @@ -74,13 +74,24 @@ in each caller: none) or a local diff, not a tighter check here; the guarantee is stated narrowly rather than left to read as complete. -**Base-ref checkout — now a property of the tree.** `review-verdict.yml` checks out the PR's BASE -ref (`ref: ${{ github.event.pull_request.base.sha }}`, `persist-credentials: false`), never the -head, so the workflow that judges a PR cannot be rewritten by that same PR to weaken its own -judgment. The checkout and its `ref` are the security-relevant parts — a bare `run:` calling the +**Base-ref checkout — binds the SCRIPTS to the base, not the workflow itself.** `review-verdict.yml` +checks out the PR's BASE ref (`ref: ${{ github.event.pull_request.base.sha }}`, +`persist-credentials: false`), never the head, so the *scripts the job executes* — above all +`scripts/pr-changed-files.sh` — come from the already-reviewed base rather than from the PR under +judgment. The checkout and its `ref` are the security-relevant parts: a bare `run:` calling the script would not have been sufficient, since the script would then have come from wherever the runner happened to be. +**It does NOT mean a PR cannot rewrite the gate that judges it (#672).** Gitea resolves a +`pull_request` workflow *definition* from the PR's own head, so a PR editing `review-verdict.yml` +runs its own rewritten copy — which can delete this checkout, or simply post +`review-verdict/h10=success` for its head sha and stop. Branch protection does not close that: it +requires the *context*, not an author, and carries `required_approvals: 0`. An earlier revision of +this paragraph said the workflow "cannot be rewritten by that same PR to weaken its own judgment", +which is true of the scripts and false of the workflow — and stated in the one sentence a reader +resolving this record from the catalog is most likely to stop at. Contained today only by push +access being required; tracked in #672. + An earlier revision of this record stated the requirement in the future tense, because the wiring was staged over two PRs: the workflow runs the BASE version of the gate, and until the shared script existed on `main` a wired workflow would have exited 127 on its own PR and blocked the merge gate @@ -89,8 +100,7 @@ through the combined status. That staging is complete. Both halves are asserted matching it — `head.sha` for `base.sha` is a nine-character diff, and a text-level check would still pass if a second checkout step took the head afterwards and won. -**What is deliberately NOT claimed.** The base-ref checkout closes the self-rewrite route through -the *scripts*; it does not make the workflow immune to everything a PR controls. The `PROTECTED` +**What is deliberately NOT claimed.** Beyond the head-resolution hole above, the `PROTECTED` path list remains the guard that stops a bot-authored PR from editing the gate and exempting itself, and mutation testing was what established that `PROTECTED` is load-bearing only on the BOT path — it and `DOCS_ONLY` are disjoint patterns, so on the docs-only path that clause can never fire. A