Compare commits

..
2 Commits
Author SHA1 Message Date
mathias 61c68c01b0 chore(release): plugin.json version 0.22.1
Skip-Release: true
2026-10-01 20:22:24 +00:00
mathiasandClaude Opus 5.5 b0d58d5adf feat(hooks): mcp-destructive-guard asks before irreversible MCP verbs (#52)
CI / Check / Test (push) Successful in 5s
CI / Tag release (push) Successful in 2s
PreToolUse hook on mcp__.*: asks for repo_delete, file_delete, branch_delete, pr_merge, tbd_ship, repo_update, repo_mirror_push, workflow_run_trigger, tag_create, release_create on any server name, and for file_write_branch unless it targets a plain non-main/master branch with non-empty content under exactly one exact-case branch/content key. Fails closed; never emits allow.

Refs mathias/infra#508. Reviewed in three rounds by pr-gatekeeper with execution against gitea-mcp and Gitea 1.27.1.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Mathias <mthbqv@gmail.com>
2026-10-01 20:21:52 +00:00
5 changed files with 216 additions and 2 deletions
+1 -1
View File
@@ -1,7 +1,7 @@
{ {
"name": "skills", "name": "skills",
"description": "Mathias's engineering skills library, plus the hooks and MCP servers that make it enforceable: TDD, clean code, debugging, specs, discovery, CI, and secret hygiene.", "description": "Mathias's engineering skills library, plus the hooks and MCP servers that make it enforceable: TDD, clean code, debugging, specs, discovery, CI, and secret hygiene.",
"version": "0.22.0", "version": "0.22.1",
"author": { "author": {
"name": "Mathias", "name": "Mathias",
"url": "https://git.d-ma.be/mathias" "url": "https://git.d-ma.be/mathias"
+1 -1
View File
@@ -29,7 +29,7 @@ descriptions and "use when" triggers.
/plugin install skills@mathias /plugin install skills@mathias
``` ```
The plugin ships skills, the `secret-guard` + `pre-task-ritual` hooks, and the The plugin ships skills, the `secret-guard`, `mcp-destructive-guard` and `pre-task-ritual` hooks, and the
brain/gitea/infra MCP servers as one versioned unit. Skills are namespaced brain/gitea/infra MCP servers as one versioned unit. Skills are namespaced
(`skills:tdd`). Updates land when `.claude-plugin/plugin.json`'s `version` (`skills:tdd`). Updates land when `.claude-plugin/plugin.json`'s `version`
moves — CI bumps it on every release. moves — CI bumps it on every release.
+10
View File
@@ -33,6 +33,16 @@
"timeout": 5 "timeout": 5
} }
] ]
},
{
"matcher": "mcp__.*",
"hooks": [
{
"type": "command",
"command": "bash \"${CLAUDE_PLUGIN_ROOT}/hooks/mcp-destructive-guard.sh\"",
"timeout": 5
}
]
} }
] ]
} }
+86
View File
@@ -0,0 +1,86 @@
#!/usr/bin/env bash
# PreToolUse hook (mcp__.* matcher) — makes a human approve irreversible MCP
# verbs instead of letting the model run them unattended.
#
# Why (infra#508, infra ADR-0027): gitea-mcp authenticates by PAT pass-through,
# so an agent holds the full scope of the caller's PAT. repo_delete's own guard
# (confirm == repo) is filled in by the model on both sides: it stops typos,
# not an injected or mistaken agent. The infra-mcp README claimed client-side
# deny lists covered this; on 2026-10-01 none existed in any settings file.
#
# Policy: match gitea-mcp's verb spellings on ANY server name. The same tool is
# reachable as mcp__plugin_skills_gitea__*, mcp__claude_ai_gitea__* or under
# whatever name a host registers it with, so keying on the server name would
# let a rename bypass the gate. It does NOT recognise other servers' spellings
# of the same operation (e.g. a third-party `delete_repo`).
#
# file_write_branch is gated when it would land on main/master (on infra that
# is a Flux deploy within ~60s — the same effect as pr_merge) or empty a file
# (the same effect as file_delete). Writes to other branches pass.
#
# Emits "ask" or nothing; never "allow" (a hook allow would override the
# user's own deny/ask rules). Always exits 0: Claude Code ignores a hook's
# JSON on a non-zero exit. Anything it cannot parse — including a host
# without jq — fails closed to "ask". The ask JSON is a literal, so the
# fail-closed path needs no tool that might be missing.
#
# Scope limit, recorded in infra ADR-0027: this gates MCP calls only. A
# session with the PAT in its environment can still reach the Gitea REST API
# through a shell.
set -u
DESTRUCTIVE='repo_delete|file_delete|branch_delete|pr_merge|tbd_ship|repo_update|repo_mirror_push|workflow_run_trigger|tag_create|release_create'
ask() {
# $1 must contain only [A-Za-z0-9 _.:()'/-] — it is spliced into JSON verbatim.
printf '{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"ask","permissionDecisionReason":"%s"}}\n' "$1"
exit 0
}
INPUT="$(cat)"
command -v jq >/dev/null 2>&1 \
|| ask "mcp-destructive-guard: jq is not installed, so this MCP call cannot be checked. Approve only if you expected it."
if ! TOOL_NAME="$(printf '%s' "$INPUT" | jq -er '.tool_name | strings | select(length > 0)' 2>/dev/null)"; then
ask "mcp-destructive-guard: could not read a tool name from this call, so it cannot be shown safe. Approve only if you expected it."
fi
case "$TOOL_NAME" in
mcp__*__*) ;;
*) exit 0 ;;
esac
verb="${TOOL_NAME##*__}"
safe_verb="$(printf '%s' "$verb" | tr -cd 'A-Za-z0-9_-')"
if printf '%s' "$verb" | grep -Eqix "($DESTRUCTIVE)"; then
ask "Irreversible MCP verb '$safe_verb' (infra ADR-0027): needs your approval. The model filled in every argument, including any confirm field."
fi
if printf '%s' "$verb" | grep -Eqix 'file_write_branch'; then
# Cleared only when there is exactly ONE key that case-folds to "branch" and
# one to "content": gitea-mcp decodes with Go encoding/json, which matches
# keys case-insensitively and lets the last one win, so {"branch":"feat/x",
# "Branch":"main"} landed on main while a lowercase-only read saw feat/x.
# The branch is cleared by SHAPE (allowlist), not by a blocklist of names:
# "refs/heads/main" created a second ref that wedged API writes to main.
verdict="$(printf '%s' "$INPUT" | jq -r '
.tool_input as $i
| if ($i | type) != "object" then "ask" else
([$i | to_entries[] | select(.key | ascii_downcase == "branch")]) as $b
| ([$i | to_entries[] | select(.key | ascii_downcase == "content")]) as $c
| if ($b | length) != 1 or ($c | length) != 1 then "ask"
elif ($b[0].key != "branch") or ($c[0].key != "content") then "ask"
elif ($b[0].value | type) != "string" or ($c[0].value | type) != "string" then "ask"
elif ($b[0].value | test("^[A-Za-z0-9][A-Za-z0-9._/-]*$") | not) then "ask"
elif ($b[0].value | test("\\.\\.|^refs/|/$")) then "ask"
elif ($b[0].value | ascii_downcase | . == "main" or . == "master") then "ask"
elif ($c[0].value | test("^\\s*$")) then "ask"
else "pass" end
end' 2>/dev/null || echo ask)"
[ "$verdict" = "pass" ] \
|| ask "file_write_branch to main/master, an unusual branch name, or emptying a file (infra ADR-0027): needs your approval."
fi
exit 0
+118
View File
@@ -0,0 +1,118 @@
#!/bin/sh
# Tests for hooks/mcp-destructive-guard.sh — the PreToolUse gate that makes a
# human approve irreversible MCP verbs (infra#508, infra ADR-0027).
#
# Contract: reads PreToolUse hook JSON on stdin, ALWAYS exits 0, and answers
# in hookSpecificOutput.permissionDecision. "ask" = a human must approve;
# empty stdout ("NONE") = no opinion, normal permission rules apply. The guard
# never emits "allow": an allow from a hook would bypass the user's own rules.
#
# Cases marked (gatekeeper) are escapes found by the skills#52 review: each
# passed the first version of this suite.
set -u
ROOT=$(CDPATH= cd -- "$(dirname -- "$0")/.." && pwd)
GUARD="$ROOT/hooks/mcp-destructive-guard.sh"
pass=0
fail=0
# run_guard <json> [PATH] — invoked exactly as hooks.json does: `bash <path>`.
# Prints "<decision> rc=<exit>", decision NONE for an empty answer.
run_guard() {
if [ $# -ge 2 ]; then
_out=$(printf '%s' "$1" | PATH="$2" "$BASH_BIN" "$GUARD"); _rc=$?
else
_out=$(printf '%s' "$1" | bash "$GUARD"); _rc=$?
fi
if [ -z "$_out" ]; then _d=NONE
else _d=$(printf '%s' "$_out" | jq -r '.hookSpecificOutput.permissionDecision // "NONE"' 2>/dev/null || echo BROKEN)
fi
printf '%s rc=%s' "$_d" "$_rc"
}
BASH_BIN=$(command -v bash)
check() {
_want=$1 _json=$2 _label=$3
_got=$(run_guard "$_json")
if [ "$_got" = "$_want rc=0" ]; then
pass=$((pass + 1)); printf ' ok %s\n' "$_label"
else
fail=$((fail + 1)); printf ' FAIL %s\n want=%s rc=0 got=%s\n in=%s\n' "$_label" "$_want" "$_got" "$_json"
fi
}
tool() { printf '{"tool_name":"%s","tool_input":{"owner":"mathias","repo":"x"}}' "$1"; }
echo "mcp-destructive-guard tests"
for v in repo_delete file_delete branch_delete pr_merge tbd_ship repo_update repo_mirror_push \
workflow_run_trigger tag_create release_create; do
check ask "$(tool "mcp__plugin_skills_gitea__$v")" "plugin gitea $v asks"
done
# The verb is matched on any server name: connector, bare, or renamed.
check ask "$(tool mcp__claude_ai_gitea__repo_delete)" "connector namespace asks"
check ask "$(tool mcp__gitea__repo_delete)" "bare server name asks"
check ask "$(tool mcp__git__repo_delete)" "renamed server still asks"
check ask "$(tool mcp__plugin_skills_gitea__REPO_DELETE)" "uppercase verb asks"
# The verb is the LAST segment (gatekeeper: parsing the second segment stayed green).
check ask "$(tool mcp__a__b__repo_delete)" "verb taken from last segment"
check NONE "$(tool mcp__repo_delete__issue_get)" "server named like a verb does not trigger"
# file_write_branch: content landing on main, or emptying a file, is the same
# effect as the gated pr_merge / file_delete (gatekeeper).
fwb() { printf '{"tool_name":"mcp__plugin_skills_gitea__file_write_branch","tool_input":{"branch":"%s","content":"%s"}}' "$1" "$2"; }
check ask "$(fwb main x)" "file_write_branch to main asks"
check ask "$(fwb master x)" "file_write_branch to master asks"
check ask "$(fwb feat/y '')" "file_write_branch emptying a file asks"
check ask '{"tool_name":"mcp__plugin_skills_gitea__file_write_branch","tool_input":{"content":"x"}}' "file_write_branch without branch asks"
check NONE "$(fwb feat/y x)" "file_write_branch to a feature branch passes"
# Go's encoding/json matches keys case-insensitively, last one wins: gitea-mcp
# wrote to main from {"branch":"feat/y8","Branch":"main"} while the hook read
# only `branch` (gatekeeper round 2, verified against a real Gitea). So the
# guard clears a write only when exactly one key case-folds to branch/content.
FWB=mcp__plugin_skills_gitea__file_write_branch
check ask "{\"tool_name\":\"$FWB\",\"tool_input\":{\"branch\":\"feat/y8\",\"Branch\":\"main\",\"content\":\"x\"}}" "case-variant Branch key asks"
check ask "{\"tool_name\":\"$FWB\",\"tool_input\":{\"branch\":\"feat/y\",\"content\":\"x\",\"Content\":\"\"}}" "case-variant Content key asks"
check ask "{\"tool_name\":\"$FWB\",\"tool_input\":{\"BRANCH\":\"feat/y\",\"content\":\"x\"}}" "only an uppercase BRANCH key asks"
check ask "$(fwb Main x)" "file_write_branch to Main asks"
check ask "{\"tool_name\":\"$FWB\",\"tool_input\":{\"branch\":\"feat/y\"}}" "missing content key asks"
check ask "$(fwb feat/y ' ')" "whitespace-only content asks"
check ask "{\"tool_name\":\"$FWB\",\"tool_input\":\"branch=feat/y\"}" "non-object tool_input asks"
# Branch names are cleared by shape, not by blocklist: refs/heads/main created a
# second ref that wedged every API write to main (gatekeeper side finding).
check ask "$(fwb refs/heads/main x)" "refs/ prefixed branch asks"
check ask "$(fwb ' main' x)" "branch with whitespace asks"
check ask "$(fwb '../main' x)" "branch with .. asks"
check NONE "$(fwb release/v1.2_x x)" "ordinary branch name passes"
# Reversible / read verbs pass through.
for v in issue_close issue_create file_read repo_list pr_create; do
check NONE "$(tool "mcp__plugin_skills_gitea__$v")" "$v passes through"
done
check NONE "$(tool mcp__plugin_skills_gitea__repo_delete_dry_run_list)" "suffix near-miss passes"
check NONE "$(tool Bash)" "non-MCP tool passes"
check NONE "$(tool repo_delete)" "non-MCP tool named like a verb passes"
# Malformed input fails CLOSED (gatekeeper: non-string tool_name got NONE).
check ask 'not json' "non-JSON asks"
check ask '{}' "missing tool_name asks"
check ask '{"tool_name":null}' "null tool_name asks"
check ask '{"tool_name":""}' "empty tool_name asks"
check ask '{"tool_name":5}' "number tool_name asks"
check ask '{"tool_name":["mcp__x__repo_delete"]}' "array tool_name asks"
check ask '{"tool_name":{"a":1}}' "object tool_name asks"
# Without jq on PATH the guard must still answer ask, not stay silent
# (gatekeeper: the fail-closed path itself needed jq).
nojq=$(mktemp -d); trap 'rm -rf "$nojq"' EXIT
for b in bash cat grep tr sed; do ln -s "$(command -v $b)" "$nojq/$b"; done
_got=$(run_guard "$(tool mcp__plugin_skills_gitea__repo_delete)" "$nojq")
if [ "$_got" = "ask rc=0" ]; then pass=$((pass+1)); echo " ok no jq on PATH still asks"; else fail=$((fail+1)); echo " FAIL no jq on PATH: got=$_got"; fi
_got=$(run_guard "$(tool mcp__plugin_skills_gitea__issue_get)" "$nojq")
if [ "$_got" = "ask rc=0" ]; then pass=$((pass+1)); echo " ok no jq on PATH asks even for a read verb (cannot parse = cannot clear)"; else fail=$((fail+1)); echo " FAIL no jq read verb: got=$_got"; fi
printf '\n%d passed, %d failed\n' "$pass" "$fail"
[ "$fail" -eq 0 ]