diff --git a/docs/cli/settings.md b/docs/cli/settings.md index efecff135c..ffe0e1e72d 100644 --- a/docs/cli/settings.md +++ b/docs/cli/settings.md @@ -98,11 +98,13 @@ they appear in the UI. ### Security -| UI Label | Setting | Description | Default | -| -------------------------- | ------------------------------ | -------------------------------------------------- | ------- | -| Disable YOLO Mode | `security.disableYoloMode` | Disable YOLO mode, even if enabled by a flag. | `false` | -| Blocks extensions from Git | `security.blockGitExtensions` | Blocks installing and loading extensions from Git. | `false` | -| Folder Trust | `security.folderTrust.enabled` | Setting to track whether Folder trust is enabled. | `false` | +| UI Label | Setting | Description | Default | +| ----------------------------- | ----------------------------------------------- | --------------------------------------------------------- | ------- | +| Disable YOLO Mode | `security.disableYoloMode` | Disable YOLO mode, even if enabled by a flag. | `false` | +| Blocks extensions from Git | `security.blockGitExtensions` | Blocks installing and loading extensions from Git. | `false` | +| Folder Trust | `security.folderTrust.enabled` | Setting to track whether Folder trust is enabled. | `false` | +| Allowed Environment Variables | `security.environmentVariableRedaction.allowed` | Environment variables to always allow (bypass redaction). | `[]` | +| Blocked Environment Variables | `security.environmentVariableRedaction.blocked` | Environment variables to always redact. | `[]` | ### Experimental diff --git a/docs/get-started/configuration.md b/docs/get-started/configuration.md index 8040c34186..a6e0d68f34 100644 --- a/docs/get-started/configuration.md +++ b/docs/get-started/configuration.md @@ -746,6 +746,22 @@ their corresponding top-level category object in your `settings.json` file. - **Default:** `false` - **Requires restart:** Yes +- **`security.environmentVariableRedaction.allowed`** (array): + - **Description:** Environment variables to always allow (bypass redaction). + - **Default:** `[]` + - **Requires restart:** Yes + +- **`security.environmentVariableRedaction.blocked`** (array): + - **Description:** Environment variables to always redact. + - **Default:** `[]` + - **Requires restart:** Yes + +- **`security.environmentVariableRedaction.enabled`** (boolean): + - **Description:** Enable redaction of environment variables that may contain + secrets. + - **Default:** `false` + - **Requires restart:** Yes + - **`security.auth.selectedType`** (string): - **Description:** The currently selected authentication type. - **Default:** `undefined` @@ -1171,6 +1187,52 @@ the `advanced.excludedEnvVars` setting in your `settings.json` file. - Specifies the endpoint for the code assist server. - This is useful for development and testing. +### Environment variable redaction + +To prevent accidental leakage of sensitive information, Gemini CLI automatically +redacts potential secrets from environment variables when executing tools (such +as shell commands). This "best effort" redaction applies to variables inherited +from the system or loaded from `.env` files. + +**Default Redaction Rules:** + +- **By Name:** Variables are redacted if their names contain sensitive terms + like `TOKEN`, `SECRET`, `PASSWORD`, `KEY`, `AUTH`, `CREDENTIAL`, `PRIVATE`, or + `CERT`. +- **By Value:** Variables are redacted if their values match known secret + patterns, such as: + - Private keys (RSA, OpenSSH, PGP, etc.) + - Certificates + - URLs containing credentials + - API keys and tokens (GitHub, Google, AWS, Stripe, Slack, etc.) +- **Specific Blocklist:** Certain variables like `CLIENT_ID`, `DB_URI`, + `DATABASE_URL`, and `CONNECTION_STRING` are always redacted by default. + +**Allowlist (Never Redacted):** + +- Common system variables (e.g., `PATH`, `HOME`, `USER`, `SHELL`, `TERM`, + `LANG`). +- Variables starting with `GEMINI_CLI_`. +- GitHub Action specific variables. + +**Configuration:** + +You can customize this behavior in your `settings.json` file: + +- **`security.allowedEnvironmentVariables`**: A list of variable names to + _never_ redact, even if they match sensitive patterns. +- **`security.blockedEnvironmentVariables`**: A list of variable names to + _always_ redact, even if they don't match sensitive patterns. + +```json +{ + "security": { + "allowedEnvironmentVariables": ["MY_PUBLIC_KEY", "NOT_A_SECRET_TOKEN"], + "blockedEnvironmentVariables": ["INTERNAL_IP_ADDRESS"] + } +} +``` + ## Command-line arguments Arguments passed directly when running the CLI can override other configurations diff --git a/packages/cli/src/commands/mcp/list.ts b/packages/cli/src/commands/mcp/list.ts index 4fbdf68fc2..b41baec960 100644 --- a/packages/cli/src/commands/mcp/list.ts +++ b/packages/cli/src/commands/mcp/list.ts @@ -59,10 +59,23 @@ async function testMCPConnection( version: '0.0.1', }); + const settings = loadSettings(); + const sanitizationConfig = { + enableEnvironmentVariableRedaction: true, + allowedEnvironmentVariables: [], + blockedEnvironmentVariables: + settings.merged.advanced?.excludedEnvVars || [], + }; + let transport; try { // Use the same transport creation logic as core - transport = await createTransport(serverName, config, false); + transport = await createTransport( + serverName, + config, + false, + sanitizationConfig, + ); } catch (_error) { await client.close(); return MCPServerStatus.DISCONNECTED; diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 74f9fb3f49..8b0f3aeeca 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -619,8 +619,12 @@ export async function loadCliConfig( mcpServers: settings.mcpServers, allowedMcpServers: argv.allowedMcpServerNames ?? settings.mcp?.allowed, blockedMcpServers: argv.allowedMcpServerNames - ? [] // explicitly allowed servers overrides everything + ? undefined : settings.mcp?.excluded, + blockedEnvironmentVariables: + settings.security?.environmentVariableRedaction?.blocked, + enableEnvironmentVariableRedaction: + settings.security?.environmentVariableRedaction?.enabled, userMemory: memoryContent, geminiMdFileCount: fileCount, geminiMdFilePaths: filePaths, diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index 9145918a48..4009e212d6 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -1203,6 +1203,48 @@ const SETTINGS_SCHEMA = { }, }, }, + environmentVariableRedaction: { + type: 'object', + label: 'Environment Variable Redaction', + category: 'Security', + requiresRestart: false, + default: {}, + description: 'Settings for environment variable redaction.', + showInDialog: false, + properties: { + allowed: { + type: 'array', + label: 'Allowed Environment Variables', + category: 'Security', + requiresRestart: true, + default: [] as string[], + description: + 'Environment variables to always allow (bypass redaction).', + showInDialog: false, + items: { type: 'string' }, + }, + blocked: { + type: 'array', + label: 'Blocked Environment Variables', + category: 'Security', + requiresRestart: true, + default: [] as string[], + description: 'Environment variables to always redact.', + showInDialog: false, + items: { type: 'string' }, + }, + enabled: { + type: 'boolean', + label: 'Enable Environment Variable Redaction', + category: 'Security', + requiresRestart: true, + default: false, + description: + 'Enable redaction of environment variables that may contain secrets.', + showInDialog: true, + }, + }, + }, auth: { type: 'object', label: 'Authentication', diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index adcc02ff97..4986a22617 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -899,6 +899,7 @@ Logging in with Google... Restarting Gemini CLI to continue. ), pager: settings.merged.tools?.shell?.pager, showColor: settings.merged.tools?.shell?.showColor, + sanitizationConfig: config.sanitizationConfig, }); const isFocused = useFocus(); diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 29d9a6667c..fcc16c1e65 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -177,6 +177,7 @@ import { SimpleExtensionLoader, } from '../utils/extensionLoader.js'; import { McpClientManager } from '../tools/mcp-client-manager.js'; +import type { EnvironmentSanitizationConfig } from '../services/environmentSanitization.js'; export type { FileFilteringOptions }; export { @@ -284,6 +285,9 @@ export interface ConfigParameters { enableExtensionReloading?: boolean; allowedMcpServers?: string[]; blockedMcpServers?: string[]; + allowedEnvironmentVariables?: string[]; + blockedEnvironmentVariables?: string[]; + enableEnvironmentVariableRedaction?: boolean; noBrowser?: boolean; summarizeToolOutput?: Record; folderTrust?: boolean; @@ -340,6 +344,9 @@ export class Config { private mcpClientManager?: McpClientManager; private allowedMcpServers: string[]; private blockedMcpServers: string[]; + private allowedEnvironmentVariables: string[]; + private blockedEnvironmentVariables: string[]; + private readonly enableEnvironmentVariableRedaction: boolean; private promptRegistry!: PromptRegistry; private resourceRegistry!: ResourceRegistry; private agentRegistry!: AgentRegistry; @@ -479,6 +486,10 @@ export class Config { this.mcpServers = params.mcpServers; this.allowedMcpServers = params.allowedMcpServers ?? []; this.blockedMcpServers = params.blockedMcpServers ?? []; + this.allowedEnvironmentVariables = params.allowedEnvironmentVariables ?? []; + this.blockedEnvironmentVariables = params.blockedEnvironmentVariables ?? []; + this.enableEnvironmentVariableRedaction = + params.enableEnvironmentVariableRedaction ?? false; this.userMemory = params.userMemory ?? ''; this.geminiMdFileCount = params.geminiMdFileCount ?? 0; this.geminiMdFilePaths = params.geminiMdFilePaths ?? []; @@ -547,6 +558,7 @@ export class Config { terminalHeight: params.shellExecutionConfig?.terminalHeight ?? 24, showColor: params.shellExecutionConfig?.showColor ?? false, pager: params.shellExecutionConfig?.pager ?? 'cat', + sanitizationConfig: this.sanitizationConfig, }; this.truncateToolOutputThreshold = params.truncateToolOutputThreshold ?? @@ -1069,6 +1081,15 @@ export class Config { return this.blockedMcpServers; } + get sanitizationConfig(): EnvironmentSanitizationConfig { + return { + allowedEnvironmentVariables: this.allowedEnvironmentVariables, + blockedEnvironmentVariables: this.blockedEnvironmentVariables, + enableEnvironmentVariableRedaction: + this.enableEnvironmentVariableRedaction, + }; + } + setMcpServers(mcpServers: Record): void { this.mcpServers = mcpServers; } @@ -1488,6 +1509,9 @@ export class Config { config.terminalHeight ?? this.shellExecutionConfig.terminalHeight, showColor: config.showColor ?? this.shellExecutionConfig.showColor, pager: config.pager ?? this.shellExecutionConfig.pager, + sanitizationConfig: + config.sanitizationConfig ?? + this.shellExecutionConfig.sanitizationConfig, }; } getScreenReader(): boolean { diff --git a/packages/core/src/core/coreToolScheduler.test.ts b/packages/core/src/core/coreToolScheduler.test.ts index 3a45f73e2f..6474331adb 100644 --- a/packages/core/src/core/coreToolScheduler.test.ts +++ b/packages/core/src/core/coreToolScheduler.test.ts @@ -251,6 +251,11 @@ function createMockConfig(overrides: Partial = {}): Config { getShellExecutionConfig: () => ({ terminalWidth: 90, terminalHeight: 30, + sanitizationConfig: { + enableEnvironmentVariableRedaction: true, + allowedEnvironmentVariables: [], + blockedEnvironmentVariables: [], + }, }), storage: { getProjectTempDir: () => '/tmp', @@ -1444,6 +1449,11 @@ describe('CoreToolScheduler request queueing', () => { getShellExecutionConfig: () => ({ terminalWidth: 80, terminalHeight: 24, + sanitizationConfig: { + enableEnvironmentVariableRedaction: true, + allowedEnvironmentVariables: [], + blockedEnvironmentVariables: [], + }, }), isInteractive: () => false, }); @@ -1556,6 +1566,11 @@ describe('CoreToolScheduler request queueing', () => { getShellExecutionConfig: () => ({ terminalWidth: 80, terminalHeight: 24, + sanitizationConfig: { + enableEnvironmentVariableRedaction: true, + allowedEnvironmentVariables: [], + blockedEnvironmentVariables: [], + }, }), getToolRegistry: () => toolRegistry, getHookSystem: () => undefined, diff --git a/packages/core/src/hooks/hookRunner.test.ts b/packages/core/src/hooks/hookRunner.test.ts index 1853718aa9..090ae8e0d8 100644 --- a/packages/core/src/hooks/hookRunner.test.ts +++ b/packages/core/src/hooks/hookRunner.test.ts @@ -7,12 +7,11 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process'; import { HookRunner } from './hookRunner.js'; -import { HookEventName, HookType } from './types.js'; +import { HookEventName, HookType, ConfigSource } from './types.js'; import type { HookConfig } from './types.js'; import type { HookInput } from './types.js'; import type { Readable, Writable } from 'node:stream'; import type { Config } from '../config/config.js'; -import { ConfigSource } from './types.js'; // Mock type for the child_process spawn type MockChildProcessWithoutNullStreams = ChildProcessWithoutNullStreams & { @@ -70,6 +69,9 @@ describe('HookRunner', () => { mockConfig = { isTrustedFolder: vi.fn().mockReturnValue(true), + sanitizationConfig: { + enableEnvironmentVariableRedaction: true, + }, } as unknown as Config; hookRunner = new HookRunner(mockConfig); diff --git a/packages/core/src/hooks/hookRunner.ts b/packages/core/src/hooks/hookRunner.ts index a70e2fe8ad..0f7b903521 100644 --- a/packages/core/src/hooks/hookRunner.ts +++ b/packages/core/src/hooks/hookRunner.ts @@ -18,6 +18,7 @@ import type { } from './types.js'; import type { LLMRequest } from './hookTranslator.js'; import { debugLogger } from '../utils/debugLogger.js'; +import { sanitizeEnvironment } from '../services/environmentSanitization.js'; import { escapeShellArg, getShellConfiguration, @@ -238,7 +239,7 @@ export class HookRunner { // Set up environment variables const env = { - ...process.env, + ...sanitizeEnvironment(process.env, this.config.sanitizationConfig), GEMINI_PROJECT_DIR: input.cwd, CLAUDE_PROJECT_DIR: input.cwd, // For compatibility }; diff --git a/packages/core/src/services/environmentSanitization.test.ts b/packages/core/src/services/environmentSanitization.test.ts new file mode 100644 index 0000000000..cc26d7547d --- /dev/null +++ b/packages/core/src/services/environmentSanitization.test.ts @@ -0,0 +1,309 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it } from 'vitest'; +import { + ALWAYS_ALLOWED_ENVIRONMENT_VARIABLES, + NEVER_ALLOWED_ENVIRONMENT_VARIABLES, + NEVER_ALLOWED_NAME_PATTERNS, + NEVER_ALLOWED_VALUE_PATTERNS, + sanitizeEnvironment, +} from './environmentSanitization.js'; + +const EMPTY_OPTIONS = { + allowedEnvironmentVariables: [], + blockedEnvironmentVariables: [], + enableEnvironmentVariableRedaction: true, +}; + +describe('sanitizeEnvironment', () => { + it('should allow safe, common environment variables', () => { + const env = { + PATH: '/usr/bin', + HOME: '/home/user', + USER: 'user', + SystemRoot: 'C:\\Windows', + LANG: 'en_US.UTF-8', + }; + const sanitized = sanitizeEnvironment(env, EMPTY_OPTIONS); + expect(sanitized).toEqual(env); + }); + + it('should allow variables prefixed with GEMINI_CLI_', () => { + const env = { + GEMINI_CLI_FOO: 'bar', + GEMINI_CLI_BAZ: 'qux', + }; + const sanitized = sanitizeEnvironment(env, EMPTY_OPTIONS); + expect(sanitized).toEqual(env); + }); + + it('should redact variables with sensitive names from the denylist', () => { + const env = { + CLIENT_ID: 'sensitive-id', + DB_URI: 'sensitive-uri', + DATABASE_URL: 'sensitive-url', + SAFE_VAR: 'is-safe', + }; + const sanitized = sanitizeEnvironment(env, EMPTY_OPTIONS); + expect(sanitized).toEqual({ + SAFE_VAR: 'is-safe', + }); + }); + + it('should redact variables with names matching all sensitive patterns (case-insensitive)', () => { + const env = { + // Patterns + MY_API_TOKEN: 'token-value', + AppSecret: 'secret-value', + db_password: 'password-value', + ORA_PASSWD: 'password-value', + ANOTHER_KEY: 'key-value', + some_auth_var: 'auth-value', + USER_CREDENTIAL: 'cred-value', + AWS_CREDS: 'creds-value', + PRIVATE_STUFF: 'private-value', + SSL_CERT: 'cert-value', + // Safe variable + USEFUL_INFO: 'is-ok', + }; + const sanitized = sanitizeEnvironment(env, EMPTY_OPTIONS); + expect(sanitized).toEqual({ + USEFUL_INFO: 'is-ok', + }); + }); + + it('should redact variables with values matching all private key patterns', () => { + const env = { + RSA_KEY: '-----BEGIN RSA PRIVATE KEY-----...', + OPENSSH_KEY: '-----BEGIN OPENSSH PRIVATE KEY-----...', + EC_KEY: '-----BEGIN EC PRIVATE KEY-----...', + PGP_KEY: '-----BEGIN PGP PRIVATE KEY-----...', + CERTIFICATE: '-----BEGIN CERTIFICATE-----...', + SAFE_VAR: 'is-safe', + }; + const sanitized = sanitizeEnvironment(env, EMPTY_OPTIONS); + expect(sanitized).toEqual({ + SAFE_VAR: 'is-safe', + }); + }); + + it('should redact variables with values matching all token and credential patterns', () => { + const env = { + // GitHub + GITHUB_TOKEN_GHP: 'ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx', + GITHUB_TOKEN_GHO: 'gho_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx', + GITHUB_TOKEN_GHU: 'ghu_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx', + GITHUB_TOKEN_GHS: 'ghs_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx', + GITHUB_TOKEN_GHR: 'ghr_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx', + GITHUB_PAT: 'github_pat_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx', + // Google + GOOGLE_KEY: 'AIzaSyxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx', + // AWS + AWS_KEY: 'AKIAxxxxxxxxxxxxxxxx', + // JWT + JWT_TOKEN: 'eyJhbGciOiJIUzI1NiJ9.e30.ZRrHA157xAA_7962-a_3rA', + // Stripe + STRIPE_SK_LIVE: 'sk_live_xxxxxxxxxxxxxxxxxxxxxxxx', + STRIPE_RK_LIVE: 'rk_live_xxxxxxxxxxxxxxxxxxxxxxxx', + STRIPE_SK_TEST: 'sk_test_xxxxxxxxxxxxxxxxxxxxxxxx', + STRIPE_RK_TEST: 'rk_test_xxxxxxxxxxxxxxxxxxxxxxxx', + // Slack + SLACK_XOXB: 'xoxb-xxxxxxxxxxxx-xxxxxxxxxxxx-xxxxxxxx', + SLACK_XOXA: 'xoxa-xxxxxxxxxxxx-xxxxxxxxxxxx-xxxxxxxx', + SLACK_XOXP: 'xoxp-xxxxxxxxxxxx-xxxxxxxxxxxx-xxxxxxxx', + SLACK_XOXB_2: 'xoxr-xxxxxxxxxxxx-xxxxxxxxxxxx-xxxxxxxx', + // URL Credentials + CREDS_IN_HTTPS_URL: 'https://user:password@example.com', + CREDS_IN_HTTP_URL: 'http://user:password@example.com', + CREDS_IN_FTP_URL: 'ftp://user:password@example.com', + CREDS_IN_SMTP_URL: 'smtp://user:password@example.com', + // Safe variable + SAFE_VAR: 'is-safe', + }; + const sanitized = sanitizeEnvironment(env, EMPTY_OPTIONS); + expect(sanitized).toEqual({ + SAFE_VAR: 'is-safe', + }); + }); + + it('should not redact variables that look similar to sensitive patterns', () => { + const env = { + // Not a credential in URL + SAFE_URL: 'https://example.com/foo/bar', + // Not a real JWT + NOT_A_JWT: 'this.is.not.a.jwt', + // Too short to be a token + ALMOST_A_TOKEN: 'ghp_12345', + // Contains a sensitive word, but in a safe context in the value + PUBLIC_KEY_INFO: 'This value describes a public key', + // Variable names that could be false positives + KEYNOTE_SPEAKER: 'Dr. Jane Goodall', + CERTIFIED_DIVER: 'true', + AUTHENTICATION_FLOW: 'oauth', + PRIVATE_JET_OWNER: 'false', + }; + const sanitized = sanitizeEnvironment(env, EMPTY_OPTIONS); + expect(sanitized).toEqual({ + SAFE_URL: 'https://example.com/foo/bar', + NOT_A_JWT: 'this.is.not.a.jwt', + }); + }); + + it('should not redact variables with undefined or empty values if name is safe', () => { + const env: NodeJS.ProcessEnv = { + EMPTY_VAR: '', + UNDEFINED_VAR: undefined, + ANOTHER_SAFE_VAR: 'value', + }; + const sanitized = sanitizeEnvironment(env, EMPTY_OPTIONS); + expect(sanitized).toEqual({ + EMPTY_VAR: '', + ANOTHER_SAFE_VAR: 'value', + }); + }); + + it('should allow variables that do not match any redaction rules', () => { + const env = { + NODE_ENV: 'development', + APP_VERSION: '1.0.0', + }; + const sanitized = sanitizeEnvironment(env, EMPTY_OPTIONS); + expect(sanitized).toEqual(env); + }); + + it('should handle an empty environment', () => { + const env = {}; + const sanitized = sanitizeEnvironment(env, EMPTY_OPTIONS); + expect(sanitized).toEqual({}); + }); + + it('should handle a mixed environment with allowed and redacted variables', () => { + const env = { + // Allowed + PATH: '/usr/bin', + HOME: '/home/user', + GEMINI_CLI_VERSION: '1.2.3', + NODE_ENV: 'production', + // Redacted by name + API_KEY: 'should-be-redacted', + MY_SECRET: 'super-secret', + // Redacted by value + GH_TOKEN: 'ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx', + JWT: 'eyJhbGciOiJIUzI1NiJ9.e30.ZRrHA157xAA_7962-a_3rA', + // Allowed by name but redacted by value + RANDOM_VAR: '-----BEGIN CERTIFICATE-----...', + }; + const sanitized = sanitizeEnvironment(env, EMPTY_OPTIONS); + expect(sanitized).toEqual({ + PATH: '/usr/bin', + HOME: '/home/user', + GEMINI_CLI_VERSION: '1.2.3', + NODE_ENV: 'production', + }); + }); + + it('should ensure all names in the sets are capitalized', () => { + for (const name of ALWAYS_ALLOWED_ENVIRONMENT_VARIABLES) { + expect(name).toBe(name.toUpperCase()); + } + for (const name of NEVER_ALLOWED_ENVIRONMENT_VARIABLES) { + expect(name).toBe(name.toUpperCase()); + } + }); + + it('should ensure all of the regex in the patterns lists are case insensitive', () => { + for (const pattern of NEVER_ALLOWED_NAME_PATTERNS) { + expect(pattern.flags).toContain('i'); + } + for (const pattern of NEVER_ALLOWED_VALUE_PATTERNS) { + expect(pattern.flags).toContain('i'); + } + }); + + it('should allow variables specified in allowedEnvironmentVariables', () => { + const env = { + MY_TOKEN: 'secret-token', + OTHER_SECRET: 'another-secret', + }; + const allowed = ['MY_TOKEN']; + const sanitized = sanitizeEnvironment(env, { + allowedEnvironmentVariables: allowed, + blockedEnvironmentVariables: [], + enableEnvironmentVariableRedaction: true, + }); + expect(sanitized).toEqual({ + MY_TOKEN: 'secret-token', + }); + }); + + it('should block variables specified in blockedEnvironmentVariables', () => { + const env = { + SAFE_VAR: 'safe-value', + BLOCKED_VAR: 'blocked-value', + }; + const blocked = ['BLOCKED_VAR']; + const sanitized = sanitizeEnvironment(env, { + allowedEnvironmentVariables: [], + blockedEnvironmentVariables: blocked, + enableEnvironmentVariableRedaction: true, + }); + expect(sanitized).toEqual({ + SAFE_VAR: 'safe-value', + }); + }); + + it('should prioritize allowed over blocked if a variable is in both (though user configuration should avoid this)', () => { + const env = { + CONFLICT_VAR: 'value', + }; + const allowed = ['CONFLICT_VAR']; + const blocked = ['CONFLICT_VAR']; + const sanitized = sanitizeEnvironment(env, { + allowedEnvironmentVariables: allowed, + blockedEnvironmentVariables: blocked, + enableEnvironmentVariableRedaction: true, + }); + expect(sanitized).toEqual({ + CONFLICT_VAR: 'value', + }); + }); + + it('should be case insensitive for allowed and blocked lists', () => { + const env = { + MY_TOKEN: 'secret-token', + BLOCKED_VAR: 'blocked-value', + }; + const allowed = ['my_token']; + const blocked = ['blocked_var']; + const sanitized = sanitizeEnvironment(env, { + allowedEnvironmentVariables: allowed, + blockedEnvironmentVariables: blocked, + enableEnvironmentVariableRedaction: true, + }); + expect(sanitized).toEqual({ + MY_TOKEN: 'secret-token', + }); + }); + + it('should not perform any redaction if enableEnvironmentVariableRedaction is false', () => { + const env = { + MY_API_TOKEN: 'token-value', + AppSecret: 'secret-value', + db_password: 'password-value', + RSA_KEY: '-----BEGIN RSA PRIVATE KEY-----...', + GITHUB_TOKEN_GHP: 'ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx', + SAFE_VAR: 'is-safe', + }; + const options = { + allowedEnvironmentVariables: [], + blockedEnvironmentVariables: [], + enableEnvironmentVariableRedaction: false, + }; + const sanitized = sanitizeEnvironment(env, options); + expect(sanitized).toEqual(env); + }); +}); diff --git a/packages/core/src/services/environmentSanitization.ts b/packages/core/src/services/environmentSanitization.ts new file mode 100644 index 0000000000..5d82ac227d --- /dev/null +++ b/packages/core/src/services/environmentSanitization.ts @@ -0,0 +1,191 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +export type EnvironmentSanitizationConfig = { + allowedEnvironmentVariables: string[]; + blockedEnvironmentVariables: string[]; + enableEnvironmentVariableRedaction: boolean; +}; + +export function sanitizeEnvironment( + processEnv: NodeJS.ProcessEnv, + config: EnvironmentSanitizationConfig, +): NodeJS.ProcessEnv { + if (!config.enableEnvironmentVariableRedaction) { + return { ...processEnv }; + } + + const results: NodeJS.ProcessEnv = {}; + + const allowedSet = new Set( + (config.allowedEnvironmentVariables || []).map((k) => k.toUpperCase()), + ); + const blockedSet = new Set( + (config.blockedEnvironmentVariables || []).map((k) => k.toUpperCase()), + ); + + // Enable strict sanitization in GitHub actions. + const isStrictSanitization = !!processEnv['GITHUB_SHA']; + + for (const key in processEnv) { + const value = processEnv[key]; + + if ( + !shouldRedactEnvironmentVariable( + key, + value, + allowedSet, + blockedSet, + isStrictSanitization, + ) + ) { + results[key] = value; + } + } + + return results; +} + +export const ALWAYS_ALLOWED_ENVIRONMENT_VARIABLES: ReadonlySet = + new Set([ + // Cross-platform + 'PATH', + // Windows specific + 'SYSTEMROOT', + 'COMSPEC', + 'PATHEXT', + 'WINDIR', + 'TEMP', + 'TMP', + 'USERPROFILE', + 'SYSTEMDRIVE', + // Unix/Linux/macOS specific + 'HOME', + 'LANG', + 'SHELL', + 'TMPDIR', + 'USER', + 'LOGNAME', + // GitHub Action-related variables + 'ADDITIONAL_CONTEXT', + 'AVAILABLE_LABELS', + 'BRANCH_NAME', + 'DESCRIPTION', + 'EVENT_NAME', + 'GITHUB_ENV', + 'IS_PULL_REQUEST', + 'ISSUES_TO_TRIAGE', + 'ISSUE_BODY', + 'ISSUE_NUMBER', + 'ISSUE_TITLE', + 'PULL_REQUEST_NUMBER', + 'REPOSITORY', + 'TITLE', + 'TRIGGERING_ACTOR', + ]); + +export const NEVER_ALLOWED_ENVIRONMENT_VARIABLES: ReadonlySet = new Set( + [ + 'CLIENT_ID', + 'DB_URI', + 'CONNECTION_STRING', + 'AWS_DEFAULT_REGION', + 'AZURE_CLIENT_ID', + 'AZURE_TENANT_ID', + 'SLACK_WEBHOOK_URL', + 'TWILIO_ACCOUNT_SID', + 'DATABASE_URL', + 'GOOGLE_CLOUD_PROJECT', + 'GOOGLE_CLOUD_ACCOUNT', + 'FIREBASE_PROJECT_ID', + ], +); + +export const NEVER_ALLOWED_NAME_PATTERNS = [ + /TOKEN/i, + /SECRET/i, + /PASSWORD/i, + /PASSWD/i, + /KEY/i, + /AUTH/i, + /CREDENTIAL/i, + /CREDS/i, + /PRIVATE/i, + /CERT/i, +] as const; + +export const NEVER_ALLOWED_VALUE_PATTERNS = [ + /-----BEGIN (RSA|OPENSSH|EC|PGP) PRIVATE KEY-----/i, + /-----BEGIN CERTIFICATE-----/i, + // Credentials in URL + /(https?|ftp|smtp):\/\/[^:]+:[^@]+@/i, + // GitHub tokens (classic, fine-grained, OAuth, etc.) + /(ghp|gho|ghu|ghs|ghr|github_pat)_[a-zA-Z0-9_]{36,}/i, + // Google API keys + /AIzaSy[a-zA-Z0-9_\\-]{33}/i, + // Amazon AWS Access Key ID + /AKIA[A-Z0-9]{16}/i, + // Generic OAuth/JWT tokens + /eyJ[a-zA-Z0-9_-]*\.[a-zA-Z0-9_-]*\.[a-zA-Z0-9_-]*/i, + // Stripe API keys + /(s|r)k_(live|test)_[0-9a-zA-Z]{24}/i, + // Slack tokens (bot, user, etc.) + /xox[abpr]-[a-zA-Z0-9-]+/i, +] as const; + +function shouldRedactEnvironmentVariable( + key: string, + value: string | undefined, + allowedSet?: Set, + blockedSet?: Set, + isStrictSanitization = false, +): boolean { + key = key.toUpperCase(); + value = value?.toUpperCase(); + + // User overrides take precedence. + if (allowedSet?.has(key)) { + return false; + } + if (blockedSet?.has(key)) { + return true; + } + + // These are never redacted. + if ( + ALWAYS_ALLOWED_ENVIRONMENT_VARIABLES.has(key) || + key.startsWith('GEMINI_CLI_') + ) { + return false; + } + + // These are always redacted. + if (NEVER_ALLOWED_ENVIRONMENT_VARIABLES.has(key)) { + return true; + } + + // If in strict mode (e.g. GitHub Action), and not explicitly allowed, redact it. + if (isStrictSanitization) { + return true; + } + + for (const pattern of NEVER_ALLOWED_NAME_PATTERNS) { + if (pattern.test(key)) { + return true; + } + } + + // Redact if the value looks like a key/cert. + if (value) { + for (const pattern of NEVER_ALLOWED_VALUE_PATTERNS) { + if (pattern.test(value)) { + return true; + } + } + } + + return false; +} diff --git a/packages/core/src/services/shellExecutionService.test.ts b/packages/core/src/services/shellExecutionService.test.ts index 39a708a1b0..4061ed5b84 100644 --- a/packages/core/src/services/shellExecutionService.test.ts +++ b/packages/core/src/services/shellExecutionService.test.ts @@ -83,6 +83,11 @@ const shellExecutionConfig: ShellExecutionConfig = { pager: 'cat', showColor: false, disableDynamicLineTrimming: true, + sanitizationConfig: { + enableEnvironmentVariableRedaction: true, + allowedEnvironmentVariables: [], + blockedEnvironmentVariables: [], + }, }; const createMockSerializeTerminalToObjectReturnValue = ( @@ -551,7 +556,13 @@ describe('ShellExecutionService', () => { onOutputEventMock, new AbortController().signal, true, - {}, + { + sanitizationConfig: { + enableEnvironmentVariableRedaction: true, + allowedEnvironmentVariables: [], + blockedEnvironmentVariables: [], + }, + }, ); const result = await handle.result; @@ -1070,7 +1081,13 @@ describe('ShellExecutionService child_process fallback', () => { onOutputEventMock, abortController.signal, true, - {}, + { + sanitizationConfig: { + enableEnvironmentVariableRedaction: true, + allowedEnvironmentVariables: [], + blockedEnvironmentVariables: [], + }, + }, ); abortController.abort(); @@ -1258,7 +1275,13 @@ describe('ShellExecutionService execution method selection', () => { onOutputEventMock, abortController.signal, false, // shouldUseNodePty - {}, + { + sanitizationConfig: { + enableEnvironmentVariableRedaction: true, + allowedEnvironmentVariables: [], + blockedEnvironmentVariables: [], + }, + }, ); // Simulate exit to allow promise to resolve diff --git a/packages/core/src/services/shellExecutionService.ts b/packages/core/src/services/shellExecutionService.ts index 44ef55a994..28f9321596 100644 --- a/packages/core/src/services/shellExecutionService.ts +++ b/packages/core/src/services/shellExecutionService.ts @@ -19,6 +19,10 @@ import { serializeTerminalToObject, type AnsiOutput, } from '../utils/terminalSerializer.js'; +import { + sanitizeEnvironment, + type EnvironmentSanitizationConfig, +} from './environmentSanitization.js'; const { Terminal } = pkg; const SIGKILL_TIMEOUT_MS = 200; @@ -80,6 +84,7 @@ export interface ShellExecutionConfig { showColor?: boolean; defaultFg?: string; defaultBg?: string; + sanitizationConfig: EnvironmentSanitizationConfig; // Used for testing disableDynamicLineTrimming?: boolean; scrollback?: number; @@ -148,74 +153,6 @@ const getFullBufferText = (terminal: pkg.Terminal): string => { return lines.join('\n'); }; -function getSanitizedEnv(): NodeJS.ProcessEnv { - const isRunningInGithub = - process.env['GITHUB_SHA'] || process.env['SURFACE'] === 'Github'; - - if (!isRunningInGithub) { - // For local runs, we want to preserve the user's full environment. - return { ...process.env }; - } - - // For CI runs (GitHub), we sanitize the environment for security. - const env: NodeJS.ProcessEnv = {}; - const essentialVars = [ - // Cross-platform - 'PATH', - // Windows specific - 'Path', - 'SYSTEMROOT', - 'SystemRoot', - 'COMSPEC', - 'ComSpec', - 'PATHEXT', - 'WINDIR', - 'TEMP', - 'TMP', - 'USERPROFILE', - 'SYSTEMDRIVE', - 'SystemDrive', - // Unix/Linux/macOS specific - 'HOME', - 'LANG', - 'SHELL', - 'TMPDIR', - 'USER', - 'LOGNAME', - // GitHub Action-related variables - 'ADDITIONAL_CONTEXT', - 'AVAILABLE_LABELS', - 'BRANCH_NAME', - 'DESCRIPTION', - 'EVENT_NAME', - 'GITHUB_ENV', - 'IS_PULL_REQUEST', - 'ISSUES_TO_TRIAGE', - 'ISSUE_BODY', - 'ISSUE_NUMBER', - 'ISSUE_TITLE', - 'PULL_REQUEST_NUMBER', - 'REPOSITORY', - 'TITLE', - 'TRIGGERING_ACTOR', - ]; - - for (const key of essentialVars) { - if (process.env[key] !== undefined) { - env[key] = process.env[key]; - } - } - - // Always carry over variables and secrets with GEMINI_CLI_*. - for (const key in process.env) { - if (key.startsWith('GEMINI_CLI_')) { - env[key] = process.env[key]; - } - } - - return env; -} - /** * A centralized service for executing shell commands with robust process * management, cross-platform compatibility, and streaming output capabilities. @@ -265,6 +202,7 @@ export class ShellExecutionService { cwd, onOutputEvent, abortSignal, + shellExecutionConfig.sanitizationConfig, ); } @@ -303,6 +241,7 @@ export class ShellExecutionService { cwd: string, onOutputEvent: (event: ShellOutputEvent) => void, abortSignal: AbortSignal, + sanitizationConfig: EnvironmentSanitizationConfig, ): ShellExecutionHandle { try { const isWindows = os.platform() === 'win32'; @@ -317,7 +256,7 @@ export class ShellExecutionService { shell: false, detached: !isWindows, env: { - ...getSanitizedEnv(), + ...sanitizeEnvironment(process.env, sanitizationConfig), GEMINI_CLI: '1', TERM: 'xterm-256color', PAGER: 'cat', @@ -531,7 +470,10 @@ export class ShellExecutionService { cols, rows, env: { - ...getSanitizedEnv(), + ...sanitizeEnvironment( + process.env, + shellExecutionConfig.sanitizationConfig, + ), GEMINI_CLI: '1', TERM: 'xterm-256color', PAGER: shellExecutionConfig.pager ?? 'cat', diff --git a/packages/core/src/tools/mcp-client.test.ts b/packages/core/src/tools/mcp-client.test.ts index 192025689d..a448fd288b 100644 --- a/packages/core/src/tools/mcp-client.test.ts +++ b/packages/core/src/tools/mcp-client.test.ts @@ -35,6 +35,13 @@ import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; import { coreEvents } from '../utils/events.js'; +import type { EnvironmentSanitizationConfig } from '../services/environmentSanitization.js'; + +const EMPTY_CONFIG: EnvironmentSanitizationConfig = { + enableEnvironmentVariableRedaction: true, + allowedEnvironmentVariables: [], + blockedEnvironmentVariables: [], +}; vi.mock('@modelcontextprotocol/sdk/client/stdio.js'); vi.mock('@modelcontextprotocol/sdk/client/index.js'); @@ -124,7 +131,7 @@ describe('mcp-client', () => { promptRegistry, resourceRegistry, workspaceContext, - {} as Config, + { sanitizationConfig: EMPTY_CONFIG } as Config, false, ); await client.connect(); @@ -204,7 +211,7 @@ describe('mcp-client', () => { promptRegistry, resourceRegistry, workspaceContext, - {} as Config, + { sanitizationConfig: EMPTY_CONFIG } as Config, false, ); await client.connect(); @@ -255,7 +262,7 @@ describe('mcp-client', () => { promptRegistry, resourceRegistry, workspaceContext, - {} as Config, + { sanitizationConfig: EMPTY_CONFIG } as Config, false, ); await client.connect(); @@ -310,7 +317,7 @@ describe('mcp-client', () => { promptRegistry, resourceRegistry, workspaceContext, - {} as Config, + { sanitizationConfig: EMPTY_CONFIG } as Config, false, ); await client.connect(); @@ -369,7 +376,7 @@ describe('mcp-client', () => { promptRegistry, resourceRegistry, workspaceContext, - {} as Config, + { sanitizationConfig: EMPTY_CONFIG } as Config, false, ); await client.connect(); @@ -442,7 +449,7 @@ describe('mcp-client', () => { promptRegistry, resourceRegistry, workspaceContext, - {} as Config, + { sanitizationConfig: EMPTY_CONFIG } as Config, false, ); await client.connect(); @@ -518,7 +525,7 @@ describe('mcp-client', () => { promptRegistry, resourceRegistry, workspaceContext, - {} as Config, + { sanitizationConfig: EMPTY_CONFIG } as Config, false, ); await client.connect(); @@ -601,7 +608,7 @@ describe('mcp-client', () => { promptRegistry, resourceRegistry, workspaceContext, - {} as Config, + { sanitizationConfig: EMPTY_CONFIG } as Config, false, ); await client.connect(); @@ -681,7 +688,7 @@ describe('mcp-client', () => { mockedPromptRegistry, resourceRegistry, workspaceContext, - {} as Config, + { sanitizationConfig: EMPTY_CONFIG } as Config, false, ); await client.connect(); @@ -730,7 +737,7 @@ describe('mcp-client', () => { {} as PromptRegistry, {} as ResourceRegistry, workspaceContext, - {} as Config, + { sanitizationConfig: EMPTY_CONFIG } as Config, false, ); @@ -766,7 +773,7 @@ describe('mcp-client', () => { {} as PromptRegistry, {} as ResourceRegistry, workspaceContext, - {} as Config, + { sanitizationConfig: EMPTY_CONFIG } as Config, false, ); @@ -821,7 +828,7 @@ describe('mcp-client', () => { {} as PromptRegistry, {} as ResourceRegistry, workspaceContext, - {} as Config, + { sanitizationConfig: EMPTY_CONFIG } as Config, false, onToolsUpdatedSpy, ); @@ -891,7 +898,7 @@ describe('mcp-client', () => { {} as PromptRegistry, {} as ResourceRegistry, workspaceContext, - {} as Config, + { sanitizationConfig: EMPTY_CONFIG } as Config, false, ); @@ -961,7 +968,7 @@ describe('mcp-client', () => { {} as PromptRegistry, {} as ResourceRegistry, workspaceContext, - {} as Config, + { sanitizationConfig: EMPTY_CONFIG } as Config, false, onToolsUpdatedSpy, ); @@ -973,7 +980,7 @@ describe('mcp-client', () => { {} as PromptRegistry, {} as ResourceRegistry, workspaceContext, - {} as Config, + { sanitizationConfig: EMPTY_CONFIG } as Config, false, onToolsUpdatedSpy, ); @@ -1055,7 +1062,7 @@ describe('mcp-client', () => { {} as PromptRegistry, {} as ResourceRegistry, workspaceContext, - {} as Config, + { sanitizationConfig: EMPTY_CONFIG } as Config, false, ); @@ -1119,7 +1126,7 @@ describe('mcp-client', () => { {} as PromptRegistry, {} as ResourceRegistry, workspaceContext, - {} as Config, + { sanitizationConfig: EMPTY_CONFIG } as Config, false, onToolsUpdatedSpy, ); @@ -1170,6 +1177,7 @@ describe('mcp-client', () => { httpUrl: 'http://test-server', }, false, + EMPTY_CONFIG, ); expect(transport).toBeInstanceOf(StreamableHTTPClientTransport); @@ -1187,6 +1195,7 @@ describe('mcp-client', () => { headers: { Authorization: 'derp' }, }, false, + EMPTY_CONFIG, ); expect(transport).toBeInstanceOf(StreamableHTTPClientTransport); @@ -1207,6 +1216,7 @@ describe('mcp-client', () => { url: 'http://test-server', }, false, + EMPTY_CONFIG, ); expect(transport).toBeInstanceOf(StreamableHTTPClientTransport); expect(transport).toMatchObject({ @@ -1223,6 +1233,7 @@ describe('mcp-client', () => { headers: { Authorization: 'derp' }, }, false, + EMPTY_CONFIG, ); expect(transport).toBeInstanceOf(StreamableHTTPClientTransport); @@ -1242,6 +1253,7 @@ describe('mcp-client', () => { type: 'http', }, false, + EMPTY_CONFIG, ); expect(transport).toBeInstanceOf(StreamableHTTPClientTransport); @@ -1259,6 +1271,7 @@ describe('mcp-client', () => { type: 'sse', }, false, + EMPTY_CONFIG, ); expect(transport).toBeInstanceOf(SSEClientTransport); @@ -1275,6 +1288,7 @@ describe('mcp-client', () => { url: 'http://test-server', }, false, + EMPTY_CONFIG, ); expect(transport).toBeInstanceOf(StreamableHTTPClientTransport); @@ -1293,6 +1307,7 @@ describe('mcp-client', () => { headers: { Authorization: 'Bearer token' }, }, false, + EMPTY_CONFIG, ); expect(transport).toBeInstanceOf(StreamableHTTPClientTransport); @@ -1313,6 +1328,7 @@ describe('mcp-client', () => { headers: { 'X-API-Key': 'key123' }, }, false, + EMPTY_CONFIG, ); expect(transport).toBeInstanceOf(SSEClientTransport); @@ -1332,6 +1348,7 @@ describe('mcp-client', () => { url: 'http://test-server-url', }, false, + EMPTY_CONFIG, ); // httpUrl should take priority and create HTTP transport @@ -1357,13 +1374,14 @@ describe('mcp-client', () => { cwd: 'test/cwd', }, false, + EMPTY_CONFIG, ); expect(mockedTransport).toHaveBeenCalledWith({ command: 'test-command', args: ['--foo', 'bar'], cwd: 'test/cwd', - env: { ...process.env, FOO: 'bar' }, + env: expect.objectContaining({ FOO: 'bar' }), stderr: 'pipe', }); }); @@ -1393,6 +1411,7 @@ describe('mcp-client', () => { }, }, false, + EMPTY_CONFIG, ); expect(transport).toBeInstanceOf(StreamableHTTPClientTransport); @@ -1425,6 +1444,7 @@ describe('mcp-client', () => { }, }, false, + EMPTY_CONFIG, ); expect(transport).toBeInstanceOf(StreamableHTTPClientTransport); @@ -1456,6 +1476,7 @@ describe('mcp-client', () => { }, }, false, + EMPTY_CONFIG, ); expect(transport).toBeInstanceOf(StreamableHTTPClientTransport); @@ -1476,6 +1497,7 @@ describe('mcp-client', () => { }, }, false, + EMPTY_CONFIG, ); expect(transport).toBeInstanceOf(SSEClientTransport); @@ -1495,6 +1517,7 @@ describe('mcp-client', () => { }, }, false, + EMPTY_CONFIG, ), ).rejects.toThrow( 'URL must be provided in the config for Google Credentials provider', @@ -1656,6 +1679,7 @@ describe('connectToMcpServer with OAuth', () => { { httpUrl: serverUrl, oauth: { enabled: true } }, false, workspaceContext, + EMPTY_CONFIG, ); expect(client).toBe(mockedClient); @@ -1700,6 +1724,7 @@ describe('connectToMcpServer with OAuth', () => { { httpUrl: serverUrl, oauth: { enabled: true } }, false, workspaceContext, + EMPTY_CONFIG, ); expect(client).toBe(mockedClient); @@ -1754,6 +1779,7 @@ describe('connectToMcpServer - HTTP→SSE fallback', () => { { url: 'http://test-server', type: 'http' }, false, workspaceContext, + EMPTY_CONFIG, ), ).rejects.toThrow('Connection failed'); @@ -1772,6 +1798,7 @@ describe('connectToMcpServer - HTTP→SSE fallback', () => { { url: 'http://test-server', type: 'sse' }, false, workspaceContext, + EMPTY_CONFIG, ), ).rejects.toThrow('Connection failed'); @@ -1789,6 +1816,7 @@ describe('connectToMcpServer - HTTP→SSE fallback', () => { { url: 'http://test-server' }, false, workspaceContext, + EMPTY_CONFIG, ); expect(client).toBe(mockedClient); @@ -1810,6 +1838,7 @@ describe('connectToMcpServer - HTTP→SSE fallback', () => { { url: 'http://test-server' }, false, workspaceContext, + EMPTY_CONFIG, ), ).rejects.toThrow('Server error'); @@ -1826,6 +1855,7 @@ describe('connectToMcpServer - HTTP→SSE fallback', () => { { url: 'http://test-server' }, false, workspaceContext, + EMPTY_CONFIG, ); expect(client).toBe(mockedClient); @@ -1895,6 +1925,7 @@ describe('connectToMcpServer - OAuth with transport fallback', () => { { url: 'http://test-server', oauth: { enabled: true } }, false, workspaceContext, + EMPTY_CONFIG, ); expect(client).toBe(mockedClient); diff --git a/packages/core/src/tools/mcp-client.ts b/packages/core/src/tools/mcp-client.ts index 86f0043a7d..db7e102c89 100644 --- a/packages/core/src/tools/mcp-client.ts +++ b/packages/core/src/tools/mcp-client.ts @@ -60,6 +60,10 @@ import { debugLogger } from '../utils/debugLogger.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; import { coreEvents } from '../utils/events.js'; import type { ResourceRegistry } from '../resources/resource-registry.js'; +import { + sanitizeEnvironment, + type EnvironmentSanitizationConfig, +} from '../services/environmentSanitization.js'; export const MCP_DEFAULT_TIMEOUT_MSEC = 10 * 60 * 1000; // default to 10 minutes @@ -137,6 +141,7 @@ export class McpClient { this.serverConfig, this.debugMode, this.workspaceContext, + this.cliConfig.sanitizationConfig, ); this.registerNotificationHandlers(); @@ -820,6 +825,7 @@ export async function connectAndDiscover( mcpServerConfig, debugMode, workspaceContext, + cliConfig.sanitizationConfig, ); mcpClient.onerror = (error) => { @@ -1329,6 +1335,7 @@ export async function connectToMcpServer( mcpServerConfig: MCPServerConfig, debugMode: boolean, workspaceContext: WorkspaceContext, + sanitizationConfig: EnvironmentSanitizationConfig, ): Promise { const mcpClient = new Client( { @@ -1395,6 +1402,7 @@ export async function connectToMcpServer( mcpServerName, mcpServerConfig, debugMode, + sanitizationConfig, ); try { await mcpClient.connect(transport, { @@ -1711,6 +1719,7 @@ export async function createTransport( mcpServerName: string, mcpServerConfig: MCPServerConfig, debugMode: boolean, + sanitizationConfig: EnvironmentSanitizationConfig, ): Promise { const noUrl = !mcpServerConfig.url && !mcpServerConfig.httpUrl; if (noUrl) { @@ -1782,7 +1791,7 @@ export async function createTransport( command: mcpServerConfig.command, args: mcpServerConfig.args || [], env: { - ...process.env, + ...sanitizeEnvironment(process.env, sanitizationConfig), ...(mcpServerConfig.env || {}), } as Record, cwd: mcpServerConfig.cwd, diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index e6387dbee5..b4f79a8b0c 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -254,7 +254,13 @@ export class ShellToolInvocation extends BaseToolInvocation< }, combinedController.signal, this.config.getEnableInteractiveShell(), - { ...shellExecutionConfig, pager: 'cat' }, + { + ...shellExecutionConfig, + pager: 'cat', + sanitizationConfig: + shellExecutionConfig?.sanitizationConfig ?? + this.config.sanitizationConfig, + }, ); if (pid && setPidCallback) { diff --git a/schemas/settings.schema.json b/schemas/settings.schema.json index 8e88a88119..6eb95354d6 100644 --- a/schemas/settings.schema.json +++ b/schemas/settings.schema.json @@ -1231,6 +1231,43 @@ }, "additionalProperties": false }, + "environmentVariableRedaction": { + "title": "Environment Variable Redaction", + "description": "Settings for environment variable redaction.", + "markdownDescription": "Settings for environment variable redaction.\n\n- Category: `Security`\n- Requires restart: `no`\n- Default: `{}`", + "default": {}, + "type": "object", + "properties": { + "allowed": { + "title": "Allowed Environment Variables", + "description": "Environment variables to always allow (bypass redaction).", + "markdownDescription": "Environment variables to always allow (bypass redaction).\n\n- Category: `Security`\n- Requires restart: `yes`\n- Default: `[]`", + "default": [], + "type": "array", + "items": { + "type": "string" + } + }, + "blocked": { + "title": "Blocked Environment Variables", + "description": "Environment variables to always redact.", + "markdownDescription": "Environment variables to always redact.\n\n- Category: `Security`\n- Requires restart: `yes`\n- Default: `[]`", + "default": [], + "type": "array", + "items": { + "type": "string" + } + }, + "enabled": { + "title": "Enable Environment Variable Redaction", + "description": "Enable redaction of environment variables that may contain secrets.", + "markdownDescription": "Enable redaction of environment variables that may contain secrets.\n\n- Category: `Security`\n- Requires restart: `yes`\n- Default: `false`", + "default": false, + "type": "boolean" + } + }, + "additionalProperties": false + }, "auth": { "title": "Authentication", "description": "Authentication settings.",