Repository navigation
fix(core): resolve policy engine bugs affecting tool approvals - #26540
Abhijit-2592 wants to merge 2 commits into
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses critical bugs within the policy engine that hindered the persistence of tool approvals and caused redundant user prompts in permissive modes. By refining regex matching, adjusting downgrade logic, and improving shell command parsing, the changes ensure a smoother and more consistent user experience when executing tools. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
Size Change: +347 B (0%) Total Size: 34 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Code Review
This pull request modifies the policy engine to remove sandbox checks for shell redirection in AUTO_EDIT and YOLO modes, updates JSON property matching patterns to use hex null characters, and enhances shell wrapper stripping to support direct script execution. A critical security concern was raised regarding the removal of the sandbox check in AUTO_EDIT mode, as it could allow unauthorized file writes via shell redirection when a sandbox is not active.
Abhijit-2592
left a comment
There was a problem hiding this comment.
I have applied the suggested security fix. The sandboxEnabled check is now restored for AUTO_EDIT mode, ensuring that shell redirection requires a sandbox in that mode, while YOLO mode remains unaffected. Thanks for catching this!
Fixes #24772, #16970. - Fixes regex null-byte mismatch in `buildParamArgsPattern` so "always allow" works. - Removes sandbox requirement from `shouldDowngradeForRedirection` so YOLO/AUTO_EDIT modes behave correctly. - Enhances `stripShellWrapper` to recognize generic shell scripts (e.g., `sh script.sh`).
eaaa9e3 to
11eadac
Compare
| } | ||
|
|
||
| // In AUTO_EDIT mode, only bypass downgrade if sandboxing is enabled. | ||
| const sandboxEnabled = !(this.sandboxManager instanceof NoopSandboxManager); |
There was a problem hiding this comment.
i'm not sure this is going to fix the issue the user is reporting, since wouldn't this still ask for approval?
Summary
This PR fixes several critical issues in the policy engine that were preventing tool approvals from persisting correctly and causing unnecessary approval prompts in permissive modes (
YOLO,AUTO_EDIT).Details
buildParamArgsPatternutility was incorrectly escaping null bytes using\\\\0. This caused property-matching regexes to fail against thestableStringifyoutput (which uses literal\x00markers), breaking permanent approvals for specific files or patterns.shouldDowngradeForRedirectionto skip the automatic downgrade toASK_USERwhen the approval mode isYOLOunconditionally. ForAUTO_EDITmode, the downgrade is skipped only if a sandbox is enabled, addressing security concerns regarding unauthorized file writes via shell redirection.stripShellWrapperto recognize and strip generic shell program calls (e.g.,sh script.sh,bash script.sh), allowing better matching against approved script paths.Related Issues
Fixes #24772
Fixes #16970
How to Validate
npm run test -w @google/gemini-cli-core -- src/policy/policy-engine.test.tsto verify policy engine logic.npm run test -w @google/gemini-cli-core -- src/utils/shell-utils.test.tsto verify shell wrapper stripping.run_shell_commandwith a specific script and ensuring it doesn't prompt again in the same or future sessions.Pre-Merge Checklist