diff --git a/.github/scripts/policy-sign.mjs b/.github/scripts/policy-sign.mjs index 63d349d29..3c7c45556 100644 --- a/.github/scripts/policy-sign.mjs +++ b/.github/scripts/policy-sign.mjs @@ -1,8 +1,9 @@ // A pull request that edits an acceptance gate needs another person's signature (#2054). // // The gates that judge this repository — `merge-gate.mjs`, `review-gate.mjs`, `pass-label.mjs`, -// `risk-gate.mjs` and `scripts/self-test-catalog.mjs` — live in the same repository they judge, so a -// pull request can change what "pass" means and every gate downstream will read the new rule. This +// `risk-gate.mjs`, `enqueue-guard.mjs`, `enqueue-rules.mjs` and `scripts/self-test-catalog.mjs` — live +// in the same repository they judge, so a pull request can change what "pass" means and every gate +// downstream will read the new rule. This // is the smallest structural step against that: `policy/gates.json` names the files, and a pull // request touching one of them must carry `Policy-sign: ` in its body, naming a // comment written by someone other than the pull request's author. A person cannot sign their own diff --git a/.github/scripts/policy-sign.test.mjs b/.github/scripts/policy-sign.test.mjs index e095fe1b8..9a5e793f9 100644 --- a/.github/scripts/policy-sign.test.mjs +++ b/.github/scripts/policy-sign.test.mjs @@ -116,6 +116,47 @@ test('#2054: a signature written by a bot account is refused', () => { assert.match(verdict.reason, /bot account/) }) +// #2093: the two files that decide whether a pull request may enter the merge queue are gates too. +// A pull request that loosens `enqueue-rules.mjs` or the guard that posts its verdict can otherwise +// merge with nobody else's signature, while one that touches `merge-gate.mjs` cannot. +test('#2093: a change to the enqueue guard needs another person\'s signature', () => { + const file = '.github/scripts/enqueue-guard.mjs' + const unsigned = judge({ files: ['core/src/x.js', file], body: 'Fixes #1\n\nEases the queue gate.', ...PR, head: HEAD, comments: [], policy }) + assert.equal(unsigned.ok, false, 'the guard that decides what may enter the queue is a gate file') + assert.match(unsigned.reason, /`Policy-sign:`/) + assert.match(unsigned.reason, /enqueue-guard\.mjs/, 'the refusal names the file that made the signature required') + + const stale = judge({ + files: [file], + body: 'Fixes #1\n\nPolicy-sign: 42', + ...PR, + head: HEAD, + comments: [{ id: 42, body: `ok at ${OTHER_TIP}`, user: { login: 'Birfy', type: 'User' } }], + policy, + }) + assert.equal(stale.ok, false, 'a signature for another tip is refused here exactly as for merge-gate.mjs') + assert.match(stale.reason, /does not name this tip/) +}) + +test('#2093: a change to the enqueue rules needs another person\'s signature', () => { + const file = '.github/scripts/enqueue-rules.mjs' + const unsigned = judge({ files: [file], body: 'Fixes #1\n\nEases the queue rules.', ...PR, head: HEAD, comments: [], policy }) + assert.equal(unsigned.ok, false, 'the rules the guard reads are a gate file') + assert.match(unsigned.reason, /`Policy-sign:`/) + assert.match(unsigned.reason, /enqueue-rules\.mjs/, 'the refusal names the file that made the signature required') + + const stale = judge({ + files: [file], + body: 'Fixes #1\n\nPolicy-sign: 42', + ...PR, + head: HEAD, + comments: [{ id: 42, body: `ok at ${OTHER_TIP}`, user: { login: 'Birfy', type: 'User' } }], + policy, + }) + assert.equal(stale.ok, false, 'a signature for another tip is refused here exactly as for merge-gate.mjs') + assert.match(stale.reason, /does not name this tip/) +}) + test('#2054: the line is read from the body only, on its own line', () => { assert.equal(parseSignature('Fixes #1\n\nPolicy-sign: 42\n'), '42') assert.equal(parseSignature('Fixes #1\n\n> Policy-sign: 42'), undefined, 'a quoted line is prose, not the signature') diff --git a/changelog.d/2093-gate-sign-coverage.changed.md b/changelog.d/2093-gate-sign-coverage.changed.md new file mode 100644 index 000000000..b9ea80449 --- /dev/null +++ b/changelog.d/2093-gate-sign-coverage.changed.md @@ -0,0 +1 @@ +The gate signature now covers the queue gates it enforces: a pull request that changes `.github/scripts/enqueue-guard.mjs` or `.github/scripts/enqueue-rules.mjs` must carry a `Policy-sign:` line naming a comment by someone other than its author, and that comment must name the head, exactly as a change to `merge-gate.mjs` already did — a pull request can no longer loosen what may enter the merge queue without a second person's signature ([#2093](https://github.com/ThinkFlowLab/FlyBot/issues/2093)). diff --git a/docs/governance.md b/docs/governance.md index 5c3b0bc24..2b158a214 100644 --- a/docs/governance.md +++ b/docs/governance.md @@ -142,10 +142,11 @@ store, and the file it archives is the durable part. ## 9. Changing a gate needs a second person -The acceptance gates — `merge-gate`, `review-gate`, `pass-label`, `risk-gate`, -`scripts/self-test-catalog.mjs`, and the checker and workflow step that enforce this rule — are -listed in [`policy/gates.json`](../policy/gates.json). A pull request that edits one of them, or -anything under `policy/`, must carry one line in its body: +The acceptance gates — `merge-gate`, `review-gate`, `pass-label`, `risk-gate`, the +`enqueue-guard` and `enqueue-rules` that decide the merge queue, `scripts/self-test-catalog.mjs`, +and the checker and workflow step that enforce this rule — are listed in +[`policy/gates.json`](../policy/gates.json). A pull request that edits one of them, or anything +under `policy/`, must carry one line in its body: ``` Policy-sign: diff --git a/policy/gates.json b/policy/gates.json index 21a301467..2cd3933a9 100644 --- a/policy/gates.json +++ b/policy/gates.json @@ -9,6 +9,8 @@ ".github/scripts/risk-gate.mjs", "scripts/self-test-catalog.mjs", ".github/scripts/policy-sign.mjs", + ".github/scripts/enqueue-guard.mjs", + ".github/scripts/enqueue-rules.mjs", ".github/workflows/test.yml", "policy/" ]