Skip to content

Commit 129ec91

Browse files
committed
fix: Preserve newlines in quoted strings for auto-approve command parsing
- Fix parseCommand() to protect newlines inside quoted strings before splitting - Track quote state (single/double) and handle escaped quotes - Use placeholder to preserve newlines in quotes during line splitting - Restore actual newlines after parsing completes - Update tests to expect newlines preserved in quoted strings - Fixes auto-approve for multi-line git commit messages This resolves the issue where multi-line git commit messages like: git commit -m "feat: title - point a - point b" Were incorrectly split into separate commands, causing auto-approve to fail.
1 parent 52fb8e7 commit 129ec91

File tree

2 files changed

+59
-18
lines changed

2 files changed

+59
-18
lines changed

webview-ui/src/utils/__tests__/command-validation.spec.ts

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -110,15 +110,14 @@ describe("Command Validation", () => {
110110
])
111111
})
112112

113-
it("splits on actual newlines even within quotes", () => {
114-
// Note: Since we split by newlines first, actual newlines in the input
115-
// will split the command, even if they appear to be within quotes
113+
it("preserves newlines within quotes", () => {
114+
// Newlines inside quoted strings should be preserved as part of the command
116115
// Using template literal to create actual newline
117116
const commandWithNewlineInQuotes = `echo "Hello
118117
World"
119118
git status`
120-
// The quotes get stripped because they're no longer properly paired after splitting
121-
expect(parseCommand(commandWithNewlineInQuotes)).toEqual(["echo Hello", "World", "git status"])
119+
// The newlines inside quotes are preserved, so we get two commands
120+
expect(parseCommand(commandWithNewlineInQuotes)).toEqual(['echo "Hello\nWorld"', "git status"])
122121
})
123122

124123
it("handles quoted strings on single line", () => {
@@ -1083,10 +1082,7 @@ describe("Unified Command Decision Functions", () => {
10831082
expect(getCommandDecision("dangerous", allowed, denied)).toBe("ask_user")
10841083
expect(getCommandDecision("npm install && dangerous", allowed, denied)).toBe("ask_user")
10851084
})
1086-
// Real-world regression: multi-line git commit message in quotes should not be treated as separate commands
1087-
// Current behavior splits on newlines before quote handling, which causes follow-up lines to be considered commands.
1088-
// This test reproduces the failure mode for visibility. A future fix could change parseCommand to preserve
1089-
// newlines inside quotes for specific cases (e.g., git commit -m).
1085+
// Real-world regression: multi-line git commit message in quotes should be treated as a single command
10901086
describe("real-world: multi-line git commit message", () => {
10911087
it("auto-approves when commit message is single-line", () => {
10921088
const allowed = ["cd", "git add", "git commit"]
@@ -1096,17 +1092,30 @@ describe("Unified Command Decision Functions", () => {
10961092
expect(getCommandDecision(command, allowed, [])).toBe("auto_approve")
10971093
})
10981094

1099-
it("asks user when commit message is multi-line due to newline splitting (bug reproduction)", () => {
1095+
it("auto-approves when commit message is multi-line (newlines preserved in quotes)", () => {
11001096
const allowed = ["cd", "git add", "git commit"]
11011097
// Simplified reproduction of the user's example: a multi-line quoted commit message
11021098
const command = `cd /repo && git add src/a.ts src/b.ts && git commit -m "feat: title
11031099
11041100
- point a
11051101
- point b"`
11061102

1107-
// Because parseCommand splits on actual newlines first, the lines after the first are treated as separate
1108-
// commands (e.g., "- point a"), which are not in the allowlist, so the overall decision becomes "ask_user".
1109-
expect(getCommandDecision(command, allowed, [])).toBe("ask_user")
1103+
// parseCommand now preserves newlines inside quotes, so the entire commit message
1104+
// stays as part of the git commit command and gets auto-approved
1105+
expect(getCommandDecision(command, allowed, [])).toBe("auto_approve")
1106+
})
1107+
1108+
it("preserves newlines in the parsed command", () => {
1109+
const command = `git commit -m "feat: title
1110+
1111+
- point a
1112+
- point b"`
1113+
1114+
const parsed = parseCommand(command)
1115+
expect(parsed).toHaveLength(1)
1116+
expect(parsed[0]).toContain("\n")
1117+
expect(parsed[0]).toContain("- point a")
1118+
expect(parsed[0]).toContain("- point b")
11101119
})
11111120
})
11121121
})

webview-ui/src/utils/command-validation.ts

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -127,18 +127,49 @@ export function containsDangerousSubstitution(source: string): boolean {
127127
* chaining operators (&&, ||, ;, |, or &) and newlines.
128128
*
129129
* Uses shell-quote to properly handle:
130-
* - Quoted strings (preserves quotes)
130+
* - Quoted strings (preserves quotes and newlines within quotes)
131131
* - Subshell commands ($(cmd), `cmd`, <(cmd), >(cmd))
132132
* - PowerShell redirections (2>&1)
133133
* - Chain operators (&&, ||, ;, |, &)
134-
* - Newlines as command separators
134+
* - Newlines as command separators (but not within quotes)
135135
*/
136136
export function parseCommand(command: string): string[] {
137137
if (!command?.trim()) return []
138138

139-
// Split by newlines first (handle different line ending formats)
139+
// First, protect newlines inside quoted strings by replacing them with a placeholder
140+
// This prevents splitting multi-line quoted strings (e.g., git commit -m "multi\nline")
141+
const quotedStringPlaceholder = "___NEWLINE_IN_QUOTE___"
142+
let protectedCommand = command
143+
144+
// Track quote state and replace newlines inside quotes
145+
let inDoubleQuote = false
146+
let inSingleQuote = false
147+
let result = ""
148+
149+
for (let i = 0; i < command.length; i++) {
150+
const char = command[i]
151+
const prevChar = i > 0 ? command[i - 1] : ""
152+
153+
// Toggle quote state (ignore escaped quotes)
154+
if (char === '"' && prevChar !== "\\") {
155+
inDoubleQuote = !inDoubleQuote
156+
result += char
157+
} else if (char === "'" && prevChar !== "\\") {
158+
inSingleQuote = !inSingleQuote
159+
result += char
160+
} else if ((char === "\n" || char === "\r") && (inDoubleQuote || inSingleQuote)) {
161+
// Replace newlines inside quotes with placeholder
162+
result += quotedStringPlaceholder
163+
} else {
164+
result += char
165+
}
166+
}
167+
168+
protectedCommand = result
169+
170+
// Split by newlines (handle different line ending formats)
140171
// This regex splits on \r\n (Windows), \n (Unix), or \r (old Mac)
141-
const lines = command.split(/\r\n|\r|\n/)
172+
const lines = protectedCommand.split(/\r\n|\r|\n/)
142173
const allCommands: string[] = []
143174

144175
for (const line of lines) {
@@ -150,7 +181,8 @@ export function parseCommand(command: string): string[] {
150181
allCommands.push(...lineCommands)
151182
}
152183

153-
return allCommands
184+
// Restore newlines in quoted strings
185+
return allCommands.map((cmd) => cmd.replace(new RegExp(quotedStringPlaceholder, "g"), "\n"))
154186
}
155187

156188
/**

0 commit comments

Comments
 (0)