Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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)
})
})
})
45 changes: 40 additions & 5 deletions webview-ui/src/utils/__tests__/command-validation.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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", () => {
Expand Down
101 changes: 101 additions & 0 deletions webview-ui/src/utils/command-validation-quote-protection.ts
Original file line number Diff line number Diff line change
@@ -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 !== "\\") {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The escape check (prev char !== '\') only looks at the immediate previous character. For robust handling of multiple consecutive backslashes, consider counting the number of preceding backslashes.

Suggested change
if (quoteChar === '"' && prevChar !== "\\") {
if (quoteChar === '"' && (i === 0 || command.slice(0, i).match(/\\*$/)[0].length % 2 === 0)) {

// Found closing quote
result += quoteChar
i++
break
Comment on lines +47 to +51
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The escaped quote detection doesn't handle consecutive backslashes correctly. When you have an escaped backslash followed by a quote (e.g., "test\\" representing the shell command "test\"), the function checks only if the previous character is a backslash but doesn't verify if that backslash itself is escaped. This means "test\\" would be treated as having an escaped quote when the quote should actually close the string. While this edge case may be rare in git commit messages, it could cause newlines outside quotes to be incorrectly protected if a command contains escaped backslashes before quotes. Consider counting consecutive backslashes to properly determine if a quote is escaped.

} 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
}
25 changes: 20 additions & 5 deletions webview-ui/src/utils/command-validation.ts
Original file line number Diff line number Diff line change
@@ -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 }

Expand Down Expand Up @@ -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) {
Expand All @@ -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"),
)
}

/**
Expand Down