From 01e57e90b93994e12c460241c17893529df967d8 Mon Sep 17 00:00:00 2001 From: Steve Nims Date: Sat, 13 Dec 2025 09:34:45 -0500 Subject: [PATCH 1/2] fix: harden example hook validation scripts against bypass attacks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add command chaining detection to validate-bash.sh before the safe command allowlist to prevent bypasses like `echo; rm -rf /` - Check for semicolons, pipes, command substitution ($(), ``), and logical operators (&&, ||) - Add documentation comments to validate-write.sh explaining the limitations of literal ".." path traversal checks - Include guidance for production hardening (realpath, allowed prefixes) The example scripts teach secure patterns but previously had gaps that could propagate insecure practices if users copied them directly. Fixes #161 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../hook-development/examples/validate-bash.sh | 14 ++++++++++++++ .../hook-development/examples/validate-write.sh | 7 +++++++ 2 files changed, 21 insertions(+) diff --git a/plugins/plugin-dev/skills/hook-development/examples/validate-bash.sh b/plugins/plugin-dev/skills/hook-development/examples/validate-bash.sh index e364324..5f790b1 100755 --- a/plugins/plugin-dev/skills/hook-development/examples/validate-bash.sh +++ b/plugins/plugin-dev/skills/hook-development/examples/validate-bash.sh @@ -16,7 +16,21 @@ if [ -z "$command" ]; then exit 0 fi +# SECURITY: Check for command chaining/injection patterns FIRST +# These checks must run before the "safe command" allowlist to prevent bypasses +# like: echo $(rm -rf /), ls; malicious, pwd && evil, whoami | exfil +# Note: This is not exhaustive - production hooks should consider additional +# patterns like newlines, null bytes, and shell-specific syntax +# shellcheck disable=SC2016 # Single quotes intentional - matching literal $( and ` characters +if [[ "$command" == *";"* ]] || [[ "$command" == *"|"* ]] || + [[ "$command" == *'$('* ]] || [[ "$command" == *'`'* ]] || + [[ "$command" == *"&&"* ]] || [[ "$command" == *"||"* ]]; then + echo '{"hookSpecificOutput": {"permissionDecision": "ask"}, "systemMessage": "Command chaining detected - requires review"}' >&2 + exit 2 +fi + # Check for obviously safe commands (quick approval) +# IMPORTANT: This check is only safe because chaining patterns are caught above if [[ "$command" =~ ^(ls|pwd|echo|date|whoami)(\s|$) ]]; then exit 0 fi diff --git a/plugins/plugin-dev/skills/hook-development/examples/validate-write.sh b/plugins/plugin-dev/skills/hook-development/examples/validate-write.sh index c4dde41..999a40e 100755 --- a/plugins/plugin-dev/skills/hook-development/examples/validate-write.sh +++ b/plugins/plugin-dev/skills/hook-development/examples/validate-write.sh @@ -17,6 +17,13 @@ if [ -z "$file_path" ]; then fi # Check for path traversal +# NOTE: This basic check catches literal ".." but has limitations: +# - Does not detect URL-encoded traversal (%2e%2e) +# - Cannot detect symlink-based traversal where resolved path escapes bounds +# - Shell expansion could bypass in some contexts +# For production hooks, consider using: +# resolved=$(realpath -m "$file_path" 2>/dev/null || echo "$file_path") +# and comparing against an allowed directory prefix if [[ "$file_path" == *".."* ]]; then jq -n --arg path "$file_path" \ '{"hookSpecificOutput": {"permissionDecision": "deny"}, "systemMessage": "Path traversal detected in: \($path)"}' >&2 From 7ee87c106d9af086f4934e3b77a6c031643bdb1d Mon Sep 17 00:00:00 2001 From: Steve Nims Date: Sat, 13 Dec 2025 09:40:34 -0500 Subject: [PATCH 2/2] docs: address PR review feedback - enhance documentation clarity - Add concrete examples (\n, \x00) to validate-bash.sh comment about additional patterns, making the guidance more actionable - Add path traversal limitations documentation to migration.md for consistency with validate-write.sh improvements These changes improve the educational value of the example scripts and reference documentation by providing specific, concrete guidance on security considerations. Addresses suggestions from PR #164 review. --- .../skills/hook-development/examples/validate-bash.sh | 2 +- .../skills/hook-development/references/migration.md | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/plugins/plugin-dev/skills/hook-development/examples/validate-bash.sh b/plugins/plugin-dev/skills/hook-development/examples/validate-bash.sh index 5f790b1..087861c 100755 --- a/plugins/plugin-dev/skills/hook-development/examples/validate-bash.sh +++ b/plugins/plugin-dev/skills/hook-development/examples/validate-bash.sh @@ -20,7 +20,7 @@ fi # These checks must run before the "safe command" allowlist to prevent bypasses # like: echo $(rm -rf /), ls; malicious, pwd && evil, whoami | exfil # Note: This is not exhaustive - production hooks should consider additional -# patterns like newlines, null bytes, and shell-specific syntax +# patterns like newlines (\n), null bytes (\x00), and shell-specific syntax # shellcheck disable=SC2016 # Single quotes intentional - matching literal $( and ` characters if [[ "$command" == *";"* ]] || [[ "$command" == *"|"* ]] || [[ "$command" == *'$('* ]] || [[ "$command" == *'`'* ]] || diff --git a/plugins/plugin-dev/skills/hook-development/references/migration.md b/plugins/plugin-dev/skills/hook-development/references/migration.md index 587cae3..f53c0fc 100644 --- a/plugins/plugin-dev/skills/hook-development/references/migration.md +++ b/plugins/plugin-dev/skills/hook-development/references/migration.md @@ -108,6 +108,13 @@ input=$(cat) file_path=$(echo "$input" | jq -r '.tool_input.file_path') # Check for path traversal +# NOTE: This basic check catches literal ".." but has limitations: +# - Does not detect URL-encoded traversal (%2e%2e) +# - Cannot detect symlink-based traversal where resolved path escapes bounds +# - Shell expansion could bypass in some contexts +# For production hooks, consider using: +# resolved=$(realpath -m "$file_path" 2>/dev/null || echo "$file_path") +# and comparing against an allowed directory prefix if [[ "$file_path" == *".."* ]]; then echo '{"decision": "deny", "reason": "Path traversal detected"}' >&2 exit 2