mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-02-01 22:48:03 +00:00
Fix unintended credential exposure to MCP Servers (#17311)
Co-authored-by: Tommaso Sciortino <sciortino@gmail.com>
This commit is contained in:
@@ -739,10 +739,21 @@ The MCP integration tracks several states:
|
|||||||
cautiously and only for servers you completely control
|
cautiously and only for servers you completely control
|
||||||
- **Access tokens:** Be security-aware when configuring environment variables
|
- **Access tokens:** Be security-aware when configuring environment variables
|
||||||
containing API keys or tokens
|
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
|
- **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
|
- **Private data:** Using broadly scoped personal access tokens can lead to
|
||||||
information leakage between repositories
|
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
|
### Performance and resource management
|
||||||
|
|
||||||
|
|||||||
@@ -128,6 +128,13 @@ async function addMcpServer(
|
|||||||
|
|
||||||
settings.setValue(settingsScope, 'mcpServers', mcpServers);
|
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) {
|
if (isExistingServer) {
|
||||||
debugLogger.log(`MCP server "${name}" updated in ${scope} settings.`);
|
debugLogger.log(`MCP server "${name}" updated in ${scope} settings.`);
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
@@ -46,6 +46,9 @@ describe('sanitizeEnvironment', () => {
|
|||||||
CLIENT_ID: 'sensitive-id',
|
CLIENT_ID: 'sensitive-id',
|
||||||
DB_URI: 'sensitive-uri',
|
DB_URI: 'sensitive-uri',
|
||||||
DATABASE_URL: 'sensitive-url',
|
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',
|
SAFE_VAR: 'is-safe',
|
||||||
};
|
};
|
||||||
const sanitized = sanitizeEnvironment(env, EMPTY_OPTIONS);
|
const sanitized = sanitizeEnvironment(env, EMPTY_OPTIONS);
|
||||||
|
|||||||
@@ -103,6 +103,9 @@ export const NEVER_ALLOWED_ENVIRONMENT_VARIABLES: ReadonlySet<string> = new Set(
|
|||||||
'GOOGLE_CLOUD_PROJECT',
|
'GOOGLE_CLOUD_PROJECT',
|
||||||
'GOOGLE_CLOUD_ACCOUNT',
|
'GOOGLE_CLOUD_ACCOUNT',
|
||||||
'FIREBASE_PROJECT_ID',
|
'FIREBASE_PROJECT_ID',
|
||||||
|
'GEMINI_API_KEY',
|
||||||
|
'GOOGLE_API_KEY',
|
||||||
|
'GOOGLE_APPLICATION_CREDENTIALS',
|
||||||
],
|
],
|
||||||
);
|
);
|
||||||
|
|
||||||
|
|||||||
@@ -1471,7 +1471,7 @@ describe('mcp-client', () => {
|
|||||||
{
|
{
|
||||||
command: 'test-command',
|
command: 'test-command',
|
||||||
args: ['--foo', 'bar'],
|
args: ['--foo', 'bar'],
|
||||||
env: { FOO: 'bar' },
|
env: { GEMINI_CLI_FOO: 'bar' },
|
||||||
cwd: 'test/cwd',
|
cwd: 'test/cwd',
|
||||||
},
|
},
|
||||||
false,
|
false,
|
||||||
@@ -1482,11 +1482,80 @@ describe('mcp-client', () => {
|
|||||||
command: 'test-command',
|
command: 'test-command',
|
||||||
args: ['--foo', 'bar'],
|
args: ['--foo', 'bar'],
|
||||||
cwd: 'test/cwd',
|
cwd: 'test/cwd',
|
||||||
env: expect.objectContaining({ FOO: 'bar' }),
|
env: expect.objectContaining({ GEMINI_CLI_FOO: 'bar' }),
|
||||||
stderr: 'pipe',
|
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');
|
||||||
|
});
|
||||||
|
|
||||||
describe('useGoogleCredentialProvider', () => {
|
describe('useGoogleCredentialProvider', () => {
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
// Mock GoogleAuth client
|
// Mock GoogleAuth client
|
||||||
|
|||||||
@@ -33,7 +33,11 @@ import {
|
|||||||
type Tool as McpTool,
|
type Tool as McpTool,
|
||||||
} 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,
|
||||||
|
GeminiCLIExtension,
|
||||||
|
MCPServerConfig,
|
||||||
|
} 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';
|
||||||
@@ -1870,10 +1874,23 @@ export async function createTransport(
|
|||||||
const transport = new StdioClientTransport({
|
const transport = new StdioClientTransport({
|
||||||
command: mcpServerConfig.command,
|
command: mcpServerConfig.command,
|
||||||
args: mcpServerConfig.args || [],
|
args: mcpServerConfig.args || [],
|
||||||
env: {
|
env: sanitizeEnvironment(
|
||||||
...sanitizeEnvironment(process.env, sanitizationConfig),
|
{
|
||||||
...(mcpServerConfig.env || {}),
|
...process.env,
|
||||||
} as Record<string, string>,
|
...getExtensionEnvironment(mcpServerConfig.extension),
|
||||||
|
...(mcpServerConfig.env || {}),
|
||||||
|
},
|
||||||
|
{
|
||||||
|
...sanitizationConfig,
|
||||||
|
allowedEnvironmentVariables: [
|
||||||
|
...(sanitizationConfig.allowedEnvironmentVariables ?? []),
|
||||||
|
...(mcpServerConfig.extension?.resolvedSettings?.map(
|
||||||
|
(s) => s.envVar,
|
||||||
|
) ?? []),
|
||||||
|
],
|
||||||
|
enableEnvironmentVariableRedaction: true,
|
||||||
|
},
|
||||||
|
) as Record<string, string>,
|
||||||
cwd: mcpServerConfig.cwd,
|
cwd: mcpServerConfig.cwd,
|
||||||
stderr: 'pipe',
|
stderr: 'pipe',
|
||||||
});
|
});
|
||||||
@@ -1924,3 +1941,15 @@ export function isEnabled(
|
|||||||
)
|
)
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function getExtensionEnvironment(
|
||||||
|
extension?: GeminiCLIExtension,
|
||||||
|
): Record<string, string> {
|
||||||
|
const env: Record<string, string> = {};
|
||||||
|
if (extension?.resolvedSettings) {
|
||||||
|
for (const setting of extension.resolvedSettings) {
|
||||||
|
env[setting.envVar] = setting.value;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return env;
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user