fix(hooks): pre-bash-guard matches command words, not command text

[Arasz]

pre-bash-guard.sh matched destructive patterns against the raw command string, so it could not tell a command from a mention of one. Every one of these was blocked in a real session, and none of them deletes anything:

grep -rn 'rm -rf' docs/                     # the search pattern
python3 build.py  # the old cleanup used rm -rf
git log --grep='git reset --hard'           # a --grep value
echo "never run git push --force on main"   # a quoted argument
python3 - <<'PY' … PY                       # anywhere in the script body

Same shape as the defect ADR-006 fixed one tier down, and the same cost — a guard that blocks work people legitimately need is one they learn to route around.

What changed

The command is split on ;, &&, ||, |, newlines and grouping, and each segment is matched on its command word: token 0 after stripping VAR=value assignments, sudo/env/command/nohup, and any leading path, so /bin/rm still reads as rm. Quoted strings, # comments and heredoc bodies are data and never trigger a rule.

Coverage went up rather than down. A command-word check alone would have missed these; all are blocked:

sudo rm -rf · /bin/rm -rf · rm.exe -rf	prefixes and paths stripped
xargs rm -rf · find … -exec rm -rf · timeout 30 rm -rf	wrappers that build a command from their arguments
$(rm -rf …) · `rm -rf …`	substitution bodies scanned as commands
bash -lc '…' · sh -ec '…'	any short cluster containing c, not an exact -c token — that match is one letter from a bypass

rm -rf target checking got stricter too: every target must be on the allowlist, not just one of them.

The harness was reporting false passes

Worth flagging on its own. run_with_timeout's fallback backgrounds the hook, and bash gives an asynchronous command /dev/null for stdin unless it carries its own redirection — so the hook read an empty payload, blocked nothing, and every case passed. Wherever timeout is absent (macOS, minimal Linux images) the entire pre-bash-guard suite was vacuous, including the existing force-push test. One <&0 fixes it.

Verification

24 cases, both directions of every rule. Run against the old guard, the six false-positive cases go red and everything else stays green — the block and no-bypass cases exist to prove the parse costs no coverage:

FAIL: allows a grep whose search pattern is 'rm -rf' (exit 2, expected 0)
FAIL: allows a command with the phrase in a trailing # comment (exit 2, expected 0)
FAIL: allows git log --grep for a destructive phrase (exit 2, expected 0)
FAIL: allows a heredoc script body containing the phrase (exit 2, expected 0)
FAIL: allows echoing documentation about force push (exit 2, expected 0)
FAIL: allows a commit message naming a destructive command (exit 2, expected 0)
PASS: still blocks git reset --hard
PASS: still blocks git clean -fd
PASS: still blocks git checkout .
PASS: still blocks git push -f
PASS: still blocks rm -rf on a project path
PASS: allows rm -rf bin
PASS: blocks rm -rf behind env assignments and sudo
PASS: blocks rm -rf inside a combined bash -lc flag
PASS: blocks rm -rf inside a command substitution
PASS: blocks rm -rf passed through xargs
PASS: blocks rm -rf passed through find -exec
6 hook test(s) failed

All 24 pass after the change, on bash 3.2 (macOS — exercises the watchdog fallback) and bash 5.2 (Debian — exercises the GNU timeout path). I have not run it under Git Bash on Windows; that is what the existing CI job covers.

Timing, median of 20 runs on macOS/bash 3.2, dotnet build src/Api/Api.csproj -c Release: 20.9ms → 9.9ms, since parsing happens in-process instead of forking six echo | grep pipelines. A 24KB command is 250ms. My first draft was 6.7s there — scanning bash strings one character at a time re-slices the remainder on every step, so the tokenizer splits on whitespace first and only then scans.

shellcheck --severity=warning clean on hooks/*.sh and .github/scripts/*.sh; check_docs_consistency.py and the version-consistency check both pass.

Scope

New checks I'd also like to contribute — unscoped process kills, dotnet build-server shutdown, recursive deletes of ~/.nuget and ~/.dotnet — are held back for a separate PR so this one stays a bug fix. ADR-007 records why the guard parses instead of matching, the alternatives rejected, and the two-directional test any new rule needs.

The guard is a guardrail against accidents, not a sandbox: eval, base64, or a variable holding the command name still get through, and the ADR says so.

🤖 Generated with Claude Code