fix: enforce denied commands regardless of auto-approval settings#12293
Draft
roomote-v0[bot] wants to merge 1 commit intomainfrom
Draft
fix: enforce denied commands regardless of auto-approval settings#12293roomote-v0[bot] wants to merge 1 commit intomainfrom
roomote-v0[bot] wants to merge 1 commit intomainfrom
Conversation
Previously, the denied commands list was only checked when alwaysAllowExecute was true. This meant agents could bypass the deny list by wrapping denied commands inside chains (&&, ||, ;) or multi-line scripts like heredocs. Now the deny list check runs before the autoApprovalEnabled guard, so denied commands are always blocked regardless of whether auto-approval or alwaysAllowExecute is enabled. Fixes #12292
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
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.
Related GitHub Issue
Closes: #12292
Description
This PR attempts to address Issue #12292. Feedback and guidance are welcome.
Problem: The denied commands list was only checked inside the
alwaysAllowExecute === trueguard incheckAutoApproval(). This meant:alwaysAllowExecutewas false, the deny list was completely ignored&&,||,;) or multi-line scripts (heredocs)Fix: Moved the denied commands check to run before the
autoApprovalEnabledguard incheckAutoApproval(). Now denied commands are always blocked regardless of whether auto-approval oralwaysAllowExecuteis enabled. The existinggetCommandDecision()function already handles command chain parsing and longest-prefix-match conflict resolution, so the fix is minimal -- just calling it earlier in the flow.The longest-prefix-match logic is preserved, so more specific allow entries (e.g.,
rm -i) can still override a broader deny entry (e.g.,rm).Test Procedure
checkAutoApproval.spec.tscovering:autoApprovalEnabledis falsealwaysAllowExecuteis false&&chains are caughtcommands.spec.tsforgetCommandDecisioncovering chained/wrapped command denial scenariosRun tests:
cd src && pnpm vitest run core/auto-approval/Pre-Submission Checklist
Documentation Updates
Additional Notes
The core change is a single 8-line block added to
src/core/auto-approval/index.tsthat checks denied commands before the auto-approval guard. The rest is test coverage.Interactively review PR in Roo Code Cloud