mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
feat: highlight denied commands in command execution code block
When a command is blocked by the denied commands list, a yellow warning banner now appears below the code block showing which specific sub-commands triggered the denial. This helps users understand why a command is asking for approval when all other commands in the block appear to be allowed. - Add getDeniedSubcommands utility to identify denied sub-commands - Add DeniedCommandsBanner component with yellow warning styling - Add i18n string for denied command detection message - Add tests for utility and component integration
This commit is contained in:
parent
ad25634905
commit
ff21dac28e
5 changed files with 244 additions and 2 deletions
|
|
@ -1,7 +1,7 @@
|
|||
import { useCallback, useState, memo, useMemo } from "react"
|
||||
import { useEvent } from "react-use"
|
||||
import { t } from "i18next"
|
||||
import { ChevronDown, OctagonX } from "lucide-react"
|
||||
import { ChevronDown, OctagonX, ShieldAlert } from "lucide-react"
|
||||
|
||||
import { type ExtensionMessage, type CommandExecutionStatus, commandExecutionStatusSchema } from "@roo-code/types"
|
||||
|
||||
|
|
@ -11,6 +11,7 @@ import { parseCommand } from "@roo/parse-command"
|
|||
|
||||
import { vscode } from "@src/utils/vscode"
|
||||
import { extractPatternsFromCommand } from "@src/utils/command-parser"
|
||||
import { getDeniedSubcommands } from "@src/utils/command-denied"
|
||||
import { useExtensionState } from "@src/context/ExtensionStateContext"
|
||||
import { cn } from "@src/lib/utils"
|
||||
|
||||
|
|
@ -54,6 +55,12 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec
|
|||
// streaming output (this is the case for running commands).
|
||||
const output = streamingOutput || parsedOutput
|
||||
|
||||
// Identify denied sub-commands within the full command
|
||||
const deniedSubcommands = useMemo(
|
||||
() => getDeniedSubcommands(command, allowedCommands, deniedCommands),
|
||||
[command, allowedCommands, deniedCommands],
|
||||
)
|
||||
|
||||
// Extract command patterns from the actual command that was executed
|
||||
const commandPatterns = useMemo<CommandPattern[]>(() => {
|
||||
// First get all individual commands (including subshell commands) using parseCommand
|
||||
|
|
@ -202,6 +209,7 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec
|
|||
<div className="bg-vscode-editor-background border border-vscode-border rounded-xs ml-6 mt-2">
|
||||
<div className="p-2">
|
||||
<CodeBlock source={command} language="shell" />
|
||||
{deniedSubcommands.length > 0 && <DeniedCommandsBanner deniedSubcommands={deniedSubcommands} />}
|
||||
<OutputContainer isExpanded={isExpanded} output={output} />
|
||||
</div>
|
||||
{command && command.trim() && (
|
||||
|
|
@ -232,6 +240,36 @@ const OutputContainerInternal = ({ isExpanded, output }: { isExpanded: boolean;
|
|||
|
||||
const OutputContainer = memo(OutputContainerInternal)
|
||||
|
||||
const DeniedCommandsBanner = ({ deniedSubcommands }: { deniedSubcommands: string[] }) => (
|
||||
<div
|
||||
className="flex items-start gap-1.5 mt-2 px-2 py-1.5 rounded text-xs border"
|
||||
style={{
|
||||
backgroundColor: "var(--vscode-inputValidation-warningBackground, rgba(255, 204, 0, 0.1))",
|
||||
borderColor: "var(--vscode-inputValidation-warningBorder, #cca700)",
|
||||
color: "var(--vscode-inputValidation-warningForeground, var(--vscode-foreground))",
|
||||
}}
|
||||
data-testid="denied-commands-banner">
|
||||
<ShieldAlert
|
||||
className="size-3.5 shrink-0 mt-0.5"
|
||||
style={{ color: "var(--vscode-inputValidation-warningBorder, #cca700)" }}
|
||||
/>
|
||||
<div>
|
||||
<span>{t("chat:commandExecution.deniedCommandDetected")}: </span>
|
||||
{deniedSubcommands.map((cmd, i) => (
|
||||
<code
|
||||
key={i}
|
||||
className="px-1 py-0.5 rounded font-mono"
|
||||
style={{
|
||||
backgroundColor: "rgba(255, 204, 0, 0.15)",
|
||||
color: "var(--vscode-inputValidation-warningBorder, #cca700)",
|
||||
}}>
|
||||
{cmd}
|
||||
</code>
|
||||
))}
|
||||
</div>
|
||||
</div>
|
||||
)
|
||||
|
||||
const parseCommandAndOutput = (text: string | undefined) => {
|
||||
if (!text) {
|
||||
return { command: "", output: "" }
|
||||
|
|
|
|||
|
|
@ -605,4 +605,55 @@ Output:
|
|||
expect(terminalOutput).toHaveTextContent("0 total")
|
||||
})
|
||||
})
|
||||
|
||||
describe("denied commands banner", () => {
|
||||
it("should show denied commands banner when command matches deny list", () => {
|
||||
render(
|
||||
<ExtensionStateWrapper>
|
||||
<CommandExecution executionId="test-denied-1" text="rm -rf /" />
|
||||
</ExtensionStateWrapper>,
|
||||
)
|
||||
|
||||
const banner = screen.getByTestId("denied-commands-banner")
|
||||
expect(banner).toBeInTheDocument()
|
||||
expect(banner.textContent).toContain("rm -rf /")
|
||||
})
|
||||
|
||||
it("should not show denied commands banner when command is allowed", () => {
|
||||
render(
|
||||
<ExtensionStateWrapper>
|
||||
<CommandExecution executionId="test-denied-2" text="npm install" />
|
||||
</ExtensionStateWrapper>,
|
||||
)
|
||||
|
||||
expect(screen.queryByTestId("denied-commands-banner")).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
it("should show denied commands in chained commands", () => {
|
||||
render(
|
||||
<ExtensionStateWrapper>
|
||||
<CommandExecution executionId="test-denied-3" text="npm install && rm -rf /" />
|
||||
</ExtensionStateWrapper>,
|
||||
)
|
||||
|
||||
const banner = screen.getByTestId("denied-commands-banner")
|
||||
expect(banner).toBeInTheDocument()
|
||||
expect(banner.textContent).toContain("rm -rf /")
|
||||
})
|
||||
|
||||
it("should not show banner when deniedCommands list is empty", () => {
|
||||
const stateWithNoDenied = {
|
||||
...mockExtensionState,
|
||||
deniedCommands: [],
|
||||
}
|
||||
|
||||
render(
|
||||
<ExtensionStateContext.Provider value={stateWithNoDenied as any}>
|
||||
<CommandExecution executionId="test-denied-4" text="rm -rf /" />
|
||||
</ExtensionStateContext.Provider>,
|
||||
)
|
||||
|
||||
expect(screen.queryByTestId("denied-commands-banner")).not.toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -283,7 +283,8 @@
|
|||
"expandOutput": "Expand output",
|
||||
"collapseOutput": "Collapse output",
|
||||
"expandManagement": "Expand command management section",
|
||||
"collapseManagement": "Collapse command management section"
|
||||
"collapseManagement": "Collapse command management section",
|
||||
"deniedCommandDetected": "Denied command detected"
|
||||
},
|
||||
"response": "Response",
|
||||
"arguments": "Arguments",
|
||||
|
|
|
|||
74
webview-ui/src/utils/__tests__/command-denied.spec.ts
Normal file
74
webview-ui/src/utils/__tests__/command-denied.spec.ts
Normal file
|
|
@ -0,0 +1,74 @@
|
|||
// pnpm --filter @roo-code/vscode-webview test src/utils/__tests__/command-denied.spec.ts
|
||||
|
||||
import { getDeniedSubcommands } from "../command-denied"
|
||||
|
||||
vi.mock("@roo/parse-command", () => ({
|
||||
parseCommand: (command: string) => {
|
||||
if (!command?.trim()) return []
|
||||
// Simple split by &&, ||, ;, | for testing
|
||||
return command
|
||||
.split(/\s*(?:&&|\|\||;|\|)\s*/)
|
||||
.map((c) => c.trim())
|
||||
.filter(Boolean)
|
||||
},
|
||||
}))
|
||||
|
||||
describe("getDeniedSubcommands", () => {
|
||||
it("should return empty array when command is empty", () => {
|
||||
expect(getDeniedSubcommands("", ["npm"], ["rm"])).toEqual([])
|
||||
})
|
||||
|
||||
it("should return empty array when deniedCommands is empty", () => {
|
||||
expect(getDeniedSubcommands("rm -rf /", ["npm"], [])).toEqual([])
|
||||
})
|
||||
|
||||
it("should return empty array when no sub-commands are denied", () => {
|
||||
expect(getDeniedSubcommands("npm install", ["npm"], ["rm"])).toEqual([])
|
||||
})
|
||||
|
||||
it("should identify a single denied command", () => {
|
||||
expect(getDeniedSubcommands("rm -rf /", ["npm"], ["rm"])).toEqual(["rm -rf /"])
|
||||
})
|
||||
|
||||
it("should identify denied commands in chained commands", () => {
|
||||
const result = getDeniedSubcommands("npm install && rm -rf /", ["npm"], ["rm"])
|
||||
expect(result).toEqual(["rm -rf /"])
|
||||
})
|
||||
|
||||
it("should identify multiple denied commands", () => {
|
||||
const result = getDeniedSubcommands("rm file.txt && npm install && rm -rf /tmp", ["npm"], ["rm"])
|
||||
expect(result).toEqual(["rm file.txt", "rm -rf /tmp"])
|
||||
})
|
||||
|
||||
it("should respect longest prefix match - allow wins when more specific", () => {
|
||||
// "rm -i" is allowed and more specific than denied "rm"
|
||||
const result = getDeniedSubcommands("rm -i file.txt", ["rm -i"], ["rm"])
|
||||
expect(result).toEqual([])
|
||||
})
|
||||
|
||||
it("should respect longest prefix match - deny wins when more specific", () => {
|
||||
// "git push" is denied and more specific than allowed "git"
|
||||
const result = getDeniedSubcommands("git push origin main", ["git"], ["git push"])
|
||||
expect(result).toEqual(["git push origin main"])
|
||||
})
|
||||
|
||||
it("should respect longest prefix match - deny wins when equal length", () => {
|
||||
const result = getDeniedSubcommands("rm file.txt", ["rm"], ["rm"])
|
||||
expect(result).toEqual(["rm file.txt"])
|
||||
})
|
||||
|
||||
it("should handle commands with no allowed list matches", () => {
|
||||
const result = getDeniedSubcommands("rm -rf /", [], ["rm"])
|
||||
expect(result).toEqual(["rm -rf /"])
|
||||
})
|
||||
|
||||
it("should be case-insensitive", () => {
|
||||
const result = getDeniedSubcommands("RM -rf /", ["npm"], ["rm"])
|
||||
expect(result).toEqual(["RM -rf /"])
|
||||
})
|
||||
|
||||
it("should handle mixed allowed and denied in chain", () => {
|
||||
const result = getDeniedSubcommands("git status && rm file && npm test", ["git", "npm"], ["rm"])
|
||||
expect(result).toEqual(["rm file"])
|
||||
})
|
||||
})
|
||||
78
webview-ui/src/utils/command-denied.ts
Normal file
78
webview-ui/src/utils/command-denied.ts
Normal file
|
|
@ -0,0 +1,78 @@
|
|||
import { parseCommand } from "@roo/parse-command"
|
||||
|
||||
/**
|
||||
* Find the longest matching prefix from a list of prefixes for a given command.
|
||||
* Case-insensitive prefix matching with wildcard support.
|
||||
*
|
||||
* This mirrors the logic in `src/core/auto-approval/commands.ts` so the webview
|
||||
* can independently determine which sub-commands are denied.
|
||||
*/
|
||||
function findLongestPrefixMatch(command: string, prefixes: string[]): string | null {
|
||||
if (!command || !prefixes?.length) {
|
||||
return null
|
||||
}
|
||||
|
||||
const trimmedCommand = command.trim().toLowerCase()
|
||||
let longestMatch: string | null = null
|
||||
|
||||
for (const prefix of prefixes) {
|
||||
const lowerPrefix = prefix.toLowerCase()
|
||||
if (lowerPrefix === "*" || trimmedCommand.startsWith(lowerPrefix)) {
|
||||
if (!longestMatch || lowerPrefix.length > longestMatch.length) {
|
||||
longestMatch = lowerPrefix
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return longestMatch
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if a single sub-command is denied based on the longest prefix match rule.
|
||||
* A command is considered denied when the deny list has a matching prefix that is
|
||||
* at least as long as any matching allow list prefix.
|
||||
*/
|
||||
function isSubcommandDenied(command: string, allowedCommands: string[], deniedCommands: string[]): boolean {
|
||||
if (!command?.trim() || !deniedCommands?.length) {
|
||||
return false
|
||||
}
|
||||
|
||||
const cmdWithoutRedirection = command.replace(/\d*>&\d*/, "").trim()
|
||||
const longestDeniedMatch = findLongestPrefixMatch(cmdWithoutRedirection, deniedCommands)
|
||||
|
||||
if (!longestDeniedMatch) {
|
||||
return false
|
||||
}
|
||||
|
||||
const longestAllowedMatch = findLongestPrefixMatch(cmdWithoutRedirection, allowedCommands || [])
|
||||
|
||||
if (!longestAllowedMatch) {
|
||||
return true
|
||||
}
|
||||
|
||||
// Deny list wins when its match is longer or equal
|
||||
return longestDeniedMatch.length >= longestAllowedMatch.length
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the list of denied sub-commands from a full command string.
|
||||
* Parses the command into sub-commands (splitting by &&, ||, ;, |, etc.)
|
||||
* and returns the ones that match the deny list.
|
||||
*
|
||||
* @param command - Full command string (may contain chained commands)
|
||||
* @param allowedCommands - List of allowed command prefixes
|
||||
* @param deniedCommands - List of denied command prefixes
|
||||
* @returns Array of sub-command strings that are denied
|
||||
*/
|
||||
export function getDeniedSubcommands(command: string, allowedCommands: string[], deniedCommands: string[]): string[] {
|
||||
if (!command?.trim() || !deniedCommands?.length) {
|
||||
return []
|
||||
}
|
||||
|
||||
const subCommands = parseCommand(command)
|
||||
|
||||
return subCommands.filter((cmd) => {
|
||||
const trimmed = cmd.trim()
|
||||
return trimmed && isSubcommandDenied(trimmed, allowedCommands, deniedCommands)
|
||||
})
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue