Skip to content

fix(security): Shell commands auto-executing in 'suggest' mode without permission#197

Merged
tibo-openai merged 3 commits into
openai:mainfrom
binbandit:suggestion-fix
Apr 17, 2025
Merged

fix(security): Shell commands auto-executing in 'suggest' mode without permission#197
tibo-openai merged 3 commits into
openai:mainfrom
binbandit:suggestion-fix

Conversation

@binbandit

Copy link
Copy Markdown
Contributor

Problem

There's a security vulnerability in the current implementation where shell commands are being executed without requesting user permission even when in 'suggest' mode. According to our documentation:

In Suggest mode (default): All file writes/patches and ALL shell/Bash commands should require approval.

However, the current implementation in approvals.ts was auto-approving commands deemed "safe" by the isSafeCommand function, bypassing the user permission requirement. This is a security risk as users expect all shell commands to require explicit approval in 'suggest' mode.

Solution

This PR fixes the issue by modifying the canAutoApprove function in approvals.ts to respect the 'suggest' mode policy for all shell commands:

  1. Added an early check at the beginning of canAutoApprove to immediately return { type: "ask-user" } when the policy is suggest, regardless of whether the command is considered "safe" or not.

  2. Added a similar check in the bash command handling section to ensure bash commands also respect the 'suggest' mode.

  3. Updated tests to verify the new behavior, ensuring that all shell commands require approval in 'suggest' mode, while still being auto-approved in 'auto-edit' and 'full-auto' modes when appropriate.

Testing

All tests pass, confirming that the fix works as expected. The updated tests verify that:

  • All commands (even "safe" ones) require approval in 'suggest' mode
  • Safe commands are still auto-approved in 'auto-edit' mode
  • Bash commands with redirects still require approval in all modes

This change ensures that the behavior matches what's documented and what users expect, improving security by requiring explicit permission for all shell commands in the default 'suggest' mode.

@tibo-openai tibo-openai merged commit b62ef70 into openai:main Apr 17, 2025
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 17, 2025
@tibo-openai

Copy link
Copy Markdown
Collaborator

good find, ty!

@fouad-openai

Copy link
Copy Markdown
Collaborator

Hey! Sorry, this was a docs issue and not unintended behavior.

The purpose of "suggest" is to allow the model to read files, but require approval for editing files or running arbitrary (non-read) commands. There's a fair case for adding another approval mode like "always-ask" that is the strictest where even file-read commands like grep have to be approved.

Thanks for the contribution! I'm going to revert this PR now that README.md should be clearer.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants