From 6510347d5b09db183828e37911cc647ca60b9072 Mon Sep 17 00:00:00 2001 From: Gal Zahavi <38544478+galz10@users.noreply.github.com> Date: Tue, 24 Feb 2026 11:58:44 -0800 Subject: [PATCH] feat(extensions): enforce folder trust for local extension install (#19703) --- integration-tests/symlink-install.test.ts | 136 +++++++++ .../src/commands/extensions/install.test.ts | 276 ++++++++++++++++-- .../cli/src/commands/extensions/install.ts | 105 ++++++- packages/cli/src/config/extension-manager.ts | 13 +- packages/cli/src/config/extension.test.ts | 120 ++++---- .../cli/src/config/extensions/consent.test.ts | 2 +- packages/cli/src/config/extensions/consent.ts | 13 +- packages/cli/src/config/settings.test.ts | 39 ++- packages/cli/src/config/settings.ts | 4 - 9 files changed, 592 insertions(+), 116 deletions(-) create mode 100644 integration-tests/symlink-install.test.ts diff --git a/integration-tests/symlink-install.test.ts b/integration-tests/symlink-install.test.ts new file mode 100644 index 0000000000..be4a5ac398 --- /dev/null +++ b/integration-tests/symlink-install.test.ts @@ -0,0 +1,136 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it, beforeEach, afterEach } from 'vitest'; +import { TestRig, InteractiveRun } from './test-helper.js'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import { + writeFileSync, + mkdirSync, + symlinkSync, + readFileSync, + unlinkSync, +} from 'node:fs'; +import { join, dirname } from 'node:path'; +import { GEMINI_DIR } from '@google/gemini-cli-core'; +import * as pty from '@lydell/node-pty'; +import { fileURLToPath } from 'node:url'; + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const BUNDLE_PATH = join(__dirname, '..', 'bundle/gemini.js'); + +const extension = `{ + "name": "test-symlink-extension", + "version": "0.0.1" +}`; + +const otherExtension = `{ + "name": "malicious-extension", + "version": "6.6.6" +}`; + +describe('extension symlink install spoofing protection', () => { + let rig: TestRig; + + beforeEach(() => { + rig = new TestRig(); + }); + + afterEach(async () => await rig.cleanup()); + + it('canonicalizes the trust path and prevents symlink spoofing', async () => { + // Enable folder trust for this test + rig.setup('symlink spoofing test', { + settings: { + security: { + folderTrust: { + enabled: true, + }, + }, + }, + }); + + const realExtPath = join(rig.testDir!, 'real-extension'); + mkdirSync(realExtPath); + writeFileSync(join(realExtPath, 'gemini-extension.json'), extension); + + const maliciousExtPath = join( + os.tmpdir(), + `malicious-extension-${Date.now()}`, + ); + mkdirSync(maliciousExtPath); + writeFileSync( + join(maliciousExtPath, 'gemini-extension.json'), + otherExtension, + ); + + const symlinkPath = join(rig.testDir!, 'symlink-extension'); + symlinkSync(realExtPath, symlinkPath); + + // Function to run a command with a PTY to avoid headless mode + const runPty = (args: string[]) => { + const ptyProcess = pty.spawn(process.execPath, [BUNDLE_PATH, ...args], { + name: 'xterm-color', + cols: 80, + rows: 80, + cwd: rig.testDir!, + env: { + ...process.env, + GEMINI_CLI_HOME: rig.homeDir!, + GEMINI_CLI_INTEGRATION_TEST: 'true', + GEMINI_PTY_INFO: 'node-pty', + }, + }); + return new InteractiveRun(ptyProcess); + }; + + // 1. Install via symlink, trust it + const run1 = runPty(['extensions', 'install', symlinkPath]); + await run1.expectText('Do you want to trust this folder', 30000); + await run1.type('y\r'); + await run1.expectText('trust this workspace', 30000); + await run1.type('y\r'); + await run1.expectText('Do you want to continue', 30000); + await run1.type('y\r'); + await run1.expectText('installed successfully', 30000); + await run1.kill(); + + // 2. Verify trustedFolders.json contains the REAL path, not the symlink path + const trustedFoldersPath = join( + rig.homeDir!, + GEMINI_DIR, + 'trustedFolders.json', + ); + // Wait for file to be written + let attempts = 0; + while (!fs.existsSync(trustedFoldersPath) && attempts < 50) { + await new Promise((resolve) => setTimeout(resolve, 100)); + attempts++; + } + + const trustedFolders = JSON.parse( + readFileSync(trustedFoldersPath, 'utf-8'), + ); + const trustedPaths = Object.keys(trustedFolders); + const canonicalRealExtPath = fs.realpathSync(realExtPath); + + expect(trustedPaths).toContain(canonicalRealExtPath); + expect(trustedPaths).not.toContain(symlinkPath); + + // 3. Swap the symlink to point to the malicious extension + unlinkSync(symlinkPath); + symlinkSync(maliciousExtPath, symlinkPath); + + // 4. Try to install again via the same symlink path. + // It should NOT be trusted because the real path changed. + const run2 = runPty(['extensions', 'install', symlinkPath]); + await run2.expectText('Do you want to trust this folder', 30000); + await run2.type('n\r'); + await run2.expectText('Installation aborted', 30000); + await run2.kill(); + }, 60000); +}); diff --git a/packages/cli/src/commands/extensions/install.test.ts b/packages/cli/src/commands/extensions/install.test.ts index d7cbaa1799..7fa84fa868 100644 --- a/packages/cli/src/commands/extensions/install.test.ts +++ b/packages/cli/src/commands/extensions/install.test.ts @@ -16,14 +16,20 @@ import { } from 'vitest'; import { handleInstall, installCommand } from './install.js'; import yargs from 'yargs'; -import { debugLogger, type GeminiCLIExtension } from '@google/gemini-cli-core'; +import * as core from '@google/gemini-cli-core'; +import type { inferInstallMetadata } from '../../config/extension-manager.js'; +import { ExtensionManager } from '../../config/extension-manager.js'; import type { - ExtensionManager, - inferInstallMetadata, -} from '../../config/extension-manager.js'; -import type { requestConsentNonInteractive } from '../../config/extensions/consent.js'; + promptForConsentNonInteractive, + requestConsentNonInteractive, +} from '../../config/extensions/consent.js'; +import type { + isWorkspaceTrusted, + loadTrustedFolders, +} from '../../config/trustedFolders.js'; import type * as fs from 'node:fs/promises'; import type { Stats } from 'node:fs'; +import * as path from 'node:path'; const mockInstallOrUpdateExtension: Mock< typeof ExtensionManager.prototype.installOrUpdateExtension @@ -31,28 +37,54 @@ const mockInstallOrUpdateExtension: Mock< const mockRequestConsentNonInteractive: Mock< typeof requestConsentNonInteractive > = vi.hoisted(() => vi.fn()); +const mockPromptForConsentNonInteractive: Mock< + typeof promptForConsentNonInteractive +> = vi.hoisted(() => vi.fn()); const mockStat: Mock = vi.hoisted(() => vi.fn()); const mockInferInstallMetadata: Mock = vi.hoisted( () => vi.fn(), ); +const mockIsWorkspaceTrusted: Mock = vi.hoisted(() => + vi.fn(), +); +const mockLoadTrustedFolders: Mock = vi.hoisted(() => + vi.fn(), +); +const mockDiscover: Mock = + vi.hoisted(() => vi.fn()); vi.mock('../../config/extensions/consent.js', () => ({ requestConsentNonInteractive: mockRequestConsentNonInteractive, + promptForConsentNonInteractive: mockPromptForConsentNonInteractive, + INSTALL_WARNING_MESSAGE: 'warning', })); -vi.mock('../../config/extension-manager.js', async (importOriginal) => { +vi.mock('../../config/trustedFolders.js', () => ({ + isWorkspaceTrusted: mockIsWorkspaceTrusted, + loadTrustedFolders: mockLoadTrustedFolders, + TrustLevel: { + TRUST_FOLDER: 'TRUST_FOLDER', + }, +})); + +vi.mock('@google/gemini-cli-core', async (importOriginal) => { const actual = - await importOriginal(); + await importOriginal(); return { ...actual, - ExtensionManager: vi.fn().mockImplementation(() => ({ - installOrUpdateExtension: mockInstallOrUpdateExtension, - loadExtensions: vi.fn(), - })), - inferInstallMetadata: mockInferInstallMetadata, + FolderTrustDiscoveryService: { + discover: mockDiscover, + }, }; }); +vi.mock('../../config/extension-manager.js', async (importOriginal) => ({ + ...(await importOriginal< + typeof import('../../config/extension-manager.js') + >()), + inferInstallMetadata: mockInferInstallMetadata, +})); + vi.mock('../../utils/errors.js', () => ({ getErrorMessage: vi.fn((error: Error) => error.message), })); @@ -83,12 +115,31 @@ describe('handleInstall', () => { let processSpy: MockInstance; beforeEach(() => { - debugLogSpy = vi.spyOn(debugLogger, 'log'); - debugErrorSpy = vi.spyOn(debugLogger, 'error'); + debugLogSpy = vi.spyOn(core.debugLogger, 'log'); + debugErrorSpy = vi.spyOn(core.debugLogger, 'error'); processSpy = vi .spyOn(process, 'exit') .mockImplementation(() => undefined as never); + vi.spyOn(ExtensionManager.prototype, 'loadExtensions').mockResolvedValue( + [], + ); + vi.spyOn( + ExtensionManager.prototype, + 'installOrUpdateExtension', + ).mockImplementation(mockInstallOrUpdateExtension); + + mockIsWorkspaceTrusted.mockReturnValue({ isTrusted: true, source: 'file' }); + mockDiscover.mockResolvedValue({ + commands: [], + mcps: [], + hooks: [], + skills: [], + settings: [], + securityWarnings: [], + discoveryErrors: [], + }); + mockInferInstallMetadata.mockImplementation(async (source, args) => { if ( source.startsWith('http://') || @@ -114,12 +165,29 @@ describe('handleInstall', () => { mockStat.mockClear(); mockInferInstallMetadata.mockClear(); vi.clearAllMocks(); + vi.restoreAllMocks(); }); + function createMockExtension( + overrides: Partial = {}, + ): core.GeminiCLIExtension { + return { + name: 'mock-extension', + version: '1.0.0', + isActive: true, + path: '/mock/path', + contextFiles: [], + id: 'mock-id', + ...overrides, + }; + } + it('should install an extension from a http source', async () => { - mockInstallOrUpdateExtension.mockResolvedValue({ - name: 'http-extension', - } as unknown as GeminiCLIExtension); + mockInstallOrUpdateExtension.mockResolvedValue( + createMockExtension({ + name: 'http-extension', + }), + ); await handleInstall({ source: 'http://google.com', @@ -131,9 +199,11 @@ describe('handleInstall', () => { }); it('should install an extension from a https source', async () => { - mockInstallOrUpdateExtension.mockResolvedValue({ - name: 'https-extension', - } as unknown as GeminiCLIExtension); + mockInstallOrUpdateExtension.mockResolvedValue( + createMockExtension({ + name: 'https-extension', + }), + ); await handleInstall({ source: 'https://google.com', @@ -145,9 +215,11 @@ describe('handleInstall', () => { }); it('should install an extension from a git source', async () => { - mockInstallOrUpdateExtension.mockResolvedValue({ - name: 'git-extension', - } as unknown as GeminiCLIExtension); + mockInstallOrUpdateExtension.mockResolvedValue( + createMockExtension({ + name: 'git-extension', + }), + ); await handleInstall({ source: 'git@some-url', @@ -171,9 +243,11 @@ describe('handleInstall', () => { }); it('should install an extension from a sso source', async () => { - mockInstallOrUpdateExtension.mockResolvedValue({ - name: 'sso-extension', - } as unknown as GeminiCLIExtension); + mockInstallOrUpdateExtension.mockResolvedValue( + createMockExtension({ + name: 'sso-extension', + }), + ); await handleInstall({ source: 'sso://google.com', @@ -185,12 +259,14 @@ describe('handleInstall', () => { }); it('should install an extension from a local path', async () => { - mockInstallOrUpdateExtension.mockResolvedValue({ - name: 'local-extension', - } as unknown as GeminiCLIExtension); + mockInstallOrUpdateExtension.mockResolvedValue( + createMockExtension({ + name: 'local-extension', + }), + ); mockStat.mockResolvedValue({} as Stats); await handleInstall({ - source: '/some/path', + source: path.join('/', 'some', 'path'), }); expect(debugLogSpy).toHaveBeenCalledWith( @@ -208,4 +284,144 @@ describe('handleInstall', () => { expect(debugErrorSpy).toHaveBeenCalledWith('Install extension failed'); expect(processSpy).toHaveBeenCalledWith(1); }); + + it('should proceed if local path is already trusted', async () => { + mockInstallOrUpdateExtension.mockResolvedValue( + createMockExtension({ + name: 'local-extension', + }), + ); + mockStat.mockResolvedValue({} as Stats); + mockIsWorkspaceTrusted.mockReturnValue({ isTrusted: true, source: 'file' }); + + await handleInstall({ + source: path.join('/', 'some', 'path'), + }); + + expect(mockIsWorkspaceTrusted).toHaveBeenCalled(); + expect(mockPromptForConsentNonInteractive).not.toHaveBeenCalled(); + expect(debugLogSpy).toHaveBeenCalledWith( + 'Extension "local-extension" installed successfully and enabled.', + ); + }); + + it('should prompt and proceed if user accepts trust', async () => { + mockInstallOrUpdateExtension.mockResolvedValue( + createMockExtension({ + name: 'local-extension', + }), + ); + mockStat.mockResolvedValue({} as Stats); + mockIsWorkspaceTrusted.mockReturnValue({ + isTrusted: undefined, + source: undefined, + }); + mockPromptForConsentNonInteractive.mockResolvedValue(true); + const mockSetValue = vi.fn(); + mockLoadTrustedFolders.mockReturnValue({ + setValue: mockSetValue, + user: { path: '', config: {} }, + errors: [], + rules: [], + isPathTrusted: vi.fn(), + }); + + await handleInstall({ + source: path.join('/', 'untrusted', 'path'), + }); + + expect(mockIsWorkspaceTrusted).toHaveBeenCalled(); + expect(mockPromptForConsentNonInteractive).toHaveBeenCalled(); + expect(mockSetValue).toHaveBeenCalledWith( + expect.stringContaining(path.join('untrusted', 'path')), + 'TRUST_FOLDER', + ); + expect(debugLogSpy).toHaveBeenCalledWith( + 'Extension "local-extension" installed successfully and enabled.', + ); + }); + + it('should prompt and abort if user denies trust', async () => { + mockStat.mockResolvedValue({} as Stats); + mockIsWorkspaceTrusted.mockReturnValue({ + isTrusted: undefined, + source: undefined, + }); + mockPromptForConsentNonInteractive.mockResolvedValue(false); + + await handleInstall({ + source: path.join('/', 'evil', 'path'), + }); + + expect(mockIsWorkspaceTrusted).toHaveBeenCalled(); + expect(mockPromptForConsentNonInteractive).toHaveBeenCalled(); + expect(debugErrorSpy).toHaveBeenCalledWith( + expect.stringContaining('Installation aborted: Folder'), + ); + expect(processSpy).toHaveBeenCalledWith(1); + }); + + it('should include discovery results in trust prompt', async () => { + mockInstallOrUpdateExtension.mockResolvedValue( + createMockExtension({ + name: 'local-extension', + }), + ); + mockStat.mockResolvedValue({} as Stats); + mockIsWorkspaceTrusted.mockReturnValue({ + isTrusted: undefined, + source: undefined, + }); + mockDiscover.mockResolvedValue({ + commands: ['custom-cmd'], + mcps: [], + hooks: [], + skills: ['cool-skill'], + settings: [], + securityWarnings: ['Security risk!'], + discoveryErrors: ['Read error'], + }); + mockPromptForConsentNonInteractive.mockResolvedValue(true); + mockLoadTrustedFolders.mockReturnValue({ + setValue: vi.fn(), + user: { path: '', config: {} }, + errors: [], + rules: [], + isPathTrusted: vi.fn(), + }); + + await handleInstall({ + source: '/untrusted/path', + }); + + expect(mockPromptForConsentNonInteractive).toHaveBeenCalledWith( + expect.stringContaining('This folder contains:'), + false, + ); + expect(mockPromptForConsentNonInteractive).toHaveBeenCalledWith( + expect.stringContaining('custom-cmd'), + false, + ); + expect(mockPromptForConsentNonInteractive).toHaveBeenCalledWith( + expect.stringContaining('cool-skill'), + false, + ); + expect(mockPromptForConsentNonInteractive).toHaveBeenCalledWith( + expect.stringContaining('Security Warnings:'), + false, + ); + expect(mockPromptForConsentNonInteractive).toHaveBeenCalledWith( + expect.stringContaining('Security risk!'), + false, + ); + expect(mockPromptForConsentNonInteractive).toHaveBeenCalledWith( + expect.stringContaining('Discovery Errors:'), + false, + ); + expect(mockPromptForConsentNonInteractive).toHaveBeenCalledWith( + expect.stringContaining('Read error'), + false, + ); + }); }); +// Implementation completed. diff --git a/packages/cli/src/commands/extensions/install.ts b/packages/cli/src/commands/extensions/install.ts index b094dc63f4..5255dfeb83 100644 --- a/packages/cli/src/commands/extensions/install.ts +++ b/packages/cli/src/commands/extensions/install.ts @@ -5,10 +5,16 @@ */ import type { CommandModule } from 'yargs'; -import { debugLogger } from '@google/gemini-cli-core'; +import chalk from 'chalk'; +import { + debugLogger, + FolderTrustDiscoveryService, + getRealPath, +} from '@google/gemini-cli-core'; import { getErrorMessage } from '../../utils/errors.js'; import { INSTALL_WARNING_MESSAGE, + promptForConsentNonInteractive, requestConsentNonInteractive, } from '../../config/extensions/consent.js'; import { @@ -16,6 +22,11 @@ import { inferInstallMetadata, } from '../../config/extension-manager.js'; import { loadSettings } from '../../config/settings.js'; +import { + isWorkspaceTrusted, + loadTrustedFolders, + TrustLevel, +} from '../../config/trustedFolders.js'; import { promptForSetting } from '../../config/extensions/extensionSettings.js'; import { exitCli } from '../utils.js'; @@ -36,6 +47,95 @@ export async function handleInstall(args: InstallArgs) { allowPreRelease: args.allowPreRelease, }); + const workspaceDir = process.cwd(); + const settings = loadSettings(workspaceDir).merged; + + if (installMetadata.type === 'local' || installMetadata.type === 'link') { + const resolvedPath = getRealPath(source); + installMetadata.source = resolvedPath; + const trustResult = isWorkspaceTrusted(settings, resolvedPath); + if (trustResult.isTrusted !== true) { + const discoveryResults = + await FolderTrustDiscoveryService.discover(resolvedPath); + + const hasDiscovery = + discoveryResults.commands.length > 0 || + discoveryResults.mcps.length > 0 || + discoveryResults.hooks.length > 0 || + discoveryResults.skills.length > 0 || + discoveryResults.settings.length > 0; + + const promptLines = [ + '', + chalk.bold('Do you trust the files in this folder?'), + '', + `The extension source at "${resolvedPath}" is not trusted.`, + '', + 'Trusting a folder allows Gemini CLI to load its local configurations,', + 'including custom commands, hooks, MCP servers, agent skills, and', + 'settings. These configurations could execute code on your behalf or', + 'change the behavior of the CLI.', + '', + ]; + + if (discoveryResults.discoveryErrors.length > 0) { + promptLines.push(chalk.red('❌ Discovery Errors:')); + for (const error of discoveryResults.discoveryErrors) { + promptLines.push(chalk.red(` • ${error}`)); + } + promptLines.push(''); + } + + if (discoveryResults.securityWarnings.length > 0) { + promptLines.push(chalk.yellow('⚠️ Security Warnings:')); + for (const warning of discoveryResults.securityWarnings) { + promptLines.push(chalk.yellow(` • ${warning}`)); + } + promptLines.push(''); + } + + if (hasDiscovery) { + promptLines.push(chalk.bold('This folder contains:')); + const groups = [ + { label: 'Commands', items: discoveryResults.commands }, + { label: 'MCP Servers', items: discoveryResults.mcps }, + { label: 'Hooks', items: discoveryResults.hooks }, + { label: 'Skills', items: discoveryResults.skills }, + { label: 'Setting overrides', items: discoveryResults.settings }, + ].filter((g) => g.items.length > 0); + + for (const group of groups) { + promptLines.push( + ` • ${chalk.bold(group.label)} (${group.items.length}):`, + ); + for (const item of group.items) { + promptLines.push(` - ${item}`); + } + } + promptLines.push(''); + } + + promptLines.push( + chalk.yellow( + 'Do you want to trust this folder and continue with the installation? [y/N]: ', + ), + ); + + const confirmed = await promptForConsentNonInteractive( + promptLines.join('\n'), + false, + ); + if (confirmed) { + const trustedFolders = loadTrustedFolders(); + await trustedFolders.setValue(resolvedPath, TrustLevel.TRUST_FOLDER); + } else { + throw new Error( + `Installation aborted: Folder "${resolvedPath}" is not trusted.`, + ); + } + } + } + const requestConsent = args.consent ? () => Promise.resolve(true) : requestConsentNonInteractive; @@ -44,12 +144,11 @@ export async function handleInstall(args: InstallArgs) { debugLogger.log(INSTALL_WARNING_MESSAGE); } - const workspaceDir = process.cwd(); const extensionManager = new ExtensionManager({ workspaceDir, requestConsent, requestSetting: promptForSetting, - settings: loadSettings(workspaceDir).merged, + settings, }); await extensionManager.loadExtensions(); const extension = diff --git a/packages/cli/src/config/extension-manager.ts b/packages/cli/src/config/extension-manager.ts index 179959d83e..805178ac70 100644 --- a/packages/cli/src/config/extension-manager.ts +++ b/packages/cli/src/config/extension-manager.ts @@ -32,6 +32,7 @@ import { ExtensionUninstallEvent, ExtensionUpdateEvent, getErrorMessage, + getRealPath, logExtensionDisable, logExtensionEnable, logExtensionInstallEvent, @@ -202,13 +203,11 @@ export class ExtensionManager extends ExtensionLoader { const extensionsDir = ExtensionStorage.getUserExtensionsDir(); await fs.promises.mkdir(extensionsDir, { recursive: true }); - if ( - !path.isAbsolute(installMetadata.source) && - (installMetadata.type === 'local' || installMetadata.type === 'link') - ) { - installMetadata.source = path.resolve( - this.workspaceDir, - installMetadata.source, + if (installMetadata.type === 'local' || installMetadata.type === 'link') { + installMetadata.source = getRealPath( + path.isAbsolute(installMetadata.source) + ? installMetadata.source + : path.resolve(this.workspaceDir, installMetadata.source), ); } diff --git a/packages/cli/src/config/extension.test.ts b/packages/cli/src/config/extension.test.ts index 0148fc7729..affcd0cef0 100644 --- a/packages/cli/src/config/extension.test.ts +++ b/packages/cli/src/config/extension.test.ts @@ -25,6 +25,7 @@ import { KeychainTokenStorage, loadAgentsFromDirectory, loadSkillsFromDir, + getRealPath, } from '@google/gemini-cli-core'; import { loadSettings, @@ -186,11 +187,11 @@ describe('extension tests', () => { errors: [], }); vi.mocked(loadSkillsFromDir).mockResolvedValue([]); - tempHomeDir = fs.mkdtempSync( - path.join(os.tmpdir(), 'gemini-cli-test-home-'), + tempHomeDir = getRealPath( + fs.mkdtempSync(path.join(os.tmpdir(), 'gemini-cli-test-home-')), ); - tempWorkspaceDir = fs.mkdtempSync( - path.join(tempHomeDir, 'gemini-cli-test-workspace-'), + tempWorkspaceDir = getRealPath( + fs.mkdtempSync(path.join(tempHomeDir, 'gemini-cli-test-workspace-')), ); userExtensionsDir = path.join(tempHomeDir, EXTENSIONS_DIRECTORY_NAME); mockRequestConsent = vi.fn(); @@ -329,12 +330,14 @@ describe('extension tests', () => { }); it('should load a linked extension correctly', async () => { - const sourceExtDir = createExtension({ - extensionsDir: tempWorkspaceDir, - name: 'my-linked-extension', - version: '1.0.0', - contextFileName: 'context.md', - }); + const sourceExtDir = getRealPath( + createExtension({ + extensionsDir: tempWorkspaceDir, + name: 'my-linked-extension', + version: '1.0.0', + contextFileName: 'context.md', + }), + ); fs.writeFileSync(path.join(sourceExtDir, 'context.md'), 'linked context'); await extensionManager.loadExtensions(); @@ -361,18 +364,20 @@ describe('extension tests', () => { }); it('should hydrate ${extensionPath} correctly for linked extensions', async () => { - const sourceExtDir = createExtension({ - extensionsDir: tempWorkspaceDir, - name: 'my-linked-extension-with-path', - version: '1.0.0', - mcpServers: { - 'test-server': { - command: 'node', - args: ['${extensionPath}${/}server${/}index.js'], - cwd: '${extensionPath}${/}server', + const sourceExtDir = getRealPath( + createExtension({ + extensionsDir: tempWorkspaceDir, + name: 'my-linked-extension-with-path', + version: '1.0.0', + mcpServers: { + 'test-server': { + command: 'node', + args: ['${extensionPath}${/}server${/}index.js'], + cwd: '${extensionPath}${/}server', + }, }, - }, - }); + }), + ); await extensionManager.loadExtensions(); await extensionManager.installOrUpdateExtension({ @@ -844,11 +849,13 @@ describe('extension tests', () => { it('should generate id from the original source for linked extensions', async () => { const extDevelopmentDir = path.join(tempHomeDir, 'local_extensions'); - const actualExtensionDir = createExtension({ - extensionsDir: extDevelopmentDir, - name: 'link-ext-name', - version: '1.0.0', - }); + const actualExtensionDir = getRealPath( + createExtension({ + extensionsDir: extDevelopmentDir, + name: 'link-ext-name', + version: '1.0.0', + }), + ); await extensionManager.loadExtensions(); await extensionManager.installOrUpdateExtension({ type: 'link', @@ -994,11 +1001,13 @@ describe('extension tests', () => { describe('installExtension', () => { it('should install an extension from a local path', async () => { - const sourceExtDir = createExtension({ - extensionsDir: tempHomeDir, - name: 'my-local-extension', - version: '1.0.0', - }); + const sourceExtDir = getRealPath( + createExtension({ + extensionsDir: tempHomeDir, + name: 'my-local-extension', + version: '1.0.0', + }), + ); const targetExtDir = path.join(userExtensionsDir, 'my-local-extension'); const metadataPath = path.join(targetExtDir, INSTALL_METADATA_FILENAME); @@ -1040,7 +1049,7 @@ describe('extension tests', () => { }); it('should throw an error and cleanup if gemini-extension.json is missing', async () => { - const sourceExtDir = path.join(tempHomeDir, 'bad-extension'); + const sourceExtDir = getRealPath(path.join(tempHomeDir, 'bad-extension')); fs.mkdirSync(sourceExtDir, { recursive: true }); const configPath = path.join(sourceExtDir, EXTENSIONS_CONFIG_FILENAME); @@ -1056,7 +1065,7 @@ describe('extension tests', () => { }); it('should throw an error for invalid JSON in gemini-extension.json', async () => { - const sourceExtDir = path.join(tempHomeDir, 'bad-json-ext'); + const sourceExtDir = getRealPath(path.join(tempHomeDir, 'bad-json-ext')); fs.mkdirSync(sourceExtDir, { recursive: true }); const configPath = path.join(sourceExtDir, EXTENSIONS_CONFIG_FILENAME); fs.writeFileSync(configPath, '{ "name": "bad-json", "version": "1.0.0"'); // Malformed JSON @@ -1066,22 +1075,17 @@ describe('extension tests', () => { source: sourceExtDir, type: 'local', }), - ).rejects.toThrow( - new RegExp( - `^Failed to load extension config from ${configPath.replace( - /\\/g, - '\\\\', - )}`, - ), - ); + ).rejects.toThrow(`Failed to load extension config from ${configPath}`); }); it('should throw an error for missing name in gemini-extension.json', async () => { - const sourceExtDir = createExtension({ - extensionsDir: tempHomeDir, - name: 'missing-name-ext', - version: '1.0.0', - }); + const sourceExtDir = getRealPath( + createExtension({ + extensionsDir: tempHomeDir, + name: 'missing-name-ext', + version: '1.0.0', + }), + ); const configPath = path.join(sourceExtDir, EXTENSIONS_CONFIG_FILENAME); // Overwrite with invalid config fs.writeFileSync(configPath, JSON.stringify({ version: '1.0.0' })); @@ -1134,11 +1138,13 @@ describe('extension tests', () => { }); it('should install a linked extension', async () => { - const sourceExtDir = createExtension({ - extensionsDir: tempHomeDir, - name: 'my-linked-extension', - version: '1.0.0', - }); + const sourceExtDir = getRealPath( + createExtension({ + extensionsDir: tempHomeDir, + name: 'my-linked-extension', + version: '1.0.0', + }), + ); const targetExtDir = path.join(userExtensionsDir, 'my-linked-extension'); const metadataPath = path.join(targetExtDir, INSTALL_METADATA_FILENAME); const configPath = path.join(targetExtDir, EXTENSIONS_CONFIG_FILENAME); @@ -1439,11 +1445,13 @@ ${INSTALL_WARNING_MESSAGE}`, }); it('should save the autoUpdate flag to the install metadata', async () => { - const sourceExtDir = createExtension({ - extensionsDir: tempHomeDir, - name: 'my-local-extension', - version: '1.0.0', - }); + const sourceExtDir = getRealPath( + createExtension({ + extensionsDir: tempHomeDir, + name: 'my-local-extension', + version: '1.0.0', + }), + ); const targetExtDir = path.join(userExtensionsDir, 'my-local-extension'); const metadataPath = path.join(targetExtDir, INSTALL_METADATA_FILENAME); diff --git a/packages/cli/src/config/extensions/consent.test.ts b/packages/cli/src/config/extensions/consent.test.ts index 4180a72b16..a7c07413b4 100644 --- a/packages/cli/src/config/extensions/consent.test.ts +++ b/packages/cli/src/config/extensions/consent.test.ts @@ -84,7 +84,7 @@ describe('consent', () => { { input: '', expected: true }, { input: 'n', expected: false }, { input: 'N', expected: false }, - { input: 'yes', expected: false }, + { input: 'yes', expected: true }, ])( 'should return $expected for input "$input"', async ({ input, expected }) => { diff --git a/packages/cli/src/config/extensions/consent.ts b/packages/cli/src/config/extensions/consent.ts index 9c3ea83bb6..9a63054d12 100644 --- a/packages/cli/src/config/extensions/consent.ts +++ b/packages/cli/src/config/extensions/consent.ts @@ -91,10 +91,12 @@ export async function requestConsentInteractive( * This should not be called from interactive mode as it will break the CLI. * * @param prompt A yes/no prompt to ask the user - * @returns Whether or not the user answers 'y' (yes). Defaults to 'yes' on enter. + * @param defaultValue Whether to resolve as true or false on enter. + * @returns Whether or not the user answers 'y' (yes). */ -async function promptForConsentNonInteractive( +export async function promptForConsentNonInteractive( prompt: string, + defaultValue = true, ): Promise { const readline = await import('node:readline'); const rl = readline.createInterface({ @@ -105,7 +107,12 @@ async function promptForConsentNonInteractive( return new Promise((resolve) => { rl.question(prompt, (answer) => { rl.close(); - resolve(['y', ''].includes(answer.trim().toLowerCase())); + const trimmedAnswer = answer.trim().toLowerCase(); + if (trimmedAnswer === '') { + resolve(defaultValue); + } else { + resolve(['y', 'yes'].includes(trimmedAnswer)); + } }); }); } diff --git a/packages/cli/src/config/settings.test.ts b/packages/cli/src/config/settings.test.ts index 6b2f18bb58..8fd0bd81b0 100644 --- a/packages/cli/src/config/settings.test.ts +++ b/packages/cli/src/config/settings.test.ts @@ -75,6 +75,7 @@ import { SettingScope, LoadedSettings, sanitizeEnvVar, + createTestMergedSettings, } from './settings.js'; import { FatalConfigError, @@ -1838,36 +1839,50 @@ describe('Settings Loading and Merging', () => { expect(process.env['GEMINI_API_KEY']).toEqual('test-key'); }); - it('does not load env files from untrusted spaces', () => { + it('does not load env files from untrusted spaces when sandboxed', () => { setup({ isFolderTrustEnabled: true, isWorkspaceTrustedValue: false }); const settings = { security: { folderTrust: { enabled: true } }, + tools: { sandbox: true }, } as Settings; loadEnvironment(settings, MOCK_WORKSPACE_DIR, isWorkspaceTrusted); expect(process.env['TESTTEST']).not.toEqual('1234'); }); - it('does not load env files when trust is undefined', () => { + it('does load env files from untrusted spaces when NOT sandboxed', () => { + setup({ isFolderTrustEnabled: true, isWorkspaceTrustedValue: false }); + const settings = { + security: { folderTrust: { enabled: true } }, + tools: { sandbox: false }, + } as Settings; + loadEnvironment(settings, MOCK_WORKSPACE_DIR, isWorkspaceTrusted); + + expect(process.env['TESTTEST']).toEqual('1234'); + }); + + it('does not load env files when trust is undefined and sandboxed', () => { delete process.env['TESTTEST']; // isWorkspaceTrusted returns {isTrusted: undefined} for matched rules with no trust value, or no matching rules. setup({ isFolderTrustEnabled: true, isWorkspaceTrustedValue: undefined }); const settings = { security: { folderTrust: { enabled: true } }, + tools: { sandbox: true }, } as Settings; const mockTrustFn = vi.fn().mockReturnValue({ isTrusted: undefined }); loadEnvironment(settings, MOCK_WORKSPACE_DIR, mockTrustFn); expect(process.env['TESTTEST']).not.toEqual('1234'); - expect(process.env['GEMINI_API_KEY']).not.toEqual('test-key'); + expect(process.env['GEMINI_API_KEY']).toEqual('test-key'); }); it('loads whitelisted env files from untrusted spaces if sandboxing is enabled', () => { setup({ isFolderTrustEnabled: true, isWorkspaceTrustedValue: false }); - const settings = loadSettings(MOCK_WORKSPACE_DIR); - settings.merged.tools.sandbox = true; - loadEnvironment(settings.merged, MOCK_WORKSPACE_DIR); + const settings = createTestMergedSettings({ + tools: { sandbox: true }, + }); + loadEnvironment(settings, MOCK_WORKSPACE_DIR, isWorkspaceTrusted); // GEMINI_API_KEY is in the whitelist, so it should be loaded. expect(process.env['GEMINI_API_KEY']).toEqual('test-key'); @@ -1880,10 +1895,10 @@ describe('Settings Loading and Merging', () => { process.argv.push('-s'); try { setup({ isFolderTrustEnabled: true, isWorkspaceTrustedValue: false }); - const settings = loadSettings(MOCK_WORKSPACE_DIR); - // Ensure sandbox is NOT in settings to test argv sniffing - settings.merged.tools.sandbox = undefined; - loadEnvironment(settings.merged, MOCK_WORKSPACE_DIR); + const settings = createTestMergedSettings({ + tools: { sandbox: false }, + }); + loadEnvironment(settings, MOCK_WORKSPACE_DIR, isWorkspaceTrusted); expect(process.env['GEMINI_API_KEY']).toEqual('test-key'); expect(process.env['TESTTEST']).not.toEqual('1234'); @@ -2782,7 +2797,7 @@ describe('Settings Loading and Merging', () => { MOCK_WORKSPACE_DIR, ); - expect(process.env['GEMINI_API_KEY']).toBeUndefined(); + expect(process.env['GEMINI_API_KEY']).toEqual('secret'); }); it('should NOT be tricked by positional arguments that look like flags', () => { @@ -2801,7 +2816,7 @@ describe('Settings Loading and Merging', () => { MOCK_WORKSPACE_DIR, ); - expect(process.env['GEMINI_API_KEY']).toBeUndefined(); + expect(process.env['GEMINI_API_KEY']).toEqual('secret'); }); }); diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts index c3f7c447eb..657968a3b6 100644 --- a/packages/cli/src/config/settings.ts +++ b/packages/cli/src/config/settings.ts @@ -573,10 +573,6 @@ export function loadEnvironment( relevantArgs.includes('-s') || relevantArgs.includes('--sandbox'); - if (trustResult.isTrusted !== true && !isSandboxed) { - return; - } - // Cloud Shell environment variable handling if (process.env['CLOUD_SHELL'] === 'true') { setUpCloudShellEnvironment(envFilePath, isTrusted, isSandboxed);