From 5653ec40e0a1db64bc83206e8445cce996b0ffbd Mon Sep 17 00:00:00 2001 From: Ivan Pasichnyk Date: Fri, 1 May 2026 22:14:56 -0700 Subject: [PATCH] fix(hook): secret redaction was broken; rewrite via Python + add tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PostToolUse hook claimed to redact bearer tokens / api keys / passwords from observation summaries before writing them to disk. A code review found the redaction was broken in three concrete ways: 1. The keyword=value sed pattern used `\047` to mean a single quote inside a POSIX bracket expression. Both BSD sed (macOS) and GNU sed treat `\047` as literal characters there, so `api_key="..."` and `password="..."` patterns never matched. Verified empirically: `api_key="abc12345xyz_secretvalue"` came through unchanged. 2. The Bearer pattern's character class (`[A-Za-z0-9_\.\-\/+=]+`) had unnecessary backslash escapes that fouled the match. Result: `Bearer sk-ant-api03-realsecret` was redacted as `Bearer [REDACTED]-ant-api03-realsecret` — the `sk-` got eaten, the rest of the API key leaked into observations on disk. This is the actual Anthropic key prefix and the most realistic case. 3. Inline-env-var assignments (`MY_TOKEN=abc curl ...`) were never caught — only `export TOKEN=...` form. Common shell pattern, fully bypassed redaction. Plus secondary issues: 4. The `ls*` skip-list pattern matched `lsof`, `lsblk`, `lsattr` and other non-read-only commands, dropping their observations. 5. The Bash branch only redacted SUMMARY, not COMMAND. The full command also gets persisted into context.command in the JSONL. Fix: - `redact_secrets()` helper rewritten in Python via `python3 -c` (sed character classes diverge between BSD and GNU; Python regex is portable and verified). Three rules: a. inline ENV=val with secret-implying variable name → REDACTED b. keyword (token/password/api_key/secret/credential/auth) = value (4+ non-space/quote chars) → REDACTED c. Bearer X, sk-..., ghp_..., AKIA... → REDACTED - Read-only skip patterns tightened: `ls` and `ls *` instead of `ls*`, same for `pwd`. Adds `grep` and `rg` to the skip list. - Bash branch now redacts COMMAND before deriving SUMMARY, so both on-disk fields are clean. - Drops the brittle `if grep ... TOKEN|SECRET` heuristic — replaced by the rule (a) regex, which is more general and tested. Adds tests/test_post_tool_use_hook.sh — 19 assertions covering all six redaction cases that previously failed, the read-only skip list (including the lsof/lsblk regression), and sensitive-file-path skips on Write. Passes 19/19 against the new hook; would fail 6+ on the prior version. Performance: each tool call now invokes python3 once or twice. ~50ms overhead, acceptable for a hook that fires on Edit/Write/Bash. The hook is still non-blocking and fast-exits on read-only commands before reaching Python. --- openexp/hooks/post-tool-use.sh | 53 +++++++++++--- tests/test_post_tool_use_hook.sh | 121 +++++++++++++++++++++++++++++++ 2 files changed, 164 insertions(+), 10 deletions(-) create mode 100755 tests/test_post_tool_use_hook.sh diff --git a/openexp/hooks/post-tool-use.sh b/openexp/hooks/post-tool-use.sh index 0e1f92f..ba599ef 100755 --- a/openexp/hooks/post-tool-use.sh +++ b/openexp/hooks/post-tool-use.sh @@ -22,6 +22,41 @@ case "$TOOL_NAME" in *) exit 0 ;; esac +# redact_secrets: read text on stdin, write redacted text to stdout. +# Uses Python for portable, correct regex (sed character classes diverge +# between BSD and GNU and were broken in earlier versions of this hook). +redact_secrets() { + python3 -c ' +import re, sys +s = sys.stdin.read() + +# 1. Inline env-var assignments where the variable name implies a secret. +# Catches: ANTHROPIC_API_KEY=sk-ant-..., MY_TOKEN=abc, GH_PASSWORD=... +s = re.sub( + r"(^|\s)([A-Z][A-Z0-9_]*(?:TOKEN|SECRET|KEY|PASSWORD|API|PASS|PWD|AUTH)[A-Z0-9_]*)\s*=\s*\S+", + r"\1\2=[REDACTED]", + s, +) + +# 2. keyword=value or keyword: value or keyword="value" forms in prose. +# Case-insensitive so "API_KEY", "api_key", "Api-Key" all match. +s = re.sub( + r"(token|password|api[_-]?key|secret|credential|auth)\s*[:=]\s*[\"\x27]?[^\s\"\x27]{4,}[\"\x27]?", + lambda m: m.group(1) + "=[REDACTED]", + s, + flags=re.IGNORECASE, +) + +# 3. Bearer / token-prefixed values (sk-ant-..., sk-..., ghp_..., AKIA...). +s = re.sub(r"Bearer\s+[A-Za-z0-9._/+=\-]+", "Bearer [REDACTED]", s) +s = re.sub(r"\bsk-[A-Za-z0-9_\-]{16,}", "[REDACTED]", s) +s = re.sub(r"\bghp_[A-Za-z0-9]{20,}", "[REDACTED]", s) +s = re.sub(r"\bAKIA[A-Z0-9]{16}\b", "[REDACTED]", s) + +sys.stdout.write(s) +' +} + FILE_PATH="" COMMAND="" SUMMARY="" @@ -47,23 +82,21 @@ case "$TOOL_NAME" in COMMAND=$(echo "$INPUT" | jq -r '.tool_input.command // empty') [ -z "$COMMAND" ] && exit 0 BASE_CMD=$(echo "$COMMAND" | sed 's|^/[^ ]*/||') + # Skip read-only commands. Patterns require either end-of-string or a + # space after the command name to avoid matching lsof/lsblk/catfish/etc. case "$BASE_CMD" in - ls*|cat\ *|pwd*|echo\ *|head\ *|tail\ *|wc\ *|which\ *|type\ *|cd\ *|test\ *|\[\ *|find\ *) exit 0 ;; + ls|ls\ *|cat\ *|pwd|pwd\ *|echo\ *|head\ *|tail\ *|wc\ *|which\ *|type\ *|cd\ *|test\ *|\[\ *|find\ *|grep\ *|rg\ *) exit 0 ;; esac - if echo "$COMMAND" | grep -qiE '(export.*TOKEN|export.*SECRET|export.*KEY|export.*PASSWORD)'; then - SUMMARY="Ran: [env variable setup - REDACTED]" - COMMAND="" - else - SUMMARY="Ran: ${COMMAND:0:300}" - fi + # Redact the command BEFORE deriving SUMMARY or CONTEXT — both end up on disk. + COMMAND=$(printf '%s' "$COMMAND" | redact_secrets) + SUMMARY="Ran: ${COMMAND:0:300}" ;; esac [ -z "$SUMMARY" ] && exit 0 -# Redact tokens/passwords from summary -SUMMARY=$(echo "$SUMMARY" | sed -E 's/(token|password|api_key|secret|credential)["\047 :=]+[^ "\047]{8,}/\1=[REDACTED]/gi') -SUMMARY=$(echo "$SUMMARY" | sed -E 's/Bearer [A-Za-z0-9_\.\-\/+=]+/Bearer [REDACTED]/g') +# Redact summary (covers Wrote/Edited cases plus a defence-in-depth pass on Bash). +SUMMARY=$(printf '%s' "$SUMMARY" | redact_secrets) OBS_ID="obs-$(date +%Y%m%d)-$(openssl rand -hex 4)" diff --git a/tests/test_post_tool_use_hook.sh b/tests/test_post_tool_use_hook.sh new file mode 100755 index 0000000..e12591b --- /dev/null +++ b/tests/test_post_tool_use_hook.sh @@ -0,0 +1,121 @@ +#!/bin/bash +# Smoke tests for openexp/hooks/post-tool-use.sh +# +# Runs the hook against a set of synthetic Claude Code PostToolUse payloads +# and asserts that: +# - secret-bearing commands are written to the observation file with +# [REDACTED] in place of the actual secret +# - read-only commands (ls, find, cat, …) are skipped (no observation written) +# - lsof / lsblk / similar non-read-only commands ARE captured (regression +# test for the earlier `ls*` glob that swallowed them) +# - sensitive file paths (.env, .ssh/*, *.pem) are skipped on Write/Edit +# +# Usage: bash tests/test_post_tool_use_hook.sh + +set -uo pipefail + +HOOK="$(cd "$(dirname "$0")/.." && pwd)/openexp/hooks/post-tool-use.sh" +[ -x "$HOOK" ] || { echo "Hook not executable: $HOOK"; exit 2; } + +TMPDIR=$(mktemp -d) +trap 'rm -rf "$TMPDIR"' EXIT +OBS_FILE="$TMPDIR/observations-$(date +%Y-%m-%d).jsonl" +touch "$OBS_FILE" + +PASS=0 +FAIL=0 + +run_hook() { + # $1 = command string. Builds the JSON payload and pipes into the hook. + jq -n --arg cmd "$1" \ + '{tool_name:"Bash",session_id:"t",cwd:"/tmp/proj",tool_input:{command:$cmd}}' \ + | OPENEXP_OBSERVATIONS_DIR="$TMPDIR" bash "$HOOK" +} + +run_write_hook() { + jq -n --arg fp "$1" \ + '{tool_name:"Write",session_id:"t",cwd:"/tmp/proj",tool_input:{file_path:$fp}}' \ + | OPENEXP_OBSERVATIONS_DIR="$TMPDIR" bash "$HOOK" +} + +last_summary() { tail -1 "$OBS_FILE" | jq -r '.summary // empty'; } +file_lines() { wc -l < "$OBS_FILE" | tr -d ' '; } + +assert_redacted() { + local label="$1" cmd="$2" forbidden="$3" + run_hook "$cmd" + local s + s=$(last_summary) + if echo "$s" | grep -q "REDACTED" && ! echo "$s" | grep -q "$forbidden"; then + PASS=$((PASS+1)); printf " PASS %s\n" "$label" + else + FAIL=$((FAIL+1)); printf " FAIL %s\n summary: %s\n" "$label" "$s" + fi +} + +assert_skipped() { + local label="$1" cmd="$2" + local before=$(file_lines) + run_hook "$cmd" + local after=$(file_lines) + if [ "$before" = "$after" ]; then + PASS=$((PASS+1)); printf " PASS %s\n" "$label" + else + FAIL=$((FAIL+1)); printf " FAIL %s (was captured but should skip)\n" "$label" + fi +} + +assert_captured() { + local label="$1" cmd="$2" + local before=$(file_lines) + run_hook "$cmd" + local after=$(file_lines) + if [ "$after" != "$before" ]; then + PASS=$((PASS+1)); printf " PASS %s\n" "$label" + else + FAIL=$((FAIL+1)); printf " FAIL %s (was skipped but should capture)\n" "$label" + fi +} + +assert_write_skipped() { + local label="$1" path="$2" + local before=$(file_lines) + run_write_hook "$path" + local after=$(file_lines) + if [ "$before" = "$after" ]; then + PASS=$((PASS+1)); printf " PASS %s\n" "$label" + else + FAIL=$((FAIL+1)); printf " FAIL %s (was captured but should skip)\n" "$label" + fi +} + +echo "=== Secret redaction (Bash) ===" +assert_redacted "api_key= form" 'curl -d api_key="abc12345xyz_secretvalue" url' "abc12345xyz_secretvalue" +assert_redacted "Bearer sk-ant-..." 'curl -H "Authorization: Bearer sk-ant-api03-realsecret" url' "sk-ant-api03-realsecret" +assert_redacted "--token=val" 'npm publish --token=secretvalue1234567890' "secretvalue1234567890" +assert_redacted "--password=\"val\"" 'mysql -u root --password="hunter2hunter2"' "hunter2hunter2" +assert_redacted "inline ENV=val" 'ANTHROPIC_API_KEY=sk-ant-api03-realsecret npm test' "sk-ant-api03-realsecret" +assert_redacted "export GITHUB_TOKEN" 'export GITHUB_TOKEN=ghp_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' "ghp_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + +echo "=== Read-only commands skipped ===" +assert_skipped "ls -la" 'ls -la' +assert_skipped "find / -name x" 'find / -name x' +assert_skipped "cat /etc/hosts" 'cat /etc/hosts' +assert_skipped "pwd" 'pwd' +assert_skipped "echo hi" 'echo hi' +assert_skipped "head -1 file" 'head -1 file' + +echo "=== Non-read-only commands captured (regression: ls* glob) ===" +assert_captured "lsof -i :8080" 'lsof -i :8080' +assert_captured "lsblk" 'lsblk' + +echo "=== Sensitive file paths skipped (Write) ===" +assert_write_skipped ".env file" '/home/u/.env' +assert_write_skipped ".env.local" '/home/u/.env.local' +assert_write_skipped "credentials.json" '/home/u/credentials.json' +assert_write_skipped ".ssh/id_rsa" '/home/u/.ssh/id_rsa' +assert_write_skipped "*.pem" '/home/u/cert.pem' + +echo "" +echo "Result: $PASS passed, $FAIL failed" +[ "$FAIL" = "0" ]