Skip to content

Commit f5772f4

Browse files
committed
hooks: strip heredoc bodies before dangerous-command scan
Closes #100. The dangerous-command blocker regex-matches against the raw `command` string. Bodies of bash heredocs (cat > note.md <<EOF ... EOF) are data, not commands, so a journal entry or repro that quotes a forbidden phrase like "git push --force" or the docker-compose teardown invocation was tripping the blocker and refusing benign Bash invocations. Strip heredoc bodies from the scannable text before the pattern loop: /<<-?\s*['"]?(\w+)['"]?\n[\s\S]*?\n[ \t]*\1\s*$/gm Handles `<<DELIM`, `<<-DELIM`, `<<"DELIM"`, `<<'DELIM'`. The `[ \t]*` before the closing delimiter covers `<<-` heredocs where bash strips leading tabs from the terminator line. Real destructive commands outside the heredoc still match (verified by test). Tests added (hooks.test.ts): - allows heredoc body that names a forbidden phrase as data - allows heredoc with quoted delimiter and dangerous-looking body - blocks real dangerous command outside the heredoc - blocks dash-stripped heredoc opener with real dangerous trailing command Stash-bisect: 13/2 fail without the fix, 15/0 pass with the fix. This is option 2 from #100 (heredoc-strip, no new dep). Echo/printf false-positives still exist; the long-term option 1 (shell-token-aware scan) is left for a future, larger PR. This narrow patch addresses the highest-frequency case.
1 parent ff74713 commit f5772f4

2 files changed

Lines changed: 92 additions & 1 deletion

File tree

src/agent/__tests__/hooks.test.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,4 +210,81 @@ describe("createDangerousCommandBlocker", () => {
210210

211211
expect(result).toEqual({ continue: true });
212212
});
213+
214+
test("allows heredoc body that names a forbidden phrase as data", async () => {
215+
const hook = createDangerousCommandBlocker();
216+
const callback = hook.hooks[0];
217+
218+
const result = await callback(
219+
makeHookInput({
220+
hook_event_name: "PreToolUse",
221+
tool_name: "Bash",
222+
tool_input: {
223+
command: "cat > note.md <<EOF\nReminder: git push --force is forbidden\nEOF",
224+
},
225+
}),
226+
undefined,
227+
{ signal: new AbortController().signal },
228+
);
229+
230+
expect(result).toEqual({ continue: true });
231+
});
232+
233+
test("allows heredoc with quoted delimiter and dangerous-looking body", async () => {
234+
const hook = createDangerousCommandBlocker();
235+
const callback = hook.hooks[0];
236+
237+
const result = await callback(
238+
makeHookInput({
239+
hook_event_name: "PreToolUse",
240+
tool_name: "Bash",
241+
tool_input: {
242+
command:
243+
"node <<'NODEEOF'\n// repro: docker compose down should not be triggered\nconsole.log('ok');\nNODEEOF",
244+
},
245+
}),
246+
undefined,
247+
{ signal: new AbortController().signal },
248+
);
249+
250+
expect(result).toEqual({ continue: true });
251+
});
252+
253+
test("blocks real dangerous command outside the heredoc", async () => {
254+
const hook = createDangerousCommandBlocker();
255+
const callback = hook.hooks[0];
256+
257+
const result = await callback(
258+
makeHookInput({
259+
hook_event_name: "PreToolUse",
260+
tool_name: "Bash",
261+
tool_input: {
262+
command: "cat > note.md <<EOF\nharmless body\nEOF\ngit push --force origin main",
263+
},
264+
}),
265+
undefined,
266+
{ signal: new AbortController().signal },
267+
);
268+
269+
expect(result).toHaveProperty("decision", "block");
270+
});
271+
272+
test("blocks dash-stripped heredoc opener with real dangerous trailing command", async () => {
273+
const hook = createDangerousCommandBlocker();
274+
const callback = hook.hooks[0];
275+
276+
const result = await callback(
277+
makeHookInput({
278+
hook_event_name: "PreToolUse",
279+
tool_name: "Bash",
280+
tool_input: {
281+
command: "cat <<-EOF\n\tharmless body\n\tEOF\nrm -rf /",
282+
},
283+
}),
284+
undefined,
285+
{ signal: new AbortController().signal },
286+
);
287+
288+
expect(result).toHaveProperty("decision", "block");
289+
});
213290
});

src/agent/hooks.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,19 @@ const DANGEROUS_COMMANDS: { pattern: RegExp; label: string }[] = [
2525
{ pattern: /kill\s+-9\s+1(\s|$)/, label: "kill init" },
2626
];
2727

28+
// Heredoc bodies are data, not commands. Stripping them before the
29+
// dangerous-command scan prevents prose, repros, and journal entries
30+
// that quote a forbidden phrase from tripping the blocker.
31+
// Matches `<<DELIM`, `<<-DELIM`, `<<"DELIM"`, `<<'DELIM'`; replaces the
32+
// payload (and the closing delimiter line) with an empty string and
33+
// keeps the surrounding command text so a real destructive command
34+
// outside the heredoc still matches. The optional `[ \t]*` before the
35+
// closing delimiter handles `<<-` heredocs where Bash strips leading
36+
// tabs from the terminator at parse time.
37+
function stripHeredocBodies(command: string): string {
38+
return command.replace(/<<-?\s*['"]?(\w+)['"]?\n[\s\S]*?\n[ \t]*\1\s*$/gm, "");
39+
}
40+
2841
export function createFileTracker(): {
2942
hook: HookCallbackMatcher;
3043
getTrackedFiles: () => string[];
@@ -59,8 +72,9 @@ export function createDangerousCommandBlocker(): HookCallbackMatcher {
5972
if (input.hook_event_name !== "PreToolUse") return { continue: true };
6073
const command = (input.tool_input as Record<string, unknown>)?.command;
6174
if (typeof command === "string") {
75+
const scannable = stripHeredocBodies(command);
6276
for (const { pattern, label } of DANGEROUS_COMMANDS) {
63-
if (pattern.test(command)) {
77+
if (pattern.test(scannable)) {
6478
return {
6579
decision: "block",
6680
reason: `Blocked dangerous command: "${label}"`,

0 commit comments

Comments
 (0)