fix(hooks): three silent-failure bugs found during hook audit

hooks/hooks_test.sh

@@ -97,6 +97,126 @@ run_hook "rtk-rewrite.sh" \
   '{"tool_name":"Bash","tool_input":{"command":"go test ./..."}}' \
   "rtk-rewrite: pass through" false
 
+# rtk-rewrite.sh — exit-code protocol coverage via a PATH shim.
+# Contract:
+#   rtk rc 0 → devkit decision "allow" + rewrite
+#   rtk rc 3 → devkit decision "allow" + rewrite   (devkit suppresses rtk's ask;
+#                                                   safety-check.sh already
+#                                                   guards destructive ops earlier
+#                                                   in the PreToolUse chain)
+#   rtk rc 1 → pass through (no rewrite available)
+#   rtk rc 2 → pass through (rtk deny — CC's native safety handles it)
+#   rtk other → pass through (fail-open)
+#   malformed JSON stdin → no-op without crashing the hook
+# The shim lets us test each branch deterministically without depending on
+# the real rtk version's rule set.
+rtk_shim_dir=$(mktemp -d)
+GUARD_TMPS+=("$rtk_shim_dir")
+cat > "$rtk_shim_dir/rtk" <<'SHIM'
+#!/bin/sh
+# Fake rtk: $RTK_SHIM_MODE selects the exit code + stdout behavior.
+[ "$1" = "rewrite" ] || exit 0
+shift
+case "$RTK_SHIM_MODE" in
+  allow)       printf 'rtk %s' "$*"; exit 0 ;;
+  ask)         printf 'rtk %s' "$*"; exit 3 ;;
+  no-equiv)    exit 1 ;;
+  deny)        exit 2 ;;
+  unknown)     printf 'rtk %s' "$*"; exit 42 ;;
+  empty-allow) exit 0 ;;
+  noop-rewrite) printf '%s' "$*"; exit 0 ;;  # rewrite equals input
+  *)           exit 1 ;;
+esac
+SHIM
+chmod +x "$rtk_shim_dir/rtk"
+
+rtk_shim_run() {
+  local mode="$1" input="$2"
+  PATH="$rtk_shim_dir:$PATH" RTK_SHIM_MODE="$mode" \
+    bash "$HOOK_DIR/rtk-rewrite.sh" <<< "$input" 2>/dev/null || true
+}
+
+# exit 0 → permissionDecision=allow + rewrite
+out=$(rtk_shim_run allow '{"tool_input":{"command":"ls -la","description":"list"}}')
+if printf '%s' "$out" | jq -e '.hookSpecificOutput.permissionDecision=="allow" and .hookSpecificOutput.updatedInput.command=="rtk ls -la"' >/dev/null 2>&1; then
+  pass "rtk-rewrite: exit 0 → allow + rewrite"
+else
+  fail "rtk-rewrite: exit 0 (got: $out)"
+fi
+
+# exit 3 → STILL permissionDecision=allow + rewrite (devkit suppresses rtk's ask)
+out=$(rtk_shim_run ask '{"tool_input":{"command":"git status","description":"status"}}')
+if printf '%s' "$out" | jq -e '.hookSpecificOutput.permissionDecision=="allow" and .hookSpecificOutput.updatedInput.command=="rtk git status"' >/dev/null 2>&1; then
+  pass "rtk-rewrite: exit 3 → allow + rewrite (ask suppressed)"
+else
+  fail "rtk-rewrite: exit 3 (got: $out)"
+fi
+
+# exit 1 → silent pass-through
+out=$(rtk_shim_run no-equiv '{"tool_input":{"command":"nothing","description":""}}')
+if [[ -z "$out" ]]; then
+  pass "rtk-rewrite: exit 1 → pass through"
+else
+  fail "rtk-rewrite: exit 1 should no-op (got: $out)"
+fi
+
+# exit 2 → silent pass-through (CC handles deny natively)
+out=$(rtk_shim_run deny '{"tool_input":{"command":"rm -rf /","description":""}}')
+if [[ -z "$out" ]]; then
+  pass "rtk-rewrite: exit 2 → pass through"
+else
+  fail "rtk-rewrite: exit 2 should no-op (got: $out)"
+fi
+
+# Unknown exit code → fail-open pass-through
+out=$(rtk_shim_run unknown '{"tool_input":{"command":"weird","description":""}}')
+if [[ -z "$out" ]]; then
+  pass "rtk-rewrite: unknown exit code → pass through (fail-open)"
+else
+  fail "rtk-rewrite: unknown exit should no-op (got: $out)"
+fi
+
+# exit 0 but empty stdout → no-op (pins the [[ -z $REWRITTEN ]] guard)
+out=$(rtk_shim_run empty-allow '{"tool_input":{"command":"anything","description":""}}')
+if [[ -z "$out" ]]; then
+  pass "rtk-rewrite: exit 0 with empty stdout → no-op"
+else
+  fail "rtk-rewrite: empty-rewrite should no-op (got: $out)"
+fi
+
+# rewrite == input → no-op (pins the command-equality guard)
+out=$(rtk_shim_run noop-rewrite '{"tool_input":{"command":"foo bar","description":""}}')
+if [[ -z "$out" ]]; then
+  pass "rtk-rewrite: rewrite == input → no-op"
+else
+  fail "rtk-rewrite: 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
+  pass "rtk-rewrite: empty command → no-op"
+else
+  fail "rtk-rewrite: empty command should no-op (got: $out)"
+fi
+
+# Malformed JSON stdin → hook must exit 0 cleanly (set -e regression test).
+# Before the `|| true` guard in rtk-rewrite.sh, `jq -r` on malformed input +
+# set -e + pipefail caused the whole hook to exit non-zero, which CC treats
+# as a blocking error. We call the hook directly (not under $()) so rc is
+# preserved, then assert rc=0 and empty stdout. Note: this suite runs under
+# `set -uo pipefail` (no -e), so we don't toggle -e here.
+PATH="$rtk_shim_dir:$PATH" RTK_SHIM_MODE=allow \
+  bash "$HOOK_DIR/rtk-rewrite.sh" > /tmp/rtk-malformed.out 2>/dev/null <<< 'not valid json {{{'
+rc=$?
+out=$(cat /tmp/rtk-malformed.out)
+rm -f /tmp/rtk-malformed.out
+if [[ "$rc" -eq 0 ]] && [[ -z "$out" ]]; then
+  pass "rtk-rewrite: malformed JSON → exit 0 silent no-op"
+else
+  fail "rtk-rewrite: malformed JSON should exit 0 silently (rc=$rc out=$out)"
+fi
+
 # pr-gate.sh — should allow non-push commands
 run_hook "pr-gate.sh" \
   '{"tool_name":"Bash","tool_input":{"command":"git status"}}' \
