jcarr
/
gemini-cli
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Force restart on trust level change to reload settings (#6713)

This commit is contained in:
shrutip90 2025-08-21 00:38:12 -07:00 committed by GitHub
parent 0242ecd83a
commit ba5309c405
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 273 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

64
packages/cli/src/config/settings.test.ts
View File

@ -27,6 +27,11 @@ vi.mock('./settings.js', async (importActual) => {
};
});
// Mock trustedFolders
vi.mock('./trustedFolders.js', () => ({
isWorkspaceTrusted: vi.fn(),
}));
// NOW import everything else, including the (now effectively re-exported) settings.js
import * as pathActual from 'path'; // Restored for MOCK_WORKSPACE_SETTINGS_PATH
import {
@ -41,6 +46,7 @@ import {
} from 'vitest';
import * as fs from 'fs'; // fs will be mocked separately
import stripJsonComments from 'strip-json-comments'; // Will be mocked separately
import { isWorkspaceTrusted } from './trustedFolders.js';
// These imports will get the versions from the vi.mock('./settings.js', ...) factory.
import {
@ -97,6 +103,7 @@ describe('Settings Loading and Merging', () => {
(mockFsExistsSync as Mock).mockReturnValue(false);
(fs.readFileSync as Mock).mockReturnValue('{}'); // Return valid empty JSON
(mockFsMkdirSync as Mock).mockImplementation(() => undefined);
vi.mocked(isWorkspaceTrusted).mockReturnValue(true);
});
afterEach(() => {
@ -1421,4 +1428,61 @@ describe('Settings Loading and Merging', () => {
]);
});
});
describe('with workspace trust', () => {
it('should merge workspace settings when workspace is trusted', () => {
(mockFsExistsSync as Mock).mockReturnValue(true);
const userSettingsContent = { theme: 'dark', sandbox: false };
const workspaceSettingsContent = {
sandbox: true,
contextFileName: 'WORKSPACE.md',
};
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
return JSON.stringify(userSettingsContent);
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
return JSON.stringify(workspaceSettingsContent);
return '{}';
},
);
const settings = loadSettings(MOCK_WORKSPACE_DIR);
expect(settings.merged.sandbox).toBe(true);
expect(settings.merged.contextFileName).toBe('WORKSPACE.md');
expect(settings.merged.theme).toBe('dark');
});
it('should NOT merge workspace settings when workspace is not trusted', () => {
vi.mocked(isWorkspaceTrusted).mockReturnValue(false);
(mockFsExistsSync as Mock).mockReturnValue(true);
const userSettingsContent = {
theme: 'dark',
sandbox: false,
contextFileName: 'USER.md',
};
const workspaceSettingsContent = {
sandbox: true,
contextFileName: 'WORKSPACE.md',
};
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
return JSON.stringify(userSettingsContent);
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
return JSON.stringify(workspaceSettingsContent);
return '{}';
},
);
const settings = loadSettings(MOCK_WORKSPACE_DIR);
expect(settings.merged.sandbox).toBe(false); // User setting
expect(settings.merged.contextFileName).toBe('USER.md'); // User setting
expect(settings.merged.theme).toBe('dark'); // User setting
});
});
});

26
packages/cli/src/config/settings.ts
View File

