mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
Accidental execution of tool syntax fix (#3456)
Co-authored-by: cte <cestreich@gmail.com>
This commit is contained in:
xyOz
hannesrudolph
parent
16b9ecfe30
commit
1bc4a2db16
8 changed files with 442 additions and 183 deletions
|
|
@ -335,6 +335,158 @@ const isEmptyTextContent = (block: AssistantMessageContent) =>
|
|||
expect(result[5].type).toBe("tool_use")
|
||||
expect((result[5] as ToolUse).name).toBe("execute_command")
|
||||
})
|
||||
|
||||
it("treats inline <read_file> mention as plain text", () => {
|
||||
const message = "Use the `<read_file>` 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:",
|
||||
"```",
|
||||
"<read_file>",
|
||||
"<path>demo.txt</path>",
|
||||
"</read_file>",
|
||||
"```",
|
||||
].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 = ["<read_file>", "<path>demo.txt</path>", "</read_file>"].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:",
|
||||
"> ```",
|
||||
"> <read_file><path>test.txt</path></read_file>",
|
||||
"> ```",
|
||||
].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 \\`<read_file>\\` tag"
|
||||
|
||||
const result = parser(message)
|
||||
expect(result).toHaveLength(1)
|
||||
expect(result[0].type).toBe("text")
|
||||
})
|
||||
|
||||
it("handles incomplete code blocks", () => {
|
||||
const message = [
|
||||
"```javascript",
|
||||
"<read_file><path>test.txt</path></read_file>",
|
||||
// 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 = "<!-- <read_file><path>test.txt</path></read_file> -->"
|
||||
|
||||
// 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("<!--")
|
||||
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("-->")
|
||||
})
|
||||
|
||||
it("handles tool tags with HTML-like attributes", () => {
|
||||
const message = '<read_file class="example"><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")
|
||||
})
|
||||
|
||||
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 = "**<read_file>**<path>test.txt</path></read_file>"
|
||||
|
||||
// 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](<read_file><path>test.txt</path></read_file>)"
|
||||
|
||||
// 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 = "<execute_command><command>"
|
||||
for (let i = 0; i < 10; i++) {
|
||||
message += `echo "Level ${i}" && `
|
||||
}
|
||||
message += 'echo "Done"</command></execute_command>'
|
||||
|
||||
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")
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -81,16 +81,13 @@ const runBenchmark = () => {
|
|||
const namePadding = maxNameLength + 2
|
||||
|
||||
console.log(
|
||||
`| ${"Test Case".padEnd(namePadding)} | V1 Time (ms) | V2 Time (ms) | V1/V2 Ratio | V1 Heap (bytes) | V2 Heap (bytes) |`,
|
||||
)
|
||||
console.log(
|
||||
`| ${"-".repeat(namePadding)} | ------------ | ------------ | ----------- | ---------------- | ---------------- |`,
|
||||
`| ${"Test Case".padEnd(namePadding)} | V1 Time (ms) | V2 Time (ms) | V1 Heap (bytes) | V2 Heap (bytes) |`,
|
||||
)
|
||||
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)
|
||||
|
|
@ -99,7 +96,6 @@ 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)} |`,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -1,2 +1,3 @@
|
|||
export { type AssistantMessageContent, parseAssistantMessage } from "./parseAssistantMessage"
|
||||
export { presentAssistantMessage } from "./presentAssistantMessage"
|
||||
export { parseAssistantMessageV2 } from "./parseAssistantMessageV2"
|
||||
|
|
|
|||
|
|
@ -13,10 +13,52 @@ 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. <read_file>) 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)
|
||||
|
|
@ -45,6 +87,7 @@ 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.
|
||||
|
|
@ -89,6 +132,25 @@ 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",
|
||||
|
|
@ -151,5 +213,6 @@ export function parseAssistantMessage(assistantMessage: string): AssistantMessag
|
|||
contentBlocks.push(currentTextContent)
|
||||
}
|
||||
|
||||
return contentBlocks
|
||||
// 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))
|
||||
}
|
||||
|
|
|
|||
|
|
@ -46,6 +46,14 @@ 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. <read_file>) 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<string, ToolName>()
|
||||
const toolParamOpenTags = new Map<string, ToolParamName>()
|
||||
|
|
@ -63,16 +71,39 @@ 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 = `</${currentParamName}>`
|
||||
// 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 &&
|
||||
assistantMessage.startsWith(
|
||||
closeTag,
|
||||
currentCharIndex - closeTag.length + 1, // Start checking from potential start of tag.
|
||||
)
|
||||
// Start checking from potential start of tag.
|
||||
assistantMessage.startsWith(closeTag, currentCharIndex - closeTag.length + 1)
|
||||
) {
|
||||
// Found the closing tag for the parameter.
|
||||
const value = assistantMessage
|
||||
|
|
@ -175,6 +206,23 @@ 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
|
||||
|
|
@ -201,22 +249,13 @@ 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.
|
||||
currentToolUse = {
|
||||
type: "tool_use",
|
||||
name: toolName,
|
||||
params: {},
|
||||
partial: true, // Assume partial until closing tag is found.
|
||||
}
|
||||
|
||||
// Assume partial until closing tag is found.
|
||||
currentToolUse = { type: "tool_use", name: toolName, params: {}, partial: true }
|
||||
currentToolUseStart = currentCharIndex + 1 // Tool content starts after the opening tag.
|
||||
startedNewTool = true
|
||||
|
||||
|
|
|
|||
|
|
@ -60,7 +60,11 @@ 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, parseAssistantMessage, presentAssistantMessage } from "../assistant-message"
|
||||
import {
|
||||
type AssistantMessageContent,
|
||||
parseAssistantMessageV2 as parseAssistantMessage,
|
||||
presentAssistantMessage,
|
||||
} from "../assistant-message"
|
||||
import { truncateConversationIfNeeded } from "../sliding-window"
|
||||
import { ClineProvider } from "../webview/ClineProvider"
|
||||
import { MultiSearchReplaceDiffStrategy } from "../diff/strategies/multi-search-replace"
|
||||
|
|
|
|||
|
|
@ -1,158 +0,0 @@
|
|||
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)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
@ -1,3 +1,8 @@
|
|||
/* 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", () => {
|
||||
|
|
@ -119,3 +124,160 @@ 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)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue