Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
52 changes: 52 additions & 0 deletions hooks/no-chaining.sh
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,56 @@ npm_segment_safe() {
[[ "$script" =~ ^(test|lint|check|typecheck|type-check|build|compile)(:[a-zA-Z0-9_-]+)?$ ]]
}

# Check if an 'npx' segment is safe to use as a pipe SOURCE.
#
# Safe binaries: vitest, jest, mocha, tsc, eslint, prettier, biome, oxlint
# (test runners, linters, type checkers, formatters — all read-only or
# side-effect-free when piped)
#
# Handles --prefix <path> and other value-consuming flags before the binary name.
npx_segment_safe() {
local segment="$1"

# Must start with npx
[[ "$segment" =~ ^npx([[:space:]]|$) ]] || return 1

# Tokenize into a bash array (splits on whitespace)
local tokens
read -ra tokens <<< "$segment"

local i=1 # start after 'npx'
local binary=""

# npx flags that consume the next token as their value
local token
while [ $i -lt ${#tokens[@]} ]; do
token="${tokens[$i]}"
case "$token" in
--call|-c|--package|-p)
# --call/-c runs arbitrary shell code; --package/-p allows arbitrary
# package installation with a trojan binary name — never safe
return 1
;;
--prefix)
# Skip flag and its value argument
i=$((i + 2))
continue
;;
-*)
# Boolean flag (--yes, --no, etc.) — skip
i=$((i + 1))
continue
;;
esac
# First non-flag token is the binary name
binary="$token"
break
done

# Safe binaries — test runners, linters, type checkers, formatters
[[ "$binary" =~ ^(vitest|jest|mocha|tsc|eslint|prettier|biome|oxlint)$ ]]
}

