Revert unintended credentials exposure#18840
Conversation
|
Hi @Adib234, thank you so much for your contribution to Gemini CLI! We really appreciate the time and effort you've put into this. We're making some updates to our contribution process to improve how we track and review changes. Please take a moment to review our recent discussion post: Improving Our Contribution Process & Introducing New Guidelines. Key Update: Starting January 26, 2026, the Gemini CLI project will require all pull requests to be associated with an existing issue. Any pull requests not linked to an issue by that date will be automatically closed. Thank you for your understanding and for being a part of our community! |
Summary of ChangesHello @Adib234, 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 reverts previous changes related to environment variable sanitization and redaction for MCP servers. The primary impact is that certain sensitive API keys will no longer be automatically redacted when spawning MCP servers, and the associated security warnings and documentation have been removed. This streamlines the environment configuration but shifts the responsibility for sensitive variable handling more directly to the user. 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
|
|
Size Change: -1.07 kB (0%) Total Size: 24.3 MB
ℹ️ View Unchanged
|
gemini-cli
bot
added
the
priority/p1
![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 reverts several security-related features concerning MCP server integration, specifically removing the automatic redaction of sensitive environment variables (like API keys) for stdio transports, deleting security warnings, and removing related security documentation. These changes introduce a critical security vulnerability, potentially leading to credential leakage to MCP server subprocesses. I strongly advise against merging this PR in its current state.
I am having trouble creating individual review comments. Click here to see my feedback.
packages/core/src/services/environmentSanitization.ts (106-108)
These environment variables should not be removed from the denylist. Removing them introduces a critical security vulnerability by allowing sensitive credentials like API keys to be leaked to MCP server subprocesses. Please revert this part of the change.
packages/core/src/tools/mcp-client.ts (1901-1904)
This change introduces two critical security issues. First, it makes environment variable redaction opt-in (defaulting to false) instead of forcing it on for stdio transports. Second, it applies mcpServerConfig.env after sanitization, allowing these user-provided variables to bypass security checks entirely. This could lead to credential leakage. The environment for the subprocess should be constructed from all sources and then sanitized as a whole.
env: sanitizeEnvironment(
{
...process.env,
...(mcpServerConfig.env || {}),
},
{
...sanitizationConfig,
enableEnvironmentVariableRedaction: true,
},
) as Record<string, string>,docs/tools/mcp-server.md (742-756)
This change removes crucial security documentation about environment variable redaction, explicit environment variable passing, and the risks of untrusted servers. This information is vital for users to securely configure and use MCP servers. Please restore these sections to ensure users are aware of the potential security implications.
packages/cli/src/commands/mcp/add.ts (131-136)
Removing this security warning is a regression in user security awareness. Users should be explicitly warned about the risks of running stdio MCP servers, as they can inherit and potentially expose sensitive environment variables. Please restore this warning.
|
/patch preview |
|
✅ Patch workflow(s) dispatched successfully! 📋 Details:
🔗 Track Progress: |
|
🚀 Patch PR Created! 📋 Patch Details:
📝 Next Steps:
🔗 Track Progress: |
|
🚀 Patch Release Started! 📋 Release Details:
⏳ Status: The patch release is now running. You'll receive another update when it completes. 🔗 Track Progress: |
|
❌ Patch Release Failed! 📋 Details:
🔍 Next Steps:
🔗 Troubleshooting: |
|
/patch stable |
|
✅ Patch workflow(s) dispatched successfully! 📋 Details:
🔗 Track Progress: |
# Conflicts: # packages/core/src/tools/mcp-client.ts
|
🚀 Patch PR Created! 📋 Patch Details:
📝 Next Steps:
🔗 Track Progress: |
|
🚀 Patch Release Started! 📋 Release Details:
⏳ Status: The patch release is now running. You'll receive another update when it completes. 🔗 Track Progress: |
|
✅ Patch Release Complete! 📦 Release Details:
🎉 Status: Your patch has been successfully released and published to npm! 📝 What's Available:
🔗 Links: |
…ini/gemini-cli (#37) * fix(cli): resolve double rendering in shpool and address vscode lint warnings (google-gemini#18704) * feat(plan): document and validate Plan Mode policy overrides (google-gemini#18825) * Fix pressing any key to exit select mode. (google-gemini#18421) * fix(cli): update F12 behavior to only open drawer if browser fails (google-gemini#18829) * feat(plan): allow skills to be enabled in plan mode (google-gemini#18817) Co-authored-by: Jerop Kipruto <jerop@google.com> * docs(plan): add documentation for plan mode tools (google-gemini#18827) * Remove experimental note in extension settings docs (google-gemini#18822) * Update prompt and grep tool definition to limit context size (google-gemini#18780) * docs(plan): add `ask_user` tool documentation (google-gemini#18830) * Revert unintended credentials exposure (google-gemini#18840) * feat(core): update internal utility models to Gemini 3 (google-gemini#18773) * feat(a2a): add value-resolver for auth credential resolution (google-gemini#18653) * Removed getPlainTextLength (google-gemini#18848) * More grep prompt tweaks (google-gemini#18846) * refactor(cli): Reactive useSettingsStore hook (google-gemini#14915) * fix(mcp): Ensure that stdio MCP server execution has the `GEMINI_CLI=1` env variable populated. (google-gemini#18832) * fix(core): improve headless mode detection for flags and query args (google-gemini#18855) * refactor(cli): simplify UI and remove legacy inline tool confirmation logic (google-gemini#18566) * feat(cli): deprecate --allowed-tools and excludeTools in favor of policy engine (google-gemini#18508) * fix(workflows): improve maintainer detection for automated PR actions (google-gemini#18869) * refactor(cli): consolidate useToolScheduler and delete legacy implementation (google-gemini#18567) * Update changelog for v0.28.0 and v0.29.0-preview0 (google-gemini#18819) * fix(core): ensure sub-agents are registered regardless of tools.allowed (google-gemini#18870) --------- Co-authored-by: Brad Dux <959674+braddux@users.noreply.github.com> Co-authored-by: Jerop Kipruto <jerop@google.com> Co-authored-by: Jacob Richman <jacob314@gmail.com> Co-authored-by: Sandy Tao <sandytao520@icloud.com> Co-authored-by: Adib234 <30782825+Adib234@users.noreply.github.com> Co-authored-by: christine betts <chrstn@uw.edu> Co-authored-by: Christian Gunderman <gundermanc@gmail.com> Co-authored-by: Adam Weidman <65992621+adamfweidman@users.noreply.github.com> Co-authored-by: Dev Randalpura <devrandalpura@google.com> Co-authored-by: Pyush Sinha <pyushsinha20@gmail.com> Co-authored-by: Richie Foreman <richie.foreman@gmail.com> Co-authored-by: Gal Zahavi <38544478+galz10@users.noreply.github.com> Co-authored-by: Abhi <43648792+abhipatel12@users.noreply.github.com> Co-authored-by: Abhijit Balaji <abhijitbalaji@google.com> Co-authored-by: Bryan Morgan <bryanmorgan@google.com> Co-authored-by: g-samroberts <158088236+g-samroberts@users.noreply.github.com> Co-authored-by: matt korwel <matt.korwel@gmail.com>
2 participants
Summary
Reverts #17311
Details
Related Issues
How to Validate
Pre-Merge Checklist