Fix extension MCP server env var loading (#20374)

This commit is contained in:
christine betts
2026-02-27 09:49:10 -05:00
committed by GitHub
parent 522e95439c
commit 58df1c6237
2 changed files with 127 additions and 6 deletions

View File

@@ -1792,6 +1792,80 @@ describe('mcp-client', () => {
expect(callArgs.env!['GEMINI_CLI_EXT_VAR']).toBeUndefined(); expect(callArgs.env!['GEMINI_CLI_EXT_VAR']).toBeUndefined();
}); });
it('should include extension settings with defined values in environment', async () => {
const mockedTransport = vi
.spyOn(SdkClientStdioLib, 'StdioClientTransport')
.mockReturnValue({} as SdkClientStdioLib.StdioClientTransport);
await createTransport(
'test-server',
{
command: 'test-command',
extension: {
name: 'test-ext',
resolvedSettings: [
{
envVar: 'GEMINI_CLI_EXT_VAR',
value: 'defined-value',
sensitive: false,
name: 'ext-setting',
},
],
version: '',
isActive: false,
path: '',
contextFiles: [],
id: '',
},
},
false,
EMPTY_CONFIG,
);
const callArgs = mockedTransport.mock.calls[0][0];
expect(callArgs.env).toBeDefined();
expect(callArgs.env!['GEMINI_CLI_EXT_VAR']).toBe('defined-value');
});
it('should resolve environment variables in mcpServerConfig.env using extension settings', async () => {
const mockedTransport = vi
.spyOn(SdkClientStdioLib, 'StdioClientTransport')
.mockReturnValue({} as SdkClientStdioLib.StdioClientTransport);
await createTransport(
'test-server',
{
command: 'test-command',
env: {
RESOLVED_VAR: '$GEMINI_CLI_EXT_VAR',
},
extension: {
name: 'test-ext',
resolvedSettings: [
{
envVar: 'GEMINI_CLI_EXT_VAR',
value: 'ext-value',
sensitive: false,
name: 'ext-setting',
},
],
version: '',
isActive: false,
path: '',
contextFiles: [],
id: '',
},
},
false,
EMPTY_CONFIG,
);
const callArgs = mockedTransport.mock.calls[0][0];
expect(callArgs.env).toBeDefined();
expect(callArgs.env!['GEMINI_CLI_EXT_VAR']).toBe('ext-value');
expect(callArgs.env!['RESOLVED_VAR']).toBe('ext-value');
});
it('should expand environment variables in mcpServerConfig.env and not redact them', async () => { it('should expand environment variables in mcpServerConfig.env and not redact them', async () => {
const mockedTransport = vi const mockedTransport = vi
.spyOn(SdkClientStdioLib, 'StdioClientTransport') .spyOn(SdkClientStdioLib, 'StdioClientTransport')

View File

@@ -34,7 +34,11 @@ import {
ProgressNotificationSchema, ProgressNotificationSchema,
} from '@modelcontextprotocol/sdk/types.js'; } from '@modelcontextprotocol/sdk/types.js';
import { parse } from 'shell-quote'; import { parse } from 'shell-quote';
import type { Config, MCPServerConfig } from '../config/config.js'; import type {
Config,
MCPServerConfig,
GeminiCLIExtension,
} from '../config/config.js';
import { AuthProviderType } from '../config/config.js'; import { AuthProviderType } from '../config/config.js';
import { GoogleCredentialProvider } from '../mcp/google-auth-provider.js'; import { GoogleCredentialProvider } from '../mcp/google-auth-provider.js';
import { ServiceAccountImpersonationProvider } from '../mcp/sa-impersonation-provider.js'; import { ServiceAccountImpersonationProvider } from '../mcp/sa-impersonation-provider.js';
@@ -778,15 +782,25 @@ async function handleAutomaticOAuth(
* *
* @param mcpServerConfig The MCP server configuration * @param mcpServerConfig The MCP server configuration
* @param headers Additional headers * @param headers Additional headers
* @param sanitizationConfig Configuration for environment sanitization
*/ */
function createTransportRequestInit( function createTransportRequestInit(
mcpServerConfig: MCPServerConfig, mcpServerConfig: MCPServerConfig,
headers: Record<string, string>, headers: Record<string, string>,
sanitizationConfig: EnvironmentSanitizationConfig,
): RequestInit { ): RequestInit {
const extensionEnv = getExtensionEnvironment(mcpServerConfig.extension);
const expansionEnv = { ...process.env, ...extensionEnv };
const sanitizedEnv = sanitizeEnvironment(expansionEnv, {
...sanitizationConfig,
enableEnvironmentVariableRedaction: true,
});
const expandedHeaders: Record<string, string> = {}; const expandedHeaders: Record<string, string> = {};
if (mcpServerConfig.headers) { if (mcpServerConfig.headers) {
for (const [key, value] of Object.entries(mcpServerConfig.headers)) { for (const [key, value] of Object.entries(mcpServerConfig.headers)) {
expandedHeaders[key] = expandEnvVars(value, process.env); expandedHeaders[key] = expandEnvVars(value, sanitizedEnv);
} }
} }
@@ -826,12 +840,14 @@ function createAuthProvider(
* @param mcpServerName The name of the MCP server * @param mcpServerName The name of the MCP server
* @param mcpServerConfig The MCP server configuration * @param mcpServerConfig The MCP server configuration
* @param accessToken The OAuth access token * @param accessToken The OAuth access token
* @param sanitizationConfig Configuration for environment sanitization
* @returns The transport with OAuth token, or null if creation fails * @returns The transport with OAuth token, or null if creation fails
*/ */
async function createTransportWithOAuth( async function createTransportWithOAuth(
mcpServerName: string, mcpServerName: string,
mcpServerConfig: MCPServerConfig, mcpServerConfig: MCPServerConfig,
accessToken: string, accessToken: string,
sanitizationConfig: EnvironmentSanitizationConfig,
): Promise<StreamableHTTPClientTransport | SSEClientTransport | null> { ): Promise<StreamableHTTPClientTransport | SSEClientTransport | null> {
try { try {
const headers: Record<string, string> = { const headers: Record<string, string> = {
@@ -840,7 +856,11 @@ async function createTransportWithOAuth(
const transportOptions: const transportOptions:
| StreamableHTTPClientTransportOptions | StreamableHTTPClientTransportOptions
| SSEClientTransportOptions = { | SSEClientTransportOptions = {
requestInit: createTransportRequestInit(mcpServerConfig, headers), requestInit: createTransportRequestInit(
mcpServerConfig,
headers,
sanitizationConfig,
),
}; };
return createUrlTransport(mcpServerName, mcpServerConfig, transportOptions); return createUrlTransport(mcpServerName, mcpServerConfig, transportOptions);
@@ -1435,6 +1455,7 @@ async function showAuthRequiredMessage(serverName: string): Promise<never> {
* @param config The MCP server configuration * @param config The MCP server configuration
* @param accessToken The OAuth access token to use * @param accessToken The OAuth access token to use
* @param httpReturned404 Whether the HTTP transport returned 404 (indicating SSE-only server) * @param httpReturned404 Whether the HTTP transport returned 404 (indicating SSE-only server)
* @param sanitizationConfig Configuration for environment sanitization
*/ */
async function retryWithOAuth( async function retryWithOAuth(
client: Client, client: Client,
@@ -1442,6 +1463,7 @@ async function retryWithOAuth(
config: MCPServerConfig, config: MCPServerConfig,
accessToken: string, accessToken: string,
httpReturned404: boolean, httpReturned404: boolean,
sanitizationConfig: EnvironmentSanitizationConfig,
): Promise<void> { ): Promise<void> {
if (httpReturned404) { if (httpReturned404) {
// HTTP returned 404, only try SSE // HTTP returned 404, only try SSE
@@ -1462,6 +1484,7 @@ async function retryWithOAuth(
serverName, serverName,
config, config,
accessToken, accessToken,
sanitizationConfig,
); );
if (!httpTransport) { if (!httpTransport) {
throw new Error( throw new Error(
@@ -1741,6 +1764,7 @@ export async function connectToMcpServer(
mcpServerConfig, mcpServerConfig,
accessToken, accessToken,
httpReturned404, httpReturned404,
sanitizationConfig,
); );
return mcpClient; return mcpClient;
} else { } else {
@@ -1813,6 +1837,7 @@ export async function connectToMcpServer(
mcpServerName, mcpServerName,
mcpServerConfig, mcpServerConfig,
accessToken, accessToken,
sanitizationConfig,
); );
if (!oauthTransport) { if (!oauthTransport) {
throw new Error( throw new Error(
@@ -1960,7 +1985,11 @@ export async function createTransport(
const transportOptions: const transportOptions:
| StreamableHTTPClientTransportOptions | StreamableHTTPClientTransportOptions
| SSEClientTransportOptions = { | SSEClientTransportOptions = {
requestInit: createTransportRequestInit(mcpServerConfig, headers), requestInit: createTransportRequestInit(
mcpServerConfig,
headers,
sanitizationConfig,
),
authProvider, authProvider,
}; };
@@ -1968,8 +1997,11 @@ export async function createTransport(
} }
if (mcpServerConfig.command) { if (mcpServerConfig.command) {
const extensionEnv = getExtensionEnvironment(mcpServerConfig.extension);
const expansionEnv = { ...process.env, ...extensionEnv };
// 1. Sanitize the base process environment to prevent unintended leaks of system-wide secrets. // 1. Sanitize the base process environment to prevent unintended leaks of system-wide secrets.
const sanitizedEnv = sanitizeEnvironment(process.env, { const sanitizedEnv = sanitizeEnvironment(expansionEnv, {
...sanitizationConfig, ...sanitizationConfig,
enableEnvironmentVariableRedaction: true, enableEnvironmentVariableRedaction: true,
}); });
@@ -1977,6 +2009,7 @@ export async function createTransport(
const finalEnv: Record<string, string> = { const finalEnv: Record<string, string> = {
[GEMINI_CLI_IDENTIFICATION_ENV_VAR]: [GEMINI_CLI_IDENTIFICATION_ENV_VAR]:
GEMINI_CLI_IDENTIFICATION_ENV_VAR_VALUE, GEMINI_CLI_IDENTIFICATION_ENV_VAR_VALUE,
...extensionEnv,
}; };
for (const [key, value] of Object.entries(sanitizedEnv)) { for (const [key, value] of Object.entries(sanitizedEnv)) {
if (value !== undefined) { if (value !== undefined) {
@@ -1987,7 +2020,7 @@ export async function createTransport(
// Expand and merge explicit environment variables from the MCP configuration. // Expand and merge explicit environment variables from the MCP configuration.
if (mcpServerConfig.env) { if (mcpServerConfig.env) {
for (const [key, value] of Object.entries(mcpServerConfig.env)) { for (const [key, value] of Object.entries(mcpServerConfig.env)) {
finalEnv[key] = expandEnvVars(value, process.env); finalEnv[key] = expandEnvVars(value, expansionEnv);
} }
} }
@@ -2045,6 +2078,20 @@ interface NamedTool {
name?: string; name?: string;
} }
function getExtensionEnvironment(
extension?: GeminiCLIExtension,
): Record<string, string> {
const env: Record<string, string> = {};
if (extension?.resolvedSettings) {
for (const setting of extension.resolvedSettings) {
if (setting.value !== undefined) {
env[setting.envVar] = setting.value;
}
}
}
return env;
}
/** Visible for testing */ /** Visible for testing */
export function isEnabled( export function isEnabled(
funcDecl: NamedTool, funcDecl: NamedTool,