fix(commands): correct multi-line quoted command parsing, auto-approval, and malformed-command error surfacing (#483)

* fix(commands): treat multi-line quoted argument as a single command for auto-approval

parseCommand split on every newline before any quote handling, so newlines
inside a quoted argument (e.g. a multi-line script passed to sh -c) were treated
as separate commands. Single-quoted and ANSI-C ($'...') strings were also not
fully masked, leaking placeholders and bogus sub-commands. This defeated
allowlist auto-approval and produced a noisy command-pattern breakdown in the UI.

Mask quoted strings (single, double, ANSI-C) before splitting on unquoted
newlines, using a single left-to-right alternation so a quote of one style
inside the other does not start a spurious match. Genuine unquoted newlines
still split into separate sub-commands.

* fix(commands): mask ANSI-C quoted strings in top-level newline split

Handle $'...' (ANSI-C) quoting in parseCommand's pre-split masking so an
escaped apostrophe inside the quoted body no longer terminates the match
early and leak an embedded newline, which would split a single command into
bogus sub-commands. Add a regression test covering an escaped apostrophe
plus newline inside an ANSI-C argument.

* test(commands): cover shell-quote parse-failure fallback restoration

Mock shell-quote parse() to throw and assert the fallback path restores the
ANSI-C single-quote placeholder, closing the patch-coverage gap on the parse-
failure branch.

* fix(commands): reject unterminated-quote commands from auto-approval

Add findUnterminatedQuote, a quote-aware state-machine scanner that detects a command containing an unclosed quote (a shell syntax error, common in LLM-generated commands with nested quotes). parseCommand now returns such input as a single opaque token instead of splitting on embedded newlines, so a line intended to live inside the unclosed quote cannot surface as an independently auto-approvable sub-command.

The scanner returns { quoteType, openIndex } to support a future execution-layer rejection that surfaces a located error to the model; that pre-execution rejection is intentionally deferred to a follow-up.

* fix(commands): make top-level quote masking comment-aware

Replace the cross-line quote-masking regex in parseCommand() with a state machine (maskTopLevelQuotes) that mirrors findUnterminatedQuote. A quote inside a # comment no longer pairs with a quote on a later line, so a comment can no longer hide a real newline separator and merge two distinct commands.

* fix(commands): add heredoc and locale-quote support to command parser; fix pattern extractor

- parseCommand: mask heredocs (<<, <<-, all delimiter quoting styles) as single
  atomic tokens before newline splitting; unterminated heredocs returned as opaque token
- parseCommand: add locale-quote ($"...") support alongside existing ANSI-C ($'...')
- findUnterminatedQuote: extend QuoteType with "locale" and "heredoc" variants
- extractPatternsFromCommand (webview): pre-split via parseCommand before shell-quote
  tokenization, preventing spurious EOF/body-line/operator tokens in allow/deny selector
- Update changeset to cover all three fix areas

* fix(commands): handle herestring (<<<) in parse-command and findUnterminatedQuote

- Add explicit <<< passthrough in maskTopLevelQuotes: emit all three < chars
  verbatim and advance i by 3 so the second < does not re-trigger the heredoc
  branch on the next iteration
- Same fix in findUnterminatedQuote for the same root cause
- Add herestring test suite covering single-line, multi-command split, and
  single-quoted/ANSI-C quoted multiline word cases

* fix(commands): address CodeRabbit review comments

- findUnterminatedQuote: track stripTabs for <<- so tab-stripping only
  applies when the heredoc opener used <<- (consistent with maskTopLevelQuotes)
- findUnterminatedQuote: add doubleIsLocale flag so an unterminated $"..."
  region returns quoteType "locale" instead of "double"
- findUnterminatedQuote: add tests for unterminated locale quote and balanced
  <<- with indented terminator
- CommandExecution: exclude multi-line opaque tokens (heredoc bodies,
  unterminated quotes) from the raw-command pattern set so body-line words
  never surface as independently approvable patterns
- CommandExecution.spec: strengthen fragment assertions to use per-fragment
  substring checks, exposing the CommandExecution leak bug

* refactor(commands): consolidate quote scanners; add ParseResult; remove dead arrayIndexing

- Unify findUnterminatedQuote and maskTopLevelQuotes under a single
  scanTopLevelQuotes state machine -- one pass, no duplicate quoting
  logic between the two functions
- Change parseCommand return type from string[] to ParseResult
  { commands: string[]; parseError: UnterminatedQuote | null } so
  callers can distinguish a parse error from a normal single-command
  result without a separate findUnterminatedQuote call
- getCommandDecision reads parseError from parseCommand instead of
  calling findUnterminatedQuote independently; returns the new
  malformed_command CommandDecision variant for shell syntax errors
- Remove dead arrayIndexing bucket and __ARRAY_N__ restore (never
  populated; caused undefined return when input contained the literal
  string __ARRAY_0__)
- Add safety-boundary test: unterminated-quote commands return
  malformed_command even when prefix is allowlisted, wildcard, or
  the exact command string is on the allowlist
- Add findUnterminatedQuote test: closed quote followed by # comment
  with apostrophe returns null (not an open region)
- Update all parseCommand call sites to destructure .commands

* feat(commands): add message field to UnterminatedQuote; surface malformed command as toolError in ExecuteCommandTool

* feat(commands): add error status to CommandExecutionStatus; render error card for malformed commands

* i18n: add malformedCommand translation to all 17 non-English locales

* fix: guard parseCommand against literal placeholder token collisions

Commands containing text like __QUOTE_0__ or __SQUOTE_0__ would be
silently corrupted by restorePlaceholders() -- the restore regexes
would match the literal tokens and substitute array entries (or
'undefined') in their place.

Fix: pre-escape __ -> \x00 in parseCommand before any masking begins,
then post-unescape \x00 -> __ across all output commands at the return.
\x00 (null byte, U+0000) is safe as a sentinel because the OS
terminates command strings at the first \x00, so it can never appear
in real shell command text.

Adds one regression test covering all eight internal placeholder
namespaces.

Author: awschmeder

.changeset/fix-multiline-quoted-command-parsing.md

@@ -0,0 +1,17 @@
+---
+"zoo-code": patch
+---
+
+Fix command auto-approval for multi-line shell constructs that must be treated as a single command.
+
+**Quoted multi-line arguments** (`sh -c '...'`, `sh -c $'...'`, `sh -c "..."`): the parser previously split on every newline before handling quotes, so newlines inside a quoted argument were treated as separate commands, defeating allowlist auto-approval. Single-quoted, ANSI-C (`$'...'`), and double-quoted strings are now masked before the newline split so embedded newlines and operators stay within their command.
+
+**Heredocs** (`<< EOF`, `<< 'EOF'`, `<< "EOF"`, `<<- EOF`): the entire heredoc -- opener line, body, and terminator -- is now treated as a single quoted region. Body lines are not split into independent sub-commands. All heredoc delimiter quoting styles (unquoted, single-quoted, double-quoted, backslash-escaped) are supported. An unterminated heredoc (missing terminator) is treated as malformed and returned as a single opaque token.
+
+**Locale quoting** (`$"..."`): treated as a distinct token analogous to ANSI-C quoting, preserving the `$` prefix and preventing the double-quote handler from stripping it.
+
+Quote masking is comment-aware: a quote character inside a `#` comment is not paired with a quote on a later line, so a comment cannot hide a real newline separator and merge two distinct commands. Commands with an unterminated quote are detected with a quote-aware scanner and returned as a single opaque token, preventing a line inside the unclosed quote from surfacing as an independently auto-approvable command. Genuine unquoted newlines still split into separate sub-commands, each of which must be allowlisted for auto-approval.
+
+**Pattern selector (UI)**: the command pattern breakdown shown after execution now uses the same heredoc- and quote-aware parser (`parseCommand`) before extracting patterns, so an unterminated or terminated heredoc no longer produces spurious tokens like `EOF`, body-line words, or `<<` fragments in the allow/deny selector.
+
+Note: this change only prevents *auto-approval* of fragments from a malformed command; it does not reject malformed commands before execution, which will be addressed in a separate PR to keep the scope focused here.

packages/types/src/terminal.ts

@@ -29,6 +29,11 @@ export const commandExecutionStatusSchema = z.discriminatedUnion("status", [
 		executionId: z.string(),
 		status: z.literal("timeout"),
 	}),
+	z.object({
+		executionId: z.string(),
+		status: z.literal("error"),
+		message: z.string().optional(),
+	}),
 ])
 
 export type CommandExecutionStatus = z.infer<typeof commandExecutionStatusSchema>

