fix(extensions): preserve symlinks in extension source path while enforcing folder trust (#20867)

This commit is contained in:
Gal Zahavi
2026-03-04 16:06:19 -08:00
committed by GitHub
parent a5fd5d0b9f
commit 3481032980
5 changed files with 191 additions and 19 deletions
+11 -3
View File
@@ -28,6 +28,9 @@ import {
type Config, type Config,
type UserTierId, type UserTierId,
type ToolLiveOutput, type ToolLiveOutput,
type AnsiLine,
type AnsiOutput,
type AnsiToken,
isSubagentProgress, isSubagentProgress,
EDIT_TOOL_NAMES, EDIT_TOOL_NAMES,
processRestorableToolCalls, processRestorableToolCalls,
@@ -344,10 +347,15 @@ export class Task {
outputAsText = outputChunk; outputAsText = outputChunk;
} else if (isSubagentProgress(outputChunk)) { } else if (isSubagentProgress(outputChunk)) {
outputAsText = JSON.stringify(outputChunk); outputAsText = JSON.stringify(outputChunk);
} else { } else if (Array.isArray(outputChunk)) {
outputAsText = outputChunk const ansiOutput: AnsiOutput = outputChunk;
.map((line) => line.map((token) => token.text).join('')) outputAsText = ansiOutput
.map((line: AnsiLine) =>
line.map((token: AnsiToken) => token.text).join(''),
)
.join('\n'); .join('\n');
} else {
outputAsText = String(outputChunk);
} }
logger.info( logger.info(
@@ -5,6 +5,7 @@
*/ */
import type { CommandModule } from 'yargs'; import type { CommandModule } from 'yargs';
import * as path from 'node:path';
import chalk from 'chalk'; import chalk from 'chalk';
import { import {
debugLogger, debugLogger,
@@ -51,12 +52,13 @@ export async function handleInstall(args: InstallArgs) {
const settings = loadSettings(workspaceDir).merged; const settings = loadSettings(workspaceDir).merged;
if (installMetadata.type === 'local' || installMetadata.type === 'link') { if (installMetadata.type === 'local' || installMetadata.type === 'link') {
const resolvedPath = getRealPath(source); const absolutePath = path.resolve(source);
installMetadata.source = resolvedPath; const realPath = getRealPath(absolutePath);
const trustResult = isWorkspaceTrusted(settings, resolvedPath); installMetadata.source = absolutePath;
const trustResult = isWorkspaceTrusted(settings, absolutePath);
if (trustResult.isTrusted !== true) { if (trustResult.isTrusted !== true) {
const discoveryResults = const discoveryResults =
await FolderTrustDiscoveryService.discover(resolvedPath); await FolderTrustDiscoveryService.discover(realPath);
const hasDiscovery = const hasDiscovery =
discoveryResults.commands.length > 0 || discoveryResults.commands.length > 0 ||
@@ -69,7 +71,7 @@ export async function handleInstall(args: InstallArgs) {
'', '',
chalk.bold('Do you trust the files in this folder?'), chalk.bold('Do you trust the files in this folder?'),
'', '',
`The extension source at "${resolvedPath}" is not trusted.`, `The extension source at "${absolutePath}" is not trusted.`,
'', '',
'Trusting a folder allows Gemini CLI to load its local configurations,', 'Trusting a folder allows Gemini CLI to load its local configurations,',
'including custom commands, hooks, MCP servers, agent skills, and', 'including custom commands, hooks, MCP servers, agent skills, and',
@@ -127,10 +129,10 @@ export async function handleInstall(args: InstallArgs) {
); );
if (confirmed) { if (confirmed) {
const trustedFolders = loadTrustedFolders(); const trustedFolders = loadTrustedFolders();
await trustedFolders.setValue(resolvedPath, TrustLevel.TRUST_FOLDER); await trustedFolders.setValue(realPath, TrustLevel.TRUST_FOLDER);
} else { } else {
throw new Error( throw new Error(
`Installation aborted: Folder "${resolvedPath}" is not trusted.`, `Installation aborted: Folder "${absolutePath}" is not trusted.`,
); );
} }
} }
@@ -12,6 +12,13 @@ import { ExtensionManager } from './extension-manager.js';
import { createTestMergedSettings } from './settings.js'; import { createTestMergedSettings } from './settings.js';
import { createExtension } from '../test-utils/createExtension.js'; import { createExtension } from '../test-utils/createExtension.js';
import { EXTENSIONS_DIRECTORY_NAME } from './extensions/variables.js'; import { EXTENSIONS_DIRECTORY_NAME } from './extensions/variables.js';
import {
TrustLevel,
loadTrustedFolders,
isWorkspaceTrusted,
} from './trustedFolders.js';
import { getRealPath } from '@google/gemini-cli-core';
import type { MergedSettings } from './settings.js';
const mockHomedir = vi.hoisted(() => vi.fn(() => '/tmp/mock-home')); const mockHomedir = vi.hoisted(() => vi.fn(() => '/tmp/mock-home'));
@@ -185,4 +192,157 @@ describe('ExtensionManager', () => {
fs.rmSync(externalDir, { recursive: true, force: true }); fs.rmSync(externalDir, { recursive: true, force: true });
}); });
}); });
describe('symlink handling', () => {
let extensionDir: string;
let symlinkDir: string;
beforeEach(() => {
extensionDir = path.join(tempHomeDir, 'extension');
symlinkDir = path.join(tempHomeDir, 'symlink-ext');
fs.mkdirSync(extensionDir, { recursive: true });
fs.writeFileSync(
path.join(extensionDir, 'gemini-extension.json'),
JSON.stringify({ name: 'test-ext', version: '1.0.0' }),
);
fs.symlinkSync(extensionDir, symlinkDir, 'dir');
});
it('preserves symlinks in installMetadata.source when linking', async () => {
const manager = new ExtensionManager({
workspaceDir: tempWorkspaceDir,
settings: {
security: {
folderTrust: { enabled: false }, // Disable trust for simplicity in this test
},
experimental: { extensionConfig: false },
admin: { extensions: { enabled: true }, mcp: { enabled: true } },
hooksConfig: { enabled: true },
} as unknown as MergedSettings,
requestConsent: () => Promise.resolve(true),
requestSetting: null,
});
// Trust the workspace to allow installation
const trustedFolders = loadTrustedFolders();
await trustedFolders.setValue(tempWorkspaceDir, TrustLevel.TRUST_FOLDER);
const installMetadata = {
source: symlinkDir,
type: 'link' as const,
};
await manager.loadExtensions();
const extension = await manager.installOrUpdateExtension(installMetadata);
// Desired behavior: it preserves symlinks (if they were absolute or relative as provided)
expect(extension.installMetadata?.source).toBe(symlinkDir);
});
it('works with the new install command logic (preserves symlink but trusts real path)', async () => {
// This simulates the logic in packages/cli/src/commands/extensions/install.ts
const absolutePath = path.resolve(symlinkDir);
const realPath = getRealPath(absolutePath);
const settings = {
security: {
folderTrust: { enabled: true },
},
experimental: { extensionConfig: false },
admin: { extensions: { enabled: true }, mcp: { enabled: true } },
hooksConfig: { enabled: true },
} as unknown as MergedSettings;
// Trust the REAL path
const trustedFolders = loadTrustedFolders();
await trustedFolders.setValue(realPath, TrustLevel.TRUST_FOLDER);
// Check trust of the symlink path
const trustResult = isWorkspaceTrusted(settings, absolutePath);
expect(trustResult.isTrusted).toBe(true);
const manager = new ExtensionManager({
workspaceDir: tempWorkspaceDir,
settings,
requestConsent: () => Promise.resolve(true),
requestSetting: null,
});
const installMetadata = {
source: absolutePath,
type: 'link' as const,
};
await manager.loadExtensions();
const extension = await manager.installOrUpdateExtension(installMetadata);
expect(extension.installMetadata?.source).toBe(absolutePath);
expect(extension.installMetadata?.source).not.toBe(realPath);
});
it('enforces allowedExtensions using the real path', async () => {
const absolutePath = path.resolve(symlinkDir);
const realPath = getRealPath(absolutePath);
const settings = {
security: {
folderTrust: { enabled: false },
// Only allow the real path, not the symlink path
allowedExtensions: [realPath.replace(/\\/g, '\\\\')],
},
experimental: { extensionConfig: false },
admin: { extensions: { enabled: true }, mcp: { enabled: true } },
hooksConfig: { enabled: true },
} as unknown as MergedSettings;
const manager = new ExtensionManager({
workspaceDir: tempWorkspaceDir,
settings,
requestConsent: () => Promise.resolve(true),
requestSetting: null,
});
const installMetadata = {
source: absolutePath,
type: 'link' as const,
};
await manager.loadExtensions();
// This should pass because realPath is allowed
const extension = await manager.installOrUpdateExtension(installMetadata);
expect(extension.name).toBe('test-ext');
// Now try with a settings that only allows the symlink path string
const settingsOnlySymlink = {
security: {
folderTrust: { enabled: false },
// Only allow the symlink path string explicitly
allowedExtensions: [absolutePath.replace(/\\/g, '\\\\')],
},
experimental: { extensionConfig: false },
admin: { extensions: { enabled: true }, mcp: { enabled: true } },
hooksConfig: { enabled: true },
} as unknown as MergedSettings;
const manager2 = new ExtensionManager({
workspaceDir: tempWorkspaceDir,
settings: settingsOnlySymlink,
requestConsent: () => Promise.resolve(true),
requestSetting: null,
});
// This should FAIL because it checks the real path against the pattern
// (Unless symlinkDir === extensionDir, which shouldn't happen in this test setup)
if (absolutePath !== realPath) {
await expect(
manager2.installOrUpdateExtension(installMetadata),
).rejects.toThrow(
/is not allowed by the "allowedExtensions" security setting/,
);
}
});
});
}); });
+9 -7
View File
@@ -161,7 +161,9 @@ export class ExtensionManager extends ExtensionLoader {
const extensionAllowed = this.settings.security?.allowedExtensions.some( const extensionAllowed = this.settings.security?.allowedExtensions.some(
(pattern) => { (pattern) => {
try { try {
return new RegExp(pattern).test(installMetadata.source); return new RegExp(pattern).test(
getRealPath(installMetadata.source),
);
} catch (e) { } catch (e) {
throw new Error( throw new Error(
`Invalid regex pattern in allowedExtensions setting: "${pattern}. Error: ${getErrorMessage(e)}`, `Invalid regex pattern in allowedExtensions setting: "${pattern}. Error: ${getErrorMessage(e)}`,
@@ -210,11 +212,9 @@ export class ExtensionManager extends ExtensionLoader {
await fs.promises.mkdir(extensionsDir, { recursive: true }); await fs.promises.mkdir(extensionsDir, { recursive: true });
if (installMetadata.type === 'local' || installMetadata.type === 'link') { if (installMetadata.type === 'local' || installMetadata.type === 'link') {
installMetadata.source = getRealPath( installMetadata.source = path.isAbsolute(installMetadata.source)
path.isAbsolute(installMetadata.source)
? installMetadata.source ? installMetadata.source
: path.resolve(this.workspaceDir, installMetadata.source), : path.resolve(this.workspaceDir, installMetadata.source);
);
} }
let tempDir: string | undefined; let tempDir: string | undefined;
@@ -262,7 +262,7 @@ Would you like to attempt to install via "git clone" instead?`,
installMetadata.type === 'local' || installMetadata.type === 'local' ||
installMetadata.type === 'link' installMetadata.type === 'link'
) { ) {
localSourcePath = installMetadata.source; localSourcePath = getRealPath(installMetadata.source);
} else { } else {
throw new Error(`Unsupported install type: ${installMetadata.type}`); throw new Error(`Unsupported install type: ${installMetadata.type}`);
} }
@@ -638,7 +638,9 @@ Would you like to attempt to install via "git clone" instead?`,
const extensionAllowed = this.settings.security?.allowedExtensions.some( const extensionAllowed = this.settings.security?.allowedExtensions.some(
(pattern) => { (pattern) => {
try { try {
return new RegExp(pattern).test(installMetadata?.source); return new RegExp(pattern).test(
getRealPath(installMetadata?.source ?? ''),
);
} catch (e) { } catch (e) {
throw new Error( throw new Error(
`Invalid regex pattern in allowedExtensions setting: "${pattern}. Error: ${getErrorMessage(e)}`, `Invalid regex pattern in allowedExtensions setting: "${pattern}. Error: ${getErrorMessage(e)}`,
@@ -506,7 +506,7 @@ describe('Trusted Folders', () => {
const realDir = path.join(tempDir, 'real'); const realDir = path.join(tempDir, 'real');
const symlinkDir = path.join(tempDir, 'symlink'); const symlinkDir = path.join(tempDir, 'symlink');
fs.mkdirSync(realDir); fs.mkdirSync(realDir);
fs.symlinkSync(realDir, symlinkDir); fs.symlinkSync(realDir, symlinkDir, 'dir');
// Rule uses realpath // Rule uses realpath
const config = { [realDir]: TrustLevel.TRUST_FOLDER }; const config = { [realDir]: TrustLevel.TRUST_FOLDER };