fix(auto-approval): avoid heredoc false positives

This commit is contained in:
Hannes Rudolph 2026-01-13 22:55:13 -07:00
parent d689de34f8
commit 9ad34926ed
3 changed files with 83 additions and 3 deletions

View file

@ -0,0 +1,32 @@
import { containsDangerousSubstitution, getCommandDecision } from "../commands"
describe("containsDangerousSubstitution()", () => {
it("does not flag Python assignment grouping '=(' inside heredoc content", () => {
const cmd = [
"python - <<'PY'",
"import json, pathlib",
"data = {'notes': ''}",
"data['notes']=(data.get('notes','')+'; policy set to linked-only by user').lstrip('; ')",
"print(data['notes'])",
"PY",
].join("\n")
expect(containsDangerousSubstitution(cmd)).toBe(false)
})
it("does not flag simple assignment grouping pattern x=(...)", () => {
expect(containsDangerousSubstitution("x=(1+2)")).toBe(false)
})
it("does flag zsh process substitution when used as an argument", () => {
expect(containsDangerousSubstitution("cat =(echo hi)")).toBe(true)
})
})
describe("getCommandDecision()", () => {
it("auto-approves allowed heredoc command when no dangerous substitution is present", () => {
const cmd = ["python - <<'PY'", "x=(1+2)", "print(x)", "PY"].join("\n")
expect(getCommandDecision(cmd, ["python"], [])).toBe("auto_approve")
})
})

View file

@ -46,7 +46,9 @@ export function containsDangerousSubstitution(source: string): boolean {
// Check for zsh process substitution =(...) which executes commands
// =(...) creates a temporary file containing the output of the command, but executes it
const zshProcessSubstitution = /=\([^)]+\)/.test(source)
// Only match when preceded by whitespace or command separators (argument position),
// not when preceded by identifiers/brackets (assignment position like x=(...) or data['key']=(...))
const zshProcessSubstitution = /(^|[\s&|;])=\([^)]+\)/.test(source)
// Check for zsh glob qualifiers with code execution (e:...:)
// Patterns like *(e:whoami:) or ?(e:rm -rf /:) execute commands during glob expansion

View file

@ -2,6 +2,30 @@ import { parse } from "shell-quote"
export type ShellToken = string | { op: string } | { command: string }
type HeredocStart = {
delimiter: string
stripLeadingTabs: boolean
}
function parseHeredocStart(line: string): HeredocStart | null {
// Matches:
// - <<EOF
// - <<'EOF'
// - <<"EOF"
// - <<-EOF (strip leading tabs in terminator line)
//
// Notes:
// - Intentionally minimal: supports common heredoc forms used in Roo tool commands.
// - Delimiter is restricted to [A-Za-z0-9_] to avoid overly broad matches.
const match = line.match(/<<(-)?\s*(?:'([A-Za-z0-9_]+)'|"([A-Za-z0-9_]+)"|([A-Za-z0-9_]+))/)
if (!match) return null
return {
stripLeadingTabs: match[1] === "-",
delimiter: match[2] ?? match[3] ?? match[4] ?? "",
}
}
/**
* Split a command string into individual sub-commands by
* chaining operators (&&, ||, ;, |, or &) and newlines.
@ -23,13 +47,35 @@ export function parseCommand(command: string): string[] {
const lines = command.split(/\r\n|\r|\n/)
const allCommands: string[] = []
for (const line of lines) {
for (let i = 0; i < lines.length; i++) {
const line = lines[i]
// Skip empty lines
if (!line.trim()) {
continue
}
// Process each line through the existing parsing logic
// If this line starts a heredoc, treat the entire heredoc block as a single command.
// The heredoc body lines are not shell commands and should not affect command approval.
const heredocStart = parseHeredocStart(line)
if (heredocStart) {
const blockLines: string[] = [line]
const { delimiter, stripLeadingTabs } = heredocStart
for (i = i + 1; i < lines.length; i++) {
const bodyLine = lines[i]
blockLines.push(bodyLine)
const compareLine = stripLeadingTabs ? bodyLine.replace(/^\t+/, "") : bodyLine
if (compareLine.trimEnd() === delimiter) {
break
}
}
allCommands.push(blockLines.join("\n"))
continue
}
// Process each non-heredoc line through the existing parsing logic
const lineCommands = parseCommandLine(line)
allCommands.push(...lineCommands)
}