mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-03-25 21:41:12 -07:00
fix(core): allow environment variable expansion and explicit overrides for MCP servers (#18837)
This commit is contained in:
Gal Zahavi
@@ -1704,6 +1704,40 @@ describe('mcp-client', () => {
|
||||
expect(callArgs.env!['GEMINI_CLI_EXT_VAR']).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should expand environment variables in mcpServerConfig.env and not redact them', async () => {
|
||||
const mockedTransport = vi
|
||||
.spyOn(SdkClientStdioLib, 'StdioClientTransport')
|
||||
.mockReturnValue({} as SdkClientStdioLib.StdioClientTransport);
|
||||
|
||||
const originalEnv = process.env;
|
||||
process.env = {
|
||||
...originalEnv,
|
||||
GEMINI_TEST_VAR: 'expanded-value',
|
||||
};
|
||||
|
||||
try {
|
||||
await createTransport(
|
||||
'test-server',
|
||||
{
|
||||
command: 'test-command',
|
||||
env: {
|
||||
TEST_EXPANDED: 'Value is $GEMINI_TEST_VAR',
|
||||
SECRET_KEY: 'intentional-secret-123',
|
||||
},
|
||||
},
|
||||
false,
|
||||
EMPTY_CONFIG,
|
||||
);
|
||||
|
||||
const callArgs = mockedTransport.mock.calls[0][0];
|
||||
expect(callArgs.env).toBeDefined();
|
||||
expect(callArgs.env!['TEST_EXPANDED']).toBe('Value is expanded-value');
|
||||
expect(callArgs.env!['SECRET_KEY']).toBe('intentional-secret-123');
|
||||
} finally {
|
||||
process.env = originalEnv;
|
||||
}
|
||||
});
|
||||
|
||||
describe('useGoogleCredentialProvider', () => {
|
||||
beforeEach(() => {
|
||||
// Mock GoogleAuth client
|
||||
|
||||
@@ -70,6 +70,7 @@ import {
|
||||
sanitizeEnvironment,
|
||||
type EnvironmentSanitizationConfig,
|
||||
} from '../services/environmentSanitization.js';
|
||||
import { expandEnvVars } from '../utils/envExpansion.js';
|
||||
import {
|
||||
GEMINI_CLI_IDENTIFICATION_ENV_VAR,
|
||||
GEMINI_CLI_IDENTIFICATION_ENV_VAR_VALUE,
|
||||
@@ -783,9 +784,16 @@ function createTransportRequestInit(
|
||||
mcpServerConfig: MCPServerConfig,
|
||||
headers: Record<string, string>,
|
||||
): RequestInit {
|
||||
const expandedHeaders: Record<string, string> = {};
|
||||
if (mcpServerConfig.headers) {
|
||||
for (const [key, value] of Object.entries(mcpServerConfig.headers)) {
|
||||
expandedHeaders[key] = expandEnvVars(value, process.env);
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
headers: {
|
||||
...mcpServerConfig.headers,
|
||||
...expandedHeaders,
|
||||
...headers,
|
||||
},
|
||||
};
|
||||
@@ -1970,15 +1978,33 @@ export async function createTransport(
|
||||
}
|
||||
|
||||
if (mcpServerConfig.command) {
|
||||
// 1. Sanitize the base process environment to prevent unintended leaks of system-wide secrets.
|
||||
const sanitizedEnv = sanitizeEnvironment(process.env, {
|
||||
...sanitizationConfig,
|
||||
enableEnvironmentVariableRedaction: true,
|
||||
});
|
||||
|
||||
const finalEnv: Record<string, string> = {
|
||||
[GEMINI_CLI_IDENTIFICATION_ENV_VAR]:
|
||||
GEMINI_CLI_IDENTIFICATION_ENV_VAR_VALUE,
|
||||
};
|
||||
for (const [key, value] of Object.entries(sanitizedEnv)) {
|
||||
if (value !== undefined) {
|
||||
finalEnv[key] = value;
|
||||
}
|
||||
}
|
||||
|
||||
// Expand and merge explicit environment variables from the MCP configuration.
|
||||
if (mcpServerConfig.env) {
|
||||
for (const [key, value] of Object.entries(mcpServerConfig.env)) {
|
||||
finalEnv[key] = expandEnvVars(value, process.env);
|
||||
}
|
||||
}
|
||||
|
||||
let transport: Transport = new StdioClientTransport({
|
||||
command: mcpServerConfig.command,
|
||||
args: mcpServerConfig.args || [],
|
||||
env: {
|
||||
...sanitizeEnvironment(process.env, sanitizationConfig),
|
||||
...(mcpServerConfig.env || {}),
|
||||
[GEMINI_CLI_IDENTIFICATION_ENV_VAR]:
|
||||
GEMINI_CLI_IDENTIFICATION_ENV_VAR_VALUE,
|
||||
} as Record<string, string>,
|
||||
env: finalEnv,
|
||||
cwd: mcpServerConfig.cwd,
|
||||
stderr: 'pipe',
|
||||
});
|
||||
|
||||
117
packages/core/src/utils/envExpansion.test.ts
Normal file
117
packages/core/src/utils/envExpansion.test.ts
Normal file
@@ -0,0 +1,117 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2026 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { expandEnvVars } from './envExpansion.js';
|
||||
|
||||
describe('expandEnvVars', () => {
|
||||
const defaultEnv = {
|
||||
USER: 'morty',
|
||||
HOME: '/home/morty',
|
||||
TEMP: 'C:\\Temp',
|
||||
EMPTY: '',
|
||||
};
|
||||
|
||||
describe('POSIX behavior (non-Windows)', () => {
|
||||
beforeEach(() => {
|
||||
vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin');
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it.each([
|
||||
['$VAR (POSIX)', 'Hello $USER', defaultEnv, 'Hello morty'],
|
||||
[
|
||||
'${VAR} (POSIX)',
|
||||
'Welcome to ${HOME}',
|
||||
defaultEnv,
|
||||
'Welcome to /home/morty',
|
||||
],
|
||||
[
|
||||
'should NOT expand %VAR% on non-Windows',
|
||||
'Data in %TEMP%',
|
||||
defaultEnv,
|
||||
'Data in %TEMP%',
|
||||
],
|
||||
[
|
||||
'mixed formats (only POSIX expanded)',
|
||||
'$USER lives in ${HOME} on %TEMP%',
|
||||
defaultEnv,
|
||||
'morty lives in /home/morty on %TEMP%',
|
||||
],
|
||||
[
|
||||
'missing variables (POSIX only)',
|
||||
'Missing $UNDEFINED and ${NONE} and %MISSING%',
|
||||
defaultEnv,
|
||||
'Missing and and %MISSING%',
|
||||
],
|
||||
[
|
||||
'empty or undefined values',
|
||||
'Value is "$EMPTY"',
|
||||
defaultEnv,
|
||||
'Value is ""',
|
||||
],
|
||||
[
|
||||
'original string if no variables',
|
||||
'No vars here',
|
||||
defaultEnv,
|
||||
'No vars here',
|
||||
],
|
||||
['literal values like "1234"', '1234', defaultEnv, '1234'],
|
||||
['empty input string', '', defaultEnv, ''],
|
||||
[
|
||||
'complex paths',
|
||||
'${HOME}/bin:$PATH',
|
||||
{ ...defaultEnv, PATH: '/usr/bin' },
|
||||
'/home/morty/bin:/usr/bin',
|
||||
],
|
||||
])('should handle %s', (_, input, env, expected) => {
|
||||
expect(expandEnvVars(input, env)).toBe(expected);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Windows behavior', () => {
|
||||
beforeEach(() => {
|
||||
vi.spyOn(process, 'platform', 'get').mockReturnValue('win32');
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it.each([
|
||||
['$VAR (POSIX)', 'Hello $USER', defaultEnv, 'Hello morty'],
|
||||
[
|
||||
'${VAR} (POSIX)',
|
||||
'Welcome to ${HOME}',
|
||||
defaultEnv,
|
||||
'Welcome to /home/morty',
|
||||
],
|
||||
[
|
||||
'should expand %VAR% on Windows',
|
||||
'Data in %TEMP%',
|
||||
defaultEnv,
|
||||
'Data in C:\\Temp',
|
||||
],
|
||||
[
|
||||
'mixed formats (both expanded)',
|
||||
'$USER lives in ${HOME} on %TEMP%',
|
||||
defaultEnv,
|
||||
'morty lives in /home/morty on C:\\Temp',
|
||||
],
|
||||
[
|
||||
'missing variables (all expanded to empty)',
|
||||
'Missing $UNDEFINED and ${NONE} and %MISSING%',
|
||||
defaultEnv,
|
||||
'Missing and and ',
|
||||
],
|
||||
])('should handle %s', (_, input, env, expected) => {
|
||||
expect(expandEnvVars(input, env)).toBe(expected);
|
||||
});
|
||||
});
|
||||
});
|
||||
54
packages/core/src/utils/envExpansion.ts
Normal file
54
packages/core/src/utils/envExpansion.ts
Normal file
@@ -0,0 +1,54 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2026 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { expand } from 'dotenv-expand';
|
||||
|
||||
/**
|
||||
* Expands environment variables in a string using the provided environment record.
|
||||
* Uses the standard `dotenv-expand` library to handle expansion consistently with
|
||||
* other tools.
|
||||
*
|
||||
* Supports POSIX/Bash syntax ($VAR, ${VAR}).
|
||||
* Note: Windows syntax (%VAR%) is not natively supported by dotenv-expand.
|
||||
*
|
||||
* @param str - The string containing environment variable placeholders.
|
||||
* @param env - A record of environment variable names and their values.
|
||||
* @returns The string with environment variables expanded. Missing variables resolve to an empty string.
|
||||
*/
|
||||
export function expandEnvVars(
|
||||
str: string,
|
||||
env: Record<string, string | undefined>,
|
||||
): string {
|
||||
if (!str) return str;
|
||||
|
||||
// 1. Pre-process Windows-style variables (%VAR%) since dotenv-expand only handles POSIX ($VAR).
|
||||
// We only do this on Windows to limit the blast radius and avoid conflicts with other
|
||||
// systems where % might be a literal character (e.g. in URLs or shell commands).
|
||||
const isWindows = process.platform === 'win32';
|
||||
const processedStr = isWindows
|
||||
? str.replace(/%(\w+)%/g, (_, name) => env[name] ?? '')
|
||||
: str;
|
||||
|
||||
// 2. Use dotenv-expand for POSIX/Bash syntax ($VAR, ${VAR}).
|
||||
// dotenv-expand is designed to process an object of key-value pairs (like a .env file).
|
||||
// To expand a single string, we wrap it in an object with a temporary key.
|
||||
const dummyKey = '__GCLI_EXPAND_TARGET__';
|
||||
|
||||
// Filter out undefined values to satisfy the Record<string, string> requirement safely
|
||||
const processEnv: Record<string, string> = {};
|
||||
for (const [key, value] of Object.entries(env)) {
|
||||
if (value !== undefined) {
|
||||
processEnv[key] = value;
|
||||
}
|
||||
}
|
||||
|
||||
const result = expand({
|
||||
parsed: { [dummyKey]: processedStr },
|
||||
processEnv,
|
||||
});
|
||||
|
||||
return result.parsed?.[dummyKey] ?? '';
|
||||
}
|
||||
Reference in New Issue
Block a user