diff --git a/packages/cli/src/commands/mcp/remove.test.ts b/packages/cli/src/commands/mcp/remove.test.ts index 021b9c12d6..ef8f35f096 100644 --- a/packages/cli/src/commands/mcp/remove.test.ts +++ b/packages/cli/src/commands/mcp/remove.test.ts @@ -21,6 +21,17 @@ import * as path from 'node:path'; import * as os from 'node:os'; import { GEMINI_DIR, debugLogger } from '@google/gemini-cli-core'; +vi.mock('fs', async (importOriginal) => { + const actualFs = await importOriginal(); + return { + ...actualFs, + existsSync: vi.fn(actualFs.existsSync), + readFileSync: vi.fn(actualFs.readFileSync), + writeFileSync: vi.fn(actualFs.writeFileSync), + mkdirSync: vi.fn(actualFs.mkdirSync), + }; +}); + vi.mock('fs/promises', () => ({ readFile: vi.fn(), writeFile: vi.fn(), @@ -30,6 +41,14 @@ vi.mock('../utils.js', () => ({ exitCli: vi.fn(), })); +vi.mock('../../config/trustedFolders.js', () => ({ + isWorkspaceTrusted: vi.fn(() => ({ + isTrusted: true, + source: undefined, + })), + isFolderTrustEnabled: vi.fn(() => false), +})); + describe('mcp remove command', () => { describe('unit tests with mocks', () => { let parser: Argv; diff --git a/packages/cli/src/config/settings.test.ts b/packages/cli/src/config/settings.test.ts index 449a5e0b0b..f78790c3a1 100644 --- a/packages/cli/src/config/settings.test.ts +++ b/packages/cli/src/config/settings.test.ts @@ -105,7 +105,7 @@ vi.mock('fs', async (importOriginal) => { readFileSync: vi.fn(), writeFileSync: vi.fn(), mkdirSync: vi.fn(), - realpathSync: (p: string) => p, + realpathSync: vi.fn((p: string) => p), }; }); @@ -119,9 +119,11 @@ const mockCoreEvents = vi.hoisted(() => ({ vi.mock('@google/gemini-cli-core', async (importOriginal) => { const actual = await importOriginal(); + const os = await import('node:os'); return { ...actual, coreEvents: mockCoreEvents, + homedir: vi.fn(() => os.homedir()), }; }); @@ -1460,6 +1462,44 @@ describe('Settings Loading and Merging', () => { }); }); }); + + it('should correctly skip workspace-level loading if workspaceDir is a symlink to home', () => { + const mockHomeDir = '/mock/home/user'; + const mockSymlinkDir = '/mock/symlink/to/home'; + const mockWorkspaceSettingsPath = path.join( + mockSymlinkDir, + GEMINI_DIR, + 'settings.json', + ); + + vi.mocked(osActual.homedir).mockReturnValue(mockHomeDir); + vi.mocked(fs.realpathSync).mockImplementation((p: fs.PathLike) => { + const pStr = p.toString(); + const resolved = path.resolve(pStr); + if ( + resolved === path.resolve(mockSymlinkDir) || + resolved === path.resolve(mockHomeDir) + ) { + return mockHomeDir; + } + return pStr; + }); + + (mockFsExistsSync as Mock).mockImplementation( + (p: string) => + // Only return true for workspace settings path to see if it gets loaded + p === mockWorkspaceSettingsPath, + ); + + const settings = loadSettings(mockSymlinkDir); + + // Verify that even though the file exists, it was NOT loaded because realpath matched home + expect(fs.readFileSync).not.toHaveBeenCalledWith( + mockWorkspaceSettingsPath, + 'utf-8', + ); + expect(settings.workspace.settings).toEqual({}); + }); }); describe('excludedProjectEnvVars integration', () => { @@ -2373,3 +2413,119 @@ describe('Settings Loading and Merging', () => { }); }); }); + +describe('LoadedSettings Isolation and Serializability', () => { + let loadedSettings: LoadedSettings; + + interface TestData { + a: { + b: number; + }; + } + + beforeEach(() => { + vi.resetAllMocks(); + + // Create a minimal LoadedSettings instance + const emptyScope = { + path: '/mock/settings.json', + settings: {}, + originalSettings: {}, + } as unknown as SettingsFile; + + loadedSettings = new LoadedSettings( + emptyScope, // system + emptyScope, // systemDefaults + { ...emptyScope }, // user + emptyScope, // workspace + true, // isTrusted + ); + }); + + describe('setValue Isolation', () => { + it('should isolate state between settings and originalSettings', () => { + const complexValue: TestData = { a: { b: 1 } }; + loadedSettings.setValue(SettingScope.User, 'test', complexValue); + + const userSettings = loadedSettings.forScope(SettingScope.User); + const settingsValue = (userSettings.settings as Record)[ + 'test' + ] as TestData; + const originalValue = ( + userSettings.originalSettings as Record + )['test'] as TestData; + + // Verify they are equal but different references + expect(settingsValue).toEqual(complexValue); + expect(originalValue).toEqual(complexValue); + expect(settingsValue).not.toBe(complexValue); + expect(originalValue).not.toBe(complexValue); + expect(settingsValue).not.toBe(originalValue); + + // Modify the in-memory setting object + settingsValue.a.b = 2; + + // originalSettings should NOT be affected + expect(originalValue.a.b).toBe(1); + }); + + it('should not share references between settings and originalSettings (original servers test)', () => { + const mcpServers = { + 'test-server': { command: 'echo' }, + }; + + loadedSettings.setValue(SettingScope.User, 'mcpServers', mcpServers); + + // Modify the original object + delete (mcpServers as Record)['test-server']; + + // The settings in LoadedSettings should still have the server + const userSettings = loadedSettings.forScope(SettingScope.User); + expect( + (userSettings.settings.mcpServers as Record)[ + 'test-server' + ], + ).toBeDefined(); + expect( + (userSettings.originalSettings.mcpServers as Record)[ + 'test-server' + ], + ).toBeDefined(); + + // They should also be different objects from each other + expect(userSettings.settings.mcpServers).not.toBe( + userSettings.originalSettings.mcpServers, + ); + }); + }); + + describe('setValue Serializability', () => { + it('should preserve Map/Set types (via structuredClone)', () => { + const mapValue = { myMap: new Map([['key', 'value']]) }; + loadedSettings.setValue(SettingScope.User, 'test', mapValue); + + const userSettings = loadedSettings.forScope(SettingScope.User); + const settingsValue = (userSettings.settings as Record)[ + 'test' + ] as { myMap: Map }; + + // Map is preserved by structuredClone + expect(settingsValue.myMap).toBeInstanceOf(Map); + expect(settingsValue.myMap.get('key')).toBe('value'); + + // But it should be a different reference + expect(settingsValue.myMap).not.toBe(mapValue.myMap); + }); + + it('should handle circular references (structuredClone supports them, but deepMerge may not)', () => { + const circular: Record = { a: 1 }; + circular['self'] = circular; + + // structuredClone(circular) works, but LoadedSettings.setValue calls + // computeMergedSettings() -> customDeepMerge() which blows up on circularity. + expect(() => { + loadedSettings.setValue(SettingScope.User, 'test', circular); + }).toThrow(/Maximum call stack size exceeded/); + }); + }); +}); diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts index 90cef7c7fe..ee8c1a1fbe 100644 --- a/packages/cli/src/config/settings.ts +++ b/packages/cli/src/config/settings.ts @@ -277,8 +277,11 @@ export class LoadedSettings { this.system = system; this.systemDefaults = systemDefaults; this.user = user; - this.workspace = workspace; + this._workspaceFile = workspace; this.isTrusted = isTrusted; + this.workspace = isTrusted + ? workspace + : this.createEmptyWorkspace(workspace); this.errors = errors; this._merged = this.computeMergedSettings(); } @@ -286,10 +289,11 @@ export class LoadedSettings { readonly system: SettingsFile; readonly systemDefaults: SettingsFile; readonly user: SettingsFile; - readonly workspace: SettingsFile; - readonly isTrusted: boolean; + workspace: SettingsFile; + isTrusted: boolean; readonly errors: SettingsError[]; + private _workspaceFile: SettingsFile; private _merged: MergedSettings; private _remoteAdminSettings: Partial | undefined; @@ -297,6 +301,26 @@ export class LoadedSettings { return this._merged; } + setTrusted(isTrusted: boolean): void { + if (this.isTrusted === isTrusted) { + return; + } + this.isTrusted = isTrusted; + this.workspace = isTrusted + ? this._workspaceFile + : this.createEmptyWorkspace(this._workspaceFile); + this._merged = this.computeMergedSettings(); + coreEvents.emitSettingsChanged(); + } + + private createEmptyWorkspace(workspace: SettingsFile): SettingsFile { + return { + ...workspace, + settings: {}, + originalSettings: {}, + }; + } + private computeMergedSettings(): MergedSettings { const merged = mergeSettings( this.system.settings, @@ -341,8 +365,21 @@ export class LoadedSettings { setValue(scope: LoadableSettingScope, key: string, value: unknown): void { const settingsFile = this.forScope(scope); - setNestedProperty(settingsFile.settings, key, value); - setNestedProperty(settingsFile.originalSettings, key, value); + + // Clone value to prevent reference sharing between settings and originalSettings + const valueToSet = + typeof value === 'object' && value !== null + ? structuredClone(value) + : value; + + setNestedProperty(settingsFile.settings, key, valueToSet); + // Use a fresh clone for originalSettings to ensure total independence + setNestedProperty( + settingsFile.originalSettings, + key, + structuredClone(valueToSet), + ); + this._merged = this.computeMergedSettings(); saveSettings(settingsFile); coreEvents.emitSettingsChanged(); @@ -592,9 +629,10 @@ export function loadSettings( // For the initial trust check, we can only use user and system settings. const initialTrustCheckSettings = customDeepMerge( getMergeStrategyForPath, - {}, - systemSettings, + getDefaultsFromSchema(), + systemDefaultSettings, userSettings, + systemSettings, ); const isTrusted = isWorkspaceTrusted(initialTrustCheckSettings as Settings, workspaceDir) @@ -672,57 +710,55 @@ export function migrateDeprecatedSettings( removeDeprecated = false, ): boolean { let anyModified = false; + + const migrateBoolean = ( + settings: Record, + oldKey: string, + newKey: string, + ): boolean => { + let modified = false; + const oldValue = settings[oldKey]; + const newValue = settings[newKey]; + + if (typeof oldValue === 'boolean') { + if (typeof newValue === 'boolean') { + // Both exist, trust the new one + if (removeDeprecated) { + delete settings[oldKey]; + modified = true; + } + } else { + // Only old exists, migrate to new (inverted) + settings[newKey] = !oldValue; + if (removeDeprecated) { + delete settings[oldKey]; + } + modified = true; + } + } + return modified; + }; + const processScope = (scope: LoadableSettingScope) => { const settings = loadedSettings.forScope(scope).settings; - // Migrate inverted boolean settings (disableX -> enableX) - // These settings were renamed and their boolean logic inverted + // Migrate general settings const generalSettings = settings.general as | Record | undefined; - const uiSettings = settings.ui as Record | undefined; - const contextSettings = settings.context as - | Record - | undefined; - - // Migrate general settings (disableAutoUpdate, disableUpdateNag) if (generalSettings) { - const newGeneral: Record = { ...generalSettings }; + const newGeneral = { ...generalSettings }; let modified = false; - if (typeof newGeneral['disableAutoUpdate'] === 'boolean') { - if (typeof newGeneral['enableAutoUpdate'] === 'boolean') { - // Both exist, trust the new one - if (removeDeprecated) { - delete newGeneral['disableAutoUpdate']; - modified = true; - } - } else { - const oldValue = newGeneral['disableAutoUpdate']; - newGeneral['enableAutoUpdate'] = !oldValue; - if (removeDeprecated) { - delete newGeneral['disableAutoUpdate']; - } - modified = true; - } - } - - if (typeof newGeneral['disableUpdateNag'] === 'boolean') { - if (typeof newGeneral['enableAutoUpdateNotification'] === 'boolean') { - // Both exist, trust the new one - if (removeDeprecated) { - delete newGeneral['disableUpdateNag']; - modified = true; - } - } else { - const oldValue = newGeneral['disableUpdateNag']; - newGeneral['enableAutoUpdateNotification'] = !oldValue; - if (removeDeprecated) { - delete newGeneral['disableUpdateNag']; - } - modified = true; - } - } + modified = + migrateBoolean(newGeneral, 'disableAutoUpdate', 'enableAutoUpdate') || + modified; + modified = + migrateBoolean( + newGeneral, + 'disableUpdateNag', + 'enableAutoUpdateNotification', + ) || modified; if (modified) { loadedSettings.setValue(scope, 'general', newGeneral); @@ -731,94 +767,63 @@ export function migrateDeprecatedSettings( } // Migrate ui settings + const uiSettings = settings.ui as Record | undefined; if (uiSettings) { - const newUi: Record = { ...uiSettings }; - let modified = false; - - // Migrate ui.accessibility.disableLoadingPhrases -> ui.accessibility.enableLoadingPhrases + const newUi = { ...uiSettings }; const accessibilitySettings = newUi['accessibility'] as | Record | undefined; - if ( - accessibilitySettings && - typeof accessibilitySettings['disableLoadingPhrases'] === 'boolean' - ) { - const newAccessibility: Record = { - ...accessibilitySettings, - }; - if ( - typeof accessibilitySettings['enableLoadingPhrases'] === 'boolean' - ) { - // Both exist, trust the new one - if (removeDeprecated) { - delete newAccessibility['disableLoadingPhrases']; - newUi['accessibility'] = newAccessibility; - modified = true; - } - } else { - const oldValue = accessibilitySettings['disableLoadingPhrases']; - newAccessibility['enableLoadingPhrases'] = !oldValue; - if (removeDeprecated) { - delete newAccessibility['disableLoadingPhrases']; - } - newUi['accessibility'] = newAccessibility; - modified = true; - } - } - if (modified) { - loadedSettings.setValue(scope, 'ui', newUi); - anyModified = true; + if (accessibilitySettings) { + const newAccessibility = { ...accessibilitySettings }; + if ( + migrateBoolean( + newAccessibility, + 'disableLoadingPhrases', + 'enableLoadingPhrases', + ) + ) { + newUi['accessibility'] = newAccessibility; + loadedSettings.setValue(scope, 'ui', newUi); + anyModified = true; + } } } // Migrate context settings + const contextSettings = settings.context as + | Record + | undefined; if (contextSettings) { - const newContext: Record = { ...contextSettings }; - let modified = false; - - // Migrate context.fileFiltering.disableFuzzySearch -> context.fileFiltering.enableFuzzySearch + const newContext = { ...contextSettings }; const fileFilteringSettings = newContext['fileFiltering'] as | Record | undefined; - if ( - fileFilteringSettings && - typeof fileFilteringSettings['disableFuzzySearch'] === 'boolean' - ) { - const newFileFiltering: Record = { - ...fileFilteringSettings, - }; - if (typeof fileFilteringSettings['enableFuzzySearch'] === 'boolean') { - // Both exist, trust the new one - if (removeDeprecated) { - delete newFileFiltering['disableFuzzySearch']; - newContext['fileFiltering'] = newFileFiltering; - modified = true; - } - } else { - const oldValue = fileFilteringSettings['disableFuzzySearch']; - newFileFiltering['enableFuzzySearch'] = !oldValue; - if (removeDeprecated) { - delete newFileFiltering['disableFuzzySearch']; - } - newContext['fileFiltering'] = newFileFiltering; - modified = true; - } - } - if (modified) { - loadedSettings.setValue(scope, 'context', newContext); - anyModified = true; + if (fileFilteringSettings) { + const newFileFiltering = { ...fileFilteringSettings }; + if ( + migrateBoolean( + newFileFiltering, + 'disableFuzzySearch', + 'enableFuzzySearch', + ) + ) { + newContext['fileFiltering'] = newFileFiltering; + loadedSettings.setValue(scope, 'context', newContext); + anyModified = true; + } } } // Migrate experimental agent settings - anyModified ||= migrateExperimentalSettings( - settings, - loadedSettings, - scope, - removeDeprecated, - ); + anyModified = + migrateExperimentalSettings( + settings, + loadedSettings, + scope, + removeDeprecated, + ) || anyModified; }; processScope(SettingScope.User); diff --git a/packages/cli/src/config/trustedFolders.test.ts b/packages/cli/src/config/trustedFolders.test.ts index 1c5268167c..75acd5097e 100644 --- a/packages/cli/src/config/trustedFolders.test.ts +++ b/packages/cli/src/config/trustedFolders.test.ts @@ -53,6 +53,7 @@ vi.mock('fs', async (importOriginal) => { readFileSync: vi.fn(), writeFileSync: vi.fn(), mkdirSync: vi.fn(), + realpathSync: vi.fn((p) => p), }; }); vi.mock('strip-json-comments', () => ({ @@ -60,22 +61,23 @@ vi.mock('strip-json-comments', () => ({ })); describe('Trusted Folders Loading', () => { - let mockFsExistsSync: Mocked; let mockStripJsonComments: Mocked; let mockFsWriteFileSync: Mocked; beforeEach(() => { resetTrustedFoldersForTesting(); vi.resetAllMocks(); - mockFsExistsSync = vi.mocked(fs.existsSync); mockStripJsonComments = vi.mocked(stripJsonComments); mockFsWriteFileSync = vi.mocked(fs.writeFileSync); vi.mocked(osActual.homedir).mockReturnValue('/mock/home/user'); (mockStripJsonComments as unknown as Mock).mockImplementation( (jsonString: string) => jsonString, ); - (mockFsExistsSync as Mock).mockReturnValue(false); - (fs.readFileSync as Mock).mockReturnValue('{}'); + vi.mocked(fs.existsSync).mockReturnValue(false); + vi.mocked(fs.readFileSync).mockReturnValue('{}'); + vi.mocked(fs.realpathSync).mockImplementation((p: fs.PathLike) => + p.toString(), + ); }); afterEach(() => { @@ -90,13 +92,16 @@ describe('Trusted Folders Loading', () => { describe('isPathTrusted', () => { function setup({ config = {} as Record } = {}) { - (mockFsExistsSync as Mock).mockImplementation( - (p) => p === getTrustedFoldersPath(), + vi.mocked(fs.existsSync).mockImplementation( + (p: fs.PathLike) => p.toString() === getTrustedFoldersPath(), + ); + vi.mocked(fs.readFileSync).mockImplementation( + (p: fs.PathOrFileDescriptor) => { + if (p.toString() === getTrustedFoldersPath()) + return JSON.stringify(config); + return '{}'; + }, ); - (fs.readFileSync as Mock).mockImplementation((p) => { - if (p === getTrustedFoldersPath()) return JSON.stringify(config); - return '{}'; - }); const folders = loadTrustedFolders(); @@ -124,26 +129,62 @@ describe('Trusted Folders Loading', () => { expect(folders.isPathTrusted('/trustedparent/trustme')).toBe(true); // No explicit rule covers this file - expect(folders.isPathTrusted('/secret/bankaccounts.json')).toBe( - undefined, - ); - expect(folders.isPathTrusted('/secret/mine/privatekey.pem')).toBe( - undefined, - ); + expect(folders.isPathTrusted('/secret/bankaccounts.json')).toBe(false); + expect(folders.isPathTrusted('/secret/mine/privatekey.pem')).toBe(false); expect(folders.isPathTrusted('/user/someotherfolder')).toBe(undefined); }); + + it('prioritizes the longest matching path (precedence)', () => { + const { folders } = setup({ + config: { + '/a': TrustLevel.TRUST_FOLDER, + '/a/b': TrustLevel.DO_NOT_TRUST, + '/a/b/c': TrustLevel.TRUST_FOLDER, + '/parent/trustme': TrustLevel.TRUST_PARENT, // effective path is /parent + '/parent/trustme/butnotthis': TrustLevel.DO_NOT_TRUST, + }, + }); + + // /a/b/c/d matches /a (len 2), /a/b (len 4), /a/b/c (len 6). + // /a/b/c wins (TRUST_FOLDER). + expect(folders.isPathTrusted('/a/b/c/d')).toBe(true); + + // /a/b/x matches /a (len 2), /a/b (len 4). + // /a/b wins (DO_NOT_TRUST). + expect(folders.isPathTrusted('/a/b/x')).toBe(false); + + // /a/x matches /a (len 2). + // /a wins (TRUST_FOLDER). + expect(folders.isPathTrusted('/a/x')).toBe(true); + + // Overlap with TRUST_PARENT + // /parent/trustme/butnotthis/file matches: + // - /parent/trustme (len 15, TRUST_PARENT -> effective /parent) + // - /parent/trustme/butnotthis (len 26, DO_NOT_TRUST) + // /parent/trustme/butnotthis wins. + expect(folders.isPathTrusted('/parent/trustme/butnotthis/file')).toBe( + false, + ); + + // /parent/other matches /parent/trustme (len 15, effective /parent) + expect(folders.isPathTrusted('/parent/other')).toBe(true); + }); }); it('should load user rules if only user file exists', () => { const userPath = getTrustedFoldersPath(); - (mockFsExistsSync as Mock).mockImplementation((p) => p === userPath); + vi.mocked(fs.existsSync).mockImplementation( + (p: fs.PathLike) => p.toString() === userPath, + ); const userContent = { '/user/folder': TrustLevel.TRUST_FOLDER, }; - (fs.readFileSync as Mock).mockImplementation((p) => { - if (p === userPath) return JSON.stringify(userContent); - return '{}'; - }); + vi.mocked(fs.readFileSync).mockImplementation( + (p: fs.PathOrFileDescriptor) => { + if (p.toString() === userPath) return JSON.stringify(userContent); + return '{}'; + }, + ); const { rules, errors } = loadTrustedFolders(); expect(rules).toEqual([ @@ -154,11 +195,15 @@ describe('Trusted Folders Loading', () => { it('should handle JSON parsing errors gracefully', () => { const userPath = getTrustedFoldersPath(); - (mockFsExistsSync as Mock).mockImplementation((p) => p === userPath); - (fs.readFileSync as Mock).mockImplementation((p) => { - if (p === userPath) return 'invalid json'; - return '{}'; - }); + vi.mocked(fs.existsSync).mockImplementation( + (p: fs.PathLike) => p.toString() === userPath, + ); + vi.mocked(fs.readFileSync).mockImplementation( + (p: fs.PathOrFileDescriptor) => { + if (p.toString() === userPath) return 'invalid json'; + return '{}'; + }, + ); const { rules, errors } = loadTrustedFolders(); expect(rules).toEqual([]); @@ -171,14 +216,18 @@ describe('Trusted Folders Loading', () => { const customPath = '/custom/path/to/trusted_folders.json'; process.env['GEMINI_CLI_TRUSTED_FOLDERS_PATH'] = customPath; - (mockFsExistsSync as Mock).mockImplementation((p) => p === customPath); + vi.mocked(fs.existsSync).mockImplementation( + (p: fs.PathLike) => p.toString() === customPath, + ); const userContent = { '/user/folder/from/env': TrustLevel.TRUST_FOLDER, }; - (fs.readFileSync as Mock).mockImplementation((p) => { - if (p === customPath) return JSON.stringify(userContent); - return '{}'; - }); + vi.mocked(fs.readFileSync).mockImplementation( + (p: fs.PathOrFileDescriptor) => { + if (p.toString() === customPath) return JSON.stringify(userContent); + return '{}'; + }, + ); const { rules, errors } = loadTrustedFolders(); expect(rules).toEqual([ @@ -221,14 +270,16 @@ describe('isWorkspaceTrusted', () => { beforeEach(() => { resetTrustedFoldersForTesting(); vi.spyOn(process, 'cwd').mockImplementation(() => mockCwd); - vi.spyOn(fs, 'readFileSync').mockImplementation((p) => { - if (p === getTrustedFoldersPath()) { - return JSON.stringify(mockRules); - } - return '{}'; - }); + vi.spyOn(fs, 'readFileSync').mockImplementation( + (p: fs.PathOrFileDescriptor) => { + if (p.toString() === getTrustedFoldersPath()) { + return JSON.stringify(mockRules); + } + return '{}'; + }, + ); vi.spyOn(fs, 'existsSync').mockImplementation( - (p) => p === getTrustedFoldersPath(), + (p: fs.PathLike) => p.toString() === getTrustedFoldersPath(), ); }); @@ -241,12 +292,14 @@ describe('isWorkspaceTrusted', () => { it('should throw a fatal error if the config is malformed', () => { mockCwd = '/home/user/projectA'; // This mock needs to be specific to this test to override the one in beforeEach - vi.spyOn(fs, 'readFileSync').mockImplementation((p) => { - if (p === getTrustedFoldersPath()) { - return '{"foo": "bar",}'; // Malformed JSON with trailing comma - } - return '{}'; - }); + vi.spyOn(fs, 'readFileSync').mockImplementation( + (p: fs.PathOrFileDescriptor) => { + if (p.toString() === getTrustedFoldersPath()) { + return '{"foo": "bar",}'; // Malformed JSON with trailing comma + } + return '{}'; + }, + ); expect(() => isWorkspaceTrusted(mockSettings)).toThrow(FatalConfigError); expect(() => isWorkspaceTrusted(mockSettings)).toThrow( /Please fix the configuration file/, @@ -255,12 +308,14 @@ describe('isWorkspaceTrusted', () => { it('should throw a fatal error if the config is not a JSON object', () => { mockCwd = '/home/user/projectA'; - vi.spyOn(fs, 'readFileSync').mockImplementation((p) => { - if (p === getTrustedFoldersPath()) { - return 'null'; - } - return '{}'; - }); + vi.spyOn(fs, 'readFileSync').mockImplementation( + (p: fs.PathOrFileDescriptor) => { + if (p.toString() === getTrustedFoldersPath()) { + return 'null'; + } + return '{}'; + }, + ); expect(() => isWorkspaceTrusted(mockSettings)).toThrow(FatalConfigError); expect(() => isWorkspaceTrusted(mockSettings)).toThrow( /not a valid JSON object/, @@ -303,10 +358,10 @@ describe('isWorkspaceTrusted', () => { }); }); - it('should return undefined for a child of an untrusted folder', () => { + it('should return false for a child of an untrusted folder', () => { mockCwd = '/home/user/untrusted/src'; mockRules['/home/user/untrusted'] = TrustLevel.DO_NOT_TRUST; - expect(isWorkspaceTrusted(mockSettings).isTrusted).toBeUndefined(); + expect(isWorkspaceTrusted(mockSettings).isTrusted).toBe(false); }); it('should return undefined when no rules match', () => { @@ -316,12 +371,12 @@ describe('isWorkspaceTrusted', () => { expect(isWorkspaceTrusted(mockSettings).isTrusted).toBeUndefined(); }); - it('should prioritize trust over distrust', () => { + it('should prioritize specific distrust over parent trust', () => { mockCwd = '/home/user/projectA/untrusted'; mockRules['/home/user/projectA'] = TrustLevel.TRUST_FOLDER; mockRules['/home/user/projectA/untrusted'] = TrustLevel.DO_NOT_TRUST; expect(isWorkspaceTrusted(mockSettings)).toEqual({ - isTrusted: true, + isTrusted: false, source: 'file', }); }); @@ -351,6 +406,19 @@ describe('isWorkspaceTrusted', () => { }); describe('isWorkspaceTrusted with IDE override', () => { + const mockCwd = '/home/user/projectA'; + + beforeEach(() => { + resetTrustedFoldersForTesting(); + vi.spyOn(process, 'cwd').mockImplementation(() => mockCwd); + vi.spyOn(fs, 'realpathSync').mockImplementation((p: fs.PathLike) => + p.toString(), + ); + vi.spyOn(fs, 'existsSync').mockImplementation((p: fs.PathLike) => + p.toString().endsWith('trustedFolders.json') ? false : true, + ); + }); + afterEach(() => { vi.clearAllMocks(); ideContextStore.clear(); @@ -390,10 +458,15 @@ describe('isWorkspaceTrusted with IDE override', () => { }); it('should fall back to config when ideTrust is undefined', () => { - vi.spyOn(fs, 'existsSync').mockReturnValue(true); - vi.spyOn(fs, 'readFileSync').mockReturnValue( - JSON.stringify({ [process.cwd()]: TrustLevel.TRUST_FOLDER }), + vi.spyOn(fs, 'existsSync').mockImplementation((p) => + p === getTrustedFoldersPath() || p === mockCwd ? true : false, ); + vi.spyOn(fs, 'readFileSync').mockImplementation((p) => { + if (p === getTrustedFoldersPath()) { + return JSON.stringify({ [mockCwd]: TrustLevel.TRUST_FOLDER }); + } + return '{}'; + }); expect(isWorkspaceTrusted(mockSettings)).toEqual({ isTrusted: true, source: 'file', @@ -419,8 +492,11 @@ describe('isWorkspaceTrusted with IDE override', () => { describe('Trusted Folders Caching', () => { beforeEach(() => { resetTrustedFoldersForTesting(); - vi.mocked(fs.existsSync).mockReturnValue(true); - vi.mocked(fs.readFileSync).mockReturnValue('{}'); + vi.spyOn(fs, 'existsSync').mockReturnValue(true); + vi.spyOn(fs, 'readFileSync').mockReturnValue('{}'); + vi.spyOn(fs, 'realpathSync').mockImplementation((p: fs.PathLike) => + p.toString(), + ); }); afterEach(() => { @@ -454,14 +530,20 @@ describe('invalid trust levels', () => { beforeEach(() => { resetTrustedFoldersForTesting(); vi.spyOn(process, 'cwd').mockImplementation(() => mockCwd); - vi.spyOn(fs, 'readFileSync').mockImplementation((p) => { - if (p === getTrustedFoldersPath()) { - return JSON.stringify(mockRules); - } - return '{}'; - }); + vi.spyOn(fs, 'realpathSync').mockImplementation((p: fs.PathLike) => + p.toString(), + ); + vi.spyOn(fs, 'readFileSync').mockImplementation( + (p: fs.PathOrFileDescriptor) => { + if (p.toString() === getTrustedFoldersPath()) { + return JSON.stringify(mockRules); + } + return '{}'; + }, + ); vi.spyOn(fs, 'existsSync').mockImplementation( - (p) => p === getTrustedFoldersPath(), + (p: fs.PathLike) => + p.toString() === getTrustedFoldersPath() || p.toString() === mockCwd, ); }); @@ -495,3 +577,179 @@ describe('invalid trust levels', () => { expect(() => isWorkspaceTrusted(mockSettings)).toThrow(FatalConfigError); }); }); + +describe('Trusted Folders realpath caching', () => { + beforeEach(() => { + resetTrustedFoldersForTesting(); + vi.resetAllMocks(); + vi.spyOn(fs, 'realpathSync').mockImplementation((p: fs.PathLike) => + p.toString(), + ); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('should only call fs.realpathSync once for the same path', () => { + const mockPath = '/some/path'; + const mockRealPath = '/real/path'; + + vi.spyOn(fs, 'existsSync').mockReturnValue(true); + const realpathSpy = vi + .spyOn(fs, 'realpathSync') + .mockReturnValue(mockRealPath); + vi.spyOn(fs, 'readFileSync').mockReturnValue( + JSON.stringify({ + [mockPath]: TrustLevel.TRUST_FOLDER, + '/another/path': TrustLevel.TRUST_FOLDER, + }), + ); + + const folders = loadTrustedFolders(); + + // Call isPathTrusted multiple times with the same path + folders.isPathTrusted(mockPath); + folders.isPathTrusted(mockPath); + folders.isPathTrusted(mockPath); + + // fs.realpathSync should only be called once for mockPath (at the start of isPathTrusted) + // And once for each rule in the config (if they are different) + + // Let's check calls for mockPath + const mockPathCalls = realpathSpy.mock.calls.filter( + (call) => call[0] === mockPath, + ); + + expect(mockPathCalls.length).toBe(1); + }); + + it('should cache results for rule paths in the loop', () => { + const rulePath = '/rule/path'; + const locationPath = '/location/path'; + + vi.spyOn(fs, 'existsSync').mockReturnValue(true); + const realpathSpy = vi + .spyOn(fs, 'realpathSync') + .mockImplementation((p: fs.PathLike) => p.toString()); // identity for simplicity + vi.spyOn(fs, 'readFileSync').mockReturnValue( + JSON.stringify({ + [rulePath]: TrustLevel.TRUST_FOLDER, + }), + ); + + const folders = loadTrustedFolders(); + + // First call + folders.isPathTrusted(locationPath); + const firstCallCount = realpathSpy.mock.calls.length; + expect(firstCallCount).toBe(2); // locationPath and rulePath + + // Second call with same location and same config + folders.isPathTrusted(locationPath); + const secondCallCount = realpathSpy.mock.calls.length; + + // Should still be 2 because both were cached + expect(secondCallCount).toBe(2); + }); +}); + +describe('isWorkspaceTrusted with Symlinks', () => { + const mockSettings: Settings = { + security: { + folderTrust: { + enabled: true, + }, + }, + }; + + beforeEach(() => { + resetTrustedFoldersForTesting(); + vi.resetAllMocks(); + vi.spyOn(fs, 'realpathSync').mockImplementation((p: fs.PathLike) => + p.toString(), + ); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('should trust a folder even if CWD is a symlink and rule is realpath', () => { + const symlinkPath = '/var/folders/project'; + const realPath = '/private/var/folders/project'; + + vi.spyOn(process, 'cwd').mockReturnValue(symlinkPath); + + // Mock fs.existsSync to return true for trust config and both paths + vi.spyOn(fs, 'existsSync').mockImplementation((p: fs.PathLike) => { + const pathStr = p.toString(); + if (pathStr === getTrustedFoldersPath()) return true; + if (pathStr === symlinkPath) return true; + if (pathStr === realPath) return true; + return false; + }); + + // Mock realpathSync to resolve symlink to realpath + vi.spyOn(fs, 'realpathSync').mockImplementation((p: fs.PathLike) => { + const pathStr = p.toString(); + if (pathStr === symlinkPath) return realPath; + if (pathStr === realPath) return realPath; + return pathStr; + }); + + // Rule is saved with realpath + const mockRules = { + [realPath]: TrustLevel.TRUST_FOLDER, + }; + vi.spyOn(fs, 'readFileSync').mockImplementation( + (p: fs.PathOrFileDescriptor) => { + if (p.toString() === getTrustedFoldersPath()) + return JSON.stringify(mockRules); + return '{}'; + }, + ); + + // Should be trusted because both resolve to the same realpath + expect(isWorkspaceTrusted(mockSettings).isTrusted).toBe(true); + }); + + it('should trust a folder even if CWD is realpath and rule is a symlink', () => { + const symlinkPath = '/var/folders/project'; + const realPath = '/private/var/folders/project'; + + vi.spyOn(process, 'cwd').mockReturnValue(realPath); + + // Mock fs.existsSync + vi.spyOn(fs, 'existsSync').mockImplementation((p: fs.PathLike) => { + const pathStr = p.toString(); + if (pathStr === getTrustedFoldersPath()) return true; + if (pathStr === symlinkPath) return true; + if (pathStr === realPath) return true; + return false; + }); + + // Mock realpathSync + vi.spyOn(fs, 'realpathSync').mockImplementation((p: fs.PathLike) => { + const pathStr = p.toString(); + if (pathStr === symlinkPath) return realPath; + if (pathStr === realPath) return realPath; + return pathStr; + }); + + // Rule is saved with symlink path + const mockRules = { + [symlinkPath]: TrustLevel.TRUST_FOLDER, + }; + vi.spyOn(fs, 'readFileSync').mockImplementation( + (p: fs.PathOrFileDescriptor) => { + if (p.toString() === getTrustedFoldersPath()) + return JSON.stringify(mockRules); + return '{}'; + }, + ); + + // Should be trusted because both resolve to the same realpath + expect(isWorkspaceTrusted(mockSettings).isTrusted).toBe(true); + }); +}); diff --git a/packages/cli/src/config/trustedFolders.ts b/packages/cli/src/config/trustedFolders.ts index c462460d06..5de5b605cd 100644 --- a/packages/cli/src/config/trustedFolders.ts +++ b/packages/cli/src/config/trustedFolders.ts @@ -36,7 +36,9 @@ export enum TrustLevel { DO_NOT_TRUST = 'DO_NOT_TRUST', } -export function isTrustLevel(value: unknown): value is TrustLevel { +export function isTrustLevel( + value: string | number | boolean | object | null | undefined, +): value is TrustLevel { return ( typeof value === 'string' && Object.values(TrustLevel).includes(value as TrustLevel) @@ -63,6 +65,32 @@ export interface TrustResult { source: 'ide' | 'file' | undefined; } +const realPathCache = new Map(); + +/** + * FOR TESTING PURPOSES ONLY. + * Clears the real path cache. + */ +export function clearRealPathCacheForTesting(): void { + realPathCache.clear(); +} + +function getRealPath(location: string): string { + let realPath = realPathCache.get(location); + if (realPath !== undefined) { + return realPath; + } + + try { + realPath = fs.existsSync(location) ? fs.realpathSync(location) : location; + } catch { + realPath = location; + } + + realPathCache.set(location, realPath); + return realPath; +} + export class LoadedTrustedFolders { constructor( readonly user: TrustedFoldersFile, @@ -88,39 +116,36 @@ export class LoadedTrustedFolders { config?: Record, ): boolean | undefined { const configToUse = config ?? this.user.config; - const trustedPaths: string[] = []; - const untrustedPaths: string[] = []; - for (const rule of Object.entries(configToUse).map( - ([path, trustLevel]) => ({ path, trustLevel }), - )) { - switch (rule.trustLevel) { - case TrustLevel.TRUST_FOLDER: - trustedPaths.push(rule.path); - break; - case TrustLevel.TRUST_PARENT: - trustedPaths.push(path.dirname(rule.path)); - break; - case TrustLevel.DO_NOT_TRUST: - untrustedPaths.push(rule.path); - break; - default: - // Do nothing for unknown trust levels. - break; + // Resolve location to its realpath for canonical comparison + const realLocation = getRealPath(location); + + let longestMatchLen = -1; + let longestMatchTrust: TrustLevel | undefined = undefined; + + for (const [rulePath, trustLevel] of Object.entries(configToUse)) { + const effectivePath = + trustLevel === TrustLevel.TRUST_PARENT + ? path.dirname(rulePath) + : rulePath; + + // Resolve effectivePath to its realpath for canonical comparison + const realEffectivePath = getRealPath(effectivePath); + + if (isWithinRoot(realLocation, realEffectivePath)) { + if (rulePath.length > longestMatchLen) { + longestMatchLen = rulePath.length; + longestMatchTrust = trustLevel; + } } } - for (const trustedPath of trustedPaths) { - if (isWithinRoot(location, trustedPath)) { - return true; - } - } - - for (const untrustedPath of untrustedPaths) { - if (path.normalize(location) === path.normalize(untrustedPath)) { - return false; - } - } + if (longestMatchTrust === TrustLevel.DO_NOT_TRUST) return false; + if ( + longestMatchTrust === TrustLevel.TRUST_FOLDER || + longestMatchTrust === TrustLevel.TRUST_PARENT + ) + return true; return undefined; } @@ -150,6 +175,7 @@ let loadedTrustedFolders: LoadedTrustedFolders | undefined; */ export function resetTrustedFoldersForTesting(): void { loadedTrustedFolders = undefined; + clearRealPathCacheForTesting(); } export function loadTrustedFolders(): LoadedTrustedFolders { @@ -161,11 +187,13 @@ export function loadTrustedFolders(): LoadedTrustedFolders { const userConfig: Record = {}; const userPath = getTrustedFoldersPath(); - // Load user trusted folders try { if (fs.existsSync(userPath)) { const content = fs.readFileSync(userPath, 'utf-8'); - const parsed: unknown = JSON.parse(stripJsonComments(content)); + const parsed = JSON.parse(stripJsonComments(content)) as Record< + string, + string + >; if ( typeof parsed !== 'object' || @@ -190,7 +218,7 @@ export function loadTrustedFolders(): LoadedTrustedFolders { } } } - } catch (error: unknown) { + } catch (error) { errors.push({ message: getErrorMessage(error), path: userPath, diff --git a/packages/cli/src/gemini.test.tsx b/packages/cli/src/gemini.test.tsx index a5c615444f..41f9978d7c 100644 --- a/packages/cli/src/gemini.test.tsx +++ b/packages/cli/src/gemini.test.tsx @@ -12,6 +12,7 @@ import { beforeEach, afterEach, type MockInstance, + type Mock, } from 'vitest'; import { main, @@ -20,53 +21,48 @@ import { startInteractiveUI, getNodeMemoryArgs, } from './gemini.js'; +import { loadCliConfig, parseArguments } from './config/config.js'; +import { loadSandboxConfig } from './config/sandboxConfig.js'; +import { terminalCapabilityManager } from './ui/utils/terminalCapabilityManager.js'; +import { start_sandbox } from './utils/sandbox.js'; +import { validateNonInteractiveAuth } from './validateNonInterActiveAuth.js'; import os from 'node:os'; import v8 from 'node:v8'; import { type CliArgs } from './config/config.js'; +import { type LoadedSettings, loadSettings } from './config/settings.js'; import { - type LoadedSettings, - type Settings, - createTestMergedSettings, -} from './config/settings.js'; + createMockConfig, + createMockSettings, +} from './test-utils/mockConfig.js'; import { appEvents, AppEvent } from './utils/events.js'; - -function createMockSettings( - overrides: Record = {}, -): LoadedSettings { - const merged = createTestMergedSettings( - (overrides['merged'] as Partial) || {}, - ); - - return { - system: { settings: {} }, - systemDefaults: { settings: {} }, - user: { settings: {} }, - workspace: { settings: {} }, - errors: [], - ...overrides, - merged, - } as unknown as LoadedSettings; -} import { type Config, type ResumedSessionData, debugLogger, coreEvents, + AuthType, } from '@google/gemini-cli-core'; import { act } from 'react'; import { type InitializationResult } from './core/initializer.js'; - +import { runNonInteractive } from './nonInteractiveCli.js'; +// Hoisted constants and mocks const performance = vi.hoisted(() => ({ now: vi.fn(), })); vi.stubGlobal('performance', performance); +const runNonInteractiveSpy = vi.hoisted(() => vi.fn()); +vi.mock('./nonInteractiveCli.js', () => ({ + runNonInteractive: runNonInteractiveSpy, +})); + vi.mock('@google/gemini-cli-core', async (importOriginal) => { const actual = await importOriginal(); return { ...actual, recordSlowRender: vi.fn(), + logUserPrompt: vi.fn(), writeToStdout: vi.fn((...args) => process.stdout.write( ...(args as Parameters), @@ -94,6 +90,30 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => { enterAlternateScreen: vi.fn(), disableLineWrapping: vi.fn(), getVersion: vi.fn(() => Promise.resolve('1.0.0')), + startupProfiler: { + start: vi.fn(() => ({ + end: vi.fn(), + })), + flush: vi.fn(), + }, + ClearcutLogger: { + getInstance: vi.fn(() => ({ + logStartSessionEvent: vi.fn().mockResolvedValue(undefined), + logEndSessionEvent: vi.fn().mockResolvedValue(undefined), + logUserPrompt: vi.fn(), + addDefaultFields: vi.fn((data) => data), + })), + clearInstance: vi.fn(), + }, + coreEvents: { + ...actual.coreEvents, + emitFeedback: vi.fn(), + emitConsoleLog: vi.fn(), + listenerCount: vi.fn().mockReturnValue(0), + on: vi.fn(), + off: vi.fn(), + drainBacklogs: vi.fn(), + }, }; }); @@ -152,15 +172,7 @@ vi.mock('./ui/utils/terminalCapabilityManager.js', () => ({ })); vi.mock('./config/config.js', () => ({ - loadCliConfig: vi.fn().mockResolvedValue({ - getSandbox: vi.fn(() => false), - getQuestion: vi.fn(() => ''), - isInteractive: () => false, - setTerminalBackground: vi.fn(), - storage: { - getProjectTempDir: vi.fn().mockReturnValue('/tmp/gemini-test'), - }, - } as unknown as Config), + loadCliConfig: vi.fn().mockImplementation(async () => createMockConfig()), parseArguments: vi.fn().mockResolvedValue({}), isDebugMode: vi.fn(() => false), })); @@ -188,18 +200,31 @@ vi.mock('./utils/events.js', async (importOriginal) => { }; }); +import * as readStdinModule from './utils/readStdin.js'; + vi.mock('./utils/sandbox.js', () => ({ sandbox_command: vi.fn(() => ''), // Default to no sandbox command start_sandbox: vi.fn(() => Promise.resolve()), // Mock as an async function that resolves })); vi.mock('./utils/relaunch.js', () => ({ - relaunchAppInChildProcess: vi.fn(), - relaunchOnExitCode: vi.fn(), + relaunchAppInChildProcess: vi.fn().mockResolvedValue(undefined), + relaunchOnExitCode: vi.fn(async (fn) => { + await fn(); + }), })); vi.mock('./config/sandboxConfig.js', () => ({ - loadSandboxConfig: vi.fn(), + loadSandboxConfig: vi.fn().mockResolvedValue({ + command: 'docker', + image: 'test-image', + }), +})); + +vi.mock('./deferred.js', () => ({ + runDeferredCommand: vi.fn().mockResolvedValue(undefined), + setDeferredCommand: vi.fn(), + defer: vi.fn((m) => m), })); vi.mock('./ui/utils/mouse.js', () => ({ @@ -208,14 +233,14 @@ vi.mock('./ui/utils/mouse.js', () => ({ isIncompleteMouseSequence: vi.fn(), })); -const runNonInteractiveSpy = vi.hoisted(() => vi.fn()); -vi.mock('./nonInteractiveCli.js', () => ({ - runNonInteractive: runNonInteractiveSpy, +vi.mock('./validateNonInterActiveAuth.js', () => ({ + validateNonInteractiveAuth: vi.fn().mockResolvedValue('google'), })); describe('gemini.tsx main function', () => { let originalEnvGeminiSandbox: string | undefined; let originalEnvSandbox: string | undefined; + let originalIsTTY: boolean | undefined; let initialUnhandledRejectionListeners: NodeJS.UnhandledRejectionListener[] = []; @@ -228,6 +253,10 @@ describe('gemini.tsx main function', () => { initialUnhandledRejectionListeners = process.listeners('unhandledRejection'); + + originalIsTTY = process.stdin.isTTY; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (process.stdin as any).isTTY = true; }); afterEach(() => { @@ -249,6 +278,10 @@ describe('gemini.tsx main function', () => { process.removeListener('unhandledRejection', listener); } }); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (process.stdin as any).isTTY = originalIsTTY; + vi.restoreAllMocks(); }); @@ -379,6 +412,8 @@ describe('getNodeMemoryArgs', () => { describe('gemini.tsx main function kitty protocol', () => { let originalEnvNoRelaunch: string | undefined; + let originalIsTTY: boolean | undefined; + let originalIsRaw: boolean | undefined; let setRawModeSpy: MockInstance< (mode: boolean) => NodeJS.ReadStream & { fd: 0 } >; @@ -395,14 +430,12 @@ describe('gemini.tsx main function kitty protocol', () => { } setRawModeSpy = vi.spyOn(process.stdin, 'setRawMode'); - Object.defineProperty(process.stdin, 'isTTY', { - value: true, - configurable: true, - }); - Object.defineProperty(process.stdin, 'isRaw', { - value: false, - configurable: true, - }); + originalIsTTY = process.stdin.isTTY; + originalIsRaw = process.stdin.isRaw; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (process.stdin as any).isTTY = true; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (process.stdin as any).isRaw = false; }); afterEach(() => { @@ -412,56 +445,21 @@ describe('gemini.tsx main function kitty protocol', () => { } else { delete process.env['GEMINI_CLI_NO_RELAUNCH']; } + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (process.stdin as any).isTTY = originalIsTTY; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (process.stdin as any).isRaw = originalIsRaw; vi.restoreAllMocks(); }); it('should call setRawMode and detectCapabilities when isInteractive is true', async () => { - const { terminalCapabilityManager } = await import( - './ui/utils/terminalCapabilityManager.js' - ); - const { loadCliConfig, parseArguments } = await import( - './config/config.js' - ); - const { loadSettings } = await import('./config/settings.js'); - vi.mocked(loadCliConfig).mockResolvedValue({ - isInteractive: () => true, - getQuestion: () => '', - getSandbox: () => false, - getDebugMode: () => false, - getListExtensions: () => false, - getListSessions: () => false, - getDeleteSession: () => undefined, - getMcpServers: () => ({}), - getMcpClientManager: vi.fn(), - initialize: vi.fn(), - getIdeMode: () => false, - getExperimentalZedIntegration: () => false, - getScreenReader: () => false, - getGeminiMdFileCount: () => 0, - getPolicyEngine: vi.fn(), - getMessageBus: () => ({ - subscribe: vi.fn(), + vi.mocked(loadCliConfig).mockResolvedValue( + createMockConfig({ + isInteractive: () => true, + getQuestion: () => '', + getSandbox: () => undefined, }), - getEnableHooks: () => false, - getHookSystem: () => undefined, - getToolRegistry: vi.fn(), - getContentGeneratorConfig: vi.fn(), - getModel: () => 'gemini-pro', - getEmbeddingModel: () => 'embedding-001', - getApprovalMode: () => 'default', - getCoreTools: () => [], - getTelemetryEnabled: () => false, - getTelemetryLogPromptsEnabled: () => false, - getFileFilteringRespectGitIgnore: () => true, - getOutputFormat: () => 'text', - getExtensions: () => [], - getUsageStatisticsEnabled: () => false, - getRemoteAdminSettings: () => undefined, - setTerminalBackground: vi.fn(), - storage: { - getProjectTempDir: vi.fn().mockReturnValue('/tmp/gemini-test'), - }, - } as unknown as Config); + ); vi.mocked(loadSettings).mockReturnValue( createMockSettings({ merged: { @@ -514,10 +512,6 @@ describe('gemini.tsx main function kitty protocol', () => { { flag: 'listSessions' }, { flag: 'deleteSession', value: 'session-id' }, ])('should handle --$flag flag', async ({ flag, value }) => { - const { loadCliConfig, parseArguments } = await import( - './config/config.js' - ); - const { loadSettings } = await import('./config/settings.js'); const { listSessions, deleteSession } = await import('./utils/sessions.js'); const processExitSpy = vi .spyOn(process, 'exit') @@ -542,32 +536,24 @@ describe('gemini.tsx main function kitty protocol', () => { promptInteractive: false, } as any); // eslint-disable-line @typescript-eslint/no-explicit-any - const mockConfig = { + const mockConfig = createMockConfig({ isInteractive: () => false, getQuestion: () => '', - getSandbox: () => false, - getDebugMode: () => false, + getSandbox: () => undefined, getListExtensions: () => flag === 'listExtensions', getListSessions: () => flag === 'listSessions', getDeleteSession: () => (flag === 'deleteSession' ? value : undefined), - getExtensions: () => [{ name: 'ext1' }], - getPolicyEngine: vi.fn(), - getMessageBus: () => ({ subscribe: vi.fn() }), - getEnableHooks: () => false, - getHookSystem: () => undefined, - initialize: vi.fn(), - getContentGeneratorConfig: vi.fn(), - getMcpServers: () => ({}), - getMcpClientManager: vi.fn(), - getIdeMode: () => false, - getExperimentalZedIntegration: () => false, - getScreenReader: () => false, - getGeminiMdFileCount: () => 0, - getProjectRoot: () => '/', - getRemoteAdminSettings: () => undefined, - setTerminalBackground: vi.fn(), - refreshAuth: vi.fn(), - } as unknown as Config; + getExtensions: () => [ + { + name: 'ext1', + id: 'ext1', + version: '1.0.0', + isActive: true, + path: '/path/to/ext1', + contextFiles: [], + }, + ], + }); vi.mocked(loadCliConfig).mockResolvedValue(mockConfig); vi.mock('./utils/sessions.js', () => ({ @@ -602,13 +588,7 @@ describe('gemini.tsx main function kitty protocol', () => { }); it('should handle sandbox activation', async () => { - const { loadCliConfig, parseArguments } = await import( - './config/config.js' - ); - const { loadSandboxConfig } = await import('./config/sandboxConfig.js'); - const { start_sandbox } = await import('./utils/sandbox.js'); - const { relaunchOnExitCode } = await import('./utils/relaunch.js'); - const { loadSettings } = await import('./config/settings.js'); + vi.stubEnv('SANDBOX', ''); const processExitSpy = vi .spyOn(process, 'exit') .mockImplementation((code) => { @@ -623,7 +603,7 @@ describe('gemini.tsx main function kitty protocol', () => { createMockSettings({ merged: { advanced: {}, - security: { auth: {} }, + security: { auth: { selectedType: 'google' } }, ui: {}, }, workspace: { settings: {} }, @@ -632,75 +612,16 @@ describe('gemini.tsx main function kitty protocol', () => { }), ); - const mockConfig = { + const mockConfig = createMockConfig({ isInteractive: () => false, getQuestion: () => '', - getSandbox: () => true, - getDebugMode: () => false, - getListExtensions: () => false, - getListSessions: () => false, - getDeleteSession: () => undefined, - getExtensions: () => [], - getPolicyEngine: vi.fn(), - getMessageBus: () => ({ subscribe: vi.fn() }), - getEnableHooks: () => false, - getHookSystem: () => undefined, - initialize: vi.fn(), - getContentGeneratorConfig: vi.fn(), - getMcpServers: () => ({}), - getMcpClientManager: vi.fn(), - getIdeMode: () => false, - getExperimentalZedIntegration: () => false, - getScreenReader: () => false, - getGeminiMdFileCount: () => 0, - getProjectRoot: () => '/', - refreshAuth: vi.fn(), - getRemoteAdminSettings: () => undefined, - setTerminalBackground: vi.fn(), - getToolRegistry: () => ({ getAllTools: () => [] }), - getModel: () => 'gemini-pro', - getEmbeddingModel: () => 'embedding-model', - getCoreTools: () => [], - getApprovalMode: () => 'default', - getPreviewFeatures: () => false, - getTargetDir: () => '/', - getUsageStatisticsEnabled: () => false, - getTelemetryEnabled: () => false, - getTelemetryTarget: () => 'none', - getTelemetryOtlpEndpoint: () => '', - getTelemetryOtlpProtocol: () => 'grpc', - getTelemetryLogPromptsEnabled: () => false, - getContinueOnFailedApiCall: () => false, - getShellToolInactivityTimeout: () => 0, - getTruncateToolOutputThreshold: () => 0, - getUseRipgrep: () => false, - getUseWriteTodos: () => false, - getHooks: () => undefined, - getExperiments: () => undefined, - getFileFilteringRespectGitIgnore: () => true, - getOutputFormat: () => 'text', - getFolderTrust: () => false, - getPendingIncludeDirectories: () => [], - getWorkspaceContext: () => ({ getDirectories: () => ['/'] }), - getModelAvailabilityService: () => ({ - reset: vi.fn(), - resetTurn: vi.fn(), - }), - getBaseLlmClient: () => ({}), - getGeminiClient: () => ({}), - getContentGenerator: () => ({}), - isTrustedFolder: () => true, - isYoloModeDisabled: () => true, - isPlanEnabled: () => false, - isEventDrivenSchedulerEnabled: () => false, - } as unknown as Config; + getSandbox: () => ({ command: 'docker', image: 'test-image' }), + }); vi.mocked(loadCliConfig).mockResolvedValue(mockConfig); vi.mocked(loadSandboxConfig).mockResolvedValue({ command: 'docker', - } as any); // eslint-disable-line @typescript-eslint/no-explicit-any - vi.mocked(relaunchOnExitCode).mockImplementation(async (fn) => { - await fn(); + image: 'test-image', }); process.env['GEMINI_API_KEY'] = 'test-key'; @@ -718,10 +639,6 @@ describe('gemini.tsx main function kitty protocol', () => { }); it('should log warning when theme is not found', async () => { - const { loadCliConfig, parseArguments } = await import( - './config/config.js' - ); - const { loadSettings } = await import('./config/settings.js'); const { themeManager } = await import('./ui/themes/theme-manager.js'); const debugLoggerWarnSpy = vi .spyOn(debugLogger, 'warn') @@ -748,42 +665,13 @@ describe('gemini.tsx main function kitty protocol', () => { vi.mocked(parseArguments).mockResolvedValue({ promptInteractive: false, } as any); // eslint-disable-line @typescript-eslint/no-explicit-any - vi.mocked(loadCliConfig).mockResolvedValue({ - isInteractive: () => false, - getQuestion: () => 'test', - getSandbox: () => false, - getDebugMode: () => false, - getPolicyEngine: vi.fn(), - getMessageBus: () => ({ subscribe: vi.fn() }), - getEnableHooks: () => false, - getHookSystem: () => undefined, - initialize: vi.fn(), - getContentGeneratorConfig: vi.fn(), - getMcpServers: () => ({}), - getMcpClientManager: vi.fn(), - getIdeMode: () => false, - getExperimentalZedIntegration: () => false, - getScreenReader: () => false, - getGeminiMdFileCount: () => 0, - getProjectRoot: () => '/', - getListExtensions: () => false, - getListSessions: () => false, - getDeleteSession: () => undefined, - getToolRegistry: vi.fn(), - getExtensions: () => [], - getModel: () => 'gemini-pro', - getEmbeddingModel: () => 'embedding-001', - getApprovalMode: () => 'default', - getCoreTools: () => [], - getTelemetryEnabled: () => false, - getTelemetryLogPromptsEnabled: () => false, - getFileFilteringRespectGitIgnore: () => true, - getOutputFormat: () => 'text', - getUsageStatisticsEnabled: () => false, - refreshAuth: vi.fn(), - setTerminalBackground: vi.fn(), - getRemoteAdminSettings: () => undefined, - } as any); // eslint-disable-line @typescript-eslint/no-explicit-any + vi.mocked(loadCliConfig).mockResolvedValue( + createMockConfig({ + isInteractive: () => false, + getQuestion: () => 'test', + getSandbox: () => undefined, + }), + ); vi.spyOn(themeManager, 'setActiveTheme').mockReturnValue(false); @@ -803,10 +691,6 @@ describe('gemini.tsx main function kitty protocol', () => { }); it('should handle session selector error', async () => { - const { loadCliConfig, parseArguments } = await import( - './config/config.js' - ); - const { loadSettings } = await import('./config/settings.js'); const { SessionSelector } = await import('./utils/sessionUtils.js'); vi.mocked(SessionSelector).mockImplementation( () => @@ -837,44 +721,13 @@ describe('gemini.tsx main function kitty protocol', () => { promptInteractive: false, resume: 'session-id', } as any); // eslint-disable-line @typescript-eslint/no-explicit-any - vi.mocked(loadCliConfig).mockResolvedValue({ - isInteractive: () => true, - getQuestion: () => '', - getSandbox: () => false, - getDebugMode: () => false, - getPolicyEngine: vi.fn(), - getMessageBus: () => ({ subscribe: vi.fn() }), - getEnableHooks: () => false, - getHookSystem: () => undefined, - initialize: vi.fn(), - getContentGeneratorConfig: vi.fn(), - getMcpServers: () => ({}), - getMcpClientManager: vi.fn(), - getIdeMode: () => false, - getExperimentalZedIntegration: () => false, - getScreenReader: () => false, - getGeminiMdFileCount: () => 0, - getProjectRoot: () => '/', - getListExtensions: () => false, - getListSessions: () => false, - getDeleteSession: () => undefined, - getToolRegistry: vi.fn(), - getExtensions: () => [], - getModel: () => 'gemini-pro', - getEmbeddingModel: () => 'embedding-001', - getApprovalMode: () => 'default', - getCoreTools: () => [], - getTelemetryEnabled: () => false, - getTelemetryLogPromptsEnabled: () => false, - getFileFilteringRespectGitIgnore: () => true, - getOutputFormat: () => 'text', - getUsageStatisticsEnabled: () => false, - getRemoteAdminSettings: () => undefined, - setTerminalBackground: vi.fn(), - storage: { - getProjectTempDir: vi.fn().mockReturnValue('/tmp/gemini-test'), - }, - } as any); // eslint-disable-line @typescript-eslint/no-explicit-any + vi.mocked(loadCliConfig).mockResolvedValue( + createMockConfig({ + isInteractive: () => true, + getQuestion: () => '', + getSandbox: () => undefined, + }), + ); try { await main(); @@ -892,10 +745,6 @@ describe('gemini.tsx main function kitty protocol', () => { }); it.skip('should log error when cleanupExpiredSessions fails', async () => { - const { loadCliConfig, parseArguments } = await import( - './config/config.js' - ); - const { loadSettings } = await import('./config/settings.js'); const { cleanupExpiredSessions } = await import( './utils/sessionCleanup.js' ); @@ -923,44 +772,13 @@ describe('gemini.tsx main function kitty protocol', () => { vi.mocked(parseArguments).mockResolvedValue({ promptInteractive: false, } as any); // eslint-disable-line @typescript-eslint/no-explicit-any - vi.mocked(loadCliConfig).mockResolvedValue({ - isInteractive: () => false, - getQuestion: () => 'test', - getSandbox: () => false, - getDebugMode: () => false, - getPolicyEngine: vi.fn(), - getMessageBus: () => ({ subscribe: vi.fn() }), - getEnableHooks: () => false, - getHookSystem: () => undefined, - initialize: vi.fn(), - getContentGeneratorConfig: vi.fn(), - getMcpServers: () => ({}), - getMcpClientManager: vi.fn(), - getIdeMode: () => false, - getExperimentalZedIntegration: () => false, - getScreenReader: () => false, - getGeminiMdFileCount: () => 0, - getProjectRoot: () => '/', - getListExtensions: () => false, - getListSessions: () => false, - getDeleteSession: () => undefined, - getToolRegistry: vi.fn(), - getExtensions: () => [], - getModel: () => 'gemini-pro', - getEmbeddingModel: () => 'embedding-001', - getApprovalMode: () => 'default', - getCoreTools: () => [], - getTelemetryEnabled: () => false, - getTelemetryLogPromptsEnabled: () => false, - getFileFilteringRespectGitIgnore: () => true, - getOutputFormat: () => 'text', - getUsageStatisticsEnabled: () => false, - getRemoteAdminSettings: () => undefined, - setTerminalBackground: vi.fn(), - storage: { - getProjectTempDir: vi.fn().mockReturnValue('/tmp/gemini-test'), - }, - } as any); // eslint-disable-line @typescript-eslint/no-explicit-any + vi.mocked(loadCliConfig).mockResolvedValue( + createMockConfig({ + isInteractive: () => false, + getQuestion: () => 'test', + getSandbox: () => undefined, + }), + ); // The mock is already set up at the top of the test @@ -980,17 +798,18 @@ describe('gemini.tsx main function kitty protocol', () => { }); it('should read from stdin in non-interactive mode', async () => { - const { loadCliConfig, parseArguments } = await import( - './config/config.js' - ); - const { loadSettings } = await import('./config/settings.js'); - const { readStdin } = await import('./utils/readStdin.js'); + vi.stubEnv('SANDBOX', 'true'); + vi.mocked(loadSandboxConfig).mockResolvedValue(undefined); const processExitSpy = vi .spyOn(process, 'exit') .mockImplementation((code) => { throw new MockProcessExitError(code); }); + const readStdinSpy = vi + .spyOn(readStdinModule, 'readStdin') + .mockResolvedValue('stdin-data'); + vi.mocked(loadSettings).mockReturnValue( createMockSettings({ merged: { advanced: {}, security: { auth: {} }, ui: {} }, @@ -1003,52 +822,17 @@ describe('gemini.tsx main function kitty protocol', () => { vi.mocked(parseArguments).mockResolvedValue({ promptInteractive: false, } as any); // eslint-disable-line @typescript-eslint/no-explicit-any - vi.mocked(loadCliConfig).mockResolvedValue({ - isInteractive: () => false, - getQuestion: () => 'test-question', - getSandbox: () => false, - getDebugMode: () => false, - getPolicyEngine: vi.fn(), - getMessageBus: () => ({ subscribe: vi.fn() }), - getEnableHooks: () => false, - getHookSystem: () => undefined, - initialize: vi.fn(), - getContentGeneratorConfig: vi.fn(), - getMcpServers: () => ({}), - getMcpClientManager: vi.fn(), - getIdeMode: () => false, - getExperimentalZedIntegration: () => false, - getScreenReader: () => false, - getGeminiMdFileCount: () => 0, - getProjectRoot: () => '/', - getListExtensions: () => false, - getListSessions: () => false, - getDeleteSession: () => undefined, - getToolRegistry: vi.fn(), - getExtensions: () => [], - getModel: () => 'gemini-pro', - getEmbeddingModel: () => 'embedding-001', - getApprovalMode: () => 'default', - getCoreTools: () => [], - getTelemetryEnabled: () => false, - getTelemetryLogPromptsEnabled: () => false, - getFileFilteringRespectGitIgnore: () => true, - getOutputFormat: () => 'text', - getUsageStatisticsEnabled: () => false, - refreshAuth: vi.fn(), - setTerminalBackground: vi.fn(), - getRemoteAdminSettings: () => undefined, - } as any); // eslint-disable-line @typescript-eslint/no-explicit-any - - vi.mock('./utils/readStdin.js', () => ({ - readStdin: vi.fn().mockResolvedValue('stdin-data'), - })); + vi.mocked(loadCliConfig).mockResolvedValue( + createMockConfig({ + isInteractive: () => false, + getQuestion: () => 'test-question', + getSandbox: () => undefined, + }), + ); // Mock stdin to be non-TTY - Object.defineProperty(process.stdin, 'isTTY', { - value: false, - configurable: true, - }); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (process.stdin as any).isTTY = false; process.env['GEMINI_API_KEY'] = 'test-key'; try { @@ -1059,24 +843,21 @@ describe('gemini.tsx main function kitty protocol', () => { delete process.env['GEMINI_API_KEY']; } - expect(readStdin).toHaveBeenCalled(); + expect(readStdinSpy).toHaveBeenCalled(); // In this test setup, runNonInteractive might be called on the mocked module, // but we need to ensure we are checking the correct spy instance. // Since vi.mock is hoisted, runNonInteractiveSpy is defined early. - expect(runNonInteractiveSpy).toHaveBeenCalled(); - const callArgs = runNonInteractiveSpy.mock.calls[0][0]; - expect(callArgs.input).toBe('test-question'); + expect(runNonInteractive).toHaveBeenCalled(); + const callArgs = vi.mocked(runNonInteractive).mock.calls[0][0]; + expect(callArgs.input).toBe('stdin-data\n\ntest-question'); expect(processExitSpy).toHaveBeenCalledWith(0); processExitSpy.mockRestore(); - Object.defineProperty(process.stdin, 'isTTY', { - value: true, - configurable: true, - }); }); }); describe('gemini.tsx main function exit codes', () => { let originalEnvNoRelaunch: string | undefined; + let originalIsTTY: boolean | undefined; beforeEach(() => { originalEnvNoRelaunch = process.env['GEMINI_CLI_NO_RELAUNCH']; @@ -1086,6 +867,8 @@ describe('gemini.tsx main function exit codes', () => { }); // Mock stderr to avoid cluttering output vi.spyOn(process.stderr, 'write').mockImplementation(() => true); + + originalIsTTY = process.stdin.isTTY; }); afterEach(() => { @@ -1094,15 +877,13 @@ describe('gemini.tsx main function exit codes', () => { } else { delete process.env['GEMINI_CLI_NO_RELAUNCH']; } + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (process.stdin as any).isTTY = originalIsTTY; vi.restoreAllMocks(); }); it('should exit with 42 for invalid input combination (prompt-interactive with non-TTY)', async () => { - const { loadCliConfig, parseArguments } = await import( - './config/config.js' - ); - const { loadSettings } = await import('./config/settings.js'); - vi.mocked(loadCliConfig).mockResolvedValue({} as Config); + vi.mocked(loadCliConfig).mockResolvedValue(createMockConfig()); vi.mocked(loadSettings).mockReturnValue( createMockSettings({ merged: { security: { auth: {} }, ui: {} }, @@ -1111,10 +892,8 @@ describe('gemini.tsx main function exit codes', () => { vi.mocked(parseArguments).mockResolvedValue({ promptInteractive: true, } as unknown as CliArgs); - Object.defineProperty(process.stdin, 'isTTY', { - value: false, - configurable: true, - }); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (process.stdin as any).isTTY = false; try { await main(); @@ -1126,18 +905,18 @@ describe('gemini.tsx main function exit codes', () => { }); it('should exit with 41 for auth failure during sandbox setup', async () => { - const { loadCliConfig, parseArguments } = await import( - './config/config.js' + vi.stubEnv('SANDBOX', ''); + vi.mocked(loadSandboxConfig).mockResolvedValue({ + command: 'docker', + image: 'test-image', + }); + vi.mocked(loadCliConfig).mockResolvedValue( + createMockConfig({ + refreshAuth: vi.fn().mockRejectedValue(new Error('Auth failed')), + getRemoteAdminSettings: vi.fn().mockReturnValue(undefined), + isInteractive: vi.fn().mockReturnValue(true), + }), ); - const { loadSettings } = await import('./config/settings.js'); - const { loadSandboxConfig } = await import('./config/sandboxConfig.js'); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - vi.mocked(loadSandboxConfig).mockResolvedValue({} as any); - vi.mocked(loadCliConfig).mockResolvedValue({ - refreshAuth: vi.fn().mockRejectedValue(new Error('Auth failed')), - getRemoteAdminSettings: vi.fn().mockReturnValue(undefined), - isInteractive: vi.fn().mockReturnValue(true), - } as unknown as Config); vi.mocked(loadSettings).mockReturnValue( createMockSettings({ merged: { @@ -1145,10 +924,7 @@ describe('gemini.tsx main function exit codes', () => { }, }), ); - vi.mocked(parseArguments).mockResolvedValue({} as unknown as CliArgs); - vi.mock('./config/auth.js', () => ({ - validateAuthMethod: vi.fn().mockReturnValue(null), - })); + vi.mocked(parseArguments).mockResolvedValue({} as CliArgs); try { await main(); @@ -1160,49 +936,13 @@ describe('gemini.tsx main function exit codes', () => { }); it('should exit with 42 for session resume failure', async () => { - const { loadCliConfig, parseArguments } = await import( - './config/config.js' + vi.mocked(loadCliConfig).mockResolvedValue( + createMockConfig({ + isInteractive: () => false, + getQuestion: () => 'test', + getSandbox: () => undefined, + }), ); - const { loadSettings } = await import('./config/settings.js'); - - vi.mocked(loadCliConfig).mockResolvedValue({ - isInteractive: () => false, - getQuestion: () => 'test', - getSandbox: () => false, - getDebugMode: () => false, - getListExtensions: () => false, - getListSessions: () => false, - getDeleteSession: () => undefined, - getMcpServers: () => ({}), - getMcpClientManager: vi.fn(), - initialize: vi.fn(), - getIdeMode: () => false, - getExperimentalZedIntegration: () => false, - getScreenReader: () => false, - getGeminiMdFileCount: () => 0, - getPolicyEngine: vi.fn(), - getMessageBus: () => ({ subscribe: vi.fn() }), - getEnableHooks: () => false, - getHookSystem: () => undefined, - getToolRegistry: vi.fn(), - getContentGeneratorConfig: vi.fn(), - getModel: () => 'gemini-pro', - getEmbeddingModel: () => 'embedding-001', - getApprovalMode: () => 'default', - getCoreTools: () => [], - getTelemetryEnabled: () => false, - getTelemetryLogPromptsEnabled: () => false, - getFileFilteringRespectGitIgnore: () => true, - getOutputFormat: () => 'text', - getExtensions: () => [], - getUsageStatisticsEnabled: () => false, - getRemoteAdminSettings: () => undefined, - setTerminalBackground: vi.fn(), - storage: { - getProjectTempDir: vi.fn().mockReturnValue('/tmp/gemini-test'), - }, - refreshAuth: vi.fn(), - } as unknown as Config); vi.mocked(loadSettings).mockReturnValue( createMockSettings({ merged: { security: { auth: {} }, ui: {} }, @@ -1233,59 +973,21 @@ describe('gemini.tsx main function exit codes', () => { }); it('should exit with 42 for no input provided', async () => { - const { loadCliConfig, parseArguments } = await import( - './config/config.js' + vi.mocked(loadCliConfig).mockResolvedValue( + createMockConfig({ + isInteractive: () => false, + getQuestion: () => '', + getSandbox: () => undefined, + }), ); - const { loadSettings } = await import('./config/settings.js'); - - vi.mocked(loadCliConfig).mockResolvedValue({ - isInteractive: () => false, - getQuestion: () => '', - getSandbox: () => false, - getDebugMode: () => false, - getListExtensions: () => false, - getListSessions: () => false, - getDeleteSession: () => undefined, - getMcpServers: () => ({}), - getMcpClientManager: vi.fn(), - initialize: vi.fn(), - getIdeMode: () => false, - getExperimentalZedIntegration: () => false, - getScreenReader: () => false, - getGeminiMdFileCount: () => 0, - getPolicyEngine: vi.fn(), - getMessageBus: () => ({ subscribe: vi.fn() }), - getEnableHooks: () => false, - getHookSystem: () => undefined, - getToolRegistry: vi.fn(), - getContentGeneratorConfig: vi.fn(), - getModel: () => 'gemini-pro', - getEmbeddingModel: () => 'embedding-001', - getApprovalMode: () => 'default', - getCoreTools: () => [], - getTelemetryEnabled: () => false, - getTelemetryLogPromptsEnabled: () => false, - getFileFilteringRespectGitIgnore: () => true, - getOutputFormat: () => 'text', - getExtensions: () => [], - getUsageStatisticsEnabled: () => false, - setTerminalBackground: vi.fn(), - storage: { - getProjectTempDir: vi.fn().mockReturnValue('/tmp/gemini-test'), - }, - refreshAuth: vi.fn(), - getRemoteAdminSettings: () => undefined, - } as unknown as Config); vi.mocked(loadSettings).mockReturnValue( createMockSettings({ merged: { security: { auth: {} }, ui: {} }, }), ); vi.mocked(parseArguments).mockResolvedValue({} as unknown as CliArgs); - Object.defineProperty(process.stdin, 'isTTY', { - value: true, // Simulate TTY so it doesn't try to read stdin - configurable: true, - }); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (process.stdin as any).isTTY = true; process.env['GEMINI_API_KEY'] = 'test-key'; try { @@ -1300,52 +1002,18 @@ describe('gemini.tsx main function exit codes', () => { }); it('should validate and refresh auth in non-interactive mode when no auth type is selected but env var is present', async () => { - const { loadCliConfig, parseArguments } = await import( - './config/config.js' - ); - const { loadSettings } = await import('./config/settings.js'); - const { AuthType } = await import('@google/gemini-cli-core'); - const refreshAuthSpy = vi.fn(); - - vi.mocked(loadCliConfig).mockResolvedValue({ - isInteractive: () => false, - getQuestion: () => 'test prompt', - getSandbox: () => false, - getDebugMode: () => false, - getListExtensions: () => false, - getListSessions: () => false, - getDeleteSession: () => undefined, - getMcpServers: () => ({}), - getMcpClientManager: vi.fn(), - initialize: vi.fn(), - getIdeMode: () => false, - getExperimentalZedIntegration: () => false, - getScreenReader: () => false, - getGeminiMdFileCount: () => 0, - getPolicyEngine: vi.fn(), - getMessageBus: () => ({ subscribe: vi.fn() }), - getEnableHooks: () => false, - getHookSystem: () => undefined, - getToolRegistry: vi.fn(), - getContentGeneratorConfig: vi.fn(), - getModel: () => 'gemini-pro', - getEmbeddingModel: () => 'embedding-001', - getApprovalMode: () => 'default', - getCoreTools: () => [], - getTelemetryEnabled: () => false, - getTelemetryLogPromptsEnabled: () => false, - getFileFilteringRespectGitIgnore: () => true, - getOutputFormat: () => 'text', - getExtensions: () => [], - getUsageStatisticsEnabled: () => false, - setTerminalBackground: vi.fn(), - storage: { - getProjectTempDir: vi.fn().mockReturnValue('/tmp/gemini-test'), - }, - refreshAuth: refreshAuthSpy, - getRemoteAdminSettings: () => undefined, - } as unknown as Config); + vi.mocked(loadCliConfig).mockResolvedValue( + createMockConfig({ + isInteractive: () => false, + getQuestion: () => 'test prompt', + getSandbox: () => undefined, + refreshAuth: refreshAuthSpy, + }), + ); + vi.mocked(validateNonInteractiveAuth).mockResolvedValue( + AuthType.USE_GEMINI, + ); vi.mocked(loadSettings).mockReturnValue( createMockSettings({ @@ -1412,18 +1080,111 @@ describe('validateDnsResolutionOrder', () => { }); }); +describe('project hooks loading based on trust', () => { + let loadCliConfig: Mock; + let loadSettings: Mock; + let parseArguments: Mock; + + beforeEach(async () => { + // Dynamically import and get the mocked functions + const configModule = await import('./config/config.js'); + loadCliConfig = vi.mocked(configModule.loadCliConfig); + parseArguments = vi.mocked(configModule.parseArguments); + parseArguments.mockResolvedValue({ startupMessages: [] }); + + const settingsModule = await import('./config/settings.js'); + loadSettings = vi.mocked(settingsModule.loadSettings); + + vi.clearAllMocks(); + // Mock the main function's dependencies to isolate the config loading part + vi.mock('./nonInteractiveCli.js', () => ({ + runNonInteractive: vi.fn().mockResolvedValue(undefined), + })); + + vi.spyOn(process, 'exit').mockImplementation((() => {}) as unknown as ( + code?: string | number | null, + ) => never); + + // Default mock implementation for loadCliConfig + loadCliConfig.mockResolvedValue( + createMockConfig({ + getQuestion: vi.fn().mockReturnValue('test question'), + }), + ); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('should load project hooks when workspace is trusted', async () => { + const hooks = { 'before-model': 'echo "trusted"' }; + loadSettings.mockReturnValue( + createMockSettings({ + workspace: { + isTrusted: true, + settings: { hooks }, + }, + merged: { + security: { auth: { selectedType: 'google' } }, + }, + }), + ); + + await main(); + + expect(loadCliConfig).toHaveBeenCalledWith( + expect.anything(), + expect.anything(), + expect.anything(), + expect.objectContaining({ + projectHooks: hooks, + }), + ); + }); + + it('should NOT load project hooks when workspace is not trusted', async () => { + loadSettings.mockReturnValue( + createMockSettings({ + workspace: { + isTrusted: false, + settings: {}, + }, + merged: { + security: { auth: { selectedType: 'google' } }, + }, + }), + ); + + await main(); + + expect(loadCliConfig).toHaveBeenCalledWith( + expect.anything(), + expect.anything(), + expect.anything(), + expect.objectContaining({ + projectHooks: undefined, + }), + ); + }); +}); + describe('startInteractiveUI', () => { // Mock dependencies - const mockConfig = { + const mockConfig = createMockConfig({ getProjectRoot: () => '/root', getScreenReader: () => false, getDebugMode: () => false, - } as unknown as Config; + }); const mockSettings = { merged: { ui: { hideWindowTitle: false, useAlternateBuffer: true, + incrementalRendering: true, + }, + general: { + debugKeystrokeLogging: false, }, }, } as LoadedSettings; diff --git a/packages/cli/src/test-utils/mockConfig.ts b/packages/cli/src/test-utils/mockConfig.ts new file mode 100644 index 0000000000..537f2097f6 --- /dev/null +++ b/packages/cli/src/test-utils/mockConfig.ts @@ -0,0 +1,178 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { vi } from 'vitest'; +import type { Config } from '@google/gemini-cli-core'; +import type { LoadedSettings, Settings } from '../config/settings.js'; +import { createTestMergedSettings } from '../config/settings.js'; + +/** + * Creates a mocked Config object with default values and allows overrides. + */ +export const createMockConfig = (overrides: Partial = {}): Config => + ({ + getSandbox: vi.fn(() => undefined), + getQuestion: vi.fn(() => ''), + isInteractive: vi.fn(() => false), + setTerminalBackground: vi.fn(), + storage: { + getProjectTempDir: vi.fn().mockReturnValue('/tmp/gemini-test'), + }, + getDebugMode: vi.fn(() => false), + getProjectRoot: vi.fn(() => '/'), + refreshAuth: vi.fn().mockResolvedValue(undefined), + getRemoteAdminSettings: vi.fn(() => undefined), + initialize: vi.fn().mockResolvedValue(undefined), + getPolicyEngine: vi.fn(() => ({})), + getMessageBus: vi.fn(() => ({ subscribe: vi.fn() })), + getHookSystem: vi.fn(() => ({ + fireSessionEndEvent: vi.fn().mockResolvedValue(undefined), + fireSessionStartEvent: vi.fn().mockResolvedValue(undefined), + })), + getListExtensions: vi.fn(() => false), + getExtensions: vi.fn(() => []), + getListSessions: vi.fn(() => false), + getDeleteSession: vi.fn(() => undefined), + setSessionId: vi.fn(), + getSessionId: vi.fn().mockReturnValue('mock-session-id'), + getContentGeneratorConfig: vi.fn(() => ({ authType: 'google' })), + getExperimentalZedIntegration: vi.fn(() => false), + isBrowserLaunchSuppressed: vi.fn(() => false), + setRemoteAdminSettings: vi.fn(), + isYoloModeDisabled: vi.fn(() => false), + isPlanEnabled: vi.fn(() => false), + isEventDrivenSchedulerEnabled: vi.fn(() => false), + getCoreTools: vi.fn(() => []), + getAllowedTools: vi.fn(() => []), + getApprovalMode: vi.fn(() => 'default'), + getFileFilteringRespectGitIgnore: vi.fn(() => true), + getOutputFormat: vi.fn(() => 'text'), + getUsageStatisticsEnabled: vi.fn(() => true), + getScreenReader: vi.fn(() => false), + getGeminiMdFileCount: vi.fn(() => 0), + getDeferredCommand: vi.fn(() => undefined), + getFileSystemService: vi.fn(() => ({})), + clientVersion: '1.0.0', + getModel: vi.fn().mockReturnValue('gemini-pro'), + getWorkingDir: vi.fn().mockReturnValue('/mock/cwd'), + getToolRegistry: vi.fn().mockReturnValue({ + getTools: vi.fn().mockReturnValue([]), + getAllTools: vi.fn().mockReturnValue([]), + }), + getAgentRegistry: vi.fn().mockReturnValue({}), + getPromptRegistry: vi.fn().mockReturnValue({}), + getResourceRegistry: vi.fn().mockReturnValue({}), + getSkillManager: vi.fn().mockReturnValue({ + isAdminEnabled: vi.fn().mockReturnValue(false), + }), + getFileService: vi.fn().mockReturnValue({}), + getGitService: vi.fn().mockResolvedValue({}), + getUserMemory: vi.fn().mockReturnValue(''), + getGeminiMdFilePaths: vi.fn().mockReturnValue([]), + getShowMemoryUsage: vi.fn().mockReturnValue(false), + getAccessibility: vi.fn().mockReturnValue({}), + getTelemetryEnabled: vi.fn().mockReturnValue(false), + getTelemetryLogPromptsEnabled: vi.fn().mockReturnValue(false), + getTelemetryOtlpEndpoint: vi.fn().mockReturnValue(''), + getTelemetryOtlpProtocol: vi.fn().mockReturnValue('grpc'), + getTelemetryTarget: vi.fn().mockReturnValue(''), + getTelemetryOutfile: vi.fn().mockReturnValue(undefined), + getTelemetryUseCollector: vi.fn().mockReturnValue(false), + getTelemetryUseCliAuth: vi.fn().mockReturnValue(false), + getGeminiClient: vi.fn().mockReturnValue({ + isInitialized: vi.fn().mockReturnValue(true), + }), + updateSystemInstructionIfInitialized: vi.fn().mockResolvedValue(undefined), + getModelRouterService: vi.fn().mockReturnValue({}), + getModelAvailabilityService: vi.fn().mockReturnValue({}), + getEnableRecursiveFileSearch: vi.fn().mockReturnValue(true), + getFileFilteringEnableFuzzySearch: vi.fn().mockReturnValue(true), + getFileFilteringRespectGeminiIgnore: vi.fn().mockReturnValue(true), + getFileFilteringOptions: vi.fn().mockReturnValue({}), + getCustomExcludes: vi.fn().mockReturnValue([]), + getCheckpointingEnabled: vi.fn().mockReturnValue(false), + getProxy: vi.fn().mockReturnValue(undefined), + getBugCommand: vi.fn().mockReturnValue(undefined), + getExtensionManagement: vi.fn().mockReturnValue(true), + getExtensionLoader: vi.fn().mockReturnValue({}), + getEnabledExtensions: vi.fn().mockReturnValue([]), + getEnableExtensionReloading: vi.fn().mockReturnValue(false), + getDisableLLMCorrection: vi.fn().mockReturnValue(false), + getNoBrowser: vi.fn().mockReturnValue(false), + getAgentsSettings: vi.fn().mockReturnValue({}), + getSummarizeToolOutputConfig: vi.fn().mockReturnValue(undefined), + getIdeMode: vi.fn().mockReturnValue(false), + getFolderTrust: vi.fn().mockReturnValue(true), + isTrustedFolder: vi.fn().mockReturnValue(true), + getCompressionThreshold: vi.fn().mockResolvedValue(undefined), + getUserCaching: vi.fn().mockResolvedValue(false), + getNumericalRoutingEnabled: vi.fn().mockResolvedValue(false), + getClassifierThreshold: vi.fn().mockResolvedValue(undefined), + getBannerTextNoCapacityIssues: vi.fn().mockResolvedValue(''), + getBannerTextCapacityIssues: vi.fn().mockResolvedValue(''), + isInteractiveShellEnabled: vi.fn().mockReturnValue(false), + isSkillsSupportEnabled: vi.fn().mockReturnValue(false), + reloadSkills: vi.fn().mockResolvedValue(undefined), + reloadAgents: vi.fn().mockResolvedValue(undefined), + getUseRipgrep: vi.fn().mockReturnValue(false), + getEnableInteractiveShell: vi.fn().mockReturnValue(false), + getSkipNextSpeakerCheck: vi.fn().mockReturnValue(false), + getContinueOnFailedApiCall: vi.fn().mockReturnValue(false), + getRetryFetchErrors: vi.fn().mockReturnValue(false), + getEnableShellOutputEfficiency: vi.fn().mockReturnValue(true), + getShellToolInactivityTimeout: vi.fn().mockReturnValue(300000), + getShellExecutionConfig: vi.fn().mockReturnValue({}), + setShellExecutionConfig: vi.fn(), + getEnablePromptCompletion: vi.fn().mockReturnValue(false), + getEnableToolOutputTruncation: vi.fn().mockReturnValue(true), + getTruncateToolOutputThreshold: vi.fn().mockReturnValue(1000), + getTruncateToolOutputLines: vi.fn().mockReturnValue(100), + getNextCompressionTruncationId: vi.fn().mockReturnValue(1), + getUseWriteTodos: vi.fn().mockReturnValue(false), + getFileExclusions: vi.fn().mockReturnValue({}), + getEnableHooks: vi.fn().mockReturnValue(true), + getEnableHooksUI: vi.fn().mockReturnValue(true), + getMcpClientManager: vi.fn().mockReturnValue({ + getMcpInstructions: vi.fn().mockReturnValue(''), + getMcpServers: vi.fn().mockReturnValue({}), + }), + getEnableEventDrivenScheduler: vi.fn().mockReturnValue(false), + getAdminSkillsEnabled: vi.fn().mockReturnValue(false), + getDisabledSkills: vi.fn().mockReturnValue([]), + getExperimentalJitContext: vi.fn().mockReturnValue(false), + getTerminalBackground: vi.fn().mockReturnValue(undefined), + getEmbeddingModel: vi.fn().mockReturnValue('embedding-model'), + getQuotaErrorOccurred: vi.fn().mockReturnValue(false), + getMaxSessionTurns: vi.fn().mockReturnValue(100), + getExcludeTools: vi.fn().mockReturnValue(new Set()), + getAllowedMcpServers: vi.fn().mockReturnValue([]), + getBlockedMcpServers: vi.fn().mockReturnValue([]), + getExperiments: vi.fn().mockReturnValue(undefined), + getPreviewFeatures: vi.fn().mockReturnValue(false), + getHasAccessToPreviewModel: vi.fn().mockReturnValue(false), + ...overrides, + }) as unknown as Config; + +/** + * Creates a mocked LoadedSettings object for tests. + */ +export function createMockSettings( + overrides: Record = {}, +): LoadedSettings { + const merged = createTestMergedSettings( + (overrides['merged'] as Partial) || {}, + ); + + return { + system: { settings: {} }, + systemDefaults: { settings: {} }, + user: { settings: {} }, + workspace: { settings: {} }, + errors: [], + ...overrides, + merged, + } as unknown as LoadedSettings; +} diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index b7f4060c15..7c10569902 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -224,7 +224,7 @@ export const AppContainer = (props: AppContainerProps) => { const activeHooks = useHookDisplayState(); const [updateInfo, setUpdateInfo] = useState(null); const [isTrustedFolder, setIsTrustedFolder] = useState( - isWorkspaceTrusted(settings.merged).isTrusted, + () => isWorkspaceTrusted(settings.merged).isTrusted, ); const [queueErrorMessage, setQueueErrorMessage] = useState( diff --git a/packages/cli/src/ui/components/FolderTrustDialog.test.tsx b/packages/cli/src/ui/components/FolderTrustDialog.test.tsx index 8bf6a634cd..9ba7f19cb6 100644 --- a/packages/cli/src/ui/components/FolderTrustDialog.test.tsx +++ b/packages/cli/src/ui/components/FolderTrustDialog.test.tsx @@ -32,11 +32,12 @@ vi.mock('node:process', async () => { describe('FolderTrustDialog', () => { beforeEach(() => { vi.clearAllMocks(); + vi.useRealTimers(); mockedCwd.mockReturnValue('/home/user/project'); }); it('should render the dialog with title and description', () => { - const { lastFrame } = renderWithProviders( + const { lastFrame, unmount } = renderWithProviders( , ); @@ -44,11 +45,12 @@ describe('FolderTrustDialog', () => { expect(lastFrame()).toContain( 'Trusting a folder allows Gemini to execute commands it suggests.', ); + unmount(); }); it('should display exit message and call process.exit and not call onSelect when escape is pressed', async () => { const onSelect = vi.fn(); - const { lastFrame, stdin } = renderWithProviders( + const { lastFrame, stdin, unmount } = renderWithProviders( , ); @@ -67,24 +69,27 @@ describe('FolderTrustDialog', () => { ); }); expect(onSelect).not.toHaveBeenCalled(); + unmount(); }); it('should display restart message when isRestarting is true', () => { - const { lastFrame } = renderWithProviders( + const { lastFrame, unmount } = renderWithProviders( , ); expect(lastFrame()).toContain('Gemini CLI is restarting'); + unmount(); }); it('should call relaunchApp when isRestarting is true', async () => { vi.useFakeTimers(); const relaunchApp = vi.spyOn(processUtils, 'relaunchApp'); - renderWithProviders( + const { unmount } = renderWithProviders( , ); await vi.advanceTimersByTimeAsync(250); expect(relaunchApp).toHaveBeenCalled(); + unmount(); vi.useRealTimers(); }); @@ -106,7 +111,7 @@ describe('FolderTrustDialog', () => { }); it('should not call process.exit when "r" is pressed and isRestarting is false', async () => { - const { stdin } = renderWithProviders( + const { stdin, unmount } = renderWithProviders( , ); @@ -117,31 +122,35 @@ describe('FolderTrustDialog', () => { await waitFor(() => { expect(mockedExit).not.toHaveBeenCalled(); }); + unmount(); }); describe('directory display', () => { it('should correctly display the folder name for a nested directory', () => { mockedCwd.mockReturnValue('/home/user/project'); - const { lastFrame } = renderWithProviders( + const { lastFrame, unmount } = renderWithProviders( , ); expect(lastFrame()).toContain('Trust folder (project)'); + unmount(); }); it('should correctly display the parent folder name for a nested directory', () => { mockedCwd.mockReturnValue('/home/user/project'); - const { lastFrame } = renderWithProviders( + const { lastFrame, unmount } = renderWithProviders( , ); expect(lastFrame()).toContain('Trust parent folder (user)'); + unmount(); }); it('should correctly display an empty parent folder name for a directory directly under root', () => { mockedCwd.mockReturnValue('/project'); - const { lastFrame } = renderWithProviders( + const { lastFrame, unmount } = renderWithProviders( , ); expect(lastFrame()).toContain('Trust parent folder ()'); + unmount(); }); }); }); diff --git a/packages/cli/src/ui/hooks/toolMapping.test.ts b/packages/cli/src/ui/hooks/toolMapping.test.ts index 41dc974adb..b40c3c7dea 100644 --- a/packages/cli/src/ui/hooks/toolMapping.test.ts +++ b/packages/cli/src/ui/hooks/toolMapping.test.ts @@ -21,17 +21,6 @@ import { } from '@google/gemini-cli-core'; import { ToolCallStatus } from '../types.js'; -vi.mock('@google/gemini-cli-core', async (importOriginal) => { - const actual = - await importOriginal(); - return { - ...actual, - debugLogger: { - warn: vi.fn(), - }, - }; -}); - describe('toolMapping', () => { beforeEach(() => { vi.clearAllMocks(); diff --git a/packages/cli/src/ui/hooks/useThemeCommand.ts b/packages/cli/src/ui/hooks/useThemeCommand.ts index 38a06ea32e..790019db15 100644 --- a/packages/cli/src/ui/hooks/useThemeCommand.ts +++ b/packages/cli/src/ui/hooks/useThemeCommand.ts @@ -74,7 +74,6 @@ export const useThemeCommand = ( const handleThemeSelect = useCallback( (themeName: string, scope: LoadableSettingScope) => { try { - // Merge user and workspace custom themes (workspace takes precedence) const mergedCustomThemes = { ...(loadedSettings.user.settings.ui?.customThemes || {}), ...(loadedSettings.workspace.settings.ui?.customThemes || {}), diff --git a/packages/cli/src/ui/utils/textUtils.ts b/packages/cli/src/ui/utils/textUtils.ts index 569ede8697..4d3cd1ded5 100644 --- a/packages/cli/src/ui/utils/textUtils.ts +++ b/packages/cli/src/ui/utils/textUtils.ts @@ -30,6 +30,18 @@ export const getAsciiArtWidth = (asciiArt: string): number => { * code units so that surrogate‑pair emoji count as one "column".) * ---------------------------------------------------------------------- */ +/** + * Checks if a string contains only ASCII characters (0-127). + */ +export function isAscii(str: string): boolean { + for (let i = 0; i < str.length; i++) { + if (str.charCodeAt(i) > 127) { + return false; + } + } + return true; +} + // Cache for code points const MAX_STRING_LENGTH_TO_CACHE = 1000; const codePointsCache = new LRUCache( @@ -37,15 +49,8 @@ const codePointsCache = new LRUCache( ); export function toCodePoints(str: string): string[] { - // ASCII fast path - check if all chars are ASCII (0-127) - let isAscii = true; - for (let i = 0; i < str.length; i++) { - if (str.charCodeAt(i) > 127) { - isAscii = false; - break; - } - } - if (isAscii) { + // ASCII fast path + if (isAscii(str)) { return str.split(''); } @@ -68,6 +73,9 @@ export function toCodePoints(str: string): string[] { } export function cpLen(str: string): number { + if (isAscii(str)) { + return str.length; + } return toCodePoints(str).length; } @@ -79,6 +87,9 @@ export function cpIndexToOffset(str: string, cpIndex: number): number { } export function cpSlice(str: string, start: number, end?: number): string { + if (isAscii(str)) { + return str.slice(start, end); + } // Slice by code‑point indices and re‑join. const arr = toCodePoints(str).slice(start, end); return arr.join(''); diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 6ec0444d80..d5066d6437 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -918,6 +918,7 @@ export class Config { await this.getSkillManager().discoverSkills( this.storage, this.getExtensions(), + this.isTrustedFolder(), ); this.getSkillManager().setDisabledSkills(this.disabledSkills); @@ -1924,6 +1925,7 @@ export class Config { await this.getSkillManager().discoverSkills( this.storage, this.getExtensions(), + this.isTrustedFolder(), ); this.getSkillManager().setDisabledSkills(this.disabledSkills); diff --git a/packages/core/src/services/contextManager.test.ts b/packages/core/src/services/contextManager.test.ts index 4a86100812..ce487ea973 100644 --- a/packages/core/src/services/contextManager.test.ts +++ b/packages/core/src/services/contextManager.test.ts @@ -40,6 +40,7 @@ describe('ContextManager', () => { getMcpClientManager: vi.fn().mockReturnValue({ getMcpInstructions: vi.fn().mockReturnValue('MCP Instructions'), }), + isTrustedFolder: vi.fn().mockReturnValue(true), } as unknown as Config; contextManager = new ContextManager(mockConfig); @@ -112,6 +113,24 @@ describe('ContextManager', () => { fileCount: 2, }); }); + + it('should not load environment memory if folder is not trusted', async () => { + vi.mocked(mockConfig.isTrustedFolder).mockReturnValue(false); + const mockGlobalResult = { + files: [ + { path: '/home/user/.gemini/GEMINI.md', content: 'Global Content' }, + ], + }; + vi.mocked(memoryDiscovery.loadGlobalMemory).mockResolvedValue( + mockGlobalResult, + ); + + await contextManager.refresh(); + + expect(memoryDiscovery.loadEnvironmentMemory).not.toHaveBeenCalled(); + expect(contextManager.getEnvironmentMemory()).toBe(''); + expect(contextManager.getGlobalMemory()).toContain('Global Content'); + }); }); describe('discoverContext', () => { @@ -150,5 +169,16 @@ describe('ContextManager', () => { expect(result).toBe(''); }); + + it('should return empty string if folder is not trusted', async () => { + vi.mocked(mockConfig.isTrustedFolder).mockReturnValue(false); + + const result = await contextManager.discoverContext('/app/src/file.ts', [ + '/app', + ]); + + expect(memoryDiscovery.loadJitSubdirectoryMemory).not.toHaveBeenCalled(); + expect(result).toBe(''); + }); }); }); diff --git a/packages/core/src/services/contextManager.ts b/packages/core/src/services/contextManager.ts index 01a10a5f77..ec161988c3 100644 --- a/packages/core/src/services/contextManager.ts +++ b/packages/core/src/services/contextManager.ts @@ -43,6 +43,10 @@ export class ContextManager { } private async loadEnvironmentMemory(): Promise { + if (!this.config.isTrustedFolder()) { + this.environmentMemory = ''; + return; + } const result = await loadEnvironmentMemory( [...this.config.getWorkspaceContext().getDirectories()], this.config.getExtensionLoader(), @@ -68,6 +72,9 @@ export class ContextManager { accessedPath: string, trustedRoots: string[], ): Promise { + if (!this.config.isTrustedFolder()) { + return ''; + } const result = await loadJitSubdirectoryMemory( accessedPath, trustedRoots, @@ -101,9 +108,7 @@ export class ContextManager { } private markAsLoaded(paths: string[]): void { - for (const p of paths) { - this.loadedPaths.add(p); - } + paths.forEach((p) => this.loadedPaths.add(p)); } getLoadedPaths(): ReadonlySet { diff --git a/packages/core/src/skills/skillManager.test.ts b/packages/core/src/skills/skillManager.test.ts index 0171ca0f61..06a6bdb1a4 100644 --- a/packages/core/src/skills/skillManager.test.ts +++ b/packages/core/src/skills/skillManager.test.ts @@ -78,13 +78,19 @@ description: project-desc }; vi.spyOn(Storage, 'getUserSkillsDir').mockReturnValue(userDir); + vi.spyOn(Storage, 'getUserAgentSkillsDir').mockReturnValue( + '/non-existent-user-agent', + ); const storage = new Storage('/dummy'); vi.spyOn(storage, 'getProjectSkillsDir').mockReturnValue(projectDir); + vi.spyOn(storage, 'getProjectAgentSkillsDir').mockReturnValue( + '/non-existent-project-agent', + ); const service = new SkillManager(); // @ts-expect-error accessing private method for testing vi.spyOn(service, 'discoverBuiltinSkills').mockResolvedValue(undefined); - await service.discoverSkills(storage, [mockExtension]); + await service.discoverSkills(storage, [mockExtension], true); const skills = service.getSkills(); expect(skills).toHaveLength(3); @@ -135,13 +141,19 @@ description: project-desc }; vi.spyOn(Storage, 'getUserSkillsDir').mockReturnValue(userDir); + vi.spyOn(Storage, 'getUserAgentSkillsDir').mockReturnValue( + '/non-existent-user-agent', + ); const storage = new Storage('/dummy'); vi.spyOn(storage, 'getProjectSkillsDir').mockReturnValue(projectDir); + vi.spyOn(storage, 'getProjectAgentSkillsDir').mockReturnValue( + '/non-existent-project-agent', + ); const service = new SkillManager(); // @ts-expect-error accessing private method for testing vi.spyOn(service, 'discoverBuiltinSkills').mockResolvedValue(undefined); - await service.discoverSkills(storage, [mockExtension]); + await service.discoverSkills(storage, [mockExtension], true); const skills = service.getSkills(); expect(skills).toHaveLength(1); @@ -149,7 +161,7 @@ description: project-desc // Test User > Extension vi.spyOn(storage, 'getProjectSkillsDir').mockReturnValue('/non-existent'); - await service.discoverSkills(storage, [mockExtension]); + await service.discoverSkills(storage, [mockExtension], true); expect(service.getSkills()[0].description).toBe('user-desc'); }); @@ -173,7 +185,7 @@ description: project-desc vi.spyOn(storage, 'getProjectSkillsDir').mockReturnValue('/non-existent'); vi.spyOn(Storage, 'getUserSkillsDir').mockReturnValue('/non-existent'); - await service.discoverSkills(storage); + await service.discoverSkills(storage, [], true); const skills = service.getSkills(); expect(skills).toHaveLength(1); @@ -196,12 +208,18 @@ body1`, const storage = new Storage('/dummy'); vi.spyOn(storage, 'getProjectSkillsDir').mockReturnValue(testRootDir); + vi.spyOn(storage, 'getProjectAgentSkillsDir').mockReturnValue( + '/non-existent-project-agent', + ); vi.spyOn(Storage, 'getUserSkillsDir').mockReturnValue('/non-existent'); + vi.spyOn(Storage, 'getUserAgentSkillsDir').mockReturnValue( + '/non-existent-user-agent', + ); const service = new SkillManager(); // @ts-expect-error accessing private method for testing vi.spyOn(service, 'discoverBuiltinSkills').mockResolvedValue(undefined); - await service.discoverSkills(storage); + await service.discoverSkills(storage, [], true); service.setDisabledSkills(['skill1']); expect(service.getSkills()).toHaveLength(0); @@ -209,6 +227,40 @@ body1`, expect(service.getAllSkills()[0].disabled).toBe(true); }); + it('should skip workspace skills if folder is not trusted', async () => { + const projectDir = path.join(testRootDir, 'workspace'); + await fs.mkdir(path.join(projectDir, 'skill-project'), { recursive: true }); + + await fs.writeFile( + path.join(projectDir, 'skill-project', 'SKILL.md'), + `--- +name: skill-project +description: project-desc +--- +`, + ); + + const storage = new Storage('/dummy'); + vi.spyOn(storage, 'getProjectSkillsDir').mockReturnValue(projectDir); + vi.spyOn(storage, 'getProjectAgentSkillsDir').mockReturnValue( + '/non-existent-project-agent', + ); + vi.spyOn(Storage, 'getUserSkillsDir').mockReturnValue('/non-existent'); + vi.spyOn(Storage, 'getUserAgentSkillsDir').mockReturnValue( + '/non-existent-user-agent', + ); + + const service = new SkillManager(); + // @ts-expect-error accessing private method for testing + vi.spyOn(service, 'discoverBuiltinSkills').mockResolvedValue(undefined); + + // Call with isTrusted = false + await service.discoverSkills(storage, [], false); + + const skills = service.getSkills(); + expect(skills).toHaveLength(0); + }); + it('should filter built-in skills in getDisplayableSkills', async () => { const service = new SkillManager(); @@ -303,14 +355,20 @@ body1`, }); vi.spyOn(Storage, 'getUserSkillsDir').mockReturnValue(userDir); + vi.spyOn(Storage, 'getUserAgentSkillsDir').mockReturnValue( + '/non-existent-user-agent', + ); const storage = new Storage('/dummy'); vi.spyOn(storage, 'getProjectSkillsDir').mockReturnValue(projectDir); + vi.spyOn(storage, 'getProjectAgentSkillsDir').mockReturnValue( + '/non-existent-project-agent', + ); const service = new SkillManager(); // @ts-expect-error accessing private method for testing vi.spyOn(service, 'discoverBuiltinSkills').mockResolvedValue(undefined); - await service.discoverSkills(storage, []); + await service.discoverSkills(storage, [], true); expect(emitFeedbackSpy).toHaveBeenCalledWith( 'warning', @@ -356,12 +414,18 @@ body1`, }); vi.spyOn(Storage, 'getUserSkillsDir').mockReturnValue(userDir); + vi.spyOn(Storage, 'getUserAgentSkillsDir').mockReturnValue( + '/non-existent-user-agent', + ); const storage = new Storage('/dummy'); vi.spyOn(storage, 'getProjectSkillsDir').mockReturnValue('/non-existent'); + vi.spyOn(storage, 'getProjectAgentSkillsDir').mockReturnValue( + '/non-existent-project-agent', + ); const service = new SkillManager(); - await service.discoverSkills(storage, []); + await service.discoverSkills(storage, [], true); // UI warning should not be called expect(emitFeedbackSpy).not.toHaveBeenCalled(); diff --git a/packages/core/src/skills/skillManager.ts b/packages/core/src/skills/skillManager.ts index be0d3d81ff..02e9d72898 100644 --- a/packages/core/src/skills/skillManager.ts +++ b/packages/core/src/skills/skillManager.ts @@ -47,6 +47,7 @@ export class SkillManager { async discoverSkills( storage: Storage, extensions: GeminiCLIExtension[] = [], + isTrusted: boolean = false, ): Promise { this.clearSkills(); @@ -71,6 +72,13 @@ export class SkillManager { this.addSkillsWithPrecedence(userAgentSkills); // 4. Workspace skills (highest precedence) + if (!isTrusted) { + debugLogger.debug( + 'Workspace skills disabled because folder is not trusted.', + ); + return; + } + const projectSkills = await loadSkillsFromDir( storage.getProjectSkillsDir(), ); diff --git a/packages/core/src/skills/skillManagerAlias.test.ts b/packages/core/src/skills/skillManagerAlias.test.ts index 0764721de9..8c02ba8a11 100644 --- a/packages/core/src/skills/skillManagerAlias.test.ts +++ b/packages/core/src/skills/skillManagerAlias.test.ts @@ -112,7 +112,7 @@ describe('SkillManager Alias', () => { // @ts-expect-error accessing private method for testing vi.spyOn(service, 'discoverBuiltinSkills').mockResolvedValue(undefined); - await service.discoverSkills(storage, []); + await service.discoverSkills(storage, [], true); const skills = service.getSkills(); expect(skills).toHaveLength(4); @@ -169,7 +169,7 @@ describe('SkillManager Alias', () => { // @ts-expect-error accessing private method for testing vi.spyOn(service, 'discoverBuiltinSkills').mockResolvedValue(undefined); - await service.discoverSkills(storage, []); + await service.discoverSkills(storage, [], true); const skills = service.getSkills(); expect(skills).toHaveLength(1);