From c0c1c9a2b10f39b9a26a0ce7e4dfc9b2048ee220 Mon Sep 17 00:00:00 2001 From: skateddu Date: Thu, 13 Aug 2026 15:45:28 +0200 Subject: [PATCH] fix(hooks): stop matching patterns against heredoc prose Both PreToolUse hooks matched their patterns against the raw command string, so text carried in a heredoc body was read as code. A `gh pr create` whose description quoted tooling after an `&&`, or a destructive command at the start of a line, was denied outright. Found in practice: enforce-uv refused the `gh pr create` that opened the documentation coherence pass, because the PR body quoted a verification command. protect-main had the same latent flaw and had escaped it only because earlier PR bodies happened to put a backtick where the pattern needed whitespace. Both now match against the command with heredoc bodies stripped, via a shared lib. Rewrites still emit the original command, and code before or after a heredoc is unaffected. Adding that lib surfaced a second bug: .gitignore's unanchored `lib/` rule silently excluded it, and would do the same to a nested source directory in any project built from this template. Co-Authored-By: Claude Opus 5 --- .claude/hooks/enforce-uv.sh | 29 ++++--- .claude/hooks/lib/command-text.sh | 44 +++++++++++ .claude/hooks/protect-main.sh | 16 +++- .gitignore | 7 +- CHANGELOG.md | 2 + README.md | 4 + tests/unit/test_heredoc_false_positives.py | 90 ++++++++++++++++++++++ 7 files changed, 175 insertions(+), 17 deletions(-) create mode 100644 .claude/hooks/lib/command-text.sh create mode 100644 tests/unit/test_heredoc_false_positives.py diff --git a/.claude/hooks/enforce-uv.sh b/.claude/hooks/enforce-uv.sh index 8d7b7d1..fded52d 100644 --- a/.claude/hooks/enforce-uv.sh +++ b/.claude/hooks/enforce-uv.sh @@ -5,6 +5,9 @@ # Requires: jq (https://jqlang.github.io/jq/) set -euo pipefail +# shellcheck source=lib/command-text.sh +source "$(dirname "${BASH_SOURCE[0]}")/lib/command-text.sh" + # Emit a PreToolUse "deny" decision in the current hook output format and exit. # See https://code.claude.com/docs/en/hooks (PreToolUse uses hookSpecificOutput). deny() { @@ -26,54 +29,58 @@ command=$(echo "$input" | jq -r '.tool_input.command // empty') [ -z "$command" ] && exit 0 +# Match against the code the shell will run, not against prose carried in a +# heredoc body. Rewrites below still emit the original, unmodified command. +scannable=$(printf '%s\n' "$command" | strip_heredoc_bodies) + # Skip if already using uv -[[ "$command" == *"uv run"* || "$command" == *"uv add"* || "$command" == *"uv remove"* || "$command" == *"uv sync"* || "$command" == *"uv pip"* || "$command" == *"uvx "* ]] && exit 0 +[[ "$scannable" == *"uv run"* || "$scannable" == *"uv add"* || "$scannable" == *"uv remove"* || "$scannable" == *"uv sync"* || "$scannable" == *"uv pip"* || "$scannable" == *"uvx "* ]] && exit 0 # Auto-rewrite a simple, single bare-tool invocation (command starts with the tool) # to run under uv. Compound commands and pip are left to the deny rules below, where # the right transformation is ambiguous (e.g. pip install -e . is not uv add). -if echo "$command" | grep -qE '^\s*(python3?|pytest|ruff|mypy|bandit)\b' \ - && ! echo "$command" | grep -qE '^\s*python3?\s+-m\s+pip\b'; then +if echo "$scannable" | grep -qE '^\s*(python3?|pytest|ruff|mypy|bandit)\b' \ + && ! echo "$scannable" | grep -qE '^\s*python3?\s+-m\s+pip\b'; then trimmed=$(echo "$command" | sed -E 's/^[[:space:]]+//') allow_rewrite "uv run $trimmed" "Rewrote to run under uv's managed environment: uv run $trimmed" fi # pip install → uv add -if echo "$command" | grep -qE '(^|[;&|]\s*)pip3?\s+install\b'; then +if echo "$scannable" | grep -qE '(^|[;&|]\s*)pip3?\s+install\b'; then deny "Use \`uv add \` instead of pip install. For dev deps: \`uv add --dev \`. To sync: \`uv sync\`." fi # Any other pip usage -if echo "$command" | grep -qE '(^|[;&|]\s*)pip3?\s'; then +if echo "$scannable" | grep -qE '(^|[;&|]\s*)pip3?\s'; then deny "Use uv instead of pip. Examples: \`uv add \`, \`uv remove \`, \`uv sync\`, \`uv pip list\`." fi # python -m pip -if echo "$command" | grep -qE '(^|[;&|]\s*)python3?\s+-m\s+pip\b'; then +if echo "$scannable" | grep -qE '(^|[;&|]\s*)python3?\s+-m\s+pip\b'; then deny "Use uv instead of python -m pip. Examples: \`uv add \`, \`uv sync\`." fi # Bare python → uv run python -if echo "$command" | grep -qE '(^|[;&|]\s*)python3?\s'; then +if echo "$scannable" | grep -qE '(^|[;&|]\s*)python3?\s'; then deny "Use \`uv run python ...\` instead of bare python. This ensures the correct virtual environment." fi # Bare pytest → uv run pytest -if echo "$command" | grep -qE '(^|[;&|]\s*)pytest\b'; then +if echo "$scannable" | grep -qE '(^|[;&|]\s*)pytest\b'; then deny "Use \`uv run pytest ...\` instead of bare pytest." fi # Bare ruff → uv run ruff -if echo "$command" | grep -qE '(^|[;&|]\s*)ruff\b'; then +if echo "$scannable" | grep -qE '(^|[;&|]\s*)ruff\b'; then deny "Use \`uv run ruff ...\` instead of bare ruff." fi # Bare mypy → uv run mypy -if echo "$command" | grep -qE '(^|[;&|]\s*)mypy\b'; then +if echo "$scannable" | grep -qE '(^|[;&|]\s*)mypy\b'; then deny "Use \`uv run mypy ...\` instead of bare mypy." fi # Bare bandit → uv run bandit -if echo "$command" | grep -qE '(^|[;&|]\s*)bandit\b'; then +if echo "$scannable" | grep -qE '(^|[;&|]\s*)bandit\b'; then deny "Use \`uv run bandit ...\` instead of bare bandit." fi diff --git a/.claude/hooks/lib/command-text.sh b/.claude/hooks/lib/command-text.sh new file mode 100644 index 0000000..de0313f --- /dev/null +++ b/.claude/hooks/lib/command-text.sh @@ -0,0 +1,44 @@ +#!/usr/bin/env bash +# Shared helper for the PreToolUse hooks that pattern-match a Bash command. +# +# A hook receives the whole command string, which mixes code with data. A +# heredoc body carrying prose — a pull request description, a commit message, +# a generated file — can contain text like "&& ruff format --check" or +# "rm -rf / " that a naive pattern reads as an invocation, and the hook denies a +# command that never intended to run either. Stripping heredoc bodies leaves +# roughly what the shell will actually execute. +# +# Deliberate limitation: a heredoc fed to an interpreter (`bash </lib/` in any project built from this template. Anchored both to the repository root, where distutils actually writes them - **Hooks** (`session-start.sh`): `input=$(cat)` assigned the payload to a variable the script never read (shellcheck `SC2034`), caught by the new CI on its first run. Not a bug — the hook inspects the filesystem, not the payload — but the dead assignment implied otherwise. Replaced with `cat >/dev/null` and a comment noting stdin is drained so Claude Code's write to the pipe completes - **Hooks** (`protect-main.sh`): the broad `rm -rf` guard used `\b` to close each dangerous target (`/`, `.`, `..`, `~`), which does not behave as a token boundary — it matches on any adjacent word character and never matches at end-of-string. The guard let through the most common forms (`rm -rf .`, `rm -rf ..`, `rm -rf /`, `rm -rf ~`) while incorrectly blocking legitimate targets like `rm -rf .git` and `rm -rf ~/tmp-dir`. Replaced with `($|\s)` so it matches the whole token - **Settings** (`.claude/settings.json`): the `enforce-uv.sh`/`protect-main.sh` `PreToolUse` entries combined multiple patterns in one `if` string; the field holds exactly one permission rule with no `||` or list syntax, so the condition never matched and both hooks silently stopped firing — taking `protect-main.sh`'s force-push, push-to-main, `git reset --hard` and `rm -rf` guards with them. Split each pattern into its own handler entry (8 for `enforce-uv.sh`, 2 for `protect-main.sh`) diff --git a/README.md b/README.md index 3ebabc1..a7215b1 100644 --- a/README.md +++ b/README.md @@ -99,6 +99,8 @@ claude-code-python-setup/ │ │ ├── revise-claude-md.md │ │ └── test-coverage.md │ ├── hooks/ # Deterministic guardrails +│ │ ├── lib/ +│ │ │ └── command-text.sh # Strips heredoc bodies before pattern matching │ │ ├── session-start.sh # Check uv env (.venv, lockfile) at session start │ │ ├── guard-secrets.sh # Block prompts containing hardcoded secrets │ │ ├── enforce-uv.sh # Rewrite bare python/pytest to uv run, block pip @@ -306,6 +308,7 @@ Key characteristics: - **Composable**: multiple hooks can run on the same event (e.g., enforce-uv + protect-main both run on Bash) - **Conditional**: an `if` field (permission-rule syntax, e.g. `Bash(git *)`) scopes a hook to matching commands so it doesn't spawn a process on every tool call — `enforce-uv` and `protect-main` use this to skip non-Python/non-git commands - **Dependency**: requires `jq` for JSON parsing of hook input +- **Code vs. text**: a hook receives the whole command string, which mixes code with data. The two `PreToolUse` hooks strip heredoc bodies (via `hooks/lib/command-text.sh`) before matching, so a `gh pr create` whose description quotes `pip install` or `rm -rf /` isn't mistaken for running them This setup hooks into `SessionStart`, `UserPromptSubmit`, `PreToolUse`, `PostToolUse`, and `Stop`. Claude Code exposes **32 events** in total. Among the ones most useful to extend this setup: @@ -577,6 +580,7 @@ So each test runs the real script in a subprocess with a crafted payload and ass | `test_session_start_hook.py` | Missing `.venv` or a stale `uv.lock` is reported; a healthy project stays silent | | `test_verify_hook.py` | The `stop_hook_active` loop guard and the non-Python-project exit | | `test_auto_lint_hook.py` | Non-Python files, deleted files and payloads without a path are ignored | +| `test_heredoc_false_positives.py` | Tooling quoted inside a heredoc body is text, not an invocation — while code around the heredoc is still caught | Two gaps are deliberate and worth knowing: diff --git a/tests/unit/test_heredoc_false_positives.py b/tests/unit/test_heredoc_false_positives.py new file mode 100644 index 0000000..6511d6c --- /dev/null +++ b/tests/unit/test_heredoc_false_positives.py @@ -0,0 +1,90 @@ +"""Hook decisions on commands that carry prose in a heredoc body. + +A `gh pr create --body "$(cat <<'EOF' … EOF)"` ships a description that can +quote shell commands. Read as code, that text made both PreToolUse hooks deny a +command that never intended to run anything of the kind. The hooks now match +against the command with heredoc bodies stripped. +""" + +import pytest + +from tests.hook_harness import RunHook, bash_payload + +# The exact shape that denied the `gh pr create` for the documentation PR. +PR_BODY_QUOTING_TOOLING = """gh pr create --title "docs: coherence pass" --body "$(cat <<'EOF' +Run the checks locally with: + + uv run ruff check . && ruff format --check . && pytest + +The guard also blocks rm -rf / and git push --force origin main. +EOF +)" +""" + +# grep is line-based, so text at the start of a line sits in command position as +# far as the patterns are concerned. That is the shape that actually misfires. +COMMIT_MESSAGE_QUOTING_TOOLING = """git commit -F - <<'EOF' +fix: stop the guard from missing these + +Reproduce with: +pip install requests +rm -rf / --no-preserve-root +EOF +""" + +PROSE_COMMANDS = [ + ("pr body", PR_BODY_QUOTING_TOOLING), + ("commit message", COMMIT_MESSAGE_QUOTING_TOOLING), +] + + +@pytest.mark.parametrize( + ("label", "command"), PROSE_COMMANDS, ids=[label for label, _ in PROSE_COMMANDS] +) +def test_enforce_uv_ignores_tooling_named_in_a_heredoc( + run_hook: RunHook, label: str, command: str +) -> None: + result = run_hook("enforce-uv", bash_payload(command)) + + assert result.is_silent, f"{label} should pass through, hook emitted: {result.stdout!r}" + + +@pytest.mark.parametrize( + ("label", "command"), PROSE_COMMANDS, ids=[label for label, _ in PROSE_COMMANDS] +) +def test_protect_main_ignores_commands_named_in_a_heredoc( + run_hook: RunHook, label: str, command: str +) -> None: + result = run_hook("protect-main", bash_payload(command)) + + assert result.is_silent, f"{label} should pass through, hook emitted: {result.stdout!r}" + + +def test_enforce_uv_still_denies_a_command_after_a_heredoc(run_hook: RunHook) -> None: + """Stripping the body must not blind the hook to code around it.""" + command = "cat <<'EOF' > notes.txt\nsome prose\nEOF\npip install requests\n" + + result = run_hook("enforce-uv", bash_payload(command)) + + assert result.permission_decision == "deny" + + +def test_protect_main_still_denies_a_command_after_a_heredoc(run_hook: RunHook) -> None: + command = "cat <<'EOF' > notes.txt\nsome prose\nEOF\ngit push --force origin main\n" + + result = run_hook("protect-main", bash_payload(command)) + + assert result.permission_decision == "deny" + + +@pytest.mark.parametrize( + "opener", + ["< None: + command = f"cat {opener} > notes.txt\npip install requests\nEOF\n" + + result = run_hook("enforce-uv", bash_payload(command)) + + assert result.is_silent, f"{opener} body should be ignored, hook emitted: {result.stdout!r}"