[Fix] Chained commands next to a multi-line quoted argument skip the allowlist check

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

@@ -160,6 +160,71 @@ describe("getCommandDecision — multi-line script wrapped in a quoted argument"
 	})
 })
 
+describe("getCommandDecision — chained command next to a multi-line quoted argument", () => {
+	// Every command in the chain must match the allowlist on its own, also when
+	// one of them has a quoted argument that spans lines.
+	const chained = `cd cmd-test && node -e "\nconsole.log('hello world');\n"`
+	const reversed = `node -e "\nconsole.log('hello world');\n" && cd cmd-test`
+
+	it.each([chained, reversed])("asks user when only cd is allowed: %j", (command) => {
+		expect(getCommandDecision(command, ["cd"])).toBe("ask_user")
+	})
+
+	it.each([chained, reversed])("auto-approves when every command is allowed: %j", (command) => {
+		expect(getCommandDecision(command, ["cd", "node"])).toBe("auto_approve")
+	})
+
+	it.each([chained, reversed])("auto-denies when the chained command is denied: %j", (command) => {
+		expect(getCommandDecision(command, ["cd"], ["node"])).toBe("auto_deny")
+	})
+})
+
+describe("getCommandDecision — # inside a word and glob patterns", () => {
+	it.each(["echo safe#blocked && node -v", "echo safe#blocked 'a\nb' && node -v"])(
+		"applies a deny rule that contains # and checks the chained command: %j",
+		(command) => {
+			expect(getCommandDecision(command, ["echo"], ["echo safe#blocked"])).toBe("auto_deny")
+			expect(getCommandDecision(command, ["echo"])).toBe("ask_user")
+		},
+	)
+
+	it.each(["rm *", "rm * 'a\nb'"])("applies a deny rule for a glob: %j", (command) => {
+		expect(getCommandDecision(command, ["rm"], ["rm *"])).toBe("auto_deny")
+	})
+
+	it("auto-approves a glob argument when the command is allowed", () => {
+		expect(getCommandDecision("ls *.ts && echo x", ["ls", "echo"])).toBe("auto_approve")
+	})
+})
+
+describe("getCommandDecision — outer word attached to a command substitution", () => {
+	it.each(["reboot$(printf 'x')", "reboot$(printf '\n')"])(
+		"checks the outer command when only the inner command is allowed: %j",
+		(command) => {
+			expect(getCommandDecision(command, ["printf"])).toBe("ask_user")
+			expect(getCommandDecision(command, ["printf"], ["reboot"])).toBe("auto_deny")
+			expect(getCommandDecision(command, ["printf", "reboot"])).toBe("auto_approve")
+		},
+	)
+
+	it.each(["$(printf re)boot", "$(printf re)boot 'a\nb'", "`printf re`boot 'a\nb'"])(
+		"asks the user when a substitution is part of the command name: %j",
+		(command) => {
+			expect(getCommandDecision(command, ["printf", "boot"])).toBe("ask_user")
+			expect(getCommandDecision(command, ["printf", "boot"], ["reboot"])).toBe("ask_user")
+		},
+	)
+
+	it.each(["echo*$(reboot)", "echo*$(reboot) 'a\nb'", "ls *$(reboot).ts"])(
+		"checks a substitution inside a glob: %j",
+		(command) => {
+			expect(getCommandDecision(command, ["echo", "ls"])).toBe("ask_user")
+			expect(getCommandDecision(command, ["echo", "ls"], ["reboot"])).toBe("auto_deny")
+			expect(getCommandDecision(command, ["echo", "ls", "reboot"])).toBe("auto_approve")
+		},
+	)
+})
+
 describe("getCommandDecisionDetailed", () => {
 	it("names the offending sub-command and matched denied prefix on chained commands", () => {
 		const result = getCommandDecisionDetailed("git status && rm x && npm test", ["git", "npm"], ["rm"])

src/shared/__tests__/parse-command.spec.ts

@@ -157,6 +157,72 @@ describe("parseCommand", () => {
 		})
 	})
 
+	describe("chain operators on a line with a multi-line quoted argument", () => {
+		it.each([
+			{
+				input: `cd cmd-test && node -e "\nconsole.log('hello world');\n"`,
+				expected: ["cd cmd-test", `node -e "\nconsole.log('hello world');\n"`],
+			},
+			{
+				input: `node -e "\nconsole.log(1)\n" || echo failed`,
+				expected: [`node -e "\nconsole.log(1)\n"`, "echo failed"],
+			},
+			{
+				input: "cd a; node -e '\nx\n' | tee out.txt",
+				expected: ["cd a", "node -e '\nx\n'", "tee out.txt"],
+			},
+			{
+				input: 'git commit -m "line 1\nline 2" && git push',
+				expected: ['git commit -m "line 1\nline 2"', "git push"],
+			},
+			{
+				input: `echo $'first\nline' && printf $"second\r\nline"`,
+				expected: [`echo $'first\nline'`, `printf $"second\r\nline"`],
+			},
+			{
+				input: "echo 'first\nline' & echo done",
+				expected: ["echo 'first\nline'", "echo done"],
+			},
+			{
+				input: "echo 'first\nline';ls",
+				expected: ["echo 'first\nline'", "ls"],
+			},
+		])("splits $input into each chained command", ({ input, expected }) => {
+			expect(parseCommand(input).commands).toEqual(expected)
+		})
+
+		it("restores each quote by its full index when a line has more than ten quotes", () => {
+			const singleLineQuotes = Array.from({ length: 10 }, (_, i) => `'q${i}'`).join(" ")
+			const input = `echo ${singleLineQuotes} 'q10\nend' && ls`
+			expect(parseCommand(input).commands).toEqual([`echo ${singleLineQuotes} 'q10\nend'`, "ls"])
+		})
+
+		it("keeps operators inside the multi-line quoted argument", () => {
+			const input = `sh -c "cd a && node x\necho done" && ls`
+			expect(parseCommand(input).commands).toEqual([`sh -c "cd a && node x\necho done"`, "ls"])
+		})
+	})
+
+	describe("# inside a word and glob patterns", () => {
+		it.each([
+			{ input: "echo safe#blocked && node -v", expected: ["echo safe#blocked", "node -v"] },
+			{ input: "echo safe#blocked 'a\nb' && node -v", expected: ["echo safe#blocked 'a\nb'", "node -v"] },
+			{ input: "curl http://x/#frag && node y", expected: ["curl http://x/#frag", "node y"] },
+			{ input: "rm *", expected: ["rm *"] },
+			{ input: "rm * 'a\nb'", expected: ["rm * 'a\nb'"] },
+			{ input: "ls *.ts && echo x", expected: ["ls *.ts", "echo x"] },
+		])("keeps the word text in $input", ({ input, expected }) => {
+			expect(parseCommand(input).commands).toEqual(expected)
+		})
+
+		it.each([
+			{ input: "echo hi # comment && rm x", expected: ["echo hi"] },
+			{ input: "echo hi 'a\nb' # comment && rm x", expected: ["echo hi 'a\nb'"] },
+		])("drops a comment that starts a word in $input", ({ input, expected }) => {
+			expect(parseCommand(input).commands).toEqual(expected)
+		})
+	})
+
 	describe("real-world wrapped multi-line script (regression)", () => {
 		it("treats a wrapper command with an embedded multi-line script as a single command", () => {
 			const input = [
@@ -183,6 +249,19 @@ describe("parseCommand", () => {
 			const { commands: result } = parseCommand("echo $(whoami)")
 			expect(result).toContain("whoami")
 		})
+
+		it.each([
+			{ input: "reboot$(printf 'x')", expected: ["printf 'x'", "reboot$(printf 'x')"] },
+			{ input: "reboot$(printf '\n')", expected: ["printf '\n'", "reboot$(printf '\n')"] },
+			{ input: "$(printf re)boot", expected: ["printf re", "$(printf re)boot"] },
+			{ input: "$(printf re)boot 'a\nb'", expected: ["printf re", "$(printf re)boot 'a\nb'"] },
+			{ input: "`printf re`boot", expected: ["printf re", "$(printf re)boot"] },
+			{ input: "echo*$(reboot)", expected: ["reboot", "echo*$(reboot)"] },
+			{ input: "ls *$(reboot).ts 'a\nb'", expected: ["reboot", "ls *$(reboot).ts 'a\nb'"] },
+			{ input: "echo $(a)$(b)", expected: ["echo", "a", "b"] },
+		])("keeps the outer word and every substitution in $input", ({ input, expected }) => {
+			expect(parseCommand(input).commands).toEqual(expected)
+		})
 	})
 
 	describe("shell-quote parse-failure fallback", () => {
@@ -201,6 +280,13 @@ describe("parseCommand", () => {
 			expect(result.join(" ")).not.toContain("SQUOTE")
 			expect(result.join(" ")).not.toContain("__")
 		})
+
+		it("splits outer operators and keeps operators inside a multi-line quote in the fallback path", () => {
+			forceShellQuoteFailure = true
+
+			const { commands: result } = parseCommand(`cd a && node -e "x && y\nz | w" ; ls`)
+			expect(result).toEqual(["cd a", `node -e "x && y\nz | w"`, "ls"])
+		})
 	})
 
 	describe("malformed input with unterminated quotes", () => {

src/shared/parse-command.ts

@@ -1,6 +1,7 @@
 import { parse } from "shell-quote"
 
-export type ShellToken = string | { op: string } | { command: string }
+export type ShellToken =
+	string | { op: "glob"; pattern: string } | { op: string } | { command: string } | { comment: string }
 
 /**
  * The style of quoting that opened a region.
@@ -516,14 +517,18 @@
 		// Restore top-level quote placeholders before per-line parsing so that
 		// parseCommandLine sees the original quoted content and can apply its own
 		// masking for operator splitting.
-		const restoredLine = line.replace(/__TOPLEVEL_QUOTE_(\d+)__/g, (_, i) => topLevelQuotes[parseInt(i)])
+		const restoreQuotes = (text: string) =>
+			text.replace(/__TOPLEVEL_QUOTE_(\d+)__/g, (_, i) => topLevelQuotes[parseInt(i)])
+		const restoredLine = restoreQuotes(line)
 
 		// If the restored line contains embedded newlines it means a top-level
-		// quote (e.g. a heredoc) spanned multiple lines. The entire restored
-		// string is a single atomic command -- passing it through parseCommandLine
-		// would let shell-quote split on the embedded newlines and << operators.
+		// quote (e.g. a heredoc) spanned multiple lines. Passing the restored text
+		// to parseCommandLine would let shell-quote split on the embedded newlines
+		// and << operators, so split the masked line instead and restore each
+		// quote inside its own sub-command. Each chained command must reach the
+		// allowlist check on its own.
 		if (restoredLine.includes("\n")) {
-			allCommands.push(restoredLine)
+			allCommands.push(...parseCommandLine(line).map(restoreQuotes))
 			continue
 		}
 
@@ -650,6 +655,12 @@
 		return `__QUOTE_${quotes.length - 1}__`
 	})
 
+	// The shell starts a comment only at a `#` that begins a word. shell-quote
+	// starts one at any `#`, so a word such as `safe#x` would hide the rest of
+	// the line, including chained commands, from the allowlist check. Mask a `#`
+	// that follows a word character so shell-quote keeps it as text.
+	processedCommand = processedCommand.replace(/(?<=[^\s;&|()<>])#/g, "__HASH__")
+
 	let tokens: ShellToken[]
 	try {
 		tokens = parse(processedCommand) as ShellToken[]
@@ -681,6 +692,26 @@
 	const commands: string[] = []
 	let currentCommand: string[] = []
 
+	const addWord = (word: string) => {
+		const subshellMatches = [...word.matchAll(/__SUBSH_(\d+)__/g)]
+		if (subshellMatches.length === 0) {
+			currentCommand.push(word)
+			return
+		}
+		if (word.replace(/__SUBSH_\d+__/g, "")) {
+			// The substitution output is part of this word, for example
+			// `$(printf re)boot` runs `reboot`. Keep the substitution text in the
+			// word: the allowlist check must not see only the literal part.
+			currentCommand.push(word.replace(/__SUBSH_(\d+)__/g, (_, i) => `$(${subshells[parseInt(i)]})`))
+		} else if (currentCommand.length > 0) {
+			commands.push(currentCommand.join(" "))
+			currentCommand = []
+		}
+		for (const match of subshellMatches) {
+			commands.push(subshells[parseInt(match[1])])
+		}
+	}
+
 	for (const token of tokens) {
 		if (typeof token === "object" && "op" in token) {
 			// Chain operator - split command
@@ -689,22 +720,16 @@
 					commands.push(currentCommand.join(" "))
 					currentCommand = []
 				}
+			} else if ("pattern" in token) {
+				// shell-quote returns a glob as { op: "glob", pattern }. Keep the glob
+				// text (for example `*`) so that deny rules can match it.
+				addWord(token.pattern)
 			} else {
 				// Other operators (>) are part of the command
 				currentCommand.push(token.op)
 			}
 		} else if (typeof token === "string") {
-			// Check if it's a subshell placeholder
-			const subshellMatch = token.match(/__SUBSH_(\d+)__/)
-			if (subshellMatch) {
-				if (currentCommand.length > 0) {
-					commands.push(currentCommand.join(" "))
-					currentCommand = []
-				}
-				commands.push(subshells[parseInt(subshellMatch[1])])
-			} else {
-				currentCommand.push(token)
-			}
+			addWord(token)
 		}
 	}
 
@@ -755,5 +780,6 @@
 	// Restore variable references
 	result = result.replace(/__VAR_(\d+)__/g, (_, i) => variables[parseInt(i)])
 	result = result.replace(/__SUBSH_(\d+)__/g, (_, i) => subshells[parseInt(i)])
+	result = result.replace(/__HASH__/g, "#")
 	return result
 }