fix(core): allow environment variable expansion and explicit overrides for MCP servers#18837
fix(core): allow environment variable expansion and explicit overrides for MCP servers#18837
Conversation
…s for MCP servers
Summary of ChangesHello @galz10, 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 addresses issues where MCP servers failed to connect due to unexpanded environment variables and over-zealous sanitization of user-provided credentials. It introduces a robust environment variable expansion utility and refines the security model for passing environment variables to MCP servers. The changes ensure that user-defined variables are correctly expanded and explicitly trusted, while the base environment remains sanitized, enhancing both functionality and security. 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 introduces a valuable feature for expanding environment variables in MCP server configurations and refines the environment sanitization logic. A security review found no vulnerabilities in the audited files. However, there are a couple of high-severity issues that need attention: unit tests for Windows-style paths are flawed due to improper handling of backslash characters, and the documentation includes a misleading example for default value expansion (e.g., ${VAR:-default}) which is not supported by the new expandEnvVars function.
|
Size Change: +4.89 kB (+0.02%) Total Size: 25.7 MB
ℹ️ View Unchanged
|
jacob314
left a comment
There was a problem hiding this comment.
Approved once the expandEnvVars logic is hardened.
If we must keep it we should go through at least a couple iterations with Gemini 3.0 pro to harden it.

Summary
Fixes a regression where environment variable expansion was missing in MCP server configurations, and sanitization was redacting explicitly provided user credentials (introduced in #17311). This caused MCP servers to fail with 'Connection closed' errors when configured with dynamic secrets.
Details
PR #17311 introduced environment sanitization that merged all environment variables into a single pool before shredding them. This prevented the use of dynamic variables like $VAR in the configuration and accidentally redacted intentional secrets provided by the user.
This PR:
expandEnvVarsutility inpackages/core/src/utils/envExpansion.tsthat supports$VAR,${VAR}, and%VAR%.createTransportinpackages/core/src/tools/mcp-client.tsto expand variables in theenvconfig using the host process environment.docs/tools/mcp-server.mdto document expansion and clarify security behavior for explicit variables.Related Issues
Fixes unintended credential exposure regression from #17311.
How to Validate
settings.jsonwith an environment variable reference:"env": { "API_KEY": "$MY_SECRET" }.MY_SECRET=1234in the shell environment.API_KEY=1234)."env": { "SECRET_KEY": "my-secret" }.--debug).Pre-Merge Checklist