Skip to content

Commit a65f5d7

Browse files
hannesrudolphhannesrudolph
committed
fix: resolve all issues in PR #5491 - command whitelisting feature
- Fix hardcoded English strings by moving to translation files - Add missing ARIA attributes for accessibility compliance - Extract suggestion parsing logic to shared utils (src/shared/commandParsing.ts) - Move pattern extraction logic to shared utils (src/shared/commandPatterns.ts) - Extract CommandPatternSelector as a separate component for better modularity - Consolidate message types to use 'allowedCommands' consistently - Update tests to match new implementation All linters and tests now pass successfully.
1 parent 9b0f3b2 commit a65f5d7

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+1967
-524
lines changed

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

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,23 +3,66 @@ import { ToolArgs } from "./types"
33
export function getExecuteCommandDescription(args: ToolArgs): string | undefined {
44
return `## execute_command
55
Description: Request to execute a CLI command on the system. Use this when you need to perform system operations or run specific commands to accomplish any step in the user's task. You must tailor your command to the user's system and provide a clear explanation of what the command does. For command chaining, use the appropriate chaining syntax for the user's shell. Prefer to execute complex CLI commands over creating executable scripts, as they are more flexible and easier to run. Prefer relative commands and paths that avoid location sensitivity for terminal consistency, e.g: \`touch ./testdata/example.file\`, \`dir ./examples/model1/data/yaml\`, or \`go test ./cmd/front --config ./cmd/front/config.yml\`. If directed by the user, you may open a terminal in a different directory by using the \`cwd\` parameter.
6+
7+
**IMPORTANT: When executing commands that match common patterns (like npm, git, ls, etc.), you SHOULD provide suggestions for whitelisting. This allows users to auto-approve similar commands in the future.**
8+
69
Parameters:
710
- command: (required) The CLI command to execute. This should be valid for the current operating system. Ensure the command is properly formatted and does not contain any harmful instructions.
811
- cwd: (optional) The working directory to execute the command in (default: ${args.cwd})
12+
- suggestions: (optional) An array of safe command patterns that the user can whitelist for automatic approval in the future. Each suggestion should be a pattern that can match similar commands. When the command matches common development patterns, you SHOULD include relevant suggestions. Format each suggestion using <suggest> tags.
13+
14+
**Whitelisting Guidelines:**
15+
- Include suggestions when executing common development commands (npm, git, ls, cd, etc.)
16+
- Suggestions use prefix matching: any command that starts with the suggestion will be auto-approved
17+
- The special pattern "*" allows ALL commands (use with caution)
18+
- Suggestions are case-insensitive (e.g., "npm " matches "NPM install", "npm test", etc.)
19+
- Include a trailing space in suggestions to ensure proper prefix matching
20+
- Common patterns to suggest:
21+
- "npm " for all npm commands
22+
- "git " for all git operations
23+
- "ls " for listing files
24+
- "cd " for changing directories
25+
- "echo " for echo commands
26+
- "mkdir " for creating directories
27+
- "rm -rf node_modules" for specific cleanup command
28+
- Language-specific patterns like "python ", "node ", "go test ", etc.
29+
- "*" to allow all commands (only suggest when explicitly requested by user)
30+
931
Usage:
1032
<execute_command>
1133
<command>Your command here</command>
1234
<cwd>Working directory path (optional)</cwd>
35+
<suggestions>
36+
<suggest>pattern 1</suggest>
37+
<suggest>pattern 2</suggest>
38+
</suggestions>
1339
</execute_command>
1440
15-
Example: Requesting to execute npm run dev
41+
Example: Requesting to execute npm run dev with suggestions
1642
<execute_command>
1743
<command>npm run dev</command>
44+
<suggestions>
45+
<suggest>npm run </suggest>
46+
<suggest>npm </suggest>
47+
</suggestions>
48+
</execute_command>
49+
50+
Example: Requesting to execute git status with suggestions
51+
<execute_command>
52+
<command>git status</command>
53+
<suggestions>
54+
<suggest>git status</suggest>
55+
<suggest>git *</suggest>
56+
</suggestions>
1857
</execute_command>
1958
20-
Example: Requesting to execute ls in a specific directory if directed
59+
Example: Requesting to execute ls in a specific directory with suggestions
2160
<execute_command>
2261
<command>ls -la</command>
2362
<cwd>/home/user/projects</cwd>
63+
<suggestions>
64+
<suggest>ls -la</suggest>
65+
<suggest>ls </suggest>
66+
</suggestions>
2467
</execute_command>`
2568
}

