Skip to content

Conversation

@roomote
Copy link
Contributor

@roomote roomote bot commented Jul 21, 2025

Fixes #5997 where Roo executed commands outside the configured allowed command set.

This PR adds validation in executeCommandTool to check commands against user-configured allowed and denied command lists before execution. Denied commands take precedence over allowed commands.

See PR description for full details.


Important

Adds command validation in executeCommandTool to check against allowed/denied lists before execution, with tests to verify behavior.

  • Behavior:
    • Adds command validation in executeCommandTool to check against allowed/denied lists before execution.
    • Denied commands take precedence over allowed commands.
    • Skips validation if alwaysAllowExecute is true.
  • Tests:
    • New test file executeCommandValidation.spec.ts to verify command validation logic.
    • Tests include allowed, denied, and edge cases like empty strings and unavailable provider.
  • Misc:
    • Refactors executeCommandTool.ts to include validation logic and handle command execution accordingly.

This description was created by Ellipsis for 5466af4. You can customize this summary. It will automatically update as commits are pushed.

- Add validation in executeCommandTool to check commands against user-configured allowed and denied command lists
- Denied commands take precedence over allowed commands
- Empty command strings in lists are ignored
- Add comprehensive tests for command validation scenarios
- Fixes #5997 where Roo executed commands outside the allowed command set
@roomote roomote bot requested review from cte, jr and mrubens as code owners July 21, 2025 07:18
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels Jul 21, 2025
if (isDenied) {
await cline.say(
"error",
`Command '${baseCommand}' is in the denied commands list and cannot be executed.`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using the internationalization function (t) for error messages instead of hardcoding them (e.g. for denied commands). This ensures UI consistency with other translatable strings.

Suggested change
`Command '${baseCommand}' is in the denied commands list and cannot be executed.`,
t('common:errors:command_denied', { command: baseCommand }),

This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Jul 21, 2025
@daniel-lxs
Copy link
Member

The issue needs some scoping to properly fix the potential issue, closing for now.

@daniel-lxs daniel-lxs closed this Jul 22, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jul 22, 2025
@github-project-automation github-project-automation bot moved this from Triage to Done in Roo Code Roadmap Jul 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Roo ran commands outside the allowed command set

4 participants