src/core/auto-approval/__tests__/commands.spec.ts

@@ -99,3 +99,63 @@ describe("getCommandDecision — integration with dangerous substitution checks"
 		expect(getCommandDecision('echo "${var@P}"', allowedCommands)).toBe("ask_user")
 	})
 })
+
+describe("getCommandDecision — multi-line script wrapped in a quoted argument", () => {
+	// A wrapper command (e.g. sh -c '...') carrying a multi-line script as a
+	// single quoted argument must be treated as one command. The embedded
+	// newlines belong to the quoted string and must not be mistaken for a
+	// multi-statement sequence that would defeat allowlist auto-approval.
+	const wrappedSingleQuoted = [
+		`sh -c 'kubectl exec pod -- python3 -c "`,
+		`import urllib.request`,
+		`url = \\"http://127.0.0.1:49527/\\"`,
+		`try:`,
+		`    with urllib.request.urlopen(url, timeout=10) as r:`,
+		`        print(r.status)`,
+		`except Exception as e:`,
+		`    print(\\"fetch failed:\\", e)`,
+		`"'`,
+	].join("\n")
+
+	it("auto-approves a multi-line single-quoted script when the wrapper prefix is allowed", () => {
+		expect(getCommandDecision(wrappedSingleQuoted, ["sh"])).toBe("auto_approve")
+	})
+
+	it("auto-approves a multi-line double-quoted script when the wrapper prefix is allowed", () => {
+		const wrappedDoubleQuoted = 'sh -c "echo line1\necho line2"'
+		expect(getCommandDecision(wrappedDoubleQuoted, ["sh"])).toBe("auto_approve")
+	})
+
+	it("asks user when the wrapper prefix is not in the allowlist", () => {
+		expect(getCommandDecision(wrappedSingleQuoted, ["git"])).toBe("ask_user")
+	})
+
+	it("still splits genuine multi-statement scripts and asks when a statement is not allowed", () => {
+		// Real unquoted newlines separate independent statements; each must be on
+		// the allowlist for auto-approval to engage.
+		const multiStatement = "echo hello\nrm -rf /tmp/x"
+		expect(getCommandDecision(multiStatement, ["echo"])).toBe("ask_user")
+	})
+
+	it("auto-approves a genuine multi-statement script when every statement is allowed", () => {
+		const multiStatement = "echo hello\nls -la"
+		expect(getCommandDecision(multiStatement, ["echo", "ls"])).toBe("auto_approve")
+	})
+
+	it("auto-approves an ANSI-C quoted ($'...') multi-line argument when the wrapper prefix is allowed", () => {
+		const ansiC = "sh -c $'echo 1\necho 2'"
+		expect(getCommandDecision(ansiC, ["sh"])).toBe("auto_approve")
+	})
+
+	it("returns malformed_command for a command with an unterminated quote regardless of allowlist", () => {
+		// An unterminated quote is a shell syntax error. Even if the leading word
+		// is on the allowlist (or the allowlist is the wildcard), or the exact
+		// full command string is listed, the command must not be auto-approved --
+		// the shell would report a syntax error and any prefix that ran before
+		// the error could have unintended side effects.
+		const malformed = "sh -c 'echo a\necho b"
+		expect(getCommandDecision(malformed, ["sh"])).toBe("malformed_command")
+		expect(getCommandDecision(malformed, ["*"])).toBe("malformed_command")
+		expect(getCommandDecision(malformed, [malformed])).toBe("malformed_command")
+	})
+})

