From 55524061abec9b304fab01d93ee4cab18cf91621 Mon Sep 17 00:00:00 2001 From: jnjpng Date: Fri, 23 Jan 2026 17:26:50 -0800 Subject: [PATCH] feat: add rm -rf block hook script and fix stderr (#661) --- hooks/block-rm-rf.sh | 20 ++++++++++++++++++++ src/hooks/executor.ts | 19 +++++++++++++------ src/tests/hooks/executor.test.ts | 8 ++++---- src/tests/hooks/integration.test.ts | 17 +++++++++-------- 4 files changed, 46 insertions(+), 18 deletions(-) create mode 100755 hooks/block-rm-rf.sh diff --git a/hooks/block-rm-rf.sh b/hooks/block-rm-rf.sh new file mode 100755 index 0000000..4185200 --- /dev/null +++ b/hooks/block-rm-rf.sh @@ -0,0 +1,20 @@ +#!/bin/bash +# Block dangerous rm -rf commands + +input=$(cat) +tool_name=$(echo "$input" | jq -r '.tool_name') + +# Only check Bash commands +if [ "$tool_name" != "Bash" ]; then + exit 0 +fi + +command=$(echo "$input" | jq -r '.tool_input.command') + +# Check for rm -rf pattern (handles -rf, -fr, -rfi, etc.) +if echo "$command" | grep -qE 'rm\s+(-[a-zA-Z]*r[a-zA-Z]*f|-[a-zA-Z]*f[a-zA-Z]*r)'; then + echo "Blocked: rm -rf commands must be ran manually, use rm and rmdir instead." >&2 + exit 2 +fi + +exit 0 diff --git a/src/hooks/executor.ts b/src/hooks/executor.ts index a736f37..9dadbe7 100644 --- a/src/hooks/executor.ts +++ b/src/hooks/executor.ts @@ -275,11 +275,12 @@ export async function executeHooks( const result = await executeHookCommand(hook, input, workingDirectory); results.push(result); - // Collect feedback from stdout when hook blocks + // Collect feedback from stderr when hook blocks + // Format: [command]: {stderr} per spec if (result.exitCode === HookExitCode.BLOCK) { blocked = true; - if (result.stdout) { - feedback.push(result.stdout); + if (result.stderr) { + feedback.push(`[${hook.command}]: ${result.stderr}`); } // Stop processing more hooks after a block break; @@ -320,11 +321,17 @@ export async function executeHooksParallel( let blocked = false; let errored = false; - for (const result of results) { + // Zip hooks with results to access command for formatting + for (let i = 0; i < results.length; i++) { + const result = results[i]; + const hook = hooks[i]; + if (!result || !hook) continue; + + // Format: [command]: {stderr} per spec if (result.exitCode === HookExitCode.BLOCK) { blocked = true; - if (result.stdout) { - feedback.push(result.stdout); + if (result.stderr) { + feedback.push(`[${hook.command}]: ${result.stderr}`); } } if (result.exitCode === HookExitCode.ERROR) { diff --git a/src/tests/hooks/executor.test.ts b/src/tests/hooks/executor.test.ts index 10ca6ca..494c04f 100644 --- a/src/tests/hooks/executor.test.ts +++ b/src/tests/hooks/executor.test.ts @@ -188,7 +188,7 @@ describe.skipIf(isWindows)("Hooks Executor", () => { test("stops on first blocking hook", async () => { const hooks: HookCommand[] = [ { type: "command", command: "echo 'allowed'" }, - { type: "command", command: "echo 'blocked' && exit 2" }, + { type: "command", command: "echo 'blocked' >&2 && exit 2" }, { type: "command", command: "echo 'should not run'" }, ]; @@ -203,7 +203,7 @@ describe.skipIf(isWindows)("Hooks Executor", () => { expect(result.blocked).toBe(true); expect(result.results).toHaveLength(2); // Only first two ran - expect(result.feedback).toContain("blocked"); + expect(result.feedback[0]).toContain("blocked"); }); test("continues after error but tracks it", async () => { @@ -247,7 +247,7 @@ describe.skipIf(isWindows)("Hooks Executor", () => { const hooks: HookCommand[] = [ { type: "command", - command: "echo 'Reason: file is dangerous' && exit 2", + command: "echo 'Reason: file is dangerous' >&2 && exit 2", }, ]; @@ -261,7 +261,7 @@ describe.skipIf(isWindows)("Hooks Executor", () => { const result = await executeHooks(hooks, input, tempDir); expect(result.blocked).toBe(true); - expect(result.feedback).toContain("Reason: file is dangerous"); + expect(result.feedback[0]).toContain("Reason: file is dangerous"); }); test("collects error feedback from stderr", async () => { diff --git a/src/tests/hooks/integration.test.ts b/src/tests/hooks/integration.test.ts index 8249ed6..60b15a4 100644 --- a/src/tests/hooks/integration.test.ts +++ b/src/tests/hooks/integration.test.ts @@ -110,7 +110,8 @@ describe.skipIf(isWindows)("Hooks Integration Tests", () => { hooks: [ { type: "command", - command: "echo 'Blocked: write to sensitive file' && exit 2", + command: + "echo 'Blocked: write to sensitive file' >&2 && exit 2", }, ], }, @@ -125,7 +126,7 @@ describe.skipIf(isWindows)("Hooks Integration Tests", () => { ); expect(result.blocked).toBe(true); - expect(result.feedback).toContain("Blocked: write to sensitive file"); + expect(result.feedback[0]).toContain("Blocked: write to sensitive file"); }); test("matches by tool name pattern", async () => { @@ -277,7 +278,7 @@ describe.skipIf(isWindows)("Hooks Integration Tests", () => { hooks: [ { type: "command", - command: "echo 'Denied: dangerous command' && exit 2", + command: "echo 'Denied: dangerous command' >&2 && exit 2", }, ], }, @@ -293,7 +294,7 @@ describe.skipIf(isWindows)("Hooks Integration Tests", () => { ); expect(result.blocked).toBe(true); - expect(result.feedback).toContain("Denied: dangerous command"); + expect(result.feedback[0]).toContain("Denied: dangerous command"); }); test("receives permission type and scope in input", async () => { @@ -599,7 +600,7 @@ describe.skipIf(isWindows)("Hooks Integration Tests", () => { hooks: [ { type: "command", - command: "echo 'Cannot compact now' && exit 2", + command: "echo 'Cannot compact now' >&2 && exit 2", }, ], }, @@ -615,7 +616,7 @@ describe.skipIf(isWindows)("Hooks Integration Tests", () => { ); expect(result.blocked).toBe(true); - expect(result.feedback).toContain("Cannot compact now"); + expect(result.feedback[0]).toContain("Cannot compact now"); }); test("receives context info in input", async () => { @@ -887,7 +888,7 @@ describe.skipIf(isWindows)("Hooks Integration Tests", () => { matcher: "*", hooks: [ { type: "command", command: "echo 'check 1'" }, - { type: "command", command: "echo 'BLOCKED' && exit 2" }, + { type: "command", command: "echo 'BLOCKED' >&2 && exit 2" }, { type: "command", command: "echo 'should not run'" }, ], }, @@ -898,7 +899,7 @@ describe.skipIf(isWindows)("Hooks Integration Tests", () => { expect(result.blocked).toBe(true); expect(result.results).toHaveLength(2); - expect(result.feedback).toContain("BLOCKED"); + expect(result.feedback[0]).toContain("BLOCKED"); }); test("error hooks do not block subsequent hooks", async () => {