Skip to content

Commit 5f5f5d9

Browse files
sjnimsclaude
andauthored
fix: harden example hook validation scripts against bypass attacks (#164)
## Summary Hardens the example hook validation scripts to prevent security bypass patterns that users might inadvertently copy when creating their own hooks. ## Problem Fixes #161 The example hook validation scripts in `skills/hook-development/examples/` had security gaps: 1. **validate-bash.sh**: The "safe command" allowlist (ls, pwd, echo, etc.) could be bypassed with command chaining: - `echo $(rm -rf /)` - command substitution - `ls; rm -rf /` - semicolon chaining - `pwd && malicious` - AND chaining 2. **validate-write.sh**: The path traversal check only detected literal `..` without documenting limitations like URL-encoding or symlink traversal. ## Solution ### validate-bash.sh Added command chaining detection **before** the safe command allowlist: - Checks for `;`, `|`, `$(`, `` ` ``, `&&`, `||` - Returns `"permissionDecision": "ask"` to require user review - Added comment explaining this is the critical ordering ### validate-write.sh Added comprehensive documentation about limitations: - Notes that literal `..` check doesn't catch URL-encoded or symlink traversal - Provides guidance for production hardening using `realpath` - Suggests comparing against allowed directory prefixes ### Alternatives Considered 1. **More comprehensive bash validation** - Could add newline, null byte, and shell-specific syntax detection. Decided against to keep examples teachable rather than production-ready. 2. **Add realpath resolution to validate-write.sh** - Decided documentation is more appropriate for an example script, as realpath behavior varies across systems. ## Changes - `plugins/plugin-dev/skills/hook-development/examples/validate-bash.sh`: Added command chaining detection block with explanatory comments - `plugins/plugin-dev/skills/hook-development/examples/validate-write.sh`: Added documentation comments about path traversal limitations ## Testing - [x] Verified semicolon chaining now caught: `echo '{"tool_input": {"command": "echo; malicious"}}' | bash validate-bash.sh` → exit 2 - [x] Verified simple commands still pass: `echo '{"tool_input": {"command": "echo hello"}}' | bash validate-bash.sh` → exit 0 - [x] shellcheck passes on both modified scripts - [x] markdownlint passes on related documentation --- 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 444ed21 commit 5f5f5d9

3 files changed

Lines changed: 28 additions & 0 deletions

File tree

plugins/plugin-dev/skills/hook-development/examples/validate-bash.sh

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,21 @@ if [ -z "$command" ]; then
1616
exit 0
1717
fi
1818

19+
# SECURITY: Check for command chaining/injection patterns FIRST
20+
# These checks must run before the "safe command" allowlist to prevent bypasses
21+
# like: echo $(rm -rf /), ls; malicious, pwd && evil, whoami | exfil
22+
# Note: This is not exhaustive - production hooks should consider additional
23+
# patterns like newlines (\n), null bytes (\x00), and shell-specific syntax
24+
# shellcheck disable=SC2016 # Single quotes intentional - matching literal $( and ` characters
25+
if [[ "$command" == *";"* ]] || [[ "$command" == *"|"* ]] ||
26+
[[ "$command" == *'$('* ]] || [[ "$command" == *'`'* ]] ||
27+
[[ "$command" == *"&&"* ]] || [[ "$command" == *"||"* ]]; then
28+
echo '{"hookSpecificOutput": {"permissionDecision": "ask"}, "systemMessage": "Command chaining detected - requires review"}' >&2
29+
exit 2
30+
fi
31+
1932
# Check for obviously safe commands (quick approval)
33+
# IMPORTANT: This check is only safe because chaining patterns are caught above
2034
if [[ "$command" =~ ^(ls|pwd|echo|date|whoami)(\s|$) ]]; then
2135
exit 0
2236
fi

plugins/plugin-dev/skills/hook-development/examples/validate-write.sh

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,13 @@ if [ -z "$file_path" ]; then
1717
fi
1818

1919
# Check for path traversal
20+
# NOTE: This basic check catches literal ".." but has limitations:
21+
# - Does not detect URL-encoded traversal (%2e%2e)
22+
# - Cannot detect symlink-based traversal where resolved path escapes bounds
23+
# - Shell expansion could bypass in some contexts
24+
# For production hooks, consider using:
25+
# resolved=$(realpath -m "$file_path" 2>/dev/null || echo "$file_path")
26+
# and comparing against an allowed directory prefix
2027
if [[ "$file_path" == *".."* ]]; then
2128
jq -n --arg path "$file_path" \
2229
'{"hookSpecificOutput": {"permissionDecision": "deny"}, "systemMessage": "Path traversal detected in: \($path)"}' >&2

plugins/plugin-dev/skills/hook-development/references/migration.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,13 @@ input=$(cat)
108108
file_path=$(echo "$input" | jq -r '.tool_input.file_path')
109109

110110
# Check for path traversal
111+
# NOTE: This basic check catches literal ".." but has limitations:
112+
# - Does not detect URL-encoded traversal (%2e%2e)
113+
# - Cannot detect symlink-based traversal where resolved path escapes bounds
114+
# - Shell expansion could bypass in some contexts
115+
# For production hooks, consider using:
116+
# resolved=$(realpath -m "$file_path" 2>/dev/null || echo "$file_path")
117+
# and comparing against an allowed directory prefix
111118
if [[ "$file_path" == *".."* ]]; then
112119
echo '{"decision": "deny", "reason": "Path traversal detected"}' >&2
113120
exit 2

0 commit comments

Comments
 (0)