@ -16,6 +16,7 @@ import {
import stripJsonComments from 'strip-json-comments';
import { DefaultLight } from '../ui/themes/default-light.js';
import { DefaultDark } from '../ui/themes/default.js';
import { isWorkspaceTrusted } from './trustedFolders.js';
import { Settings, MemoryImportFormat } from './settingsSchema.js';
export type { Settings, MemoryImportFormat };
@ -73,34 +74,37 @@ function mergeSettings(
system: Settings,
user: Settings,
workspace: Settings,
isTrusted: boolean,
): Settings {
const safeWorkspace = isTrusted ? workspace : ({} as Settings);
// folderTrust is not supported at workspace level.
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const { folderTrust, ...workspaceWithoutFolderTrust } = workspace;
const { folderTrust, ...safeWorkspaceWithoutFolderTrust } = safeWorkspace;
return {
...user,
...workspaceWithoutFolderTrust,
...safeWorkspaceWithoutFolderTrust,
...system,
customThemes: {
...(user.customThemes || {}),
...(workspace.customThemes || {}),
...(safeWorkspace.customThemes || {}),
...(system.customThemes || {}),
},
mcpServers: {
...(user.mcpServers || {}),
...(workspace.mcpServers || {}),
...(safeWorkspace.mcpServers || {}),
...(system.mcpServers || {}),
},
includeDirectories: [
...(system.includeDirectories || []),
...(user.includeDirectories || []),
...(workspace.includeDirectories || []),
...(safeWorkspace.includeDirectories || []),
],
chatCompression: {
...(system.chatCompression || {}),
...(user.chatCompression || {}),
...(workspace.chatCompression || {}),
...(safeWorkspace.chatCompression || {}),
},
};
}
@ -111,11 +115,13 @@ export class LoadedSettings {
user: SettingsFile,
workspace: SettingsFile,
errors: SettingsError[],
isTrusted: boolean,
) {
this.system = system;
this.user = user;
this.workspace = workspace;
this.errors = errors;
this.isTrusted = isTrusted;
this._merged = this.computeMergedSettings();
}
@ -123,6 +129,7 @@ export class LoadedSettings {
readonly user: SettingsFile;
readonly workspace: SettingsFile;
readonly errors: SettingsError[];
readonly isTrusted: boolean;
private _merged: Settings;
@ -135,6 +142,7 @@ export class LoadedSettings {
this.system.settings,
this.user.settings,
this.workspace.settings,
this.isTrusted,
);
}
@ -403,11 +411,16 @@ export function loadSettings(workspaceDir: string): LoadedSettings {
}
}
// For the initial trust check, we can only use user and system settings.
const initialTrustCheckSettings = { ...systemSettings, ...userSettings };
const isTrusted = isWorkspaceTrusted(initialTrustCheckSettings) ?? true;
// Create a temporary merged settings object to pass to loadEnvironment.
const tempMergedSettings = mergeSettings(
systemSettings,
userSettings,
workspaceSettings,
isTrusted,
);
// loadEnviroment depends on settings so we have to create a temp version of
@ -434,6 +447,7 @@ export function loadSettings(workspaceDir: string): LoadedSettings {
settings: workspaceSettings,
},
settingsErrors,
isTrusted,
);
// Validate chatCompression settings

1
packages/cli/src/gemini.test.tsx
View File

