diff --git a/packages/cli/src/config/settings.test.ts b/packages/cli/src/config/settings.test.ts index 65a556be..83580542 100644 --- a/packages/cli/src/config/settings.test.ts +++ b/packages/cli/src/config/settings.test.ts @@ -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 + }); + }); }); diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts index 3df98d95..3f94fe65 100644 --- a/packages/cli/src/config/settings.ts +++ b/packages/cli/src/config/settings.ts @@ -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 diff --git a/packages/cli/src/gemini.test.tsx b/packages/cli/src/gemini.test.tsx index c4de8308..a22f1ff9 100644 --- a/packages/cli/src/gemini.test.tsx +++ b/packages/cli/src/gemini.test.tsx @@ -149,6 +149,7 @@ describe('gemini.tsx main function', () => { userSettingsFile, workspaceSettingsFile, [settingsError], + true, ); loadSettingsMock.mockReturnValue(mockLoadedSettings); diff --git a/packages/cli/src/ui/App.test.tsx b/packages/cli/src/ui/App.test.tsx index 6a019ab6..f78ec580 100644 --- a/packages/cli/src/ui/App.test.tsx +++ b/packages/cli/src/ui/App.test.tsx @@ -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, ); }; diff --git a/packages/cli/src/ui/App.tsx b/packages/cli/src/ui/App.tsx index 01c6581c..6bf4f770 100644 --- a/packages/cli/src/ui/App.tsx +++ b/packages/cli/src/ui/App.tsx @@ -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 ? ( - + ) : shellConfirmationRequest ? ( ) : confirmationRequest ? ( diff --git a/packages/cli/src/ui/components/AuthDialog.test.tsx b/packages/cli/src/ui/components/AuthDialog.test.tsx index 38c6972a..5f08eae4 100644 --- a/packages/cli/src/ui/components/AuthDialog.test.tsx +++ b/packages/cli/src/ui/components/AuthDialog.test.tsx @@ -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( diff --git a/packages/cli/src/ui/components/FolderTrustDialog.test.tsx b/packages/cli/src/ui/components/FolderTrustDialog.test.tsx index e2b695e2..92aa0d58 100644 --- a/packages/cli/src/ui/components/FolderTrustDialog.test.tsx +++ b/packages/cli/src/ui/components/FolderTrustDialog.test.tsx @@ -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( , @@ -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( - , + , ); - 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( + , + ); + + stdin.write('\x1b'); // escape key + + await waitFor(() => { + expect(onSelect).not.toHaveBeenCalled(); + }); + }); + + it('should display restart message when isRestarting is true', () => { + const { lastFrame } = renderWithProviders( + , + ); + + 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( + , + ); + + 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( + , + ); + + stdin.write('r'); + + await waitFor(() => { + expect(process.exit).not.toHaveBeenCalled(); + }); + }); }); diff --git a/packages/cli/src/ui/components/FolderTrustDialog.tsx b/packages/cli/src/ui/components/FolderTrustDialog.tsx index 30f3ff52..88aea32c 100644 --- a/packages/cli/src/ui/components/FolderTrustDialog.tsx +++ b/packages/cli/src/ui/components/FolderTrustDialog.tsx @@ -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 = ({ onSelect, + isRestarting, }) => { useKeypress( (key) => { @@ -32,7 +35,16 @@ export const FolderTrustDialog: React.FC = ({ onSelect(FolderTrustChoice.DO_NOT_TRUST); } }, - { isActive: true }, + { isActive: !isRestarting }, + ); + + useKeypress( + (key) => { + if (key.name === 'r') { + process.exit(0); + } + }, + { isActive: !!isRestarting }, ); const options: Array> = [ @@ -51,24 +63,38 @@ export const FolderTrustDialog: React.FC = ({ ]; return ( - - - Do you trust this folder? - - Trusting a folder allows Gemini to execute commands it suggests. This - is a security feature to prevent accidental execution in untrusted - directories. - - + + + + Do you trust this folder? + + Trusting a folder allows Gemini to execute commands it suggests. + This is a security feature to prevent accidental execution in + untrusted directories. + + - + + + {isRestarting && ( + + + To see changes, Gemini CLI must be restarted. Press r to exit and + apply changes now. + + + )} ); }; diff --git a/packages/cli/src/ui/components/SettingsDialog.test.tsx b/packages/cli/src/ui/components/SettingsDialog.test.tsx index ee01b2cf..76a12e57 100644 --- a/packages/cli/src/ui/components/SettingsDialog.test.tsx +++ b/packages/cli/src/ui/components/SettingsDialog.test.tsx @@ -140,6 +140,7 @@ describe('SettingsDialog', () => { path: '/workspace/settings.json', }, [], + true, ); describe('Initial Rendering', () => { diff --git a/packages/cli/src/ui/hooks/useFolderTrust.test.ts b/packages/cli/src/ui/hooks/useFolderTrust.test.ts index 3103cab4..54e7fe86 100644 --- a/packages/cli/src/ui/hooks/useFolderTrust.test.ts +++ b/packages/cli/src/ui/hooks/useFolderTrust.test.ts @@ -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 + }); }); diff --git a/packages/cli/src/ui/hooks/useFolderTrust.ts b/packages/cli/src/ui/hooks/useFolderTrust.ts index 28b82b30..c13d9e6e 100644 --- a/packages/cli/src/ui/hooks/useFolderTrust.ts +++ b/packages/cli/src/ui/hooks/useFolderTrust.ts @@ -20,6 +20,7 @@ export const useFolderTrust = ( ) => { const [isTrusted, setIsTrusted] = useState(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, }; }; diff --git a/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx b/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx index dba6bb6d..a343b209 100644 --- a/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx +++ b/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx @@ -22,6 +22,7 @@ describe('', () => { { path: '', settings: {} }, { path: '', settings: {} }, [], + true, ); beforeEach(() => { @@ -220,6 +221,7 @@ Another paragraph. { path: '', settings: { showLineNumbers: false } }, { path: '', settings: {} }, [], + true, ); const { lastFrame } = render(