# Helper: check that every segment in a newline-separated list starts with a safe
# read-only command. Used for both | and || allowlisting.
all_segments_safe() {
Expand Down Expand Up @@ -187,6 +237,8 @@ if printf '%s' "$no_double_pipe" | grep -qF '|'; then
source_ok=1
elif npm_segment_safe "$source_segment"; then
source_ok=1
elif npx_segment_safe "$source_segment"; then
source_ok=1
fi

sinks_ok=1
Expand Down
72 changes: 72 additions & 0 deletions hooks/no-chaining.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,78 @@ assert_blocked "blocks: npm --prefix /path install | tail" \
assert_blocked "blocks: npm --prefix /path run deploy | tail" \
"npm --prefix /path run deploy | tail -20"

# ========== npx as pipe source: safe binaries ==========

assert_allowed "allows: npx vitest run | tail" \
"npx vitest run tests/unit/foo.test.ts | tail -30"

assert_allowed "allows: npx --prefix /path vitest run | tail" \
"npx --prefix /Users/josephfung/Projects/myapp vitest run tests/unit/foo.test.ts | tail -30"

assert_allowed "allows: npx --prefix /path vitest run 2>&1 | tail" \
"npx --prefix /Users/josephfung/Projects/myapp vitest run tests/unit/foo.test.ts 2>&1 | tail -30"

assert_allowed "allows: npx --prefix /path vitest run 2>&1 | grep" \
"npx --prefix /Users/josephfung/Projects/myapp vitest run tests/unit/foo.test.ts 2>&1 | grep -E '(PASS|FAIL)'"

assert_allowed "allows: npx --prefix /path vitest run | grep | head" \
"npx --prefix /Users/josephfung/Projects/myapp vitest run tests/unit/foo.test.ts --reporter=verbose 2>&1 | grep -E '(PASS|FAIL)' | head -30"

assert_allowed "allows: npx jest | tail" \
"npx jest --verbose | tail -30"

assert_allowed "allows: npx mocha | grep" \
"npx mocha tests/ | grep passing"

assert_allowed "allows: npx tsc --noEmit | tail" \
"npx tsc --noEmit | tail -40"

assert_allowed "allows: npx eslint | grep" \
"npx eslint src/ | grep error"

assert_allowed "allows: npx prettier --check | tail" \
"npx prettier --check src/ | tail -20"

assert_allowed "allows: npx biome check | grep" \
"npx biome check src/ | grep error"

assert_allowed "allows: npx oxlint | grep" \
"npx oxlint src/ | grep error"

assert_allowed "allows: npx --yes vitest run | tail" \
"npx --yes vitest run | tail -30"

# ========== npx as pipe source: unsafe binaries ==========

assert_blocked "blocks: npx ts-node | tail" \
"npx ts-node script.ts | tail -20"

assert_blocked "blocks: npx arbitrary-package | tail" \
"npx some-package | tail -20"

assert_blocked "blocks: npx tsx | tail" \
"npx tsx script.ts | tail -20"

assert_blocked "blocks: bare npx piped" \
"npx | tail -20"

assert_blocked "blocks: npx --call with safe binary name" \
"npx --call 'echo pwned' vitest | tail -20"

assert_blocked "blocks: npx --package with safe binary name" \
"npx --package malicious-pkg vitest | tail -20"

assert_blocked "blocks: npx -p with safe binary name" \
"npx -p malicious-pkg vitest | tail -20"

# ========== npx as pipe source: safe source, unsafe sink ==========

assert_blocked "blocks: npx vitest piped to xargs" \
"npx vitest run | xargs rm"

assert_blocked "blocks: npx vitest piped to bash" \
"npx vitest run | bash"

Comment thread
josephfung marked this conversation as resolved.
# ========== npm as pipe source: safe source, unsafe sink ==========

assert_blocked "blocks: npm test piped to xargs" \
Expand Down
120 changes: 94 additions & 26 deletions install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -236,13 +236,16 @@ install_plugins

hooks_merged=()
hooks_skipped=()
hooks_failed=()
settings_failed=()
perms_merged=()
perms_skipped=()

merge_hooks() {
merge_settings() {
local src="$SCRIPT_DIR/settings/hooks.json"
local dst="$HOME/.claude/settings.json"

if [ ! -f "$src" ]; then
warned+=("settings/hooks.json not found — hooks and permissions not merged")
return
fi

Expand All @@ -251,12 +254,21 @@ merge_hooks() {
echo "{}" > "$dst"
fi

# Use Python to merge hooks without duplicates.
# For each hook event type (PreToolUse, etc.) and each matcher entry in the
# Use Python to merge hooks and permissions without duplicates.
#
# Hooks: for each event type (PreToolUse, etc.) and each matcher entry in the
# source, find or create the matching entry in the destination, then add any
# hook commands that are not already present (matched by "command" field).
merge_result="$(python3 - "$src" "$dst" <<'PYEOF'
import sys, json, copy
#
# Permissions: for each entry in source permissions.allow, add it to the
# destination permissions.allow array if not already present (exact match).
#
# Writes to a temp file first, then atomically renames to prevent corruption
# if the script crashes mid-write. Stderr is captured for actionable errors.
local merge_stderr merge_result
merge_stderr="$(mktemp)"
merge_result="$(python3 - "$src" "$dst" <<'PYEOF' 2>"$merge_stderr"
Comment thread
josephfung marked this conversation as resolved.
import sys, json, copy, tempfile, os

src_path, dst_path = sys.argv[1], sys.argv[2]

Expand All @@ -265,12 +277,14 @@ with open(src_path) as f:
with open(dst_path) as f:
dst = json.load(f)

src_hooks = src.get("hooks", {})
dst_hooks = dst.setdefault("hooks", {})

added = []
skipped = []

# --- Merge hooks ---

src_hooks = src.get("hooks", {})
dst_hooks = dst.setdefault("hooks", {})

for event_type, src_entries in src_hooks.items():
dst_entries = dst_hooks.setdefault(event_type, [])

Expand All @@ -288,33 +302,75 @@ for event_type, src_entries in src_hooks.items():
for hook in src_entry.get("hooks", []):
cmd = hook.get("command")
if cmd in dst_cmds:
skipped.append(cmd)
skipped.append("HOOK:" + cmd)
else:
dst_entry.setdefault("hooks", []).append(copy.deepcopy(hook))
dst_cmds.add(cmd)
added.append(cmd)

with open(dst_path, "w") as f:
json.dump(dst, f, indent=2)
f.write("\n")
added.append("HOOK:" + cmd)

# --- Merge permissions ---

src_perms = src.get("permissions", {}).get("allow", [])
if src_perms:
# Validate destination permissions shape before operating on it
dst_perms = dst.get("permissions")
if dst_perms is not None and not isinstance(dst_perms, dict):
print(f"error: 'permissions' in {dst_path} is {type(dst_perms).__name__}, expected object", file=sys.stderr)
sys.exit(1)

dst_allow = dst.setdefault("permissions", {}).setdefault("allow", [])
if not isinstance(dst_allow, list):
print(f"error: 'permissions.allow' in {dst_path} is {type(dst_allow).__name__}, expected array", file=sys.stderr)
sys.exit(1)

# Only deduplicate against string entries (skip any non-hashable objects)
dst_allow_set = {e for e in dst_allow if isinstance(e, str)}

for perm in src_perms:
if perm in dst_allow_set:
skipped.append("PERM:" + perm)
else:
dst_allow.append(perm)
dst_allow_set.add(perm)
added.append("PERM:" + perm)

# Atomic write: write to temp file, then rename to prevent corruption on crash
tmp_fd, tmp_path = tempfile.mkstemp(dir=os.path.dirname(dst_path), suffix=".tmp")
try:
with os.fdopen(tmp_fd, "w", encoding="utf-8") as f:
json.dump(dst, f, indent=2)
f.write("\n")
os.rename(tmp_path, dst_path)
except:
os.unlink(tmp_path)
raise

# Report results on stdout for the shell to parse
for cmd in added:
print("ADDED:" + cmd)
for cmd in skipped:
print("SKIPPED:" + cmd)
for item in added:
print("ADDED:" + item)
for item in skipped:
print("SKIPPED:" + item)
PYEOF
)" || { hooks_failed+=("settings/hooks.json (python error)"); return; }
)" || {
local detail
detail="$(cat "$merge_stderr" 2>/dev/null)"
settings_failed+=("settings merge: ${detail:-unknown python error}")
rm -f "$merge_stderr"
return
}
rm -f "$merge_stderr"

while IFS= read -r line; do
case "$line" in
ADDED:*) hooks_merged+=("${line#ADDED:}") ;;
SKIPPED:*) hooks_skipped+=("${line#SKIPPED:}") ;;
ADDED:HOOK:*) hooks_merged+=("${line#ADDED:HOOK:}") ;;
SKIPPED:HOOK:*) hooks_skipped+=("${line#SKIPPED:HOOK:}") ;;
ADDED:PERM:*) perms_merged+=("${line#ADDED:PERM:}") ;;
SKIPPED:PERM:*) perms_skipped+=("${line#SKIPPED:PERM:}") ;;
esac
done <<< "$merge_result"
}

