Re-request consent if necessary when updating extensions (#9517)

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
This commit is contained in:
Jacob MacDonald
2025-09-25 10:57:59 -07:00
committed by GitHub
parent e209724789
commit a0c8e3bf2b
14 changed files with 279 additions and 74 deletions

View File

@@ -9,9 +9,11 @@ import { handleInstall, installCommand } from './install.js';
import yargs from 'yargs';
const mockInstallExtension = vi.hoisted(() => vi.fn());
const mockRequestConsentNonInteractive = vi.hoisted(() => vi.fn());
vi.mock('../../config/extension.js', () => ({
installExtension: mockInstallExtension,
requestConsentNonInteractive: mockRequestConsentNonInteractive,
}));
vi.mock('../../utils/errors.js', () => ({
@@ -64,6 +66,7 @@ describe('handleInstall', () => {
afterEach(() => {
mockInstallExtension.mockClear();
mockRequestConsentNonInteractive.mockClear();
vi.resetAllMocks();
});

View File

@@ -5,7 +5,10 @@
*/
import type { CommandModule } from 'yargs';
import { installExtension } from '../../config/extension.js';
import {
installExtension,
requestConsentNonInteractive,
} from '../../config/extension.js';
import type { ExtensionInstallMetadata } from '@google/gemini-cli-core';
import { getErrorMessage } from '../../utils/errors.js';
@@ -48,7 +51,10 @@ export async function handleInstall(args: InstallArgs) {
throw new Error('Either --source or --path must be provided.');
}
const name = await installExtension(installMetadata, true);
const name = await installExtension(
installMetadata,
requestConsentNonInteractive,
);
console.log(`Extension "${name}" installed successfully and enabled.`);
} catch (error) {
console.error(getErrorMessage(error));

View File

@@ -5,7 +5,10 @@
*/
import type { CommandModule } from 'yargs';
import { installExtension } from '../../config/extension.js';
import {
installExtension,
requestConsentNonInteractive,
} from '../../config/extension.js';
import type { ExtensionInstallMetadata } from '@google/gemini-cli-core';
import { getErrorMessage } from '../../utils/errors.js';
@@ -20,7 +23,10 @@ export async function handleLink(args: InstallArgs) {
source: args.path,
type: 'link',
};
const extensionName = await installExtension(installMetadata);
const extensionName = await installExtension(
installMetadata,
requestConsentNonInteractive,
);
console.log(
`Extension "${extensionName}" linked successfully and enabled.`,
);

View File

@@ -8,6 +8,7 @@ import type { CommandModule } from 'yargs';
import {
loadExtensions,
annotateActiveExtensions,
requestConsentNonInteractive,
} from '../../config/extension.js';
import {
updateAllUpdatableExtensions,
@@ -62,6 +63,7 @@ export async function handleUpdate(args: UpdateArgs) {
const updatedExtensionInfo = (await updateExtension(
extension,
workingDir,
requestConsentNonInteractive,
updateState,
() => {},
))!;
@@ -83,6 +85,7 @@ export async function handleUpdate(args: UpdateArgs) {
try {
let updateInfos = await updateAllUpdatableExtensions(
workingDir,
requestConsentNonInteractive,
extensions,
await checkForAllExtensionUpdates(extensions, new Map(), (_) => {}),
() => {},

View File

@@ -16,8 +16,10 @@ import {
enableExtension,
installExtension,
loadExtension,
loadExtensionConfig,
loadExtensions,
performWorkspaceExtensionMigration,
requestConsentNonInteractive,
uninstallExtension,
type Extension,
} from './extension.js';
@@ -258,10 +260,13 @@ describe('extension tests', () => {
});
fs.writeFileSync(path.join(sourceExtDir, 'context.md'), 'linked context');
const extensionName = await installExtension({
source: sourceExtDir,
type: 'link',
});
const extensionName = await installExtension(
{
source: sourceExtDir,
type: 'link',
},
async (_) => true,
);
expect(extensionName).toEqual('my-linked-extension');
const extensions = loadExtensions();
@@ -627,7 +632,10 @@ describe('extension tests', () => {
const targetExtDir = path.join(userExtensionsDir, 'my-local-extension');
const metadataPath = path.join(targetExtDir, INSTALL_METADATA_FILENAME);
await installExtension({ source: sourceExtDir, type: 'local' });
await installExtension(
{ source: sourceExtDir, type: 'local' },
async (_) => true,
);
expect(fs.existsSync(targetExtDir)).toBe(true);
expect(fs.existsSync(metadataPath)).toBe(true);
@@ -645,9 +653,15 @@ describe('extension tests', () => {
name: 'my-local-extension',
version: '1.0.0',
});
await installExtension({ source: sourceExtDir, type: 'local' });
await installExtension(
{ source: sourceExtDir, type: 'local' },
async (_) => true,
);
await expect(
installExtension({ source: sourceExtDir, type: 'local' }),
installExtension(
{ source: sourceExtDir, type: 'local' },
async (_) => true,
),
).rejects.toThrow(
'Extension "my-local-extension" is already installed. Please uninstall it first.',
);
@@ -659,7 +673,10 @@ describe('extension tests', () => {
const configPath = path.join(sourceExtDir, EXTENSIONS_CONFIG_FILENAME);
await expect(
installExtension({ source: sourceExtDir, type: 'local' }),
installExtension(
{ source: sourceExtDir, type: 'local' },
async (_) => true,
),
).rejects.toThrow(`Configuration file not found at ${configPath}`);
const targetExtDir = path.join(userExtensionsDir, 'bad-extension');
@@ -673,7 +690,10 @@ describe('extension tests', () => {
fs.writeFileSync(configPath, '{ "name": "bad-json", "version": "1.0.0"'); // Malformed JSON
await expect(
installExtension({ source: sourceExtDir, type: 'local' }),
installExtension(
{ source: sourceExtDir, type: 'local' },
async (_) => true,
),
).rejects.toThrow(
new RegExp(
`^Failed to load extension config from ${configPath.replace(
@@ -695,7 +715,10 @@ describe('extension tests', () => {
fs.writeFileSync(configPath, JSON.stringify({ version: '1.0.0' }));
await expect(
installExtension({ source: sourceExtDir, type: 'local' }),
installExtension(
{ source: sourceExtDir, type: 'local' },
async (_) => true,
),
).rejects.toThrow(
`Invalid configuration in ${configPath}: missing "name"`,
);
@@ -718,7 +741,10 @@ describe('extension tests', () => {
});
mockGit.getRemotes.mockResolvedValue([{ name: 'origin' }]);
await installExtension({ source: gitUrl, type: 'git' });
await installExtension(
{ source: gitUrl, type: 'git' },
async (_) => true,
);
expect(fs.existsSync(targetExtDir)).toBe(true);
expect(fs.existsSync(metadataPath)).toBe(true);
@@ -740,7 +766,10 @@ describe('extension tests', () => {
const metadataPath = path.join(targetExtDir, INSTALL_METADATA_FILENAME);
const configPath = path.join(targetExtDir, EXTENSIONS_CONFIG_FILENAME);
await installExtension({ source: sourceExtDir, type: 'link' });
await installExtension(
{ source: sourceExtDir, type: 'link' },
async (_) => true,
);
expect(fs.existsSync(targetExtDir)).toBe(true);
expect(fs.existsSync(metadataPath)).toBe(true);
@@ -762,7 +791,10 @@ describe('extension tests', () => {
version: '1.0.0',
});
await installExtension({ source: sourceExtDir, type: 'local' });
await installExtension(
{ source: sourceExtDir, type: 'local' },
async (_) => true,
);
expect(mockLogExtensionInstallEvent).toHaveBeenCalled();
});
@@ -789,7 +821,10 @@ describe('extension tests', () => {
mockQuestion.mockImplementation((_query, callback) => callback('y'));
await expect(
installExtension({ source: sourceExtDir, type: 'local' }, true),
installExtension(
{ source: sourceExtDir, type: 'local' },
requestConsentNonInteractive,
),
).resolves.toBe('my-local-extension');
expect(consoleInfoSpy).toHaveBeenCalledWith(
@@ -817,7 +852,10 @@ This extension will run the following MCP servers:
mockQuestion.mockImplementation((_query, callback) => callback('y'));
await expect(
installExtension({ source: sourceExtDir, type: 'local' }, true),
installExtension(
{ source: sourceExtDir, type: 'local' },
requestConsentNonInteractive,
),
).resolves.toBe('my-local-extension');
expect(mockQuestion).toHaveBeenCalledWith(
@@ -842,8 +880,11 @@ This extension will run the following MCP servers:
mockQuestion.mockImplementation((_query, callback) => callback('n'));
await expect(
installExtension({ source: sourceExtDir, type: 'local' }, true),
).rejects.toThrow('Installation cancelled by user.');
installExtension(
{ source: sourceExtDir, type: 'local' },
requestConsentNonInteractive,
),
).rejects.toThrow('Installation cancelled.');
expect(mockQuestion).toHaveBeenCalledWith(
expect.stringContaining('Do you want to continue? [Y/n]: '),
@@ -860,11 +901,14 @@ This extension will run the following MCP servers:
const targetExtDir = path.join(userExtensionsDir, 'my-local-extension');
const metadataPath = path.join(targetExtDir, INSTALL_METADATA_FILENAME);
await installExtension({
source: sourceExtDir,
type: 'local',
autoUpdate: true,
});
await installExtension(
{
source: sourceExtDir,
type: 'local',
autoUpdate: true,
},
async (_) => true,
);
expect(fs.existsSync(targetExtDir)).toBe(true);
expect(fs.existsSync(metadataPath)).toBe(true);
@@ -890,9 +934,22 @@ This extension will run the following MCP servers:
},
});
const mockRequestConsent = vi.fn();
await expect(
installExtension({ source: sourceExtDir, type: 'local' }, false),
installExtension(
{ source: sourceExtDir, type: 'local' },
mockRequestConsent,
process.cwd(),
// Provide its own existing config as the previous config.
await loadExtensionConfig({
extensionDir: sourceExtDir,
workspaceDir: process.cwd(),
}),
),
).resolves.toBe('my-local-extension');
expect(mockRequestConsent).not.toHaveBeenCalled();
});
});
@@ -1028,12 +1085,15 @@ This extension will run the following MCP servers:
version: '1.0.0',
});
await performWorkspaceExtensionMigration([
loadExtension({
extensionDir: ext1Path,
workspaceDir: tempWorkspaceDir,
})!,
]);
await performWorkspaceExtensionMigration(
[
loadExtension({
extensionDir: ext1Path,
workspaceDir: tempWorkspaceDir,
})!,
],
async (_) => true,
);
const userExtensionsDir = path.join(
tempHomeDir,
@@ -1051,12 +1111,15 @@ This extension will run the following MCP servers:
version: '1.0.0',
});
await performWorkspaceExtensionMigration([
loadExtension({
extensionDir: ext1Path,
workspaceDir: tempWorkspaceDir,
})!,
]);
await performWorkspaceExtensionMigration(
[
loadExtension({
extensionDir: ext1Path,
workspaceDir: tempWorkspaceDir,
})!,
],
async (_) => true,
);
const extensions = loadExtensions();
expect(extensions).toEqual([]);
@@ -1084,8 +1147,10 @@ This extension will run the following MCP servers:
workspaceDir: tempWorkspaceDir,
})!,
];
const failed =
await performWorkspaceExtensionMigration(extensionsToMigrate);
const failed = await performWorkspaceExtensionMigration(
extensionsToMigrate,
async (_) => true,
);
expect(failed).toEqual([]);
@@ -1126,7 +1191,10 @@ This extension will run the following MCP servers:
},
];
const failed = await performWorkspaceExtensionMigration(extensions);
const failed = await performWorkspaceExtensionMigration(
extensions,
async (_) => true,
);
expect(failed).toEqual(['ext2']);
});
});

View File

@@ -37,6 +37,7 @@ import {
} from './extensions/github.js';
import type { LoadExtensionContext } from './extensions/variableSchema.js';
import { ExtensionEnablementManager } from './extensions/extensionEnablement.js';
import type { UseHistoryManagerReturn } from '../ui/hooks/useHistoryManager.js';
import chalk from 'chalk';
export const EXTENSIONS_DIRECTORY_NAME = path.join(GEMINI_DIR, 'extensions');
@@ -112,6 +113,7 @@ export async function copyExtension(
export async function performWorkspaceExtensionMigration(
extensions: Extension[],
requestConsent: (consent: string) => Promise<boolean>,
): Promise<string[]> {
const failedInstallNames: string[] = [];
@@ -121,7 +123,7 @@ export async function performWorkspaceExtensionMigration(
source: extension.path,
type: 'local',
};
await installExtension(installMetadata);
await installExtension(installMetadata, requestConsent);
} catch (_) {
failedInstallNames.push(extension.config.name);
}
@@ -356,11 +358,57 @@ export function annotateActiveExtensions(
}
/**
* Asks users a prompt and awaits for a y/n response
* Requests consent from the user to perform an action, by reading a Y/n
* character from stdin.
*
* This should not be called from interactive mode as it will break the CLI.
*
* @param consentDescription The description of the thing they will be consenting to.
* @returns boolean, whether they consented or not.
*/
export async function requestConsentNonInteractive(
consentDescription: string,
): Promise<boolean> {
console.info(consentDescription);
const result = await promptForContinuationNonInteractive(
'Do you want to continue? [Y/n]: ',
);
return result;
}
/**
* Requests consent from the user to perform an action, in interactive mode.
*
* This should not be called from non-interactive mode as it will not work.
*
* @param consentDescription The description of the thing they will be consenting to.
* @returns boolean, whether they consented or not.
*/
export async function requestConsentInteractive(
_consentDescription: string,
addHistoryItem: UseHistoryManagerReturn['addItem'],
): Promise<boolean> {
addHistoryItem(
{
type: 'info',
text: 'Tried to update an extension but it has some changes that require consent, please use `gemini extensions update`.',
},
Date.now(),
);
return false;
}
/**
* Asks users a prompt and awaits for a y/n response on stdin.
*
* 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.
*/
async function promptForContinuation(prompt: string): Promise<boolean> {
async function promptForContinuationNonInteractive(
prompt: string,
): Promise<boolean> {
const readline = await import('node:readline');
const rl = readline.createInterface({
input: process.stdin,
@@ -377,8 +425,9 @@ async function promptForContinuation(prompt: string): Promise<boolean> {
export async function installExtension(
installMetadata: ExtensionInstallMetadata,
askConsent: boolean = false,
requestConsent: (consent: string) => Promise<boolean>,
cwd: string = process.cwd(),
previousExtensionConfig?: ExtensionConfig,
): Promise<string> {
const telemetryConfig = getTelemetryConfig(cwd);
let newExtensionConfig: ExtensionConfig | null = null;
@@ -450,9 +499,11 @@ export async function installExtension(
`Extension "${newExtensionName}" is already installed. Please uninstall it first.`,
);
}
if (askConsent) {
await requestConsent(newExtensionConfig);
}
await maybeRequestConsentOrFail(
newExtensionConfig,
requestConsent,
previousExtensionConfig,
);
await fs.promises.mkdir(destinationPath, { recursive: true });
if (
@@ -513,7 +564,11 @@ export async function installExtension(
}
}
async function requestConsent(extensionConfig: ExtensionConfig) {
/**
* Builds a consent string for installing an extension based on it's
* extensionConfig.
*/
function extensionConsentString(extensionConfig: ExtensionConfig): string {
const output: string[] = [];
const mcpServerEntries = Object.entries(extensionConfig.mcpServers || {});
output.push('Extensions may introduce unexpected behavior.');
@@ -541,12 +596,34 @@ async function requestConsent(extensionConfig: ExtensionConfig) {
`This extension will exclude the following core tools: ${extensionConfig.excludeTools}`,
);
}
console.info(output.join('\n'));
const shouldContinue = await promptForContinuation(
'Do you want to continue? [Y/n]: ',
);
if (!shouldContinue) {
throw new Error('Installation cancelled by user.');
return output.join('\n');
}
/**
* Requests consent from the user to install an extension (extensionConfig), if
* there is any difference between the consent string for `extensionConfig` and
* `previousExtensionConfig`.
*
* Always requests consent if previousExtensionConfig is null.
*
* Throws if the user does not consent.
*/
async function maybeRequestConsentOrFail(
extensionConfig: ExtensionConfig,
requestConsent: (consent: string) => Promise<boolean>,
previousExtensionConfig?: ExtensionConfig,
) {
const extensionConsent = extensionConsentString(extensionConfig);
if (previousExtensionConfig) {
const previousExtensionConsent = extensionConsentString(
previousExtensionConfig,
);
if (previousExtensionConsent === extensionConsent) {
return;
}
}
if (!(await requestConsent(extensionConsent))) {
throw new Error('Installation cancelled.');
}
}

View File

@@ -140,6 +140,7 @@ describe('update tests', () => {
const updateInfo = await updateExtension(
extension,
tempHomeDir,
async (_) => true,
ExtensionUpdateState.UPDATE_AVAILABLE,
() => {},
);
@@ -197,6 +198,7 @@ describe('update tests', () => {
await updateExtension(
extension,
tempHomeDir,
async (_) => true,
ExtensionUpdateState.UPDATE_AVAILABLE,
setExtensionUpdateState,
);
@@ -239,6 +241,7 @@ describe('update tests', () => {
updateExtension(
extension,
tempHomeDir,
async (_) => true,
ExtensionUpdateState.UPDATE_AVAILABLE,
setExtensionUpdateState,
),

View File

@@ -16,6 +16,7 @@ import {
loadExtension,
loadInstallMetadata,
ExtensionStorage,
loadExtensionConfig,
} from '../extension.js';
import { checkForExtensionUpdate } from './github.js';
@@ -28,6 +29,7 @@ export interface ExtensionUpdateInfo {
export async function updateExtension(
extension: GeminiCLIExtension,
cwd: string = process.cwd(),
requestConsent: (consent: string) => Promise<boolean>,
currentState: ExtensionUpdateState,
setExtensionUpdateState: (updateState: ExtensionUpdateState) => void,
): Promise<ExtensionUpdateInfo | undefined> {
@@ -52,8 +54,17 @@ export async function updateExtension(
const tempDir = await ExtensionStorage.createTmpDir();
try {
await copyExtension(extension.path, tempDir);
const previousExtensionConfig = await loadExtensionConfig({
extensionDir: extension.path,
workspaceDir: cwd,
});
await uninstallExtension(extension.name, cwd);
await installExtension(installMetadata, false, cwd);
await installExtension(
installMetadata,
requestConsent,
cwd,
previousExtensionConfig,
);
const updatedExtensionStorage = new ExtensionStorage(extension.name);
const updatedExtension = loadExtension({
@@ -85,6 +96,7 @@ export async function updateExtension(
export async function updateAllUpdatableExtensions(
cwd: string = process.cwd(),
requestConsent: (consent: string) => Promise<boolean>,
extensions: GeminiCLIExtension[],
extensionsState: Map<string, ExtensionUpdateState>,
setExtensionsUpdateState: Dispatch<
@@ -103,6 +115,7 @@ export async function updateAllUpdatableExtensions(
updateExtension(
extension,
cwd,
requestConsent,
extensionsState.get(extension.name)!,
(updateState) => {
setExtensionsUpdateState((prev) => {

View File

@@ -153,11 +153,14 @@ describe('extensionsCommand', () => {
it('should update a single extension by name', async () => {
const extension: GeminiCLIExtension = {
name: 'ext-one',
type: 'git',
version: '1.0.0',
isActive: true,
path: '/test/dir/ext-one',
autoUpdate: false,
installMetadata: {
type: 'git',
autoUpdate: false,
source: 'https://github.com/some/extension.git',
},
};
mockUpdateExtension.mockResolvedValue({
name: extension.name,
@@ -173,6 +176,7 @@ describe('extensionsCommand', () => {
expect(mockUpdateExtension).toHaveBeenCalledWith(
extension,
'/test/dir',
expect.any(Function),
ExtensionUpdateState.UPDATE_AVAILABLE,
expect.any(Function),
);
@@ -194,19 +198,25 @@ describe('extensionsCommand', () => {
it('should update multiple extensions by name', async () => {
const extensionOne: GeminiCLIExtension = {
name: 'ext-one',
type: 'git',
version: '1.0.0',
isActive: true,
path: '/test/dir/ext-one',
autoUpdate: false,
installMetadata: {
type: 'git',
autoUpdate: false,
source: 'https://github.com/some/extension.git',
},
};
const extensionTwo: GeminiCLIExtension = {
name: 'ext-two',
type: 'git',
version: '1.0.0',
isActive: true,
path: '/test/dir/ext-two',
autoUpdate: false,
installMetadata: {
type: 'git',
autoUpdate: false,
source: 'https://github.com/some/extension.git',
},
};
mockGetExtensions.mockReturnValue([extensionOne, extensionTwo]);
mockContext.ui.extensionsUpdateState.set(

View File

@@ -4,6 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
import { requestConsentInteractive } from '../../config/extension.js';
import {
updateAllUpdatableExtensions,
type ExtensionUpdateInfo,
@@ -51,6 +52,9 @@ async function updateAction(context: CommandContext, args: string) {
if (all) {
updateInfos = await updateAllUpdatableExtensions(
context.services.config!.getWorkingDir(),
// We don't have the ability to prompt for consent yet in this flow.
(description) =>
requestConsentInteractive(description, context.ui.addItem),
context.services.config!.getExtensions(),
context.ui.extensionsUpdateState,
context.ui.setExtensionsUpdateState,
@@ -75,6 +79,8 @@ async function updateAction(context: CommandContext, args: string) {
const updateInfo = await updateExtension(
extension,
workingDir,
(description) =>
requestConsentInteractive(description, context.ui.addItem),
context.ui.extensionsUpdateState.get(extension.name) ??
ExtensionUpdateState.UNKNOWN,
(updateState) => {

View File

@@ -23,8 +23,11 @@ export function WorkspaceMigrationDialog(props: {
const [failedExtensions, setFailedExtensions] = useState<string[]>([]);
onOpen();
const onMigrate = async () => {
const failed =
await performWorkspaceExtensionMigration(workspaceExtensions);
const failed = await performWorkspaceExtensionMigration(
workspaceExtensions,
// We aren't updating extensions, just moving them around, don't need to ask for consent.
async (_) => true,
);
setFailedExtensions(failed);
setMigrationComplete(true);
};

View File

@@ -91,7 +91,7 @@ describe('<ExtensionsList />', () => {
},
{
state: ExtensionUpdateState.ERROR,
expectedText: '(error checking for updates)',
expectedText: '(error)',
},
{
state: ExtensionUpdateState.UP_TO_DATE,

View File

@@ -14,6 +14,7 @@ import {
checkForAllExtensionUpdates,
updateExtension,
} from '../../config/extensions/update.js';
import { requestConsentInteractive } from '../../config/extension.js';
export const useExtensionUpdates = (
extensions: GeminiCLIExtension[],
@@ -40,13 +41,19 @@ export const useExtensionUpdates = (
continue;
}
if (extension.installMetadata?.autoUpdate) {
updateExtension(extension, cwd, currentState, (newState) => {
setExtensionsUpdateState((prev) => {
const finalState = new Map(prev);
finalState.set(extension.name, newState);
return finalState;
});
})
updateExtension(
extension,
cwd,
(description) => requestConsentInteractive(description, addItem),
currentState,
(newState) => {
setExtensionsUpdateState((prev) => {
const finalState = new Map(prev);
finalState.set(extension.name, newState);
return finalState;
});
},
)
.then((result) => {
if (!result) return;
addItem(

View File

@@ -10,7 +10,7 @@ export enum ExtensionUpdateState {
UPDATING = 'updating',
UPDATE_AVAILABLE = 'update available',
UP_TO_DATE = 'up to date',
ERROR = 'error checking for updates',
ERROR = 'error',
NOT_UPDATABLE = 'not updatable',
UNKNOWN = 'unknown',
}