mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-03-11 14:40:52 -07:00
Revert unintended credentials exposure (#18840)
This commit is contained in:
@@ -739,21 +739,10 @@ 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,13 +128,6 @@ 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,9 +46,6 @@ 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,9 +103,6 @@ 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',
|
|
||||||
],
|
],
|
||||||
);
|
);
|
||||||
|
|
||||||
|
|||||||
@@ -1623,7 +1623,7 @@ describe('mcp-client', () => {
|
|||||||
{
|
{
|
||||||
command: 'test-command',
|
command: 'test-command',
|
||||||
args: ['--foo', 'bar'],
|
args: ['--foo', 'bar'],
|
||||||
env: { GEMINI_CLI_FOO: 'bar' },
|
env: { FOO: 'bar' },
|
||||||
cwd: 'test/cwd',
|
cwd: 'test/cwd',
|
||||||
},
|
},
|
||||||
false,
|
false,
|
||||||
@@ -1634,80 +1634,11 @@ describe('mcp-client', () => {
|
|||||||
command: 'test-command',
|
command: 'test-command',
|
||||||
args: ['--foo', 'bar'],
|
args: ['--foo', 'bar'],
|
||||||
cwd: 'test/cwd',
|
cwd: 'test/cwd',
|
||||||
env: expect.objectContaining({ GEMINI_CLI_FOO: 'bar' }),
|
env: expect.objectContaining({ 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');
|
|
||||||
});
|
|
||||||
|
|
||||||
it('should exclude extension settings with undefined values from environment', async () => {
|
it('should exclude extension settings with undefined values from environment', async () => {
|
||||||
const mockedTransport = vi
|
const mockedTransport = vi
|
||||||
.spyOn(SdkClientStdioLib, 'StdioClientTransport')
|
.spyOn(SdkClientStdioLib, 'StdioClientTransport')
|
||||||
|
|||||||
@@ -34,11 +34,7 @@ import {
|
|||||||
} from '@modelcontextprotocol/sdk/types.js';
|
} from '@modelcontextprotocol/sdk/types.js';
|
||||||
import { ApprovalMode, PolicyDecision } from '../policy/types.js';
|
import { ApprovalMode, PolicyDecision } from '../policy/types.js';
|
||||||
import { parse } from 'shell-quote';
|
import { parse } from 'shell-quote';
|
||||||
import type {
|
import type { Config, MCPServerConfig } from '../config/config.js';
|
||||||
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';
|
||||||
@@ -1902,23 +1898,10 @@ export async function createTransport(
|
|||||||
command: mcpServerConfig.command,
|
command: mcpServerConfig.command,
|
||||||
args: mcpServerConfig.args || [],
|
args: mcpServerConfig.args || [],
|
||||||
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
|
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
|
||||||
env: sanitizeEnvironment(
|
env: {
|
||||||
{
|
...sanitizeEnvironment(process.env, sanitizationConfig),
|
||||||
...process.env,
|
...(mcpServerConfig.env || {}),
|
||||||
...getExtensionEnvironment(mcpServerConfig.extension),
|
} as Record<string, string>,
|
||||||
...(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',
|
||||||
});
|
});
|
||||||
@@ -1993,17 +1976,3 @@ export function isEnabled(
|
|||||||
)
|
)
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
function getExtensionEnvironment(
|
|
||||||
extension?: GeminiCLIExtension,
|
|
||||||
): Record<string, string> {
|
|
||||||
const env: Record<string, string> = {};
|
|
||||||
if (extension?.resolvedSettings) {
|
|
||||||
for (const setting of extension.resolvedSettings) {
|
|
||||||
if (setting.value) {
|
|
||||||
env[setting.envVar] = setting.value;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
return env;
|
|
||||||
}
|
|
||||||
|
|||||||
Reference in New Issue
Block a user