diff --git a/hooks/hooks_test.sh b/hooks/hooks_test.sh index 01f2444..fd1662a 100644 --- a/hooks/hooks_test.sh +++ b/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 }"}}' \ diff --git a/hooks/post-validate.sh b/hooks/post-validate.sh index 1f256aa..f513744 100755 --- a/hooks/post-validate.sh +++ 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: { diff --git a/hooks/rtk-rewrite.sh b/hooks/rtk-rewrite.sh index f105932..4052118 100755 --- a/hooks/rtk-rewrite.sh +++ b/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", diff --git a/hooks/security-patterns.sh b/hooks/security-patterns.sh index b781cf0..76c1f3a 100755 --- a/hooks/security-patterns.sh +++ b/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"