fix: resolve 'Bad substitution' error in command parsing (RooCodeInc#5743)

Author: daniel-lxs

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

@@ -276,6 +276,123 @@ done`
 				parseCommand(problematicPart)
 			}).not.toThrow("Bad substitution")
 		})
+
+		it("should handle bash arithmetic expressions with $(())", () => {
+			// Test the exact script from the user's error
+			const bashScript = `jsx_files=$(find resources/js -name "*.jsx" -type f -not -path "*/node_modules/*")
+count=0
+for file in $jsx_files; do
+  ts_file="\${file%.jsx}.tsx"
+  if [ ! -f "$ts_file" ]; then
+    cp "$file" "$ts_file"
+    count=$((count + 1))
+  fi
+done
+echo "Successfully converted $count .jsx files to .tsx"`
+
+			expect(() => {
+				parseCommand(bashScript)
+			}).not.toThrow("Bad substitution: calc.add")
+		})
+
+		it("should correctly parse commands with arithmetic expressions", () => {
+			const result = parseCommand("count=$((count + 1)) && echo $count")
+			expect(result).toEqual(["count=$((count + 1))", "echo $count"])
+		})
+
+		it("should handle nested arithmetic expressions", () => {
+			const result = parseCommand("result=$((10 * (5 + 3))) && echo $result")
+			expect(result).toEqual(["result=$((10 * (5 + 3)))", "echo $result"])
+		})
+
+		it("should handle arithmetic expressions with variables", () => {
+			const result = parseCommand("total=$((price * quantity + tax))")
+			expect(result).toEqual(["total=$((price * quantity + tax))"])
+		})
+
+		it("should handle complex parameter expansions without errors", () => {
+			const commands = [
+				"echo ${var:-default}",
+				"echo ${#array[@]}",
+				"echo ${var%suffix}",
+				"echo ${var#prefix}",
+				"echo ${var/pattern/replacement}",
+				"echo ${!var}",
+				"echo ${var:0:5}",
+				"echo ${var,,}",
+				"echo ${var^^}",
+			]
+
+			commands.forEach((cmd) => {
+				expect(() => {
+					parseCommand(cmd)
+				}).not.toThrow()
+			})
+		})
+
+		it("should handle process substitutions without errors", () => {
+			const commands = [
+				"diff <(sort file1) <(sort file2)",
+				"command >(gzip > output.gz)",
+				"while read line; do echo $line; done < <(cat file)",
+			]
+
+			commands.forEach((cmd) => {
+				expect(() => {
+					parseCommand(cmd)
+				}).not.toThrow()
+			})
+		})
+
+		it("should handle special bash variables without errors", () => {
+			const commands = [
+				"echo $?",
+				"echo $!",
+				"echo $#",
+				"echo $$",
+				"echo $@",
+				"echo $*",
+				"echo $-",
+				"echo $0",
+				"echo $1 $2 $3",
+			]
+
+			commands.forEach((cmd) => {
+				expect(() => {
+					parseCommand(cmd)
+				}).not.toThrow()
+			})
+		})
+
+		it("should handle mixed complex bash constructs", () => {
+			const complexCommand = `
+				for file in \${files[@]}; do
+					if [[ -f "\${file%.txt}.bak" ]]; then
+						count=\$((count + 1))
+						echo "Processing \${file} (\$count/\${#files[@]})"
+						result=\$(process_file "\$file" 2>&1)
+						if [[ \$? -eq 0 ]]; then
+							echo "Success: \$result" >(logger)
+						fi
+					fi
+				done
+			`
+
+			expect(() => {
+				parseCommand(complexCommand)
+			}).not.toThrow()
+		})
+
+		it("should handle fallback parsing when shell-quote fails", () => {
+			// Test a command that might cause shell-quote to fail
+			const problematicCommand = "echo ${unclosed"
+
+			expect(() => {
+				const result = parseCommand(problematicCommand)
+				// Should not throw and should return some result
+				expect(Array.isArray(result)).toBe(true)
+			}).not.toThrow()
+		})
 	})
 
 	describe("isAutoApprovedCommand (legacy behavior)", () => {

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

@@ -76,17 +76,48 @@ export function parseCommand(command: string): string[] {
 	const subshells: string[] = []
 	const quotes: string[] = []
 	const arrayIndexing: string[] = []
+	const arithmeticExpressions: string[] = []
+	const variables: string[] = []
+	const parameterExpansions: string[] = []
+	const processSubstitutions: string[] = []
 
 	// First handle PowerShell redirections by temporarily replacing them
 	let processedCommand = command.replace(/\d*>&\d*/g, (match) => {
 		redirections.push(match)
 		return `__REDIR_${redirections.length - 1}__`
 	})
 
-	// Handle array indexing expressions: ${array[...]} pattern and partial expressions
-	processedCommand = processedCommand.replace(/\$\{[^}]*\[[^\]]*(\]([^}]*\})?)?/g, (match) => {
-		arrayIndexing.push(match)
-		return `__ARRAY_${arrayIndexing.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}__`
+	})
+
+	// Handle parameter expansions: ${...} patterns (including array indexing)
+	// This covers ${var}, ${var:-default}, ${var:+alt}, ${#var}, ${var%pattern}, etc.
+	processedCommand = processedCommand.replace(/\$\{[^}]+\}/g, (match) => {
+		parameterExpansions.push(match)
+		return `__PARAM_${parameterExpansions.length - 1}__`
+	})
+
+	// Handle process substitutions: <(...) and >(...)
+	processedCommand = processedCommand.replace(/[<>]\([^)]+\)/g, (match) => {
+		processSubstitutions.push(match)
+		return `__PROCSUB_${processSubstitutions.length - 1}__`
+	})
+
+	// Handle simple variable references: $varname pattern
+	// This prevents shell-quote from splitting $count into separate tokens
+	processedCommand = processedCommand.replace(/\$[a-zA-Z_][a-zA-Z0-9_]*/g, (match) => {
+		variables.push(match)
+		return `__VAR_${variables.length - 1}__`
+	})
+
+	// Handle special bash variables: $?, $!, $#, $$, $@, $*, $-, $0-$9
+	processedCommand = processedCommand.replace(/\$[?!#$@*\-0-9]/g, (match) => {
+		variables.push(match)
+		return `__VAR_${variables.length - 1}__`
 	})
 
 	// Then handle subshell commands
@@ -106,7 +137,40 @@ export function parseCommand(command: string): string[] {
 		return `__QUOTE_${quotes.length - 1}__`
 	})
 
-	const tokens = parse(processedCommand) as ShellToken[]
+	let tokens: ShellToken[]
+	try {
+		tokens = parse(processedCommand) as ShellToken[]
+	} catch (error: any) {
+		// If shell-quote fails to parse, fall back to simple splitting
+		console.warn("shell-quote parse error:", error.message, "for command:", processedCommand)
+
+		// Simple fallback: split by common operators
+		const fallbackCommands = processedCommand
+			.split(/(?:&&|\|\||;|\|)/)
+			.map((cmd) => cmd.trim())
+			.filter((cmd) => cmd.length > 0)
+
+		// Restore all placeholders for each command
+		return fallbackCommands.map((cmd) => {
+			let result = cmd
+			// Restore quotes
+			result = result.replace(/__QUOTE_(\d+)__/g, (_, i) => quotes[parseInt(i)])
+			// Restore redirections
+			result = result.replace(/__REDIR_(\d+)__/g, (_, i) => redirections[parseInt(i)])
+			// Restore array indexing expressions
+			result = result.replace(/__ARRAY_(\d+)__/g, (_, i) => arrayIndexing[parseInt(i)])
+			// Restore arithmetic expressions
+			result = result.replace(/__ARITH_(\d+)__/g, (_, i) => arithmeticExpressions[parseInt(i)])
+			// Restore parameter expansions
+			result = result.replace(/__PARAM_(\d+)__/g, (_, i) => parameterExpansions[parseInt(i)])
+			// Restore process substitutions
+			result = result.replace(/__PROCSUB_(\d+)__/g, (_, i) => processSubstitutions[parseInt(i)])
+			// Restore variable references
+			result = result.replace(/__VAR_(\d+)__/g, (_, i) => variables[parseInt(i)])
+			return result
+		})
+	}
+
 	const commands: string[] = []
 	let currentCommand: string[] = []
 
@@ -151,6 +215,14 @@ export function parseCommand(command: string): string[] {
 		result = result.replace(/__REDIR_(\d+)__/g, (_, i) => redirections[parseInt(i)])
 		// Restore array indexing expressions
 		result = result.replace(/__ARRAY_(\d+)__/g, (_, i) => arrayIndexing[parseInt(i)])
+		// Restore arithmetic expressions
+		result = result.replace(/__ARITH_(\d+)__/g, (_, i) => arithmeticExpressions[parseInt(i)])
+		// Restore parameter expansions
+		result = result.replace(/__PARAM_(\d+)__/g, (_, i) => parameterExpansions[parseInt(i)])
+		// Restore process substitutions
+		result = result.replace(/__PROCSUB_(\d+)__/g, (_, i) => processSubstitutions[parseInt(i)])
+		// Restore variable references
+		result = result.replace(/__VAR_(\d+)__/g, (_, i) => variables[parseInt(i)])
 		return result
 	})
 }