Skip to content

Commit cf6c380

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 88c4261 commit cf6c380

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

+1998
-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
@@ -59,7 +59,45 @@ beforeEach(() => {
5959
return
6060
}
6161

62-
const didApprove = await askApproval("command", block.params.command)
62+
// Handle suggestions if provided
63+
let commandWithSuggestions = block.params.command
64+
if (block.params.suggestions) {
65+
let suggestions = block.params.suggestions
66+
// Handle both array and string formats
67+
if (typeof suggestions === "string") {
68+
const suggestionsString = suggestions
69+
70+
// First try to parse as JSON array
71+
if (suggestionsString.trim().startsWith("[")) {
72+
try {
73+
suggestions = JSON.parse(suggestionsString)
74+
} catch (jsonError) {
75+
// Fall through to XML parsing
76+
}
77+
}
78+
79+
// If not JSON or JSON parsing failed, try to parse individual <suggest> tags
80+
if (!Array.isArray(suggestions)) {
81+
const individualSuggestMatches = suggestionsString.match(/<suggest>(.*?)<\/suggest>/g)
82+
if (individualSuggestMatches) {
83+
suggestions = individualSuggestMatches
84+
.map((match) => {
85+
const content = match.match(/<suggest>(.*?)<\/suggest>/)
86+
return content ? content[1] : ""
87+
})
88+
.filter((suggestion) => suggestion.length > 0)
89+
} else {
90+
// If no XML tags found, treat as single suggestion
91+
suggestions = [suggestions]
92+
}
93+
}
94+
}
95+
if (Array.isArray(suggestions) && suggestions.length > 0) {
96+
commandWithSuggestions = `${block.params.command}\n<suggestions>\n${suggestions.join("\n")}\n</suggestions>`
97+
}
98+
}
99+
100+
const didApprove = await askApproval("command", commandWithSuggestions)
63101
if (!didApprove) {
64102
return
65103
}
@@ -309,4 +347,237 @@ describe("executeCommandTool", () => {
309347
expect(mockOptions.commandExecutionTimeout).toBeDefined()
310348
})
311349
})
350+
351+
describe("Suggestions functionality", () => {
352+
it("should pass command with suggestions when suggestions are provided as array", async () => {
353+
// Setup
354+
mockToolUse.params.command = "npm install"
355+
mockToolUse.params.suggestions = JSON.stringify([
356+
"npm install --save",
357+
"npm install --save-dev",
358+
"npm install --global",
359+
])
360+
361+
// Execute
362+
await executeCommandTool(
363+
mockCline as unknown as Task,
364+
mockToolUse,
365+
mockAskApproval as unknown as AskApproval,
366+
mockHandleError as unknown as HandleError,
367+
mockPushToolResult as unknown as PushToolResult,
368+
mockRemoveClosingTag as unknown as RemoveClosingTag,
369+
)
370+
371+
// Verify
372+
const expectedCommandWithSuggestions = `npm install
373+
<suggestions>
374+
npm install --save
375+
npm install --save-dev
376+
npm install --global
377+
</suggestions>`
378+
expect(mockAskApproval).toHaveBeenCalledWith("command", expectedCommandWithSuggestions)
379+
expect(mockExecuteCommand).toHaveBeenCalled()
380+
expect(mockPushToolResult).toHaveBeenCalledWith("Command executed")
381+
})
382+
383+
it("should pass command with suggestions when suggestions are provided as JSON string", async () => {
384+
// Setup
385+
mockToolUse.params.command = "git commit"
386+
mockToolUse.params.suggestions =
387+
'["git commit -m \\"Initial commit\\"", "git commit --amend", "git commit --no-verify"]'
388+
389+
// Execute
390+
await executeCommandTool(
391+
mockCline as unknown as Task,
392+
mockToolUse,
393+
mockAskApproval as unknown as AskApproval,
394+
mockHandleError as unknown as HandleError,
395+
mockPushToolResult as unknown as PushToolResult,
396+
mockRemoveClosingTag as unknown as RemoveClosingTag,
397+
)
398+
399+
// Verify
400+
const expectedCommandWithSuggestions = `git commit
401+
<suggestions>
402+
git commit -m "Initial commit"
403+
git commit --amend
404+
git commit --no-verify
405+
</suggestions>`
406+
expect(mockAskApproval).toHaveBeenCalledWith("command", expectedCommandWithSuggestions)
407+
expect(mockExecuteCommand).toHaveBeenCalled()
408+
expect(mockPushToolResult).toHaveBeenCalledWith("Command executed")
409+
})
410+
411+
it("should handle single suggestion as string", async () => {
412+
// Setup
413+
mockToolUse.params.command = "docker run"
414+
mockToolUse.params.suggestions = "docker run -it ubuntu:latest"
415+
416+
// Execute
417+
await executeCommandTool(
418+
mockCline as unknown as Task,
419+
mockToolUse,
420+
mockAskApproval as unknown as AskApproval,
421+
mockHandleError as unknown as HandleError,
422+
mockPushToolResult as unknown as PushToolResult,
423+
mockRemoveClosingTag as unknown as RemoveClosingTag,
424+
)
425+
426+
// Verify
427+
const expectedCommandWithSuggestions = `docker run
428+
<suggestions>
429+
docker run -it ubuntu:latest
430+
</suggestions>`
431+
expect(mockAskApproval).toHaveBeenCalledWith("command", expectedCommandWithSuggestions)
432+
expect(mockExecuteCommand).toHaveBeenCalled()
433+
expect(mockPushToolResult).toHaveBeenCalledWith("Command executed")
434+
})
435+
436+
it("should handle empty suggestions array", async () => {
437+
// Setup
438+
mockToolUse.params.command = "ls"
439+
mockToolUse.params.suggestions = JSON.stringify([])
440+
441+
// Execute
442+
await executeCommandTool(
443+
mockCline as unknown as Task,
444+
mockToolUse,
445+
mockAskApproval as unknown as AskApproval,
446+
mockHandleError as unknown as HandleError,
447+
mockPushToolResult as unknown as PushToolResult,
448+
mockRemoveClosingTag as unknown as RemoveClosingTag,
449+
)
450+
451+
// Verify - should pass command without suggestions
452+
expect(mockAskApproval).toHaveBeenCalledWith("command", "ls")
453+
expect(mockExecuteCommand).toHaveBeenCalled()
454+
expect(mockPushToolResult).toHaveBeenCalledWith("Command executed")
455+
})
456+
457+
it("should handle invalid JSON string in suggestions", async () => {
458+
// Setup
459+
mockToolUse.params.command = "echo test"
460+
mockToolUse.params.suggestions = "invalid json {"
461+
462+
// Execute
463+
await executeCommandTool(
464+
mockCline as unknown as Task,
465+
mockToolUse,
466+
mockAskApproval as unknown as AskApproval,
467+
mockHandleError as unknown as HandleError,
468+
mockPushToolResult as unknown as PushToolResult,
469+
mockRemoveClosingTag as unknown as RemoveClosingTag,
470+
)
471+
472+
// Verify - should treat invalid JSON as single suggestion
473+
const expectedCommandWithSuggestions = `echo test
474+
<suggestions>
475+
invalid json {
476+
</suggestions>`
477+
expect(mockAskApproval).toHaveBeenCalledWith("command", expectedCommandWithSuggestions)
478+
expect(mockExecuteCommand).toHaveBeenCalled()
479+
expect(mockPushToolResult).toHaveBeenCalledWith("Command executed")
480+
})
481+
482+
it("should parse individual <suggest> XML tags correctly", async () => {
483+
// Setup
484+
mockToolUse.params.command = "npm install"
485+
mockToolUse.params.suggestions =
486+
"<suggest>npm install --save</suggest><suggest>npm install --save-dev</suggest><suggest>npm install --global</suggest>"
487+
488+
// Execute
489+
await executeCommandTool(
490+
mockCline as unknown as Task,
491+
mockToolUse,
492+
mockAskApproval as unknown as AskApproval,
493+
mockHandleError as unknown as HandleError,
494+
mockPushToolResult as unknown as PushToolResult,
495+
mockRemoveClosingTag as unknown as RemoveClosingTag,
496+
)
497+
498+
// Verify
499+
const expectedCommandWithSuggestions = `npm install
500+
<suggestions>
501+
npm install --save
502+
npm install --save-dev
503+
npm install --global
504+
</suggestions>`
505+
expect(mockAskApproval).toHaveBeenCalledWith("command", expectedCommandWithSuggestions)
506+
expect(mockExecuteCommand).toHaveBeenCalled()
507+
expect(mockPushToolResult).toHaveBeenCalledWith("Command executed")
508+
})
509+
510+
it("should parse single <suggest> XML tag correctly", async () => {
511+
// Setup
512+
mockToolUse.params.command = "git push"
513+
mockToolUse.params.suggestions = "<suggest>git push origin main</suggest>"
514+
515+
// Execute
516+
await executeCommandTool(
517+
mockCline as unknown as Task,
518+
mockToolUse,
519+
mockAskApproval as unknown as AskApproval,
520+
mockHandleError as unknown as HandleError,
521+
mockPushToolResult as unknown as PushToolResult,
522+
mockRemoveClosingTag as unknown as RemoveClosingTag,
523+
)
524+
525+
// Verify
526+
const expectedCommandWithSuggestions = `git push
527+
<suggestions>
528+
git push origin main
529+
</suggestions>`
530+
expect(mockAskApproval).toHaveBeenCalledWith("command", expectedCommandWithSuggestions)
531+
expect(mockExecuteCommand).toHaveBeenCalled()
532+
expect(mockPushToolResult).toHaveBeenCalledWith("Command executed")
533+
})
534+
535+
it("should handle mixed content with <suggest> tags", async () => {
536+
// Setup
537+
mockToolUse.params.command = "docker run"
538+
mockToolUse.params.suggestions =
539+
"Some text before <suggest>docker run -it ubuntu</suggest> and <suggest>docker run -d nginx</suggest> with text after"
540+
541+
// Execute
542+
await executeCommandTool(
543+
mockCline as unknown as Task,
544+
mockToolUse,
545+
mockAskApproval as unknown as AskApproval,
546+
mockHandleError as unknown as HandleError,
547+
mockPushToolResult as unknown as PushToolResult,
548+
mockRemoveClosingTag as unknown as RemoveClosingTag,
549+
)
550+
551+
// Verify
552+
const expectedCommandWithSuggestions = `docker run
553+
<suggestions>
554+
docker run -it ubuntu
555+
docker run -d nginx
556+
</suggestions>`
557+
expect(mockAskApproval).toHaveBeenCalledWith("command", expectedCommandWithSuggestions)
558+
expect(mockExecuteCommand).toHaveBeenCalled()
559+
expect(mockPushToolResult).toHaveBeenCalledWith("Command executed")
560+
})
561+
562+
it("should work normally when no suggestions are provided", async () => {
563+
// Setup
564+
mockToolUse.params.command = "pwd"
565+
// No suggestions property
566+
567+
// Execute
568+
await executeCommandTool(
569+
mockCline as unknown as Task,
570+
mockToolUse,
571+
mockAskApproval as unknown as AskApproval,
572+
mockHandleError as unknown as HandleError,
573+
mockPushToolResult as unknown as PushToolResult,
574+
mockRemoveClosingTag as unknown as RemoveClosingTag,
575+
)
576+
577+
// Verify - should pass command without suggestions
578+
expect(mockAskApproval).toHaveBeenCalledWith("command", "pwd")
579+
expect(mockExecuteCommand).toHaveBeenCalled()
580+
expect(mockPushToolResult).toHaveBeenCalledWith("Command executed")
581+
})
582+
})
312583
})

0 commit comments

Comments
 (0)