Claude code hooks setup - #19
Merged
Merged
Conversation
strip_heredocs() treated a `<<` inside a quoted string as a heredoc opener, adopted a terminator that never arrived, and discarded every following line. Any command whose real invocation sat on a later line was never seen: echo "a << B" rm -rf node_modules <- allowed, silently Detection now runs on a probe copy of each line: heredoc terminators are unquoted first (blanking quotes without this eats `<<'EOF'` and breaks real heredoc stripping), then remaining quoted spans are blanked, then the opener is matched. The original line is what gets emitted. Fixing it inside strip_heredocs rather than by reordering its call site keeps it correct for callers that do not blank quotes — which guard-destructive.sh is about to stop doing. Also points the test suite at a throwaway HOOK_STATE_DIR. It rm -rf's its state dir repeatedly; against the live path that deletes the warned-PID file of any running session, silently disabling its orphan check. 55 assertions pass. Claude-Session: https://claude.ai/code/session_01FzB5J887WkGH3CPYJ3ekxN
…paths
The spec's matching policy ("a false positive costs more than a bypass") was
derived from R1/R2, where the decision is DENY and a false positive blocks
real work. R7 emits ASK, so it inverts: a false prompt costs one keystroke, a
miss costs node_modules, data/ or .env. Addresses review findings P-3 and P-5.
invokes_strict() drops quote-blanking and accepts sh -c / bash -c wrappers, so
`sh -c "rm -rf x"` is now gated. The pinned §A3 bypass stays for R1/R2 — they
still blank quotes, so the wrapper would have nothing to match against there.
Newly gated, from auditing what these actually do in this repo:
git clean - `git clean -fdx` removes data/ (9.0G), .env,
.claude/plans/, certificates/, docs/handoffs/. None of it
in git. It was ungated while `rm -rf` was gated.
`-n`/`--dry-run` stays silent.
git reset --hard, git checkout --, git restore
- destroy uncommitted work, routinely the only copy here.
Plain `git checkout <branch>` is not matched.
R7.3, the untracked-path rule: any rm naming a gitignored path asks whatever
its flags. `rm data/raid-tracker.db` needs no -r — it is a file — so the flag
rule never saw it. Resolved via `git check-ignore` rather than a hardcoded
list: of 20 gitignored entries only ~5 are regenerable, and a hand-written
list was already missing docs/handoffs/, certificates/, scripts/cleanup/ and
seeds.txt when first drafted.
Consequence worth knowing: CLAUDE.md's restore step
`rm data/raid-tracker.db-wal` now prompts. That is intended.
76 assertions pass.
Claude-Session: https://claude.ai/code/session_01FzB5J887WkGH3CPYJ3ekxN
The Stop hook decided "this session started it" by diffing against a port snapshot written at SessionStart. That baseline broke on /clear and resume: both are SessionEnd reasons, so re-entry deleted the state and every port holder then read as session debris — complete with a ready-to-run kill aimed at what may be the user's own server. Review finding P-1, reproduced: /clear snapshot [12345]->[] warned [99999]->[] Fork had no answer at all: it assigns a fresh session_id, so no keying scheme carried the parent's baseline. Replaced with a comparison that needs no state: a port holder younger than the claude process is one this session started. That deletes the baseline file, the SessionStart hook, the SessionEnd hook, the write-if-absent dance, and the mtime-lockstep touch that kept two files aging as a unit. Findings P-1, P-2 and P-4 dissolve rather than being patched. Hook surface goes from six to four. session_age() walks UP the process tree looking for `claude`. It must not shortcut to $PPID — a hook's parent is a per-invocation `/bin/sh -c` wrapper whose own age is always 0. Instrumenting a live hook showed: 10987 1464 0 /bin/sh -c ".../guard-build.sh" 1464 10 2778 claude Reading that 0 would have classified every holder as older than the session, so nothing would ever be reported and the check would have been silently dead. classify_holders() is kept pure, with the age passed in, so fail-open can be tested: an empty or non-numeric age reports nothing (R4.4). That case cannot be staged end-to-end — every process a test spawns is a descendant of the real claude, so "no ancestor" is unreachable from inside a session. Accepted, and unchanged from the snapshot design: a server the user starts mid-session in another terminal is younger than the session and gets reported. The message says "started after this session began" rather than claiming authorship. Also adds the first-ever test of the build guard's detection path, which had only ever been asserted in the negative. 87 assertions pass. Claude-Session: https://claude.ai/code/session_01FzB5J887WkGH3CPYJ3ekxN
Spec, in place rather than superseded:
R3 removed - the session-start snapshot, with why the baseline was the
wrong primitive and why R3.1's rationale was circular
R4.2 rewritten around process age; records the accepted misreport case
R4.4 same fail-open guarantee, new mechanism, and why it cannot be tested
end-to-end (every test process descends from the real claude)
R5.3 added - state dir must be overridable; the suite was deleting live state
R6.1 removed with the SessionEnd hook; R6.2's sweep relocated to Stop
R7.2 extended to git clean / reset --hard / checkout -- / restore
R7.3 added - untracked-path rule, and why check-ignore beats a fixed list
R7.4 added - R7 matches with invokes_strict()
Matching policy scoped to R1/R2, with the R7 inversion recorded as finding
P-3 rather than left implicit. Non-goals: fork gap closed, sh -c narrowed to
R1/R2 with the reason it is forced there, dispatcher merge rejected with the
measurement behind it.
Two constraints now carry evidence instead of assertion: the real build
cmdline (verified against an actual build, with the full process list), and
that a hook's $PPID is a sh -c wrapper with etimes=0, not claude. The second
is load-bearing for R4.2 and would have silently disabled the check.
decisions.md gains a 2026-08-03 entry that supersedes the two baseline
sections above it.
87 assertions pass. npm run build exits 0.
Claude-Session: https://claude.ai/code/session_01FzB5J887WkGH3CPYJ3ekxN
…ming Both axes of the post-implementation review independently found the same severity-1 bug, introduced in be5bda9 and reproduced by hand before fixing: git clean -fdx -> ask git clean -fdx && head -n 20 file -> ALLOWED (wrong) grep -n foo x && git clean -fdx -> ALLOWED (wrong) The dry-run exemption grepped the WHOLE command for any -flag containing an 'n', then exited the hook. So an unrelated head -n / sort -n / grep -n anywhere un-gated a real destructive clean, and because it exited rather than skipping one rule, it also disabled the rm -rf and untracked-path rules for that call — precisely the "a miss costs data/" case the strictness decision was made to avoid. Everything the file does to be command-position-aware, this one line threw away. git clean now has its own predicate, evaluated per clause: a clean is exempt only when the clause invoking it carries the dry-run flag itself. Pinned by regression tests at both the predicate and hook level. Recorded as R7.2b. Also from the review: - HOOK_DESTRUCTIVE moved into lib.sh. The tests held two copies of the pattern and one had already drifted to a narrower version than production used, so the C2 assertions were testing a pattern nothing ran. Same reasoning already applied to HOOK_WRAPPERS. - classify_holders -> holders_younger_than. It filters rather than classifies; the name should say what comes back. - rm_touches_ignored uses `local -` so globbing is restored on any exit path rather than being force-enabled. - One EXIT trap via a cleanup(), not two where the second silently replaced the first. - Tests restore DEV_PORT=3000 instead of unsetting it; lib.sh resolves it at source time, so unset left an empty port for anything running afterwards. - Stale spec heading and function name corrected. 95 assertions pass. Dry runs stay silent; no new noise on head -n / grep -n. Claude-Session: https://claude.ai/code/session_01FzB5J887WkGH3CPYJ3ekxN
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.