fix: default folder trust to untrusted for enhanced security (#15943)

This commit is contained in:
Gal Zahavi
2026-01-06 10:09:09 -08:00
committed by GitHub
parent 9a3ff6510f
commit 6f4b2ad0b9
6 changed files with 103 additions and 78 deletions

View File

@@ -413,7 +413,7 @@ export async function loadCliConfig(
const ideMode = settings.ide?.enabled ?? false;
const folderTrust = settings.security?.folderTrust?.enabled ?? false;
const trustedFolder = isWorkspaceTrusted(settings)?.isTrusted ?? true;
const trustedFolder = isWorkspaceTrusted(settings)?.isTrusted ?? false;
// Set the context filename in the server's memoryTool module BEFORE loading memory
// TODO(b/343434939): This is a bit of a hack. The contextFileName should ideally be passed

View File

@@ -7,7 +7,7 @@
import { renderWithProviders } from '../../test-utils/render.js';
import { waitFor } from '../../test-utils/async.js';
import { act } from 'react';
import { vi } from 'vitest';
import { vi, describe, it, expect, beforeEach } from 'vitest';
import { FolderTrustDialog } from './FolderTrustDialog.js';
import { ExitCodes } from '@google/gemini-cli-core';
import * as processUtils from '../../utils/processUtils.js';
@@ -74,7 +74,7 @@ describe('FolderTrustDialog', () => {
<FolderTrustDialog onSelect={vi.fn()} isRestarting={true} />,
);
expect(lastFrame()).toContain(' Gemini CLI is restarting');
expect(lastFrame()).toContain('Gemini CLI is restarting');
});
it('should call relaunchApp when isRestarting is true', async () => {
@@ -88,6 +88,21 @@ describe('FolderTrustDialog', () => {
vi.useRealTimers();
});
it('should not call relaunchApp if unmounted before timeout', async () => {
vi.useFakeTimers();
const relaunchApp = vi.spyOn(processUtils, 'relaunchApp');
const { unmount } = renderWithProviders(
<FolderTrustDialog onSelect={vi.fn()} isRestarting={true} />,
);
// Unmount immediately (before 250ms)
unmount();
await vi.advanceTimersByTimeAsync(250);
expect(relaunchApp).not.toHaveBeenCalled();
vi.useRealTimers();
});
it('should not call process.exit when "r" is pressed and isRestarting is false', async () => {
const { stdin } = renderWithProviders(
<FolderTrustDialog onSelect={vi.fn()} isRestarting={false} />,

View File

@@ -6,7 +6,7 @@
import { Box, Text } from 'ink';
import type React from 'react';
import { useEffect, useState } from 'react';
import { useEffect, useState, useCallback } from 'react';
import { theme } from '../semantic-colors.js';
import type { RadioSelectItem } from './shared/RadioButtonSelect.js';
import { RadioButtonSelect } from './shared/RadioButtonSelect.js';
@@ -33,26 +33,32 @@ export const FolderTrustDialog: React.FC<FolderTrustDialogProps> = ({
isRestarting,
}) => {
const [exiting, setExiting] = useState(false);
useEffect(() => {
const doRelaunch = async () => {
if (isRestarting) {
setTimeout(async () => {
await relaunchApp();
}, 250);
}
let timer: ReturnType<typeof setTimeout>;
if (isRestarting) {
timer = setTimeout(async () => {
await relaunchApp();
}, 250);
}
return () => {
if (timer) clearTimeout(timer);
};
// eslint-disable-next-line @typescript-eslint/no-floating-promises
doRelaunch();
}, [isRestarting]);
const handleExit = useCallback(() => {
setExiting(true);
// Give time for the UI to render the exiting message
setTimeout(async () => {
await runExitCleanup();
process.exit(ExitCodes.FATAL_CANCELLATION_ERROR);
}, 100);
}, []);
useKeypress(
(key) => {
if (key.name === 'escape') {
setExiting(true);
setTimeout(async () => {
await runExitCleanup();
process.exit(ExitCodes.FATAL_CANCELLATION_ERROR);
}, 100);
handleExit();
}
},
{ isActive: !isRestarting },

View File

@@ -4,7 +4,16 @@
* SPDX-License-Identifier: Apache-2.0
*/
import { vi, type Mock, type MockInstance } from 'vitest';
import {
vi,
describe,
it,
expect,
beforeEach,
afterEach,
type Mock,
type MockInstance,
} from 'vitest';
import { act } from 'react';
import { renderHook } from '../../test-utils/render.js';
import { waitFor } from '../../test-utils/async.js';
@@ -118,7 +127,7 @@ describe('useFolderTrust', () => {
expect(addItem).not.toHaveBeenCalled();
});
it('should handle TRUST_FOLDER choice', async () => {
it('should handle TRUST_FOLDER choice and trigger restart', async () => {
isWorkspaceTrustedSpy.mockReturnValue({
isTrusted: undefined,
source: undefined,
@@ -148,12 +157,13 @@ describe('useFolderTrust', () => {
'/test/path',
TrustLevel.TRUST_FOLDER,
);
expect(result.current.isFolderTrustDialogOpen).toBe(false);
expect(result.current.isRestarting).toBe(true);
expect(result.current.isFolderTrustDialogOpen).toBe(true);
expect(onTrustChange).toHaveBeenLastCalledWith(true);
});
});
it('should handle TRUST_PARENT choice', () => {
it('should handle TRUST_PARENT choice and trigger restart', async () => {
isWorkspaceTrustedSpy.mockReturnValue({
isTrusted: undefined,
source: undefined,
@@ -162,19 +172,22 @@ describe('useFolderTrust', () => {
useFolderTrust(mockSettings, onTrustChange, addItem),
);
act(() => {
await act(async () => {
result.current.handleFolderTrustSelect(FolderTrustChoice.TRUST_PARENT);
});
expect(mockTrustedFolders.setValue).toHaveBeenCalledWith(
'/test/path',
TrustLevel.TRUST_PARENT,
);
expect(result.current.isFolderTrustDialogOpen).toBe(false);
expect(onTrustChange).toHaveBeenLastCalledWith(true);
await waitFor(() => {
expect(mockTrustedFolders.setValue).toHaveBeenCalledWith(
'/test/path',
TrustLevel.TRUST_PARENT,
);
expect(result.current.isRestarting).toBe(true);
expect(result.current.isFolderTrustDialogOpen).toBe(true);
expect(onTrustChange).toHaveBeenLastCalledWith(true);
});
});
it('should handle DO_NOT_TRUST choice and trigger restart', () => {
it('should handle DO_NOT_TRUST choice and NOT trigger restart (implicit -> explicit)', async () => {
isWorkspaceTrustedSpy.mockReturnValue({
isTrusted: undefined,
source: undefined,
@@ -183,17 +196,19 @@ describe('useFolderTrust', () => {
useFolderTrust(mockSettings, onTrustChange, addItem),
);
act(() => {
await act(async () => {
result.current.handleFolderTrustSelect(FolderTrustChoice.DO_NOT_TRUST);
});
expect(mockTrustedFolders.setValue).toHaveBeenCalledWith(
'/test/path',
TrustLevel.DO_NOT_TRUST,
);
expect(onTrustChange).toHaveBeenLastCalledWith(false);
expect(result.current.isRestarting).toBe(true);
expect(result.current.isFolderTrustDialogOpen).toBe(true);
await waitFor(() => {
expect(mockTrustedFolders.setValue).toHaveBeenCalledWith(
'/test/path',
TrustLevel.DO_NOT_TRUST,
);
expect(onTrustChange).toHaveBeenLastCalledWith(false);
expect(result.current.isRestarting).toBe(false);
expect(result.current.isFolderTrustDialogOpen).toBe(false);
});
});
it('should do nothing for default choice', async () => {
@@ -205,7 +220,7 @@ describe('useFolderTrust', () => {
useFolderTrust(mockSettings, onTrustChange, addItem),
);
act(() => {
await act(async () => {
result.current.handleFolderTrustSelect(
'invalid_choice' as FolderTrustChoice,
);
@@ -237,7 +252,7 @@ describe('useFolderTrust', () => {
expect(result.current.isTrusted).toBe(false);
});
act(() => {
await act(async () => {
result.current.handleFolderTrustSelect(FolderTrustChoice.TRUST_FOLDER);
});
@@ -247,21 +262,23 @@ describe('useFolderTrust', () => {
});
});
it('should not set isRestarting to true when trust status does not change', () => {
it('should not set isRestarting to true when trust status does not change (true -> true)', async () => {
isWorkspaceTrustedSpy.mockReturnValue({
isTrusted: undefined,
source: undefined,
isTrusted: true,
source: 'file',
});
const { result } = renderHook(() =>
useFolderTrust(mockSettings, onTrustChange, addItem),
);
act(() => {
await act(async () => {
result.current.handleFolderTrustSelect(FolderTrustChoice.TRUST_FOLDER);
});
expect(result.current.isRestarting).toBe(false);
expect(result.current.isFolderTrustDialogOpen).toBe(false); // Dialog should close
await waitFor(() => {
expect(result.current.isRestarting).toBe(false);
expect(result.current.isFolderTrustDialogOpen).toBe(false); // Dialog should close
});
});
it('should emit feedback on failure to set value', async () => {

View File

@@ -49,25 +49,17 @@ export const useFolderTrust = (
const handleFolderTrustSelect = useCallback(
(choice: FolderTrustChoice) => {
const trustedFolders = loadTrustedFolders();
const trustLevelMap: Record<FolderTrustChoice, TrustLevel> = {
[FolderTrustChoice.TRUST_FOLDER]: TrustLevel.TRUST_FOLDER,
[FolderTrustChoice.TRUST_PARENT]: TrustLevel.TRUST_PARENT,
[FolderTrustChoice.DO_NOT_TRUST]: TrustLevel.DO_NOT_TRUST,
};
const trustLevel = trustLevelMap[choice];
if (!trustLevel) return;
const cwd = process.cwd();
let trustLevel: TrustLevel;
const wasTrusted = isTrusted ?? true;
switch (choice) {
case FolderTrustChoice.TRUST_FOLDER:
trustLevel = TrustLevel.TRUST_FOLDER;
break;
case FolderTrustChoice.TRUST_PARENT:
trustLevel = TrustLevel.TRUST_PARENT;
break;
case FolderTrustChoice.DO_NOT_TRUST:
trustLevel = TrustLevel.DO_NOT_TRUST;
break;
default:
return;
}
const trustedFolders = loadTrustedFolders();
try {
trustedFolders.setValue(cwd, trustLevel);
@@ -86,11 +78,15 @@ export const useFolderTrust = (
const currentIsTrusted =
trustLevel === TrustLevel.TRUST_FOLDER ||
trustLevel === TrustLevel.TRUST_PARENT;
setIsTrusted(currentIsTrusted);
onTrustChange(currentIsTrusted);
const needsRestart = wasTrusted !== currentIsTrusted;
if (needsRestart) {
onTrustChange(currentIsTrusted);
setIsTrusted(currentIsTrusted);
// logic: we restart if the trust state *effectively* changes from the previous state.
// previous state was `isTrusted`. If undefined, we assume false (untrusted).
const wasTrusted = isTrusted ?? false;
if (wasTrusted !== currentIsTrusted) {
setIsRestarting(true);
setIsFolderTrustDialogOpen(true);
} else {

View File

@@ -1428,22 +1428,13 @@ export class Config {
* 'false' for untrusted.
*/
isTrustedFolder(): boolean {
// isWorkspaceTrusted in cli/src/config/trustedFolder.js returns undefined
// when the file based trust value is unavailable, since it is mainly used
// in the initialization for trust dialogs, etc. Here we return true since
// config.isTrustedFolder() is used for the main business logic of blocking
// tool calls etc in the rest of the application.
//
// Default value is true since we load with trusted settings to avoid
// restarts in the more common path. If the user chooses to mark the folder
// as untrusted, the CLI will restart and we will have the trust value
// reloaded.
const context = ideContextStore.get();
if (context?.workspaceState?.isTrusted !== undefined) {
return context.workspaceState.isTrusted;
}
return this.trustedFolder ?? true;
// Default to untrusted if folder trust is enabled and no explicit value is set.
return this.folderTrust ? (this.trustedFolder ?? false) : true;
}
setIdeMode(value: boolean): void {