@@ -116,6 +236,33 @@ else
   pass "security-patterns: exits cleanly (pattern matching is format-sensitive)"
 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.
+#
+# Topology gotcha: bash's $() command substitution wraps the command in
+# an intermediate subshell (a real fork), which then becomes the hook's
+# parent — so $PPID would change per call under $() capture and this
+# test would spuriously fail. We redirect output to files instead, so
+# each hook invocation is a direct child of the test script and
+# $PPID == $$ of this script (matching production topology).
+sec_tmpdir=$(mktemp -d)
+GUARD_TMPS+=("$sec_tmpdir")
+SEC_PAYLOAD='{"tool_name":"Write","tool_input":{"file_path":"dedup_test.py","content":"import pickle; pickle.load(f)"}}'
+TMPDIR="$sec_tmpdir" bash "$HOOK_DIR/security-patterns.sh" <<< "$SEC_PAYLOAD" \
+  > "$sec_tmpdir/out1" 2>/dev/null || true
+TMPDIR="$sec_tmpdir" bash "$HOOK_DIR/security-patterns.sh" <<< "$SEC_PAYLOAD" \
+  > "$sec_tmpdir/out2" 2>/dev/null || true
+if [[ -s "$sec_tmpdir/out1" ]] && [[ ! -s "$sec_tmpdir/out2" ]]; then
+  pass "security-patterns: dedup suppresses repeat warning (PPID fix)"
+else
+  sec_first=$(cat "$sec_tmpdir/out1" 2>/dev/null || true)
+  sec_second=$(cat "$sec_tmpdir/out2" 2>/dev/null || true)
+  fail "security-patterns: dedup not working (first=$sec_first second=$sec_second)"
+fi
+
 echo ""
 echo "=== PostToolUse Hooks ==="
 
@@ -127,6 +274,31 @@ run_hook "post-validate.sh" \
 # post-validate.sh — empty input
 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.
