From e9a94748107ac24a05f51b4c5b0c0a8952374285 Mon Sep 17 00:00:00 2001 From: Adib234 <30782825+Adib234@users.noreply.github.com> Date: Wed, 11 Feb 2026 15:06:28 -0500 Subject: [PATCH] Revert unintended credentials exposure (#18840) --- docs/tools/mcp-server.md | 13 +--- packages/cli/src/commands/mcp/add.ts | 7 -- .../services/environmentSanitization.test.ts | 3 - .../src/services/environmentSanitization.ts | 3 - packages/core/src/tools/mcp-client.test.ts | 73 +------------------ packages/core/src/tools/mcp-client.ts | 41 ++--------- 6 files changed, 8 insertions(+), 132 deletions(-) diff --git a/docs/tools/mcp-server.md b/docs/tools/mcp-server.md index eb246fd86f..dd3842759c 100644 --- a/docs/tools/mcp-server.md +++ b/docs/tools/mcp-server.md @@ -739,21 +739,10 @@ The MCP integration tracks several states: cautiously and only for servers you completely control - **Access tokens:** Be security-aware when configuring environment variables containing API keys or tokens -- **Environment variable redaction:** By default, the Gemini CLI redacts - sensitive environment variables (such as `GEMINI_API_KEY`, `GOOGLE_API_KEY`, - and variables matching patterns like `*TOKEN*`, `*SECRET*`, `*PASSWORD*`) when - spawning MCP servers using the `stdio` transport. This prevents unintended - exposure of your credentials to third-party servers. -- **Explicit environment variables:** If you need to pass a specific environment - variable to an MCP server, you should define it explicitly in the `env` - property of the server configuration in `settings.json`. - **Sandbox compatibility:** When using sandboxing, ensure MCP servers are - available within the sandbox environment. + available within the sandbox environment - **Private data:** Using broadly scoped personal access tokens can lead to information leakage between repositories. -- **Untrusted servers:** Be extremely cautious when adding MCP servers from - untrusted or third-party sources. Malicious servers could attempt to - exfiltrate data or perform unauthorized actions through the tools they expose. ### Performance and resource management diff --git a/packages/cli/src/commands/mcp/add.ts b/packages/cli/src/commands/mcp/add.ts index 7d744a1daa..98e6a70879 100644 --- a/packages/cli/src/commands/mcp/add.ts +++ b/packages/cli/src/commands/mcp/add.ts @@ -128,13 +128,6 @@ async function addMcpServer( settings.setValue(settingsScope, 'mcpServers', mcpServers); - if (transport === 'stdio') { - debugLogger.warn( - 'Security Warning: Running MCP servers with stdio transport can expose inherited environment variables. ' + - 'While the Gemini CLI redacts common API keys and secrets by default, you should only run servers from trusted sources.', - ); - } - if (isExistingServer) { debugLogger.log(`MCP server "${name}" updated in ${scope} settings.`); } else { diff --git a/packages/core/src/services/environmentSanitization.test.ts b/packages/core/src/services/environmentSanitization.test.ts index 97f7e575ca..cc26d7547d 100644 --- a/packages/core/src/services/environmentSanitization.test.ts +++ b/packages/core/src/services/environmentSanitization.test.ts @@ -46,9 +46,6 @@ describe('sanitizeEnvironment', () => { CLIENT_ID: 'sensitive-id', DB_URI: 'sensitive-uri', DATABASE_URL: 'sensitive-url', - GEMINI_API_KEY: 'sensitive-gemini-key', - GOOGLE_API_KEY: 'sensitive-google-key', - GOOGLE_APPLICATION_CREDENTIALS: '/path/to/creds.json', SAFE_VAR: 'is-safe', }; const sanitized = sanitizeEnvironment(env, EMPTY_OPTIONS); diff --git a/packages/core/src/services/environmentSanitization.ts b/packages/core/src/services/environmentSanitization.ts index b30b229079..dc9c92484d 100644 --- a/packages/core/src/services/environmentSanitization.ts +++ b/packages/core/src/services/environmentSanitization.ts @@ -103,9 +103,6 @@ export const NEVER_ALLOWED_ENVIRONMENT_VARIABLES: ReadonlySet = new Set( 'GOOGLE_CLOUD_PROJECT', 'GOOGLE_CLOUD_ACCOUNT', 'FIREBASE_PROJECT_ID', - 'GEMINI_API_KEY', - 'GOOGLE_API_KEY', - 'GOOGLE_APPLICATION_CREDENTIALS', ], ); diff --git a/packages/core/src/tools/mcp-client.test.ts b/packages/core/src/tools/mcp-client.test.ts index 77dec9d657..39165bde45 100644 --- a/packages/core/src/tools/mcp-client.test.ts +++ b/packages/core/src/tools/mcp-client.test.ts @@ -1623,7 +1623,7 @@ describe('mcp-client', () => { { command: 'test-command', args: ['--foo', 'bar'], - env: { GEMINI_CLI_FOO: 'bar' }, + env: { FOO: 'bar' }, cwd: 'test/cwd', }, false, @@ -1634,80 +1634,11 @@ describe('mcp-client', () => { command: 'test-command', args: ['--foo', 'bar'], cwd: 'test/cwd', - env: expect.objectContaining({ GEMINI_CLI_FOO: 'bar' }), + env: expect.objectContaining({ FOO: 'bar' }), stderr: 'pipe', }); }); - it('should redact sensitive environment variables for command transport', async () => { - const mockedTransport = vi - .spyOn(SdkClientStdioLib, 'StdioClientTransport') - .mockReturnValue({} as SdkClientStdioLib.StdioClientTransport); - - const originalEnv = process.env; - process.env = { - ...originalEnv, - GEMINI_API_KEY: 'sensitive-key', - GEMINI_CLI_SAFE_VAR: 'safe-value', - }; - // Ensure strict sanitization is not triggered for this test - delete process.env['GITHUB_SHA']; - delete process.env['SURFACE']; - - try { - await createTransport( - 'test-server', - { - command: 'test-command', - }, - false, - EMPTY_CONFIG, - ); - - const callArgs = mockedTransport.mock.calls[0][0]; - expect(callArgs.env).toBeDefined(); - expect(callArgs.env!['GEMINI_CLI_SAFE_VAR']).toBe('safe-value'); - expect(callArgs.env!['GEMINI_API_KEY']).toBeUndefined(); - } finally { - process.env = originalEnv; - } - }); - - it('should include extension settings 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: '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'); - }); - it('should exclude extension settings with undefined values from environment', async () => { const mockedTransport = vi .spyOn(SdkClientStdioLib, 'StdioClientTransport') diff --git a/packages/core/src/tools/mcp-client.ts b/packages/core/src/tools/mcp-client.ts index c069f7a211..2588d54dba 100644 --- a/packages/core/src/tools/mcp-client.ts +++ b/packages/core/src/tools/mcp-client.ts @@ -34,11 +34,7 @@ import { } from '@modelcontextprotocol/sdk/types.js'; import { ApprovalMode, PolicyDecision } from '../policy/types.js'; import { parse } from 'shell-quote'; -import type { - Config, - GeminiCLIExtension, - MCPServerConfig, -} from '../config/config.js'; +import type { Config, MCPServerConfig } from '../config/config.js'; import { AuthProviderType } from '../config/config.js'; import { GoogleCredentialProvider } from '../mcp/google-auth-provider.js'; import { ServiceAccountImpersonationProvider } from '../mcp/sa-impersonation-provider.js'; @@ -1902,23 +1898,10 @@ export async function createTransport( command: mcpServerConfig.command, args: mcpServerConfig.args || [], // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion - env: sanitizeEnvironment( - { - ...process.env, - ...getExtensionEnvironment(mcpServerConfig.extension), - ...(mcpServerConfig.env || {}), - }, - { - ...sanitizationConfig, - allowedEnvironmentVariables: [ - ...(sanitizationConfig.allowedEnvironmentVariables ?? []), - ...(mcpServerConfig.extension?.resolvedSettings?.map( - (s) => s.envVar, - ) ?? []), - ], - enableEnvironmentVariableRedaction: true, - }, - ) as Record, + env: { + ...sanitizeEnvironment(process.env, sanitizationConfig), + ...(mcpServerConfig.env || {}), + } as Record, cwd: mcpServerConfig.cwd, stderr: 'pipe', }); @@ -1993,17 +1976,3 @@ export function isEnabled( ) ); } - -function getExtensionEnvironment( - extension?: GeminiCLIExtension, -): Record { - const env: Record = {}; - if (extension?.resolvedSettings) { - for (const setting of extension.resolvedSettings) { - if (setting.value) { - env[setting.envVar] = setting.value; - } - } - } - return env; -}