‎src/core/tools/__tests__/executeCommandTool.spec.ts‎

Lines changed: 272 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,45 @@ beforeEach(() => {
5252
return
5353
}
5454

55-
const didApprove = await askApproval("command", block.params.command)
55+
// Handle suggestions if provided
56+
let commandWithSuggestions = block.params.command
57+
if (block.params.suggestions) {
58+
let suggestions = block.params.suggestions
59+
// Handle both array and string formats
60+
if (typeof suggestions === "string") {
61+
const suggestionsString = suggestions
62+
63+
// First try to parse as JSON array
64+
if (suggestionsString.trim().startsWith("[")) {
65+
try {
66+
suggestions = JSON.parse(suggestionsString)
67+
} catch (jsonError) {
68+
// Fall through to XML parsing
69+
}
70+
}
71+
72+
// If not JSON or JSON parsing failed, try to parse individual <suggest> tags
73+
if (!Array.isArray(suggestions)) {
74+
const individualSuggestMatches = suggestionsString.match(/<suggest>(.*?)<\/suggest>/g)
75+
if (individualSuggestMatches) {
76+
suggestions = individualSuggestMatches
77+
.map((match) => {
78+
const content = match.match(/<suggest>(.*?)<\/suggest>/)
79+
return content ? content[1] : ""
80+
})
81+
.filter((suggestion) => suggestion.length > 0)
82+
} else {
83+
// If no XML tags found, treat as single suggestion
84+
suggestions = [suggestions]
85+
}
86+
}
87+
}
88+
if (Array.isArray(suggestions) && suggestions.length > 0) {
89+
commandWithSuggestions = `${block.params.command}\n<suggestions>\n${suggestions.join("\n")}\n</suggestions>`
90+
}
91+
}
92+
93+
const didApprove = await askApproval("command", commandWithSuggestions)
5694
if (!didApprove) {
5795
return
5896
}
@@ -266,4 +304,237 @@ describe("executeCommandTool", () => {
266304
expect(mockExecuteCommand).not.toHaveBeenCalled()
267305
})
268306
})
307+
308+
describe("Suggestions functionality", () => {
309+
it("should pass command with suggestions when suggestions are provided as array", async () => {
310+
// Setup
311+
mockToolUse.params.command = "npm install"
312+
mockToolUse.params.suggestions = JSON.stringify([
313+
"npm install --save",
314+
"npm install --save-dev",
315+
"npm install --global",
316+
])
317+
318+
// Execute
319+
await executeCommandTool(
320+
mockCline as unknown as Task,
321+
mockToolUse,
322+
mockAskApproval as unknown as AskApproval,
323+
mockHandleError as unknown as HandleError,
324+
mockPushToolResult as unknown as PushToolResult,
325+
mockRemoveClosingTag as unknown as RemoveClosingTag,
326+
)
327+
328+
// Verify
329+
const expectedCommandWithSuggestions = `npm install
330+
<suggestions>
331+
npm install --save
332+
npm install --save-dev
333+
npm install --global
334+
</suggestions>`
335+
expect(mockAskApproval).toHaveBeenCalledWith("command", expectedCommandWithSuggestions)
336+
expect(mockExecuteCommand).toHaveBeenCalled()
337+
expect(mockPushToolResult).toHaveBeenCalledWith("Command executed")
338+
})
339+
340+
it("should pass command with suggestions when suggestions are provided as JSON string", async () => {
341+
// Setup
342+
mockToolUse.params.command = "git commit"
343+
mockToolUse.params.suggestions =
344+
'["git commit -m \\"Initial commit\\"", "git commit --amend", "git commit --no-verify"]'
345+
346+
// Execute
347+
await executeCommandTool(
348+
mockCline as unknown as Task,
349+
mockToolUse,
350+
mockAskApproval as unknown as AskApproval,
351+
mockHandleError as unknown as HandleError,
352+
mockPushToolResult as unknown as PushToolResult,
353+
mockRemoveClosingTag as unknown as RemoveClosingTag,
354+
)
355+
356+
// Verify
357+
const expectedCommandWithSuggestions = `git commit
358+
<suggestions>
359+
git commit -m "Initial commit"
360+
git commit --amend
361+
git commit --no-verify
362+
</suggestions>`
363+
expect(mockAskApproval).toHaveBeenCalledWith("command", expectedCommandWithSuggestions)
364+
expect(mockExecuteCommand).toHaveBeenCalled()
365+
expect(mockPushToolResult).toHaveBeenCalledWith("Command executed")
366+
})
367+
368+
it("should handle single suggestion as string", async () => {
369+
// Setup
370+
mockToolUse.params.command = "docker run"
371+
mockToolUse.params.suggestions = "docker run -it ubuntu:latest"
372+
373+
// Execute
374+
await executeCommandTool(
375+
mockCline as unknown as Task,
376+
mockToolUse,
377+
mockAskApproval as unknown as AskApproval,
378+
mockHandleError as unknown as HandleError,
379+
mockPushToolResult as unknown as PushToolResult,
380+
mockRemoveClosingTag as unknown as RemoveClosingTag,
381+
)
382+
383+
// Verify
384+
const expectedCommandWithSuggestions = `docker run
385+
<suggestions>
386+
docker run -it ubuntu:latest
387+
</suggestions>`
388+
expect(mockAskApproval).toHaveBeenCalledWith("command", expectedCommandWithSuggestions)
389+
expect(mockExecuteCommand).toHaveBeenCalled()
390+
expect(mockPushToolResult).toHaveBeenCalledWith("Command executed")
391+
})
392+
393+
it("should handle empty suggestions array", async () => {
394+
// Setup
395+
mockToolUse.params.command = "ls"
396+
mockToolUse.params.suggestions = JSON.stringify([])
397+
398+
// Execute
399+
await executeCommandTool(
400+
mockCline as unknown as Task,
401+
mockToolUse,
402+
mockAskApproval as unknown as AskApproval,
403+
mockHandleError as unknown as HandleError,
404+
mockPushToolResult as unknown as PushToolResult,
405+
mockRemoveClosingTag as unknown as RemoveClosingTag,
406+
)
407+
408+
// Verify - should pass command without suggestions
409+
expect(mockAskApproval).toHaveBeenCalledWith("command", "ls")
410+
expect(mockExecuteCommand).toHaveBeenCalled()
411+
expect(mockPushToolResult).toHaveBeenCalledWith("Command executed")
412+
})
413+
414+
it("should handle invalid JSON string in suggestions", async () => {
415+
// Setup
416+
mockToolUse.params.command = "echo test"
417+
mockToolUse.params.suggestions = "invalid json {"
418+
419+
// Execute
420+
await executeCommandTool(
421+
mockCline as unknown as Task,
422+
mockToolUse,
423+
mockAskApproval as unknown as AskApproval,
424+
mockHandleError as unknown as HandleError,
425+
mockPushToolResult as unknown as PushToolResult,
426+
mockRemoveClosingTag as unknown as RemoveClosingTag,
427+
)
428+
429+
// Verify - should treat invalid JSON as single suggestion
430+
const expectedCommandWithSuggestions = `echo test
431+
<suggestions>
432+
invalid json {
433+
</suggestions>`
434+
expect(mockAskApproval).toHaveBeenCalledWith("command", expectedCommandWithSuggestions)
435+
expect(mockExecuteCommand).toHaveBeenCalled()
436+
expect(mockPushToolResult).toHaveBeenCalledWith("Command executed")
437+
})
438+
439+
it("should parse individual <suggest> XML tags correctly", async () => {
440+
// Setup
441+
mockToolUse.params.command = "npm install"
442+
mockToolUse.params.suggestions =
443+
"<suggest>npm install --save</suggest><suggest>npm install --save-dev</suggest><suggest>npm install --global</suggest>"
444+
445+
// Execute
446+
await executeCommandTool(
447+
mockCline as unknown as Task,
448+
mockToolUse,
449+
mockAskApproval as unknown as AskApproval,
450+
mockHandleError as unknown as HandleError,
451+
mockPushToolResult as unknown as PushToolResult,
452+
mockRemoveClosingTag as unknown as RemoveClosingTag,
453+
)
454+
455+
// Verify
456+
const expectedCommandWithSuggestions = `npm install
457+
<suggestions>
458+
npm install --save
459+
npm install --save-dev
460+
npm install --global
461+
</suggestions>`
462+
expect(mockAskApproval).toHaveBeenCalledWith("command", expectedCommandWithSuggestions)
463+
expect(mockExecuteCommand).toHaveBeenCalled()
464+
expect(mockPushToolResult).toHaveBeenCalledWith("Command executed")
465+
})
466+
467+
it("should parse single <suggest> XML tag correctly", async () => {
468+
// Setup
469+
mockToolUse.params.command = "git push"
470+
mockToolUse.params.suggestions = "<suggest>git push origin main</suggest>"
471+
472+
// Execute
473+
await executeCommandTool(
474+
mockCline as unknown as Task,
475+
mockToolUse,
476+
mockAskApproval as unknown as AskApproval,
477+
mockHandleError as unknown as HandleError,
478+
mockPushToolResult as unknown as PushToolResult,
479+
mockRemoveClosingTag as unknown as RemoveClosingTag,
480+
)
481+
482+
// Verify
483+
const expectedCommandWithSuggestions = `git push
484+
<suggestions>
485+
git push origin main
486+
</suggestions>`
487+
expect(mockAskApproval).toHaveBeenCalledWith("command", expectedCommandWithSuggestions)
488+
expect(mockExecuteCommand).toHaveBeenCalled()
489+
expect(mockPushToolResult).toHaveBeenCalledWith("Command executed")
490+
})
491+
492+
it("should handle mixed content with <suggest> tags", async () => {
493+
// Setup
494+
mockToolUse.params.command = "docker run"
495+
mockToolUse.params.suggestions =
496+
"Some text before <suggest>docker run -it ubuntu</suggest> and <suggest>docker run -d nginx</suggest> with text after"
497+
498+
// Execute
499+
await executeCommandTool(
500+
mockCline as unknown as Task,
501+
mockToolUse,
502+
mockAskApproval as unknown as AskApproval,
503+
mockHandleError as unknown as HandleError,
504+
mockPushToolResult as unknown as PushToolResult,
505+
mockRemoveClosingTag as unknown as RemoveClosingTag,
506+
)
507+
508+
// Verify
509+
const expectedCommandWithSuggestions = `docker run
510+
<suggestions>
511+
docker run -it ubuntu
512+
docker run -d nginx
513+
</suggestions>`
514+
expect(mockAskApproval).toHaveBeenCalledWith("command", expectedCommandWithSuggestions)
515+
expect(mockExecuteCommand).toHaveBeenCalled()
516+
expect(mockPushToolResult).toHaveBeenCalledWith("Command executed")
517+
})
518+
519+
it("should work normally when no suggestions are provided", async () => {
520+
// Setup
521+
mockToolUse.params.command = "pwd"
522+
// No suggestions property
523+
524+
// Execute
525+
await executeCommandTool(
526+
mockCline as unknown as Task,
527+
mockToolUse,
528+
mockAskApproval as unknown as AskApproval,
529+
mockHandleError as unknown as HandleError,
530+
mockPushToolResult as unknown as PushToolResult,
531+
mockRemoveClosingTag as unknown as RemoveClosingTag,
532+
)
533+
534+
// Verify - should pass command without suggestions
535+
expect(mockAskApproval).toHaveBeenCalledWith("command", "pwd")
536+
expect(mockExecuteCommand).toHaveBeenCalled()
537+
expect(mockPushToolResult).toHaveBeenCalledWith("Command executed")
538+
})
539+
})
269540
})

0 commit comments

Comments
 (0)