diff --git a/hooks/hooks.json b/hooks/hooks.json index 8db511f..5eed7bf 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -33,6 +33,16 @@ "timeout": 5 } ] + }, + { + "matcher": "mcp__.*", + "hooks": [ + { + "type": "command", + "command": "bash \"${CLAUDE_PLUGIN_ROOT}/hooks/mcp-destructive-guard.sh\"", + "timeout": 5 + } + ] } ] } diff --git a/hooks/mcp-destructive-guard.sh b/hooks/mcp-destructive-guard.sh new file mode 100755 index 0000000..d3f3e23 --- /dev/null +++ b/hooks/mcp-destructive-guard.sh @@ -0,0 +1,46 @@ +#!/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 on any settings file. +# +# Policy: match the VERB, not the server. The same tool is reachable as +# mcp__plugin_skills_gitea__*, mcp__claude_ai_gitea__* (connector) or under +# any name a host registers it with, so keying on the server name would let a +# rename bypass the gate. +# +# Emits "ask" or nothing. Never "allow": a hook allow would override the +# user's own deny/ask rules. Malformed input fails closed (ask). +# +# Scope limit, recorded in the ADR: this gates MCP calls only. A session with +# the PAT in its environment can still reach the Gitea REST API via Bash. + +set -uo pipefail + +DESTRUCTIVE='repo_delete|file_delete|branch_delete|pr_merge|tbd_ship|repo_update|repo_mirror_push' + +ask() { + jq -n --arg reason "$1" \ + '{hookSpecificOutput: {hookEventName: "PreToolUse", permissionDecision: "ask", permissionDecisionReason: $reason}}' + exit 0 +} + +INPUT="$(cat)" +if ! TOOL_NAME="$(printf '%s' "$INPUT" | jq -er '.tool_name' 2>/dev/null)"; then + ask "mcp-destructive-guard: could not parse the tool call, so it cannot be shown safe. Approve only if you expected it." +fi + +verb="${TOOL_NAME##*__}" +case "$TOOL_NAME" in + mcp__*__*) ;; + *) exit 0 ;; +esac + +if printf '%s' "$verb" | grep -Eqix "($DESTRUCTIVE)"; then + ask "Irreversible MCP verb '$verb' (infra ADR-0027): needs your approval. The model filled in every argument, including any confirm field." +fi +exit 0 diff --git a/tests/mcp_destructive_guard_test.sh b/tests/mcp_destructive_guard_test.sh new file mode 100755 index 0000000..21fdf8f --- /dev/null +++ b/tests/mcp_destructive_guard_test.sh @@ -0,0 +1,75 @@ +#!/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; +# no decision emitted ("NONE") = the guard stays out of the way and normal +# permission rules apply. The guard never emits "allow": an allow from a hook +# would bypass the user's own deny/ask rules. + +set -u + +ROOT=$(CDPATH= cd -- "$(dirname -- "$0")/.." && pwd) +GUARD="$ROOT/hooks/mcp-destructive-guard.sh" + +pass=0 +fail=0 + +# An empty stdout is a valid hook answer ("no opinion"), reported as NONE. +decode() { + _out=$(cat) + [ -z "$_out" ] && { echo NONE; return; } + printf '%s' "$_out" | jq -r '.hookSpecificOutput.permissionDecision // "NONE"' 2>/dev/null || echo BROKEN +} + +decision_for() { + jq -n --arg t "$1" '{tool_name: $t, tool_input: {owner: "mathias", repo: "x"}}' \ + | "$GUARD" | decode +} + +expect() { + _want=$1 _tool=$2 _label=$3 + _got=$(decision_for "$_tool") + if [ "$_got" = "$_want" ]; then + pass=$((pass + 1)); printf ' ok %s\n' "$_label" + else + fail=$((fail + 1)); printf ' FAIL %s\n want=%s got=%s tool=%s\n' "$_label" "$_want" "$_got" "$_tool" + fi +} + +echo "mcp-destructive-guard tests" + +# Irreversible verbs, on the plugin namespace. +for v in repo_delete file_delete branch_delete pr_merge tbd_ship repo_update repo_mirror_push; do + expect ask "mcp__plugin_skills_gitea__$v" "plugin gitea $v asks" +done + +# The same verb reached through a different server name must not slip past: +# the claude.ai connector, a bare local registration, or any future rename. +expect ask "mcp__claude_ai_gitea__repo_delete" "connector namespace asks" +expect ask "mcp__gitea__repo_delete" "bare server name asks" +expect ask "mcp__git__repo_delete" "renamed server still asks" + +# Case variants must not bypass (tool names are matched exactly by the host, +# but a guard that lowercases nothing should not depend on that). +expect ask "mcp__plugin_skills_gitea__REPO_DELETE" "uppercase verb asks" + +# Reversible / read verbs pass through untouched. +for v in issue_close issue_create file_read repo_list pr_create file_write_branch; do + expect NONE "mcp__plugin_skills_gitea__$v" "$v passes through" +done + +# Near-miss names are not the guarded verb. +expect NONE "mcp__plugin_skills_gitea__repo_delete_dry_run_list" "suffix near-miss passes" +expect NONE "Bash" "non-MCP tool passes" + +# Malformed input fails CLOSED (ask), never open. +_got=$(printf 'not json' | "$GUARD" | decode) +if [ "$_got" = "ask" ]; then pass=$((pass+1)); echo " ok malformed input asks"; else fail=$((fail+1)); echo " FAIL malformed input: got=$_got"; fi + +# Executed as a program, like Claude Code does (shebang decides interpreter). +[ -x "$GUARD" ] && { pass=$((pass+1)); echo " ok guard is executable"; } || { fail=$((fail+1)); echo " FAIL guard not executable"; } + +printf '\n%d passed, %d failed\n' "$pass" "$fail" +[ "$fail" -eq 0 ]