From 34442240ec7b0ad648f4b989f76f2b90cfa87e04 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 16:20:37 +0000 Subject: [PATCH] =?UTF-8?q?fix(settings,pm):=20deny=20the=20MCP=20enqueue?= =?UTF-8?q?=20class=20=E2=80=94=20enable/disable=5Fpr=5Fauto=5Fmerge=20joi?= =?UTF-8?q?n=20lock=201=20and=20the=20roster=20gate?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `permissions.deny` in `.claude/settings.json` is the enforced half of the dispatch charter's rule that GitHub writes go through the REST proxy and never through an MCP write tool. The enqueue pair — `mcp__github__enable_pr_auto_merge` and `mcp__github__disable_pr_auto_merge` — was the one MCP write class left outside it: hook-guarded only, so a non-governed enqueue could still reach GitHub through MCP under the bound user account, and the prose had to name the exception. The director seat ruled A: the pair joins lock 1. The REST route `PUT .../pulls/{n}/ccr/auto_merge` already serves every real enqueue, so nothing a seat does today is lost, and a tool the runtime refuses cannot be mis-called by a seat that mis-reads the hook's scope. ## What lands - `.claude/settings.json` — the two names appended to `permissions.deny` beside the fifteen `mcp__github__` entries (15 → 17), nothing else moved. - `scripts/pm/check-settings-deny-roster.mjs` — `CONTENT_WRITE_TOOLS` gains the two names in the file's order (the one declaration); the header paragraph that said the enqueue class is NOT in the roster now says it IS and why; the membership rule above the constant covers arming/disarming a landing; a new self-test battery of four fixture cases pins both names, a document denying exactly the roster reading declared = enforced, and a document missing one of the pair naming it; battery floor 7 → 8, self-test 32 → 36 cases. - `.claude/skills/pm-dispatch/references/rest-channel.md` line 53 — the 「auto-merge 备用 MCP 未拒」 clause is dropped in place; the row now says the auto-merge MCP pair is denied by lock 1 too (118 → 117 bytes, one line stays one line, the file stays at its 82-line ceiling). `guard-governed-enqueue.sh` and its self-test are untouched: the hook stays as defence in depth. No changeset — nothing published moves. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01HZfg2AwVX191qCizp88gQr --- .claude/settings.json | 4 +- .../pm-dispatch/references/rest-channel.md | 2 +- scripts/pm/check-settings-deny-roster.mjs | 55 ++++++++++++++----- 3 files changed, 44 insertions(+), 17 deletions(-) diff --git a/.claude/settings.json b/.claude/settings.json index 0acd039e6b3..428a0300bd5 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -64,7 +64,9 @@ "mcp__github__sub_issue_write", "mcp__github__merge_pull_request", "mcp__github__create_repository", - "mcp__github__fork_repository" + "mcp__github__fork_repository", + "mcp__github__enable_pr_auto_merge", + "mcp__github__disable_pr_auto_merge" ] }, "hooks": { diff --git a/.claude/skills/pm-dispatch/references/rest-channel.md b/.claude/skills/pm-dispatch/references/rest-channel.md index b7a6624e48f..82130421bc3 100644 --- a/.claude/skills/pm-dispatch/references/rest-channel.md +++ b/.claude/skills/pm-dispatch/references/rest-channel.md @@ -50,7 +50,7 @@ - ✓ auto-merge 挂载 `PUT .../pulls/{n}/ccr/auto_merge` 带 `{"merge_method":"SQUASH"}`,`DELETE` 卸载。 - ⛔ `PUT .../ccr/auto_merge` 在 draft 上 422 零存储;`DELETE` 无挂载回 422 = 本就没挂,非失败。 - 入队读 timeline `added_to_merge_queue`,落地读 `git rev-list --parents`;⛔ `auto_merge` 与回显都不作数。 -- ⛔ 永不 MCP `update_pull_request`(锁 1 已拒);ready/draft 翻转只走 ccr 路;auto-merge 备用 MCP 未拒。 +- ⛔ 永不 MCP `update_pull_request`(锁 1 已拒);ready/draft 翻转只走 ccr 路;auto-merge MCP 锁 1 同拒。 - 直合仓 `PUT .../pulls/{n}/merge`;actor 记通道令牌:REST 按会话为 `claude[bot]` 或用户,MCP 恒用户。 ## 不可迁移 —— 只有这三件,围着它们排计划;红窗守候规则住 `platform-readings.md` 配额段 diff --git a/scripts/pm/check-settings-deny-roster.mjs b/scripts/pm/check-settings-deny-roster.mjs index 391f20ce0de..5736aad3869 100644 --- a/scripts/pm/check-settings-deny-roster.mjs +++ b/scripts/pm/check-settings-deny-roster.mjs @@ -54,19 +54,23 @@ * tells a seat which tools are closed POINTS at it; a second enumeration is the * hand reconciliation this gate exists to end. * - * ## What is deliberately NOT asserted + * ## The ENQUEUE class is in the roster + * + * `enable_pr_auto_merge` and `disable_pr_auto_merge` are declared closed + * (#18282, ruled A). The REST route `PUT .../pulls/{n}/ccr/auto_merge` in + * `.claude/skills/pm-dispatch/references/rest-channel.md` already serves every + * real enqueue, so denying the MCP pair costs a seat nothing, and a tool the + * runtime refuses cannot be mis-called by a seat that mis-reads the scope of + * the PreToolUse hook `.claude/hooks/guard-governed-enqueue.sh`. That hook + * stays as defence in depth: it judges a governed PR's pinned approval, this + * gate judges membership, and neither reads the other. * - * The ENQUEUE class -- `enable_pr_auto_merge` and `disable_pr_auto_merge` -- is - * not in the roster. Both are a live fallback channel in - * `.claude/skills/pm-dispatch/references/rest-channel.md`, so whether they are - * closed is the maintainer's call and is carded separately (#18282). If they - * are ruled closed, this gate needs exactly one edit -- two more names in the - * constant -- and that is the point of a single constant. + * ## What is deliberately NOT asserted * - * Also not asserted: that `allow` and `deny` agree, that the tools exist on the - * MCP server, or that a session honours either list. This gate holds the one - * property neither prose surface can hold about itself -- that the enforced - * list still equals the declared one. + * That `allow` and `deny` agree, that the tools exist on the MCP server, or + * that a session honours either list. This gate holds the one property neither + * prose surface can hold about itself -- that the enforced list still equals + * the declared one. * * ## Exit contract * @@ -99,9 +103,9 @@ import { isEntrypoint } from '../invoked-as.mjs'; * * Membership rule, so the next editor does not have to infer it: a tool is here * when invoking it PUBLISHES or MUTATES repository content -- an issue, a - * comment, a review body, a pull request, a branch, a file, a repository. The - * enqueue pair is deliberately absent (see the header). Read-only tools are not - * in this namespace question at all. + * comment, a review body, a pull request, a branch, a file, a repository -- or + * arms or disarms a pull request's landing, which is the enqueue pair (see the + * header). Read-only tools are not in this namespace question at all. */ export const CONTENT_WRITE_TOOLS = Object.freeze([ 'mcp__github__issue_write', @@ -119,6 +123,8 @@ export const CONTENT_WRITE_TOOLS = Object.freeze([ 'mcp__github__merge_pull_request', 'mcp__github__create_repository', 'mcp__github__fork_repository', + 'mcp__github__enable_pr_auto_merge', + 'mcp__github__disable_pr_auto_merge', ]); /** The namespace this gate judges. Entries outside it are another subsystem's. */ @@ -337,12 +343,13 @@ const SELF_TEST_BATTERIES = Object.freeze({ 'settingsPathFromArgv': 3, 'the exit contract': 4, 'the shipped .claude/settings.json': 4, + 'the enqueue class — declared closed': 4, 'the dispatch-gates declaration': 3, }); // DELETING an entry silences that battery's floor exactly as effectively as // zeroing it, so the roster's own size is pinned too. -const SELF_TEST_BATTERY_FLOOR = 7; +const SELF_TEST_BATTERY_FLOOR = 8; // The key an assertion is filed under when no battery is open. It is not a // declared battery, so it reds by the same set difference rather than silently @@ -436,6 +443,24 @@ function selfTest() { assert('the shipped deny list declares no tool this roster lacks', verdict(shipped.deny, CONTENT_WRITE_TOOLS).drift, []); assert('every shipped deny entry is a string', shipped.deny.every((entry) => typeof entry === 'string'), true); + // --- the enqueue class ------------------------------------------------- + // + // Ruled closed (see the header). Pinned on FIXTURES, not on the shipped + // file: the shipped-file battery above deliberately leaves containment to + // the live run, and this battery keeps that line. What it holds is that the + // roster names both tools and that the gate's verdict moves with them. + battery('the enqueue class — declared closed'); + assert('the roster declares the auto-merge arming tool', CONTENT_WRITE_TOOLS.includes('mcp__github__enable_pr_auto_merge'), true); + assert('the roster declares the auto-merge disarming tool', CONTENT_WRITE_TOOLS.includes('mcp__github__disable_pr_auto_merge'), true); + // A settings document denying exactly the roster -- every entry, the pair + // included -- reads declared = enforced in both directions. + const exactlyTheRoster = denyList(settingsWith([...CONTENT_WRITE_TOOLS])); + assert('a document denying exactly the roster reads declared = enforced', verdict(exactlyTheRoster.deny, CONTENT_WRITE_TOOLS), { missing: [], drift: [] }); + // The pre-ruling shape, reproduced: the pair absent from the file is the + // containment finding, and it names the tool. + const withoutDisarm = CONTENT_WRITE_TOOLS.filter((tool) => tool !== 'mcp__github__disable_pr_auto_merge'); + assert('a document missing one of the pair names it as left open', verdict(withoutDisarm, CONTENT_WRITE_TOOLS).missing, ['mcp__github__disable_pr_auto_merge']); + // --- the dispatch-gates declaration ------------------------------------ // // Enforcement cannot hold any of these from here: the declaration is read by