From 0242a3dc56e3e02f15b28b654c30748e89fef8f0 Mon Sep 17 00:00:00 2001 From: Gal Zahavi <38544478+galz10@users.noreply.github.com> Date: Fri, 23 Jan 2026 21:31:42 -0800 Subject: [PATCH] feat: Enforce unified folder trust for /directory add (#17359) Co-authored-by: Jacob Richman --- .../src/ui/commands/directoryCommand.test.tsx | 60 ++++++++++--- .../cli/src/ui/commands/directoryCommand.tsx | 89 +++++++++++-------- .../MultiFolderTrustDialog.test.tsx | 27 ++++-- .../ui/components/MultiFolderTrustDialog.tsx | 3 +- 4 files changed, 123 insertions(+), 56 deletions(-) diff --git a/packages/cli/src/ui/commands/directoryCommand.test.tsx b/packages/cli/src/ui/commands/directoryCommand.test.tsx index 904e8498f3..673e9805f9 100644 --- a/packages/cli/src/ui/commands/directoryCommand.test.tsx +++ b/packages/cli/src/ui/commands/directoryCommand.test.tsx @@ -278,6 +278,21 @@ describe('directoryCommand', () => { expect(getDirectorySuggestions).toHaveBeenCalledWith('s'); expect(results).toEqual(['docs/, src/']); }); + + it('should filter out existing directories from suggestions', async () => { + const existingPath = path.resolve(process.cwd(), 'existing'); + vi.mocked(mockWorkspaceContext.getDirectories).mockReturnValue([ + existingPath, + ]); + vi.mocked(getDirectorySuggestions).mockResolvedValue([ + 'existing/', + 'new/', + ]); + + const results = await completion(mockContext, 'ex'); + + expect(results).toEqual(['new/']); + }); }); }); @@ -286,10 +301,7 @@ describe('directoryCommand', () => { beforeEach(() => { vi.spyOn(trustedFolders, 'isFolderTrustEnabled').mockReturnValue(true); - vi.spyOn(trustedFolders, 'isWorkspaceTrusted').mockReturnValue({ - isTrusted: true, - source: 'file', - }); + // isWorkspaceTrusted is no longer checked, so we don't need to mock it returning true mockIsPathTrusted = vi.fn(); const mockLoadedFolders = { isPathTrusted: mockIsPathTrusted, @@ -319,20 +331,27 @@ describe('directoryCommand', () => { ]); }); - it('should show an error for an untrusted directory', async () => { + it('should return a custom dialog for an explicitly untrusted directory (upgrade flow)', async () => { if (!addCommand?.action) throw new Error('No action'); - mockIsPathTrusted.mockReturnValue(false); + mockIsPathTrusted.mockReturnValue(false); // DO_NOT_TRUST const newPath = path.normalize('/home/user/untrusted-project'); - await addCommand.action(mockContext, newPath); + const result = await addCommand.action(mockContext, newPath); - expect(mockWorkspaceContext.addDirectories).not.toHaveBeenCalled(); - expect(mockContext.ui.addItem).toHaveBeenCalledWith( + expect(result).toEqual( expect.objectContaining({ - type: MessageType.ERROR, - text: expect.stringContaining('explicitly untrusted'), + type: 'custom_dialog', + component: expect.objectContaining({ + type: expect.any(Function), // React component for MultiFolderTrustDialog + }), }), ); + if (!result) { + throw new Error('Command did not return a result'); + } + const component = (result as OpenCustomDialogActionReturn) + .component as React.ReactElement; + expect(component.props.folders.includes(newPath)).toBeTruthy(); }); it('should return a custom dialog for a directory with undefined trust', async () => { @@ -357,6 +376,25 @@ describe('directoryCommand', () => { .component as React.ReactElement; expect(component.props.folders.includes(newPath)).toBeTruthy(); }); + + it('should prompt for directory even if workspace is untrusted', async () => { + if (!addCommand?.action) throw new Error('No action'); + // Even if workspace is untrusted, we should still check directory trust + vi.spyOn(trustedFolders, 'isWorkspaceTrusted').mockReturnValue({ + isTrusted: false, + source: 'file', + }); + mockIsPathTrusted.mockReturnValue(undefined); + const newPath = path.normalize('/home/user/new-project'); + + const result = await addCommand.action(mockContext, newPath); + + expect(result).toEqual( + expect.objectContaining({ + type: 'custom_dialog', + }), + ); + }); }); it('should correctly expand a Windows-style home directory path', () => { diff --git a/packages/cli/src/ui/commands/directoryCommand.tsx b/packages/cli/src/ui/commands/directoryCommand.tsx index 9116e216b9..be0a35a344 100644 --- a/packages/cli/src/ui/commands/directoryCommand.tsx +++ b/packages/cli/src/ui/commands/directoryCommand.tsx @@ -6,7 +6,6 @@ import { isFolderTrustEnabled, - isWorkspaceTrusted, loadTrustedFolders, } from '../../config/trustedFolders.js'; import { MultiFolderTrustDialog } from '../components/MultiFolderTrustDialog.js'; @@ -20,6 +19,7 @@ import { batchAddDirectories, } from '../utils/directoryUtils.js'; import type { Config } from '@google/gemini-cli-core'; +import * as path from 'node:path'; async function finishAddingDirectories( config: Config, @@ -38,16 +38,18 @@ async function finishAddingDirectories( return; } - try { - if (config.shouldLoadMemoryFromIncludeDirectories()) { - await refreshServerHierarchicalMemory(config); + if (added.length > 0) { + try { + if (config.shouldLoadMemoryFromIncludeDirectories()) { + await refreshServerHierarchicalMemory(config); + } + addItem({ + type: MessageType.INFO, + text: `Successfully added GEMINI.md files from the following directories if there are:\n- ${added.join('\n- ')}`, + }); + } catch (error) { + errors.push(`Error refreshing memory: ${(error as Error).message}`); } - addItem({ - type: MessageType.INFO, - text: `Successfully added GEMINI.md files from the following directories if there are:\n- ${added.join('\n- ')}`, - }); - } catch (error) { - errors.push(`Error refreshing memory: ${(error as Error).message}`); } if (added.length > 0) { @@ -92,12 +94,38 @@ export const directoryCommand: SlashCommand = { const suggestions = await getDirectorySuggestions(trimmedLastPart); - if (parts.length > 1) { - const prefix = parts.slice(0, -1).join(',') + ','; - return suggestions.map((s) => prefix + leadingWhitespace + s); + // Filter out existing directories + let filteredSuggestions = suggestions; + if (context.services.config) { + const workspaceContext = + context.services.config.getWorkspaceContext(); + const existingDirs = new Set( + workspaceContext.getDirectories().map((dir) => path.normalize(dir)), + ); + + filteredSuggestions = suggestions.filter((s) => { + const expanded = expandHomeDir(s); + const absolute = path.resolve(expanded); + + if (existingDirs.has(absolute)) { + return false; + } + if ( + absolute.endsWith(path.sep) && + existingDirs.has(absolute.slice(0, -1)) + ) { + return false; + } + return true; + }); } - return suggestions.map((s) => leadingWhitespace + s); + if (parts.length > 1) { + const prefix = parts.slice(0, -1).join(',') + ','; + return filteredSuggestions.map((s) => prefix + leadingWhitespace + s); + } + + return filteredSuggestions.map((s) => leadingWhitespace + s); }, action: async (context: CommandContext, args: string) => { const { @@ -165,47 +193,36 @@ export const directoryCommand: SlashCommand = { return; } - if ( - isFolderTrustEnabled(settings.merged) && - isWorkspaceTrusted(settings.merged).isTrusted - ) { + if (isFolderTrustEnabled(settings.merged)) { const trustedFolders = loadTrustedFolders(); - const untrustedDirs: string[] = []; - const undefinedTrustDirs: string[] = []; + const dirsToConfirm: string[] = []; const trustedDirs: string[] = []; for (const pathToAdd of pathsToProcess) { - const expandedPath = expandHomeDir(pathToAdd.trim()); + const expandedPath = path.resolve(expandHomeDir(pathToAdd.trim())); const isTrusted = trustedFolders.isPathTrusted(expandedPath); - if (isTrusted === false) { - untrustedDirs.push(pathToAdd.trim()); - } else if (isTrusted === undefined) { - undefinedTrustDirs.push(pathToAdd.trim()); - } else { + // If explicitly trusted, add immediately. + // If undefined or explicitly untrusted (DO_NOT_TRUST), prompt for confirmation. + // This allows users to "upgrade" a DO_NOT_TRUST folder to trusted via the dialog. + if (isTrusted === true) { trustedDirs.push(pathToAdd.trim()); + } else { + dirsToConfirm.push(pathToAdd.trim()); } } - if (untrustedDirs.length > 0) { - errors.push( - `The following directories are explicitly untrusted and cannot be added to a trusted workspace:\n- ${untrustedDirs.join( - '\n- ', - )}\nPlease use the permissions command to modify their trust level.`, - ); - } - if (trustedDirs.length > 0) { const result = batchAddDirectories(workspaceContext, trustedDirs); added.push(...result.added); errors.push(...result.errors); } - if (undefinedTrustDirs.length > 0) { + if (dirsToConfirm.length > 0) { return { type: 'custom_dialog', component: ( { vi.mocked(trustedFolders.loadTrustedFolders).mockReturnValue( mockTrustedFolders, ); - vi.mocked(directoryUtils.expandHomeDir).mockImplementation((path) => path); + vi.mocked(directoryUtils.expandHomeDir).mockImplementation((p) => p); mockedRadioButtonSelect.mockImplementation((props) => (
)); @@ -148,8 +149,12 @@ describe('MultiFolderTrustDialog', () => { onSelect(MultiFolderTrustChoice.YES); }); - expect(mockAddDirectory).toHaveBeenCalledWith('/path/to/folder1'); - expect(mockAddDirectory).toHaveBeenCalledWith('/path/to/folder2'); + expect(mockAddDirectory).toHaveBeenCalledWith( + path.resolve('/path/to/folder1'), + ); + expect(mockAddDirectory).toHaveBeenCalledWith( + path.resolve('/path/to/folder2'), + ); expect(mockSetValue).not.toHaveBeenCalled(); expect(mockFinishAddingDirectories).toHaveBeenCalledWith( mockConfig, @@ -169,9 +174,11 @@ describe('MultiFolderTrustDialog', () => { onSelect(MultiFolderTrustChoice.YES_AND_REMEMBER); }); - expect(mockAddDirectory).toHaveBeenCalledWith('/path/to/folder1'); + expect(mockAddDirectory).toHaveBeenCalledWith( + path.resolve('/path/to/folder1'), + ); expect(mockSetValue).toHaveBeenCalledWith( - '/path/to/folder1', + path.resolve('/path/to/folder1'), TrustLevel.TRUST_FOLDER, ); expect(mockFinishAddingDirectories).toHaveBeenCalledWith( @@ -243,8 +250,12 @@ describe('MultiFolderTrustDialog', () => { onSelect(MultiFolderTrustChoice.YES); }); - expect(mockAddDirectory).toHaveBeenCalledWith('/path/to/good'); - expect(mockAddDirectory).not.toHaveBeenCalledWith('/path/to/error'); + expect(mockAddDirectory).toHaveBeenCalledWith( + path.resolve('/path/to/good'), + ); + expect(mockAddDirectory).not.toHaveBeenCalledWith( + path.resolve('/path/to/error'), + ); expect(mockFinishAddingDirectories).toHaveBeenCalledWith( mockConfig, mockAddItem, diff --git a/packages/cli/src/ui/components/MultiFolderTrustDialog.tsx b/packages/cli/src/ui/components/MultiFolderTrustDialog.tsx index 5928f766b7..c624d5fbfd 100644 --- a/packages/cli/src/ui/components/MultiFolderTrustDialog.tsx +++ b/packages/cli/src/ui/components/MultiFolderTrustDialog.tsx @@ -13,6 +13,7 @@ import { RadioButtonSelect } from './shared/RadioButtonSelect.js'; import { useKeypress } from '../hooks/useKeypress.js'; import { loadTrustedFolders, TrustLevel } from '../../config/trustedFolders.js'; import { expandHomeDir } from '../utils/directoryUtils.js'; +import * as path from 'node:path'; import { MessageType, type HistoryItem } from '../types.js'; import type { Config } from '@google/gemini-cli-core'; @@ -120,7 +121,7 @@ export const MultiFolderTrustDialog: React.FC = ({ } else { for (const dir of folders) { try { - const expandedPath = expandHomeDir(dir); + const expandedPath = path.resolve(expandHomeDir(dir)); if (choice === MultiFolderTrustChoice.YES_AND_REMEMBER) { trustedFolders.setValue(expandedPath, TrustLevel.TRUST_FOLDER); }