From 8468a48bff974015fcc2c362fb1fe8ca97d2caf4 Mon Sep 17 00:00:00 2001 From: gemini-cli-robot Date: Wed, 4 Feb 2026 08:07:42 -0800 Subject: [PATCH] fix(patch): cherry-pick aba8c5f to release/v0.28.0-preview.0-pr-17806 to patch version v0.28.0-preview.0 and create version 0.28.0-preview.1 (#18307) --- docs/cli/settings.md | 2 +- docs/get-started/configuration.md | 2 +- packages/cli/src/config/config.test.ts | 4 +- .../config/extension-manager-scope.test.ts | 3 + packages/cli/src/config/settings.test.ts | 355 ++++++++++++++++-- packages/cli/src/config/settings.ts | 67 +++- .../cli/src/config/settingsSchema.test.ts | 2 +- packages/cli/src/config/settingsSchema.ts | 2 +- .../cli/src/config/trustedFolders.test.ts | 67 +++- packages/cli/src/config/trustedFolders.ts | 2 +- packages/cli/src/deferred.test.ts | 45 ++- packages/cli/src/test-utils/render.tsx | 17 +- packages/cli/src/test-utils/settings.ts | 79 ++++ .../src/ui/commands/directoryCommand.test.tsx | 5 + .../cli/src/ui/components/CliSpinner.test.tsx | 6 +- .../cli/src/ui/components/Composer.test.tsx | 27 +- .../ui/components/FolderTrustDialog.test.tsx | 4 +- .../cli/src/ui/components/Footer.test.tsx | 6 +- .../src/ui/components/InputPrompt.test.tsx | 6 +- .../src/ui/components/SettingsDialog.test.tsx | 170 ++++----- .../src/ui/components/StatusDisplay.test.tsx | 9 +- .../src/ui/components/ThemeDialog.test.tsx | 44 +-- .../messages/ToolConfirmationMessage.test.tsx | 6 +- .../messages/ToolGroupMessage.test.tsx | 6 +- packages/cli/src/ui/hooks/useFolderTrust.ts | 2 +- .../src/ui/hooks/usePermissionsModifyTrust.ts | 3 +- packages/test-utils/src/test-rig.ts | 89 +++-- schemas/settings.schema.json | 4 +- 28 files changed, 730 insertions(+), 304 deletions(-) create mode 100644 packages/cli/src/test-utils/settings.ts diff --git a/docs/cli/settings.md b/docs/cli/settings.md index de77d2fd2f..e925c49482 100644 --- a/docs/cli/settings.md +++ b/docs/cli/settings.md @@ -115,7 +115,7 @@ they appear in the UI. | Allow Permanent Tool Approval | `security.enablePermanentToolApproval` | Enable the "Allow for all future sessions" option in tool confirmation dialogs. | `false` | | Blocks extensions from Git | `security.blockGitExtensions` | Blocks installing and loading extensions from Git. | `false` | | Extension Source Regex Allowlist | `security.allowedExtensions` | List of Regex patterns for allowed extensions. If nonempty, only extensions that match the patterns in this list are allowed. Overrides the blockGitExtensions setting. | `[]` | -| Folder Trust | `security.folderTrust.enabled` | Setting to track whether Folder trust is enabled. | `false` | +| Folder Trust | `security.folderTrust.enabled` | Setting to track whether Folder trust is enabled. | `true` | | Enable Environment Variable Redaction | `security.environmentVariableRedaction.enabled` | Enable redaction of environment variables that may contain secrets. | `false` | ### Experimental diff --git a/docs/get-started/configuration.md b/docs/get-started/configuration.md index bc6c47a1f2..427667177a 100644 --- a/docs/get-started/configuration.md +++ b/docs/get-started/configuration.md @@ -792,7 +792,7 @@ their corresponding top-level category object in your `settings.json` file. - **`security.folderTrust.enabled`** (boolean): - **Description:** Setting to track whether Folder trust is enabled. - - **Default:** `false` + - **Default:** `true` - **Requires restart:** Yes - **`security.environmentVariableRedaction.allowed`** (array): diff --git a/packages/cli/src/config/config.test.ts b/packages/cli/src/config/config.test.ts index 2ca11be668..727b9b2c52 100644 --- a/packages/cli/src/config/config.test.ts +++ b/packages/cli/src/config/config.test.ts @@ -1556,12 +1556,12 @@ describe('loadCliConfig folderTrust', () => { expect(config.getFolderTrust()).toBe(true); }); - it('should be false by default', async () => { + it('should be true by default', async () => { process.argv = ['node', 'script.js']; const argv = await parseArguments(createTestMergedSettings()); const settings = createTestMergedSettings(); const config = await loadCliConfig(settings, 'test-session', argv); - expect(config.getFolderTrust()).toBe(false); + expect(config.getFolderTrust()).toBe(true); }); }); diff --git a/packages/cli/src/config/extension-manager-scope.test.ts b/packages/cli/src/config/extension-manager-scope.test.ts index aeddb8c707..f88673e692 100644 --- a/packages/cli/src/config/extension-manager-scope.test.ts +++ b/packages/cli/src/config/extension-manager-scope.test.ts @@ -108,6 +108,7 @@ describe('ExtensionManager Settings Scope', () => { settings: createTestMergedSettings({ telemetry: { enabled: false }, experimental: { extensionConfig: true }, + security: { folderTrust: { enabled: false } }, }), }); @@ -146,6 +147,7 @@ describe('ExtensionManager Settings Scope', () => { settings: createTestMergedSettings({ telemetry: { enabled: false }, experimental: { extensionConfig: true }, + security: { folderTrust: { enabled: false } }, }), }); @@ -182,6 +184,7 @@ describe('ExtensionManager Settings Scope', () => { settings: createTestMergedSettings({ telemetry: { enabled: false }, experimental: { extensionConfig: true }, + security: { folderTrust: { enabled: false } }, }), }); diff --git a/packages/cli/src/config/settings.test.ts b/packages/cli/src/config/settings.test.ts index f78790c3a1..10cd6d7558 100644 --- a/packages/cli/src/config/settings.test.ts +++ b/packages/cli/src/config/settings.test.ts @@ -67,13 +67,14 @@ import { getSystemSettingsPath, getSystemDefaultsPath, type Settings, - saveSettings, type SettingsFile, + saveSettings, getDefaultsFromSchema, loadEnvironment, migrateDeprecatedSettings, SettingScope, LoadedSettings, + sanitizeEnvVar, } from './settings.js'; import { FatalConfigError, GEMINI_DIR } from '@google/gemini-cli-core'; import { updateSettingsFilePreservingFormat } from '../utils/commentJson.js'; @@ -82,6 +83,7 @@ import { MergeStrategy, type SettingsSchema, } from './settingsSchema.js'; +import { createMockSettings } from '../test-utils/settings.js'; const MOCK_WORKSPACE_DIR = '/mock/workspace'; // Use the (mocked) GEMINI_DIR for consistency @@ -1746,6 +1748,7 @@ describe('Settings Loading and Merging', () => { isFolderTrustEnabled = true, isWorkspaceTrustedValue = true as boolean | undefined, }) { + delete process.env['GEMINI_API_KEY']; // reset delete process.env['TESTTEST']; // reset const geminiEnvPath = path.resolve( path.join(MOCK_WORKSPACE_DIR, GEMINI_DIR, '.env'), @@ -1779,7 +1782,8 @@ describe('Settings Loading and Merging', () => { const normalizedP = path.resolve(p.toString()); if (normalizedP === path.resolve(USER_SETTINGS_PATH)) return JSON.stringify(userSettingsContent); - if (normalizedP === geminiEnvPath) return 'TESTTEST=1234'; + if (normalizedP === geminiEnvPath) + return 'TESTTEST=1234\nGEMINI_API_KEY=test-key'; return '{}'; }, ); @@ -1793,6 +1797,7 @@ describe('Settings Loading and Merging', () => { loadEnvironment(settings, MOCK_WORKSPACE_DIR, isWorkspaceTrusted); expect(process.env['TESTTEST']).toEqual('1234'); + expect(process.env['GEMINI_API_KEY']).toEqual('test-key'); }); it('does not load env files from untrusted spaces', () => { @@ -1817,6 +1822,36 @@ describe('Settings Loading and Merging', () => { loadEnvironment(settings, MOCK_WORKSPACE_DIR, mockTrustFn); expect(process.env['TESTTEST']).not.toEqual('1234'); + expect(process.env['GEMINI_API_KEY']).not.toEqual('test-key'); + }); + + it('loads whitelisted env files from untrusted spaces if sandboxing is enabled', () => { + setup({ isFolderTrustEnabled: true, isWorkspaceTrustedValue: false }); + const settings = loadSettings(MOCK_WORKSPACE_DIR); + settings.merged.tools.sandbox = true; + loadEnvironment(settings.merged, MOCK_WORKSPACE_DIR); + + // GEMINI_API_KEY is in the whitelist, so it should be loaded. + expect(process.env['GEMINI_API_KEY']).toEqual('test-key'); + // TESTTEST is NOT in the whitelist, so it should be blocked. + expect(process.env['TESTTEST']).not.toEqual('1234'); + }); + + it('loads whitelisted env files from untrusted spaces if sandboxing is enabled via CLI flag', () => { + const originalArgv = [...process.argv]; + process.argv.push('-s'); + try { + setup({ isFolderTrustEnabled: true, isWorkspaceTrustedValue: false }); + const settings = loadSettings(MOCK_WORKSPACE_DIR); + // Ensure sandbox is NOT in settings to test argv sniffing + settings.merged.tools.sandbox = undefined; + loadEnvironment(settings.merged, MOCK_WORKSPACE_DIR); + + expect(process.env['GEMINI_API_KEY']).toEqual('test-key'); + expect(process.env['TESTTEST']).not.toEqual('1234'); + } finally { + process.argv = originalArgv; + } }); }); @@ -1975,29 +2010,7 @@ describe('Settings Loading and Merging', () => { }, }; - const loadedSettings = new LoadedSettings( - { - path: getSystemSettingsPath(), - settings: {}, - originalSettings: {}, - }, - { - path: getSystemDefaultsPath(), - settings: {}, - originalSettings: {}, - }, - { - path: USER_SETTINGS_PATH, - settings: userSettingsContent as unknown as Settings, - originalSettings: userSettingsContent as unknown as Settings, - }, - { - path: MOCK_WORKSPACE_SETTINGS_PATH, - settings: {}, - originalSettings: {}, - }, - true, - ); + const loadedSettings = createMockSettings(userSettingsContent); const setValueSpy = vi.spyOn(loadedSettings, 'setValue'); @@ -2166,11 +2179,8 @@ describe('Settings Loading and Merging', () => { describe('saveSettings', () => { it('should save settings using updateSettingsFilePreservingFormat', () => { const mockUpdateSettings = vi.mocked(updateSettingsFilePreservingFormat); - const settingsFile = { - path: '/mock/settings.json', - settings: { ui: { theme: 'dark' } }, - originalSettings: { ui: { theme: 'dark' } }, - } as unknown as SettingsFile; + const settingsFile = createMockSettings({ ui: { theme: 'dark' } }).user; + settingsFile.path = '/mock/settings.json'; saveSettings(settingsFile); @@ -2184,11 +2194,8 @@ describe('Settings Loading and Merging', () => { const mockFsMkdirSync = vi.mocked(fs.mkdirSync); mockFsExistsSync.mockReturnValue(false); - const settingsFile = { - path: '/mock/new/dir/settings.json', - settings: {}, - originalSettings: {}, - } as unknown as SettingsFile; + const settingsFile = createMockSettings({}).user; + settingsFile.path = '/mock/new/dir/settings.json'; saveSettings(settingsFile); @@ -2205,11 +2212,8 @@ describe('Settings Loading and Merging', () => { throw error; }); - const settingsFile = { - path: '/mock/settings.json', - settings: {}, - originalSettings: {}, - } as unknown as SettingsFile; + const settingsFile = createMockSettings({}).user; + settingsFile.path = '/mock/settings.json'; saveSettings(settingsFile); @@ -2412,6 +2416,277 @@ describe('Settings Loading and Merging', () => { }); }); }); + + describe('Security and Sandbox', () => { + let originalArgv: string[]; + let originalEnv: NodeJS.ProcessEnv; + + beforeEach(() => { + originalArgv = [...process.argv]; + originalEnv = { ...process.env }; + // Clear relevant env vars + delete process.env['GEMINI_API_KEY']; + delete process.env['GOOGLE_API_KEY']; + delete process.env['GOOGLE_CLOUD_PROJECT']; + delete process.env['GOOGLE_CLOUD_LOCATION']; + delete process.env['CLOUD_SHELL']; + delete process.env['MALICIOUS_VAR']; + delete process.env['FOO']; + vi.resetAllMocks(); + vi.mocked(fs.existsSync).mockReturnValue(false); + }); + + afterEach(() => { + process.argv = originalArgv; + process.env = originalEnv; + }); + + describe('sandbox detection', () => { + it('should detect sandbox when -s is a real flag', () => { + process.argv = ['node', 'gemini', '-s', 'some prompt']; + vi.mocked(isWorkspaceTrusted).mockReturnValue({ + isTrusted: false, + source: 'file', + }); + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.readFileSync).mockReturnValue( + 'FOO=bar\nGEMINI_API_KEY=secret', + ); + + loadEnvironment( + createMockSettings({ tools: { sandbox: false } }).merged, + MOCK_WORKSPACE_DIR, + ); + + // If sandboxed and untrusted, FOO should NOT be loaded, but GEMINI_API_KEY should be. + expect(process.env['FOO']).toBeUndefined(); + expect(process.env['GEMINI_API_KEY']).toBe('secret'); + }); + + it('should detect sandbox when --sandbox is a real flag', () => { + process.argv = ['node', 'gemini', '--sandbox', 'prompt']; + vi.mocked(isWorkspaceTrusted).mockReturnValue({ + isTrusted: false, + source: 'file', + }); + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.readFileSync).mockReturnValue('GEMINI_API_KEY=secret'); + + loadEnvironment( + createMockSettings({ tools: { sandbox: false } }).merged, + MOCK_WORKSPACE_DIR, + ); + + expect(process.env['GEMINI_API_KEY']).toBe('secret'); + }); + + it('should ignore sandbox flags if they appear after --', () => { + process.argv = ['node', 'gemini', '--', '-s', 'some prompt']; + vi.mocked(isWorkspaceTrusted).mockReturnValue({ + isTrusted: false, + source: 'file', + }); + vi.mocked(fs.existsSync).mockImplementation((path) => + path.toString().endsWith('.env'), + ); + vi.mocked(fs.readFileSync).mockReturnValue('GEMINI_API_KEY=secret'); + + loadEnvironment( + createMockSettings({ tools: { sandbox: false } }).merged, + MOCK_WORKSPACE_DIR, + ); + + expect(process.env['GEMINI_API_KEY']).toBeUndefined(); + }); + + it('should NOT be tricked by positional arguments that look like flags', () => { + process.argv = ['node', 'gemini', 'my -s prompt']; + vi.mocked(isWorkspaceTrusted).mockReturnValue({ + isTrusted: false, + source: 'file', + }); + vi.mocked(fs.existsSync).mockImplementation((path) => + path.toString().endsWith('.env'), + ); + vi.mocked(fs.readFileSync).mockReturnValue('GEMINI_API_KEY=secret'); + + loadEnvironment( + createMockSettings({ tools: { sandbox: false } }).merged, + MOCK_WORKSPACE_DIR, + ); + + expect(process.env['GEMINI_API_KEY']).toBeUndefined(); + }); + }); + + describe('env var sanitization', () => { + it('should strictly enforce whitelist in untrusted/sandboxed mode', () => { + process.argv = ['node', 'gemini', '-s', 'prompt']; + vi.mocked(isWorkspaceTrusted).mockReturnValue({ + isTrusted: false, + source: 'file', + }); + vi.mocked(fs.existsSync).mockImplementation((path) => + path.toString().endsWith('.env'), + ); + vi.mocked(fs.readFileSync).mockReturnValue(` +GEMINI_API_KEY=secret-key +MALICIOUS_VAR=should-be-ignored +GOOGLE_API_KEY=another-secret + `); + + loadEnvironment( + createMockSettings({ tools: { sandbox: false } }).merged, + MOCK_WORKSPACE_DIR, + ); + + expect(process.env['GEMINI_API_KEY']).toBe('secret-key'); + expect(process.env['GOOGLE_API_KEY']).toBe('another-secret'); + expect(process.env['MALICIOUS_VAR']).toBeUndefined(); + }); + + it('should sanitize shell injection characters in whitelisted env vars in untrusted mode', () => { + process.argv = ['node', 'gemini', '--sandbox', 'prompt']; + vi.mocked(isWorkspaceTrusted).mockReturnValue({ + isTrusted: false, + source: 'file', + }); + vi.mocked(fs.existsSync).mockImplementation((path) => + path.toString().endsWith('.env'), + ); + + const maliciousPayload = 'key-$(whoami)-`id`-&|;><*?[]{}'; + vi.mocked(fs.readFileSync).mockReturnValue( + `GEMINI_API_KEY=${maliciousPayload}`, + ); + + loadEnvironment( + createMockSettings({ tools: { sandbox: false } }).merged, + MOCK_WORKSPACE_DIR, + ); + + // sanitizeEnvVar: value.replace(/[^a-zA-Z0-9\-_./]/g, '') + expect(process.env['GEMINI_API_KEY']).toBe('key-whoami-id-'); + }); + + it('should allow . and / in whitelisted env vars but sanitize other characters in untrusted mode', () => { + process.argv = ['node', 'gemini', '--sandbox', 'prompt']; + vi.mocked(isWorkspaceTrusted).mockReturnValue({ + isTrusted: false, + source: 'file', + }); + vi.mocked(fs.existsSync).mockImplementation((path) => + path.toString().endsWith('.env'), + ); + + const complexPayload = 'secret-123/path.to/somewhere;rm -rf /'; + vi.mocked(fs.readFileSync).mockReturnValue( + `GEMINI_API_KEY=${complexPayload}`, + ); + + loadEnvironment( + createMockSettings({ tools: { sandbox: false } }).merged, + MOCK_WORKSPACE_DIR, + ); + + expect(process.env['GEMINI_API_KEY']).toBe( + 'secret-123/path.to/somewhererm-rf/', + ); + }); + + it('should NOT sanitize variables from trusted sources', () => { + process.argv = ['node', 'gemini', 'prompt']; + vi.mocked(isWorkspaceTrusted).mockReturnValue({ + isTrusted: true, + source: 'file', + }); + vi.mocked(fs.existsSync).mockReturnValue(true); + + vi.mocked(fs.readFileSync).mockReturnValue('FOO=$(bar)'); + + loadEnvironment( + createMockSettings({ tools: { sandbox: false } }).merged, + MOCK_WORKSPACE_DIR, + ); + + // Trusted source, no sanitization + expect(process.env['FOO']).toBe('$(bar)'); + }); + + it('should load environment variables normally when workspace is TRUSTED even if "sandboxed"', () => { + process.argv = ['node', 'gemini', '-s', 'prompt']; + vi.mocked(isWorkspaceTrusted).mockReturnValue({ + isTrusted: true, + source: 'file', + }); + vi.mocked(fs.existsSync).mockImplementation((path) => + path.toString().endsWith('.env'), + ); + vi.mocked(fs.readFileSync).mockReturnValue(` +GEMINI_API_KEY=un-sanitized;key! +MALICIOUS_VAR=allowed-because-trusted + `); + + loadEnvironment( + createMockSettings({ tools: { sandbox: false } }).merged, + MOCK_WORKSPACE_DIR, + ); + + expect(process.env['GEMINI_API_KEY']).toBe('un-sanitized;key!'); + expect(process.env['MALICIOUS_VAR']).toBe('allowed-because-trusted'); + }); + + it('should sanitize value in sanitizeEnvVar helper', () => { + expect(sanitizeEnvVar('$(calc)')).toBe('calc'); + expect(sanitizeEnvVar('`rm -rf /`')).toBe('rm-rf/'); + expect(sanitizeEnvVar('normal-project-123')).toBe('normal-project-123'); + expect(sanitizeEnvVar('us-central1')).toBe('us-central1'); + }); + }); + + describe('Cloud Shell security', () => { + it('should handle Cloud Shell special defaults securely when untrusted', () => { + process.env['CLOUD_SHELL'] = 'true'; + process.argv = ['node', 'gemini', '-s', 'prompt']; + vi.mocked(isWorkspaceTrusted).mockReturnValue({ + isTrusted: false, + source: 'file', + }); + + // No .env file + vi.mocked(fs.existsSync).mockReturnValue(false); + + loadEnvironment( + createMockSettings({ tools: { sandbox: false } }).merged, + MOCK_WORKSPACE_DIR, + ); + + expect(process.env['GOOGLE_CLOUD_PROJECT']).toBe('cloudshell-gca'); + }); + + it('should sanitize GOOGLE_CLOUD_PROJECT in Cloud Shell when loaded from .env in untrusted mode', () => { + process.env['CLOUD_SHELL'] = 'true'; + process.argv = ['node', 'gemini', '-s', 'prompt']; + vi.mocked(isWorkspaceTrusted).mockReturnValue({ + isTrusted: false, + source: 'file', + }); + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.readFileSync).mockReturnValue( + 'GOOGLE_CLOUD_PROJECT=attacker-project;inject', + ); + + loadEnvironment( + createMockSettings({ tools: { sandbox: false } }).merged, + MOCK_WORKSPACE_DIR, + ); + + expect(process.env['GOOGLE_CLOUD_PROJECT']).toBe( + 'attacker-projectinject', + ); + }); + }); + }); }); describe('LoadedSettings Isolation and Serializability', () => { diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts index ee8c1a1fbe..bcc6f2fe83 100644 --- a/packages/cli/src/config/settings.ts +++ b/packages/cli/src/config/settings.ts @@ -77,6 +77,21 @@ export const USER_SETTINGS_PATH = Storage.getGlobalSettingsPath(); export const USER_SETTINGS_DIR = path.dirname(USER_SETTINGS_PATH); export const DEFAULT_EXCLUDED_ENV_VARS = ['DEBUG', 'DEBUG_MODE']; +const AUTH_ENV_VAR_WHITELIST = [ + 'GEMINI_API_KEY', + 'GOOGLE_API_KEY', + 'GOOGLE_CLOUD_PROJECT', + 'GOOGLE_CLOUD_LOCATION', +]; + +/** + * Sanitizes an environment variable value to prevent shell injection. + * Restricts values to a safe character set: alphanumeric, -, _, ., / + */ +export function sanitizeEnvVar(value: string): string { + return value.replace(/[^a-zA-Z0-9\-_./]/g, ''); +} + export function getSystemSettingsPath(): string { if (process.env['GEMINI_CLI_SYSTEM_SETTINGS_PATH']) { return process.env['GEMINI_CLI_SYSTEM_SETTINGS_PATH']; @@ -439,26 +454,30 @@ function findEnvFile(startDir: string): string | null { } } -export function setUpCloudShellEnvironment(envFilePath: string | null): void { +export function setUpCloudShellEnvironment( + envFilePath: string | null, + isTrusted: boolean, + isSandboxed: boolean, +): void { // Special handling for GOOGLE_CLOUD_PROJECT in Cloud Shell: // Because GOOGLE_CLOUD_PROJECT in Cloud Shell tracks the project // set by the user using "gcloud config set project" we do not want to // use its value. So, unless the user overrides GOOGLE_CLOUD_PROJECT in // one of the .env files, we set the Cloud Shell-specific default here. + let value = 'cloudshell-gca'; + if (envFilePath && fs.existsSync(envFilePath)) { const envFileContent = fs.readFileSync(envFilePath); const parsedEnv = dotenv.parse(envFileContent); if (parsedEnv['GOOGLE_CLOUD_PROJECT']) { // .env file takes precedence in Cloud Shell - process.env['GOOGLE_CLOUD_PROJECT'] = parsedEnv['GOOGLE_CLOUD_PROJECT']; - } else { - // If not in .env, set to default and override global - process.env['GOOGLE_CLOUD_PROJECT'] = 'cloudshell-gca'; + value = parsedEnv['GOOGLE_CLOUD_PROJECT']; + if (!isTrusted && isSandboxed) { + value = sanitizeEnvVar(value); + } } - } else { - // If no .env file, set to default and override global - process.env['GOOGLE_CLOUD_PROJECT'] = 'cloudshell-gca'; } + process.env['GOOGLE_CLOUD_PROJECT'] = value; } export function loadEnvironment( @@ -469,13 +488,29 @@ export function loadEnvironment( const envFilePath = findEnvFile(workspaceDir); const trustResult = isWorkspaceTrustedFn(settings, workspaceDir); - if (trustResult.isTrusted !== true) { + const isTrusted = trustResult.isTrusted ?? false; + // Check settings OR check process.argv directly since this might be called + // before arguments are fully parsed. This is a best-effort sniffing approach + // that happens early in the CLI lifecycle. It is designed to detect the + // sandbox flag before the full command-line parser is initialized to ensure + // security constraints are applied when loading environment variables. + const args = process.argv.slice(2); + const doubleDashIndex = args.indexOf('--'); + const relevantArgs = + doubleDashIndex === -1 ? args : args.slice(0, doubleDashIndex); + + const isSandboxed = + !!settings.tools?.sandbox || + relevantArgs.includes('-s') || + relevantArgs.includes('--sandbox'); + + if (trustResult.isTrusted !== true && !isSandboxed) { return; } // Cloud Shell environment variable handling if (process.env['CLOUD_SHELL'] === 'true') { - setUpCloudShellEnvironment(envFilePath); + setUpCloudShellEnvironment(envFilePath, isTrusted, isSandboxed); } if (envFilePath) { @@ -491,6 +526,16 @@ export function loadEnvironment( for (const key in parsedEnv) { if (Object.hasOwn(parsedEnv, key)) { + let value = parsedEnv[key]; + // If the workspace is untrusted but we are sandboxed, only allow whitelisted variables. + if (!isTrusted && isSandboxed) { + if (!AUTH_ENV_VAR_WHITELIST.includes(key)) { + continue; + } + // Sanitize the value for untrusted sources + value = sanitizeEnvVar(value); + } + // If it's a project .env file, skip loading excluded variables. if (isProjectEnvFile && excludedVars.includes(key)) { continue; @@ -498,7 +543,7 @@ export function loadEnvironment( // Load variable only if it's not already set in the environment. if (!Object.hasOwn(process.env, key)) { - process.env[key] = parsedEnv[key]; + process.env[key] = value; } } } diff --git a/packages/cli/src/config/settingsSchema.test.ts b/packages/cli/src/config/settingsSchema.test.ts index 6e55082edb..3081ce9a10 100644 --- a/packages/cli/src/config/settingsSchema.test.ts +++ b/packages/cli/src/config/settingsSchema.test.ts @@ -294,7 +294,7 @@ describe('SettingsSchema', () => { expect( getSettingsSchema().security.properties.folderTrust.properties.enabled .default, - ).toBe(false); + ).toBe(true); expect( getSettingsSchema().security.properties.folderTrust.properties.enabled .showInDialog, diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index 63718dad0b..738a49b16b 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -1312,7 +1312,7 @@ const SETTINGS_SCHEMA = { label: 'Folder Trust', category: 'Security', requiresRestart: true, - default: false, + default: true, description: 'Setting to track whether Folder trust is enabled.', showInDialog: true, }, diff --git a/packages/cli/src/config/trustedFolders.test.ts b/packages/cli/src/config/trustedFolders.test.ts index 75acd5097e..c0d7b64cb2 100644 --- a/packages/cli/src/config/trustedFolders.test.ts +++ b/packages/cli/src/config/trustedFolders.test.ts @@ -5,7 +5,11 @@ */ import * as osActual from 'node:os'; -import { FatalConfigError, ideContextStore } from '@google/gemini-cli-core'; +import { + FatalConfigError, + ideContextStore, + AuthType, +} from '@google/gemini-cli-core'; import { describe, it, @@ -26,6 +30,9 @@ import { isWorkspaceTrusted, resetTrustedFoldersForTesting, } from './trustedFolders.js'; +import { loadEnvironment, getSettingsSchema } from './settings.js'; +import { createMockSettings } from '../test-utils/settings.js'; +import { validateAuthMethod } from './auth.js'; import type { Settings } from './settings.js'; vi.mock('os', async (importOriginal) => { @@ -53,7 +60,7 @@ vi.mock('fs', async (importOriginal) => { readFileSync: vi.fn(), writeFileSync: vi.fn(), mkdirSync: vi.fn(), - realpathSync: vi.fn((p) => p), + realpathSync: vi.fn().mockImplementation((p) => p), }; }); vi.mock('strip-json-comments', () => ({ @@ -578,6 +585,62 @@ describe('invalid trust levels', () => { }); }); +describe('Verification: Auth and Trust Interaction', () => { + let mockCwd: string; + const mockRules: Record = {}; + + beforeEach(() => { + vi.stubEnv('GEMINI_API_KEY', ''); + resetTrustedFoldersForTesting(); + vi.spyOn(process, 'cwd').mockImplementation(() => mockCwd); + vi.spyOn(fs, 'readFileSync').mockImplementation((p) => { + if (p === getTrustedFoldersPath()) { + return JSON.stringify(mockRules); + } + if (p === path.resolve(mockCwd, '.env')) { + return 'GEMINI_API_KEY=shhh-secret'; + } + return '{}'; + }); + vi.spyOn(fs, 'existsSync').mockImplementation( + (p) => + p === getTrustedFoldersPath() || p === path.resolve(mockCwd, '.env'), + ); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + Object.keys(mockRules).forEach((key) => delete mockRules[key]); + }); + + it('should verify loadEnvironment returns early and validateAuthMethod fails when untrusted', () => { + // 1. Mock untrusted workspace + mockCwd = '/home/user/untrusted'; + mockRules[mockCwd] = TrustLevel.DO_NOT_TRUST; + + // 2. Load environment (should return early) + const settings = createMockSettings({ + security: { folderTrust: { enabled: true } }, + }); + loadEnvironment(settings.merged, mockCwd); + + // 3. Verify env var NOT loaded + expect(process.env['GEMINI_API_KEY']).toBe(''); + + // 4. Verify validateAuthMethod fails + const result = validateAuthMethod(AuthType.USE_GEMINI); + expect(result).toContain( + 'you must specify the GEMINI_API_KEY environment variable', + ); + }); + + it('should identify if sandbox flag is available in Settings', () => { + const schema = getSettingsSchema(); + expect(schema.tools.properties).toBeDefined(); + expect('sandbox' in schema.tools.properties).toBe(true); + }); +}); + describe('Trusted Folders realpath caching', () => { beforeEach(() => { resetTrustedFoldersForTesting(); diff --git a/packages/cli/src/config/trustedFolders.ts b/packages/cli/src/config/trustedFolders.ts index 5de5b605cd..31827e0cab 100644 --- a/packages/cli/src/config/trustedFolders.ts +++ b/packages/cli/src/config/trustedFolders.ts @@ -250,7 +250,7 @@ export function saveTrustedFolders( /** Is folder trust feature enabled per the current applied settings */ export function isFolderTrustEnabled(settings: Settings): boolean { - const folderTrustSetting = settings.security?.folderTrust?.enabled ?? false; + const folderTrustSetting = settings.security?.folderTrust?.enabled ?? true; return folderTrustSetting; } diff --git a/packages/cli/src/deferred.test.ts b/packages/cli/src/deferred.test.ts index 8b9fb87f7a..08cbb3a093 100644 --- a/packages/cli/src/deferred.test.ts +++ b/packages/cli/src/deferred.test.ts @@ -13,7 +13,7 @@ import { } from './deferred.js'; import { ExitCodes } from '@google/gemini-cli-core'; import type { ArgumentsCamelCase, CommandModule } from 'yargs'; -import type { MergedSettings } from './config/settings.js'; +import { createMockSettings } from './test-utils/settings.js'; import type { MockInstance } from 'vitest'; const { mockRunExitCleanup, mockCoreEvents } = vi.hoisted(() => ({ @@ -46,14 +46,9 @@ describe('deferred', () => { setDeferredCommand(undefined as unknown as DeferredCommand); // Reset deferred command }); - const createMockSettings = (adminSettings: unknown = {}): MergedSettings => - ({ - admin: adminSettings, - }) as unknown as MergedSettings; - describe('runDeferredCommand', () => { it('should do nothing if no deferred command is set', async () => { - await runDeferredCommand(createMockSettings()); + await runDeferredCommand(createMockSettings().merged); expect(mockCoreEvents.emitFeedback).not.toHaveBeenCalled(); expect(mockExit).not.toHaveBeenCalled(); }); @@ -66,7 +61,9 @@ describe('deferred', () => { commandName: 'mcp', }); - const settings = createMockSettings({ mcp: { enabled: true } }); + const settings = createMockSettings({ + merged: { admin: { mcp: { enabled: true } } }, + }).merged; await runDeferredCommand(settings); expect(mockHandler).toHaveBeenCalled(); expect(mockRunExitCleanup).toHaveBeenCalled(); @@ -80,7 +77,9 @@ describe('deferred', () => { commandName: 'mcp', }); - const settings = createMockSettings({ mcp: { enabled: false } }); + const settings = createMockSettings({ + merged: { admin: { mcp: { enabled: false } } }, + }).merged; await runDeferredCommand(settings); expect(mockCoreEvents.emitFeedback).toHaveBeenCalledWith( @@ -98,7 +97,9 @@ describe('deferred', () => { commandName: 'extensions', }); - const settings = createMockSettings({ extensions: { enabled: false } }); + const settings = createMockSettings({ + merged: { admin: { extensions: { enabled: false } } }, + }).merged; await runDeferredCommand(settings); expect(mockCoreEvents.emitFeedback).toHaveBeenCalledWith( @@ -116,7 +117,9 @@ describe('deferred', () => { commandName: 'skills', }); - const settings = createMockSettings({ skills: { enabled: false } }); + const settings = createMockSettings({ + merged: { admin: { skills: { enabled: false } } }, + }).merged; await runDeferredCommand(settings); expect(mockCoreEvents.emitFeedback).toHaveBeenCalledWith( @@ -135,7 +138,7 @@ describe('deferred', () => { commandName: 'mcp', }); - const settings = createMockSettings({}); // No admin settings + const settings = createMockSettings({}).merged; // No admin settings await runDeferredCommand(settings); expect(mockHandler).toHaveBeenCalled(); @@ -163,7 +166,7 @@ describe('deferred', () => { expect(originalHandler).not.toHaveBeenCalled(); // Now manually run it to verify it captured correctly - await runDeferredCommand(createMockSettings()); + await runDeferredCommand(createMockSettings().merged); expect(originalHandler).toHaveBeenCalledWith(argv); expect(mockExit).toHaveBeenCalledWith(ExitCodes.SUCCESS); }); @@ -181,7 +184,9 @@ describe('deferred', () => { const deferredMcp = defer(commandModule, 'mcp'); await deferredMcp.handler({} as ArgumentsCamelCase); - const mcpSettings = createMockSettings({ mcp: { enabled: false } }); + const mcpSettings = createMockSettings({ + merged: { admin: { mcp: { enabled: false } } }, + }).merged; await runDeferredCommand(mcpSettings); expect(mockCoreEvents.emitFeedback).toHaveBeenCalledWith( @@ -205,10 +210,14 @@ describe('deferred', () => { // confirming it didn't capture 'mcp', 'extensions', or 'skills' // and defaulted to 'unknown' (or something else safe). const settings = createMockSettings({ - mcp: { enabled: false }, - extensions: { enabled: false }, - skills: { enabled: false }, - }); + merged: { + admin: { + mcp: { enabled: false }, + extensions: { enabled: false }, + skills: { enabled: false }, + }, + }, + }).merged; await runDeferredCommand(settings); diff --git a/packages/cli/src/test-utils/render.tsx b/packages/cli/src/test-utils/render.tsx index 09decd8f47..e3aeca6e45 100644 --- a/packages/cli/src/test-utils/render.tsx +++ b/packages/cli/src/test-utils/render.tsx @@ -10,7 +10,7 @@ import type React from 'react'; import { vi } from 'vitest'; import { act, useState } from 'react'; import os from 'node:os'; -import { LoadedSettings, type Settings } from '../config/settings.js'; +import { LoadedSettings } from '../config/settings.js'; import { KeypressProvider } from '../ui/contexts/KeypressContext.js'; import { SettingsContext } from '../ui/contexts/SettingsContext.js'; import { ShellFocusContext } from '../ui/contexts/ShellFocusContext.js'; @@ -32,6 +32,7 @@ import { TerminalProvider } from '../ui/contexts/TerminalContext.js'; import { makeFakeConfig, type Config } from '@google/gemini-cli-core'; import { FakePersistentState } from './persistentStateFake.js'; import { AppContext, type AppState } from '../ui/contexts/AppContext.js'; +import { createMockSettings } from './settings.js'; export const persistentStateMock = new FakePersistentState(); @@ -135,20 +136,6 @@ export const mockSettings = new LoadedSettings( [], ); -export const createMockSettings = ( - overrides: Partial, -): LoadedSettings => { - const settings = overrides as Settings; - return new LoadedSettings( - { path: '', settings: {}, originalSettings: {} }, - { path: '', settings: {}, originalSettings: {} }, - { path: '', settings, originalSettings: settings }, - { path: '', settings: {}, originalSettings: {} }, - true, - [], - ); -}; - // A minimal mock UIState to satisfy the context provider. // Tests that need specific UIState values should provide their own. const baseMockUiState = { diff --git a/packages/cli/src/test-utils/settings.ts b/packages/cli/src/test-utils/settings.ts new file mode 100644 index 0000000000..14b93f3578 --- /dev/null +++ b/packages/cli/src/test-utils/settings.ts @@ -0,0 +1,79 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/* eslint-disable @typescript-eslint/no-explicit-any */ + +import { + LoadedSettings, + createTestMergedSettings, + type SettingsError, +} from '../config/settings.js'; + +export interface MockSettingsFile { + settings: any; + originalSettings: any; + path: string; +} + +interface CreateMockSettingsOptions { + system?: MockSettingsFile; + systemDefaults?: MockSettingsFile; + user?: MockSettingsFile; + workspace?: MockSettingsFile; + isTrusted?: boolean; + errors?: SettingsError[]; + merged?: any; + [key: string]: any; +} + +/** + * Creates a mock LoadedSettings object for testing. + * + * @param overrides - Partial settings or LoadedSettings properties to override. + * If 'merged' is provided, it overrides the computed merged settings. + * Any functions in overrides are assigned directly to the LoadedSettings instance. + */ +export const createMockSettings = ( + overrides: CreateMockSettingsOptions = {}, +): LoadedSettings => { + const { + system, + systemDefaults, + user, + workspace, + isTrusted, + errors, + merged: mergedOverride, + ...settingsOverrides + } = overrides; + + const loaded = new LoadedSettings( + (system as any) || { path: '', settings: {}, originalSettings: {} }, + (systemDefaults as any) || { path: '', settings: {}, originalSettings: {} }, + (user as any) || { + path: '', + settings: settingsOverrides, + originalSettings: settingsOverrides, + }, + (workspace as any) || { path: '', settings: {}, originalSettings: {} }, + isTrusted ?? true, + errors || [], + ); + + if (mergedOverride) { + // @ts-expect-error - overriding private field for testing + loaded._merged = createTestMergedSettings(mergedOverride); + } + + // Assign any function overrides (e.g., vi.fn() for methods) + for (const key in overrides) { + if (typeof overrides[key] === 'function') { + (loaded as any)[key] = overrides[key]; + } + } + + return loaded; +}; diff --git a/packages/cli/src/ui/commands/directoryCommand.test.tsx b/packages/cli/src/ui/commands/directoryCommand.test.tsx index 91ace7fca5..d9c534a89e 100644 --- a/packages/cli/src/ui/commands/directoryCommand.test.tsx +++ b/packages/cli/src/ui/commands/directoryCommand.test.tsx @@ -86,6 +86,11 @@ describe('directoryCommand', () => { settings: { merged: { memoryDiscoveryMaxDirs: 1000, + security: { + folderTrust: { + enabled: false, + }, + }, }, }, }, diff --git a/packages/cli/src/ui/components/CliSpinner.test.tsx b/packages/cli/src/ui/components/CliSpinner.test.tsx index 76522c41c1..9f05df3930 100644 --- a/packages/cli/src/ui/components/CliSpinner.test.tsx +++ b/packages/cli/src/ui/components/CliSpinner.test.tsx @@ -4,10 +4,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { - renderWithProviders, - createMockSettings, -} from '../../test-utils/render.js'; +import { renderWithProviders } from '../../test-utils/render.js'; +import { createMockSettings } from '../../test-utils/settings.js'; import { CliSpinner } from './CliSpinner.js'; import { debugState } from '../debug.js'; import { describe, it, expect, beforeEach } from 'vitest'; diff --git a/packages/cli/src/ui/components/Composer.test.tsx b/packages/cli/src/ui/components/Composer.test.tsx index 4e2ad6464f..1d97c978d2 100644 --- a/packages/cli/src/ui/components/Composer.test.tsx +++ b/packages/cli/src/ui/components/Composer.test.tsx @@ -15,6 +15,7 @@ import { } from '../contexts/UIActionsContext.js'; import { ConfigContext } from '../contexts/ConfigContext.js'; import { SettingsContext } from '../contexts/SettingsContext.js'; +import { createMockSettings } from '../../test-utils/settings.js'; // Mock VimModeContext hook vi.mock('../contexts/VimModeContext.js', () => ({ useVimMode: vi.fn(() => ({ @@ -24,7 +25,6 @@ vi.mock('../contexts/VimModeContext.js', () => ({ })); import { ApprovalMode } from '@google/gemini-cli-core'; import { StreamingState } from '../types.js'; -import { mergeSettings } from '../../config/settings.js'; // Mock child components vi.mock('./LoadingIndicator.js', () => ({ @@ -168,21 +168,6 @@ const createMockConfig = (overrides = {}) => ({ ...overrides, }); -const createMockSettings = (merged = {}) => { - const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true); - return { - merged: { - ...defaultMergedSettings, - ui: { - ...defaultMergedSettings.ui, - hideFooter: false, - showMemoryUsage: false, - ...merged, - }, - }, - }; -}; - /* eslint-disable @typescript-eslint/no-explicit-any */ const renderComposer = ( uiState: UIState, @@ -207,7 +192,7 @@ describe('Composer', () => { describe('Footer Display Settings', () => { it('renders Footer by default when hideFooter is false', () => { const uiState = createMockUIState(); - const settings = createMockSettings({ hideFooter: false }); + const settings = createMockSettings({ ui: { hideFooter: false } }); const { lastFrame } = renderComposer(uiState, settings); @@ -216,7 +201,7 @@ describe('Composer', () => { it('does NOT render Footer when hideFooter is true', () => { const uiState = createMockUIState(); - const settings = createMockSettings({ hideFooter: true }); + const settings = createMockSettings({ ui: { hideFooter: true } }); const { lastFrame } = renderComposer(uiState, settings); @@ -245,8 +230,10 @@ describe('Composer', () => { getDebugMode: vi.fn(() => true), }); const settings = createMockSettings({ - hideFooter: false, - showMemoryUsage: true, + ui: { + hideFooter: false, + showMemoryUsage: true, + }, }); // Mock vim mode for this test const { useVimMode } = await import('../contexts/VimModeContext.js'); diff --git a/packages/cli/src/ui/components/FolderTrustDialog.test.tsx b/packages/cli/src/ui/components/FolderTrustDialog.test.tsx index 9ba7f19cb6..0597a8167b 100644 --- a/packages/cli/src/ui/components/FolderTrustDialog.test.tsx +++ b/packages/cli/src/ui/components/FolderTrustDialog.test.tsx @@ -101,9 +101,7 @@ describe('FolderTrustDialog', () => { ); // Unmount immediately (before 250ms) - act(() => { - unmount(); - }); + unmount(); await vi.advanceTimersByTimeAsync(250); expect(relaunchApp).not.toHaveBeenCalled(); diff --git a/packages/cli/src/ui/components/Footer.test.tsx b/packages/cli/src/ui/components/Footer.test.tsx index ed8ab8307f..4113060081 100644 --- a/packages/cli/src/ui/components/Footer.test.tsx +++ b/packages/cli/src/ui/components/Footer.test.tsx @@ -5,10 +5,8 @@ */ import { describe, it, expect, vi } from 'vitest'; -import { - renderWithProviders, - createMockSettings, -} from '../../test-utils/render.js'; +import { renderWithProviders } from '../../test-utils/render.js'; +import { createMockSettings } from '../../test-utils/settings.js'; import { Footer } from './Footer.js'; import { tildeifyPath, ToolCallDecision } from '@google/gemini-cli-core'; import type { SessionStatsState } from '../contexts/SessionContext.js'; diff --git a/packages/cli/src/ui/components/InputPrompt.test.tsx b/packages/cli/src/ui/components/InputPrompt.test.tsx index 2dcf9a0d32..56abf21927 100644 --- a/packages/cli/src/ui/components/InputPrompt.test.tsx +++ b/packages/cli/src/ui/components/InputPrompt.test.tsx @@ -4,10 +4,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { - renderWithProviders, - createMockSettings, -} from '../../test-utils/render.js'; +import { renderWithProviders } from '../../test-utils/render.js'; +import { createMockSettings } from '../../test-utils/settings.js'; import { waitFor } from '../../test-utils/async.js'; import { act, useState } from 'react'; import type { InputPromptProps } from './InputPrompt.js'; diff --git a/packages/cli/src/ui/components/SettingsDialog.test.tsx b/packages/cli/src/ui/components/SettingsDialog.test.tsx index ec8d8b55b4..ba135499ef 100644 --- a/packages/cli/src/ui/components/SettingsDialog.test.tsx +++ b/packages/cli/src/ui/components/SettingsDialog.test.tsx @@ -26,6 +26,7 @@ import { waitFor } from '../../test-utils/async.js'; import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { SettingsDialog } from './SettingsDialog.js'; import { LoadedSettings, SettingScope } from '../../config/settings.js'; +import { createMockSettings } from '../../test-utils/settings.js'; import { VimModeProvider } from '../contexts/VimModeContext.js'; import { KeypressProvider } from '../contexts/KeypressContext.js'; import { act } from 'react'; @@ -58,56 +59,6 @@ enum TerminalKeys { BACKSPACE = '\u0008', } -const createMockSettings = ( - userSettings = {}, - systemSettings = {}, - workspaceSettings = {}, -) => - new LoadedSettings( - { - settings: { ui: { customThemes: {} }, mcpServers: {}, ...systemSettings }, - originalSettings: { - ui: { customThemes: {} }, - mcpServers: {}, - ...systemSettings, - }, - path: '/system/settings.json', - }, - { - settings: {}, - originalSettings: {}, - path: '/system/system-defaults.json', - }, - { - settings: { - ui: { customThemes: {} }, - mcpServers: {}, - ...userSettings, - }, - originalSettings: { - ui: { customThemes: {} }, - mcpServers: {}, - ...userSettings, - }, - path: '/user/settings.json', - }, - { - settings: { - ui: { customThemes: {} }, - mcpServers: {}, - ...workspaceSettings, - }, - originalSettings: { - ui: { customThemes: {} }, - mcpServers: {}, - ...workspaceSettings, - }, - path: '/workspace/settings.json', - }, - true, - [], - ); - vi.mock('../../config/settingsSchema.js', async (importOriginal) => { const original = await importOriginal(); @@ -639,11 +590,23 @@ describe('SettingsDialog', () => { }); it('should show different values for different scopes', () => { - const settings = createMockSettings( - { vimMode: true }, // User settings - { vimMode: false }, // System settings - { autoUpdate: false }, // Workspace settings - ); + const settings = createMockSettings({ + user: { + settings: { vimMode: true }, + originalSettings: { vimMode: true }, + path: '', + }, + system: { + settings: { vimMode: false }, + originalSettings: { vimMode: false }, + path: '', + }, + workspace: { + settings: { autoUpdate: false }, + originalSettings: { autoUpdate: false }, + path: '', + }, + }); const onSelect = vi.fn(); const { lastFrame } = renderDialog(settings, onSelect); @@ -733,11 +696,23 @@ describe('SettingsDialog', () => { describe('Specific Settings Behavior', () => { it('should show correct display values for settings with different states', () => { - const settings = createMockSettings( - { vimMode: true, hideTips: false }, // User settings - { hideWindowTitle: true }, // System settings - { ideMode: false }, // Workspace settings - ); + const settings = createMockSettings({ + user: { + settings: { vimMode: true, hideTips: false }, + originalSettings: { vimMode: true, hideTips: false }, + path: '', + }, + system: { + settings: { hideWindowTitle: true }, + originalSettings: { hideWindowTitle: true }, + path: '', + }, + workspace: { + settings: { ideMode: false }, + originalSettings: { ideMode: false }, + path: '', + }, + }); const onSelect = vi.fn(); const { lastFrame } = renderDialog(settings, onSelect); @@ -794,11 +769,13 @@ describe('SettingsDialog', () => { describe('Settings Display Values', () => { it('should show correct values for inherited settings', () => { - const settings = createMockSettings( - {}, - { vimMode: true, hideWindowTitle: false }, // System settings - {}, - ); + const settings = createMockSettings({ + system: { + settings: { vimMode: true, hideWindowTitle: false }, + originalSettings: { vimMode: true, hideWindowTitle: false }, + path: '', + }, + }); const onSelect = vi.fn(); const { lastFrame } = renderDialog(settings, onSelect); @@ -809,11 +786,18 @@ describe('SettingsDialog', () => { }); it('should show override indicator for overridden settings', () => { - const settings = createMockSettings( - { vimMode: false }, // User overrides - { vimMode: true }, // System default - {}, - ); + const settings = createMockSettings({ + user: { + settings: { vimMode: false }, + originalSettings: { vimMode: false }, + path: '', + }, + system: { + settings: { vimMode: true }, + originalSettings: { vimMode: true }, + path: '', + }, + }); const onSelect = vi.fn(); const { lastFrame } = renderDialog(settings, onSelect); @@ -983,11 +967,13 @@ describe('SettingsDialog', () => { describe('Error Recovery', () => { it('should handle malformed settings gracefully', () => { // Create settings with potentially problematic values - const settings = createMockSettings( - { vimMode: null as unknown as boolean }, // Invalid value - {}, - {}, - ); + const settings = createMockSettings({ + user: { + settings: { vimMode: null as unknown as boolean }, + originalSettings: { vimMode: null as unknown as boolean }, + path: '', + }, + }); const onSelect = vi.fn(); const { lastFrame } = renderDialog(settings, onSelect); @@ -1198,11 +1184,13 @@ describe('SettingsDialog', () => { stdin.write('\r'); // Commit }); - settings = createMockSettings( - { 'a.string.setting': 'new value' }, - {}, - {}, - ); + settings = createMockSettings({ + user: { + settings: { 'a.string.setting': 'new value' }, + originalSettings: { 'a.string.setting': 'new value' }, + path: '', + }, + }); rerender( @@ -1550,11 +1538,23 @@ describe('SettingsDialog', () => { ])( 'should render $name correctly', ({ userSettings, systemSettings, workspaceSettings, stdinActions }) => { - const settings = createMockSettings( - userSettings, - systemSettings, - workspaceSettings, - ); + const settings = createMockSettings({ + user: { + settings: userSettings, + originalSettings: userSettings, + path: '', + }, + system: { + settings: systemSettings, + originalSettings: systemSettings, + path: '', + }, + workspace: { + settings: workspaceSettings, + originalSettings: workspaceSettings, + path: '', + }, + }); const onSelect = vi.fn(); const { lastFrame, stdin } = renderDialog(settings, onSelect); diff --git a/packages/cli/src/ui/components/StatusDisplay.test.tsx b/packages/cli/src/ui/components/StatusDisplay.test.tsx index df4bcd4b0f..e7f3e1fff9 100644 --- a/packages/cli/src/ui/components/StatusDisplay.test.tsx +++ b/packages/cli/src/ui/components/StatusDisplay.test.tsx @@ -11,6 +11,7 @@ import { StatusDisplay } from './StatusDisplay.js'; import { UIStateContext, type UIState } from '../contexts/UIStateContext.js'; import { ConfigContext } from '../contexts/ConfigContext.js'; import { SettingsContext } from '../contexts/SettingsContext.js'; +import { createMockSettings } from '../../test-utils/settings.js'; import type { TextBuffer } from './shared/text-buffer.js'; // Mock child components to simplify testing @@ -65,14 +66,6 @@ const createMockConfig = (overrides = {}) => ({ ...overrides, }); -const createMockSettings = (merged = {}) => ({ - merged: { - hooksConfig: { notifications: true }, - ui: { hideContextSummary: false }, - ...merged, - }, -}); - /* eslint-disable @typescript-eslint/no-explicit-any */ const renderStatusDisplay = ( props: { hideContextSummary: boolean } = { hideContextSummary: false }, diff --git a/packages/cli/src/ui/components/ThemeDialog.test.tsx b/packages/cli/src/ui/components/ThemeDialog.test.tsx index bcfeb5a9c9..165d4a52a2 100644 --- a/packages/cli/src/ui/components/ThemeDialog.test.tsx +++ b/packages/cli/src/ui/components/ThemeDialog.test.tsx @@ -8,52 +8,10 @@ import { renderWithProviders } from '../../test-utils/render.js'; import { waitFor } from '../../test-utils/async.js'; import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { ThemeDialog } from './ThemeDialog.js'; -import { LoadedSettings } from '../../config/settings.js'; +import { createMockSettings } from '../../test-utils/settings.js'; import { DEFAULT_THEME, themeManager } from '../themes/theme-manager.js'; import { act } from 'react'; -const createMockSettings = ( - userSettings = {}, - workspaceSettings = {}, - systemSettings = {}, -): LoadedSettings => - new LoadedSettings( - { - settings: { ui: { customThemes: {} }, ...systemSettings }, - originalSettings: { ui: { customThemes: {} }, ...systemSettings }, - path: '/system/settings.json', - }, - { - settings: {}, - originalSettings: {}, - path: '/system/system-defaults.json', - }, - { - settings: { - ui: { customThemes: {} }, - ...userSettings, - }, - originalSettings: { - ui: { customThemes: {} }, - ...userSettings, - }, - path: '/user/settings.json', - }, - { - settings: { - ui: { customThemes: {} }, - ...workspaceSettings, - }, - originalSettings: { - ui: { customThemes: {} }, - ...workspaceSettings, - }, - path: '/workspace/settings.json', - }, - true, - [], - ); - describe('ThemeDialog Snapshots', () => { const baseProps = { onSelect: vi.fn(), diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx index 9489ad1d23..283a24843f 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx @@ -10,10 +10,8 @@ import type { ToolCallConfirmationDetails, Config, } from '@google/gemini-cli-core'; -import { - renderWithProviders, - createMockSettings, -} from '../../../test-utils/render.js'; +import { renderWithProviders } from '../../../test-utils/render.js'; +import { createMockSettings } from '../../../test-utils/settings.js'; import { useToolActions } from '../../contexts/ToolActionsContext.js'; vi.mock('../../contexts/ToolActionsContext.js', async (importOriginal) => { diff --git a/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx b/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx index 2bda2d5b4e..28475b52c6 100644 --- a/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx @@ -4,10 +4,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { - renderWithProviders, - createMockSettings, -} from '../../../test-utils/render.js'; +import { renderWithProviders } from '../../../test-utils/render.js'; +import { createMockSettings } from '../../../test-utils/settings.js'; import { describe, it, expect, vi, afterEach } from 'vitest'; import { ToolGroupMessage } from './ToolGroupMessage.js'; import type { IndividualToolCallDisplay } from '../../types.js'; diff --git a/packages/cli/src/ui/hooks/useFolderTrust.ts b/packages/cli/src/ui/hooks/useFolderTrust.ts index 7bc313f97c..05915b8f43 100644 --- a/packages/cli/src/ui/hooks/useFolderTrust.ts +++ b/packages/cli/src/ui/hooks/useFolderTrust.ts @@ -27,7 +27,7 @@ export const useFolderTrust = ( const [isRestarting, setIsRestarting] = useState(false); const startupMessageSent = useRef(false); - const folderTrust = settings.merged.security.folderTrust.enabled; + const folderTrust = settings.merged.security.folderTrust.enabled ?? true; useEffect(() => { const { isTrusted: trusted } = isWorkspaceTrusted(settings.merged); diff --git a/packages/cli/src/ui/hooks/usePermissionsModifyTrust.ts b/packages/cli/src/ui/hooks/usePermissionsModifyTrust.ts index 7c3fbd8616..6503332350 100644 --- a/packages/cli/src/ui/hooks/usePermissionsModifyTrust.ts +++ b/packages/cli/src/ui/hooks/usePermissionsModifyTrust.ts @@ -88,7 +88,8 @@ export const usePermissionsModifyTrust = ( ); const [needsRestart, setNeedsRestart] = useState(false); - const isFolderTrustEnabled = !!settings.merged.security.folderTrust.enabled; + const isFolderTrustEnabled = + settings.merged.security.folderTrust.enabled ?? true; const updateTrustLevel = useCallback( (trustLevel: TrustLevel) => { diff --git a/packages/test-utils/src/test-rig.ts b/packages/test-utils/src/test-rig.ts index 1401304560..99f22817c2 100644 --- a/packages/test-utils/src/test-rig.ts +++ b/packages/test-utils/src/test-rig.ts @@ -272,6 +272,27 @@ export class InteractiveRun { } } +function isObject(item: any): item is Record { + return !!(item && typeof item === 'object' && !Array.isArray(item)); +} + +function deepMerge(target: any, source: any): any { + if (!isObject(target) || !isObject(source)) { + return source; + } + const output = { ...target }; + Object.keys(source).forEach((key) => { + const targetValue = target[key]; + const sourceValue = source[key]; + if (isObject(targetValue) && isObject(sourceValue)) { + output[key] = deepMerge(targetValue, sourceValue); + } else { + output[key] = sourceValue; + } + }); + return output; +} + export class TestRig { testDir: string | null = null; homeDir: string | null = null; @@ -316,44 +337,56 @@ export class TestRig { const projectGeminiDir = join(this.testDir!, GEMINI_DIR); mkdirSync(projectGeminiDir, { recursive: true }); + const userGeminiDir = join(this.homeDir!, GEMINI_DIR); + mkdirSync(userGeminiDir, { recursive: true }); + // In sandbox mode, use an absolute path for telemetry inside the container // The container mounts the test directory at the same path as the host const telemetryPath = join(this.homeDir!, 'telemetry.log'); // Always use home directory for telemetry - const settings = { - general: { - // Nightly releases sometimes becomes out of sync with local code and - // triggers auto-update, which causes tests to fail. - disableAutoUpdate: true, - previewFeatures: false, - }, - telemetry: { - enabled: true, - target: 'local', - otlpEndpoint: '', - outfile: telemetryPath, - }, - security: { - auth: { - selectedType: 'gemini-api-key', + const settings = deepMerge( + { + general: { + // Nightly releases sometimes becomes out of sync with local code and + // triggers auto-update, which causes tests to fail. + disableAutoUpdate: true, + previewFeatures: false, }, + telemetry: { + enabled: true, + target: 'local', + otlpEndpoint: '', + outfile: telemetryPath, + }, + security: { + auth: { + selectedType: 'gemini-api-key', + }, + folderTrust: { + enabled: false, + }, + }, + ui: { + useAlternateBuffer: true, + }, + model: { + name: DEFAULT_GEMINI_MODEL, + }, + sandbox: + env['GEMINI_SANDBOX'] !== 'false' ? env['GEMINI_SANDBOX'] : false, + // Don't show the IDE connection dialog when running from VsCode + ide: { enabled: false, hasSeenNudge: true }, }, - ui: { - useAlternateBuffer: true, - }, - model: { - name: DEFAULT_GEMINI_MODEL, - }, - sandbox: - env['GEMINI_SANDBOX'] !== 'false' ? env['GEMINI_SANDBOX'] : false, - // Don't show the IDE connection dialog when running from VsCode - ide: { enabled: false, hasSeenNudge: true }, - ...overrideSettings, // Allow tests to override/add settings - }; + overrideSettings ?? {}, + ); writeFileSync( join(projectGeminiDir, 'settings.json'), JSON.stringify(settings, null, 2), ); + writeFileSync( + join(userGeminiDir, 'settings.json'), + JSON.stringify(settings, null, 2), + ); } createFile(fileName: string, content: string) { diff --git a/schemas/settings.schema.json b/schemas/settings.schema.json index 23aa7e1de0..8bdc9e1bd7 100644 --- a/schemas/settings.schema.json +++ b/schemas/settings.schema.json @@ -1307,8 +1307,8 @@ "enabled": { "title": "Folder Trust", "description": "Setting to track whether Folder trust is enabled.", - "markdownDescription": "Setting to track whether Folder trust is enabled.\n\n- Category: `Security`\n- Requires restart: `yes`\n- Default: `false`", - "default": false, + "markdownDescription": "Setting to track whether Folder trust is enabled.\n\n- Category: `Security`\n- Requires restart: `yes`\n- Default: `true`", + "default": true, "type": "boolean" } },