fix: prevent full command chains from being extracted as patterns

Fixed the pattern extraction logic to only extract individual commands and their subcommands, never full command chains. This prevents issues where complex piped or chained commands would be incorrectly treated as single patterns.

- Updated extractCommandPatterns to stop at command boundaries (&&, ||, |, ;)
- Added comprehensive tests for command chain scenarios
- Ensures only atomic commands are whitelisted, not entire command sequences
This commit is contained in:
hannesrudolph 2025-07-15 13:33:18 -06:00
parent 0a067f005a
commit a280f876eb
3 changed files with 136 additions and 264 deletions

View file

@ -9,7 +9,7 @@ import { safeJsonParse } from "@roo/safeJsonParse"
import { vscode } from "@src/utils/vscode"
import { parseCommandAndOutput } from "@src/utils/commandParsing"
import { extractCommandPattern, getPatternDescription } from "@src/utils/commandPatterns"
import { extractCommandPatterns, getPatternDescription } from "@src/utils/commandPatterns"
import { useExtensionState } from "@src/context/ExtensionStateContext"
import { useAppTranslation } from "@src/i18n/TranslationContext"
import { cn } from "@src/lib/utils"
@ -61,97 +61,14 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec
// If no LLM suggestions but we have a command, extract patterns programmatically
if (!command?.trim()) return []
// Check if this is a chained command
const operators = ["&&", "||", ";", "|"]
const patterns: Array<{ pattern: string; description: string }> = []
// Use the new extractCommandPatterns function which handles all parsing
const extractedPatterns = extractCommandPatterns(command)
// Split by operators while respecting quotes
let inSingleQuote = false
let inDoubleQuote = false
let escapeNext = false
let currentCommand = ""
let i = 0
while (i < command.length) {
const char = command[i]
if (escapeNext) {
currentCommand += char
escapeNext = false
i++
continue
}
if (char === "\\") {
escapeNext = true
currentCommand += char
i++
continue
}
if (char === "'" && !inDoubleQuote) {
inSingleQuote = !inSingleQuote
currentCommand += char
i++
continue
}
if (char === '"' && !inSingleQuote) {
inDoubleQuote = !inDoubleQuote
currentCommand += char
i++
continue
}
// Check for operators outside quotes
if (!inSingleQuote && !inDoubleQuote) {
let foundOperator = false
for (const op of operators) {
if (command.substring(i, i + op.length) === op) {
// Found an operator, process the current command
const trimmedCommand = currentCommand.trim()
if (trimmedCommand) {
// Extract pattern for the command
const pattern = extractCommandPattern(trimmedCommand)
if (pattern) {
patterns.push({
pattern,
description: getPatternDescription(pattern),
})
}
}
currentCommand = ""
i += op.length
foundOperator = true
break
}
}
if (foundOperator) continue
}
currentCommand += char
i++
}
// Process the last command
const trimmedCommand = currentCommand.trim()
if (trimmedCommand) {
// Extract pattern for the command
const pattern = extractCommandPattern(trimmedCommand)
if (pattern) {
patterns.push({
pattern,
description: getPatternDescription(pattern),
})
}
}
// Remove duplicates
const uniquePatterns = patterns.filter(
(item, index, self) => index === self.findIndex((p) => p.pattern === item.pattern),
)
return uniquePatterns
// Convert to the expected format with descriptions
return extractedPatterns.map((pattern) => ({
pattern,
description: getPatternDescription(pattern),
}))
}, [command, suggestions])
// The command's output can either come from the text associated with the

View file

