fix(patch): cherry-pick 37be162 to release/v0.23.0-preview.2-pr-15601 to patch version v0.23.0-preview.2 and create version 0.23.0-preview.3 #15603
Conversation
Summary of ChangesHello @gemini-cli-robot, 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 significantly improves the policy management system by enabling the definition of policy rules that can apply to multiple command prefixes simultaneously. This change enhances the system's ability to handle complex shell commands more granularly, ensuring that user-defined "always allow" policies can be applied to all relevant root commands within a single operation, thereby streamlining policy configuration and improving user experience. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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 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 counter productive. 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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Size Change: +914 B (0%) Total Size: 22 MB ℹ️ View Unchanged
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request cherry-picks a patch that enhances policy handling for shell commands by allowing commandPrefix to be an array for more granular "always allow" rules and fixes the command prefix matching regex to prevent partial matches. However, a high-severity Regular Expression Denial of Service (ReDoS) vulnerability was identified, which could allow an attacker to cause a denial of service by submitting a malicious regular expression pattern when updating a policy. Additionally, two high-severity issues were found: a potential security vulnerability in packages/core/src/policy/config.ts where an empty commandPrefix could lead to an overly permissive rule, and unreachable code in packages/core/src/tools/shell.ts. Addressing these issues will make the implementation more robust and secure.
| const argsPattern = message.argsPattern | ||
| ? new RegExp(message.argsPattern) | ||
| : undefined; |
There was a problem hiding this comment.
A Regular Expression Denial of Service (ReDoS) vulnerability exists here. The message.argsPattern is passed directly to the new RegExp() constructor without validation. An attacker can supply a maliciously crafted regular expression that causes catastrophic backtracking, leading to a denial of service as the event loop is blocked.
Remediation: Before constructing a regular expression, validate the message.argsPattern to disallow patterns known to cause ReDoS. Consider using a library like safe-regex or a similar tool to check the pattern's safety. If users are not meant to provide complex patterns, escape the input to treat it as a literal string.
| ? message.commandPrefix | ||
| : [message.commandPrefix]; | ||
|
|
||
| for (const prefix of prefixes) { |
There was a problem hiding this comment.
An empty commandPrefix can lead to an overly permissive rule being created. The resulting regex "command":"(?:[\\s\"]|$)" would match any command that is empty or starts with a space. This could be a security risk if a malicious model tries to bypass other rules by adding a leading space to a command. It's safer to ignore empty prefixes.
| for (const prefix of prefixes) { | |
| for (const prefix of prefixes.filter(Boolean)) { |
| const command = stripShellWrapper(this.params.command); | ||
| const rootCommands = [...new Set(getCommandRoots(command))]; | ||
| if (rootCommands.length > 0) { | ||
| return { commandPrefix: rootCommands }; | ||
| } | ||
| return { commandPrefix: this.params.command }; |
There was a problem hiding this comment.
The fallback logic on line 101 appears to be unreachable. The validateToolParamValues method for ShellTool (lines 505-507 in the full file) ensures that getCommandRoots(params.command) never returns an empty array for a valid command. Therefore, rootCommands.length will always be greater than 0. The code can be simplified by removing the check and the unreachable fallback.
const command = stripShellWrapper(this.params.command);
const rootCommands = [...new Set(getCommandRoots(command))];
return { commandPrefix: rootCommands };
galz10
deleted the
hotfix/v0.23.0-preview.2/0.23.0-preview.3/preview/cherry-pick-37be162/pr-15601
branch
4 participants
This PR automatically cherry-picks commit 37be162 to patch version v0.23.0-preview.2 in the preview release to create version 0.23.0-preview.3.