The infra-mcp README claimed client deny lists gated destructive verbs. Checked 2026-10-01: none exist in ~/.claude/settings.json or ~/dev/.claude/settings*.json. gitea-mcp is PAT pass-through, so an agent has full PAT scope, and repo_delete's confirm is model-filled.
What
hooks/mcp-destructive-guard.sh — PreToolUse on mcp__.*. repo_delete|file_delete|branch_delete|pr_merge|tbd_ship|repo_update|repo_mirror_push → ask. Matches the verb on any server name. Never emits allow. Malformed input → ask.
tests/mcp_destructive_guard_test.sh — 21 cases incl. connector namespace, renamed server, uppercase, near-miss suffix, malformed input. Red before the hook existed (21 failed), green after.
hooks/hooks.json wiring, README hook list.
Known scope limit (not a bypass to report, unless you find it worse than stated)
Bash + Gitea REST with the PAT from env is not covered. Recorded as residual in infra ADR-0027.
Verification
task check EXIT=0, task test EXIT=0.
risk/medium — control code (rule 8): review by execution.
Refs mathias/infra#508 (decision: client-side gates for destructive MCP verbs, gateway stays deferred — Mathias, 2026-10-01).
## Why
The infra-mcp README claimed client deny lists gated destructive verbs. Checked 2026-10-01: none exist in `~/.claude/settings.json` or `~/dev/.claude/settings*.json`. gitea-mcp is PAT pass-through, so an agent has full PAT scope, and `repo_delete`'s `confirm` is model-filled.
## What
- `hooks/mcp-destructive-guard.sh` — PreToolUse on `mcp__.*`. `repo_delete|file_delete|branch_delete|pr_merge|tbd_ship|repo_update|repo_mirror_push` → `ask`. Matches the verb on any server name. Never emits `allow`. Malformed input → `ask`.
- `tests/mcp_destructive_guard_test.sh` — 21 cases incl. connector namespace, renamed server, uppercase, near-miss suffix, malformed input. Red before the hook existed (21 failed), green after.
- `hooks/hooks.json` wiring, README hook list.
## Known scope limit (not a bypass to report, unless you find it worse than stated)
Bash + Gitea REST with the PAT from env is not covered. Recorded as residual in infra ADR-0027.
## Verification
`task check` EXIT=0, `task test` EXIT=0.
risk/medium — control code (rule 8): review by execution.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---
_Created via git-mcp on behalf of @mathias_
gitea-mcp authenticates by PAT pass-through, so an agent holds the
caller's full PAT scope, and repo_delete's confirm field is filled in by
the model. The infra-mcp README said client-side deny lists covered
destructive verbs; on 2026-10-01 none existed in any settings file.
PreToolUse hook on mcp__.*: repo_delete, file_delete, branch_delete,
pr_merge, tbd_ship, repo_update and repo_mirror_push get "ask". Matched
on the verb, not the server name, so the connector namespace or a
renamed registration cannot bypass it. Never emits allow; malformed
input fails closed.
Scope limit: MCP calls only. The Gitea REST API via Bash with the PAT
in env is not covered (recorded in infra ADR-0027).
Refs mathias/infra#508
Bump-Type: minor
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Reviewed by running the hook as a program against crafted PreToolUse JSON in a clean clone. task check exits 0, task test exits 0, and CI runs 8675/8676 are green. None of the findings below is covered by that green.
Escapes: a destructive verb gets something other than ask
E1. When jq is missing, the fail-closed path fails open (hooks/mcp-destructive-guard.sh:26-34). ask() uses jq to build its own output. If jq is absent, both the parse and the "fail closed" answer fail, stdout is empty and the exit code is 0. The host reads that as "no opinion".
mkdir nojq; for b in bash cat grep; do ln -s "$(command -v $b)" nojq/$b; done
printf '{"tool_name":"mcp__plugin_skills_gitea__repo_delete"}' | PATH=$PWD/nojq bash hooks/mcp-destructive-guard.sh
# stdout: 0 bytes, rc=0
jq is present on koala and iguana today, so this is latent rather than live. It still breaks the header's own claim that malformed input fails closed, and nothing (doctor or test) checks for jq. Fix: emit the ask JSON with a literal printf, so it has no dependency that can fail.
E2. A non-string tool_name gets no opinion (:33, jq -er '.tool_name' accepts any non-null, non-false value):
The host always sends a string, so this is not reachable today. It is still unvalidated input at a boundary, and it contradicts "Malformed input fails closed". Fix: jq -er '.tool_name | strings | select(length>0)'.
Properties the PR claims but no test checks (flip tests: suite stayed 21/21 green)
jq -er changed to jq -r: {} and {"tool_name":null} then get NONE instead of ask. "missing tool_name fails closed" has no test.
ask() changed to exit 1: still 21/21. The suite reads stdout and never checks the exit code. Claude Code only processes hook JSON on exit 0, so this regression would turn every ask into "no opinion" and CI would stay green. The "ALWAYS exits 0" contract in the test header has no test behind it.
The mcp__*__* case filter removed: still 21/21.
Verb taken from the second segment instead of the last: still 21/21. No test has a tool name with extra __ segments.
(These flips did turn the suite red: drop -i, drop -x, remove a verb from the list, emit allow.)
Tests claiming more than they show (rule 9)
tests/mcp_destructive_guard_test.sh:71 has the comment "Executed as a program, like Claude Code does (shebang decides interpreter)". That is false. hooks.json runs bash "<path>", so the shebang and the executable bit do not matter there. The label "guard is executable" is honest; the comment is not.
"malformed input asks" only covers input that is not JSON at all. Valid JSON with a null, missing or wrong-type tool_name is not tested (see E2 and the -e flip).
The declared scope limit is worse than stated
The hook's user-facing reason (:44), its header, and the PR body all say the residual risk is "recorded in infra ADR-0027". infra main stops at ADR-0026, and no branch has 0027. The scope limit points to a record that does not exist.
Coverage left unguarded: a judgement call, escalated to Mathias on infra#508
I read the gitea-mcp tool list (43 tools) one by one. These still get no opinion:
file_write_branch with branch: main puts arbitrary content straight on main, which is the same effect as pr_merge/tbd_ship, both gated. On mathias/infra that is a Flux deploy within about 60 s. Writing content: "" empties a file, which is what file_delete does, and file_delete is gated. The test file asserts this pass-through on purpose (file_write_branch passes through).
workflow_run_trigger is named in infra#508 itself as one of the destructive tools, but it is missing from the list with no reason given. It can fire CD workflows.
tag_create / release_create can fire release pipelines.
The verb match only catches this server's naming. A server that calls the same operation delete_repo (upstream gitea-mcp style) gets no opinion. That fits the "match the verb" design, but the header says it covers "any name a host registers it with", which claims more than the code does.
Checked and held (not findings)
The mcp__.* matcher. Claude Code 2.1.284 runs new RegExp(matcher).test(name), which is unanchored, so it covers mcp__plugin_skills_gitea__*, mcp__claude_ai_gitea__* and any other mcp__ namespace. An invalid regex there matches everything.
Whitespace, newline, CR, NUL and Cyrillic-lookalike tool names get NONE. The host cannot produce them, because it normalises names to [a-zA-Z0-9_-]. Trailing \n gets ask.
A 50 MB input finishes in about 1.1 s, well under the 5 s timeout. The model's output limits rule out an input big enough to time out.
allow is never emitted. A top-level array, empty stdin, null or false gets ask.
PR has no risk/ label even though the body says risk/medium.
## pr-gatekeeper: BLOCKED (head d068cbc)
Reviewed by running the hook as a program against crafted PreToolUse JSON in a clean clone. `task check` exits 0, `task test` exits 0, and CI runs 8675/8676 are green. **None of the findings below is covered by that green.**
### Escapes: a destructive verb gets something other than `ask`
**E1. When jq is missing, the fail-closed path fails open** (`hooks/mcp-destructive-guard.sh:26-34`). `ask()` uses jq to build its own output. If jq is absent, both the parse and the "fail closed" answer fail, stdout is empty and the exit code is 0. The host reads that as "no opinion".
```
mkdir nojq; for b in bash cat grep; do ln -s "$(command -v $b)" nojq/$b; done
printf '{"tool_name":"mcp__plugin_skills_gitea__repo_delete"}' | PATH=$PWD/nojq bash hooks/mcp-destructive-guard.sh
# stdout: 0 bytes, rc=0
```
jq is present on koala and iguana today, so this is latent rather than live. It still breaks the header's own claim that malformed input fails closed, and nothing (doctor or test) checks for jq. Fix: emit the `ask` JSON with a literal `printf`, so it has no dependency that can fail.
**E2. A non-string `tool_name` gets no opinion** (`:33`, `jq -er '.tool_name'` accepts any non-null, non-false value):
```
{"tool_name":["mcp__x__repo_delete"]} -> NONE
{"tool_name":{"a":1}} -> NONE
{"tool_name":5} -> NONE
{"tool_name":""} -> NONE
```
The host always sends a string, so this is not reachable today. It is still unvalidated input at a boundary, and it contradicts "Malformed input fails closed". Fix: `jq -er '.tool_name | strings | select(length>0)'`.
### Properties the PR claims but no test checks (flip tests: suite stayed 21/21 green)
- **`jq -er` changed to `jq -r`**: `{}` and `{"tool_name":null}` then get NONE instead of ask. "missing tool_name fails closed" has no test.
- **`ask()` changed to `exit 1`**: still 21/21. The suite reads stdout and never checks the exit code. Claude Code only processes hook JSON on exit 0, so this regression would turn every ask into "no opinion" and CI would stay green. The "ALWAYS exits 0" contract in the test header has no test behind it.
- **The `mcp__*__*` case filter removed**: still 21/21.
- **Verb taken from the second segment instead of the last**: still 21/21. No test has a tool name with extra `__` segments.
(These flips did turn the suite red: drop `-i`, drop `-x`, remove a verb from the list, emit `allow`.)
### Tests claiming more than they show (rule 9)
- `tests/mcp_destructive_guard_test.sh:71` has the comment "Executed as a program, like Claude Code does (shebang decides interpreter)". That is false. `hooks.json` runs `bash "<path>"`, so the shebang and the executable bit do not matter there. The label "guard is executable" is honest; the comment is not.
- "malformed input asks" only covers input that is not JSON at all. Valid JSON with a null, missing or wrong-type tool_name is not tested (see E2 and the `-e` flip).
### The declared scope limit is worse than stated
- The hook's user-facing reason (`:44`), its header, and the PR body all say the residual risk is "recorded in infra ADR-0027". **infra main stops at ADR-0026, and no branch has 0027.** The scope limit points to a record that does not exist.
### Coverage left unguarded: a judgement call, escalated to Mathias on infra#508
I read the gitea-mcp tool list (43 tools) one by one. These still get no opinion:
- `file_write_branch` with `branch: main` puts arbitrary content straight on main, which is the same effect as `pr_merge`/`tbd_ship`, both gated. On `mathias/infra` that is a Flux deploy within about 60 s. Writing `content: ""` empties a file, which is what `file_delete` does, and `file_delete` is gated. The test file asserts this pass-through on purpose (`file_write_branch passes through`).
- `workflow_run_trigger` is named in infra#508 itself as one of the destructive tools, but it is missing from the list with no reason given. It can fire CD workflows.
- `tag_create` / `release_create` can fire release pipelines.
- The verb match only catches this server's naming. A server that calls the same operation `delete_repo` (upstream gitea-mcp style) gets no opinion. That fits the "match the verb" design, but the header says it covers "any name a host registers it with", which claims more than the code does.
### Checked and held (not findings)
- The `mcp__.*` matcher. Claude Code 2.1.284 runs `new RegExp(matcher).test(name)`, which is unanchored, so it covers `mcp__plugin_skills_gitea__*`, `mcp__claude_ai_gitea__*` and any other `mcp__` namespace. An invalid regex there matches everything.
- Whitespace, newline, CR, NUL and Cyrillic-lookalike tool names get NONE. The host cannot produce them, because it normalises names to `[a-zA-Z0-9_-]`. Trailing `\n` gets ask.
- A 50 MB input finishes in about 1.1 s, well under the 5 s timeout. The model's output limits rule out an input big enough to time out.
- `allow` is never emitted. A top-level array, empty stdin, `null` or `false` gets ask.
PR has no `risk/` label even though the body says risk/medium.
- Fail-closed path no longer needs jq: the ask JSON is a printf literal,
and a host without jq asks instead of staying silent.
- A tool_name that is not a non-empty string asks (was: no opinion).
- Gate workflow_run_trigger, tag_create, release_create, and
file_write_branch when it lands on main/master, has no branch, or
empties a file (same effect as pr_merge / file_delete).
- Header claim narrowed: gitea-mcp spellings on any server name, not
other servers' spellings of the same operation.
- Tests run the hook as hooks.json does (bash <path>), assert exit 0 on
every case, and cover null/missing/wrong-type tool_name, last-segment
verb parsing and a PATH without jq. 13 new cases were red first.
Refs mathias/infra#508
Bump-Type: minor
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
task check exits 0, task test exits 0 (38/38), and CI runs 8689/8690 are green. Green does not cover the escape below.
Escape: file_write_branch lands on main and the hook has no opinion (verified end to end)
The hook reads only the lowercase .branch key. gitea-mcp decodes arguments with Go encoding/json, which matches keys case-insensitively and lets the last matching key win. gitea-mcp is built with golang:1.26-alpine, and I checked this behaviour under go1.26.1.
Sorting keys during gitea-mcp's alias re-marshal would normally put lowercase branch last. That re-marshal only happens when normalizeAliases changes something, and passing name alongside repo stops it from changing anything.
tools.FileWriteBranch.Call with that tool_input: gitea-mcp at 0e2bd3d, built with go1.26.1, against a scratch Gitea 1.27.1 (same version as git.d-ma.be). Result: OK branch=main. case8.txt is on main with that content, in commit "case probe 8".
The same trick works on content: "content":"x","Content":"" empties a file while the hook sees x.
The schema has no additionalProperties:false, so the extra key is valid input for the model to send.
Fix suggestion: collect every key whose ascii_downcase is branch (and the same for content). Answer ask if there is not exactly one, or if any of them is main/master/empty. Add the repro above as a test.
Untested, while the code is correct today (flip survived 38/38)
.content // "" changed to .content. A missing content key then passes, and gitea-mcp writes an empty file in that case (it only enforces branch and message). You asked for this case explicitly, and no test covers it.
ascii_downcase removed: no test uses Main.
Non-object tool_input: no test. It is safe today because only an exact pass passes, so the guard is default-deny.
command -v jq check removed: the no-jq test still passes, because jq -er then fails and goes to the printf ask. The behaviour holds, so this is not a finding.
Your earlier flips: all now go red
ask exit 1 → 28 fail. jq -er→jq -r → 6 fail. Case filter removed → 1 fail. Second-segment parse → 1 fail. Also red: dropping strings|select, a trailing allow, dropping master, dropping empty-branch, and dropping workflow_run_trigger or release_create.
Checked and held
Branch-name lookalikes. The hook says NONE for " main", "main ", refs/heads/main, main/, ./main, main?x=1 and main%2f. I ran each through real gitea-mcp against Gitea 1.27.1, and none reached main: each was rejected with a GitRefName error or created a separate branch. Main/MAIN get ask.
tool_input that is null, missing, a string or an array gets ask. branch that is null, false, a number or an array gets ask. content that is missing, null, false or "" gets ask.
Malformed input and the no-jq case both get ask with rc=0.
Residuals (not blocking; record them in ADR-0027)
1 of the 56 estate repos has a default branch that is neither main nor master, so writes to it pass. The name was masked in my output and I did not unmask it; the repo is */brain-weekly-ingest.
Whitespace-only content (" ", "\n") passes. It has the same effect as emptying the file, but only on non-main branches, where history makes it recoverable.
Side finding for gitea-mcp, not this PR: an ungated file_write_branch with branch:"refs/heads/main" creates refs/heads/refs/heads/main. After that, every API write to main returns 500 ("dst refspec refs/heads/main matches more than one") until the ref is removed. That is a cheap denial-of-service against the deploy branch.
ADR-0027 is treated as pending (infra 265aee6 is not pushed), as instructed.
## pr-gatekeeper round 2: BLOCKED (head 4a82206)
`task check` exits 0, `task test` exits 0 (38/38), and CI runs 8689/8690 are green. Green does not cover the escape below.
### Escape: `file_write_branch` lands on main and the hook has no opinion (verified end to end)
The hook reads only the lowercase `.branch` key. gitea-mcp decodes arguments with Go `encoding/json`, which matches keys case-insensitively and lets the last matching key win. gitea-mcp is built with `golang:1.26-alpine`, and I checked this behaviour under go1.26.1.
Sorting keys during gitea-mcp's alias re-marshal would normally put lowercase `branch` last. That re-marshal only happens when `normalizeAliases` changes something, and passing `name` alongside `repo` stops it from changing anything.
```json
{"tool_name":"mcp__plugin_skills_gitea__file_write_branch",
"tool_input":{"owner":"probe","repo":"t","name":"t","path":"case8.txt",
"content":"landed-on-main-via-Branch-key","message":"case probe 8",
"branch":"feat/y8","Branch":"main"}}
```
- Hook: 0 bytes on stdout, rc=0, so no opinion.
- `tools.FileWriteBranch.Call` with that `tool_input`: gitea-mcp at 0e2bd3d, built with go1.26.1, against a scratch Gitea 1.27.1 (same version as git.d-ma.be). Result: `OK branch=main`. `case8.txt` is on `main` with that content, in commit "case probe 8".
- The same trick works on content: `"content":"x","Content":""` empties a file while the hook sees `x`.
- The schema has no `additionalProperties:false`, so the extra key is valid input for the model to send.
Fix suggestion: collect every key whose `ascii_downcase` is `branch` (and the same for `content`). Answer ask if there is not exactly one, or if any of them is main/master/empty. Add the repro above as a test.
### Untested, while the code is correct today (flip survived 38/38)
- `.content // ""` changed to `.content`. A **missing content key** then passes, and gitea-mcp writes an empty file in that case (it only enforces `branch` and `message`). You asked for this case explicitly, and no test covers it.
- `ascii_downcase` removed: no test uses `Main`.
- Non-object `tool_input`: no test. It is safe today because only an exact `pass` passes, so the guard is default-deny.
- `command -v jq` check removed: the no-jq test still passes, because `jq -er` then fails and goes to the printf ask. The behaviour holds, so this is not a finding.
### Your earlier flips: all now go red
`ask` exit 1 → 28 fail. `jq -er`→`jq -r` → 6 fail. Case filter removed → 1 fail. Second-segment parse → 1 fail. Also red: dropping `strings|select`, a trailing `allow`, dropping master, dropping empty-branch, and dropping `workflow_run_trigger` or `release_create`.
### Checked and held
- Branch-name lookalikes. The hook says NONE for `" main"`, `"main "`, `refs/heads/main`, `main/`, `./main`, `main?x=1` and `main%2f`. I ran each through real gitea-mcp against Gitea 1.27.1, and **none reached main**: each was rejected with a GitRefName error or created a separate branch. `Main`/`MAIN` get ask.
- `tool_input` that is null, missing, a string or an array gets ask. `branch` that is null, false, a number or an array gets ask. `content` that is missing, null, false or `""` gets ask.
- Malformed input and the no-jq case both get ask with rc=0.
### Residuals (not blocking; record them in ADR-0027)
- 1 of the 56 estate repos has a default branch that is neither main nor master, so writes to it pass. The name was masked in my output and I did not unmask it; the repo is `*/brain-weekly-ingest`.
- Whitespace-only content (`" "`, `"\n"`) passes. It has the same effect as emptying the file, but only on non-main branches, where history makes it recoverable.
- Side finding for gitea-mcp, not this PR: an ungated `file_write_branch` with `branch:"refs/heads/main"` creates `refs/heads/refs/heads/main`. After that, **every** API write to `main` returns 500 ("dst refspec refs/heads/main matches more than one") until the ref is removed. That is a cheap denial-of-service against the deploy branch.
- ADR-0027 is treated as pending (infra 265aee6 is not pushed), as instructed.
Round-2 review of skills#52 committed to main through gitea-mcp with
{"branch":"feat/y8","Branch":"main"}: Go's encoding/json matches keys
case-insensitively and the last one wins, while the hook read only the
lowercase key. Same trick on content emptied a file.
The guard now asks unless exactly one key case-folds to "branch" and
one to "content", both spelled exactly, both strings. The branch is
cleared by shape ([A-Za-z0-9][A-Za-z0-9._/-]*, no "..", no refs/ prefix,
no trailing slash) instead of a blocklist, since refs/heads/main created
a second ref that wedged every API write to main. Whitespace-only
content asks.
11 new cases; 6 were red before the change, 5 pinned untested-but-correct
behaviour the reviewer's flips showed nothing covered.
Refs mathias/infra#508
Bump-Type: minor
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mathias
merged commit b0d58d5adf into main2026-10-01 20:21:52 +00:00
pr-gatekeeper round 3: MERGED (squash b0d58d5, reviewed head a41f5ab)
End to end, against gitea-mcp at 0e2bd3d built with go1.26.1 and a scratch Gitea 1.27.1:
The round-2 repro (branch:"feat/a","Branch":"main") now gets ask.
Branch alone, content+Content, an escaped key branch, and nbsp-only content all get ask.
Every input the hook still passes was executed through real gitea-mcp, and main's SHA was compared before and after. Main never moved. Those inputs were HEAD, heads/main, main.lock, main/x, main\n, feat., x/./main, feat//main, duplicate exact keys, and base:"main" on a new branch. Each either wrote to a separate branch or was rejected (GitRefName, D/F conflict, a URL control character, or an empty segment).
Key folding: I scanned every non-ASCII rune at every position of branch and content under go1.26.1 encoding/json. There were 0 fold hits, so ascii_downcase matches Go's matching for these two keys.
Flips (suite 49/49):
These go red: dropping key folding for branch or content, dropping count==1, dropping exact spelling, dropping the shape allowlist, dropping the ..|^refs/|/$ check, dropping ascii_downcase on the value, whitespace changed to empty-only, dropping the content check, dropping master. The round-1 and round-2 flips stay red.
Two flips survive: the string type check and the non-object check. Both are behaviour-preserving, because jq errors out and the hook falls back to ask. Executed with each flip in place: number, array, object or bool values and non-object inputs all still get ask. That is defence in depth, not a gap.
Not blocking, hardening for later:
jq's Oniguruma $ matches before a trailing newline, so branch:"main\n" passes the hook. It is unreachable because Go's URL parse rejects control characters before any write.
HEAD is accepted as a branch name and creates refs/heads/HEAD. Writes to main kept working afterwards.
task check exits 0, task test exits 0. CI runs 8694/8695 are green.
Sequencing note: infra ADR-0027 (265aee6) is still not on infra main at merge time. The hook's reason strings cite it, and the residuals (non-main default branch on brain-weekly-ingest, host stripping unverified, whitespace and \n edge cases, the HEAD branch) need it as their record.
## pr-gatekeeper round 3: MERGED (squash b0d58d5, reviewed head a41f5ab)
**End to end, against gitea-mcp at 0e2bd3d built with go1.26.1 and a scratch Gitea 1.27.1:**
- The round-2 repro (`branch:"feat/a","Branch":"main"`) now gets ask.
- `Branch` alone, `content`+`Content`, an escaped key `branch`, and nbsp-only content all get ask.
- Every input the hook still passes was executed through real gitea-mcp, and main's SHA was compared before and after. **Main never moved.** Those inputs were `HEAD`, `heads/main`, `main.lock`, `main/x`, `main\n`, `feat.`, `x/./main`, `feat//main`, duplicate exact keys, and `base:"main"` on a new branch. Each either wrote to a separate branch or was rejected (GitRefName, D/F conflict, a URL control character, or an empty segment).
**Key folding:** I scanned every non-ASCII rune at every position of `branch` and `content` under go1.26.1 `encoding/json`. There were 0 fold hits, so `ascii_downcase` matches Go's matching for these two keys.
**Flips (suite 49/49):**
- These go red: dropping key folding for branch or content, dropping count==1, dropping exact spelling, dropping the shape allowlist, dropping the `..|^refs/|/$` check, dropping `ascii_downcase` on the value, whitespace changed to empty-only, dropping the content check, dropping master. The round-1 and round-2 flips stay red.
- Two flips survive: the string type check and the non-object check. Both are behaviour-preserving, because jq errors out and the hook falls back to ask. Executed with each flip in place: number, array, object or bool values and non-object inputs all still get ask. That is defence in depth, not a gap.
**Not blocking, hardening for later:**
- jq's Oniguruma `$` matches before a trailing newline, so `branch:"main\n"` passes the hook. It is unreachable because Go's URL parse rejects control characters before any write.
- `HEAD` is accepted as a branch name and creates `refs/heads/HEAD`. Writes to main kept working afterwards.
`task check` exits 0, `task test` exits 0. CI runs 8694/8695 are green.
**Sequencing note:** infra ADR-0027 (265aee6) is **still not on infra main** at merge time. The hook's reason strings cite it, and the residuals (non-main default branch on brain-weekly-ingest, host stripping unverified, whitespace and `\n` edge cases, the `HEAD` branch) need it as their record.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Refs mathias/infra#508 (decision: client-side gates for destructive MCP verbs, gateway stays deferred — Mathias, 2026-10-01).
Why
The infra-mcp README claimed client deny lists gated destructive verbs. Checked 2026-10-01: none exist in
~/.claude/settings.jsonor~/dev/.claude/settings*.json. gitea-mcp is PAT pass-through, so an agent has full PAT scope, andrepo_delete'sconfirmis model-filled.What
hooks/mcp-destructive-guard.sh— PreToolUse onmcp__.*.repo_delete|file_delete|branch_delete|pr_merge|tbd_ship|repo_update|repo_mirror_push→ask. Matches the verb on any server name. Never emitsallow. Malformed input →ask.tests/mcp_destructive_guard_test.sh— 21 cases incl. connector namespace, renamed server, uppercase, near-miss suffix, malformed input. Red before the hook existed (21 failed), green after.hooks/hooks.jsonwiring, README hook list.Known scope limit (not a bypass to report, unless you find it worse than stated)
Bash + Gitea REST with the PAT from env is not covered. Recorded as residual in infra ADR-0027.
Verification
task checkEXIT=0,task testEXIT=0.risk/medium — control code (rule 8): review by execution.
🤖 Generated with Claude Code
Created via git-mcp on behalf of @mathias
pr-gatekeeper: BLOCKED (head
d068cbc)Reviewed by running the hook as a program against crafted PreToolUse JSON in a clean clone.
task checkexits 0,task testexits 0, and CI runs 8675/8676 are green. None of the findings below is covered by that green.Escapes: a destructive verb gets something other than
askE1. When jq is missing, the fail-closed path fails open (
hooks/mcp-destructive-guard.sh:26-34).ask()uses jq to build its own output. If jq is absent, both the parse and the "fail closed" answer fail, stdout is empty and the exit code is 0. The host reads that as "no opinion".jq is present on koala and iguana today, so this is latent rather than live. It still breaks the header's own claim that malformed input fails closed, and nothing (doctor or test) checks for jq. Fix: emit the
askJSON with a literalprintf, so it has no dependency that can fail.E2. A non-string
tool_namegets no opinion (:33,jq -er '.tool_name'accepts any non-null, non-false value):The host always sends a string, so this is not reachable today. It is still unvalidated input at a boundary, and it contradicts "Malformed input fails closed". Fix:
jq -er '.tool_name | strings | select(length>0)'.Properties the PR claims but no test checks (flip tests: suite stayed 21/21 green)
jq -erchanged tojq -r:{}and{"tool_name":null}then get NONE instead of ask. "missing tool_name fails closed" has no test.ask()changed toexit 1: still 21/21. The suite reads stdout and never checks the exit code. Claude Code only processes hook JSON on exit 0, so this regression would turn every ask into "no opinion" and CI would stay green. The "ALWAYS exits 0" contract in the test header has no test behind it.mcp__*__*case filter removed: still 21/21.__segments.(These flips did turn the suite red: drop
-i, drop-x, remove a verb from the list, emitallow.)Tests claiming more than they show (rule 9)
tests/mcp_destructive_guard_test.sh:71has the comment "Executed as a program, like Claude Code does (shebang decides interpreter)". That is false.hooks.jsonrunsbash "<path>", so the shebang and the executable bit do not matter there. The label "guard is executable" is honest; the comment is not.-eflip).The declared scope limit is worse than stated
:44), its header, and the PR body all say the residual risk is "recorded in infra ADR-0027". infra main stops at ADR-0026, and no branch has 0027. The scope limit points to a record that does not exist.Coverage left unguarded: a judgement call, escalated to Mathias on infra#508
I read the gitea-mcp tool list (43 tools) one by one. These still get no opinion:
file_write_branchwithbranch: mainputs arbitrary content straight on main, which is the same effect aspr_merge/tbd_ship, both gated. Onmathias/infrathat is a Flux deploy within about 60 s. Writingcontent: ""empties a file, which is whatfile_deletedoes, andfile_deleteis gated. The test file asserts this pass-through on purpose (file_write_branch passes through).workflow_run_triggeris named in infra#508 itself as one of the destructive tools, but it is missing from the list with no reason given. It can fire CD workflows.tag_create/release_createcan fire release pipelines.delete_repo(upstream gitea-mcp style) gets no opinion. That fits the "match the verb" design, but the header says it covers "any name a host registers it with", which claims more than the code does.Checked and held (not findings)
mcp__.*matcher. Claude Code 2.1.284 runsnew RegExp(matcher).test(name), which is unanchored, so it coversmcp__plugin_skills_gitea__*,mcp__claude_ai_gitea__*and any othermcp__namespace. An invalid regex there matches everything.[a-zA-Z0-9_-]. Trailing\ngets ask.allowis never emitted. A top-level array, empty stdin,nullorfalsegets ask.PR has no
risk/label even though the body says risk/medium.pr-gatekeeper round 2: BLOCKED (head
4a82206)task checkexits 0,task testexits 0 (38/38), and CI runs 8689/8690 are green. Green does not cover the escape below.Escape:
file_write_branchlands on main and the hook has no opinion (verified end to end)The hook reads only the lowercase
.branchkey. gitea-mcp decodes arguments with Goencoding/json, which matches keys case-insensitively and lets the last matching key win. gitea-mcp is built withgolang:1.26-alpine, and I checked this behaviour under go1.26.1.Sorting keys during gitea-mcp's alias re-marshal would normally put lowercase
branchlast. That re-marshal only happens whennormalizeAliaseschanges something, and passingnamealongsiderepostops it from changing anything.tools.FileWriteBranch.Callwith thattool_input: gitea-mcp at 0e2bd3d, built with go1.26.1, against a scratch Gitea 1.27.1 (same version as git.d-ma.be). Result:OK branch=main.case8.txtis onmainwith that content, in commit "case probe 8"."content":"x","Content":""empties a file while the hook seesx.additionalProperties:false, so the extra key is valid input for the model to send.Fix suggestion: collect every key whose
ascii_downcaseisbranch(and the same forcontent). Answer ask if there is not exactly one, or if any of them is main/master/empty. Add the repro above as a test.Untested, while the code is correct today (flip survived 38/38)
.content // ""changed to.content. A missing content key then passes, and gitea-mcp writes an empty file in that case (it only enforcesbranchandmessage). You asked for this case explicitly, and no test covers it.ascii_downcaseremoved: no test usesMain.tool_input: no test. It is safe today because only an exactpasspasses, so the guard is default-deny.command -v jqcheck removed: the no-jq test still passes, becausejq -erthen fails and goes to the printf ask. The behaviour holds, so this is not a finding.Your earlier flips: all now go red
askexit 1 → 28 fail.jq -er→jq -r→ 6 fail. Case filter removed → 1 fail. Second-segment parse → 1 fail. Also red: droppingstrings|select, a trailingallow, dropping master, dropping empty-branch, and droppingworkflow_run_triggerorrelease_create.Checked and held
" main","main ",refs/heads/main,main/,./main,main?x=1andmain%2f. I ran each through real gitea-mcp against Gitea 1.27.1, and none reached main: each was rejected with a GitRefName error or created a separate branch.Main/MAINget ask.tool_inputthat is null, missing, a string or an array gets ask.branchthat is null, false, a number or an array gets ask.contentthat is missing, null, false or""gets ask.Residuals (not blocking; record them in ADR-0027)
*/brain-weekly-ingest." ","\n") passes. It has the same effect as emptying the file, but only on non-main branches, where history makes it recoverable.file_write_branchwithbranch:"refs/heads/main"createsrefs/heads/refs/heads/main. After that, every API write tomainreturns 500 ("dst refspec refs/heads/main matches more than one") until the ref is removed. That is a cheap denial-of-service against the deploy branch.Round-2 review of skills#52 committed to main through gitea-mcp with {"branch":"feat/y8","Branch":"main"}: Go's encoding/json matches keys case-insensitively and the last one wins, while the hook read only the lowercase key. Same trick on content emptied a file. The guard now asks unless exactly one key case-folds to "branch" and one to "content", both spelled exactly, both strings. The branch is cleared by shape ([A-Za-z0-9][A-Za-z0-9._/-]*, no "..", no refs/ prefix, no trailing slash) instead of a blocklist, since refs/heads/main created a second ref that wedged every API write to main. Whitespace-only content asks. 11 new cases; 6 were red before the change, 5 pinned untested-but-correct behaviour the reviewer's flips showed nothing covered. Refs mathias/infra#508 Bump-Type: minor Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>pr-gatekeeper round 3: MERGED (squash
b0d58d5, reviewed heada41f5ab)End to end, against gitea-mcp at 0e2bd3d built with go1.26.1 and a scratch Gitea 1.27.1:
branch:"feat/a","Branch":"main") now gets ask.Branchalone,content+Content, an escaped keybranch, and nbsp-only content all get ask.HEAD,heads/main,main.lock,main/x,main\n,feat.,x/./main,feat//main, duplicate exact keys, andbase:"main"on a new branch. Each either wrote to a separate branch or was rejected (GitRefName, D/F conflict, a URL control character, or an empty segment).Key folding: I scanned every non-ASCII rune at every position of
branchandcontentunder go1.26.1encoding/json. There were 0 fold hits, soascii_downcasematches Go's matching for these two keys.Flips (suite 49/49):
..|^refs/|/$check, droppingascii_downcaseon the value, whitespace changed to empty-only, dropping the content check, dropping master. The round-1 and round-2 flips stay red.Not blocking, hardening for later:
$matches before a trailing newline, sobranch:"main\n"passes the hook. It is unreachable because Go's URL parse rejects control characters before any write.HEADis accepted as a branch name and createsrefs/heads/HEAD. Writes to main kept working afterwards.task checkexits 0,task testexits 0. CI runs 8694/8695 are green.Sequencing note: infra ADR-0027 (265aee6) is still not on infra main at merge time. The hook's reason strings cite it, and the residuals (non-main default branch on brain-weekly-ingest, host stripping unverified, whitespace and
\nedge cases, theHEADbranch) need it as their record.