From 3e5cc319a30729dc3b47b558396e390db126cd28 Mon Sep 17 00:00:00 2001 From: hannesrudolph Date: Tue, 8 Jul 2025 16:01:01 -0600 Subject: [PATCH] feat: improve command whitelisting UI/UX similar to MCP tools - Remove 'Always Allow' button from initial command approval dialog - Add integrated whitelist functionality to CommandExecution component - Implement collapsible 'Add to Allowed Auto-Execute Commands' section - Support granular command patterns for npm and other commands - Handle chained commands with individual checkboxes - Update extractCommandPattern to avoid wildcards for better control - Add comprehensive tests for command pattern extraction --- webview-ui/src/components/chat/ChatView.tsx | 105 ++------- .../src/components/chat/CommandExecution.tsx | 207 +++++++++++++++++- .../__tests__/extract-command-pattern.spec.ts | 42 ++-- .../src/utils/extract-command-pattern.ts | 35 ++- 4 files changed, 264 insertions(+), 125 deletions(-) diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index c188576047..7f44a89239 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -25,7 +25,6 @@ import { ProfileValidator } from "@roo/ProfileValidator" import { vscode } from "@src/utils/vscode" import { validateCommand } from "@src/utils/command-validation" -import { extractCommandPattern, getPatternDescription } from "@src/utils/extract-command-pattern" import { buildDocLink } from "@src/utils/docLinks" import { useAppTranslation } from "@src/i18n/TranslationContext" import { useExtensionState } from "@src/context/ExtensionStateContext" @@ -1657,87 +1656,31 @@ const ChatViewComponent: React.ForwardRefRenderFunction ) : ( <> - {/* Command approval with auto-approve pattern */} + {/* Command approval - simplified layout */} {clineAsk === "command" && !isStreaming ? ( -
- {/* Top row: Run Command and Reject */} -
- - - handlePrimaryButtonClick(inputValue, selectedImages) - }> - {primaryButtonText} - - - - - handleSecondaryButtonClick(inputValue, selectedImages) - }> - {secondaryButtonText} - - -
- {/* Bottom row: Auto-approve pattern */} -
-
- {(() => { - const commandMessage = findLast( - messagesRef.current, - (msg) => msg.type === "ask" && msg.ask === "command", - ) - const commandText = commandMessage?.text || "" - const pattern = extractCommandPattern(commandText) - return pattern || commandText - })()} -
- { - const commandMessage = findLast( - messagesRef.current, - (msg) => msg.type === "ask" && msg.ask === "command", - ) - const commandText = commandMessage?.text || "" - const pattern = extractCommandPattern(commandText) - const description = getPatternDescription(pattern) - return pattern - ? `${t("chat:alwaysAllow.tooltip")} Will whitelist: "${pattern}" (${description})` - : t("chat:alwaysAllow.tooltip") - })()}> - { - // Extract the command pattern - const commandMessage = findLast( - messagesRef.current, - (msg) => msg.type === "ask" && msg.ask === "command", - ) - const commandText = commandMessage?.text || "" - const pattern = extractCommandPattern(commandText) - - // Add to whitelist without running - vscode.postMessage({ - type: "addToWhitelist", - pattern: pattern, - }) - - // Clear the ask state - setSendingDisabled(true) - setClineAsk(undefined) - setEnableButtons(false) - }}> - {t("chat:alwaysAllow.title")} - - -
+
+ + + handlePrimaryButtonClick(inputValue, selectedImages) + }> + {primaryButtonText} + + + + + handleSecondaryButtonClick(inputValue, selectedImages) + }> + {secondaryButtonText} + +
) : ( /* Standard two button layout */ diff --git a/webview-ui/src/components/chat/CommandExecution.tsx b/webview-ui/src/components/chat/CommandExecution.tsx index 8c92ec7e7b..77ae87a1c1 100644 --- a/webview-ui/src/components/chat/CommandExecution.tsx +++ b/webview-ui/src/components/chat/CommandExecution.tsx @@ -1,6 +1,7 @@ import { useCallback, useState, memo, useMemo } from "react" import { useEvent } from "react-use" import { ChevronDown, Skull } from "lucide-react" +import { VSCodeCheckbox } from "@vscode/webview-ui-toolkit/react" import { CommandExecutionStatus, commandExecutionStatusSchema } from "@roo-code/types" @@ -9,6 +10,7 @@ import { safeJsonParse } from "@roo/safeJsonParse" import { COMMAND_OUTPUT_STRING } from "@roo/combineCommandSequences" import { vscode } from "@src/utils/vscode" +import { extractCommandPattern, getPatternDescription } from "@src/utils/extract-command-pattern" import { useExtensionState } from "@src/context/ExtensionStateContext" import { cn } from "@src/lib/utils" import { Button } from "@src/components/ui" @@ -22,7 +24,7 @@ interface CommandExecutionProps { } export const CommandExecution = ({ executionId, text, icon, title }: CommandExecutionProps) => { - const { terminalShellIntegrationDisabled = false } = useExtensionState() + const { terminalShellIntegrationDisabled = false, allowedCommands = [] } = useExtensionState() const { command, output: parsedOutput } = useMemo(() => parseCommandAndOutput(text), [text]) @@ -31,6 +33,159 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec const [isExpanded, setIsExpanded] = useState(terminalShellIntegrationDisabled) const [streamingOutput, setStreamingOutput] = useState("") const [status, setStatus] = useState(null) + const [isPatternSectionExpanded, setIsPatternSectionExpanded] = useState(false) + + // Extract command patterns for whitelisting + // For chained commands, extract individual patterns + const commandPatterns = useMemo(() => { + if (!command?.trim()) return [] + + // Check if this is a chained command + const operators = ["&&", "||", ";", "|"] + const patterns: Array<{ pattern: string; description: string }> = [] + + // 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) { + // For npm commands, generate multiple pattern options + if (trimmedCommand.startsWith("npm ")) { + // Add the specific pattern + const specificPattern = extractCommandPattern(trimmedCommand) + if (specificPattern) { + patterns.push({ + pattern: specificPattern, + description: getPatternDescription(specificPattern), + }) + } + + // Add broader npm patterns + if (trimmedCommand.startsWith("npm run ")) { + // Add "npm run" pattern + patterns.push({ + pattern: "npm run", + description: "Allow all npm run commands", + }) + } + + // Add "npm" pattern + patterns.push({ + pattern: "npm", + description: "Allow all npm commands", + }) + } else { + // For non-npm commands, just add the extracted pattern + 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) { + // For npm commands, generate multiple pattern options + if (trimmedCommand.startsWith("npm ")) { + // Add the specific pattern + const specificPattern = extractCommandPattern(trimmedCommand) + if (specificPattern) { + patterns.push({ + pattern: specificPattern, + description: getPatternDescription(specificPattern), + }) + } + + // Add broader npm patterns + if (trimmedCommand.startsWith("npm run ")) { + // Add "npm run" pattern + patterns.push({ + pattern: "npm run", + description: "Allow all npm run commands", + }) + } + + // Add "npm" pattern + patterns.push({ + pattern: "npm", + description: "Allow all npm commands", + }) + } else { + // For non-npm commands, just add the extracted pattern + 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 + }, [command]) // The command's output can either come from the text associated with the // task message (this is the case for completed commands) or from the @@ -73,6 +228,23 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec useEvent("message", onMessage) + const handleAllowPatternChange = useCallback( + (pattern: string) => { + if (!pattern) return + + const isWhitelisted = allowedCommands.includes(pattern) + const updatedAllowedCommands = isWhitelisted + ? allowedCommands.filter((p) => p !== pattern) + : Array.from(new Set([...allowedCommands, pattern])) + + vscode.postMessage({ + type: "allowedCommands", + commands: updatedAllowedCommands, + }) + }, + [allowedCommands], + ) + return ( <>
@@ -123,6 +295,39 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec
+ + {/* Command pattern display and checkboxes */} + {commandPatterns.length > 0 && ( +
+ + {isPatternSectionExpanded && ( +
+ {commandPatterns.map((item, index) => ( + handleAllowPatternChange(item.pattern)} + className="text-xs ml-4"> + + {item.pattern} + + + ))} +
+ )} +
+ )} +
diff --git a/webview-ui/src/utils/__tests__/extract-command-pattern.spec.ts b/webview-ui/src/utils/__tests__/extract-command-pattern.spec.ts index b6715fcc4e..7a0ba000de 100644 --- a/webview-ui/src/utils/__tests__/extract-command-pattern.spec.ts +++ b/webview-ui/src/utils/__tests__/extract-command-pattern.spec.ts @@ -11,17 +11,17 @@ describe("extractCommandPattern", () => { describe("npm/yarn/pnpm/bun commands", () => { it("extracts npm run patterns", () => { - expect(extractCommandPattern("npm run build")).toBe("npm run build") - expect(extractCommandPattern("npm run test:unit")).toBe("npm run *") - expect(extractCommandPattern("yarn run dev")).toBe("yarn run dev") - expect(extractCommandPattern("pnpm run lint")).toBe("pnpm run lint") - expect(extractCommandPattern("bun run start")).toBe("bun run start") + expect(extractCommandPattern("npm run build")).toBe("npm run") + expect(extractCommandPattern("npm run test:unit")).toBe("npm run") + expect(extractCommandPattern("yarn run dev")).toBe("yarn run") + expect(extractCommandPattern("pnpm run lint")).toBe("pnpm run") + expect(extractCommandPattern("bun run start")).toBe("bun run") }) it("handles npm run with additional arguments", () => { - expect(extractCommandPattern("npm run build:prod -- --env=production")).toBe("npm run build:prod") - expect(extractCommandPattern("npm run test -- --coverage")).toBe("npm run test") - expect(extractCommandPattern("npm run build:dev --watch")).toBe("npm run *") + expect(extractCommandPattern("npm run build:prod -- --env=production")).toBe("npm run") + expect(extractCommandPattern("npm run test -- --coverage")).toBe("npm run") + expect(extractCommandPattern("npm run build:dev --watch")).toBe("npm run") }) it("extracts npm script patterns", () => { @@ -82,18 +82,16 @@ describe("extractCommandPattern", () => { describe("chained commands", () => { it("extracts patterns from all commands in chain", () => { - expect(extractCommandPattern("cd /path && npm install")).toBe("cd * && npm install") + expect(extractCommandPattern("cd /path && npm install")).toBe("cd && npm install") expect(extractCommandPattern("npm test || echo 'failed'")).toBe("npm test || echo") expect(extractCommandPattern("git pull; npm install; npm run build")).toBe( - "git pull ; npm install ; npm run build", + "git pull ; npm install ; npm run", ) expect(extractCommandPattern("echo 'start' | grep start")).toBe("echo | grep") }) it("handles complex chained commands with wildcards", () => { - expect(extractCommandPattern("cd /path/to/project && npm run build:prod --verbose")).toBe( - "cd * && npm run *", - ) + expect(extractCommandPattern("cd /path/to/project && npm run build:prod --verbose")).toBe("cd && npm run") }) }) @@ -124,7 +122,7 @@ describe("extractCommandPattern", () => { it("handles double quotes", () => { expect(extractCommandPattern('echo "hello world"')).toBe("echo") - expect(extractCommandPattern('npm run "test:unit"')).toBe("npm run *") + expect(extractCommandPattern('npm run "test:unit"')).toBe("npm run") }) it("handles quotes with spaces", () => { @@ -140,15 +138,15 @@ describe("extractCommandPattern", () => { }) it("handles cd command", () => { - expect(extractCommandPattern("cd /home/user/project")).toBe("cd *") - expect(extractCommandPattern("cd ..")).toBe("cd *") - expect(extractCommandPattern("cd")).toBe("cd *") - expect(extractCommandPattern("cd /usr/local/bin")).toBe("cd *") + expect(extractCommandPattern("cd /home/user/project")).toBe("cd") + expect(extractCommandPattern("cd ..")).toBe("cd") + expect(extractCommandPattern("cd")).toBe("cd") + expect(extractCommandPattern("cd /usr/local/bin")).toBe("cd") }) it("handles commands with environment variables", () => { - expect(extractCommandPattern("NODE_ENV=production npm start")).toBe("NODE_ENV=*") - expect(extractCommandPattern("PORT=3000 node server.js")).toBe("PORT=*") + expect(extractCommandPattern("NODE_ENV=production npm start")).toBe("NODE_ENV=production") + expect(extractCommandPattern("PORT=3000 node server.js")).toBe("PORT=3000") }) it("handles dangerous commands more carefully", () => { @@ -174,10 +172,10 @@ describe("extractCommandPattern", () => { describe("getPatternDescription", () => { it("describes npm patterns", () => { - expect(getPatternDescription("npm run")).toBe("npm run scripts") + expect(getPatternDescription("npm run")).toBe("all npm run scripts") expect(getPatternDescription("npm test")).toBe("npm test commands") expect(getPatternDescription("npm")).toBe("npm commands") - expect(getPatternDescription("yarn run")).toBe("yarn run scripts") + expect(getPatternDescription("yarn run")).toBe("all yarn run scripts") expect(getPatternDescription("pnpm build")).toBe("pnpm build commands") }) diff --git a/webview-ui/src/utils/extract-command-pattern.ts b/webview-ui/src/utils/extract-command-pattern.ts index 83f0628fb5..e8b44b08b6 100644 --- a/webview-ui/src/utils/extract-command-pattern.ts +++ b/webview-ui/src/utils/extract-command-pattern.ts @@ -151,24 +151,16 @@ function extractSingleCommandPattern(command: string): string { // 1. npm/yarn/pnpm commands - include subcommand with wildcards for scripts if (["npm", "yarn", "pnpm", "bun"].includes(baseCommand) && tokens.length > 1) { const subCommand = tokens[1] - // For "run" commands, be more specific about patterns + // For "run" commands, check the script name if (subCommand === "run" && tokens.length > 2) { - const scriptName = tokens[2] - // Check if there are additional arguments after the script name - const hasDoubleDash = tokens.includes("--") - const doubleDashIndex = hasDoubleDash ? tokens.indexOf("--") : -1 + const _scriptName = tokens[2].replace(/^["']|["']$/g, "") // Remove quotes if present - // For scripts with colons or complex names - if (scriptName && (scriptName.includes(":") || scriptName.includes("-"))) { - // If there's a double dash with additional args, include the full script name - if (hasDoubleDash && doubleDashIndex > 2) { - return `${baseCommand} run ${scriptName}` - } - // Otherwise use wildcard for flexibility - return `${baseCommand} run *` - } - // For simple script names, include the script name itself - return `${baseCommand} run ${scriptName}` + // Check if there's a -- separator (pass-through args) + const _hasPassThroughArgs = tokens.includes("--") + + // Always return just "npm run" without the script name + // This allows all npm run commands without using wildcards + return `${baseCommand} run` } // For direct scripts like "npm test", "npm build", include the script name if (!subCommand.startsWith("-")) { @@ -205,9 +197,9 @@ function extractSingleCommandPattern(command: string): string { return baseCommand } - // 6. cd command - use wildcard for flexibility + // 6. cd command - just return cd if (baseCommand === "cd") { - return "cd *" + return "cd" } // 7. Docker/kubectl commands - include subcommand @@ -231,8 +223,8 @@ function extractSingleCommandPattern(command: string): string { // This might be an environment variable like NODE_ENV=production const envMatch = baseCommand.match(/^([A-Z_]+)=/) if (envMatch) { - // Don't include the value, just the variable name pattern - return `${envMatch[1]}=*` + // Return the full environment variable assignment + return baseCommand } } @@ -266,7 +258,8 @@ export function getPatternDescription(pattern: string): string { // npm/yarn/pnpm patterns if (["npm", "yarn", "pnpm", "bun"].includes(baseCommand)) { if (tokens[1] === "run") { - return `${baseCommand} run scripts` + // For "npm run", describe what it allows + return `all ${baseCommand} run scripts` } if (tokens[1]) { return `${baseCommand} ${tokens[1]} commands`