Skip to content

Commit 238bae6

Browse files
committed
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
1 parent 70ca7a6 commit 238bae6

File tree

6 files changed

+75
-25
lines changed

6 files changed

+75
-25
lines changed

src/core/prompts/tools/__tests__/execute-command.spec.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ describe("getExecuteCommandDescription", () => {
2222
expect(description).toContain("<suggestions>")
2323
expect(description).toContain("- suggestions: (optional) Command patterns for the user to allow/deny")
2424
expect(description).toContain("Suggestion Guidelines")
25+
// Check for chained command guidance
26+
expect(description).toContain("For chained commands")
27+
expect(description).toContain("cd backend && npm install")
2528
})
2629

2730
it("should include suggestions section when disableLlmCommandSuggestions is not set", () => {

src/core/prompts/tools/execute-command.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,14 @@ Example: Requesting to execute ls in a specific directory
3737
return (
3838
baseDescription +
3939
`
40-
- 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.
40+
- suggestions: (optional) Command patterns for the user to allow/deny for future auto-approval. Use <suggest> tags.
4141
4242
**Suggestion Guidelines:**
4343
- Suggestions use prefix matching (case-insensitive)
44-
- Include the base command (e.g., "npm", "git") and optionally a more specific pattern
44+
- For simple commands: Include the base command (e.g., "npm", "git") and optionally a more specific pattern
45+
- For chained commands (using &&, ||, ;, |): Include patterns for EACH individual command in the chain
46+
- Example: For "cd backend && npm install", suggest: "cd backend && npm install", "cd", "npm install", "npm"
47+
- Include 2-4 relevant patterns total
4548
- Only suggest "*" (allow all) if explicitly requested by the user
4649
4750
Usage:
@@ -63,6 +66,17 @@ Example: Requesting to execute npm run dev
6366
</suggestions>
6467
</execute_command>
6568
69+
Example: Requesting to execute a chained command
70+
<execute_command>
71+
<command>cd backend && npm install</command>
72+
<suggestions>
73+
<suggest>cd backend && npm install</suggest>
74+
<suggest>cd</suggest>
75+
<suggest>npm install</suggest>
76+
<suggest>npm</suggest>
77+
</suggestions>
78+
</execute_command>
79+
6680
Example: Requesting to execute ls in a specific directory
6781
<execute_command>
6882
<command>ls -la</command>

webview-ui/src/components/chat/CommandExecution.tsx

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,8 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec
4545
const [isOutputExpanded, setIsOutputExpanded] = useState(false)
4646

4747
// Determine if we should show suggestions section
48-
const showSuggestions = suggestions && suggestions.length > 0
48+
// Always show suggestions if we have a command, either from LLM or programmatic generation
49+
const showSuggestions = (suggestions && suggestions.length > 0) || !!command?.trim()
4950

5051
// Use suggestions if available, otherwise extract command patterns
5152
const commandPatterns = useMemo(() => {
@@ -57,8 +58,8 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec
5758
}))
5859
}
5960

60-
// Only extract patterns if we're showing suggestions (for backward compatibility)
61-
if (!showSuggestions || !command?.trim()) return []
61+
// If no LLM suggestions but we have a command, extract patterns programmatically
62+
if (!command?.trim()) return []
6263

6364
// Check if this is a chained command
6465
const operators = ["&&", "||", ";", "|"]
@@ -151,7 +152,7 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec
151152
)
152153

153154
return uniquePatterns
154-
}, [command, suggestions, showSuggestions])
155+
}, [command, suggestions])
155156

156157
// The command's output can either come from the text associated with the
157158
// task message (this is the case for completed commands) or from the

webview-ui/src/components/chat/__tests__/CommandExecution.spec.tsx

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ describe("CommandExecution", () => {
8282
} as any)
8383
})
8484

85-
it("should render command without suggestions", () => {
85+
it("should render command with programmatic suggestions when no LLM suggestions provided", () => {
8686
renderWithProviders(
8787
<CommandExecution
8888
executionId="test-1"
@@ -92,8 +92,20 @@ describe("CommandExecution", () => {
9292
/>,
9393
)
9494

95-
expect(screen.getByText("npm install")).toBeInTheDocument()
96-
expect(screen.queryByText("Manage Command Permissions")).not.toBeInTheDocument()
95+
// Check command is rendered in code block
96+
const codeBlocks = screen.getAllByText("npm install")
97+
expect(codeBlocks.length).toBeGreaterThan(0)
98+
99+
// Should show manage permissions section even without LLM suggestions
100+
expect(screen.getByText("Manage Command Permissions")).toBeInTheDocument()
101+
102+
// Expand the section to verify programmatic patterns
103+
const sectionHeader = screen.getByText("Manage Command Permissions")
104+
fireEvent.click(sectionHeader)
105+
106+
// Should show programmatically extracted pattern
107+
const patterns = screen.getAllByText("npm install")
108+
expect(patterns.length).toBeGreaterThan(1) // Command + pattern
97109
})
98110

99111
it("should render command with suggestions section collapsed by default", () => {
@@ -220,7 +232,7 @@ describe("CommandExecution", () => {
220232
})
221233
})
222234

223-
it("should handle empty suggestions tag", () => {
235+
it("should handle empty suggestions tag and show programmatic patterns", () => {
224236
const commandWithEmptySuggestions = "ls -la<suggestions>[]</suggestions>"
225237

226238
renderWithProviders(
@@ -232,8 +244,19 @@ describe("CommandExecution", () => {
232244
/>,
233245
)
234246

235-
expect(screen.getByText("ls -la")).toBeInTheDocument()
236-
expect(screen.queryByText("Manage Command Permissions")).not.toBeInTheDocument()
247+
// Check command is rendered
248+
const codeBlocks = screen.getAllByText("ls -la")
249+
expect(codeBlocks.length).toBeGreaterThan(0)
250+
251+
// Should still show manage permissions with programmatic patterns
252+
expect(screen.getByText("Manage Command Permissions")).toBeInTheDocument()
253+
254+
// Expand the section to verify programmatic patterns
255+
const sectionHeader = screen.getByText("Manage Command Permissions")
256+
fireEvent.click(sectionHeader)
257+
258+
// Should show the base command pattern
259+
expect(screen.getByText("ls")).toBeInTheDocument()
237260
})
238261

239262
it("should handle suggestions with special characters", () => {
@@ -260,7 +283,7 @@ describe("CommandExecution", () => {
260283
expect(screen.getByText("echo `date`")).toBeInTheDocument()
261284
})
262285

263-
it("should handle malformed suggestions tag", () => {
286+
it("should handle malformed suggestions tag and show programmatic patterns", () => {
264287
const commandWithMalformedSuggestions = "pwd<suggestions>not-valid-json</suggestions>"
265288

266289
renderWithProviders(
@@ -273,9 +296,19 @@ describe("CommandExecution", () => {
273296
)
274297

275298
// Should still render the command
276-
expect(screen.getByText("pwd")).toBeInTheDocument()
277-
// Suggestions should not be shown when JSON is invalid
278-
expect(screen.queryByText("Manage Command Permissions")).not.toBeInTheDocument()
299+
const codeBlocks = screen.getAllByText("pwd")
300+
expect(codeBlocks.length).toBeGreaterThan(0)
301+
302+
// Should show manage permissions with programmatic patterns
303+
expect(screen.getByText("Manage Command Permissions")).toBeInTheDocument()
304+
305+
// Expand the section to verify programmatic patterns
306+
const sectionHeader = screen.getByText("Manage Command Permissions")
307+
fireEvent.click(sectionHeader)
308+
309+
// Should show the base command pattern (there will be multiple pwd elements)
310+
const patterns = screen.getAllByText("pwd")
311+
expect(patterns.length).toBeGreaterThan(1) // Command + pattern
279312
})
280313

281314
it("should parse suggestions from JSON array and show them when expanded", () => {

webview-ui/src/components/settings/AutoApproveSettings.tsx

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import { X } from "lucide-react"
33

44
import { useAppTranslation } from "@/i18n/TranslationContext"
55
import { VSCodeCheckbox } from "@vscode/webview-ui-toolkit/react"
6-
import { vscode } from "@/utils/vscode"
76
import { Button, Input, Slider } from "@/components/ui"
87

98
import { SetCachedStateField } from "./types"
@@ -88,7 +87,7 @@ export const AutoApproveSettings = ({
8887
const newCommands = [...currentCommands, commandInput]
8988
setCachedStateField("allowedCommands", newCommands)
9089
setCommandInput("")
91-
vscode.postMessage({ type: "allowedCommands", commands: newCommands })
90+
// Don't send message here - wait for Save button
9291
}
9392
}
9493

@@ -99,7 +98,7 @@ export const AutoApproveSettings = ({
9998
const newCommands = [...currentCommands, deniedCommandInput]
10099
setCachedStateField("deniedCommands", newCommands)
101100
setDeniedCommandInput("")
102-
vscode.postMessage({ type: "deniedCommands", commands: newCommands })
101+
// Don't send message here - wait for Save button
103102
}
104103
}
105104

@@ -318,7 +317,7 @@ export const AutoApproveSettings = ({
318317
onClick={() => {
319318
const newCommands = (allowedCommands ?? []).filter((_, i) => i !== index)
320319
setCachedStateField("allowedCommands", newCommands)
321-
vscode.postMessage({ type: "allowedCommands", commands: newCommands })
320+
// Don't send message here - wait for Save button
322321
}}>
323322
<div className="flex flex-row items-center gap-1">
324323
<div>{cmd}</div>
@@ -369,7 +368,7 @@ export const AutoApproveSettings = ({
369368
onClick={() => {
370369
const newCommands = (deniedCommands ?? []).filter((_, i) => i !== index)
371370
setCachedStateField("deniedCommands", newCommands)
372-
vscode.postMessage({ type: "deniedCommands", commands: newCommands })
371+
// Don't send message here - wait for Save button
373372
}}>
374373
<div className="flex flex-row items-center gap-1">
375374
<div>{cmd}</div>

webview-ui/src/components/settings/__tests__/SettingsView.spec.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -458,8 +458,8 @@ describe("SettingsView - Allowed Commands", () => {
458458
// Verify command was added
459459
expect(screen.getByText("npm test")).toBeInTheDocument()
460460

461-
// Verify VSCode message was sent
462-
expect(vscode.postMessage).toHaveBeenCalledWith({
461+
// Verify VSCode message was NOT sent yet (only on Save)
462+
expect(vscode.postMessage).not.toHaveBeenCalledWith({
463463
type: "allowedCommands",
464464
commands: ["npm test"],
465465
})
@@ -489,8 +489,8 @@ describe("SettingsView - Allowed Commands", () => {
489489
// Verify command was removed
490490
expect(screen.queryByText("npm test")).not.toBeInTheDocument()
491491

492-
// Verify VSCode message was sent
493-
expect(vscode.postMessage).toHaveBeenLastCalledWith({
492+
// Verify VSCode message was NOT sent yet (only on Save)
493+
expect(vscode.postMessage).not.toHaveBeenCalledWith({
494494
type: "allowedCommands",
495495
commands: [],
496496
})

0 commit comments

Comments
 (0)