diff --git a/webview-ui/src/utils/__tests__/command-validation-quote-protection.spec.ts b/webview-ui/src/utils/__tests__/command-validation-quote-protection.spec.ts new file mode 100644 index 000000000000..61e59432a800 --- /dev/null +++ b/webview-ui/src/utils/__tests__/command-validation-quote-protection.spec.ts @@ -0,0 +1,126 @@ +// npx vitest src/utils/__tests__/command-validation-quote-protection.spec.ts + +import { + protectNewlinesInQuotes, + NEWLINE_PLACEHOLDER, + CARRIAGE_RETURN_PLACEHOLDER, +} from "../command-validation-quote-protection" + +describe("protectNewlinesInQuotes", () => { + const newlinePlaceholder = NEWLINE_PLACEHOLDER + const crPlaceholder = CARRIAGE_RETURN_PLACEHOLDER + + describe("basic quote handling", () => { + it("protects newlines in double quotes", () => { + const input = 'echo "hello\nworld"' + const expected = `echo "hello${newlinePlaceholder}world"` + expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected) + }) + + it("protects newlines in single quotes", () => { + const input = "echo 'hello\nworld'" + const expected = `echo 'hello${newlinePlaceholder}world'` + expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected) + }) + + it("does not protect newlines outside quotes", () => { + const input = "echo hello\necho world" + const expected = "echo hello\necho world" + expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected) + }) + }) + + describe("quote concatenation", () => { + it("handles quote concatenation where content between quotes is NOT quoted", () => { + // In bash: echo '"'A'"' prints "A" (A is not quoted) + const input = `echo '"'A\n'"'` + // The newline after A is NOT inside quotes, so it should NOT be protected + const expected = `echo '"'A\n'"'` + expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected) + }) + + it("handles alternating quotes correctly", () => { + // echo "hello"world"test" -> hello is quoted, world is not, test is quoted + const input = `echo "hello\n"world\n"test\n"` + const expected = `echo "hello${newlinePlaceholder}"world\n"test${newlinePlaceholder}"` + expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected) + }) + + it("handles single quote after double quote", () => { + const input = `echo "hello"'world\n'` + const expected = `echo "hello"'world${newlinePlaceholder}'` + expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected) + }) + + it("handles double quote after single quote", () => { + const input = `echo 'hello'"world\n"` + const expected = `echo 'hello'"world${newlinePlaceholder}"` + expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected) + }) + }) + + describe("escaped quotes", () => { + it("handles escaped double quotes in double-quoted strings", () => { + const input = 'echo "hello\\"world\n"' + const expected = `echo "hello\\"world${newlinePlaceholder}"` + expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected) + }) + + it("does not treat backslash as escape in single quotes", () => { + // In single quotes, backslash is literal (except for \' in some shells) + const input = "echo 'hello\\'world\n'" + // The \\ is literal, the ' ends the quote, so world\n is outside quotes + const expected = "echo 'hello\\'world\n'" + expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected) + }) + }) + + describe("edge cases", () => { + it("handles unclosed quotes", () => { + const input = 'echo "unclosed\n' + const expected = `echo "unclosed${newlinePlaceholder}` + expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected) + }) + + it("handles empty string", () => { + expect(protectNewlinesInQuotes("", newlinePlaceholder, crPlaceholder)).toBe("") + }) + + it("handles string with no quotes", () => { + const input = "echo hello\nworld" + expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(input) + }) + + it("handles multiple newlines in quotes", () => { + const input = 'echo "line1\nline2\nline3"' + const expected = `echo "line1${newlinePlaceholder}line2${newlinePlaceholder}line3"` + expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected) + }) + + it("handles carriage returns", () => { + const input = 'echo "hello\rworld"' + const expected = `echo "hello${crPlaceholder}world"` + expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected) + }) + + it("handles CRLF", () => { + const input = 'echo "hello\r\nworld"' + const expected = `echo "hello${crPlaceholder}${newlinePlaceholder}world"` + expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected) + }) + }) + + describe("real-world git commit examples", () => { + it("protects newlines in git commit message", () => { + const input = `git commit -m "feat: title\n\n- point a\n- point b"` + const expected = `git commit -m "feat: title${newlinePlaceholder}${newlinePlaceholder}- point a${newlinePlaceholder}- point b"` + expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected) + }) + + it("handles complex git command with multiple quoted sections", () => { + const input = `git add . && git commit -m "feat: title\n\n- point a" && echo "done\n"` + const expected = `git add . && git commit -m "feat: title${newlinePlaceholder}${newlinePlaceholder}- point a" && echo "done${newlinePlaceholder}"` + expect(protectNewlinesInQuotes(input, newlinePlaceholder, crPlaceholder)).toBe(expected) + }) + }) +}) diff --git a/webview-ui/src/utils/__tests__/command-validation.spec.ts b/webview-ui/src/utils/__tests__/command-validation.spec.ts index e87bae07e51f..4eb289d2b343 100644 --- a/webview-ui/src/utils/__tests__/command-validation.spec.ts +++ b/webview-ui/src/utils/__tests__/command-validation.spec.ts @@ -110,15 +110,14 @@ describe("Command Validation", () => { ]) }) - it("splits on actual newlines even within quotes", () => { - // Note: Since we split by newlines first, actual newlines in the input - // will split the command, even if they appear to be within quotes + it("preserves newlines within quotes", () => { + // Newlines inside quoted strings should be preserved as part of the command // Using template literal to create actual newline const commandWithNewlineInQuotes = `echo "Hello World" git status` - // The quotes get stripped because they're no longer properly paired after splitting - expect(parseCommand(commandWithNewlineInQuotes)).toEqual(["echo Hello", "World", "git status"]) + // The newlines inside quotes are preserved, so we get two commands + expect(parseCommand(commandWithNewlineInQuotes)).toEqual(['echo "Hello\nWorld"', "git status"]) }) it("handles quoted strings on single line", () => { @@ -1083,6 +1082,42 @@ describe("Unified Command Decision Functions", () => { expect(getCommandDecision("dangerous", allowed, denied)).toBe("ask_user") expect(getCommandDecision("npm install && dangerous", allowed, denied)).toBe("ask_user") }) + // Real-world regression: multi-line git commit message in quotes should be treated as a single command + describe("real-world: multi-line git commit message", () => { + it("auto-approves when commit message is single-line", () => { + const allowed = ["cd", "git add", "git commit"] + const command = + 'cd /repo && git add src/a.ts src/b.ts && git commit -m "feat: title - one line message"' + + expect(getCommandDecision(command, allowed, [])).toBe("auto_approve") + }) + + it("auto-approves when commit message is multi-line (newlines preserved in quotes)", () => { + const allowed = ["cd", "git add", "git commit"] + // Simplified reproduction of the user's example: a multi-line quoted commit message + const command = `cd /repo && git add src/a.ts src/b.ts && git commit -m "feat: title + +- point a +- point b"` + + // parseCommand now preserves newlines inside quotes, so the entire commit message + // stays as part of the git commit command and gets auto-approved + expect(getCommandDecision(command, allowed, [])).toBe("auto_approve") + }) + + it("preserves newlines in the parsed command", () => { + const command = `git commit -m "feat: title + +- point a +- point b"` + + const parsed = parseCommand(command) + expect(parsed).toHaveLength(1) + expect(parsed[0]).toContain("\n") + expect(parsed[0]).toContain("- point a") + expect(parsed[0]).toContain("- point b") + }) + }) }) describe("CommandValidator Integration Tests", () => { diff --git a/webview-ui/src/utils/command-validation-quote-protection.ts b/webview-ui/src/utils/command-validation-quote-protection.ts new file mode 100644 index 000000000000..7c45e080e9b6 --- /dev/null +++ b/webview-ui/src/utils/command-validation-quote-protection.ts @@ -0,0 +1,101 @@ +/** + * Placeholders used to protect newlines within quoted strings during command parsing. + * These constants are used by the protectNewlinesInQuotes function to temporarily replace + * newlines that appear inside quotes, preventing them from being treated as command separators. + * We use separate placeholders for \n and \r to preserve the original line ending type. + */ +export const NEWLINE_PLACEHOLDER = "___NEWLINE___" +export const CARRIAGE_RETURN_PLACEHOLDER = "___CARRIAGE_RETURN___" + +/** + * Protect newlines inside quoted strings by replacing them with placeholders. + * This handles proper shell quoting rules where quotes can be concatenated. + * Uses separate placeholders for \n and \r to preserve the original line ending type. + * + * Examples: + * - "hello\nworld" -> newline is protected (inside double quotes) + * - 'hello\nworld' -> newline is protected (inside single quotes) + * - echo '"'A'"' -> A is NOT quoted (quote concatenation) + * - "hello"world -> world is NOT quoted + * + * @param command - The command string to process + * @param newlinePlaceholder - The placeholder string to use for \n characters + * @param carriageReturnPlaceholder - The placeholder string to use for \r characters + * @returns The command with newlines in quotes replaced by placeholders + */ +export function protectNewlinesInQuotes( + command: string, + newlinePlaceholder: string, + carriageReturnPlaceholder: string, +): string { + let result = "" + let i = 0 + + while (i < command.length) { + const char = command[i] + + if (char === '"') { + // Start of double-quoted string + result += char + i++ + + // Process until we find the closing unescaped quote + while (i < command.length) { + const quoteChar = command[i] + const prevChar = i > 0 ? command[i - 1] : "" + + if (quoteChar === '"' && prevChar !== "\\") { + // Found closing quote + result += quoteChar + i++ + break + } else if (quoteChar === "\n") { + // Replace \n inside double quotes + result += newlinePlaceholder + i++ + } else if (quoteChar === "\r") { + // Replace \r inside double quotes + result += carriageReturnPlaceholder + i++ + } else { + result += quoteChar + i++ + } + } + } else if (char === "'") { + // Start of single-quoted string + result += char + i++ + + // Process until we find the closing quote + // Note: In single quotes, backslash does NOT escape (except for \' in some shells) + while (i < command.length) { + const quoteChar = command[i] + + if (quoteChar === "'") { + // Found closing quote + result += quoteChar + i++ + break + } else if (quoteChar === "\n") { + // Replace \n inside single quotes + result += newlinePlaceholder + i++ + } else if (quoteChar === "\r") { + // Replace \r inside single quotes + result += carriageReturnPlaceholder + i++ + } else { + result += quoteChar + i++ + } + } + } else { + // Not in quotes, keep character as-is + result += char + i++ + } + } + + return result +} diff --git a/webview-ui/src/utils/command-validation.ts b/webview-ui/src/utils/command-validation.ts index 572ca32bad64..5be9f7180047 100644 --- a/webview-ui/src/utils/command-validation.ts +++ b/webview-ui/src/utils/command-validation.ts @@ -1,4 +1,9 @@ import { parse } from "shell-quote" +import { + protectNewlinesInQuotes, + NEWLINE_PLACEHOLDER, + CARRIAGE_RETURN_PLACEHOLDER, +} from "./command-validation-quote-protection" type ShellToken = string | { op: string } | { command: string } @@ -127,18 +132,23 @@ export function containsDangerousSubstitution(source: string): boolean { * chaining operators (&&, ||, ;, |, or &) and newlines. * * Uses shell-quote to properly handle: - * - Quoted strings (preserves quotes) + * - Quoted strings (preserves quotes and newlines within quotes) * - Subshell commands ($(cmd), `cmd`, <(cmd), >(cmd)) * - PowerShell redirections (2>&1) * - Chain operators (&&, ||, ;, |, &) - * - Newlines as command separators + * - Newlines as command separators (but not within quotes) */ +//kilocode_change end export function parseCommand(command: string): string[] { if (!command?.trim()) return [] - // Split by newlines first (handle different line ending formats) + // First, protect newlines inside quoted strings by replacing them with placeholders + // This prevents splitting multi-line quoted strings (e.g., git commit -m "multi\nline") + const protectedCommand = protectNewlinesInQuotes(command, NEWLINE_PLACEHOLDER, CARRIAGE_RETURN_PLACEHOLDER) + + // Split by newlines (handle different line ending formats) // This regex splits on \r\n (Windows), \n (Unix), or \r (old Mac) - const lines = command.split(/\r\n|\r|\n/) + const lines = protectedCommand.split(/\r\n|\r|\n/) const allCommands: string[] = [] for (const line of lines) { @@ -150,7 +160,12 @@ export function parseCommand(command: string): string[] { allCommands.push(...lineCommands) } - return allCommands + // Restore newlines and carriage returns in quoted strings + return allCommands.map((cmd) => + cmd + .replace(new RegExp(NEWLINE_PLACEHOLDER, "g"), "\n") + .replace(new RegExp(CARRIAGE_RETURN_PLACEHOLDER, "g"), "\r"), + ) } /**