mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-03-11 22:51:00 -07:00
Fix extension MCP server env var loading (#20374)
This commit is contained in:
@@ -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')
|
||||||
|
|||||||
@@ -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,
|
||||||
|
|||||||
Reference in New Issue
Block a user