Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 121 additions & 1 deletion .github/scripts/test-hooks.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@
# payload that merely MENTIONS "reset --hard" in a
# string field -> exit 0 (regression test for the
# no-jq raw-payload over-blocking bug)
# a destructive string quoted, commented or fed to a
# heredoc -> exit 0 (it is data, not a command)
# the same command word reached through sudo, env,
# sh -c, $(...), xargs or find -exec -> exit 2
#
# Nothing here executes a destructive command: every case is a command STRING
# handed to the guard inside a payload.
set -u

cd "$(dirname "$0")/../.." || exit 1
Expand All @@ -32,7 +39,10 @@ run_with_timeout() {
timeout --kill-after=5 "$limit" "$@"
return $?
fi
"$@" &
# <&0 is load-bearing: without an explicit redirection bash gives an
# asynchronous command /dev/null for stdin, so the hook under test would read
# an empty payload, block nothing, and every case would "pass".
"$@" <&0 &
local pid=$!
(
sleep "$limit"
Expand Down Expand Up @@ -142,6 +152,116 @@ else
fail "pre-bash-guard exited $rc (expected 0) — over-blocking regression: payload text matched instead of the parsed command"
fi

echo ""
echo "=== pre-bash-guard.sh: command word vs command text ==="

# The guard must judge what a command RUNS, not what its text contains. These
# cases pair each destructive command with a benign one that merely mentions it.
json_escape() {
local s=$1
s=${s//\\/\\\\}
s=${s//\"/\\\"}
s=${s//$'\n'/\\n}
s=${s//$'\t'/\\t}
printf '%s' "$s"
}

# guard_case <expected-exit> <label> <command string>
guard_case() {
local expect="$1" label="$2" cmd="$3"
printf '{"session_id":"ci","hook_event_name":"PreToolUse","tool_name":"Bash","tool_input":{"command":"%s","description":"ci case"}}' \
"$(json_escape "$cmd")" >"$WORKDIR/g.json"
run_hook 10 hooks/pre-bash-guard.sh "$WORKDIR/g.json" "$WORKDIR/go" "$WORKDIR/ge"
local rc=$?
if [ "$rc" -eq "$expect" ]; then
pass "$label"
else
echo " stdout: [$(cat "$WORKDIR/go")]"
echo " stderr: [$(cat "$WORKDIR/ge")]"
fail "$label (exit $rc, expected $expect)"
fi
}

# 6. A destructive string as DATA — quoted argument, comment, heredoc body.
# Each of these was blocked before the guard parsed commands.
guard_case 0 "allows a grep whose search pattern is 'rm -rf'" \
"grep -rn 'rm -rf' docs/"
guard_case 0 "allows a command with the phrase in a trailing # comment" \
"python3 build.py # the old cleanup used rm -rf and shutil.rmtree"
guard_case 0 "allows git log --grep for a destructive phrase" \
"git log --grep='git reset --hard' --oneline"
guard_case 0 "allows a heredoc script body containing the phrase" \
"python3 - <<'PY'
import shutil
# replaced the old rm -rf call
print(1)
PY"
guard_case 0 "allows echoing documentation about force push" \
'echo "never run git push --force on main"'
guard_case 0 "allows a commit message naming a destructive command" \
'git commit -m "docs: warn against rm -rf in scripts"'

# 7. Every pre-existing block must still block.
guard_case 2 "still blocks git reset --hard" "git reset --hard HEAD~1"
guard_case 2 "still blocks git clean -fd" "git clean -fd"
guard_case 2 "still blocks git checkout ." "git checkout ."
guard_case 2 "still blocks git push -f" "git push -f origin main"
guard_case 2 "still blocks rm -rf on a project path" "rm -rf /Users/dev/project/src"

# 8. Safe rm -rf targets must stay allowed.
guard_case 0 "allows rm -rf bin" "rm -rf bin"
guard_case 0 "allows rm -rf on a nested obj directory" "rm -rf ./src/App/obj"
guard_case 0 "allows rm -rf under /tmp" "rm -rf /tmp/scratch"
guard_case 0 "allows rm -rf bin with a redirect" "rm -rf bin > /dev/null"

# 9. Reaching the same command word indirectly must not evade the guard.
guard_case 2 "blocks rm -rf behind a leading path" "/bin/rm -rf /Users/dev/project"
guard_case 2 "blocks rm -rf behind env assignments and sudo" \
"FOO=bar sudo rm -rf /Users/dev/project"
guard_case 2 "blocks rm -rf in a later segment" \
"echo building && rm -rf /Users/dev/project"
guard_case 2 "blocks rm -rf inside sh -c" "sh -c 'rm -rf /Users/dev/project'"
guard_case 2 "blocks rm -rf inside a combined bash -lc flag" \
"bash -lc 'rm -rf /Users/dev/project'"
guard_case 2 "blocks git reset --hard inside bash -cx" \
"bash -cx 'git reset --hard HEAD~1'"
guard_case 2 "blocks rm -rf inside a command substitution" \
'echo $(rm -rf /Users/dev/project)'
guard_case 2 "blocks rm -rf passed through xargs" \
"find . -name obj | xargs rm -rf /Users/dev/project"
guard_case 2 "blocks rm -rf passed through find -exec" \
"find . -name '*.tmp' -exec rm -rf /Users/dev/project {} ;"

echo ""
echo "=== pre-bash-guard.sh: shared machine resources ==="

# 10. Process kills that match by pattern reap whatever else is running.
guard_case 2 "blocks pkill by pattern" "pkill -f dotnet"
guard_case 2 "blocks killall by process name" "killall dotnet"
guard_case 2 "blocks killall reached by absolute path" "/usr/bin/killall node"
guard_case 2 "blocks kill against a pgrep substitution" 'kill -9 $(pgrep -f MyApp)'
guard_case 2 "blocks kill against a process name" "kill dotnet"
guard_case 0 "allows kill against a numeric PID" "kill -9 12345"
guard_case 0 "allows kill -s with a numeric PID" "kill -s TERM 4242"
guard_case 0 "allows kill against a PID held in a variable" 'kill $SERVER_PID'
guard_case 0 "allows pgrep, which only reads" "pgrep -f dotnet"
guard_case 0 "allows a command that merely mentions pkill" \
'echo "run pkill -f dotnet if it hangs"'

# 11. Daemons and caches shared by every project on the machine.
guard_case 2 "blocks dotnet build-server shutdown" "dotnet build-server shutdown"
guard_case 0 "allows dotnet build" "dotnet build -c Release"
guard_case 2 "blocks recursive delete of the NuGet package cache" \
"rm -rf ~/.nuget/packages"
guard_case 2 "blocks recursive delete of ~/.dotnet without -f" "rm -r ~/.dotnet"
guard_case 2 "blocks a cache path whose leaf looks like build output" \
"rm -rf ~/.dotnet/bin"
guard_case 2 "blocks the cache path spelled with \$HOME" \
'rm -rf $HOME/.nuget/packages'
guard_case 0 "allows rm -rf on project build output" "rm -rf src/App/obj"
guard_case 0 "allows a command that merely mentions the cache path" \
'echo "clear ~/.nuget/packages if restore is wedged"'

echo ""
if [ "$FAILURES" -gt 0 ]; then
echo "$FAILURES hook test(s) failed"
Expand Down
17 changes: 17 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,23 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]

### Fixed
- **`hooks/pre-bash-guard.sh` now matches command words, not command text** — the guard grepped the raw command string, so it blocked commands that merely *mentioned* a destructive one: a `grep` searching for `rm -rf`, a `#` comment describing an old cleanup, `git log --grep='git reset --hard'`, `echo`ing docs about force push, and any heredoc script body containing the phrase. It now splits the command on `;`, `&&`, `||`, `|`, newlines and grouping, and matches only each segment's command word — taken 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: `sudo rm -rf`, `xargs rm -rf`, `find -exec rm -rf`, `$(...)` and `` `...` `` bodies, and combined `sh -c` flag spellings (`-lc`, `-cx`, `-ec`) are now recognised — an exact `-c` token match had been one letter away from a bypass
- Parsing runs in-process instead of spawning six `echo | grep` pipelines, so a typical command costs ~15ms instead of ~79ms
- **`.github/scripts/test-hooks.sh` reported false passes where `timeout` is absent** — the watchdog fallback backgrounds the hook, and bash gives an asynchronous command `/dev/null` for stdin unless it carries its own redirection. The hook under test read an empty payload, blocked nothing, and every case "passed". On macOS and minimal Linux images the whole pre-bash-guard suite was vacuous

### Added
- **ADR-007** — why the Bash guard parses commands instead of matching text, the alternatives rejected, and the two-directional test every new rule needs
- 42 hook behavior tests covering both directions of each guard rule: the destructive command blocked, and a benign command that merely mentions it allowed
- **`hooks/pre-bash-guard.sh` guards resources shared across the machine**, not just the working tree — on a developer box running several agents, or any shared build host, these reach past the current project:
- `pkill` and `killall` match by name or pattern, so they reap every match on the machine. `kill` is blocked when its target is not a numeric PID, a job spec, or a `$VAR` holding one — `kill $(pgrep -f MyApp)` kills whatever else matched
- `dotnet build-server shutdown` stops the MSBuild, Razor and VB/C# compiler servers for every process on the machine, not just the current build
- Recursive deletes of `~/.nuget`, `~/.dotnet`, `~/.templateengine` and `~/.local/share/NuGet`, with or without `-f`. `rm -rf ~/.dotnet/bin` was previously allowed outright, since the target ends in `bin`

## [0.12.0] — 2026-08-07

Roslyn Navigator correctness release. Symbol resolution, response contracts, and workspace reload behaviour are corrected, and two tools are added: stack trace resolution and change impact analysis.
Expand Down
53 changes: 52 additions & 1 deletion hooks/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,61 @@ Claude works:

| Script | Event | Purpose |
|---|---|---|
| `pre-bash-guard.sh` | PreToolUse (Bash) | Blocks destructive commands (force push, `git reset --hard`, unsafe `rm -rf`) |
| `pre-bash-guard.sh` | PreToolUse (Bash) | Blocks destructive commands (force push, `git reset --hard`, unsafe `rm -rf`, unscoped process kills, shared-cache deletes) — see [below](#why-does-the-bash-guard-parse-commands-instead-of-grepping-them) |
| `post-edit-format.sh` | PostToolUse (Edit\|Write) | Runs `dotnet format` on edited `.cs` files |
| `post-scaffold-restore.sh` | PostToolUse (Edit\|Write) | Runs `dotnet restore` after `.csproj` changes |

### Why does the Bash guard parse commands instead of grepping them?

Because grepping the command string cannot tell a command from a mention of
one. The guard used to match `rm\s+-[a-zA-Z]*r[a-zA-Z]*f` against the whole
string, which blocked all of these — none of which deletes anything:

```bash
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
```

So the guard splits the command on `;`, `&&`, `||`, `|`, newlines and grouping,
and matches only each segment's **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.

Parsing also closes the gaps a text scan left open. These reach a real `rm -rf`
and are all blocked:

```bash
sudo rm -rf /project xargs rm -rf find . -exec rm -rf {} \;
bash -lc 'rm -rf /project' $(rm -rf /project) /bin/rm -rf /project
```

The `-c` handling matches any short-flag cluster containing `c` (`-lc`, `-cx`,
`-ec`), not an exact `-c` token — an exact match is one letter from a bypass.

Deliberate obfuscation (`eval`, base64, a variable holding the command name)
still gets through. This is a guardrail against accidents, not a sandbox.
Rationale, alternatives, and the process for adding a rule:
[ADR-007](../knowledge/decisions/007-command-word-guard.md).

### What the guard blocks

| Rule | Blocked | Allowed |
|---|---|---|
| Force push | `git push --force`, `git push -f` | `git push origin feature/x` |
| History and worktree loss | `git reset --hard`, `git clean -fd`, `git checkout .` | `git clean -nd`, `git checkout main` |
| Recursive delete | `rm -rf /any/other/path` | `rm -rf bin`, `obj`, `TestResults`, `node_modules`, `.vs`, anything under `/tmp` |
| Shared package caches | `rm -r ~/.nuget`, `~/.dotnet`, `~/.templateengine`, `~/.local/share/NuGet` — with or without `-f` | project build output, including `src/App/bin` |
| Unscoped process kills | `pkill`, `killall`, `kill $(pgrep …)`, `kill <name>` | `kill 12345`, `kill -s TERM 4242`, `kill $SERVER_PID`, `pgrep` |
| Shared daemons | `dotnet build-server shutdown` | `dotnet build`, `dotnet nuget locals all --list` |
| Risky but allowed | — | `dotnet run` (warns about `launchSettings.json`) |

The process rules exist because a developer box often runs more than one agent.
`pkill -f dotnet` does not know which `dotnet` was yours. If you hold a PID,
kill it; the guard only stops kills that go looking for a target.

## Git pre-commit hooks (install manually)

These are standard git hooks, not Claude Code hooks. Wire them into your
Expand Down
Loading