feat: Persistent "Always Allow" policies with granular shell & MCP support#14737
feat: Persistent "Always Allow" policies with granular shell & MCP support#14737allenhutchison merged 22 commits intomainfrom
Conversation
…tool control and persistence.
…ti-cloud platform tools.
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 tool interaction experience by introducing persistent 'Always Allow' policies. It allows users to save their approval decisions, reducing friction for frequently used tools. The changes also bring granular control over shell commands and full support for Multi-Cloud Platform tools, ensuring that policy management is both flexible and robust. 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
|
![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 introduces a valuable feature for persisting "Always Allow" decisions, which will improve the user experience by reducing repetitive confirmations. The implementation of granular controls for shell commands using commandPrefix and argsPattern, as well as support for MCP tools via mcpName, is well-thought-out. The changes are logically structured, and the addition of persistence tests is a great step towards ensuring the reliability of this new functionality.
I've found one critical issue related to how policy data is persisted, which could lead to configuration file corruption. My specific comment addresses this with a suggested fix. Once that is addressed, this PR will be in excellent shape.
|
Size Change: +5.1 kB (+0.02%) Total Size: 21.6 MB
ℹ️ View Unchanged
|
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
This commit addresses all instances of the `@typescript-eslint/no-floating-promises` linting error. Promises returned by `messageBus.publish` and other asynchronous operations within tool callbacks and tests are now properly awaited or explicitly marked with `void` (where appropriate, but `await` was preferred in these cases). This ensures that asynchronous operations complete before subsequent logic executes and resolves linting warnings, improving code reliability and maintainability.
This change updates the `scripts/lint.js` to: 1. Upgrade `pip` in the virtual environment before installing `yamllint`. This resolves compatibility issues with newer Python versions (e.g. 3.14). 2. Explicitly use `https://pypi.org/simple` as the index URL for `yamllint` installation to avoid potential authentication issues with private registries.
Corrected the `yamllint` installer in `scripts/lint.js` to properly use `path.join` for constructing the cross-platform Python executable path. Previously, `path.join` was incorrectly placed directly within the shell command string, leading to a `no-undef` ESLint error during pre-commit checks. This change extracts the path construction into a JavaScript variable, ensuring it's evaluated correctly before being used in the shell command. This fully addresses the review comment on PR #14746 regarding cross-platform compatibility.
Corrected formatting for the `argsPattern` escape string in `packages/core/src/policy/config.ts`. This change aligns the string escaping with code style guidelines, splitting a long line for better readability.
gemini-cli
Bot
added
size/l
abhipatel12
left a comment
abhipatel12
left a comment
There was a problem hiding this comment.
Had a few questions! Lmk what you think!
|
|
||
| if (message.persist) { | ||
| try { | ||
| const userPoliciesDir = Storage.getUserPoliciesDir(); |
There was a problem hiding this comment.
i think this will cause a security issue. The default behavior should be to persist the policies for a project, not the user.
Example scenario:
- i grant a bunch of shell permissions when working in my project
- i clone a random project from github and now gemini launches in that project with all the permissions I already allowed
There was a problem hiding this comment.
something like this
getProjectPolicies(): string {
const policiesPath = path.join(this.getGeminiDir(), 'policies.json');
if (!fs.existsSync(policiesPath)) {
fs.mkdirSync(path.dirname(policiesPath), { recursive: true });
fs.writeFileSync(policiesPath, '{}');
}
return policiesPath;
}
There was a problem hiding this comment.
We don't currently support policy at the project level only at the user and admin level.
There was a problem hiding this comment.
We don't currently support policy at the project level only at the user and admin level.
but wont this cause a security issue once tool use permissions is persisted? if someone enables a sensitive permission for one project they dont expect that to translate to every project
…ialization and atomic writes.
…low' to 'allow always'
…roduce policy update options for tool invocations.
abhipatel12
left a comment
abhipatel12
left a comment
There was a problem hiding this comment.
Small nit about a comment but LGTM otherwise!
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
…th granular shell & MCP support (google-gemini#14737)
3 participants
Description
This PR introduces the ability for users to persist "Always Allow" decisions for tools, streamlining the user experience by reducing repetitive confirmations. It includes granular control for shell commands and full support for MCP tools.
Changes
commandPrefixandargsPatternto policy configuration. This allows users to whitelist specific shell commands (e.g.,git status) without granting blanket access to the shell tool.mcpNamein policy configuration to distinguish tools from different MCP servers.ToolConfirmationMessageUI to include the "Always allow" option.packages/core/src/policy/persistence.test.tsto verify policy persistence and matching logic.Commits
commandPrefixandargsPatternto policies for granular tool control and persistence.mcpNamein policy updates and configuration for multi-cloud platform tools.Fixes: #12898