Fix unintended credential exposure to MCP Servers #17311
Conversation
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 significantly enhances the security posture of the Gemini CLI by addressing potential credential exposure to MCP servers. It introduces robust environment variable scrubbing, expands the list of sensitive variables that are automatically redacted, and provides a secure method for extensions to pass necessary settings. Furthermore, it improves user awareness through explicit warnings and updated documentation, ensuring that users are informed about security best practices when interacting with MCP servers. 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
|
Adib234
force-pushed
the
adibakm/security-review
branch
from
8a51c7d to
3dae904
Compare
![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 aims to enhance security by preventing unintended credential exposure to MCP servers through mandatory environment variable scrubbing for stdio transport, an expanded list of sensitive variables, and clearer user warnings. However, a critical security vulnerability exists: sensitive extension settings and user-defined environment variables can bypass the redaction policy. Specifically, the new getExtensionEnvironment function passes all resolved settings, including those marked as sensitive, to the MCP server's environment, which could inadvertently leak credentials to a child process. To resolve this, ensure the sensitive flag on extension settings is respected and apply the sanitization logic to the final merged environment object for consistent policy enforcement.
|
Size Change: +1.03 kB (0%) Total Size: 23.5 MB
ℹ️ View Unchanged
|
gemini-cli
bot
added
the
priority/p1
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces important security enhancements by redacting sensitive environment variables. However, a critical regression has been identified in how extension settings are handled. The current implementation blocks sensitive settings from being passed to an extension's own MCP server, which will likely break extensions that rely on this. These extension settings are considered trusted and should not be over-redacted, as outlined in the provided comments with suggestions to fix this issue while maintaining security goals.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request effectively addresses an unintended credential exposure vulnerability in MCP servers. The changes enforce mandatory environment variable scrubbing for all stdio transport servers, expand the list of sensitive variables to be redacted, and provide a secure mechanism for passing extension-defined settings. The addition of user-facing security warnings is also a welcome improvement. The implementation is robust and correctly fixes the identified security issue. I have no high or critical severity concerns with these changes.
|
Hi there! Thank you for your contribution to Gemini CLI. We really appreciate the time and effort you've put into this pull request. To keep our backlog manageable and ensure we're focusing on current priorities, we are closing pull requests that haven't seen maintainer activity for 30 days. Currently, the team is prioritizing work associated with 🔒 maintainer only or help wanted issues. If you believe this change is still critical, please feel free to comment with updated details. Otherwise, we encourage contributors to focus on open issues labeled as help wanted. Thank you for your understanding! |
2 participants
Summary
Handles unintended credential exposure to MCP servers by environment variable scrubbing, clear user warnings and sandboxing.
Details
GEMINI_API_KEY,GOOGLE_API_KEY, andGOOGLE_APPLICATION_CREDENTIALSto the list of variables that are never allowed to be passed to child processes.- Added a security warning to the gemini mcp add command for stdio servers.
- Updated
docs/tools/mcp-server.mdwith a new "Security Considerations" section explaining environment redaction and the risks of untrusted servers.Related Issues
Closes https://github.com/google-gemini/maintainers-gemini-cli/issues/1216
How to Validate
Sensitive credentials like
GEMINI_API_KEYandGOOGLE_API_KEYshould not be passed by default to a MCP serversPre-Merge Checklist