diff --git a/hooks/hooks_test.sh b/hooks/hooks_test.sh index fd1662a..08db33f 100644 --- a/hooks/hooks_test.sh +++ b/hooks/hooks_test.sh @@ -45,7 +45,11 @@ fail() { FAIL=$((FAIL + 1)); ERRORS="$ERRORS\n FAIL: $1"; echo " FAIL: $1"; } run_hook() { local script="$1" input="$2" label="$3" expect_json="${4:-false}" - OUTPUT=$(echo "$input" | bash "$HOOK_DIR/$script" 2>/dev/null) || true + # Do NOT add `|| true` here: it would clobber $? with true's exit code + # and the EXIT check below would become dead. This suite runs under + # `set -uo pipefail` (no -e), so a non-zero exit in the command + # substitution does not abort the script — $? captures it faithfully. + OUTPUT=$(echo "$input" | bash "$HOOK_DIR/$script" 2>/dev/null) EXIT=$? # Hook must exit 0 @@ -125,6 +129,7 @@ case "$RTK_SHIM_MODE" in unknown) printf 'rtk %s' "$*"; exit 42 ;; empty-allow) exit 0 ;; noop-rewrite) printf '%s' "$*"; exit 0 ;; # rewrite equals input + ask-noop-rewrite) printf '%s' "$*"; exit 3 ;; # rc 3 (ask) + rewrite equals input *) exit 1 ;; esac SHIM @@ -192,6 +197,16 @@ else fail "rtk-rewrite: identity rewrite should no-op (got: $out)" fi +# rc 3 (ask) + rewrite == input → no-op. Exercises the intersection of +# the ask-suppression branch and the identity-rewrite guard; neither +# should emit a rewrite when the result would be a self-assignment. +out=$(rtk_shim_run ask-noop-rewrite '{"tool_input":{"command":"foo bar","description":""}}') +if [[ -z "$out" ]]; then + pass "rtk-rewrite: rc 3 + rewrite == input → no-op" +else + fail "rtk-rewrite: rc 3 identity rewrite should no-op (got: $out)" +fi + # Empty command → no-op out=$(rtk_shim_run allow '{"tool_input":{"command":"","description":""}}') if [[ -z "$out" ]]; then @@ -237,10 +252,9 @@ else fi # security-patterns.sh — dedup must survive across hook invocations. -# This pins the $$→$PPID fix: $$ is the hook's own bash PID, which is -# brand new every invocation, so dedup was silently disabled and the -# same warning fired on every save. In production Claude Code spawns -# the hook directly via fork+exec, so $PPID is the stable CC session PID. +# Invariant: a repeated (file, pattern) pair warns only on the first call +# within a CC session. Implementation detail: dedup keys off $PPID so the +# key is stable across fork+exec invocations of the hook. # # Topology gotcha: bash's $() command substitution wraps the command in # an intermediate subshell (a real fork), which then becomes the hook's @@ -263,6 +277,18 @@ else fail "security-patterns: dedup not working (first=$sec_first second=$sec_second)" fi +# Per-pattern dedup: a different pattern on the SAME file must still +# warn. Regression guard against a key collapse that flattens the dedup +# key to a global "warn-once-ever" flag. +SEC_PAYLOAD_EVAL='{"tool_name":"Write","tool_input":{"file_path":"dedup_test.py","content":"eval(user_input)"}}' +TMPDIR="$sec_tmpdir" bash "$HOOK_DIR/security-patterns.sh" <<< "$SEC_PAYLOAD_EVAL" \ + > "$sec_tmpdir/out3" 2>/dev/null || true +if [[ -s "$sec_tmpdir/out3" ]]; then + pass "security-patterns: dedup is per-pattern (different pattern, same file, still warns)" +else + fail "security-patterns: per-pattern dedup flattened to warn-once-ever" +fi + echo "" echo "=== PostToolUse Hooks ===" @@ -275,11 +301,8 @@ run_hook "post-validate.sh" \ run_hook "post-validate.sh" "" "post-validate: empty input" false # post-validate.sh — relative path inside the repo must NOT trigger the -# "outside repo" warning. Pins the realpath -m GNU-ism fix: on macOS the -# old implementation's fallback kept $FILE_PATH unresolved, so a benign -# "src/foo.go" failed the "$REPO_ROOT"/* match and generated a false -# warning on every Edit/Write. Run the hook from the repo root with a -# plain relative path and assert no warning is emitted. +# "outside repo" warning. Invariant: in-repo relative paths resolve +# against $(pwd) and match $REPO_ROOT portably on macOS and Linux. out=$(cd "$REPO_ROOT" && printf '%s' \ '{"tool_name":"Write","tool_input":{"file_path":"src/fake.go","content":"package main"}}' \ | bash "$HOOK_DIR/post-validate.sh" 2>/dev/null || true) @@ -299,6 +322,64 @@ else fail "post-validate: absolute out-of-repo path not flagged (got: $out)" fi +# post-validate.sh — cwd is a subdirectory of the repo, FILE_PATH uses +# "../foo" to reach a sibling that's still inside the repo. This is the +# common Claude Code topology (cwd = a subdir, paths written relative +# to it). The path must normalize back into REPO_ROOT, not warn. +out=$(cd "$REPO_ROOT/hooks" && printf '%s' \ + '{"tool_name":"Write","tool_input":{"file_path":"../README.md","content":"x"}}' \ + | bash "$HOOK_DIR/post-validate.sh" 2>/dev/null || true) +if [[ -z "$out" ]]; then + pass "post-validate: cwd=subdir + ../in-repo path → no warning" +else + fail "post-validate: subdir relative path wrongly flagged (got: $out)" +fi + +# post-validate.sh — .. escape from repo root. The resolved absolute +# path is OUTSIDE the repo and must warn. Before the path-normalization +# fix, "$REPO_ROOT/../sibling.go" matched the "$REPO_ROOT"/* glob and +# silently passed. +out=$(cd "$REPO_ROOT" && printf '%s' \ + '{"tool_name":"Write","tool_input":{"file_path":"../devkit-sibling-outside/foo.go","content":"x"}}' \ + | bash "$HOOK_DIR/post-validate.sh" 2>/dev/null || true) +if printf '%s' "$out" | jq -e '.hookSpecificOutput.additionalContext' >/dev/null 2>&1; then + pass "post-validate: .. escape from repo root → warn" +else + fail "post-validate: .. escape not flagged (got: $out)" +fi + +# post-validate.sh — macOS TMPDIR allowlist. /var/folders/... is the +# BSD TMPDIR and must not warn. Also confirms the /tmp allowlist. +out=$(printf '%s' \ + '{"tool_name":"Write","tool_input":{"file_path":"/var/folders/xx/foo.go","content":"package main"}}' \ + | bash "$HOOK_DIR/post-validate.sh" 2>/dev/null || true) +if [[ -z "$out" ]]; then + pass "post-validate: /var/folders (macOS TMPDIR) → no warning" +else + fail "post-validate: TMPDIR path wrongly flagged (got: $out)" +fi + +# post-validate.sh — symlinked repo root. When the user's cwd is +# reached via a symlink, REPO_ROOT (from git rev-parse) and $(pwd) can +# have different prefixes. The hook must normalize both sides so +# in-repo writes via the symlink path don't get mis-classified. +# Skipped on platforms where `ln -s` doesn't create a real symlink +# (Windows Git Bash without developer mode, some FUSE mounts, etc.). +symlink_tmp=$(mktemp -d) +GUARD_TMPS+=("$symlink_tmp") +if ln -s "$REPO_ROOT" "$symlink_tmp/link" 2>/dev/null && [ -L "$symlink_tmp/link" ]; then + out=$(cd "$symlink_tmp/link" && printf '%s' \ + '{"tool_name":"Write","tool_input":{"file_path":"README.md","content":"x"}}' \ + | bash "$HOOK_DIR/post-validate.sh" 2>/dev/null || true) + if [[ -z "$out" ]]; then + pass "post-validate: cwd via symlink to repo → no warning" + else + fail "post-validate: symlinked repo root wrongly flagged (got: $out)" + fi +else + echo " SKIP: post-validate: cwd via symlink (ln -s unavailable on this platform)" +fi + # slop-detect.sh — should allow clean code run_hook "slop-detect.sh" \ '{"tool_name":"Write","tool_input":{"file_path":"test.go","content":"func Add(a, b int) int { return a + b }"}}' \ diff --git a/hooks/post-validate.sh b/hooks/post-validate.sh index f513744..5476a0e 100755 --- a/hooks/post-validate.sh +++ b/hooks/post-validate.sh @@ -50,25 +50,69 @@ if [ "$TOOL_NAME" = "Edit" ] || [ "$TOOL_NAME" = "Write" ]; then fi fi - # Check for writes outside the git repo. - # realpath -m is GNU-only (not on macOS BSD realpath), so the previous - # implementation silently fell back to the raw FILE_PATH and mis-classified - # every relative path on macOS as "outside repo." Do a portable absolute-path - # conversion instead: absolute inputs stay as-is, relative inputs are - # prefixed with pwd. We don't normalize "." / ".." — the glob match still - # works for in-repo relative paths, and ..-escapes will (correctly) not match. + # Check for writes outside the git repo. Must resolve .. segments and + # symlinks so REPO_ROOT and ABS_PATH compare against a common canonical + # form — GNU `realpath -m` isn't available on macOS BSD realpath, so we + # do it portably here. if [ -n "$FILE_PATH" ]; then REPO_ROOT=$(git rev-parse --show-toplevel 2>/dev/null || true) if [ -n "$REPO_ROOT" ]; then + # Canonicalize REPO_ROOT so a symlinked repo path (e.g. /Users/x/dev + # → /Volumes/Work/dev) matches $(pwd -P) inside the repo. + REPO_ROOT=$(cd "$REPO_ROOT" 2>/dev/null && pwd -P) || REPO_ROOT="" + fi + if [ -n "$REPO_ROOT" ]; then + # Absolute-path detection handles POSIX (/foo) and Windows + # drive-letter (C:\foo or C:/foo) forms. On Git Bash / WSL the + # paths are usually MSYS-style (/c/Users/...) which already + # matches the /* arm, but Claude Code may also pass native + # Windows paths depending on how the tool_input was produced. case "$FILE_PATH" in - /*) ABS_PATH="$FILE_PATH" ;; + /*|[A-Za-z]:[/\\]*) ABS_PATH="$FILE_PATH" ;; *) ABS_PATH="$(pwd)/$FILE_PATH" ;; esac + # Normalize the absolute path: resolve .. / . and symlinks. Prefer + # python3, fall back to python (Windows often ships just `python`), + # then to a dirname+pwd trick which works whenever the parent + # directory exists (the common case for Write/Edit since the + # parent must already exist). + NORMALIZED="" + if command -v python3 >/dev/null 2>&1; then + NORMALIZED=$(python3 -c 'import os,sys; print(os.path.realpath(sys.argv[1]))' "$ABS_PATH" 2>/dev/null || true) + elif command -v python >/dev/null 2>&1; then + NORMALIZED=$(python -c 'import os,sys; print(os.path.realpath(sys.argv[1]))' "$ABS_PATH" 2>/dev/null || true) + fi + if [ -z "$NORMALIZED" ]; then + _dir=$(dirname -- "$ABS_PATH") + _base=$(basename -- "$ABS_PATH") + if [ -d "$_dir" ]; then + NORMALIZED="$(cd -- "$_dir" && pwd -P)/$_base" + fi + fi + if [ -z "$NORMALIZED" ]; then + # Final fallback: collapse ./ and ../ segments with sed. Won't + # resolve symlinks, but preserves the outside-repo detection + # when neither python nor the parent directory is available + # (pre-mkdir writes, minimal Git Bash installs, etc.). + NORMALIZED="$ABS_PATH" + while : ; do + _prev="$NORMALIZED" + NORMALIZED=$(printf '%s' "$NORMALIZED" | sed -E \ + -e 's|/\./|/|g' \ + -e 's|/[^/]+/\.\./|/|g' \ + -e 's|/[^/]+/\.\.$||' \ + -e 's|/\.$||') + [ "$NORMALIZED" = "$_prev" ] && break + done + fi + [ -n "$NORMALIZED" ] && ABS_PATH="$NORMALIZED" case "$ABS_PATH" in - "$REPO_ROOT"/*) + "$REPO_ROOT"/*|"$REPO_ROOT") ;; # within repo, OK - /tmp/*|/private/tmp/*|/var/folders/*) - ;; # temp files (/var/folders/* is macOS TMPDIR), OK + /tmp/*|/private/tmp/*|/var/folders/*|/private/var/folders/*) + ;; # temp files (/var/folders is macOS TMPDIR; /private/var/folders + # is its realpath-resolved form since /var is a symlink to + # /private/var on macOS), OK *) jq -n --arg file "$FILE_PATH" --arg repo "$REPO_ROOT" '{ hookSpecificOutput: { diff --git a/hooks/security-patterns.sh b/hooks/security-patterns.sh index 76c1f3a..4d23304 100755 --- a/hooks/security-patterns.sh +++ b/hooks/security-patterns.sh @@ -5,10 +5,16 @@ # Catches security anti-patterns at the moment of creation rather than in a later review. # Warns once per file+pattern per session to avoid spam. +set -euo pipefail + +# jq failures on malformed input must not abort the hook — the hook's +# contract is "fail open" (never block Claude Code on parse errors), +# so every jq pipeline gets a `|| true` tolerance guard, matching the +# pattern used in rtk-rewrite.sh. INPUT=$(cat) -TOOL_NAME=$(echo "$INPUT" | jq -r '.tool_name // empty') -FILE_PATH=$(echo "$INPUT" | jq -r '.tool_input.file_path // empty') -NEW_STRING=$(echo "$INPUT" | jq -r '.tool_input.new_string // .tool_input.content // empty') +TOOL_NAME=$(echo "$INPUT" | jq -r '.tool_name // empty' 2>/dev/null || true) +FILE_PATH=$(echo "$INPUT" | jq -r '.tool_input.file_path // empty' 2>/dev/null || true) +NEW_STRING=$(echo "$INPUT" | jq -r '.tool_input.new_string // .tool_input.content // empty' 2>/dev/null || true) # Only check Edit and Write [ "$TOOL_NAME" = "Edit" ] || [ "$TOOL_NAME" = "Write" ] || exit 0 @@ -32,7 +38,9 @@ check_pattern() { if [ -f "$SEEN_FILE" ] && grep -qF "$key" "$SEEN_FILE" 2>/dev/null; then return fi - echo "$key" >> "$SEEN_FILE" 2>/dev/null + # Under set -e, a failed write would abort the hook and turn a + # would-be "ask" warning into a hard exit. Tolerate failures. + echo "$key" >> "$SEEN_FILE" 2>/dev/null || true jq -n --arg reason "$message" '{ hookSpecificOutput: {