@ -1,197 +1,142 @@
import { describe, it, expect } from "vitest"
import { extractCommandPatterns, extractCommandPattern, getPatternDescription } from "../commandPatterns"
import { extractCommandPatterns, getPatternDescription } from "../commandPatterns"
describe("commandPatterns", () => {
describe("extractCommandPatterns", () => {
it("should handle empty or null commands", () => {
expect(extractCommandPatterns("")).toEqual([])
expect(extractCommandPatterns(" ")).toEqual([])
expect(extractCommandPatterns(null as any)).toEqual([])
expect(extractCommandPatterns(undefined as any)).toEqual([])
describe("extractCommandPatterns", () => {
describe("command chains", () => {
it("should not include the full command chain as a pattern", () => {
const patterns = extractCommandPatterns("cd backend && npm install")
expect(patterns).not.toContain("cd backend && npm install")
expect(patterns).toContain("cd")
expect(patterns).toContain("npm")
expect(patterns).toContain("npm install")
})
it("should extract simple commands", () => {
expect(extractCommandPatterns("ls")).toEqual(["ls"])
expect(extractCommandPatterns("pwd")).toEqual(["pwd"])
expect(extractCommandPatterns("echo hello")).toEqual(["echo"])
it("should handle multiple operators", () => {
const patterns = extractCommandPatterns("git status && git add . || git commit -m 'test'")
expect(patterns).not.toContain("git status && git add . || git commit -m 'test'")
expect(patterns).toContain("git")
expect(patterns).toContain("git status")
expect(patterns).toContain("git add")
expect(patterns).toContain("git commit")
})
it("should handle npm commands correctly", () => {
expect(extractCommandPatterns("npm install")).toEqual(["npm install"])
expect(extractCommandPatterns("npm run build")).toEqual(["npm run"])
expect(extractCommandPatterns("npm run test:unit")).toEqual(["npm run"])
expect(extractCommandPatterns("npm test")).toEqual(["npm test"])
expect(extractCommandPatterns("npm run build -- --watch")).toEqual(["npm run"])
it("should handle pipe operators", () => {
const patterns = extractCommandPatterns("grep map | head")
expect(patterns).not.toContain("grep map | head")
expect(patterns).toContain("grep")
expect(patterns).toContain("head")
})
it("should handle yarn/pnpm/bun commands", () => {
expect(extractCommandPatterns("yarn install")).toEqual(["yarn install"])
expect(extractCommandPatterns("pnpm run dev")).toEqual(["pnpm run"])
expect(extractCommandPatterns("bun test")).toEqual(["bun test"])
it("should handle semicolon separators", () => {
const patterns = extractCommandPatterns("cd src; npm test")
expect(patterns).not.toContain("cd src; npm test")
expect(patterns).toContain("cd")
expect(patterns).toContain("npm")
expect(patterns).toContain("npm test")
})
it("should handle git commands", () => {
expect(extractCommandPatterns("git status")).toEqual(["git status"])
expect(extractCommandPatterns('git commit -m "message"')).toEqual(["git commit"])
expect(extractCommandPatterns("git push origin main")).toEqual(["git push"])
expect(extractCommandPatterns("git log --oneline")).toEqual(["git log"])
it("should handle complex chains from real examples", () => {
const patterns = extractCommandPatterns(
"unzip -l builds/roo-cline-3.21.5-error-boundary-component.vsix|grep map|head",
)
expect(patterns).not.toContain(
"unzip -l builds/roo-cline-3.21.5-error-boundary-component.vsix|grep map|head",
)
expect(patterns).toContain("unzip")
expect(patterns).toContain("grep")
expect(patterns).toContain("head")
})
it("should handle docker/kubectl/helm commands", () => {
expect(extractCommandPatterns("docker run nginx")).toEqual(["docker run"])
expect(extractCommandPatterns("kubectl get pods")).toEqual(["kubectl get"])
expect(extractCommandPatterns("helm install myapp ./chart")).toEqual(["helm install"])
it("should handle chains with spaces around operators", () => {
const patterns = extractCommandPatterns("git diff | xclip")
expect(patterns).not.toContain("git diff | xclip")
expect(patterns).toContain("git")
expect(patterns).toContain("git diff")
expect(patterns).toContain("xclip")
})
})
describe("single commands", () => {
it("should extract base command", () => {
const patterns = extractCommandPatterns("git")
expect(patterns).toEqual(["git"])
})
it("should handle make commands", () => {
expect(extractCommandPatterns("make build")).toEqual(["make build"])
expect(extractCommandPatterns("make test")).toEqual(["make test"])
expect(extractCommandPatterns("make -j4 all")).toEqual(["make"])
it("should extract command with subcommand", () => {
const patterns = extractCommandPatterns("git status")
expect(patterns).toContain("git")
expect(patterns).toContain("git status")
})
it("should handle interpreter commands", () => {
expect(extractCommandPatterns("python script.py")).toEqual(["python"])
expect(extractCommandPatterns("python3 -m venv env")).toEqual(["python3"])
expect(extractCommandPatterns("node index.js --port 3000")).toEqual(["node"])
expect(extractCommandPatterns("ruby app.rb")).toEqual(["ruby"])
})
it("should handle dangerous commands", () => {
expect(extractCommandPatterns("rm -rf node_modules")).toEqual(["rm"])
expect(extractCommandPatterns("chmod 755 script.sh")).toEqual(["chmod"])
expect(extractCommandPatterns('find . -name "*.log" -delete')).toEqual(["find"])
})
it("should handle cd commands", () => {
expect(extractCommandPatterns("cd /home/user")).toEqual(["cd"])
expect(extractCommandPatterns("cd ..")).toEqual(["cd"])
expect(extractCommandPatterns("cd ~/projects")).toEqual(["cd"])
})
it("should handle chained commands with &&", () => {
const patterns = extractCommandPatterns("npm install && npm run build")
expect(patterns).toEqual(["npm install", "npm run"])
})
it("should handle chained commands with ||", () => {
const patterns = extractCommandPatterns('npm test || echo "Tests failed"')
expect(patterns).toEqual(["echo", "npm test"])
})
it("should handle chained commands with ;", () => {
const patterns = extractCommandPatterns("cd /tmp; ls -la; pwd")
expect(patterns).toEqual(["cd", "ls", "pwd"])
})
it("should handle piped commands", () => {
const patterns = extractCommandPatterns("ps aux | grep node")
expect(patterns).toEqual(["grep", "ps"])
})
it("should handle complex chained commands", () => {
const patterns = extractCommandPatterns('git pull && npm install && npm run build || echo "Build failed"')
expect(patterns).toEqual(["echo", "git pull", "npm install", "npm run"])
})
it("should handle quoted arguments correctly", () => {
expect(extractCommandPatterns('echo "hello world"')).toEqual(["echo"])
expect(extractCommandPatterns("echo 'hello world'")).toEqual(["echo"])
expect(extractCommandPatterns('git commit -m "feat: add new feature"')).toEqual(["git commit"])
})
it("should handle escaped characters", () => {
expect(extractCommandPatterns("echo hello\\ world")).toEqual(["echo"])
expect(extractCommandPatterns('echo "hello \\"world\\""')).toEqual(["echo"])
})
it("should handle environment variables", () => {
expect(extractCommandPatterns("NODE_ENV=production npm start")).toEqual(["npm start"])
expect(extractCommandPatterns("PORT=3000 node server.js")).toEqual(["node"])
it("should handle npm run commands", () => {
const patterns = extractCommandPatterns("npm run test")
expect(patterns).toContain("npm")
expect(patterns).toContain("npm run")
// Should stop at 'run' to allow any script
})
it("should handle script files", () => {
expect(extractCommandPatterns("./deploy.sh")).toEqual(["./deploy.sh"])
expect(extractCommandPatterns("/usr/local/bin/script.py")).toEqual(["/usr/local/bin/script.py"])
expect(extractCommandPatterns("./scripts/test.js --verbose")).toEqual(["./scripts/test.js"])
const patterns = extractCommandPatterns("../repackage.sh")
expect(patterns).toEqual(["../repackage.sh"])
})
it("should handle complex npm scripts", () => {
expect(extractCommandPatterns("npm run build:prod -- --source-maps")).toEqual(["npm run"])
expect(extractCommandPatterns("npm run test:coverage -- --watch")).toEqual(["npm run"])
it("should stop at flags", () => {
const patterns = extractCommandPatterns("git diff --stat")
expect(patterns).toContain("git")
expect(patterns).toContain("git diff")
expect(patterns).not.toContain("git diff --stat")
})
it("should return unique sorted patterns", () => {
const patterns = extractCommandPatterns("npm install && npm install && npm run build")
expect(patterns).toEqual(["npm install", "npm run"])
})
it("should handle commands with glob patterns", () => {
expect(extractCommandPatterns("rm *.log")).toEqual(["rm"])
expect(extractCommandPatterns("ls *.{js,ts}")).toEqual(["ls"])
})
it("should handle commands with redirects", () => {
expect(extractCommandPatterns('echo "test" > file.txt')).toEqual(["echo"])
expect(extractCommandPatterns("cat file.txt >> output.log")).toEqual(["cat"])
})
it("should handle subshells and command substitution", () => {
expect(extractCommandPatterns("echo $(date)")).toEqual(["echo"])
expect(extractCommandPatterns("echo `pwd`")).toEqual(["echo"])
it("should handle environment variables", () => {
const patterns = extractCommandPatterns("NODE_ENV=production npm start")
expect(patterns).toContain("npm")
expect(patterns).toContain("npm start")
expect(patterns).not.toContain("NODE_ENV=production")
})
})
describe("extractCommandPattern", () => {
it("should return the first pattern for backward compatibility", () => {
expect(extractCommandPattern("npm install && npm run build")).toBe("npm install")
expect(extractCommandPattern("git status")).toBe("git status")
expect(extractCommandPattern("")).toBe("")
})
})
describe("getPatternDescription", () => {
it("should describe npm patterns", () => {
expect(getPatternDescription("npm run")).toBe("all npm run scripts")
expect(getPatternDescription("npm install")).toBe("npm install commands")
expect(getPatternDescription("npm test")).toBe("npm test commands")
describe("edge cases", () => {
it("should handle empty command", () => {
const patterns = extractCommandPatterns("")
expect(patterns).toEqual([])
})
it("should describe git patterns", () => {
expect(getPatternDescription("git commit")).toBe("git commit commands")
expect(getPatternDescription("git push")).toBe("git push commands")
it("should handle whitespace only", () => {
const patterns = extractCommandPatterns(" ")
expect(patterns).toEqual([])
})
it("should describe script files", () => {
expect(getPatternDescription("./deploy.sh")).toBe("this specific script")
expect(getPatternDescription("/usr/bin/script.py")).toBe("this specific script")
it("should handle quoted strings with operators", () => {
const patterns = extractCommandPatterns('echo "test && test"')
expect(patterns).toContain("echo")
expect(patterns).not.toContain("test")
})
it("should describe interpreter patterns", () => {
expect(getPatternDescription("python")).toBe("python scripts")
expect(getPatternDescription("node")).toBe("node scripts")
expect(getPatternDescription("ruby")).toBe("ruby scripts")
})
it("should describe docker/kubectl patterns", () => {
expect(getPatternDescription("docker run")).toBe("docker run commands")
expect(getPatternDescription("kubectl get")).toBe("kubectl get commands")
})
it("should describe make patterns", () => {
expect(getPatternDescription("make build")).toBe("make build target")
expect(getPatternDescription("make test")).toBe("make test target")
})
it("should describe cd pattern", () => {
expect(getPatternDescription("cd")).toBe("directory navigation")
})
it("should handle empty patterns", () => {
expect(getPatternDescription("")).toBe("")
})
it("should provide default description", () => {
expect(getPatternDescription("custom-command")).toBe("custom-command commands")
it("should return sorted unique patterns", () => {
const patterns = extractCommandPatterns("git status && git add && git status")
expect(patterns).toEqual(["git", "git add", "git status"])
})
})
})
describe("getPatternDescription", () => {
it("should describe npm run patterns", () => {
expect(getPatternDescription("npm run")).toBe("all npm run scripts")
})
it("should describe git patterns", () => {
expect(getPatternDescription("git status")).toBe("git status commands")
})
it("should describe script files", () => {
expect(getPatternDescription("./build.sh")).toBe("this specific script")
})
it("should describe cd", () => {
expect(getPatternDescription("cd")).toBe("directory navigation")
})
it("should describe generic commands", () => {
expect(getPatternDescription("ls")).toBe("ls commands")
})
})

View file

@ -18,9 +18,11 @@ export function extractCommandPatterns(command: string): string[] {
const commands = splitByOperators(command, chainOperators)
for (const cmd of commands) {
const pattern = extractSingleCommandPattern(cmd.trim())
if (pattern) {
patterns.add(pattern)
const cmdPatterns = extractSingleCommandPattern(cmd.trim())
for (const pattern of cmdPatterns) {
if (pattern) {
patterns.add(pattern)
}
}
}
@ -100,16 +102,18 @@ function splitByOperators(command: string, operators: string[]): string[] {
}
/**
* Extract pattern from a single command (not chained)
* Extract patterns from a single command (not chained)
* Returns an array of patterns instead of a single pattern
*/
function extractSingleCommandPattern(command: string): string {
if (!command) return ""
function extractSingleCommandPattern(command: string): string[] {
if (!command) return []
try {
const parsed = parse(command)
if (parsed.length === 0) return ""
if (parsed.length === 0) return []
const patterns: string[] = []
const allPatterns: string[] = []
let i = 0
while (i < parsed.length) {
@ -159,9 +163,10 @@ function extractSingleCommandPattern(command: string): string {
strToken.endsWith(".js") ||
strToken.endsWith(".rb")
) {
return strToken
return [strToken]
}
patterns.push(strToken)
allPatterns.push(strToken) // Add base command
i++
continue
}
@ -185,6 +190,7 @@ function extractSingleCommandPattern(command: string): string {
// Include subcommand
if (!strToken.startsWith("-") && !strToken.includes("/")) {
patterns.push(strToken)
allPatterns.push(patterns.join(" "))
// For 'run' commands, stop here to allow any script
if (strToken === "run") {
break
@ -198,6 +204,7 @@ function extractSingleCommandPattern(command: string): string {
else if (baseCmd === "git") {
if (!strToken.startsWith("-")) {
patterns.push(strToken)
allPatterns.push(patterns.join(" "))
}
break
}
@ -206,6 +213,7 @@ function extractSingleCommandPattern(command: string): string {
else if (["docker", "kubectl", "helm"].includes(baseCmd)) {
if (!strToken.startsWith("-")) {
patterns.push(strToken)
allPatterns.push(patterns.join(" "))
}
break
}
@ -214,6 +222,7 @@ function extractSingleCommandPattern(command: string): string {
else if (baseCmd === "make") {
if (!strToken.startsWith("-")) {
patterns.push(strToken)
allPatterns.push(patterns.join(" "))
}
break
}
@ -257,11 +266,11 @@ function extractSingleCommandPattern(command: string): string {
i++
}
return patterns.join(" ")
return allPatterns
} catch (_error) {
// If parsing fails, fall back to simple first token
const tokens = command.split(/\s+/)
return tokens[0] || ""
return tokens[0] ? [tokens[0]] : []
}
}
@ -331,5 +340,6 @@ export function getPatternDescription(pattern: string): string {
*/
export function extractCommandPattern(command: string): string {
const patterns = extractCommandPatterns(command)
return patterns[0] || ""
// Return the most specific pattern (usually the last one)
return patterns[patterns.length - 1] || ""
}