fix(policy): enhance shell command safety and parsing#15034
fix(policy): enhance shell command safety and parsing#15034allenhutchison merged 20 commits intomainfrom
Conversation
…d injection and parsing, and refactor test setup.
Summary of ChangesHello @allenhutchison, 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 enhances the security and robustness of the policy engine's handling of shell commands. It introduces advanced parsing capabilities to detect and prevent various forms of command injection and substitution, ensuring that only explicitly allowed commands and their sub-components are executed. The changes also include a critical fail-safe mechanism to handle unparseable commands securely, alongside refactoring for better maintainability and comprehensive new tests to validate these security improvements. 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 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: +1.8 kB (+0.01%) Total Size: 22.3 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Code Review
This pull request significantly enhances shell command safety by implementing recursive parsing and checking for chained commands and substitutions. The introduction of a fail-safe mechanism for parsing errors and the comprehensive new tests are excellent additions for security and robustness. The refactoring in toml-loader also improves maintainability. I've identified one high-severity issue where the wrong policy rule could be reported for a denied composite command, and I've provided a suggestion to fix it.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
This change fixes an issue where the PolicyEngine would not correctly attribute the rule that caused an ASK_USER decision when processing compound shell commands (e.g., 'git status && unknown_cmd'). It ensures that if a subcommand triggers an ASK_USER decision (either by default or explicit rule), the 'matchedRule' reflects that specific cause, rather than being overwritten or left undefined. Also adds a 'name' property to PolicyRule for better debugging and updates tests to cover these scenarios.
This ensures that the tests, which use Bash-specific syntax (chained commands, subshells), always use the Bash parser in shell-utils. This avoids parsing failures on Windows where the logic would otherwise default to PowerShell parsing.
abhipatel12
left a comment
There was a problem hiding this comment.
LGTM overall! Had a few comments
…, fixed tests and build errors)
…n and the `allowRedirection` rule.
…gemini-cli into adh/fix/pr-15006-feedback
abhipatel12
left a comment
There was a problem hiding this comment.
LGTM! Couple of comment nits
…15034) Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Abhi <43648792+abhipatel12@users.noreply.github.com>
…15034) Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Abhi <43648792+abhipatel12@users.noreply.github.com>
Addressing feedback from PR #15006.
Changes:
echo $(rm -rf /)).toml-loaderfor better testability.