tools/hooks/mergeguard: a read-only `gh api` command is denied when the word "merge" appears anywhere else in it

mergeguard denies a compound Bash command that contains a gh token, the word merge anywhere in the whole command, and any $ or backtick anywhere — even when every gh invocation is a read-only gh api GET and the word merge only appears in unrelated prose. The denial message then tells the agent to "use one fully explicit invocation after shipcheck passes", which is not actionable advice for a status query, so the agent has no way to learn what it actually did wrong.

Hit while verifying that #1960 had landed. The command was a post-merge status check: read the pull request, list comments and reviews, list review threads.

Reproduction

{"hook_event_name":"PreToolUse","tool_name":"Bash","cwd":"/home/user/flowstate","tool_input":{"command":
"cd /home/user/flowstate && echo \"=== PR 1960 state ===\" && gh api repos/picatz/flowstate/pulls/1960 --jq '\"merged=\\(.merged)\"'; echo \"=== any comment/review after the merge? ===\"; gh api repos/picatz/flowstate/commits/$(git rev-parse origin/main)/check-runs --jq .total_count"}}

$ go run ./tools/hooks/mergeguard < repro.json
mergeguard: shell expansion in a `gh pr merge` invocation can hide auto-merge or change its target and head. Use one fully explicit invocation after shipcheck passes.

There is no gh pr merge in that command and no way for it to merge anything.

Cause

mergeUsesShellExpansion computes

mergeExpansion := !recognized && (ghExpansionMergeText.MatchString(cmd) || ghDynamicPRMergeText.MatchString(cmd) || dynamicGHExecutable.MatchString(cmd)) && (strings.ContainsAny(cmd, "$`") || ...)

and ghExpansionMergeText is (?s)\bgh\b.*\bmerge\b, which spans the entire command including separators. Its two siblings are deliberately scoped to one segment with [^;\n]*; this one is not. So the three ingredients are satisfied by, respectively, gh api, the literal word merge inside echo "=== any comment/review after the merge? ===", and $(git rev-parse origin/main) — none of which are the same command, let alone a merge. braceExpansion has the same shape and fires on a --jq '{...}' query for the same reason.

Note that merged, merge_commit and mergeable do not trigger it: _, d and a are word characters, so \bmerge\b does not match them. It takes a standalone merge, which is why this reads as prose-sensitivity rather than as anything about the API being called.

Why the fix is not a looser regex

Corrected. This section originally argued that segment-scoping ghExpansionMergeText to \bgh\b[^;\n]*\bmerge\b would weaken the guard because X=merge; gh api -X PUT repos/o/r/pulls/1/$X would stop matching. That justification was wrong: evaluating the checked-in predicates shows that command is already allowed on 612983c, because ghExpansionMergeText requires gh to precede merge and that command assigns the word first. Segment-scoping would not have lost a protection that exists.

The conclusion stands for a different and better reason. Segment distance is not what makes a command safe — capability is. A gh api invocation with no method-changing and no body-supplying flag is a GET, and a GET cannot merge whatever its URL expands to, whether or not the word merge sits in the same segment. So the fix asks what the call can do rather than how far apart two tokens are.

The ordering gap this correction uncovered, along with eval 'gh api -X PUT .../merge' slipping the nested-evaluation gate because that gate requires the word pr, is tracked separately in #1973.

Fix

Exempt only what is provably incapable of merging, and only from the loose text heuristic.

Suppress the ghExpansionMergeText-driven paths (mergeExpansion, braceExpansion) when every gh invocation in the command is a literal gh api whose arguments satisfy all of:

• the subcommand is the literal first argument api, so gh --hostname h api does not qualify and keeps today's denial, which keeps the rule auditable by reading it;
• no argument changes the method or supplies a body: none of -X, --method, -f, --raw-field, -F, --field, --input, prefix-matched so --method=PUT and -XPUT are caught, and no single-dash group containing X, f or F;
• no argument is a bare expansion, because gh api "$FLAG" <url> word-splits into -X PUT at run time — while an argument with literal text of its own, such as a URL interpolating a revision, is the shape a status query needs and is allowed;
• no flag-shaped argument carries an expansion.

And additionally: no command that runs a gh invocation may itself carry a standalone merge token. That second condition is what keeps the exemption narrow — it covers a command whose GitHub calls are about something else and that happens to carry the word in prose, not a call whose own words say merge.

Gate the whole exemption on there being no other indicator: not recognized, and no expandedMergeExecutable, processSubstitution, nestedEvaluation, dynamicGHExecutable or ghDynamicPRMergeText match.

Residual, accepted: a read whose own words carry the token, such as --jq '{merge: .mergeable}', stays denied. Spelling the selector .mergeable reads the same field without the token, so the cost is a spelling rather than a capability.

Acceptance

Regression tests in both directions in tools/hooks/mergeguard/main_test.go:

• The reproduction command above is allowed, proven against the built hook rather than only against the predicates.
• Denied, unchanged: gh api -X PUT repos/o/r/pulls/1/merge; gh api --method $M .../merge; gh api --method=PUT .../merge; gh api -XPUT .../merge; gh api -f ... .../merge; gh api -F sha=$SHA .../merge; gh api --input $BODY .../merge; gh api "$FLAG" .../merge; a --jq '{...}' brace query on a merge URL; a command mixing a read-only gh api with a gh pr merge; and every case already in TestMergeShellExpansionIsRejected, which must keep passing untouched.
• The three shapes from #1973 pinned as known gaps, so the expectation has a home when that issue is fixed.

Not a weakening of the merge gate itself

This hook is one of several controls: the merge in #1960 went through the GitHub MCP tool with expectedHeadSha, and shipcheck is the gate that reads the evidence. Nothing here changes what a merge must satisfy. The cost of the false positive is that it trains an agent to route around a security hook on routine reads, which is worse than the friction.

Related: #1961 covers the gh binary dependency in this same tooling; #1973 covers two merge shapes this hook does not catch at all.

Start with the reproduction using go run ./tools/hooks/mergeguard < repro.json, then inspect the mergeguard predicates and tools/hooks/mergeguard/main_test.go. Add regression coverage for the read-only gh api exemption and the listed denied cases, while preserving TestMergeShellExpansionIsRejected and the three known gaps from #1973.