From 5152e12a16bf57c1858e63d5b6a5f6d63dcb958b Mon Sep 17 00:00:00 2001 From: John Richmond <5629+jr@users.noreply.github.com> Date: Mon, 29 Sep 2025 16:23:07 -0700 Subject: [PATCH] more esoteric still --- .../__tests__/command-validation.spec.ts | 14 +++ webview-ui/src/utils/command-validation.ts | 108 +++++++++++++++++- 2 files changed, 116 insertions(+), 6 deletions(-) diff --git a/webview-ui/src/utils/__tests__/command-validation.spec.ts b/webview-ui/src/utils/__tests__/command-validation.spec.ts index 2f7b3ba4f7..d432c143fc 100644 --- a/webview-ui/src/utils/__tests__/command-validation.spec.ts +++ b/webview-ui/src/utils/__tests__/command-validation.spec.ts @@ -578,6 +578,11 @@ echo "Successfully converted $count .jsx files to .tsx"` expect(result).toEqual(["total=$((price * quantity + tax))"]) }) + it("extracts subshells inside arithmetic expressions", () => { + const cmd = "arr=(); echo $((arr[$(whoami)]))" + expect(parseCommand(cmd)).toEqual(["arr=()", "echo $((arr[$(whoami)]))", "whoami"]) + }) + it("should handle complex parameter expansions without errors", () => { const commands = [ "echo ${var:-default}", @@ -1110,6 +1115,15 @@ describe("Unified Command Decision Functions", () => { expect(getCommandDecision("echo (whoami)", ["echo", "ls"], ["whoami"])).toBe("auto_deny") }) + it("flags subshell commands inside arithmetic expressions", () => { + // When whoami appears inside $(( ... )) via $(whoami), it must be validated + expect(getCommandDecision("arr=(); echo $((arr[$(whoami)]))", ["echo", "arr="], [])).toBe("ask_user") + // If whoami is explicitly denied, the whole chain must be auto-denied + expect(getCommandDecision("arr=(); echo $((arr[$(whoami)]))", ["echo", "arr="], ["whoami"])).toBe( + "auto_deny", + ) + }) + it("properly validates subshell commands when no denylist is present", () => { expect(getCommandDecision("npm install $(echo test)", allowedCommands)).toBe("auto_approve") expect(getCommandDecision("npm install `echo test`", allowedCommands)).toBe("auto_approve") diff --git a/webview-ui/src/utils/command-validation.ts b/webview-ui/src/utils/command-validation.ts index f309722991..6f9aa2e79c 100644 --- a/webview-ui/src/utils/command-validation.ts +++ b/webview-ui/src/utils/command-validation.ts @@ -214,6 +214,8 @@ function parseCommandLine(command: string): string[] { const arithmeticExpressions: string[] = [] const variables: string[] = [] const parameterExpansions: string[] = [] + // Commands extracted from within arithmetic expressions (e.g., $(whoami) inside $((...))) + const embeddedSubshellCommands: string[] = [] // First handle PowerShell redirections by temporarily replacing them let processedCommand = command.replace(/\d*>&\d*/g, (match) => { @@ -221,15 +223,104 @@ function parseCommandLine(command: string): string[] { return `__REDIR_${redirections.length - 1}__` }) - // Handle arithmetic expressions: $((...)) pattern - // Match the entire arithmetic expression including nested parentheses - processedCommand = processedCommand.replace(/\$\(\([^)]*(?:\)[^)]*)*\)\)/g, (match) => { - arithmeticExpressions.push(match) - return `__ARITH_${arithmeticExpressions.length - 1}__` + // Protect bash array empty initializer in assignments: name=() + // Without this, shell-quote may split it into "name=", "(" and ")" + processedCommand = processedCommand.replace(/=\s*\(\s*\)/g, (_match) => { + arrayIndexing.push("()") + return `=__ARRAY_${arrayIndexing.length - 1}__` }) + // Handle arithmetic expressions: $((...)) pattern with balanced parsing. + // We must protect the whole arithmetic region from shell-quote tokenization + // while still discovering any $(...) or backticks inside it. + { + let out = "" + for (let i = 0; i < processedCommand.length; i++) { + // Detect start of $(( ... )) + if (processedCommand[i] === "$" && processedCommand[i + 1] === "(" && processedCommand[i + 2] === "(") { + const start = i + // Track balanced parentheses depth. We saw "((" + let depth = 2 + i += 3 + while (i < processedCommand.length && depth > 0) { + const ch = processedCommand[i] + if (ch === "(") depth++ + else if (ch === ")") depth-- + i++ + } + // i currently points to the char AFTER the one that closed depth to 0 + const match = processedCommand.slice(start, i) + // Extract subshells $(...) inside arithmetic, but skip arithmetic "$((" by requiring next char != "(" + match.replace(/\$\((?!\()(.*?)\)/g, (_m, inner) => { + const trimmed = String(inner).trim() + if (trimmed) { + subshells.push(trimmed) + const expanded = parseCommand(trimmed) + if (expanded.length > 0) { + embeddedSubshellCommands.push(...expanded) + } else { + embeddedSubshellCommands.push(trimmed) + } + } + return _m + }) + // Extract backtick subshells inside arithmetic + match.replace(/`((?:\\`|[^`])*)`/g, (_m, inner) => { + const unescaped = String(inner).replace(/\\`/g, "`").trim() + if (unescaped) { + subshells.push(unescaped) + const expanded = parseCommand(unescaped) + if (expanded.length > 0) { + embeddedSubshellCommands.push(...expanded) + } else { + embeddedSubshellCommands.push(unescaped) + } + } + return _m + }) + + arithmeticExpressions.push(match) + out += `__ARITH_${arithmeticExpressions.length - 1}__` + // Compensate for loop's i++ after slice end + i -= 1 + } else { + out += processedCommand[i] + } + } + processedCommand = out + } + // Handle $[...] arithmetic expressions (alternative syntax) processedCommand = processedCommand.replace(/\$\[[^\]]*\]/g, (match) => { + // Extract subshells inside $[ ... ] arithmetic expressions as well + // Skip arithmetic "$((" by ensuring char after "$(" is not "(" + match.replace(/\$\((?!\()(.*?)\)/g, (_m, inner) => { + const trimmed = String(inner).trim() + if (trimmed) { + subshells.push(trimmed) + const expanded = parseCommand(trimmed) + if (expanded.length > 0) { + embeddedSubshellCommands.push(...expanded) + } else { + embeddedSubshellCommands.push(trimmed) + } + } + return _m + }) + match.replace(/`((?:\\`|[^`])*)`/g, (_m, inner) => { + const unescaped = String(inner).replace(/\\`/g, "`").trim() + if (unescaped) { + subshells.push(unescaped) + const expanded = parseCommand(unescaped) + if (expanded.length > 0) { + embeddedSubshellCommands.push(...expanded) + } else { + embeddedSubshellCommands.push(unescaped) + } + } + return _m + }) + arithmeticExpressions.push(match) return `__ARITH_${arithmeticExpressions.length - 1}__` }) @@ -262,7 +353,8 @@ function parseCommandLine(command: string): string[] { // Then handle subshell commands $() and back-ticks processedCommand = processedCommand - .replace(/\$\((.*?)\)/g, (_, inner) => { + // Handle command substitution, but avoid arithmetic "$((" by requiring next char != "(" + .replace(/\$\((?!\()(.*?)\)/g, (_, inner) => { subshells.push(inner.trim()) return `__SUBSH_${subshells.length - 1}__` }) @@ -364,6 +456,10 @@ function parseCommandLine(command: string): string[] { commands.push(currentCommand.join(" ")) } + // Include any subshell commands discovered inside arithmetic expressions + if (embeddedSubshellCommands.length > 0) { + commands.push(...embeddedSubshellCommands) + } // Restore quotes and redirections return commands.map((cmd) => restorePlaceholders(