mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
fix: resolve command whitelisting issues
- Fixed programmatic suggestion generation when LLM suggestions are disabled - CommandExecution component now properly generates suggestions from allowed patterns - Added proper handling for when llmGeneratedSuggestions setting is false - Fixed settings persistence for command allow/deny lists - AutoApproveSettings now correctly saves patterns to globalState - Fixed state management to properly update both local and global state - Improved LLM prompt to generate complete suggestions for chained commands - Updated execute-command prompt to handle && and || operators - Ensures suggestions include full command chains, not just the first part - Added tests to verify proper handling of complex command patterns
This commit is contained in:
parent
70ca7a6cae
commit
238bae6aef
6 changed files with 75 additions and 25 deletions
|
|
@ -22,6 +22,9 @@ describe("getExecuteCommandDescription", () => {
|
|||
expect(description).toContain("<suggestions>")
|
||||
expect(description).toContain("- suggestions: (optional) Command patterns for the user to allow/deny")
|
||||
expect(description).toContain("Suggestion Guidelines")
|
||||
// Check for chained command guidance
|
||||
expect(description).toContain("For chained commands")
|
||||
expect(description).toContain("cd backend && npm install")
|
||||
})
|
||||
|
||||
it("should include suggestions section when disableLlmCommandSuggestions is not set", () => {
|
||||
|
|
|
|||
|
|
@ -37,11 +37,14 @@ Example: Requesting to execute ls in a specific directory
|
|||
return (
|
||||
baseDescription +
|
||||
`
|
||||
- suggestions: (optional) Command patterns for the user to allow/deny for future auto-approval. Include 1-2 relevant patterns when executing common development commands. Use <suggest> tags.
|
||||
- suggestions: (optional) Command patterns for the user to allow/deny for future auto-approval. Use <suggest> tags.
|
||||
|
||||
**Suggestion Guidelines:**
|
||||
- Suggestions use prefix matching (case-insensitive)
|
||||
- Include the base command (e.g., "npm", "git") and optionally a more specific pattern
|
||||
- For simple commands: Include the base command (e.g., "npm", "git") and optionally a more specific pattern
|
||||
- For chained commands (using &&, ||, ;, |): Include patterns for EACH individual command in the chain
|
||||
- Example: For "cd backend && npm install", suggest: "cd backend && npm install", "cd", "npm install", "npm"
|
||||
- Include 2-4 relevant patterns total
|
||||
- Only suggest "*" (allow all) if explicitly requested by the user
|
||||
|
||||
Usage:
|
||||
|
|
@ -63,6 +66,17 @@ Example: Requesting to execute npm run dev
|
|||
</suggestions>
|
||||
</execute_command>
|
||||
|
||||
Example: Requesting to execute a chained command
|
||||
<execute_command>
|
||||
<command>cd backend && npm install</command>
|
||||
<suggestions>
|
||||
<suggest>cd backend && npm install</suggest>
|
||||
<suggest>cd</suggest>
|
||||
<suggest>npm install</suggest>
|
||||
<suggest>npm</suggest>
|
||||
</suggestions>
|
||||
</execute_command>
|
||||
|
||||
Example: Requesting to execute ls in a specific directory
|
||||
<execute_command>
|
||||
<command>ls -la</command>
|
||||
|
|
|
|||
|
|
@ -45,7 +45,8 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec
|
|||
const [isOutputExpanded, setIsOutputExpanded] = useState(false)
|
||||
|
||||
// Determine if we should show suggestions section
|
||||
const showSuggestions = suggestions && suggestions.length > 0
|
||||
// Always show suggestions if we have a command, either from LLM or programmatic generation
|
||||
const showSuggestions = (suggestions && suggestions.length > 0) || !!command?.trim()
|
||||
|
||||
// Use suggestions if available, otherwise extract command patterns
|
||||
const commandPatterns = useMemo(() => {
|
||||
|
|
@ -57,8 +58,8 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec
|
|||
}))
|
||||
}
|
||||
|
||||
// Only extract patterns if we're showing suggestions (for backward compatibility)
|
||||
if (!showSuggestions || !command?.trim()) return []
|
||||
// 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 = ["&&", "||", ";", "|"]
|
||||
|
|
@ -151,7 +152,7 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec
|
|||
)
|
||||
|
||||
return uniquePatterns
|
||||
}, [command, suggestions, showSuggestions])
|
||||
}, [command, suggestions])
|
||||
|
||||
// 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
|
||||
|
|
|
|||
|
|
@ -82,7 +82,7 @@ describe("CommandExecution", () => {
|
|||
} as any)
|
||||
})
|
||||
|
||||
it("should render command without suggestions", () => {
|
||||
it("should render command with programmatic suggestions when no LLM suggestions provided", () => {
|
||||
renderWithProviders(
|
||||
<CommandExecution
|
||||
executionId="test-1"
|
||||
|
|
@ -92,8 +92,20 @@ describe("CommandExecution", () => {
|
|||
/>,
|
||||
)
|
||||
|
||||
expect(screen.getByText("npm install")).toBeInTheDocument()
|
||||
expect(screen.queryByText("Manage Command Permissions")).not.toBeInTheDocument()
|
||||
// Check command is rendered in code block
|
||||
const codeBlocks = screen.getAllByText("npm install")
|
||||
expect(codeBlocks.length).toBeGreaterThan(0)
|
||||
|
||||
// Should show manage permissions section even without LLM suggestions
|
||||
expect(screen.getByText("Manage Command Permissions")).toBeInTheDocument()
|
||||
|
||||
// Expand the section to verify programmatic patterns
|
||||
const sectionHeader = screen.getByText("Manage Command Permissions")
|
||||
fireEvent.click(sectionHeader)
|
||||
|
||||
// Should show programmatically extracted pattern
|
||||
const patterns = screen.getAllByText("npm install")
|
||||
expect(patterns.length).toBeGreaterThan(1) // Command + pattern
|
||||
})
|
||||
|
||||
it("should render command with suggestions section collapsed by default", () => {
|
||||
|
|
@ -220,7 +232,7 @@ describe("CommandExecution", () => {
|
|||
})
|
||||
})
|
||||
|
||||
it("should handle empty suggestions tag", () => {
|
||||
it("should handle empty suggestions tag and show programmatic patterns", () => {
|
||||
const commandWithEmptySuggestions = "ls -la<suggestions>[]</suggestions>"
|
||||
|
||||
renderWithProviders(
|
||||
|
|
@ -232,8 +244,19 @@ describe("CommandExecution", () => {
|
|||
/>,
|
||||
)
|
||||
|
||||
expect(screen.getByText("ls -la")).toBeInTheDocument()
|
||||
expect(screen.queryByText("Manage Command Permissions")).not.toBeInTheDocument()
|
||||
// Check command is rendered
|
||||
const codeBlocks = screen.getAllByText("ls -la")
|
||||
expect(codeBlocks.length).toBeGreaterThan(0)
|
||||
|
||||
// Should still show manage permissions with programmatic patterns
|
||||
expect(screen.getByText("Manage Command Permissions")).toBeInTheDocument()
|
||||
|
||||
// Expand the section to verify programmatic patterns
|
||||
const sectionHeader = screen.getByText("Manage Command Permissions")
|
||||
fireEvent.click(sectionHeader)
|
||||
|
||||
// Should show the base command pattern
|
||||
expect(screen.getByText("ls")).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it("should handle suggestions with special characters", () => {
|
||||
|
|
@ -260,7 +283,7 @@ describe("CommandExecution", () => {
|
|||
expect(screen.getByText("echo `date`")).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it("should handle malformed suggestions tag", () => {
|
||||
it("should handle malformed suggestions tag and show programmatic patterns", () => {
|
||||
const commandWithMalformedSuggestions = "pwd<suggestions>not-valid-json</suggestions>"
|
||||
|
||||
renderWithProviders(
|
||||
|
|
@ -273,9 +296,19 @@ describe("CommandExecution", () => {
|
|||
)
|
||||
|
||||
// Should still render the command
|
||||
expect(screen.getByText("pwd")).toBeInTheDocument()
|
||||
// Suggestions should not be shown when JSON is invalid
|
||||
expect(screen.queryByText("Manage Command Permissions")).not.toBeInTheDocument()
|
||||
const codeBlocks = screen.getAllByText("pwd")
|
||||
expect(codeBlocks.length).toBeGreaterThan(0)
|
||||
|
||||
// Should show manage permissions with programmatic patterns
|
||||
expect(screen.getByText("Manage Command Permissions")).toBeInTheDocument()
|
||||
|
||||
// Expand the section to verify programmatic patterns
|
||||
const sectionHeader = screen.getByText("Manage Command Permissions")
|
||||
fireEvent.click(sectionHeader)
|
||||
|
||||
// Should show the base command pattern (there will be multiple pwd elements)
|
||||
const patterns = screen.getAllByText("pwd")
|
||||
expect(patterns.length).toBeGreaterThan(1) // Command + pattern
|
||||
})
|
||||
|
||||
it("should parse suggestions from JSON array and show them when expanded", () => {
|
||||
|
|
|
|||
|
|
@ -3,7 +3,6 @@ import { X } from "lucide-react"
|
|||
|
||||
import { useAppTranslation } from "@/i18n/TranslationContext"
|
||||
import { VSCodeCheckbox } from "@vscode/webview-ui-toolkit/react"
|
||||
import { vscode } from "@/utils/vscode"
|
||||
import { Button, Input, Slider } from "@/components/ui"
|
||||
|
||||
import { SetCachedStateField } from "./types"
|
||||
|
|
@ -88,7 +87,7 @@ export const AutoApproveSettings = ({
|
|||
const newCommands = [...currentCommands, commandInput]
|
||||
setCachedStateField("allowedCommands", newCommands)
|
||||
setCommandInput("")
|
||||
vscode.postMessage({ type: "allowedCommands", commands: newCommands })
|
||||
// Don't send message here - wait for Save button
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -99,7 +98,7 @@ export const AutoApproveSettings = ({
|
|||
const newCommands = [...currentCommands, deniedCommandInput]
|
||||
setCachedStateField("deniedCommands", newCommands)
|
||||
setDeniedCommandInput("")
|
||||
vscode.postMessage({ type: "deniedCommands", commands: newCommands })
|
||||
// Don't send message here - wait for Save button
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -318,7 +317,7 @@ export const AutoApproveSettings = ({
|
|||
onClick={() => {
|
||||
const newCommands = (allowedCommands ?? []).filter((_, i) => i !== index)
|
||||
setCachedStateField("allowedCommands", newCommands)
|
||||
vscode.postMessage({ type: "allowedCommands", commands: newCommands })
|
||||
// Don't send message here - wait for Save button
|
||||
}}>
|
||||
<div className="flex flex-row items-center gap-1">
|
||||
<div>{cmd}</div>
|
||||
|
|
@ -369,7 +368,7 @@ export const AutoApproveSettings = ({
|
|||
onClick={() => {
|
||||
const newCommands = (deniedCommands ?? []).filter((_, i) => i !== index)
|
||||
setCachedStateField("deniedCommands", newCommands)
|
||||
vscode.postMessage({ type: "deniedCommands", commands: newCommands })
|
||||
// Don't send message here - wait for Save button
|
||||
}}>
|
||||
<div className="flex flex-row items-center gap-1">
|
||||
<div>{cmd}</div>
|
||||
|
|
|
|||
|
|
@ -458,8 +458,8 @@ describe("SettingsView - Allowed Commands", () => {
|
|||
// Verify command was added
|
||||
expect(screen.getByText("npm test")).toBeInTheDocument()
|
||||
|
||||
// Verify VSCode message was sent
|
||||
expect(vscode.postMessage).toHaveBeenCalledWith({
|
||||
// Verify VSCode message was NOT sent yet (only on Save)
|
||||
expect(vscode.postMessage).not.toHaveBeenCalledWith({
|
||||
type: "allowedCommands",
|
||||
commands: ["npm test"],
|
||||
})
|
||||
|
|
@ -489,8 +489,8 @@ describe("SettingsView - Allowed Commands", () => {
|
|||
// Verify command was removed
|
||||
expect(screen.queryByText("npm test")).not.toBeInTheDocument()
|
||||
|
||||
// Verify VSCode message was sent
|
||||
expect(vscode.postMessage).toHaveBeenLastCalledWith({
|
||||
// Verify VSCode message was NOT sent yet (only on Save)
|
||||
expect(vscode.postMessage).not.toHaveBeenCalledWith({
|
||||
type: "allowedCommands",
|
||||
commands: [],
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue