feat: Support Extension Hooks with Security Warning (#14460)

This commit is contained in:
Abhi
2025-12-03 15:07:37 -05:00
committed by GitHub
parent 939cb67621
commit eb3312e7ba
4 changed files with 225 additions and 10 deletions

View File

@@ -41,6 +41,8 @@ import {
type MCPServerConfig,
type ExtensionInstallMetadata,
type GeminiCLIExtension,
type HookDefinition,
type HookEventName,
} from '@google/gemini-cli-core';
import { maybeRequestConsentOrFail } from './extensions/consent.js';
import { resolveEnvVarsInObject } from '../utils/envVarResolver.js';
@@ -253,10 +255,20 @@ export class ExtensionManager extends ExtensionLoader {
);
}
const newHasHooks = fs.existsSync(
path.join(localSourcePath, 'hooks', 'hooks.json'),
);
let previousHasHooks = false;
if (isUpdate && previous && previous.hooks) {
previousHasHooks = Object.keys(previous.hooks).length > 0;
}
await maybeRequestConsentOrFail(
newExtensionConfig,
this.requestConsent,
newHasHooks,
previousExtensionConfig,
previousHasHooks,
);
const extensionId = getExtensionId(newExtensionConfig, installMetadata);
const destinationPath = new ExtensionStorage(
@@ -501,6 +513,11 @@ export class ExtensionManager extends ExtensionLoader {
)
.filter((contextFilePath) => fs.existsSync(contextFilePath));
const hooks = await this.loadExtensionHooks(effectiveExtensionPath, {
extensionPath: effectiveExtensionPath,
workspacePath: this.workspaceDir,
});
const extension = {
name: config.name,
version: config.version,
@@ -509,6 +526,7 @@ export class ExtensionManager extends ExtensionLoader {
installMetadata,
mcpServers: config.mcpServers,
excludeTools: config.excludeTools,
hooks,
isActive: this.extensionEnablementManager.isEnabled(
config.name,
this.workspaceDir,
@@ -576,6 +594,53 @@ export class ExtensionManager extends ExtensionLoader {
}
}
private async loadExtensionHooks(
extensionDir: string,
context: { extensionPath: string; workspacePath: string },
): Promise<{ [K in HookEventName]?: HookDefinition[] } | undefined> {
const hooksFilePath = path.join(extensionDir, 'hooks', 'hooks.json');
try {
const hooksContent = await fs.promises.readFile(hooksFilePath, 'utf-8');
const rawHooks = JSON.parse(hooksContent);
if (
!rawHooks ||
typeof rawHooks !== 'object' ||
typeof rawHooks.hooks !== 'object' ||
rawHooks.hooks === null ||
Array.isArray(rawHooks.hooks)
) {
debugLogger.warn(
`Invalid hooks configuration in ${hooksFilePath}: "hooks" property must be an object`,
);
return undefined;
}
// Hydrate variables in the hooks configuration
const hydratedHooks = recursivelyHydrateStrings(
rawHooks.hooks as unknown as JsonObject,
{
...context,
'/': path.sep,
pathSeparator: path.sep,
},
) as { [K in HookEventName]?: HookDefinition[] };
return hydratedHooks;
} catch (e) {
if ((e as NodeJS.ErrnoException).code === 'ENOENT') {
return undefined; // File not found is not an error here.
}
debugLogger.warn(
`Failed to load extension hooks from ${hooksFilePath}: ${getErrorMessage(
e,
)}`,
);
return undefined;
}
}
toOutputString(extension: GeminiCLIExtension): string {
const userEnabled = this.extensionEnablementManager.isEnabled(
extension.name,

View File

@@ -713,11 +713,88 @@ describe('extension tests', () => {
name: 'no-meta-name',
version: '1.0.0',
});
const extensions = await extensionManager.loadExtensions();
const extension = extensions.find((e) => e.name === 'no-meta-name');
expect(extension?.id).toBe(hashValue('no-meta-name'));
});
it('should load extension hooks and hydrate variables', async () => {
const extDir = createExtension({
extensionsDir: userExtensionsDir,
name: 'hook-extension',
version: '1.0.0',
});
const hooksDir = path.join(extDir, 'hooks');
fs.mkdirSync(hooksDir);
const hooksConfig = {
hooks: {
BeforeTool: [
{
matcher: '.*',
hooks: [
{
type: 'command',
command: 'echo ${extensionPath}',
},
],
},
],
},
};
fs.writeFileSync(
path.join(hooksDir, 'hooks.json'),
JSON.stringify(hooksConfig),
);
const extensions = await extensionManager.loadExtensions();
expect(extensions).toHaveLength(1);
const extension = extensions[0];
expect(extension.hooks).toBeDefined();
expect(extension.hooks?.BeforeTool).toHaveLength(1);
expect(extension.hooks?.BeforeTool?.[0].hooks[0].command).toBe(
`echo ${extDir}`,
);
});
it('should warn about hooks during installation', async () => {
const requestConsentSpy = vi.fn().mockResolvedValue(true);
extensionManager.setRequestConsent(requestConsentSpy);
const sourceExtDir = path.join(
tempWorkspaceDir,
'hook-extension-source',
);
fs.mkdirSync(sourceExtDir, { recursive: true });
const hooksDir = path.join(sourceExtDir, 'hooks');
fs.mkdirSync(hooksDir);
fs.writeFileSync(
path.join(hooksDir, 'hooks.json'),
JSON.stringify({ hooks: {} }),
);
fs.writeFileSync(
path.join(sourceExtDir, 'gemini-extension.json'),
JSON.stringify({
name: 'hook-extension-install',
version: '1.0.0',
}),
);
await extensionManager.loadExtensions();
await extensionManager.installOrUpdateExtension({
source: sourceExtDir,
type: 'local',
});
expect(requestConsentSpy).toHaveBeenCalledWith(
expect.stringContaining('⚠️ This extension contains Hooks'),
);
});
});
});

View File

@@ -112,20 +112,31 @@ describe('consent', () => {
it('should request consent if there is no previous config', async () => {
const requestConsent = vi.fn().mockResolvedValue(true);
await maybeRequestConsentOrFail(baseConfig, requestConsent, undefined);
await maybeRequestConsentOrFail(
baseConfig,
requestConsent,
false,
undefined,
);
expect(requestConsent).toHaveBeenCalledTimes(1);
});
it('should not request consent if configs are identical', async () => {
const requestConsent = vi.fn().mockResolvedValue(true);
await maybeRequestConsentOrFail(baseConfig, requestConsent, baseConfig);
await maybeRequestConsentOrFail(
baseConfig,
requestConsent,
false,
baseConfig,
false,
);
expect(requestConsent).not.toHaveBeenCalled();
});
it('should throw an error if consent is denied', async () => {
const requestConsent = vi.fn().mockResolvedValue(false);
await expect(
maybeRequestConsentOrFail(baseConfig, requestConsent, undefined),
maybeRequestConsentOrFail(baseConfig, requestConsent, false, undefined),
).rejects.toThrow('Installation cancelled for "test-ext".');
});
@@ -141,7 +152,12 @@ describe('consent', () => {
excludeTools: ['tool1', 'tool2'],
};
const requestConsent = vi.fn().mockResolvedValue(true);
await maybeRequestConsentOrFail(config, requestConsent, undefined);
await maybeRequestConsentOrFail(
config,
requestConsent,
false,
undefined,
);
const expectedConsentString = [
'Installing extension "test-ext".',
@@ -163,7 +179,13 @@ describe('consent', () => {
mcpServers: { server1: { command: 'npm', args: ['start'] } },
};
const requestConsent = vi.fn().mockResolvedValue(true);
await maybeRequestConsentOrFail(newConfig, requestConsent, prevConfig);
await maybeRequestConsentOrFail(
newConfig,
requestConsent,
false,
prevConfig,
false,
);
expect(requestConsent).toHaveBeenCalledTimes(1);
});
@@ -174,7 +196,13 @@ describe('consent', () => {
contextFileName: 'new-context.md',
};
const requestConsent = vi.fn().mockResolvedValue(true);
await maybeRequestConsentOrFail(newConfig, requestConsent, prevConfig);
await maybeRequestConsentOrFail(
newConfig,
requestConsent,
false,
prevConfig,
false,
);
expect(requestConsent).toHaveBeenCalledTimes(1);
});
@@ -185,7 +213,41 @@ describe('consent', () => {
excludeTools: ['new-tool'],
};
const requestConsent = vi.fn().mockResolvedValue(true);
await maybeRequestConsentOrFail(newConfig, requestConsent, prevConfig);
await maybeRequestConsentOrFail(
newConfig,
requestConsent,
false,
prevConfig,
false,
);
expect(requestConsent).toHaveBeenCalledTimes(1);
});
it('should include warning when hooks are present', async () => {
const requestConsent = vi.fn().mockResolvedValue(true);
await maybeRequestConsentOrFail(
baseConfig,
requestConsent,
true,
undefined,
);
expect(requestConsent).toHaveBeenCalledWith(
expect.stringContaining(
'⚠️ This extension contains Hooks which can automatically execute commands.',
),
);
});
it('should request consent if hooks status changes', async () => {
const requestConsent = vi.fn().mockResolvedValue(true);
await maybeRequestConsentOrFail(
baseConfig,
requestConsent,
true,
baseConfig,
false,
);
expect(requestConsent).toHaveBeenCalledTimes(1);
});
});

View File

@@ -103,7 +103,10 @@ async function promptForConsentInteractive(
* Builds a consent string for installing an extension based on it's
* extensionConfig.
*/
function extensionConsentString(extensionConfig: ExtensionConfig): string {
function extensionConsentString(
extensionConfig: ExtensionConfig,
hasHooks: boolean,
): string {
const sanitizedConfig = escapeAnsiCtrlCodes(extensionConfig);
const output: string[] = [];
const mcpServerEntries = Object.entries(sanitizedConfig.mcpServers || {});
@@ -130,6 +133,11 @@ function extensionConsentString(extensionConfig: ExtensionConfig): string {
`This extension will exclude the following core tools: ${sanitizedConfig.excludeTools}`,
);
}
if (hasHooks) {
output.push(
'⚠️ This extension contains Hooks which can automatically execute commands.',
);
}
return output.join('\n');
}
@@ -145,12 +153,15 @@ function extensionConsentString(extensionConfig: ExtensionConfig): string {
export async function maybeRequestConsentOrFail(
extensionConfig: ExtensionConfig,
requestConsent: (consent: string) => Promise<boolean>,
hasHooks: boolean,
previousExtensionConfig?: ExtensionConfig,
previousHasHooks?: boolean,
) {
const extensionConsent = extensionConsentString(extensionConfig);
const extensionConsent = extensionConsentString(extensionConfig, hasHooks);
if (previousExtensionConfig) {
const previousExtensionConsent = extensionConsentString(
previousExtensionConfig,
previousHasHooks ?? false,
);
if (previousExtensionConsent === extensionConsent) {
return;