feat: Enforce unified folder trust for /directory add (#17359)

Co-authored-by: Jacob Richman <jacob314@gmail.com>
This commit is contained in:
Gal Zahavi
2026-01-23 21:31:42 -08:00
committed by GitHub
parent 3c2482a084
commit 0242a3dc56
4 changed files with 123 additions and 56 deletions

View File

@@ -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<MultiFolderTrustDialogProps>;
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<MultiFolderTrustDialogProps>;
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', () => {

View File

@@ -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: (
<MultiFolderTrustDialog
folders={undefinedTrustDirs}
folders={dirsToConfirm}
onComplete={context.ui.removeComponent}
trustedDirs={added}
errors={errors}

View File

@@ -5,7 +5,7 @@
*/
import { render } from '../../test-utils/render.js';
import { act } from 'react-dom/test-utils';
import { act } from 'react';
import {
MultiFolderTrustDialog,
MultiFolderTrustChoice,
@@ -22,6 +22,7 @@ import type { Config } from '@google/gemini-cli-core';
import { MessageType } from '../types.js';
import { useKeypress } from '../hooks/useKeypress.js';
import { RadioButtonSelect } from './shared/RadioButtonSelect.js';
import * as path from 'node:path';
// Mocks
vi.mock('../hooks/useKeypress.js');
@@ -64,7 +65,7 @@ describe('MultiFolderTrustDialog', () => {
vi.mocked(trustedFolders.loadTrustedFolders).mockReturnValue(
mockTrustedFolders,
);
vi.mocked(directoryUtils.expandHomeDir).mockImplementation((path) => path);
vi.mocked(directoryUtils.expandHomeDir).mockImplementation((p) => p);
mockedRadioButtonSelect.mockImplementation((props) => (
<div data-testid="RadioButtonSelect" {...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,

View File

@@ -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<MultiFolderTrustDialogProps> = ({
} 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);
}