fix(cli): Exit CLI when trust save unsuccessful during launch (#11968)

This commit is contained in:
shrutip90
2025-11-14 11:56:39 -08:00
committed by GitHub
parent 6d83d3440c
commit d683e1c0db
7 changed files with 174 additions and 30 deletions

View File

@@ -76,11 +76,17 @@ export class LoadedTrustedFolders {
* @param location path
* @returns
*/
isPathTrusted(location: string): boolean | undefined {
isPathTrusted(
location: string,
config?: Record<string, TrustLevel>,
): 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<string, TrustLevel>,
): 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,

View File

@@ -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();

View File

@@ -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 },

View File

@@ -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);
});
});

View File

@@ -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;

View File

@@ -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);
});
});

View File

@@ -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 {