diff --git a/.github/workflows/no-mistakes-required.yml b/.github/workflows/no-mistakes-required.yml index 514bae2..4d01a44 100644 --- a/.github/workflows/no-mistakes-required.yml +++ b/.github/workflows/no-mistakes-required.yml @@ -44,6 +44,7 @@ jobs: PR_BODY: ${{ github.event.pull_request.body }} PR_AUTHOR: ${{ github.event.pull_request.user.login }} PR_NUMBER: ${{ github.event.pull_request.number }} + PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} run: | set -eu marker='Updates from [git push no-mistakes](https://github.com/kunchenguid/no-mistakes)' @@ -170,7 +171,31 @@ jobs: exit 1 fi - echo "Attestation head_sha: $(printf '%s' "$payload" | jq -r '.head_sha // "(absent)"')" + attested_head="$(printf '%s' "$payload" | jq -r '.head_sha // ""')" + echo "Attestation head_sha: ${attested_head:-(absent)}" + + # Head binding. The attestation describes the commit no-mistakes ran + # its steps on; a later push moves the PR head without rewriting the + # body, so an attestation that does not name the current head proves + # nothing about the code being merged. A `synchronize` whose body was + # NOT rewritten by no-mistakes going red is the intended contract, not + # a false positive. + if [ -z "$attested_head" ] || [ -z "${PR_HEAD_SHA:-}" ] || [ "$attested_head" != "$PR_HEAD_SHA" ]; then + echo "::error::The no-mistakes pipeline attestation is STALE for the current head of PR #${PR_NUMBER}." + { + echo + echo "Attestation head_sha: ${attested_head:-(absent)}" + echo "PR head sha: ${PR_HEAD_SHA:-(absent)}" + echo + echo "A commit was pushed after the no-mistakes run, so the attestation does not" + echo "describe the code this PR now proposes to merge." + echo + echo "Re-run 'git push no-mistakes' to refresh it." + echo + echo "PR author: ${PR_AUTHOR}" + } >&2 + exit 1 + fi tab="$(printf '\t')" gate_status=0 diff --git a/test/no-mistakes-gate.test.mjs b/test/no-mistakes-gate.test.mjs index 4ac5aef..85a3742 100644 --- a/test/no-mistakes-gate.test.mjs +++ b/test/no-mistakes-gate.test.mjs @@ -45,13 +45,14 @@ if (process.env.CI && !runnable) { ); } -function runGate(body) { +function runGate(body, headSha = HEAD_SHA) { const result = spawnSync("bash", [scriptPath], { env: { ...process.env, PR_BODY: body, PR_AUTHOR: "somedev", PR_NUMBER: "42", + PR_HEAD_SHA: headSha, }, encoding: "utf8", }); @@ -80,9 +81,9 @@ function prBody(attestationPayload) { ].join("\n"); } -function attestation(steps) { +function attestation(steps, headSha = HEAD_SHA) { return JSON.stringify({ - head_sha: HEAD_SHA, + head_sha: headSha, steps: steps.map(([step, status]) => ({ step, status })), }); } @@ -177,6 +178,48 @@ describe("no-mistakes PR gate", { skip: !runnable }, () => { }); } + // Head binding: the attestation describes the commit no-mistakes ran on, so + // an attestation naming any other commit says nothing about what is being + // merged. A synchronize whose body was not rewritten by no-mistakes going red + // is the contract, not a false positive. + it("accepts an attestation whose head_sha is the PR's current head", () => { + const { code, output } = runGate( + prBody(attestation(HEALTHY_STEPS, HEAD_SHA)), + HEAD_SHA, + ); + assert.equal(code, 0); + assert.match(output, new RegExp(`Attestation head_sha: ${HEAD_SHA}`)); + }); + + it("rejects an attestation whose head_sha is not the PR's current head", () => { + const staleSha = "0000000000000000000000000000000000000000"; + const { code, output } = runGate( + prBody(attestation(HEALTHY_STEPS, staleSha)), + HEAD_SHA, + ); + assert.equal(code, 1); + assert.match(output, /attestation is STALE for the current head/); + assert.match(output, /Re-run 'git push no-mistakes' to refresh it/); + assert.ok(output.includes(staleSha) && output.includes(HEAD_SHA)); + }); + + it("fails closed when the attestation carries no head_sha at all", () => { + const payload = JSON.stringify({ + steps: HEALTHY_STEPS.map(([step, status]) => ({ step, status })), + }); + const { code, output } = runGate(prBody(payload), HEAD_SHA); + assert.equal(code, 1); + assert.match(output, /attestation is STALE for the current head/); + assert.match(output, /Attestation head_sha: \(absent\)/); + }); + + it("fails closed when the PR head sha is unavailable", () => { + const { code, output } = runGate(prBody(attestation(HEALTHY_STEPS)), ""); + assert.equal(code, 1); + assert.match(output, /attestation is STALE for the current head/); + assert.match(output, /PR head sha: +\(absent\)/); + }); + it("fails closed on an attestation payload that is not valid JSON", () => { const { code, output } = runGate( prBody('{"head_sha":"abc","steps":[{"step":"review",'),