src/core/auto-approval/commands.ts

@@ -1,5 +1,6 @@
 import { parseCommand } from "../../shared/parse-command"
 
+
 /**
  * Detect dangerous parameter substitutions that could lead to command execution.
  * These patterns are never auto-approved and always require explicit user approval.
@@ -204,7 +205,7 @@ export function isAutoDeniedSingleCommand(
 /**
  * Command approval decision types
  */
-export type CommandDecision = "auto_approve" | "auto_deny" | "ask_user"
+export type CommandDecision = "auto_approve" | "auto_deny" | "ask_user" | "malformed_command"
 
 /**
  * Unified command validation that implements the longest prefix match rule.
@@ -224,6 +225,7 @@ export type CommandDecision = "auto_approve" | "auto_deny" | "ask_user"
  * - `"auto_approve"`: All sub-commands are explicitly allowed and no dangerous patterns detected
  * - `"auto_deny"`: At least one sub-command is explicitly denied
  * - `"ask_user"`: Mixed or no matches found, requires user decision, or contains dangerous patterns
+ * - `"malformed_command"`: Command contains an unterminated quote -- a shell syntax error that must not be auto-approved
  *
  * **Examples:**
  * ```typescript
@@ -262,8 +264,19 @@ export function getCommandDecision(
 		return "auto_approve"
 	}
 
-	// Parse into sub-commands (split by &&, ||, ;, |)
-	const subCommands = parseCommand(command)
+	// Parse into sub-commands (split by &&, ||, ;, |). parseCommand also
+	// detects shell syntax errors (unterminated quotes, unclosed heredocs) and
+	// returns a non-null parseError in that case.
+	const { commands: subCommands, parseError } = parseCommand(command)
+
+	// Reject commands with a shell syntax error. An unterminated quote means
+	// the shell would report a parse error; in a compound command it may
+	// partially execute the well-formed prefix before aborting. Returning a
+	// distinct decision lets callers surface a useful message to the agent
+	// rather than silently presenting the command for user approval.
+	if (parseError !== null) {
+		return "malformed_command"
+	}
 
 	// Check each sub-command and collect decisions
 	const decisions: CommandDecision[] = subCommands.map((cmd) => {

src/core/tools/ExecuteCommandTool.ts

@@ -12,6 +12,7 @@ import { Task } from "../task/Task"
 import { ToolUse, ToolResponse } from "../../shared/tools"
 import { formatResponse } from "../prompts/responses"
 import { unescapeHtmlEntities } from "../../utils/text-normalization"
+import { parseCommand } from "../../shared/parse-command"
 import {
 	ExitCodeDetails,
 	RooTerminalCallbacks,
@@ -86,6 +87,21 @@ export class ExecuteCommandTool extends BaseTool<"execute_command"> {
 
 			task.consecutiveMistakeCount = 0
 
+			// Detect shell syntax errors (unterminated quotes, unclosed heredocs) before
+			// presenting the command for approval. Surfacing this as a tool error gives
+			// the agent a precise, actionable message so it can retry with a corrected
+			// command, rather than receiving a generic denial from the approval dialog.
+			const { parseError } = parseCommand(canonicalCommand)
+			if (parseError !== null) {
+				const executionId = task.lastMessageTs?.toString() ?? Date.now().toString()
+				const provider = await task.providerRef.deref()
+				const errorStatus: CommandExecutionStatus = { executionId, status: "error", message: parseError.message }
+				provider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(errorStatus) })
+				task.didToolFailInCurrentTurn = true
+				pushToolResult(formatResponse.toolError(parseError.message))
+				return
+			}
+
 			const didApprove = await askApproval("command", canonicalCommand)
 
 			if (!didApprove) {