fix(patch): cherry-pick e9a9474 to release/v0.29.0-preview.0-pr-18840 to patch version v0.29.0-preview.0 and create version 0.29.0-preview.1#18841
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 cherry-picks a commit to create a patch release for a preview version. The primary goal of the underlying change is to streamline and simplify the handling of environment variables for Model Context Protocol (MCP) servers. This involves removing specific redaction logic for certain API keys and related documentation and tests, as well as simplifying how extension-specific environment variables are managed during MCP server setup. Highlights
Changelog
Activity
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
|
There was a problem hiding this comment.
Code Review
This pull request removes environment variable sanitization for MCP servers, which introduces a critical security vulnerability. By making environment redaction configurable and defaulting it to off, and by allowing user-provided environment variables to bypass sanitization, there is a high risk of sensitive credential exposure to third-party tools. Specifically, changes in packages/core/src/tools/mcp-client.ts effectively disable environment redaction, and packages/core/src/services/environmentSanitization.ts removes several sensitive keys from the denylist. Furthermore, the removal of the security warning in the CLI command for adding MCP servers hides these risks from the user. It is strongly recommended to restore hardcoded security guarantees and user-facing warnings to prevent unintentional leakage of sensitive credentials.
I am having trouble creating individual review comments. Click here to see my feedback.
packages/core/src/tools/mcp-client.ts (1935-1938)
This change introduces a critical security regression by making environment redaction configurable and defaulting it to false. Previously, redaction was hardcoded to true, preventing sensitive credentials like GOOGLE_API_KEY from leaking to MCP servers. Now, sanitizeEnvironment is called with enableEnvironmentVariableRedaction defaulting to false, effectively disabling redaction. Additionally, user-configured mcpServerConfig.env is spread after sanitization, completely bypassing the redaction logic. This creates a significant risk of unintentional leakage of sensitive credentials (e.g., AWS_SECRET_ACCESS_KEY, GITHUB_TOKEN) to MCP server subprocesses. It is recommended to restore a safer implementation that enables redaction by default for MCP servers.
env: sanitizeEnvironment(
{
...process.env,
...(mcpServerConfig.env || {}),
},
{
...sanitizationConfig,
enableEnvironmentVariableRedaction: true,
},
) as Record<string, string>,packages/cli/src/commands/mcp/add.ts (131-136)
Removing this security warning is highly discouraged, especially given the concurrent weakening of environment sanitization in the core package. Running third-party MCP servers via stdio transport is a high-risk operation as it involves executing external code that can inherit the user's environment. The warning correctly informed users that while the CLI attempts to redact secrets, they should only run servers from trusted sources. Additionally, the CLI's use of shell-quote for variable expansion in the command string means that secrets can still be leaked via command-line arguments, a risk that this warning helped mitigate.
packages/core/src/services/environmentSanitization.ts (106-108)
The removal of GEMINI_API_KEY, GOOGLE_API_KEY, and GOOGLE_APPLICATION_CREDENTIALS from the NEVER_ALLOWED_ENVIRONMENT_VARIABLES set weakens the security posture of environment sanitization. These variables often contain sensitive credentials that should not be leaked to subprocesses.
Even if the goal is to allow users to explicitly pass these variables, they should still be redacted by default when present in the ambient environment. If my other suggestion to re-enable redaction for MCP servers is adopted, these variables should be added back to this list to ensure they are properly redacted.
|
Size Change: -1.07 kB (0%) Total Size: 23.9 MB
ℹ️ View Unchanged
|
31a7a81
into
release/v0.29.0-preview.0-pr-18840
… to patch version v0.29.0-preview.0 and create version 0.29.0-preview.1 (google-gemini#18841) Co-authored-by: Adib234 <30782825+Adib234@users.noreply.github.com>
… to patch version v0.29.0-preview.0 and create version 0.29.0-preview.1 (google-gemini#18841) Co-authored-by: Adib234 <30782825+Adib234@users.noreply.github.com>
This PR automatically cherry-picks commit e9a9474 to patch version v0.29.0-preview.0 in the preview release to create version 0.29.0-preview.1.