diff --git a/src/core/assistant-message/__tests__/parseAssistantMessage.test.ts b/src/core/assistant-message/__tests__/parseAssistantMessage.test.ts index aa57e13896..19f88a91d7 100644 --- a/src/core/assistant-message/__tests__/parseAssistantMessage.test.ts +++ b/src/core/assistant-message/__tests__/parseAssistantMessage.test.ts @@ -335,158 +335,6 @@ const isEmptyTextContent = (block: AssistantMessageContent) => expect(result[5].type).toBe("tool_use") expect((result[5] as ToolUse).name).toBe("execute_command") }) - - it("treats inline mention as plain text", () => { - const message = "Use the `` tool when you need to read a file." - const result = parser(message) - expect(result).toHaveLength(1) - expect(result[0].type).toBe("text") - }) - - it("treats fenced code block mention as plain text", () => { - const message = [ - "Here is an example:", - "```", - "", - "demo.txt", - "", - "```", - ].join("\n") - const result = parser(message) - expect(result).toHaveLength(1) - expect(result[0].type).toBe("text") - }) - - it("parses real tool invocation with newline correctly", () => { - const invocation = ["", "demo.txt", ""].join("\n") - const result = parser(invocation) - expect(result).toHaveLength(1) - const tool = result[0] as ToolUse - expect(tool.type).toBe("tool_use") - expect(tool.name).toBe("read_file") - expect(tool.params.path).toBe("demo.txt") - }) - }) - - describe("code block edge cases", () => { - it("handles nested code blocks with tool tags", () => { - const message = [ - "Here's a nested code block:", - "> ```", - "> test.txt", - "> ```", - ].join("\n") - - const result = parser(message) - expect(result).toHaveLength(1) - expect(result[0].type).toBe("text") - }) - - it("handles escaped backticks", () => { - const message = "This has an escaped \\`\\` tag" - - const result = parser(message) - expect(result).toHaveLength(1) - expect(result[0].type).toBe("text") - }) - - it("handles incomplete code blocks", () => { - const message = [ - "```javascript", - "test.txt", - // Missing closing backticks - ].join("\n") - - const result = parser(message) - expect(result).toHaveLength(1) - expect(result[0].type).toBe("text") - }) - }) - - describe("tool tag edge cases", () => { - it("handles HTML comments containing tool tags", () => { - const message = "" - - // Note: Currently both parsers treat tool tags in HTML comments as actual tool invocations - // This test documents the current behavior rather than the ideal behavior - const result = parser(message) - expect(result).toHaveLength(3) - expect(result[0].type).toBe("text") - expect((result[0] as TextContent).content).toBe("") - }) - - it("handles tool tags with HTML-like attributes", () => { - const message = 'test.txt' - - // This should be treated as plain text since it's not a valid tool format - const result = parser(message) - expect(result).toHaveLength(1) - expect(result[0].type).toBe("text") - }) - - it("handles malformed tool tags with extra spaces", () => { - const message = "< read_file >< path >test.txt< /path >< /read_file >" - - // This should be treated as plain text since it's not a valid tool format - const result = parser(message) - expect(result).toHaveLength(1) - expect(result[0].type).toBe("text") - }) - }) - - describe("mixed formatting edge cases", () => { - it("handles tool tags with markdown formatting", () => { - const message = "****test.txt" - - // The opening tag is malformed due to the bold formatting - const result = parser(message) - expect(result).toHaveLength(1) - expect(result[0].type).toBe("text") - }) - - it("handles tool tags in markdown links", () => { - const message = "[Example](test.txt)" - - // Note: Currently both parsers treat tool tags in markdown links as actual tool invocations - // This test documents the current behavior rather than the ideal behavior - const result = parser(message) - expect(result).toHaveLength(3) - expect(result[0].type).toBe("text") - expect((result[0] as TextContent).content).toBe("[Example](") - expect(result[1].type).toBe("tool_use") - expect((result[1] as ToolUse).name).toBe("read_file") - expect(result[2].type).toBe("text") - expect((result[2] as TextContent).content).toBe(")") - }) - }) - - describe("performance edge cases", () => { - it("handles very long text with no tool uses", () => { - const message = "A".repeat(10000) - - const result = parser(message) - expect(result).toHaveLength(1) - expect(result[0].type).toBe("text") - expect((result[0] as TextContent).content.length).toBe(10000) - }) - - it("handles deeply nested tool parameters", () => { - // Create a message with a tool that has many nested parameters - let message = "" - for (let i = 0; i < 10; i++) { - message += `echo "Level ${i}" && ` - } - message += 'echo "Done"' - - const result = parser(message).filter((block) => !isEmptyTextContent(block)) - expect(result).toHaveLength(1) - expect(result[0].type).toBe("tool_use") - expect((result[0] as ToolUse).name).toBe("execute_command") - }) }) }) }) diff --git a/src/core/assistant-message/__tests__/parseAssistantMessageBenchmark.ts b/src/core/assistant-message/__tests__/parseAssistantMessageBenchmark.ts index 1779fae75a..bea161330b 100644 --- a/src/core/assistant-message/__tests__/parseAssistantMessageBenchmark.ts +++ b/src/core/assistant-message/__tests__/parseAssistantMessageBenchmark.ts @@ -81,13 +81,16 @@ const runBenchmark = () => { const namePadding = maxNameLength + 2 console.log( - `| ${"Test Case".padEnd(namePadding)} | V1 Time (ms) | V2 Time (ms) | V1 Heap (bytes) | V2 Heap (bytes) |`, + `| ${"Test Case".padEnd(namePadding)} | V1 Time (ms) | V2 Time (ms) | V1/V2 Ratio | V1 Heap (bytes) | V2 Heap (bytes) |`, + ) + console.log( + `| ${"-".repeat(namePadding)} | ------------ | ------------ | ----------- | ---------------- | ---------------- |`, ) - console.log(`| ${"-".repeat(namePadding)} | ------------ | ------------ | ---------------- | ---------------- |`) for (const testCase of testCases) { const v1Time = measureExecutionTime(parseAssistantMessageV1, testCase.input) const v2Time = measureExecutionTime(parseAssistantMessageV2, testCase.input) + const timeRatio = v1Time / v2Time const v1Memory = measureMemoryUsage(parseAssistantMessageV1, testCase.input) const v2Memory = measureMemoryUsage(parseAssistantMessageV2, testCase.input) @@ -96,6 +99,7 @@ const runBenchmark = () => { `| ${testCase.name.padEnd(namePadding)} | ` + `${v1Time.toFixed(4).padStart(12)} | ` + `${v2Time.toFixed(4).padStart(12)} | ` + + `${timeRatio.toFixed(2).padStart(11)} | ` + `${formatNumber(Math.round(v1Memory.heapUsed)).padStart(16)} | ` + `${formatNumber(Math.round(v2Memory.heapUsed)).padStart(16)} |`, ) diff --git a/src/core/assistant-message/index.ts b/src/core/assistant-message/index.ts index ee707b8bf4..72201b7722 100644 --- a/src/core/assistant-message/index.ts +++ b/src/core/assistant-message/index.ts @@ -1,3 +1,2 @@ export { type AssistantMessageContent, parseAssistantMessage } from "./parseAssistantMessage" export { presentAssistantMessage } from "./presentAssistantMessage" -export { parseAssistantMessageV2 } from "./parseAssistantMessageV2" diff --git a/src/core/assistant-message/parseAssistantMessage.ts b/src/core/assistant-message/parseAssistantMessage.ts index ae861519c8..f641cabdaa 100644 --- a/src/core/assistant-message/parseAssistantMessage.ts +++ b/src/core/assistant-message/parseAssistantMessage.ts @@ -13,52 +13,10 @@ export function parseAssistantMessage(assistantMessage: string): AssistantMessag let currentParamValueStartIndex = 0 let accumulator = "" - // Track whether we are inside markdown code blocks or inline code to avoid treating textual mentions - // of tool tags (e.g. ) as actual tool invocations. - let insideCodeBlock = false // ``` fenced code block - let insideInlineCode = false // `inline code` - - // Helper to decide if we should parse for tool-related tags at the current position - const shouldParseToolTags = () => !insideCodeBlock && !insideInlineCode - for (let i = 0; i < assistantMessage.length; i++) { const char = assistantMessage[i] - - // Detect fenced code block (```). - if (!insideInlineCode && assistantMessage.slice(i, i + 3) === "```") { - insideCodeBlock = !insideCodeBlock - // Append the full trio of backticks to accumulator. - accumulator += "```" - i += 2 // Skip the two extra backticks we just added. - - // When toggling code block state, continue to next iteration as - // these chars are text. - continue - } - - // Detect inline code (`) when not inside a fenced code block and not part of triple backticks - if (!insideCodeBlock && char === "`") { - insideInlineCode = !insideInlineCode - } - accumulator += char - // If we are in any kind of code context, treat everything as plain text. - if (!shouldParseToolTags()) { - // Handle accumulating text content block. - if (currentTextContent === undefined) { - currentTextContentStartIndex = i - } - - currentTextContent = { - type: "text", - content: accumulator.slice(currentTextContentStartIndex).trim(), - partial: true, - } - - continue - } - // There should not be a param without a tool use. if (currentToolUse && currentParamName) { const currentParamValue = accumulator.slice(currentParamValueStartIndex) @@ -87,7 +45,6 @@ export function parseAssistantMessage(assistantMessage: string): AssistantMessag continue } else { const possibleParamOpeningTags = toolParamNames.map((name) => `<${name}>`) - for (const paramOpeningTag of possibleParamOpeningTags) { if (accumulator.endsWith(paramOpeningTag)) { // Start of a new parameter. @@ -132,25 +89,6 @@ export function parseAssistantMessage(assistantMessage: string): AssistantMessag for (const toolUseOpeningTag of possibleToolUseOpeningTags) { if (accumulator.endsWith(toolUseOpeningTag)) { - // Check that this is likely an actual tool invocation and not - // an inline textual reference. - // We consider it an invocation only if the next non-whitespace - // character is a newline (\n or \r) or an opening angle bracket - // '<' (which would start the first parameter tag). - - let j = i + 1 // Position after the closing '>' of the opening tag. - - while (j < assistantMessage.length && assistantMessage[j] === " ") { - j++ - } - - const nextChar = assistantMessage[j] ?? "" - - if (nextChar && nextChar !== "<" && nextChar !== "\n" && nextChar !== "\r") { - // Treat as plain text, not a tool invocation. - continue - } - // Start of a new tool use. currentToolUse = { type: "tool_use", @@ -213,6 +151,5 @@ export function parseAssistantMessage(assistantMessage: string): AssistantMessag contentBlocks.push(currentTextContent) } - // Remove any empty text blocks that may have been created by whitespace or newlines before/after tool calls - return contentBlocks.filter((block) => !(block.type === "text" && block.content.trim().length === 0)) + return contentBlocks } diff --git a/src/core/assistant-message/parseAssistantMessageV2.ts b/src/core/assistant-message/parseAssistantMessageV2.ts index f04084f57e..d24a67f83d 100644 --- a/src/core/assistant-message/parseAssistantMessageV2.ts +++ b/src/core/assistant-message/parseAssistantMessageV2.ts @@ -46,14 +46,6 @@ export function parseAssistantMessageV2(assistantMessage: string): AssistantMess let currentParamValueStart = 0 // Index *after* the opening tag of the current param. let currentParamName: ToolParamName | undefined = undefined - // Track whether we are inside markdown code blocks or inline code to avoid treating textual mentions - // of tool tags (e.g. ) as actual tool invocations. - let insideCodeBlock = false // ``` fenced code block - let insideInlineCode = false // `inline code` - - // Helper to decide if we should parse for tool-related tags at the current position - const shouldParseToolTags = () => !insideCodeBlock && !insideInlineCode - // Precompute tags for faster lookups. const toolUseOpenTags = new Map() const toolParamOpenTags = new Map() @@ -71,39 +63,16 @@ export function parseAssistantMessageV2(assistantMessage: string): AssistantMess for (let i = 0; i < len; i++) { const currentCharIndex = i - // Detect fenced code block (```). - if (!insideInlineCode && i + 2 < len && assistantMessage.slice(i, i + 3) === "```") { - insideCodeBlock = !insideCodeBlock - i += 2 // Skip the two extra backticks. - continue - } - - // Detect inline code (`) when not inside a fenced code block and not - // part of triple backticks. - if (!insideCodeBlock && assistantMessage[i] === "`") { - insideInlineCode = !insideInlineCode - } - - // If we are in any kind of code context, treat everything as plain text. - if (!shouldParseToolTags()) { - // If we're not already tracking text content, start now. - if (!currentTextContent) { - currentTextContentStart = currentCharIndex - currentTextContent = { type: "text", content: "", partial: true } - } - - continue - } - // Parsing a tool parameter if (currentToolUse && currentParamName) { const closeTag = `` - - // Check if the string *ending* at index `i` matches the closing tag. + // Check if the string *ending* at index `i` matches the closing tag if ( currentCharIndex >= closeTag.length - 1 && - // Start checking from potential start of tag. - assistantMessage.startsWith(closeTag, currentCharIndex - closeTag.length + 1) + assistantMessage.startsWith( + closeTag, + currentCharIndex - closeTag.length + 1, // Start checking from potential start of tag. + ) ) { // Found the closing tag for the parameter. const value = assistantMessage @@ -206,23 +175,6 @@ export function parseAssistantMessageV2(assistantMessage: string): AssistantMess currentCharIndex >= tag.length - 1 && assistantMessage.startsWith(tag, currentCharIndex - tag.length + 1) ) { - // Check that this is likely an actual tool invocation and not - // an inline textual reference. - // We consider it an invocation only if the next non-whitespace - // character is a newline (\n or \r) or an opening angle bracket - // '<' (which would start the first parameter tag). - let j = currentCharIndex + 1 // Position after the closing '>' of the opening tag. - - while (j < len && assistantMessage[j] === " ") { - j++ - } - - const nextChar = assistantMessage[j] ?? "" - - if (nextChar && nextChar !== "<" && nextChar !== "\n" && nextChar !== "\r") { - // Treat as plain text, not a tool invocation. - continue - } // End current text block if one was active. if (currentTextContent) { currentTextContent.content = assistantMessage @@ -249,13 +201,22 @@ export function parseAssistantMessageV2(assistantMessage: string): AssistantMess .trim() if (potentialText.length > 0) { - contentBlocks.push({ type: "text", content: potentialText, partial: false }) + contentBlocks.push({ + type: "text", + content: potentialText, + partial: false, + }) } } // Start the new tool use. - // Assume partial until closing tag is found. - currentToolUse = { type: "tool_use", name: toolName, params: {}, partial: true } + currentToolUse = { + type: "tool_use", + name: toolName, + params: {}, + partial: true, // Assume partial until closing tag is found. + } + currentToolUseStart = currentCharIndex + 1 // Tool content starts after the opening tag. startedNewTool = true diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 2d2180e391..bdc041ef57 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -60,11 +60,7 @@ import { SYSTEM_PROMPT } from "../prompts/system" import { ToolRepetitionDetector } from "../tools/ToolRepetitionDetector" import { FileContextTracker } from "../context-tracking/FileContextTracker" import { RooIgnoreController } from "../ignore/RooIgnoreController" -import { - type AssistantMessageContent, - parseAssistantMessageV2 as parseAssistantMessage, - presentAssistantMessage, -} from "../assistant-message" +import { type AssistantMessageContent, parseAssistantMessage, presentAssistantMessage } from "../assistant-message" import { truncateConversationIfNeeded } from "../sliding-window" import { ClineProvider } from "../webview/ClineProvider" import { MultiSearchReplaceDiffStrategy } from "../diff/strategies/multi-search-replace" diff --git a/webview-ui/src/__tests__/utils/command-validation.test.ts b/webview-ui/src/__tests__/utils/command-validation.test.ts new file mode 100644 index 0000000000..d1e3fc6010 --- /dev/null +++ b/webview-ui/src/__tests__/utils/command-validation.test.ts @@ -0,0 +1,158 @@ +import { parseCommand, validateCommand } from "../../utils/command-validation" + +/** + * Tests for the command-validation module + * + * These tests include a reproduction of a bug where the shell-quote library + * used in parseCommand throws an error when parsing commands that contain + * the Bash $RANDOM variable in array indexing expressions. + * + * Error: "Bad substitution: levels[$RANDOM" + * + * The issue occurs specifically with complex expressions like: + * ${levels[$RANDOM % ${#levels[@]}]} + */ +describe("command-validation", () => { + describe("parseCommand", () => { + it("should correctly parse simple commands", () => { + const result = parseCommand("echo hello") + expect(result).toEqual(["echo hello"]) + }) + + it("should correctly parse commands with chaining operators", () => { + const result = parseCommand("echo hello && echo world") + expect(result).toEqual(["echo hello", "echo world"]) + }) + + it("should correctly parse commands with quotes", () => { + const result = parseCommand('echo "hello world"') + expect(result).toEqual(['echo "hello world"']) + }) + + it("should correctly parse commands with redirections", () => { + const result = parseCommand("echo hello 2>&1") + expect(result).toEqual(["echo hello 2>&1"]) + }) + + it("should correctly parse commands with subshells", () => { + const result = parseCommand("echo $(date)") + expect(result).toEqual(["echo", "date"]) + }) + + it("should not throw an error when parsing commands with simple array indexing", () => { + // Simple array indexing works fine + const commandWithArrayIndex = "value=${array[$index]}" + + expect(() => { + parseCommand(commandWithArrayIndex) + }).not.toThrow() + }) + + it("should not throw an error when parsing commands with $RANDOM in array index", () => { + // This test reproduces the specific bug reported in the error + const commandWithRandom = "level=${levels[$RANDOM % ${#levels[@]}]}" + + expect(() => { + parseCommand(commandWithRandom) + }).not.toThrow() + }) + + it("should not throw an error with simple $RANDOM variable", () => { + // Simple $RANDOM usage works fine + const commandWithRandom = "echo $RANDOM" + + expect(() => { + parseCommand(commandWithRandom) + }).not.toThrow() + }) + + it("should not throw an error with simple array indexing using $RANDOM", () => { + // Simple array indexing with $RANDOM works fine + const commandWithRandomIndex = "echo ${array[$RANDOM]}" + + expect(() => { + parseCommand(commandWithRandomIndex) + }).not.toThrow() + }) + + it("should not throw an error with complex array indexing using $RANDOM and arithmetic", () => { + // This test reproduces the exact expression from the original error + const commandWithComplexRandom = "echo ${levels[$RANDOM % ${#levels[@]}]}" + + expect(() => { + parseCommand(commandWithComplexRandom) + }).not.toThrow("Bad substitution") + }) + + it("should not throw an error when parsing the full log generator command", () => { + // This is the exact command from the original error message + const logGeneratorCommand = `while true; do \\ + levels=(INFO WARN ERROR DEBUG); \\ + msgs=("User logged in" "Connection timeout" "Processing request" "Cache miss" "Database query"); \\ + level=\${levels[$RANDOM % \${#levels[@]}]}; \\ + msg=\${msgs[$RANDOM % \${#msgs[@]}]}; \\ + echo "\$(date '+%Y-%m-%d %H:%M:%S') [$level] $msg"; \\ + sleep 1; \\ +done` + + // This reproduces the original error + expect(() => { + parseCommand(logGeneratorCommand) + }).not.toThrow("Bad substitution: levels[$RANDOM") + }) + + it("should not throw an error when parsing just the problematic part", () => { + // This isolates just the part mentioned in the error message + const problematicPart = "level=${levels[$RANDOM" + + expect(() => { + parseCommand(problematicPart) + }).not.toThrow("Bad substitution") + }) + }) + + describe("validateCommand", () => { + it("should validate allowed commands", () => { + const result = validateCommand("echo hello", ["echo"]) + expect(result).toBe(true) + }) + + it("should reject disallowed commands", () => { + const result = validateCommand("rm -rf /", ["echo", "ls"]) + expect(result).toBe(false) + }) + + it("should not fail validation for commands with simple $RANDOM variable", () => { + const commandWithRandom = "echo $RANDOM" + + expect(() => { + validateCommand(commandWithRandom, ["echo"]) + }).not.toThrow() + }) + + it("should not fail validation for commands with simple array indexing using $RANDOM", () => { + const commandWithRandomIndex = "echo ${array[$RANDOM]}" + + expect(() => { + validateCommand(commandWithRandomIndex, ["echo"]) + }).not.toThrow() + }) + + it("should return false for the full log generator command due to subshell detection", () => { + // This is the exact command from the original error message + const logGeneratorCommand = `while true; do \\ + levels=(INFO WARN ERROR DEBUG); \\ + msgs=("User logged in" "Connection timeout" "Processing request" "Cache miss" "Database query"); \\ + level=\${levels[$RANDOM % \${#levels[@]}]}; \\ + msg=\${msgs[$RANDOM % \${#msgs[@]}]}; \\ + echo "\$(date '+%Y-%m-%d %H:%M:%S') [$level] $msg"; \\ + sleep 1; \\ +done` + + // validateCommand should return false due to subshell detection + // without throwing an error + const result = validateCommand(logGeneratorCommand, ["while"]) + expect(result).toBe(false) + }) + }) +}) diff --git a/webview-ui/src/utils/__tests__/command-validation.test.ts b/webview-ui/src/utils/__tests__/command-validation.test.ts index 9d89beec46..c1c73548b9 100644 --- a/webview-ui/src/utils/__tests__/command-validation.test.ts +++ b/webview-ui/src/utils/__tests__/command-validation.test.ts @@ -1,8 +1,3 @@ -/* eslint-disable no-useless-escape */ -/* eslint-disable no-template-curly-in-string */ - -// npx jest src/utils/__tests__/command-validation.test.ts - import { parseCommand, isAllowedSingleCommand, validateCommand } from "../command-validation" describe("Command Validation", () => { @@ -124,160 +119,3 @@ describe("Command Validation", () => { }) }) }) - -/** - * Tests for the command-validation module - * - * These tests include a reproduction of a bug where the shell-quote library - * used in parseCommand throws an error when parsing commands that contain - * the Bash $RANDOM variable in array indexing expressions. - * - * Error: "Bad substitution: levels[$RANDOM" - * - * The issue occurs specifically with complex expressions like: - * ${levels[$RANDOM % ${#levels[@]}]} - */ -describe("command-validation", () => { - describe("parseCommand", () => { - it("should correctly parse simple commands", () => { - const result = parseCommand("echo hello") - expect(result).toEqual(["echo hello"]) - }) - - it("should correctly parse commands with chaining operators", () => { - const result = parseCommand("echo hello && echo world") - expect(result).toEqual(["echo hello", "echo world"]) - }) - - it("should correctly parse commands with quotes", () => { - const result = parseCommand('echo "hello world"') - expect(result).toEqual(['echo "hello world"']) - }) - - it("should correctly parse commands with redirections", () => { - const result = parseCommand("echo hello 2>&1") - expect(result).toEqual(["echo hello 2>&1"]) - }) - - it("should correctly parse commands with subshells", () => { - const result = parseCommand("echo $(date)") - expect(result).toEqual(["echo", "date"]) - }) - - it("should not throw an error when parsing commands with simple array indexing", () => { - // Simple array indexing works fine - const commandWithArrayIndex = "value=${array[$index]}" - - expect(() => { - parseCommand(commandWithArrayIndex) - }).not.toThrow() - }) - - it("should not throw an error when parsing commands with $RANDOM in array index", () => { - // This test reproduces the specific bug reported in the error - const commandWithRandom = "level=${levels[$RANDOM % ${#levels[@]}]}" - - expect(() => { - parseCommand(commandWithRandom) - }).not.toThrow() - }) - - it("should not throw an error with simple $RANDOM variable", () => { - // Simple $RANDOM usage works fine - const commandWithRandom = "echo $RANDOM" - - expect(() => { - parseCommand(commandWithRandom) - }).not.toThrow() - }) - - it("should not throw an error with simple array indexing using $RANDOM", () => { - // Simple array indexing with $RANDOM works fine - const commandWithRandomIndex = "echo ${array[$RANDOM]}" - - expect(() => { - parseCommand(commandWithRandomIndex) - }).not.toThrow() - }) - - it("should not throw an error with complex array indexing using $RANDOM and arithmetic", () => { - // This test reproduces the exact expression from the original error - const commandWithComplexRandom = "echo ${levels[$RANDOM % ${#levels[@]}]}" - - expect(() => { - parseCommand(commandWithComplexRandom) - }).not.toThrow("Bad substitution") - }) - - it("should not throw an error when parsing the full log generator command", () => { - // This is the exact command from the original error message - const logGeneratorCommand = `while true; do \\ - levels=(INFO WARN ERROR DEBUG); \\ - msgs=("User logged in" "Connection timeout" "Processing request" "Cache miss" "Database query"); \\ - level=\${levels[$RANDOM % \${#levels[@]}]}; \\ - msg=\${msgs[$RANDOM % \${#msgs[@]}]}; \\ - echo "\$(date '+%Y-%m-%d %H:%M:%S') [$level] $msg"; \\ - sleep 1; \\ -done` - - // This reproduces the original error - expect(() => { - parseCommand(logGeneratorCommand) - }).not.toThrow("Bad substitution: levels[$RANDOM") - }) - - it("should not throw an error when parsing just the problematic part", () => { - // This isolates just the part mentioned in the error message - const problematicPart = "level=${levels[$RANDOM" - - expect(() => { - parseCommand(problematicPart) - }).not.toThrow("Bad substitution") - }) - }) - - describe("validateCommand", () => { - it("should validate allowed commands", () => { - const result = validateCommand("echo hello", ["echo"]) - expect(result).toBe(true) - }) - - it("should reject disallowed commands", () => { - const result = validateCommand("rm -rf /", ["echo", "ls"]) - expect(result).toBe(false) - }) - - it("should not fail validation for commands with simple $RANDOM variable", () => { - const commandWithRandom = "echo $RANDOM" - - expect(() => { - validateCommand(commandWithRandom, ["echo"]) - }).not.toThrow() - }) - - it("should not fail validation for commands with simple array indexing using $RANDOM", () => { - const commandWithRandomIndex = "echo ${array[$RANDOM]}" - - expect(() => { - validateCommand(commandWithRandomIndex, ["echo"]) - }).not.toThrow() - }) - - it("should return false for the full log generator command due to subshell detection", () => { - // This is the exact command from the original error message - const logGeneratorCommand = `while true; do \\ - levels=(INFO WARN ERROR DEBUG); \\ - msgs=("User logged in" "Connection timeout" "Processing request" "Cache miss" "Database query"); \\ - level=\${levels[$RANDOM % \${#levels[@]}]}; \\ - msg=\${msgs[$RANDOM % \${#msgs[@]}]}; \\ - echo "\$(date '+%Y-%m-%d %H:%M:%S') [$level] $msg"; \\ - sleep 1; \\ -done` - - // validateCommand should return false due to subshell detection - // without throwing an error - const result = validateCommand(logGeneratorCommand, ["while"]) - expect(result).toBe(false) - }) - }) -})