merge_hooks
merge_settings

# ---------------------------------------------------------------------------
# CLAUDE.md injection
Expand Down Expand Up @@ -459,10 +515,22 @@ if [ ${#hooks_skipped[@]} -gt 0 ]; then
for f in "${hooks_skipped[@]}"; do echo " - $f"; done
fi

if [ ${#hooks_failed[@]} -gt 0 ]; then
if [ ${#settings_failed[@]} -gt 0 ]; then
echo ""
echo "Settings merge failures:"
for f in "${settings_failed[@]}"; do echo " ✗ $f"; done
fi

if [ ${#perms_merged[@]} -gt 0 ]; then
echo ""
echo "Permissions merged into ~/.claude/settings.json:"
for f in "${perms_merged[@]}"; do echo " ✓ $f"; done
fi

if [ ${#perms_skipped[@]} -gt 0 ]; then
echo ""
echo "Hook merge failures:"
for f in "${hooks_failed[@]}"; do echo " $f"; done
echo "Permissions already present (skipped):"
for f in "${perms_skipped[@]}"; do echo " - $f"; done
fi

case "$claude_md_result" in
Expand Down
17 changes: 12 additions & 5 deletions settings/claude-md.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,16 @@ The full allowlist for pipe sources and sinks:
`uniq`, `wc` — plus `git` (read-only subcommands: `branch`, `diff`, `log`,
`ls-files`, `rev-parse`, `show`, `status`).

Additionally allowed as a pipe **source only**: `npm test`, `npm t`, `npm ls`,
`npm list`, `npm audit`, `npm outdated`, `npm view`, `npm info`, and
`npm run <script>` where the script name is `test`, `lint`, `check`,
`typecheck`, `type-check`, `build`, or `compile` (colon-namespaced variants
like `test:unit` also work). The `--prefix <path>` flag is supported.
Additionally allowed as a pipe **source only**:

- `npm test`, `npm t`, `npm ls`, `npm list`, `npm audit`, `npm outdated`,
`npm view`, `npm info`, and `npm run <script>` where the script name is
`test`, `lint`, `check`, `typecheck`, `type-check`, `build`, or `compile`
(colon-namespaced variants like `test:unit` also work).
- `npx <binary>` where the binary is `vitest`, `jest`, `mocha`, `tsc`,
`eslint`, `prettier`, `biome`, or `oxlint`.

Both `npm --prefix <path>` and `npx --prefix <path>` are supported.

**Never use `xargs` in pipes** — it is not on the allowlist and will be blocked.
Use the Grep tool or pass file lists explicitly instead.
Expand All @@ -57,6 +62,8 @@ Use the Grep tool or pass file lists explicitly instead.
# Allowed:
npm test | tail -60
npm run lint | grep error
npx vitest run tests/unit/foo.test.ts | tail -30
npx --prefix /path/to/project jest --verbose | grep PASS
git log --oneline | grep feat
find . -name "*.ts" | grep src

Expand Down
10 changes: 9 additions & 1 deletion settings/hooks.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
{
"_comment": "Merge this into your ~/.claude/settings.json under hooks",
"_comment": "Merged into ~/.claude/settings.json by install.sh (hooks + permissions)",
"permissions": {
"allow": [
"Bash(git -C:*)",
"Bash(npm --prefix:*)",
"Bash(pnpm --prefix:*)",
"Bash(npx --prefix:*)"
]
},
"hooks": {
"UserPromptSubmit": [
{
Expand Down
Loading