From 1ce65ceae0ad2929c3cfcac71328b7669b0836de Mon Sep 17 00:00:00 2001 From: ooadcsulb22 Date: Mon, 9 Nov 2026 19:28:46 -0800 Subject: [PATCH 1/4] fix(auto-approval): split chained commands on lines with multi-line quotes A line that contained a quoted argument with embedded newlines went into the parse result as one command, so only its first sub-command reached the allowlist check. Split the masked line on chain operators and restore the quotes in each sub-command. Closes #2039 Co-authored-by: Cursor --- .../auto-approval/__tests__/commands.spec.ts | 19 +++++++++++++ src/shared/__tests__/parse-command.spec.ts | 28 +++++++++++++++++++ src/shared/parse-command.ts | 14 ++++++---- 3 files changed, 56 insertions(+), 5 deletions(-) diff --git a/src/core/auto-approval/__tests__/commands.spec.ts b/src/core/auto-approval/__tests__/commands.spec.ts index ade53c2b20..ad191473c7 100644 --- a/src/core/auto-approval/__tests__/commands.spec.ts +++ b/src/core/auto-approval/__tests__/commands.spec.ts @@ -160,6 +160,25 @@ 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("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"]) diff --git a/src/shared/__tests__/parse-command.spec.ts b/src/shared/__tests__/parse-command.spec.ts index fa41d31c6c..ee29c40c5a 100644 --- a/src/shared/__tests__/parse-command.spec.ts +++ b/src/shared/__tests__/parse-command.spec.ts @@ -157,6 +157,34 @@ 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"], + }, + ])("splits $input into each chained command", ({ input, expected }) => { + expect(parseCommand(input).commands).toEqual(expected) + }) + + 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("real-world wrapped multi-line script (regression)", () => { it("treats a wrapper command with an embedded multi-line script as a single command", () => { const input = [ diff --git a/src/shared/parse-command.ts b/src/shared/parse-command.ts index 4195d99d4e..992a136c0d 100644 --- a/src/shared/parse-command.ts +++ b/src/shared/parse-command.ts @@ -516,14 +516,18 @@ export function parseCommand(command: string): ParseResult { // 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 } From 8665401468a4ad8063982680ebd6dd0aeee41e8b Mon Sep 17 00:00:00 2001 From: ooadcsulb22 Date: Mon, 9 Nov 2026 19:53:45 -0800 Subject: [PATCH 2/4] fix(auto-approval): keep # inside words and glob text in parsed commands shell-quote starts a comment at any #, so a word such as safe#x dropped the rest of the line, including chained commands, from the allowlist check. shell-quote also returns globs as an op token, so rm * became rm glob and a deny rule for rm * did not match. Both gaps also affect single-line commands. Add tests for indexed quote restoration with more than ten quotes, the & operator, ANSI-C and locale quotes, and the # and glob cases. Co-authored-by: Cursor --- .../auto-approval/__tests__/commands.spec.ts | 18 +++++++++ src/shared/__tests__/parse-command.spec.ts | 38 +++++++++++++++++++ src/shared/parse-command.ts | 13 ++++++- 3 files changed, 68 insertions(+), 1 deletion(-) diff --git a/src/core/auto-approval/__tests__/commands.spec.ts b/src/core/auto-approval/__tests__/commands.spec.ts index ad191473c7..954c2efcb6 100644 --- a/src/core/auto-approval/__tests__/commands.spec.ts +++ b/src/core/auto-approval/__tests__/commands.spec.ts @@ -179,6 +179,24 @@ describe("getCommandDecision — chained command next to a multi-line quoted arg }) }) +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("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"]) diff --git a/src/shared/__tests__/parse-command.spec.ts b/src/shared/__tests__/parse-command.spec.ts index ee29c40c5a..8fdfadad5f 100644 --- a/src/shared/__tests__/parse-command.spec.ts +++ b/src/shared/__tests__/parse-command.spec.ts @@ -175,16 +175,54 @@ describe("parseCommand", () => { 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 = [ diff --git a/src/shared/parse-command.ts b/src/shared/parse-command.ts index 992a136c0d..c1468b70c1 100644 --- a/src/shared/parse-command.ts +++ b/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. @@ -654,6 +655,12 @@ function parseCommandLine(command: string): string[] { 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[] @@ -693,6 +700,9 @@ function parseCommandLine(command: string): string[] { commands.push(currentCommand.join(" ")) currentCommand = [] } + } else if (token.op === "glob" && "pattern" in token) { + // Keep the glob text (for example `*`) so that deny rules can match it. + currentCommand.push(token.pattern) } else { // Other operators (>) are part of the command currentCommand.push(token.op) @@ -759,5 +769,6 @@ function restorePlaceholders( // 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 } From f44e53c4698afa7b2e265923d1ea882a9f109d75 Mon Sep 17 00:00:00 2001 From: ooadcsulb22 Date: Mon, 9 Nov 2026 20:16:11 -0800 Subject: [PATCH 3/4] fix(auto-approval): keep the outer word next to a command substitution A word such as reboot$(printf x) gave only the substituted command, so the allowlist check did not see reboot. Keep the text outside the substitution in the outer command, and extract every substitution in the word. Simplify the glob check so that each condition is testable, and add a fallback-path test for a chain with a multi-line quote. Co-authored-by: Cursor --- .../auto-approval/__tests__/commands.spec.ts | 11 +++++++++ src/shared/__tests__/parse-command.spec.ts | 15 ++++++++++++ src/shared/parse-command.ts | 23 +++++++++++++------ 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/src/core/auto-approval/__tests__/commands.spec.ts b/src/core/auto-approval/__tests__/commands.spec.ts index 954c2efcb6..cb035b94d5 100644 --- a/src/core/auto-approval/__tests__/commands.spec.ts +++ b/src/core/auto-approval/__tests__/commands.spec.ts @@ -197,6 +197,17 @@ describe("getCommandDecision — # inside a word and glob patterns", () => { }) }) +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") + }, + ) +}) + 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"]) diff --git a/src/shared/__tests__/parse-command.spec.ts b/src/shared/__tests__/parse-command.spec.ts index 8fdfadad5f..97e7d3686a 100644 --- a/src/shared/__tests__/parse-command.spec.ts +++ b/src/shared/__tests__/parse-command.spec.ts @@ -249,6 +249,14 @@ describe("parseCommand", () => { const { commands: result } = parseCommand("echo $(whoami)") expect(result).toContain("whoami") }) + + it.each([ + { input: "reboot$(printf 'x')", expected: ["printf 'x'", "reboot"] }, + { input: "reboot$(printf '\n')", expected: ["printf '\n'", "reboot"] }, + { 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", () => { @@ -267,6 +275,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", () => { diff --git a/src/shared/parse-command.ts b/src/shared/parse-command.ts index c1468b70c1..970437565d 100644 --- a/src/shared/parse-command.ts +++ b/src/shared/parse-command.ts @@ -700,22 +700,31 @@ function parseCommandLine(command: string): string[] { commands.push(currentCommand.join(" ")) currentCommand = [] } - } else if (token.op === "glob" && "pattern" in token) { - // Keep the glob text (for example `*`) so that deny rules can match it. + } 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. currentCommand.push(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) { + // Check if the word contains subshell placeholders + const subshellMatches = [...token.matchAll(/__SUBSH_(\d+)__/g)] + if (subshellMatches.length > 0) { + // Text outside the substitution is part of the outer command, for + // example `reboot` in `reboot$(printf x)`. It must stay in the + // command so that the allowlist check sees it. + const outerText = token.replace(/__SUBSH_\d+__/g, "") + if (outerText) { + currentCommand.push(outerText) + } else if (currentCommand.length > 0) { commands.push(currentCommand.join(" ")) currentCommand = [] } - commands.push(subshells[parseInt(subshellMatch[1])]) + for (const match of subshellMatches) { + commands.push(subshells[parseInt(match[1])]) + } } else { currentCommand.push(token) } From 2f263cd13a3fdf61e08947f56c4f979c06a30b4e Mon Sep 17 00:00:00 2001 From: ooadcsulb22 Date: Mon, 9 Nov 2026 20:35:16 -0800 Subject: [PATCH 4/4] fix(auto-approval): keep the substitution text in a word and check globs A substitution that is part of a word changes the word. For example, $(printf re)boot runs reboot. The parser kept only the literal text (boot), so an allow rule for boot approved the command. Keep the substitution text in the word, so that the word matches no allow rule unless the rule covers it. A glob that contains a substitution, for example echo*$(reboot), did not give the inner command. Handle glob words the same as other words, so that the allowlist and deny rules check the inner command. Co-authored-by: Cursor --- .../auto-approval/__tests__/commands.spec.ts | 17 ++++++++ src/shared/__tests__/parse-command.spec.ts | 9 +++- src/shared/parse-command.ts | 42 ++++++++++--------- 3 files changed, 46 insertions(+), 22 deletions(-) diff --git a/src/core/auto-approval/__tests__/commands.spec.ts b/src/core/auto-approval/__tests__/commands.spec.ts index cb035b94d5..f56547588a 100644 --- a/src/core/auto-approval/__tests__/commands.spec.ts +++ b/src/core/auto-approval/__tests__/commands.spec.ts @@ -206,6 +206,23 @@ describe("getCommandDecision — outer word attached to a command substitution", 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", () => { diff --git a/src/shared/__tests__/parse-command.spec.ts b/src/shared/__tests__/parse-command.spec.ts index 97e7d3686a..7595dff326 100644 --- a/src/shared/__tests__/parse-command.spec.ts +++ b/src/shared/__tests__/parse-command.spec.ts @@ -251,8 +251,13 @@ describe("parseCommand", () => { }) it.each([ - { input: "reboot$(printf 'x')", expected: ["printf 'x'", "reboot"] }, - { input: "reboot$(printf '\n')", expected: ["printf '\n'", "reboot"] }, + { 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) diff --git a/src/shared/parse-command.ts b/src/shared/parse-command.ts index 970437565d..68d548d968 100644 --- a/src/shared/parse-command.ts +++ b/src/shared/parse-command.ts @@ -692,6 +692,26 @@ function parseCommandLine(command: string): string[] { 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 @@ -703,31 +723,13 @@ function parseCommandLine(command: string): string[] { } 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. - currentCommand.push(token.pattern) + addWord(token.pattern) } else { // Other operators (>) are part of the command currentCommand.push(token.op) } } else if (typeof token === "string") { - // Check if the word contains subshell placeholders - const subshellMatches = [...token.matchAll(/__SUBSH_(\d+)__/g)] - if (subshellMatches.length > 0) { - // Text outside the substitution is part of the outer command, for - // example `reboot` in `reboot$(printf x)`. It must stay in the - // command so that the allowlist check sees it. - const outerText = token.replace(/__SUBSH_\d+__/g, "") - if (outerText) { - currentCommand.push(outerText) - } else if (currentCommand.length > 0) { - commands.push(currentCommand.join(" ")) - currentCommand = [] - } - for (const match of subshellMatches) { - commands.push(subshells[parseInt(match[1])]) - } - } else { - currentCommand.push(token) - } + addWord(token) } }