From 768da86b7ec87d5fbd170a489fa83419e4ac7f42 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 16 Sep 2026 11:15:05 +0000 Subject: [PATCH] fix(hooks): watch the ccr auto-merge route in guard-governed-enqueue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The guard's declared job is to refuse an enqueue-class call on a governed PR that carries no authorized approval. It watched three spellings and a compliant seat could reach none of them: both MCP tool names sit in `permissions.deny`, there is no `gh` in the agent container, and the REST arm required a literal `/merge` segment that the protocol's only auto-merge route never carries. Adds the `PUT .../pulls//ccr/auto_merge` route to the same URL reader, feeding the SAME imported predicate chain the `/merge` arm feeds — no second mechanism, no arm weakened, no arm removed. The MCP arm stays as defence in depth. That route, unlike `/merge`, carries a non-enqueue verb on the same path: `DELETE` is the disarm. So the method is read there, and only there — the disarm and the read verbs are allowed, everything else on that path is the mount. `ccr/ready_for_review` is not an enqueue and is not an arm. Self-test: 66 -> 79 cases, covering block / allow / approved on the new spelling, the disarm, the ready flip, the merge arm unchanged under both a named and an unnamed method, and two wired rows (the Bash matcher door, the route named in code) so the arm cannot be dropped silently. Claude-Session: https://claude.ai/code/session_01HPfcjvF23QBoBj7P47DDxs Co-authored-by: Claude --- .../hooks/guard-governed-enqueue.selftest.sh | 68 ++++++++++++++ .claude/hooks/guard-governed-enqueue.sh | 93 +++++++++++++++++-- 2 files changed, 151 insertions(+), 10 deletions(-) diff --git a/.claude/hooks/guard-governed-enqueue.selftest.sh b/.claude/hooks/guard-governed-enqueue.selftest.sh index 2d9f332ea51..3c564ba7b0f 100755 --- a/.claude/hooks/guard-governed-enqueue.selftest.sh +++ b/.claude/hooks/guard-governed-enqueue.selftest.sh @@ -295,6 +295,53 @@ expect allow 'the same Bash spelling on an approved PR' \ "$(bash_call 'gh pr merge 13794 -R objectstack-ai/objectstack')" \ "OS_GOVERNED_ENQUEUE_FIXTURE=$F_PINNED" +echo "== the CCR auto-merge mount is an enqueue, and the ONLY route a seat can reach ==" +# Both MCP tool names sit in `permissions.deny` and there is no `gh` in this +# container, so `PUT .../pulls//ccr/auto_merge` is the spelling a compliant +# seat actually writes — and it carries no literal `/merge` segment, so the +# merge-URL reader above never saw it. Three verdicts are pinned on ONE payload +# shape, so the fixture is the only thing that moves between them and the rows +# cannot all be passing for some reason other than the predicates. +CCR_MOUNT='curl -sS -X PUT https://api.github.com/repos/objectstack-ai/objectstack/pulls/13794/ccr/auto_merge -H "Content-Type: application/json" -d "{}"' +expect block 'a REST PUT .../pulls//ccr/auto_merge through curl' \ + "$(bash_call "$CCR_MOUNT")" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED" +expect_says 'approve BEFORE enqueue' 'the ccr mount earns the SAME refusal, not a second one' \ + "$(bash_call "$CCR_MOUNT")" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED" +expect_says 'AGENTS.md' 'the ccr refusal names the governed hit, so the register really ran' \ + "$(bash_call "$CCR_MOUNT")" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED" +expect allow 'the same ccr mount on a diff that is not governed' \ + "$(bash_call "$CCR_MOUNT")" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_CLEAR" +expect allow 'the same ccr mount with an AUTHORIZED APPROVED review' \ + "$(bash_call "$CCR_MOUNT")" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_PINNED" +expect block 'gh api --method PUT .../pulls//ccr/auto_merge' \ + "$(bash_call 'gh api --method PUT /repos/objectstack-ai/objectstack/pulls/13794/ccr/auto_merge')" \ + "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED" + +# ⛔ THE DISARM IS NOT AN ENQUEUE. `DELETE` on that SAME path unmounts auto-merge +# — the corrective call a seat makes on a PR that should not be queued — so +# refusing it would block the repair this file's own refusal text asks for. The +# method is therefore read on this route. The row is not vacuous: the block row +# at the top of this section is the same URL, the same fixture and the same +# segment shape with `PUT`, so only the method separates them. +expect allow 'DELETE on the ccr auto-merge path is the DISARM, never the mount' \ + "$(bash_call 'curl -sS -X DELETE https://api.github.com/repos/objectstack-ai/objectstack/pulls/13794/ccr/auto_merge')" \ + "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED" +# ...and the ready flip is not an enqueue either: it queues nothing, and it is +# the step that PRODUCES the review this guard is waiting for. +expect allow 'POST .../pulls//ccr/ready_for_review is not an arm' \ + "$(bash_call 'curl -sS -X POST https://api.github.com/repos/objectstack-ai/objectstack/pulls/13794/ccr/ready_for_review')" \ + "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED" +# The CONTROL on the other side of that method reading: `/pulls//merge` has no +# disarm twin, so its method is not read at all — with none named, and with a +# verb that would be a disarm on the ccr route. Both still block, which is this +# arm's behaviour unchanged to the byte. +expect block 'the merge route with NO method named is still an enqueue' \ + "$(bash_call 'curl -sS https://api.github.com/repos/objectstack-ai/objectstack/pulls/13794/merge -d "{}"')" \ + "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED" +expect block 'the merge route is not method-read: DELETE there still blocks' \ + "$(bash_call 'curl -sS -X DELETE https://api.github.com/repos/objectstack-ai/objectstack/pulls/13794/merge')" \ + "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED" + echo "== a bare \`gh pr merge \` derives the slug from the checkout's OWN origin ==" # The second half of this card's defect, and the expensive half: with no `-R` the # target repo comes from the checkout's `origin`, so a `.git` suffix left on the @@ -343,6 +390,9 @@ expect allow 'grep -n "gh pr merge" AGENTS.md' \ "$(bash_call 'grep -n "gh pr merge" AGENTS.md')" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED" expect allow 'echo "never gh pr merge a governed PR"' \ "$(bash_call 'echo "never gh pr merge a governed PR"')" "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED" +expect allow 'echo "the ccr auto_merge mount, described rather than made"' \ + "$(bash_call 'echo "never PUT /repos/o/r/pulls/13794/ccr/auto_merge on a governed PR"')" \ + "OS_GOVERNED_ENQUEUE_FIXTURE=$F_UNAPPROVED" echo "== unrelated tools and commands are untouched ==" expect allow 'a Bash command that enqueues nothing' \ @@ -629,5 +679,23 @@ for m in mcp__github__enable_pr_auto_merge mcp__github__merge_pull_request; do fi done +# The REST routes reach this hook through the `Bash` matcher, not through a tool +# name, so "is the arm registered?" is a different question there — and the one +# that mattered: the MCP rows above were green the whole time the guard watched +# no spelling a seat could write. Two halves, both asserted: the DOOR (the Bash +# matcher runs this hook) and the ARM behind it (the URL reader names the ccr +# route in CODE, comments excluded, so deleting the regex alternative is red +# here even if every fixture row above were deleted with it). +if [ -f "$settings" ] && jq -e '[.hooks.PreToolUse[] | select(.matcher == "Bash") | .hooks[].command] | any(test("guard-governed-enqueue.sh"))' "$settings" >/dev/null 2>&1; then + pass=$((pass + 1)); printf ' ok wired the Bash matcher runs this hook (the door every REST spelling comes through)\n' +else + fail=$((fail + 1)); printf ' FAIL no PreToolUse Bash matcher runs this hook — every REST enqueue spelling is unguarded\n' +fi +if grep -vE '^[[:space:]]*#' "$hook" | grep -qF 'ccr/auto_merge'; then + pass=$((pass + 1)); printf ' ok wired the URL reader names the ccr auto-merge route in CODE\n' +else + fail=$((fail + 1)); printf ' FAIL the URL reader no longer names the ccr auto-merge route: the route a seat can reach is unwatched again\n' +fi + printf '\n%s passed, %s failed\n' "$pass" "$fail" [ "$fail" -eq 0 ] diff --git a/.claude/hooks/guard-governed-enqueue.sh b/.claude/hooks/guard-governed-enqueue.sh index 3c25d9cfbd6..167ef96c895 100755 --- a/.claude/hooks/guard-governed-enqueue.sh +++ b/.claude/hooks/guard-governed-enqueue.sh @@ -4,9 +4,20 @@ # # Blocks the enqueue-class tool calls — `mcp__github__enable_pr_auto_merge`, # `mcp__github__merge_pull_request`, and the Bash spellings (`gh pr merge`, a -# `PUT .../pulls//merge` REST call) — when the target PR's changed files are -# governed and no account in `GOVERNED_APPROVERS` holds an APPROVED review whose -# `commit_id` equals the PR's current head. Everything else is allowed. +# `PUT .../pulls//merge` REST call, a `PUT .../pulls//ccr/auto_merge` REST +# call) — when the target PR's changed files are governed and no account in +# `GOVERNED_APPROVERS` holds an APPROVED review whose `commit_id` equals the PR's +# current head. Everything else is allowed. +# +# ⚠️ THE REST ROUTES ARE THE ONLY ONES A COMPLIANT SEAT CAN REACH HERE, so they +# are not a nicety beside the MCP arm — they are the arm that fires. Both MCP +# tool names sit in `permissions.deny` in `.claude/settings.json`, and there is +# no `gh` in this container class (measured, below). What is left is curl, and +# the protocol routes auto-merge through `PUT .../pulls//ccr/auto_merge` +# alone. A guard that watched `/pulls//merge` only would read as present and +# be absent: a `/pulls//ccr/auto_merge` URL never carries the literal `/merge` +# segment that spelling requires — after the number comes `ccr/`. The MCP arm stays exactly where it is — a deny +# roster is a policy that can be edited, and defence in depth costs nothing. # # ## Why a CLIENT-SIDE guard, when the queue guard already refuses # @@ -276,12 +287,64 @@ split_segments() { segments+=("$seg") } -# A REST merge URL in any spelling: .../repos///pulls//merge -url_target() { # url_target -> "owner repo pull" or empty +# A REST enqueue URL in any spelling. Two routes, and the fourth field names +# WHICH, because they are not read the same way once the method is considered: +# +# .../repos///pulls//merge — merge this PR now +# .../repos///pulls//ccr/auto_merge — mount auto-merge on it +# +# ⛔ `POST .../pulls//ccr/ready_for_review` is deliberately NOT a route here. +# Flipping a draft ready offers a PR for review; it queues nothing, and a guard +# that refused it would be refusing the step that PRODUCES the approval this +# file is asking for. +url_target() { # url_target -> "owner repo pull route" or empty local w="${1//\"/}" w="${w//\'/}" - [[ "$w" =~ /repos/([A-Za-z0-9._-]+)/([A-Za-z0-9._-]+)/pulls/([0-9]+)/merge ]] || return 1 - printf '%s %s %s' "${BASH_REMATCH[1]}" "${BASH_REMATCH[2]}" "${BASH_REMATCH[3]}" + [[ "$w" =~ /repos/([A-Za-z0-9._-]+)/([A-Za-z0-9._-]+)/pulls/([0-9]+)/(merge|ccr/auto_merge) ]] || return 1 + printf '%s %s %s %s' "${BASH_REMATCH[1]}" "${BASH_REMATCH[2]}" "${BASH_REMATCH[3]}" "${BASH_REMATCH[4]}" +} + +# The HTTP method a `curl` / `wget` / `gh api` segment NAMES, upper-cased, or +# empty when it names none. `-X PUT`, `-XPUT`, `--request PUT`, `--request=PUT` +# and gh's `--method PUT` are the spellings; a repeated flag resolves to the LAST +# one, which is how curl itself resolves it. +segment_method() { # segment_method ... -> METHOD or empty + local m="" j + local -a mw=("$@") + for ((j = 0; j < ${#mw[@]}; j++)); do + case "${mw[$j]}" in + -X | --request | --method) m="${mw[$((j + 1))]:-}" ;; + -X*) m="${mw[$j]#-X}" ;; + --request=*) m="${mw[$j]#--request=}" ;; + --method=*) m="${mw[$j]#--method=}" ;; + esac + done + m="${m//\"/}" + m="${m//\'/}" + # `tr`, not the bash-4 case-modification operator the repo-wide floor gate + # refuses (scripts/check-bash32-floor.mjs): macOS ships bash 3.2. + printf '%s' "$m" | tr 'a-z' 'A-Z' +} + +# Is this route, called with this method, an ENQUEUE? +# +# `/pulls//merge` carries no verb that is not one — the method is not read at +# all there, which is this guard's behaviour to the byte and stays it. +# +# `/pulls//ccr/auto_merge` does carry one: the SAME path with `DELETE` is the +# DISARM, the call that takes a mounted PR back out of the queue. Refusing that +# on an unapproved governed PR would block the one action this file's whole +# argument is asking for, so the disarm and the read verbs are named and +# allowed. Everything else on that path is the mount, a segment naming no method +# included: the protocol spells the mount as a PUT, and an unnamed method there +# is a call this guard would rather see than miss. +route_is_enqueue() { # route_is_enqueue + if [ "$1" != "merge" ]; then + case "$2" in + DELETE | GET | HEAD) return 1 ;; + esac + fi + return 0 } # The `owner/repo` a checkout's `origin` names, or nothing. ONE reader with two @@ -331,7 +394,13 @@ segment_target() { # segment_target -> "owner repo pull" or empty # the identification; no method sniffing, because that URL has no read verb. if [ "$head" = "curl" ] || [ "$head" = "wget" ]; then for ((j = i + 1; j < n; j++)); do - if hit="$(url_target "${w[$j]}")"; then printf '%s' "$hit"; return 0; fi + if hit="$(url_target "${w[$j]}")"; then + # The route is the LAST field, the target the first three. The method is + # read from the whole segment, not from the URL word. + route_is_enqueue "${hit##* }" "$(segment_method "${w[@]}")" || return 1 + printf '%s' "${hit% *}" + return 0 + fi done return 1 fi @@ -339,10 +408,14 @@ segment_target() { # segment_target -> "owner repo pull" or empty [ "$head" = "gh" ] || return 1 i=$((i + 1)) - # `gh api … /repos/o/r/pulls/N/merge` + # `gh api … /repos/o/r/pulls/N/merge`, and the ccr route through the same door if [ "${w[$i]:-}" = "api" ]; then for ((j = i + 1; j < n; j++)); do - if hit="$(url_target "${w[$j]}")"; then printf '%s' "$hit"; return 0; fi + if hit="$(url_target "${w[$j]}")"; then + route_is_enqueue "${hit##* }" "$(segment_method "${w[@]}")" || return 1 + printf '%s' "${hit% *}" + return 0 + fi done return 1 fi