Skip to content

fix(hooks): watch the ccr auto-merge route in guard-governed-enqueue #18447

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
os-elon-musk wants to merge 1 commit into from
+151 −10
Closed

fix(hooks): watch the ccr auto-merge route in guard-governed-enqueue #18447

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions .claude/hooks/guard-governed-enqueue.selftest.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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/<n>/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/<n>/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/<n>/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/<n>/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/<n>/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 <n>\` 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
Expand Down Expand Up @@ -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' \
Expand Down Expand Up @@ -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 ]
93 changes: 83 additions & 10 deletions .claude/hooks/guard-governed-enqueue.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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/<n>/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/<n>/merge` REST call, a `PUT .../pulls/<n>/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/<n>/ccr/auto_merge`
# alone. A guard that watched `/pulls/<n>/merge` only would read as present and
# be absent: a `/pulls/<n>/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
#
Expand Down Expand Up @@ -276,12 +287,64 @@ split_segments() {
segments+=("$seg")
}

# A REST merge URL in any spelling: .../repos/<owner>/<repo>/pulls/<n>/merge
url_target() { # url_target <word> -> "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/<owner>/<repo>/pulls/<n>/merge — merge this PR now
# .../repos/<owner>/<repo>/pulls/<n>/ccr/auto_merge — mount auto-merge on it
#
# ⛔ `POST .../pulls/<n>/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 <word> -> "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 <word>... -> 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/<n>/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/<n>/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 <route> <method>
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
Expand Down Expand Up @@ -331,18 +394,28 @@ segment_target() { # segment_target <segment> -> "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

[ "$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
Expand Down
Loading