diff --git a/hooks/no-chaining.sh b/hooks/no-chaining.sh index d8a1789..7a43013 100755 --- a/hooks/no-chaining.sh +++ b/hooks/no-chaining.sh @@ -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 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() { @@ -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 diff --git a/hooks/no-chaining.test.sh b/hooks/no-chaining.test.sh index aeeaf7c..6e1aa34 100644 --- a/hooks/no-chaining.test.sh +++ b/hooks/no-chaining.test.sh @@ -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" + # ========== npm as pipe source: safe source, unsafe sink ========== assert_blocked "blocks: npm test piped to xargs" \ diff --git a/install.sh b/install.sh index 3258299..2d292f7 100755 --- a/install.sh +++ b/install.sh @@ -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 @@ -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" +import sys, json, copy, tempfile, os src_path, dst_path = sys.argv[1], sys.argv[2] @@ -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, []) @@ -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 @@ -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 diff --git a/settings/claude-md.md b/settings/claude-md.md index cc2ddf1..6844171 100644 --- a/settings/claude-md.md +++ b/settings/claude-md.md @@ -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