diff --git a/packages/cli/src/config/trustedFolders.ts b/packages/cli/src/config/trustedFolders.ts index 6602ec85cb..d308404ba3 100644 --- a/packages/cli/src/config/trustedFolders.ts +++ b/packages/cli/src/config/trustedFolders.ts @@ -76,11 +76,17 @@ export class LoadedTrustedFolders { * @param location path * @returns */ - isPathTrusted(location: string): boolean | undefined { + isPathTrusted( + location: string, + config?: Record, + ): boolean | undefined { + const configToUse = config ?? this.user.config; const trustedPaths: string[] = []; const untrustedPaths: string[] = []; - for (const rule of this.rules) { + for (const rule of Object.entries(configToUse).map( + ([path, trustLevel]) => ({ path, trustLevel }), + )) { switch (rule.trustLevel) { case TrustLevel.TRUST_FOLDER: trustedPaths.push(rule.path); @@ -113,8 +119,19 @@ export class LoadedTrustedFolders { } setValue(path: string, trustLevel: TrustLevel): void { + const originalTrustLevel = this.user.config[path]; this.user.config[path] = trustLevel; - saveTrustedFolders(this.user); + try { + saveTrustedFolders(this.user); + } catch (e) { + // Revert the in-memory change if the save failed. + if (originalTrustLevel === undefined) { + delete this.user.config[path]; + } else { + this.user.config[path] = originalTrustLevel; + } + throw e; + } } } @@ -174,21 +191,17 @@ export function loadTrustedFolders(): LoadedTrustedFolders { export function saveTrustedFolders( trustedFoldersFile: TrustedFoldersFile, ): void { - try { - // Ensure the directory exists - const dirPath = path.dirname(trustedFoldersFile.path); - if (!fs.existsSync(dirPath)) { - fs.mkdirSync(dirPath, { recursive: true }); - } - - fs.writeFileSync( - trustedFoldersFile.path, - JSON.stringify(trustedFoldersFile.config, null, 2), - { encoding: 'utf-8', mode: 0o600 }, - ); - } catch (error) { - console.error('Error saving trusted folders file:', error); + // Ensure the directory exists + const dirPath = path.dirname(trustedFoldersFile.path); + if (!fs.existsSync(dirPath)) { + fs.mkdirSync(dirPath, { recursive: true }); } + + fs.writeFileSync( + trustedFoldersFile.path, + JSON.stringify(trustedFoldersFile.config, null, 2), + { encoding: 'utf-8', mode: 0o600 }, + ); } /** Is folder trust feature enabled per the current applied settings */ @@ -201,10 +214,7 @@ function getWorkspaceTrustFromLocalConfig( trustConfig?: Record, ): TrustResult { const folders = loadTrustedFolders(); - - if (trustConfig) { - folders.user.config = trustConfig; - } + const configToUse = trustConfig ?? folders.user.config; if (folders.errors.length > 0) { const errorMessages = folders.errors.map( @@ -215,7 +225,7 @@ function getWorkspaceTrustFromLocalConfig( ); } - const isTrusted = folders.isPathTrusted(process.cwd()); + const isTrusted = folders.isPathTrusted(process.cwd(), configToUse); return { isTrusted, source: isTrusted !== undefined ? 'file' : undefined, diff --git a/packages/cli/src/ui/components/PermissionsModifyTrustDialog.test.tsx b/packages/cli/src/ui/components/PermissionsModifyTrustDialog.test.tsx index bc065ded42..00247cedff 100644 --- a/packages/cli/src/ui/components/PermissionsModifyTrustDialog.test.tsx +++ b/packages/cli/src/ui/components/PermissionsModifyTrustDialog.test.tsx @@ -148,10 +148,11 @@ describe('PermissionsModifyTrustDialog', () => { }); }); - it('should commit, restart, and exit on `r` keypress', async () => { + it('should commit and restart `r` keypress', async () => { const mockRelaunchApp = vi .spyOn(processUtils, 'relaunchApp') .mockResolvedValue(undefined); + mockCommitTrustLevelChange.mockReturnValue(true); vi.mocked(usePermissionsModifyTrust).mockReturnValue({ cwd: '/test/dir', currentTrustLevel: TrustLevel.DO_NOT_TRUST, @@ -175,7 +176,6 @@ describe('PermissionsModifyTrustDialog', () => { await waitFor(() => { expect(mockCommitTrustLevelChange).toHaveBeenCalled(); expect(mockRelaunchApp).toHaveBeenCalled(); - expect(onExit).toHaveBeenCalled(); }); mockRelaunchApp.mockRestore(); diff --git a/packages/cli/src/ui/components/PermissionsModifyTrustDialog.tsx b/packages/cli/src/ui/components/PermissionsModifyTrustDialog.tsx index 3814e9ecc6..211d405e40 100644 --- a/packages/cli/src/ui/components/PermissionsModifyTrustDialog.tsx +++ b/packages/cli/src/ui/components/PermissionsModifyTrustDialog.tsx @@ -62,9 +62,12 @@ export function PermissionsModifyTrustDialog({ onExit(); } if (needsRestart && key.name === 'r') { - commitTrustLevelChange(); - relaunchApp(); - onExit(); + const success = commitTrustLevelChange(); + if (success) { + relaunchApp(); + } else { + onExit(); + } } }, { isActive: true }, diff --git a/packages/cli/src/ui/hooks/useFolderTrust.test.ts b/packages/cli/src/ui/hooks/useFolderTrust.test.ts index e97c23da46..ecfccf1f64 100644 --- a/packages/cli/src/ui/hooks/useFolderTrust.test.ts +++ b/packages/cli/src/ui/hooks/useFolderTrust.test.ts @@ -14,8 +14,10 @@ import { FolderTrustChoice } from '../components/FolderTrustDialog.js'; import type { LoadedTrustedFolders } from '../../config/trustedFolders.js'; import { TrustLevel } from '../../config/trustedFolders.js'; import * as trustedFolders from '../../config/trustedFolders.js'; +import { coreEvents } from '@google/gemini-cli-core'; const mockedCwd = vi.hoisted(() => vi.fn()); +const mockedExit = vi.hoisted(() => vi.fn()); vi.mock('node:process', async () => { const actual = @@ -23,6 +25,7 @@ vi.mock('node:process', async () => { return { ...actual, cwd: mockedCwd, + exit: mockedExit, platform: 'linux', }; }); @@ -35,6 +38,7 @@ describe('useFolderTrust', () => { let addItem: Mock; beforeEach(() => { + vi.useFakeTimers(); mockSettings = { merged: { security: { @@ -60,6 +64,7 @@ describe('useFolderTrust', () => { }); afterEach(() => { + vi.useRealTimers(); vi.clearAllMocks(); }); @@ -260,4 +265,30 @@ describe('useFolderTrust', () => { expect(result.current.isRestarting).toBe(false); expect(result.current.isFolderTrustDialogOpen).toBe(false); // Dialog should close }); + + it('should emit feedback on failure to set value', () => { + isWorkspaceTrustedSpy.mockReturnValue({ + isTrusted: undefined, + source: undefined, + }); + (mockTrustedFolders.setValue as Mock).mockImplementation(() => { + throw new Error('test error'); + }); + const emitFeedbackSpy = vi.spyOn(coreEvents, 'emitFeedback'); + const { result } = renderHook(() => + useFolderTrust(mockSettings, onTrustChange, addItem), + ); + + act(() => { + result.current.handleFolderTrustSelect(FolderTrustChoice.TRUST_FOLDER); + }); + + vi.runAllTimers(); + + expect(emitFeedbackSpy).toHaveBeenCalledWith( + 'error', + 'Failed to save trust settings. Exiting Gemini CLI.', + ); + expect(mockedExit).toHaveBeenCalledWith(1); + }); }); diff --git a/packages/cli/src/ui/hooks/useFolderTrust.ts b/packages/cli/src/ui/hooks/useFolderTrust.ts index 55b57faed4..079d727b78 100644 --- a/packages/cli/src/ui/hooks/useFolderTrust.ts +++ b/packages/cli/src/ui/hooks/useFolderTrust.ts @@ -14,6 +14,7 @@ import { } from '../../config/trustedFolders.js'; import * as process from 'node:process'; import { type HistoryItemWithoutId, MessageType } from '../types.js'; +import { coreEvents } from '@google/gemini-cli-core'; export const useFolderTrust = ( settings: LoadedSettings, @@ -67,7 +68,19 @@ export const useFolderTrust = ( return; } - trustedFolders.setValue(cwd, trustLevel); + try { + trustedFolders.setValue(cwd, trustLevel); + } catch (_e) { + coreEvents.emitFeedback( + 'error', + 'Failed to save trust settings. Exiting Gemini CLI.', + ); + setTimeout(() => { + process.exit(1); + }, 100); + return; + } + const currentIsTrusted = trustLevel === TrustLevel.TRUST_FOLDER || trustLevel === TrustLevel.TRUST_PARENT; diff --git a/packages/cli/src/ui/hooks/usePermissionsModifyTrust.test.ts b/packages/cli/src/ui/hooks/usePermissionsModifyTrust.test.ts index d317170c18..bfe35bfb05 100644 --- a/packages/cli/src/ui/hooks/usePermissionsModifyTrust.test.ts +++ b/packages/cli/src/ui/hooks/usePermissionsModifyTrust.test.ts @@ -19,6 +19,7 @@ import { usePermissionsModifyTrust } from './usePermissionsModifyTrust.js'; import { TrustLevel } from '../../config/trustedFolders.js'; import type { LoadedSettings } from '../../config/settings.js'; import type { LoadedTrustedFolders } from '../../config/trustedFolders.js'; +import { coreEvents } from '@google/gemini-cli-core'; // Hoist mocks const mockedCwd = vi.hoisted(() => vi.fn()); @@ -259,4 +260,70 @@ describe('usePermissionsModifyTrust', () => { expect.any(Number), ); }); + + it('should emit feedback when setValue throws in updateTrustLevel', () => { + const mockSetValue = vi.fn().mockImplementation(() => { + throw new Error('test error'); + }); + mockedLoadTrustedFolders.mockReturnValue({ + user: { config: {} }, + setValue: mockSetValue, + } as unknown as LoadedTrustedFolders); + + mockedIsWorkspaceTrusted.mockReturnValue({ + isTrusted: true, + source: 'file', + }); + + const emitFeedbackSpy = vi.spyOn(coreEvents, 'emitFeedback'); + + const { result } = renderHook(() => + usePermissionsModifyTrust(mockOnExit, mockAddItem), + ); + + act(() => { + result.current.updateTrustLevel(TrustLevel.TRUST_PARENT); + }); + + expect(emitFeedbackSpy).toHaveBeenCalledWith( + 'error', + 'Failed to save trust settings. Your changes may not persist.', + ); + expect(mockOnExit).toHaveBeenCalled(); + }); + + it('should emit feedback when setValue throws in commitTrustLevelChange', () => { + const mockSetValue = vi.fn().mockImplementation(() => { + throw new Error('test error'); + }); + mockedLoadTrustedFolders.mockReturnValue({ + user: { config: {} }, + setValue: mockSetValue, + } as unknown as LoadedTrustedFolders); + + mockedIsWorkspaceTrusted + .mockReturnValueOnce({ isTrusted: false, source: 'file' }) + .mockReturnValueOnce({ isTrusted: true, source: 'file' }); + + const emitFeedbackSpy = vi.spyOn(coreEvents, 'emitFeedback'); + + const { result } = renderHook(() => + usePermissionsModifyTrust(mockOnExit, mockAddItem), + ); + + act(() => { + result.current.updateTrustLevel(TrustLevel.TRUST_FOLDER); + }); + + act(() => { + const success = result.current.commitTrustLevelChange(); + expect(success).toBe(false); + }); + + expect(emitFeedbackSpy).toHaveBeenCalledWith( + 'error', + 'Failed to save trust settings. Your changes may not persist.', + ); + expect(result.current.needsRestart).toBe(false); + }); }); diff --git a/packages/cli/src/ui/hooks/usePermissionsModifyTrust.ts b/packages/cli/src/ui/hooks/usePermissionsModifyTrust.ts index f5a10ff38f..64e2011b0d 100644 --- a/packages/cli/src/ui/hooks/usePermissionsModifyTrust.ts +++ b/packages/cli/src/ui/hooks/usePermissionsModifyTrust.ts @@ -16,6 +16,7 @@ import { useSettings } from '../contexts/SettingsContext.js'; import { MessageType } from '../types.js'; import { type UseHistoryManagerReturn } from './useHistoryManager.js'; import type { LoadedSettings } from '../../config/settings.js'; +import { coreEvents } from '@google/gemini-cli-core'; interface TrustState { currentTrustLevel: TrustLevel | undefined; @@ -101,7 +102,14 @@ export const usePermissionsModifyTrust = ( setNeedsRestart(true); } else { const folders = loadTrustedFolders(); - folders.setValue(cwd, trustLevel); + try { + folders.setValue(cwd, trustLevel); + } catch (_e) { + coreEvents.emitFeedback( + 'error', + 'Failed to save trust settings. Your changes may not persist.', + ); + } onExit(); } }, @@ -111,8 +119,20 @@ export const usePermissionsModifyTrust = ( const commitTrustLevelChange = useCallback(() => { if (pendingTrustLevel) { const folders = loadTrustedFolders(); - folders.setValue(cwd, pendingTrustLevel); + try { + folders.setValue(cwd, pendingTrustLevel); + return true; + } catch (_e) { + coreEvents.emitFeedback( + 'error', + 'Failed to save trust settings. Your changes may not persist.', + ); + setNeedsRestart(false); + setPendingTrustLevel(undefined); + return false; + } } + return true; }, [cwd, pendingTrustLevel]); return {