Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions config/claude/security.sh
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,10 @@ matches_pattern() {
# Extract pattern from Bash(...) format
if [[ $pattern =~ ^Bash\((.+)\)$ ]]; then
local check_pattern="${BASH_REMATCH[1]}"
# Remove trailing :* if present (legacy format)
check_pattern="${check_pattern%:*}"
# Convert legacy trailing :* into a prefix glob (e.g. sudo:* -> sudo*)
if [[ $check_pattern == *':*' ]]; then
check_pattern="${check_pattern%:*}*"
fi

# Use bash glob matching (extended globbing)
shopt -s extglob
Expand All @@ -53,8 +55,7 @@ matches_pattern() {

# Split command at logical operators to catch hidden dangerous commands
# This handles: cmd1 ; cmd2, cmd1 && cmd2, cmd1 || cmd2, cmd1 | cmd2
# shellcheck disable=SC2001
IFS=$'\n' read -r -d '' -a segments < <(echo "$command" | sed 's/[;&|]\+/\n/g' && printf '\0') || true
IFS=$'\n' read -r -d '' -a segments < <(echo "$command" | sed -E 's/[;&|]+/\n/g' && printf '\0') || true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For better robustness and portability, it's recommended to use printf instead of echo when piping variable content to other commands. echo can have surprising behavior if the variable's content starts with a hyphen (-) or contains backslash escape sequences, and its implementation varies between shells. printf '%s' "$command" is a safer alternative that will print the string exactly as is.

Suggested change
IFS=$'\n' read -r -d '' -a segments < <(echo "$command" | sed -E 's/[;&|]+/\n/g' && printf '\0') || true
IFS=$'\n' read -r -d '' -a segments < <(printf '%s' "$command" | sed -E 's/[;&|]+ /\n/g' && printf '\0') || true


for segment in "${segments[@]}"; do
# Trim leading/trailing whitespace
Expand Down
64 changes: 52 additions & 12 deletions spec/notify_spec.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,14 @@ End

Describe 'Notification hook (no Pushover)'
setup() {
# Create mock osascript that does nothing
MOCK_BIN=$(mktemp -d)
printf '#!/bin/sh\nexit 0\n' >"$MOCK_BIN/osascript"
chmod +x "$MOCK_BIN/osascript"
export PATH="$MOCK_BIN:$PATH"
mock_bin_setup osascript

# Unset Pushover credentials to ensure clean test environment
unset PUSHOVER_API_TOKEN
unset PUSHOVER_USER_KEY
}
cleanup() {
rm -rf "$MOCK_BIN"
mock_bin_cleanup
}
Before 'setup'
After 'cleanup'
Expand All @@ -47,22 +43,25 @@ It 'exits 0 for waiting notification'
When run bash -c 'echo "{\"message\": \"Claude is waiting for your input\"}" | env HOME=/nonexistent bash '"$SCRIPT"
The status should be success
End

It 'sends Basso notification for permission request'
When run bash -c 'echo "{\"message\": \"Claude needs your permission to use Bash\"}" | env HOME=/nonexistent bash '"$SCRIPT"'; cat "$MOCK_LOG"'
The status should be success
The output should include 'Bash permission required'
The output should include 'sound name "Basso"'
End
End

Describe 'SessionEnd hook (no Pushover)'
setup() {
# Create mock osascript that does nothing
MOCK_BIN=$(mktemp -d)
printf '#!/bin/sh\nexit 0\n' >"$MOCK_BIN/osascript"
chmod +x "$MOCK_BIN/osascript"
export PATH="$MOCK_BIN:$PATH"
mock_bin_setup osascript

# Unset Pushover credentials to ensure clean test environment
unset PUSHOVER_API_TOKEN
unset PUSHOVER_USER_KEY
}
cleanup() {
rm -rf "$MOCK_BIN"
mock_bin_cleanup
}
Before 'setup'
After 'cleanup'
Expand All @@ -73,4 +72,45 @@ When run bash -c 'echo "{\"reason\": \"user_exit\"}" | env HOME=/nonexistent bas
The status should be success
End
End

Describe 'other hooks (no Pushover)'
setup() {
mock_bin_setup osascript
TEMP_HOME=$(mktemp -d)

unset PUSHOVER_API_TOKEN
unset PUSHOVER_USER_KEY
}
cleanup() {
rm -rf "$TEMP_HOME"
mock_bin_cleanup
}
Before 'setup'
After 'cleanup'

It 'notifies on PreCompact auto trigger'
When run bash -c 'echo "{\"trigger\": \"auto\"}" | env HOME=/nonexistent bash '"$SCRIPT"'; cat "$MOCK_LOG"'
The status should be success
The output should include 'Auto-compacting context'
End

It 'notifies on SubagentStop'
When run bash -c 'echo "{\"stop_hook_active\": true}" | env HOME=/nonexistent bash '"$SCRIPT"'; cat "$MOCK_LOG"'
The status should be success
The output should include 'Subagent task completed'
End

It 'notifies on Stop with cwd and session id'
When run bash -c 'echo "{\"cwd\": \"'"$TEMP_HOME"'/work\", \"session_id\": \"abcdef0123456789\"}" | env HOME="'"$TEMP_HOME"'" bash '"$SCRIPT"'; cat "$MOCK_LOG"'
The status should be success
The output should include 'Work completed in ~/work (abcdef01)'
End

It 'warns on risky PreToolUse Bash command'
When run bash -c 'echo "{\"tool\": {\"name\": \"Bash\", \"input\": \"rm -rf /\"}}" | env HOME=/nonexistent bash '"$SCRIPT"'; cat "$MOCK_LOG"'
The status should be success
The output should include 'Risky: rm -rf /'
The output should include 'sound name "Basso"'
End
End
End
86 changes: 64 additions & 22 deletions spec/pushover_spec.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,14 @@ SCRIPT="$PWD/config/claude/pushover.sh"

Describe 'credential handling'
setup() {
# Create a mock curl in case credentials leak through
MOCK_BIN=$(mktemp -d)
printf '#!/bin/sh\nexit 0\n' >"$MOCK_BIN/curl"
chmod +x "$MOCK_BIN/curl"
export PATH="$MOCK_BIN:$PATH"
mock_bin_setup curl

# Unset credentials to ensure clean test environment
unset PUSHOVER_API_TOKEN
unset PUSHOVER_USER_KEY
}
cleanup() {
rm -rf "$MOCK_BIN"
mock_bin_cleanup
}
Before 'setup'
After 'cleanup'
Expand All @@ -32,60 +28,106 @@ End

Describe 'SessionEnd hook'
setup() {
# Create a mock curl that does nothing
MOCK_BIN=$(mktemp -d)
printf '#!/bin/sh\nexit 0\n' >"$MOCK_BIN/curl"
chmod +x "$MOCK_BIN/curl"
export PATH="$MOCK_BIN:$PATH"
mock_bin_setup curl

# Set test credentials
export PUSHOVER_API_TOKEN="test_token"
export PUSHOVER_USER_KEY="test_user"
}
cleanup() {
rm -rf "$MOCK_BIN"
mock_bin_cleanup
}
Before 'setup'
After 'cleanup'

It 'skips notification for "other" reason'
When run bash -c 'echo "{\"reason\": \"other\"}" | bash '"$SCRIPT"
When run bash -c 'echo "{\"reason\": \"other\"}" | bash '"$SCRIPT"'; cat "$MOCK_LOG"'
The status should be success
The output should eq ''
End

It 'processes notification for "user_exit" reason'
When run bash -c 'echo "{\"reason\": \"user_exit\"}" | bash '"$SCRIPT"
When run bash -c 'echo "{\"reason\": \"user_exit\"}" | bash '"$SCRIPT"'; cat "$MOCK_LOG"'
The status should be success
The output should include 'priority=0'
The output should include 'https://api.pushover.net/1/messages.json'
End
End

Describe 'Notification hook'
setup() {
# Create a mock curl that does nothing
MOCK_BIN=$(mktemp -d)
printf '#!/bin/sh\nexit 0\n' >"$MOCK_BIN/curl"
chmod +x "$MOCK_BIN/curl"
export PATH="$MOCK_BIN:$PATH"
mock_bin_setup curl

# Set test credentials
export PUSHOVER_API_TOKEN="test_token"
export PUSHOVER_USER_KEY="test_user"
}
cleanup() {
rm -rf "$MOCK_BIN"
mock_bin_cleanup
}
Before 'setup'
After 'cleanup'

It 'skips login notification'
When run bash -c 'echo "{\"message\": \"Claude Code login successful\"}" | bash '"$SCRIPT"
When run bash -c 'echo "{\"message\": \"Claude Code login successful\"}" | bash '"$SCRIPT"'; cat "$MOCK_LOG"'
The status should be success
The output should eq ''
End

It 'processes waiting notification'
When run bash -c 'echo "{\"message\": \"Claude is waiting for your input\"}" | bash '"$SCRIPT"
When run bash -c 'echo "{\"message\": \"Claude is waiting for your input\", \"cwd\": \"/tmp\"}" | bash '"$SCRIPT"'; cat "$MOCK_LOG"'
The status should be success
The output should include 'priority=1'
The output should include 'Waiting'
End

It 'processes permission notification at high priority'
When run bash -c 'echo "{\"message\": \"Claude needs your permission to use Bash\", \"cwd\": \"/tmp\"}" | bash '"$SCRIPT"'; cat "$MOCK_LOG"'
The status should be success
The output should include 'priority=1'
The output should include 'Permission required'
End
End

Describe 'Stop hook'
setup() {
mock_bin_setup curl

export PUSHOVER_API_TOKEN="test_token"
export PUSHOVER_USER_KEY="test_user"

TRANSCRIPT=$(mktemp)
}
cleanup() {
rm -f "$TRANSCRIPT"
mock_bin_cleanup
}
Before 'setup'
After 'cleanup'

It 'notifies plan ready for approval (plan mode + ExitPlanMode + no files)'
cat >"$TRANSCRIPT" <<'JSON'
{"type":"user","message":{"content":[{"type":"text","text":"Do the thing"}]}}
{"cwd":"/tmp/project"}
{"type":"assistant","message":{"content":[{"type":"tool_use","name":"ExitPlanMode"}]}}
JSON
When run bash -c 'echo "{\"hook_event_name\": \"Stop\", \"transcript_path\": \"'"$TRANSCRIPT"'\", \"permission_mode\": \"plan\"}" | bash '"$SCRIPT"'; cat "$MOCK_LOG"'
The status should be success
The output should include 'Plan ready for approval'
The output should include 'priority=1'
End

It 'notifies work completed when files were modified'
cat >"$TRANSCRIPT" <<'JSON'
{"type":"user","message":{"content":[{"type":"text","text":"Update config"}]}}
{"cwd":"/tmp/project"}
{"type":"tool_use","tool_use":{"name":"Write","input":{"file_path":"config/foo"}}}
{"type":"assistant","message":{"content":[{"type":"tool_use","name":"Bash"}]}}
JSON
When run bash -c 'echo "{\"hook_event_name\": \"Stop\", \"transcript_path\": \"'"$TRANSCRIPT"'\", \"permission_mode\": \"plan\"}" | bash '"$SCRIPT"'; cat "$MOCK_LOG"'
The status should be success
The output should include 'Work completed'
The output should include 'priority=0'
End
End
End
14 changes: 14 additions & 0 deletions spec/security_spec.sh
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,20 @@ When run bash -c "HOME='$TEMP_HOME' bash '$SCRIPT'"
The status should eq 2
The stderr should include 'BLOCKED'
End

It 'blocks sudo with arguments (legacy sudo:* pattern)'
Data '{"tool": {"name": "Bash", "input": {"command": "sudo rm -rf /"}}}'
When run bash -c "HOME='$TEMP_HOME' bash '$SCRIPT'"
The status should eq 2
The stderr should include 'BLOCKED'
End

It 'blocks hidden dangerous command after semicolon'
Data '{"tool": {"name": "Bash", "input": {"command": "echo ok; rm -rf /*"}}}'
When run bash -c "HOME='$TEMP_HOME' bash '$SCRIPT'"
The status should eq 2
The stderr should include 'BLOCKED'
End
End

Describe 'edge cases'
Expand Down
34 changes: 34 additions & 0 deletions spec/support/custom_matcher.sh
Original file line number Diff line number Diff line change
@@ -1 +1,35 @@
#!/usr/bin/env bash

set -euo pipefail

mock_bin_setup() {
MOCK_BIN="$(mktemp -d)"
MOCK_LOG="$MOCK_BIN/mock.log"
: >"$MOCK_LOG"

MOCK_ORIGINAL_PATH="${PATH:-}"
export MOCK_BIN MOCK_LOG MOCK_ORIGINAL_PATH
export PATH="$MOCK_BIN:$MOCK_ORIGINAL_PATH"

local cmd
for cmd in "$@"; do
cat >"$MOCK_BIN/$cmd" <<'EOF'
#!/usr/bin/env bash
set -euo pipefail
: "${MOCK_LOG:?MOCK_LOG must be set}"
printf '%s\n' "$0 $*" >>"$MOCK_LOG"

Copilot AI Dec 13, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mock script uses $* which may not properly handle arguments with spaces or special characters. Consider using $@ instead, which preserves the original argument structure and is more suitable for logging invocations. While this may not cause issues in the current tests, it could lead to incorrect logging if mocked commands are called with arguments containing spaces.

Suggested change
printf '%s\n' "$0 $*" >>"$MOCK_LOG"
printf '%s' "$0" >>"$MOCK_LOG"
for arg in "$@"; do printf ' %s' "$arg" >>"$MOCK_LOG"; done
printf '\n' >>"$MOCK_LOG"

Copilot uses AI. Check for mistakes.
exit 0
EOF
chmod +x "$MOCK_BIN/$cmd"
done
}

mock_bin_cleanup() {
if [[ -n "${MOCK_ORIGINAL_PATH:-}" ]]; then
export PATH="$MOCK_ORIGINAL_PATH"
fi
if [[ -n "${MOCK_BIN:-}" ]]; then
rm -rf "$MOCK_BIN"
fi
unset MOCK_BIN MOCK_LOG MOCK_ORIGINAL_PATH
}
Comment on lines +5 to +35

Copilot AI Dec 13, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mock harness functions lack documentation comments. Consider adding comments to explain the purpose and usage of mock_bin_setup and mock_bin_cleanup, including the environment variables they set (MOCK_BIN, MOCK_LOG, MOCK_ORIGINAL_PATH) and how they should be used in tests. This would improve maintainability and help other developers understand how to use these testing utilities.

Copilot uses AI. Check for mistakes.
Loading