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
This commit is contained in:
hannesrudolph 2025-07-08 16:01:01 -06:00
parent 69ac7ec4b8
commit 3e5cc319a3
4 changed files with 264 additions and 125 deletions

View file

@ -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<ChatViewRef, ChatViewPro
</StandardTooltip>
) : (
<>
{/* Command approval with auto-approve pattern */}
{/* Command approval - simplified layout */}
{clineAsk === "command" && !isStreaming ? (
<div className="flex flex-col gap-[6px]">
{/* Top row: Run Command and Reject */}
<div className="flex gap-[6px]">
<StandardTooltip content={t("chat:runCommand.tooltip")}>
<VSCodeButton
appearance="primary"
disabled={!enableButtons}
className="flex-1"
onClick={() =>
handlePrimaryButtonClick(inputValue, selectedImages)
}>
{primaryButtonText}
</VSCodeButton>
</StandardTooltip>
<StandardTooltip content={t("chat:reject.tooltip")}>
<VSCodeButton
appearance="secondary"
disabled={!enableButtons}
className="flex-1"
onClick={() =>
handleSecondaryButtonClick(inputValue, selectedImages)
}>
{secondaryButtonText}
</VSCodeButton>
</StandardTooltip>
</div>
{/* Bottom row: Auto-approve pattern */}
<div className="flex items-center gap-[6px]">
<div className="flex-1 px-2 py-1 bg-vscode-input-background text-vscode-input-foreground rounded text-sm font-mono">
{(() => {
const commandMessage = findLast(
messagesRef.current,
(msg) => msg.type === "ask" && msg.ask === "command",
)
const commandText = commandMessage?.text || ""
const pattern = extractCommandPattern(commandText)
return pattern || commandText
})()}
</div>
<StandardTooltip
content={(() => {
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")
})()}>
<VSCodeButton
appearance="secondary"
disabled={!enableButtons}
onClick={() => {
// 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")}
</VSCodeButton>
</StandardTooltip>
</div>
<div className="flex gap-[6px]">
<StandardTooltip content={t("chat:runCommand.tooltip")}>
<VSCodeButton
appearance="primary"
disabled={!enableButtons}
className="flex-1"
onClick={() =>
handlePrimaryButtonClick(inputValue, selectedImages)
}>
{primaryButtonText}
</VSCodeButton>
</StandardTooltip>
<StandardTooltip content={t("chat:reject.tooltip")}>
<VSCodeButton
appearance="secondary"
disabled={!enableButtons}
className="flex-1"
onClick={() =>
handleSecondaryButtonClick(inputValue, selectedImages)
}>
{secondaryButtonText}
</VSCodeButton>
</StandardTooltip>
</div>
) : (
/* Standard two button layout */

View file

@ -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<CommandExecutionStatus | null>(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 (
<>
<div className="flex flex-row items-center justify-between gap-2 mb-1">
@ -123,6 +295,39 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec
<div className="w-full bg-vscode-editor-background border border-vscode-border rounded-xs p-2">
<CodeBlock source={command} language="shell" />
{/* Command pattern display and checkboxes */}
{commandPatterns.length > 0 && (
<div className="mt-2 pt-2 border-t border-border/25">
<button
onClick={() => setIsPatternSectionExpanded(!isPatternSectionExpanded)}
className="flex items-center gap-1 text-xs text-vscode-descriptionForeground hover:text-vscode-foreground transition-colors w-full text-left">
<ChevronDown
className={cn("size-3 transition-transform duration-200", {
"rotate-0": isPatternSectionExpanded,
"-rotate-90": !isPatternSectionExpanded,
})}
/>
<span>Add to Allowed Auto-Execute Commands</span>
</button>
{isPatternSectionExpanded && (
<div className="mt-2 space-y-2">
{commandPatterns.map((item, index) => (
<VSCodeCheckbox
key={`${item.pattern}-${index}`}
checked={allowedCommands.includes(item.pattern)}
onChange={() => handleAllowPatternChange(item.pattern)}
className="text-xs ml-4">
<span className="font-medium text-vscode-foreground whitespace-nowrap">
{item.pattern}
</span>
</VSCodeCheckbox>
))}
</div>
)}
</div>
)}
<OutputContainer isExpanded={isExpanded} output={output} />
</div>
</>

View file

@ -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")
})

View file

@ -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`