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..087861c 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 (\n), null bytes (\x00), 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 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