fix(hooks): resolve #73 #74 #75 follow-ups (test harness, path handling, set -euo)

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 }"}}' \

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: {

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: {