fix(gateway): replace bare text approval with /approve and /deny commands#2002
Merged
fix(gateway): replace bare text approval with /approve and /deny commands#2002
Conversation
…ands The gateway approval system previously intercepted bare 'yes'/'no' text from the user's next message to approve/deny dangerous commands. This was fragile and dangerous — if the agent asked a clarify question and the user said 'yes' to answer it, the gateway would execute the pending dangerous command instead. (Fixes #1888) Changes: - Remove bare text matching ('yes', 'y', 'approve', 'ok', etc.) from _handle_message approval check - Add /approve and /deny as gateway-only slash commands in the command registry - /approve supports scoping: /approve (one-time), /approve session, /approve always (permanent) - Add 5-minute timeout for stale approvals - Gateway appends structured instructions to the agent response when a dangerous command is pending, telling the user exactly how to respond - 9 tests covering approve, deny, timeout, scoping, and verification that bare 'yes' no longer triggers execution Credit to @solo386 and @FlyByNight69420 for identifying and reporting this security issue in PR #1971 and issue #1888.
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1888. Replaces the fragile bare text-matching approval system in the gateway with explicit
/approveand/denyslash commands.The bug: The gateway intercepted bare
yes/y/approve/oktext from the user's next message to approve dangerous commands. If the agent asked a clarify question (or moved on to other work) and the user said "yes" to answer it, the gateway would execute the pending dangerous command instead. Security concern raised by @FlyByNight69420 in #1888 and addressed by @solo386 in PR #1971.The fix: Rather than patching the text matching, replaced the entire mechanism:
yes,y,approve,ok,go,do it, etc.)/approveand/denyas gateway-only slash commands in the command registry/approvesupports scoping:/approve— one-time execution/approve session— approve this pattern for the session/approve always— approve permanently/approve//denyinstructions to the agent response when a dangerous command is pendingTest Plan
9 tests in
tests/gateway/test_approve_deny_commands.py:All 1211 gateway tests + 72 approval tests pass.