+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)
+if [[ -z "$out" ]]; then
+  pass "post-validate: relative path inside repo → no warning (macOS realpath fix)"
+else
+  fail "post-validate: relative in-repo path wrongly flagged (got: $out)"
+fi
+
+# post-validate.sh — absolute path outside repo SHOULD trigger warning.
+out=$(printf '%s' \
+  '{"tool_name":"Write","tool_input":{"file_path":"/opt/nonexistent/foo.go","content":"package main"}}' \
+  | 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: absolute path outside repo → warn"
+else
+  fail "post-validate: absolute out-of-repo path not flagged (got: $out)"
+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,16 +50,25 @@ if [ "$TOOL_NAME" = "Edit" ] || [ "$TOOL_NAME" = "Write" ]; then
     fi
   fi
 
-  # Check for writes outside the git repo
+  # 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.
   if [ -n "$FILE_PATH" ]; then
     REPO_ROOT=$(git rev-parse --show-toplevel 2>/dev/null || true)
     if [ -n "$REPO_ROOT" ]; then
-      ABS_PATH=$(realpath -m "$FILE_PATH" 2>/dev/null || echo "$FILE_PATH")
+      case "$FILE_PATH" in
+        /*) ABS_PATH="$FILE_PATH" ;;
+        *)  ABS_PATH="$(pwd)/$FILE_PATH" ;;
+      esac
       case "$ABS_PATH" in
         "$REPO_ROOT"/*)
           ;; # within repo, OK
-        /tmp/*|/private/tmp/*)
-          ;; # temp files, OK
+        /tmp/*|/private/tmp/*|/var/folders/*)
+          ;; # temp files (/var/folders/* is macOS TMPDIR), OK
         *)
           jq -n --arg file "$FILE_PATH" --arg repo "$REPO_ROOT" '{
             hookSpecificOutput: {

hooks/rtk-rewrite.sh

@@ -1,26 +1,45 @@
 #!/bin/bash
-# devkit RTK hook — rewrites Bash commands through rtk for token savings
+# devkit RTK hook — rewrites Bash commands through rtk for token savings.
 # Runs on PreToolUse for Bash tool. No-op if rtk is not installed.
+#
+# rtk rewrite exit-code protocol (documented in the rtk binary strings,
+# but NOT in `rtk rewrite --help` which still shows the old contract):
+#   0 + stdout   rewrite found, no deny/ask rule matched
+#   1            no RTK equivalent
+#   2            deny rule matched (CC's native safety check handles it)
+#   3 + stdout   ask rule matched
+#
+# Devkit policy: both 0 and 3 auto-apply the rewrite. rtk's "ask" rules
+# are noisy on common commands (ls, git, find) and devkit's safety-check.sh
+# already fires earlier in the Bash PreToolUse chain for destructive ops,
+# so a second per-command prompt from rtk is redundant.
+set -euo pipefail
 
-# Skip if rtk is not installed
 command -v rtk >/dev/null 2>&1 || exit 0
 
 INPUT=$(cat)
-COMMAND=$(echo "$INPUT" | jq -r '.tool_input.command // empty')
+# jq failures on malformed stdin must not crash the hook — under set -e
+# the pipeline would exit non-zero and CC would treat that as a blocking
+# error. `|| true` inside the substitution degrades cleanly to empty.
+COMMAND=$(printf '%s' "$INPUT" | jq -r '.tool_input.command // empty' 2>/dev/null || true)
 
-# Skip empty commands
-[ -z "$COMMAND" ] && exit 0
+[[ -z "$COMMAND" ]] && exit 0
 
-# Try to rewrite through rtk
-REWRITTEN=$(rtk rewrite "$COMMAND" 2>/dev/null) || exit 0
+set +e
+REWRITTEN=$(rtk rewrite "$COMMAND" 2>/dev/null)
+RC=$?
+set -e
 
-# If rewrite produced the same command, skip
-[ "$REWRITTEN" = "$COMMAND" ] && exit 0
+case "$RC" in
+  0|3) : ;;       # rewrite available (0) or available-but-ask (3) → apply it
+  *)   exit 0 ;;  # 1 (no equivalent), 2 (deny handled natively), or unknown → pass through
+esac
 
-# Preserve original description if present
-DESCRIPTION=$(echo "$INPUT" | jq -r '.tool_input.description // empty')
+[[ -z "$REWRITTEN" ]] && exit 0
+[[ "$REWRITTEN" == "$COMMAND" ]] && exit 0
+
+DESCRIPTION=$(printf '%s' "$INPUT" | jq -r '.tool_input.description // empty' 2>/dev/null || true)
 
-# Output the rewrite
 jq -n --arg cmd "$REWRITTEN" --arg desc "$DESCRIPTION" '{
   hookSpecificOutput: {
     hookEventName: "PreToolUse",

hooks/security-patterns.sh

@@ -14,8 +14,13 @@ NEW_STRING=$(echo "$INPUT" | jq -r '.tool_input.new_string // .tool_input.conten
 [ "$TOOL_NAME" = "Edit" ] || [ "$TOOL_NAME" = "Write" ] || exit 0
 [ -z "$NEW_STRING" ] && exit 0
 
-# Session dedup — warn once per file+pattern
-SEEN_FILE="/tmp/devkit-security-seen-$$"
+# Session dedup — warn once per file+pattern per session.
+# $$ is the hook's own bash PID, which is brand new for every invocation
+# (Claude Code spawns a fresh bash per hook call), so using it silently
+# disabled dedup: every call wrote to a unique file and never found a
+# prior warning. $PPID is the parent — the Claude Code hook runner — which
+# is stable for the entire session, matching the "per session" intent.
+SEEN_FILE="${TMPDIR:-/tmp}/devkit-security-seen-${PPID}"
 
 check_pattern() {
   local pattern="$1"