@ -149,6 +149,7 @@ describe('gemini.tsx main function', () => {
userSettingsFile,
workspaceSettingsFile,
[settingsError],
true,
);
loadSettingsMock.mockReturnValue(mockLoadedSettings);

2
packages/cli/src/ui/App.test.tsx
View File

@ -211,6 +211,7 @@ vi.mock('./hooks/useFolderTrust', () => ({
useFolderTrust: vi.fn(() => ({
isFolderTrustDialogOpen: false,
handleFolderTrustSelect: vi.fn(),
isRestarting: false,
})),
}));
@ -296,6 +297,7 @@ describe('App UI', () => {
userSettingsFile,
workspaceSettingsFile,
[],
true,
);
};

11
packages/cli/src/ui/App.tsx
View File

@ -267,10 +267,8 @@ const App = ({ config, settings, startupWarnings = [], version }: AppProps) => {
const { isSettingsDialogOpen, openSettingsDialog, closeSettingsDialog } =
useSettingsCommand();
const { isFolderTrustDialogOpen, handleFolderTrustSelect } = useFolderTrust(
settings,
setIsTrustedFolder,
);
const { isFolderTrustDialogOpen, handleFolderTrustSelect, isRestarting } =
useFolderTrust(settings, setIsTrustedFolder);
const {
isAuthDialogOpen,
@ -991,7 +989,10 @@ const App = ({ config, settings, startupWarnings = [], version }: AppProps) => {
onComplete={handleIdePromptComplete}
/>
) : isFolderTrustDialogOpen ? (
<FolderTrustDialog onSelect={handleFolderTrustSelect} />
<FolderTrustDialog
onSelect={handleFolderTrustSelect}
isRestarting={isRestarting}
/>
) : shellConfirmationRequest ? (
<ShellConfirmationDialog request={shellConfirmationRequest} />
) : confirmationRequest ? (

10
packages/cli/src/ui/components/AuthDialog.test.tsx
View File

@ -45,6 +45,7 @@ describe('AuthDialog', () => {
path: '',
},
[],
true,
);
const { lastFrame } = renderWithProviders(
@ -82,6 +83,7 @@ describe('AuthDialog', () => {
path: '',
},
[],
true,
);
const { lastFrame } = renderWithProviders(
@ -115,6 +117,7 @@ describe('AuthDialog', () => {
path: '',
},
[],
true,
);
const { lastFrame } = renderWithProviders(
@ -148,6 +151,7 @@ describe('AuthDialog', () => {
path: '',
},
[],
true,
);
const { lastFrame } = renderWithProviders(
@ -182,6 +186,7 @@ describe('AuthDialog', () => {
path: '',
},
[],
true,
);
const { lastFrame } = renderWithProviders(
@ -211,6 +216,7 @@ describe('AuthDialog', () => {
path: '',
},
[],
true,
);
const { lastFrame } = renderWithProviders(
@ -242,6 +248,7 @@ describe('AuthDialog', () => {
path: '',
},
[],
true,
);
const { lastFrame } = renderWithProviders(
@ -277,6 +284,7 @@ describe('AuthDialog', () => {
path: '',
},
[],
true,
);
const { lastFrame, stdin, unmount } = renderWithProviders(
@ -316,6 +324,7 @@ describe('AuthDialog', () => {
path: '',
},
[],
true,
);
const { lastFrame, stdin, unmount } = renderWithProviders(
@ -358,6 +367,7 @@ describe('AuthDialog', () => {
path: '',
},
[],
true,
);
const { stdin, unmount } = renderWithProviders(

66
packages/cli/src/ui/components/FolderTrustDialog.test.tsx
View File

@ -8,8 +8,21 @@ import { renderWithProviders } from '../../test-utils/render.js';
import { waitFor } from '@testing-library/react';
import { vi } from 'vitest';
import { FolderTrustDialog, FolderTrustChoice } from './FolderTrustDialog.js';
import * as process from 'process';
vi.mock('process', async () => {
const actual = await vi.importActual('process');
return {
...actual,
exit: vi.fn(),
};
});
describe('FolderTrustDialog', () => {
beforeEach(() => {
vi.clearAllMocks();
});
it('should render the dialog with title and description', () => {
const { lastFrame } = renderWithProviders(
<FolderTrustDialog onSelect={vi.fn()} />,
@ -21,16 +34,63 @@ describe('FolderTrustDialog', () => {
);
});
it('should call onSelect with DO_NOT_TRUST when escape is pressed', async () => {
it('should call onSelect with DO_NOT_TRUST when escape is pressed and not restarting', async () => {
const onSelect = vi.fn();
const { stdin } = renderWithProviders(
<FolderTrustDialog onSelect={onSelect} />,
<FolderTrustDialog onSelect={onSelect} isRestarting={false} />,
);
stdin.write('\x1b');
stdin.write('\x1b'); // escape key
await waitFor(() => {
expect(onSelect).toHaveBeenCalledWith(FolderTrustChoice.DO_NOT_TRUST);
});
});
it('should not call onSelect when escape is pressed and is restarting', async () => {
const onSelect = vi.fn();
const { stdin } = renderWithProviders(
<FolderTrustDialog onSelect={onSelect} isRestarting={true} />,
);
stdin.write('\x1b'); // escape key
await waitFor(() => {
expect(onSelect).not.toHaveBeenCalled();
});
});
it('should display restart message when isRestarting is true', () => {
const { lastFrame } = renderWithProviders(
<FolderTrustDialog onSelect={vi.fn()} isRestarting={true} />,
);
expect(lastFrame()).toContain(
'To see changes, Gemini CLI must be restarted',
);
});
it('should call process.exit when "r" is pressed and isRestarting is true', async () => {
const { stdin } = renderWithProviders(
<FolderTrustDialog onSelect={vi.fn()} isRestarting={true} />,
);
stdin.write('r');
await waitFor(() => {
expect(process.exit).toHaveBeenCalledWith(0);
});
});
it('should not call process.exit when "r" is pressed and isRestarting is false', async () => {
const { stdin } = renderWithProviders(
<FolderTrustDialog onSelect={vi.fn()} isRestarting={false} />,
);
stdin.write('r');
await waitFor(() => {
expect(process.exit).not.toHaveBeenCalled();
});
});
});

62
packages/cli/src/ui/components/FolderTrustDialog.tsx
View File

@ -12,6 +12,7 @@ import {
RadioSelectItem,
} from './shared/RadioButtonSelect.js';
import { useKeypress } from '../hooks/useKeypress.js';
import * as process from 'process';
export enum FolderTrustChoice {
TRUST_FOLDER = 'trust_folder',
@ -21,10 +22,12 @@ export enum FolderTrustChoice {
interface FolderTrustDialogProps {
onSelect: (choice: FolderTrustChoice) => void;
isRestarting?: boolean;
}
export const FolderTrustDialog: React.FC<FolderTrustDialogProps> = ({
onSelect,
isRestarting,
}) => {
useKeypress(
(key) => {
@ -32,7 +35,16 @@ export const FolderTrustDialog: React.FC<FolderTrustDialogProps> = ({
onSelect(FolderTrustChoice.DO_NOT_TRUST);
}
},
{ isActive: true },
{ isActive: !isRestarting },
);
useKeypress(
(key) => {
if (key.name === 'r') {
process.exit(0);
}
},
{ isActive: !!isRestarting },
);
const options: Array<RadioSelectItem<FolderTrustChoice>> = [
@ -51,24 +63,38 @@ export const FolderTrustDialog: React.FC<FolderTrustDialogProps> = ({
];
return (
<Box
flexDirection="column"
borderStyle="round"
borderColor={Colors.AccentYellow}
padding={1}
width="100%"
marginLeft={1}
>
<Box flexDirection="column" marginBottom={1}>
<Text bold>Do you trust this folder?</Text>
<Text>
Trusting a folder allows Gemini to execute commands it suggests. This
is a security feature to prevent accidental execution in untrusted
directories.
</Text>
</Box>
<Box flexDirection="column">
<Box
flexDirection="column"
borderStyle="round"
borderColor={Colors.AccentYellow}
padding={1}
width="100%"
marginLeft={1}
>
<Box flexDirection="column" marginBottom={1}>
<Text bold>Do you trust this folder?</Text>
<Text>
Trusting a folder allows Gemini to execute commands it suggests.
This is a security feature to prevent accidental execution in
untrusted directories.
</Text>
</Box>
<RadioButtonSelect items={options} onSelect={onSelect} isFocused />
<RadioButtonSelect
items={options}
onSelect={onSelect}
isFocused={!isRestarting}
/>
</Box>
{isRestarting && (
<Box marginLeft={1} marginTop={1}>
<Text color={Colors.AccentYellow}>
To see changes, Gemini CLI must be restarted. Press r to exit and
apply changes now.
</Text>
</Box>
)}
</Box>
);
};

1
packages/cli/src/ui/components/SettingsDialog.test.tsx
View File

@ -140,6 +140,7 @@ describe('SettingsDialog', () => {
path: '/workspace/settings.json',
},
[],
true,
);
describe('Initial Rendering', () => {

49
packages/cli/src/ui/hooks/useFolderTrust.test.ts
View File

@ -82,7 +82,9 @@ describe('useFolderTrust', () => {
});
it('should handle TRUST_FOLDER choice', () => {
isWorkspaceTrustedSpy.mockReturnValue(undefined);
isWorkspaceTrustedSpy
.mockReturnValueOnce(undefined)
.mockReturnValueOnce(true);
const { result } = renderHook(() =>
useFolderTrust(mockSettings, onTrustChange),
);
@ -102,12 +104,13 @@ describe('useFolderTrust', () => {
});
it('should handle TRUST_PARENT choice', () => {
isWorkspaceTrustedSpy.mockReturnValue(undefined);
isWorkspaceTrustedSpy
.mockReturnValueOnce(undefined)
.mockReturnValueOnce(true);
const { result } = renderHook(() =>
useFolderTrust(mockSettings, onTrustChange),
);
isWorkspaceTrustedSpy.mockReturnValue(true);
act(() => {
result.current.handleFolderTrustSelect(FolderTrustChoice.TRUST_PARENT);
});
@ -120,13 +123,14 @@ describe('useFolderTrust', () => {
expect(onTrustChange).toHaveBeenLastCalledWith(true);
});
it('should handle DO_NOT_TRUST choice', () => {
isWorkspaceTrustedSpy.mockReturnValue(undefined);
it('should handle DO_NOT_TRUST choice and trigger restart', () => {
isWorkspaceTrustedSpy
.mockReturnValueOnce(undefined)
.mockReturnValueOnce(false);
const { result } = renderHook(() =>
useFolderTrust(mockSettings, onTrustChange),
);
isWorkspaceTrustedSpy.mockReturnValue(false);
act(() => {
result.current.handleFolderTrustSelect(FolderTrustChoice.DO_NOT_TRUST);
});
@ -135,8 +139,9 @@ describe('useFolderTrust', () => {
'/test/path',
TrustLevel.DO_NOT_TRUST,
);
expect(result.current.isFolderTrustDialogOpen).toBe(false);
expect(onTrustChange).toHaveBeenLastCalledWith(false);
expect(result.current.isRestarting).toBe(true);
expect(result.current.isFolderTrustDialogOpen).toBe(true);
});
it('should do nothing for default choice', () => {
@ -156,4 +161,34 @@ describe('useFolderTrust', () => {
expect(result.current.isFolderTrustDialogOpen).toBe(true);
expect(onTrustChange).toHaveBeenCalledWith(undefined);
});
it('should set isRestarting to true when trust status changes from false to true', () => {
isWorkspaceTrustedSpy.mockReturnValueOnce(false).mockReturnValueOnce(true); // Initially untrusted, then trusted
const { result } = renderHook(() =>
useFolderTrust(mockSettings, onTrustChange),
);
act(() => {
result.current.handleFolderTrustSelect(FolderTrustChoice.TRUST_FOLDER);
});
expect(result.current.isRestarting).toBe(true);
expect(result.current.isFolderTrustDialogOpen).toBe(true); // Dialog should stay open
});
it('should not set isRestarting to true when trust status does not change', () => {
isWorkspaceTrustedSpy
.mockReturnValueOnce(undefined)
.mockReturnValueOnce(true); // Initially undefined, then trust
const { result } = renderHook(() =>
useFolderTrust(mockSettings, onTrustChange),
);
act(() => {
result.current.handleFolderTrustSelect(FolderTrustChoice.TRUST_FOLDER);
});
expect(result.current.isRestarting).toBe(false);
expect(result.current.isFolderTrustDialogOpen).toBe(false); // Dialog should close
});
});

26
packages/cli/src/ui/hooks/useFolderTrust.ts
View File

@ -20,6 +20,7 @@ export const useFolderTrust = (
) => {
const [isTrusted, setIsTrusted] = useState<boolean | undefined>(undefined);
const [isFolderTrustDialogOpen, setIsFolderTrustDialogOpen] = useState(false);
const [isRestarting, setIsRestarting] = useState(false);
const { folderTrust, folderTrustFeature } = settings.merged;
useEffect(() => {
@ -38,6 +39,8 @@ export const useFolderTrust = (
const cwd = process.cwd();
let trustLevel: TrustLevel;
const wasTrusted = isTrusted ?? true;
switch (choice) {
case FolderTrustChoice.TRUST_FOLDER:
trustLevel = TrustLevel.TRUST_FOLDER;
@ -53,20 +56,27 @@ export const useFolderTrust = (
}
trustedFolders.setValue(cwd, trustLevel);
const trusted = isWorkspaceTrusted({
folderTrust,
folderTrustFeature,
} as Settings);
setIsTrusted(trusted);
setIsFolderTrustDialogOpen(false);
onTrustChange(trusted);
const newIsTrusted =
trustLevel === TrustLevel.TRUST_FOLDER ||
trustLevel === TrustLevel.TRUST_PARENT;
setIsTrusted(newIsTrusted);
onTrustChange(newIsTrusted);
const needsRestart = wasTrusted !== newIsTrusted;
if (needsRestart) {
setIsRestarting(true);
setIsFolderTrustDialogOpen(true);
} else {
setIsFolderTrustDialogOpen(false);
}
},
[onTrustChange, folderTrust, folderTrustFeature],
[onTrustChange, isTrusted],
);
return {
isTrusted,
isFolderTrustDialogOpen,
handleFolderTrustSelect,
isRestarting,
};
};

2
packages/cli/src/ui/utils/MarkdownDisplay.test.tsx
View File

@ -22,6 +22,7 @@ describe('<MarkdownDisplay />', () => {
{ path: '', settings: {} },
{ path: '', settings: {} },
[],
true,
);
beforeEach(() => {
@ -220,6 +221,7 @@ Another paragraph.
{ path: '', settings: { showLineNumbers: false } },
{ path: '', settings: {} },
[],
true,
);
const { lastFrame } = render(