From b90fa4bb89acf37e393fbb4506305081906659a6 Mon Sep 17 00:00:00 2001 From: Christine Betts Date: Thu, 29 Jan 2026 15:31:08 -0500 Subject: [PATCH 1/4] Add support for /extensions config command --- .../src/ui/commands/extensionsCommand.test.ts | 141 +++++++++- .../cli/src/ui/commands/extensionsCommand.ts | 264 +++++++++++++++++- 2 files changed, 393 insertions(+), 12 deletions(-) diff --git a/packages/cli/src/ui/commands/extensionsCommand.test.ts b/packages/cli/src/ui/commands/extensionsCommand.test.ts index 9e46ab47aa9..78bbfb828d8 100644 --- a/packages/cli/src/ui/commands/extensionsCommand.test.ts +++ b/packages/cli/src/ui/commands/extensionsCommand.test.ts @@ -53,6 +53,20 @@ vi.mock('node:fs/promises', () => ({ stat: vi.fn(), })); +vi.mock('../../config/extensions/extensionSettings.js', () => ({ + ExtensionSettingScope: { + USER: 'user', + WORKSPACE: 'workspace', + }, + getScopedEnvContents: vi.fn().mockResolvedValue({}), + promptForSetting: vi.fn(), + updateSetting: vi.fn(), +})); + +vi.mock('prompts', () => ({ + default: vi.fn(), +})); + vi.mock('../../config/extensions/update.js', () => ({ updateExtension: vi.fn(), checkForAllExtensionUpdates: vi.fn(), @@ -107,27 +121,31 @@ const allExt: GeminiCLIExtension = { describe('extensionsCommand', () => { let mockContext: CommandContext; const mockDispatchExtensionState = vi.fn(); + let mockExtensionLoader: unknown; beforeEach(() => { vi.resetAllMocks(); + mockExtensionLoader = Object.create(ExtensionManager.prototype); + Object.assign(mockExtensionLoader as object, { + enableExtension: mockEnableExtension, + disableExtension: mockDisableExtension, + installOrUpdateExtension: mockInstallExtension, + uninstallExtension: mockUninstallExtension, + getExtensions: mockGetExtensions, + loadExtensionConfig: vi.fn().mockResolvedValue({ + name: 'test-ext', + settings: [{ name: 'setting1', envVar: 'SETTING1' }], + }), + }); + mockGetExtensions.mockReturnValue([inactiveExt, activeExt, allExt]); vi.mocked(open).mockClear(); mockContext = createMockCommandContext({ services: { config: { getExtensions: mockGetExtensions, - getExtensionLoader: vi.fn().mockImplementation(() => { - const actual = Object.create(ExtensionManager.prototype); - Object.assign(actual, { - enableExtension: mockEnableExtension, - disableExtension: mockDisableExtension, - installOrUpdateExtension: mockInstallExtension, - uninstallExtension: mockUninstallExtension, - getExtensions: mockGetExtensions, - }); - return actual; - }), + getExtensionLoader: vi.fn().mockReturnValue(mockExtensionLoader), getWorkingDir: () => '/test/dir', }, }, @@ -978,4 +996,105 @@ describe('extensionsCommand', () => { expect(suggestions).toEqual(['ext1']); }); }); + + describe('config', () => { + let configAction: SlashCommand['action']; + + beforeEach(async () => { + configAction = extensionsCommand(true).subCommands?.find( + (cmd) => cmd.name === 'config', + )?.action; + + expect(configAction).not.toBeNull(); + mockContext.invocation!.name = 'config'; + + const prompts = (await import('prompts')).default; + vi.mocked(prompts).mockResolvedValue({ overwrite: true }); + + const { getScopedEnvContents } = await import( + '../../config/extensions/extensionSettings.js' + ); + vi.mocked(getScopedEnvContents).mockResolvedValue({}); + }); + + it('should configure all extensions if no args provided', async () => { + await configAction!(mockContext, ''); + expect(mockContext.ui.addItem).toHaveBeenCalledWith( + expect.objectContaining({ + type: MessageType.INFO, + text: expect.stringContaining('Configuring settings for "ext-one"'), + }), + ); + expect(mockContext.ui.addItem).toHaveBeenCalledWith( + expect.objectContaining({ + type: MessageType.INFO, + text: expect.stringContaining('Configuring settings for "ext-two"'), + }), + ); + }); + + it('should configure specific extension', async () => { + await configAction!(mockContext, 'ext-one'); + expect(mockContext.ui.addItem).toHaveBeenCalledWith( + expect.objectContaining({ + type: MessageType.INFO, + text: 'Configuring settings for "ext-one"...', + }), + ); + }); + + it('should configure specific setting for an extension', async () => { + const { updateSetting } = await import( + '../../config/extensions/extensionSettings.js' + ); + await configAction!(mockContext, 'ext-one SETTING1'); + expect(updateSetting).toHaveBeenCalledWith( + expect.anything(), + 'ext-one-id', + 'SETTING1', + expect.anything(), + 'user', + expect.anything(), + ); + }); + + it('should respect scope argument', async () => { + const { updateSetting } = await import( + '../../config/extensions/extensionSettings.js' + ); + await configAction!(mockContext, 'ext-one SETTING1 --scope=workspace'); + expect(updateSetting).toHaveBeenCalledWith( + expect.anything(), + 'ext-one-id', + 'SETTING1', + expect.anything(), + 'workspace', + expect.anything(), + ); + }); + + it('should show error for invalid extension name', async () => { + await configAction!(mockContext, '../invalid'); + expect(mockContext.ui.addItem).toHaveBeenCalledWith({ + type: MessageType.ERROR, + text: 'Invalid extension name. Names cannot contain path separators or "..".', + }); + }); + + it('should inform if extension has no settings', async () => { + const extensionLoader = + mockContext.services.config!.getExtensionLoader() as ExtensionManager; + vi.mocked(extensionLoader.loadExtensionConfig).mockResolvedValueOnce({ + name: 'ext-one', + version: '1.0.0', + settings: [], + }); + + await configAction!(mockContext, 'ext-one'); + expect(mockContext.ui.addItem).toHaveBeenCalledWith({ + type: MessageType.INFO, + text: 'Extension "ext-one" has no settings to configure.', + }); + }); + }); }); diff --git a/packages/cli/src/ui/commands/extensionsCommand.ts b/packages/cli/src/ui/commands/extensionsCommand.ts index 1258e30002b..aecf8fd993c 100644 --- a/packages/cli/src/ui/commands/extensionsCommand.ts +++ b/packages/cli/src/ui/commands/extensionsCommand.ts @@ -9,7 +9,10 @@ import { listExtensions, type ExtensionInstallMetadata, } from '@google/gemini-cli-core'; -import type { ExtensionUpdateInfo } from '../../config/extension.js'; +import type { + ExtensionConfig, + ExtensionUpdateInfo, +} from '../../config/extension.js'; import { getErrorMessage } from '../../utils/errors.js'; import { emptyIcon, @@ -32,6 +35,13 @@ import { SettingScope } from '../../config/settings.js'; import { McpServerEnablementManager } from '../../config/mcp/mcpServerEnablement.js'; import { theme } from '../semantic-colors.js'; import { stat } from 'node:fs/promises'; +import { + ExtensionSettingScope, + getScopedEnvContents, + promptForSetting, + updateSetting, +} from '../../config/extensions/extensionSettings.js'; +import prompts from 'prompts'; function showMessageIfNoExtensions( context: CommandContext, @@ -583,6 +593,249 @@ async function uninstallAction(context: CommandContext, args: string) { } } +async function configAction(context: CommandContext, args: string) { + const parts = args.trim().split(/\s+/).filter(Boolean); + let scope = ExtensionSettingScope.USER; + const otherArgs: string[] = []; + + for (const part of parts) { + if (part.startsWith('--scope=')) { + const scopeVal = part.split('=')[1]; + if (scopeVal === 'workspace') scope = ExtensionSettingScope.WORKSPACE; + else if (scopeVal === 'user') scope = ExtensionSettingScope.USER; + } else if (part === '--scope') { + // Handle next arg as scope value if needed, but simple parsing for now + } else { + otherArgs.push(part); + } + } + + const name = otherArgs[0]; + const setting = otherArgs[1]; + + if (name) { + if (name.includes('/') || name.includes('\\') || name.includes('..')) { + context.ui.addItem({ + type: MessageType.ERROR, + text: 'Invalid extension name. Names cannot contain path separators or "..".', + }); + return; + } + } + + // Case 1: Configure specific setting for an extension + if (name && setting) { + await configureSpecificSetting(context, name, setting, scope); + } + // Case 2: Configure all settings for an extension + else if (name) { + await configureExtension(context, name, scope); + } + // Case 3: Configure all extensions + else { + await configureAllExtensions(context, scope); + } +} + +async function configureSpecificSetting( + context: CommandContext, + extensionName: string, + settingKey: string, + scope: ExtensionSettingScope, +) { + const extensionManager = context.services.config?.getExtensionLoader(); + if (!(extensionManager instanceof ExtensionManager)) return; + + const extension = extensionManager + .getExtensions() + .find((e) => e.name === extensionName); + if (!extension) { + context.ui.addItem({ + type: MessageType.ERROR, + text: `Extension "${extensionName}" not found.`, + }); + return; + } + + const extensionConfig = await extensionManager.loadExtensionConfig( + extension.path, + ); + if (!extensionConfig) { + context.ui.addItem({ + type: MessageType.ERROR, + text: `Could not find configuration for extension "${extensionName}".`, + }); + return; + } + + await updateSetting( + extensionConfig, + extension.id, + settingKey, + promptForSetting, + scope, + process.cwd(), + ); + context.ui.addItem({ + type: MessageType.INFO, + text: `Setting "${settingKey}" updated.`, + }); +} + +async function configureExtension( + context: CommandContext, + extensionName: string, + scope: ExtensionSettingScope, +) { + const extensionManager = context.services.config?.getExtensionLoader(); + if (!(extensionManager instanceof ExtensionManager)) return; + + const extension = extensionManager + .getExtensions() + .find((e) => e.name === extensionName); + if (!extension) { + context.ui.addItem({ + type: MessageType.ERROR, + text: `Extension "${extensionName}" not found.`, + }); + return; + } + + const extensionConfig = await extensionManager.loadExtensionConfig( + extension.path, + ); + if ( + !extensionConfig || + !extensionConfig.settings || + extensionConfig.settings.length === 0 + ) { + context.ui.addItem({ + type: MessageType.INFO, + text: `Extension "${extensionName}" has no settings to configure.`, + }); + return; + } + + context.ui.addItem({ + type: MessageType.INFO, + text: `Configuring settings for "${extensionName}"...`, + }); + await configureExtensionSettings( + context, + extensionConfig, + extension.id, + scope, + ); + context.ui.addItem({ + type: MessageType.INFO, + text: `Configuration for "${extensionName}" completed.`, + }); +} + +async function configureAllExtensions( + context: CommandContext, + scope: ExtensionSettingScope, +) { + const extensionManager = context.services.config?.getExtensionLoader(); + if (!(extensionManager instanceof ExtensionManager)) return; + + const extensions = extensionManager.getExtensions(); + + if (extensions.length === 0) { + context.ui.addItem({ + type: MessageType.INFO, + text: 'No extensions installed.', + }); + return; + } + + for (const extension of extensions) { + const extensionConfig = await extensionManager.loadExtensionConfig( + extension.path, + ); + if ( + extensionConfig && + extensionConfig.settings && + extensionConfig.settings.length > 0 + ) { + context.ui.addItem({ + type: MessageType.INFO, + text: `\nConfiguring settings for "${extension.name}"...`, + }); + await configureExtensionSettings( + context, + extensionConfig, + extension.id, + scope, + ); + } + } + context.ui.addItem({ + type: MessageType.INFO, + text: 'All extensions configured.', + }); +} + +async function configureExtensionSettings( + context: CommandContext, + extensionConfig: ExtensionConfig, + extensionId: string, + scope: ExtensionSettingScope, +) { + const currentScopedSettings = await getScopedEnvContents( + extensionConfig, + extensionId, + scope, + process.cwd(), + ); + + let workspaceSettings: Record = {}; + if (scope === ExtensionSettingScope.USER) { + workspaceSettings = await getScopedEnvContents( + extensionConfig, + extensionId, + ExtensionSettingScope.WORKSPACE, + process.cwd(), + ); + } + + if (!extensionConfig.settings) return; + + for (const setting of extensionConfig.settings) { + const currentValue = currentScopedSettings[setting.envVar]; + const workspaceValue = workspaceSettings[setting.envVar]; + + if (workspaceValue !== undefined) { + context.ui.addItem({ + type: MessageType.INFO, + text: `Note: Setting "${setting.name}" is already configured in the workspace scope.`, + }); + } + + if (currentValue !== undefined) { + const response = await prompts({ + type: 'confirm', + name: 'overwrite', + message: `Setting "${setting.name}" (${setting.envVar}) is already set. Overwrite?`, + initial: false, + }); + + if (!response.overwrite) { + continue; + } + } + + await updateSetting( + extensionConfig, + extensionId, + setting.envVar, + promptForSetting, + scope, + process.cwd(), + ); + } +} + /** * Exported for testing. */ @@ -701,6 +954,14 @@ const restartCommand: SlashCommand = { completion: completeExtensions, }; +const configCommand: SlashCommand = { + name: 'config', + description: 'Configure extension settings', + kind: CommandKind.BUILT_IN, + autoExecute: false, + action: configAction, +}; + export function extensionsCommand( enableExtensionReloading?: boolean, ): SlashCommand { @@ -711,6 +972,7 @@ export function extensionsCommand( installCommand, uninstallCommand, linkCommand, + configCommand, ] : []; return { From 52ef5eaf208f3465992581f1990695c5778ebc60 Mon Sep 17 00:00:00 2001 From: Christine Betts Date: Thu, 29 Jan 2026 16:54:59 -0500 Subject: [PATCH 2/4] Refactor and adding UI --- .../src/commands/extensions/configure.test.ts | 67 ++-- .../cli/src/commands/extensions/configure.ts | 170 +-------- packages/cli/src/commands/extensions/utils.ts | 227 +++++++++++- .../src/ui/commands/extensionsCommand.test.ts | 126 +++---- .../cli/src/ui/commands/extensionsCommand.ts | 243 ++----------- .../ui/components/ConfigExtensionDialog.tsx | 343 ++++++++++++++++++ 6 files changed, 700 insertions(+), 476 deletions(-) create mode 100644 packages/cli/src/ui/components/ConfigExtensionDialog.tsx diff --git a/packages/cli/src/commands/extensions/configure.test.ts b/packages/cli/src/commands/extensions/configure.test.ts index fc7a3a085b4..cf86d6cc712 100644 --- a/packages/cli/src/commands/extensions/configure.test.ts +++ b/packages/cli/src/commands/extensions/configure.test.ts @@ -17,32 +17,26 @@ import yargs from 'yargs'; import { debugLogger } from '@google/gemini-cli-core'; import { updateSetting, - promptForSetting, getScopedEnvContents, type ExtensionSetting, } from '../../config/extensions/extensionSettings.js'; import prompts from 'prompts'; import * as fs from 'node:fs'; -const { - mockExtensionManager, - mockGetExtensionAndManager, - mockGetExtensionManager, - mockLoadSettings, -} = vi.hoisted(() => { - const extensionManager = { - loadExtensionConfig: vi.fn(), - getExtensions: vi.fn(), - loadExtensions: vi.fn(), - getSettings: vi.fn(), - }; - return { - mockExtensionManager: extensionManager, - mockGetExtensionAndManager: vi.fn(), - mockGetExtensionManager: vi.fn(), - mockLoadSettings: vi.fn().mockReturnValue({ merged: {} }), - }; -}); +const { mockExtensionManager, mockGetExtensionManager, mockLoadSettings } = + vi.hoisted(() => { + const extensionManager = { + loadExtensionConfig: vi.fn(), + getExtensions: vi.fn(), + loadExtensions: vi.fn(), + getSettings: vi.fn(), + }; + return { + mockExtensionManager: extensionManager, + mockGetExtensionManager: vi.fn(), + mockLoadSettings: vi.fn().mockReturnValue({ merged: {} }), + }; + }); vi.mock('../../config/extension-manager.js', () => ({ ExtensionManager: vi.fn().mockImplementation(() => mockExtensionManager), @@ -62,10 +56,13 @@ vi.mock('../utils.js', () => ({ exitCli: vi.fn(), })); -vi.mock('./utils.js', () => ({ - getExtensionAndManager: mockGetExtensionAndManager, - getExtensionManager: mockGetExtensionManager, -})); +vi.mock('./utils.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + getExtensionManager: mockGetExtensionManager, + }; +}); vi.mock('prompts'); @@ -91,10 +88,6 @@ describe('extensions configure command', () => { vi.spyOn(process, 'cwd').mockReturnValue(tempWorkspaceDir); // Default behaviors mockLoadSettings.mockReturnValue({ merged: {} }); - mockGetExtensionAndManager.mockResolvedValue({ - extension: null, - extensionManager: null, - }); mockGetExtensionManager.mockResolvedValue(mockExtensionManager); (ExtensionManager as unknown as Mock).mockImplementation( () => mockExtensionManager, @@ -117,11 +110,6 @@ describe('extensions configure command', () => { path = '/test/path', ) => { const extension = { name, path, id }; - mockGetExtensionAndManager.mockImplementation(async (n) => { - if (n === name) - return { extension, extensionManager: mockExtensionManager }; - return { extension: null, extensionManager: null }; - }); mockExtensionManager.getExtensions.mockReturnValue([extension]); mockExtensionManager.loadExtensionConfig.mockResolvedValue({ @@ -144,17 +132,14 @@ describe('extensions configure command', () => { expect.objectContaining({ name: 'test-ext' }), 'test-id', 'TEST_VAR', - promptForSetting, + expect.any(Function), 'user', tempWorkspaceDir, ); }); it('should handle missing extension', async () => { - mockGetExtensionAndManager.mockResolvedValue({ - extension: null, - extensionManager: null, - }); + mockExtensionManager.getExtensions.mockReturnValue([]); await runCommand('config missing-ext TEST_VAR'); @@ -190,7 +175,7 @@ describe('extensions configure command', () => { expect.objectContaining({ name: 'test-ext' }), 'test-id', 'VAR_1', - promptForSetting, + expect.any(Function), 'user', tempWorkspaceDir, ); @@ -205,7 +190,7 @@ describe('extensions configure command', () => { return {}; }, ); - (prompts as unknown as Mock).mockResolvedValue({ overwrite: true }); + (prompts as unknown as Mock).mockResolvedValue({ confirm: true }); (updateSetting as Mock).mockResolvedValue(undefined); await runCommand('config test-ext'); @@ -241,7 +226,7 @@ describe('extensions configure command', () => { const settings = [{ name: 'Setting 1', envVar: 'VAR_1' }]; setupExtension('test-ext', settings); (getScopedEnvContents as Mock).mockResolvedValue({ VAR_1: 'existing' }); - (prompts as unknown as Mock).mockResolvedValue({ overwrite: false }); + (prompts as unknown as Mock).mockResolvedValue({ confirm: false }); await runCommand('config test-ext'); diff --git a/packages/cli/src/commands/extensions/configure.ts b/packages/cli/src/commands/extensions/configure.ts index 0ee02fe635a..ef1222c97dd 100644 --- a/packages/cli/src/commands/extensions/configure.ts +++ b/packages/cli/src/commands/extensions/configure.ts @@ -5,18 +5,17 @@ */ import type { CommandModule } from 'yargs'; +import type { ExtensionSettingScope } from '../../config/extensions/extensionSettings.js'; import { - updateSetting, - promptForSetting, - ExtensionSettingScope, - getScopedEnvContents, -} from '../../config/extensions/extensionSettings.js'; -import { getExtensionAndManager, getExtensionManager } from './utils.js'; + configureAllExtensions, + configureExtension, + configureSpecificSetting, + getExtensionManager, +} from './utils.js'; import { loadSettings } from '../../config/settings.js'; -import { debugLogger, coreEvents } from '@google/gemini-cli-core'; +import { coreEvents, debugLogger } from '@google/gemini-cli-core'; import { exitCli } from '../utils.js'; -import prompts from 'prompts'; -import type { ExtensionConfig } from '../../config/extension.js'; + interface ConfigureArgs { name?: string; setting?: string; @@ -64,9 +63,12 @@ export const configureCommand: CommandModule = { } } + const extensionManager = await getExtensionManager(); + // Case 1: Configure specific setting for an extension if (name && setting) { await configureSpecificSetting( + extensionManager, name, setting, scope as ExtensionSettingScope, @@ -74,152 +76,20 @@ export const configureCommand: CommandModule = { } // Case 2: Configure all settings for an extension else if (name) { - await configureExtension(name, scope as ExtensionSettingScope); + await configureExtension( + extensionManager, + name, + scope as ExtensionSettingScope, + ); } // Case 3: Configure all extensions else { - await configureAllExtensions(scope as ExtensionSettingScope); + await configureAllExtensions( + extensionManager, + scope as ExtensionSettingScope, + ); } await exitCli(); }, }; - -async function configureSpecificSetting( - extensionName: string, - settingKey: string, - scope: ExtensionSettingScope, -) { - const { extension, extensionManager } = - await getExtensionAndManager(extensionName); - if (!extension || !extensionManager) { - return; - } - const extensionConfig = await extensionManager.loadExtensionConfig( - extension.path, - ); - if (!extensionConfig) { - debugLogger.error( - `Could not find configuration for extension "${extensionName}".`, - ); - return; - } - - await updateSetting( - extensionConfig, - extension.id, - settingKey, - promptForSetting, - scope, - process.cwd(), - ); -} - -async function configureExtension( - extensionName: string, - scope: ExtensionSettingScope, -) { - const { extension, extensionManager } = - await getExtensionAndManager(extensionName); - if (!extension || !extensionManager) { - return; - } - const extensionConfig = await extensionManager.loadExtensionConfig( - extension.path, - ); - if ( - !extensionConfig || - !extensionConfig.settings || - extensionConfig.settings.length === 0 - ) { - debugLogger.log( - `Extension "${extensionName}" has no settings to configure.`, - ); - return; - } - - debugLogger.log(`Configuring settings for "${extensionName}"...`); - await configureExtensionSettings(extensionConfig, extension.id, scope); -} - -async function configureAllExtensions(scope: ExtensionSettingScope) { - const extensionManager = await getExtensionManager(); - const extensions = extensionManager.getExtensions(); - - if (extensions.length === 0) { - debugLogger.log('No extensions installed.'); - return; - } - - for (const extension of extensions) { - const extensionConfig = await extensionManager.loadExtensionConfig( - extension.path, - ); - if ( - extensionConfig && - extensionConfig.settings && - extensionConfig.settings.length > 0 - ) { - debugLogger.log(`\nConfiguring settings for "${extension.name}"...`); - await configureExtensionSettings(extensionConfig, extension.id, scope); - } - } -} - -async function configureExtensionSettings( - extensionConfig: ExtensionConfig, - extensionId: string, - scope: ExtensionSettingScope, -) { - const currentScopedSettings = await getScopedEnvContents( - extensionConfig, - extensionId, - scope, - process.cwd(), - ); - - let workspaceSettings: Record = {}; - if (scope === ExtensionSettingScope.USER) { - workspaceSettings = await getScopedEnvContents( - extensionConfig, - extensionId, - ExtensionSettingScope.WORKSPACE, - process.cwd(), - ); - } - - if (!extensionConfig.settings) return; - - for (const setting of extensionConfig.settings) { - const currentValue = currentScopedSettings[setting.envVar]; - const workspaceValue = workspaceSettings[setting.envVar]; - - if (workspaceValue !== undefined) { - debugLogger.log( - `Note: Setting "${setting.name}" is already configured in the workspace scope.`, - ); - } - - if (currentValue !== undefined) { - const response = await prompts({ - type: 'confirm', - name: 'overwrite', - message: `Setting "${setting.name}" (${setting.envVar}) is already set. Overwrite?`, - initial: false, - }); - - if (!response.overwrite) { - continue; - } - } - - await updateSetting( - extensionConfig, - extensionId, - setting.envVar, - promptForSetting, - scope, - process.cwd(), - ); - } -} diff --git a/packages/cli/src/commands/extensions/utils.ts b/packages/cli/src/commands/extensions/utils.ts index 1571c56794f..2769826367a 100644 --- a/packages/cli/src/commands/extensions/utils.ts +++ b/packages/cli/src/commands/extensions/utils.ts @@ -1,14 +1,51 @@ /** * @license - * Copyright 2025 Google LLC + * Copyright 2026 Google LLC * SPDX-License-Identifier: Apache-2.0 */ - import { ExtensionManager } from '../../config/extension-manager.js'; -import { promptForSetting } from '../../config/extensions/extensionSettings.js'; import { loadSettings } from '../../config/settings.js'; import { requestConsentNonInteractive } from '../../config/extensions/consent.js'; import { debugLogger } from '@google/gemini-cli-core'; +import type { ExtensionConfig } from '../../config/extension.js'; +import prompts from 'prompts'; +import { + promptForSetting, + updateSetting, + type ExtensionSetting, + getScopedEnvContents, + ExtensionSettingScope, +} from '../../config/extensions/extensionSettings.js'; + +export interface ConfigLogger { + log(message: string): void; + error(message: string): void; +} + +export type RequestSettingCallback = ( + setting: ExtensionSetting, +) => Promise; +export type RequestConfirmationCallback = (message: string) => Promise; + +const defaultLogger: ConfigLogger = { + log: (message: string) => debugLogger.log(message), + error: (message: string) => debugLogger.error(message), +}; + +const defaultRequestSetting: RequestSettingCallback = async (setting) => + promptForSetting(setting); + +const defaultRequestConfirmation: RequestConfirmationCallback = async ( + message, +) => { + const response = await prompts({ + type: 'confirm', + name: 'confirm', + message, + initial: false, + }); + return response.confirm; +}; export async function getExtensionManager() { const workspaceDir = process.cwd(); @@ -22,16 +59,190 @@ export async function getExtensionManager() { return extensionManager; } -export async function getExtensionAndManager(name: string) { - const extensionManager = await getExtensionManager(); +export async function getExtensionAndManager( + extensionManager: ExtensionManager, + name: string, + logger: ConfigLogger = defaultLogger, +) { const extension = extensionManager .getExtensions() .find((ext) => ext.name === name); if (!extension) { - debugLogger.error(`Extension "${name}" is not installed.`); - return { extension: null, extensionManager: null }; + logger.error(`Extension "${name}" is not installed.`); + return { extension: null }; + } + + return { extension }; +} + +export async function configureSpecificSetting( + extensionManager: ExtensionManager, + extensionName: string, + settingKey: string, + scope: ExtensionSettingScope, + logger: ConfigLogger = defaultLogger, + requestSetting: RequestSettingCallback = defaultRequestSetting, +) { + const { extension } = await getExtensionAndManager( + extensionManager, + extensionName, + logger, + ); + if (!extension) { + return; + } + const extensionConfig = await extensionManager.loadExtensionConfig( + extension.path, + ); + if (!extensionConfig) { + logger.error( + `Could not find configuration for extension "${extensionName}".`, + ); + return; + } + + await updateSetting( + extensionConfig, + extension.id, + settingKey, + requestSetting, + scope, + process.cwd(), + ); + + logger.log(`Setting "${settingKey}" updated.`); +} + +export async function configureExtension( + extensionManager: ExtensionManager, + extensionName: string, + scope: ExtensionSettingScope, + logger: ConfigLogger = defaultLogger, + requestSetting: RequestSettingCallback = defaultRequestSetting, + requestConfirmation: RequestConfirmationCallback = defaultRequestConfirmation, +) { + const { extension } = await getExtensionAndManager( + extensionManager, + extensionName, + logger, + ); + if (!extension) { + return; + } + const extensionConfig = await extensionManager.loadExtensionConfig( + extension.path, + ); + if ( + !extensionConfig || + !extensionConfig.settings || + extensionConfig.settings.length === 0 + ) { + logger.log(`Extension "${extensionName}" has no settings to configure.`); + return; + } + + logger.log(`Configuring settings for "${extensionName}"...`); + await configureExtensionSettings( + extensionConfig, + extension.id, + scope, + logger, + requestSetting, + requestConfirmation, + ); +} + +export async function configureAllExtensions( + extensionManager: ExtensionManager, + scope: ExtensionSettingScope, + logger: ConfigLogger = defaultLogger, + requestSetting: RequestSettingCallback = defaultRequestSetting, + requestConfirmation: RequestConfirmationCallback = defaultRequestConfirmation, +) { + const extensions = extensionManager.getExtensions(); + + if (extensions.length === 0) { + logger.log('No extensions installed.'); + return; } - return { extension, extensionManager }; + for (const extension of extensions) { + const extensionConfig = await extensionManager.loadExtensionConfig( + extension.path, + ); + if ( + extensionConfig && + extensionConfig.settings && + extensionConfig.settings.length > 0 + ) { + logger.log(`\nConfiguring settings for "${extension.name}"...`); + await configureExtensionSettings( + extensionConfig, + extension.id, + scope, + logger, + requestSetting, + requestConfirmation, + ); + } + } +} + +export async function configureExtensionSettings( + extensionConfig: ExtensionConfig, + extensionId: string, + scope: ExtensionSettingScope, + logger: ConfigLogger = defaultLogger, + requestSetting: RequestSettingCallback = defaultRequestSetting, + requestConfirmation: RequestConfirmationCallback = defaultRequestConfirmation, +) { + const currentScopedSettings = await getScopedEnvContents( + extensionConfig, + extensionId, + scope, + process.cwd(), + ); + + let workspaceSettings: Record = {}; + if (scope === ExtensionSettingScope.USER) { + workspaceSettings = await getScopedEnvContents( + extensionConfig, + extensionId, + ExtensionSettingScope.WORKSPACE, + process.cwd(), + ); + } + + if (!extensionConfig.settings) return; + + for (const setting of extensionConfig.settings) { + const currentValue = currentScopedSettings[setting.envVar]; + const workspaceValue = workspaceSettings[setting.envVar]; + + if (workspaceValue !== undefined) { + logger.log( + `Note: Setting "${setting.name}" is already configured in the workspace scope.`, + ); + } + + if (currentValue !== undefined) { + const confirmed = await requestConfirmation( + `Setting "${setting.name}" (${setting.envVar}) is already set. Overwrite?`, + ); + + if (!confirmed) { + continue; + } + } + + await updateSetting( + extensionConfig, + extensionId, + setting.envVar, + requestSetting, + scope, + process.cwd(), + ); + } } diff --git a/packages/cli/src/ui/commands/extensionsCommand.test.ts b/packages/cli/src/ui/commands/extensionsCommand.test.ts index 78bbfb828d8..608dee19421 100644 --- a/packages/cli/src/ui/commands/extensionsCommand.test.ts +++ b/packages/cli/src/ui/commands/extensionsCommand.test.ts @@ -4,6 +4,8 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { type ReactElement } from 'react'; + import type { ExtensionLoader, GeminiCLIExtension, @@ -15,7 +17,12 @@ import { completeExtensionsAndScopes, extensionsCommand, } from './extensionsCommand.js'; +import { + ConfigExtensionDialog, + type ConfigExtensionDialogProps, +} from '../components/ConfigExtensionDialog.js'; import { type CommandContext, type SlashCommand } from './types.js'; + import { describe, it, @@ -1017,60 +1024,59 @@ describe('extensionsCommand', () => { vi.mocked(getScopedEnvContents).mockResolvedValue({}); }); - it('should configure all extensions if no args provided', async () => { - await configAction!(mockContext, ''); - expect(mockContext.ui.addItem).toHaveBeenCalledWith( - expect.objectContaining({ - type: MessageType.INFO, - text: expect.stringContaining('Configuring settings for "ext-one"'), - }), - ); - expect(mockContext.ui.addItem).toHaveBeenCalledWith( - expect.objectContaining({ - type: MessageType.INFO, - text: expect.stringContaining('Configuring settings for "ext-two"'), - }), - ); + it('should return dialog to configure all extensions if no args provided', async () => { + const result = await configAction!(mockContext, ''); + if (result?.type !== 'custom_dialog') { + throw new Error('Expected custom_dialog'); + } + const dialogResult = result; + const component = + dialogResult.component as ReactElement; + expect(component.type).toBe(ConfigExtensionDialog); + expect(component.props.configureAll).toBe(true); + expect(component.props.extensionManager).toBeDefined(); }); - it('should configure specific extension', async () => { - await configAction!(mockContext, 'ext-one'); - expect(mockContext.ui.addItem).toHaveBeenCalledWith( - expect.objectContaining({ - type: MessageType.INFO, - text: 'Configuring settings for "ext-one"...', - }), - ); + it('should return dialog to configure specific extension', async () => { + const result = await configAction!(mockContext, 'ext-one'); + if (result?.type !== 'custom_dialog') { + throw new Error('Expected custom_dialog'); + } + const dialogResult = result; + const component = + dialogResult.component as ReactElement; + expect(component.type).toBe(ConfigExtensionDialog); + expect(component.props.extensionName).toBe('ext-one'); + expect(component.props.settingKey).toBeUndefined(); + expect(component.props.configureAll).toBe(false); }); - it('should configure specific setting for an extension', async () => { - const { updateSetting } = await import( - '../../config/extensions/extensionSettings.js' - ); - await configAction!(mockContext, 'ext-one SETTING1'); - expect(updateSetting).toHaveBeenCalledWith( - expect.anything(), - 'ext-one-id', - 'SETTING1', - expect.anything(), - 'user', - expect.anything(), - ); + it('should return dialog to configure specific setting for an extension', async () => { + const result = await configAction!(mockContext, 'ext-one SETTING1'); + if (result?.type !== 'custom_dialog') { + throw new Error('Expected custom_dialog'); + } + const dialogResult = result; + const component = + dialogResult.component as ReactElement; + expect(component.type).toBe(ConfigExtensionDialog); + expect(component.props.extensionName).toBe('ext-one'); + expect(component.props.settingKey).toBe('SETTING1'); + expect(component.props.scope).toBe('user'); // Default scope }); - it('should respect scope argument', async () => { - const { updateSetting } = await import( - '../../config/extensions/extensionSettings.js' - ); - await configAction!(mockContext, 'ext-one SETTING1 --scope=workspace'); - expect(updateSetting).toHaveBeenCalledWith( - expect.anything(), - 'ext-one-id', - 'SETTING1', - expect.anything(), - 'workspace', - expect.anything(), + it('should respect scope argument passed to dialog', async () => { + const result = await configAction!( + mockContext, + 'ext-one SETTING1 --scope=workspace', ); + if (result?.type !== 'custom_dialog') { + throw new Error('Expected custom_dialog'); + } + const dialogResult = result; + const component = + dialogResult.component as ReactElement; + expect(component.props.scope).toBe('workspace'); }); it('should show error for invalid extension name', async () => { @@ -1081,20 +1087,18 @@ describe('extensionsCommand', () => { }); }); - it('should inform if extension has no settings', async () => { - const extensionLoader = - mockContext.services.config!.getExtensionLoader() as ExtensionManager; - vi.mocked(extensionLoader.loadExtensionConfig).mockResolvedValueOnce({ - name: 'ext-one', - version: '1.0.0', - settings: [], - }); - - await configAction!(mockContext, 'ext-one'); - expect(mockContext.ui.addItem).toHaveBeenCalledWith({ - type: MessageType.INFO, - text: 'Extension "ext-one" has no settings to configure.', - }); + // "should inform if extension has no settings" - This check is now inside ConfigExtensionDialog logic. + // We can test that we still return a dialog, and the dialog will handle logical checks via utils.ts + // For unit testing extensionsCommand, we just ensure delegation. + it('should return dialog even if extension has no settings (dialog handles logic)', async () => { + const result = await configAction!(mockContext, 'ext-one'); + if (result?.type !== 'custom_dialog') { + throw new Error('Expected custom_dialog'); + } + const dialogResult = result; + const component = + dialogResult.component as ReactElement; + expect(component.type).toBe(ConfigExtensionDialog); }); }); }); diff --git a/packages/cli/src/ui/commands/extensionsCommand.ts b/packages/cli/src/ui/commands/extensionsCommand.ts index aecf8fd993c..cd473020648 100644 --- a/packages/cli/src/ui/commands/extensionsCommand.ts +++ b/packages/cli/src/ui/commands/extensionsCommand.ts @@ -9,10 +9,7 @@ import { listExtensions, type ExtensionInstallMetadata, } from '@google/gemini-cli-core'; -import type { - ExtensionConfig, - ExtensionUpdateInfo, -} from '../../config/extension.js'; +import type { ExtensionUpdateInfo } from '../../config/extension.js'; import { getErrorMessage } from '../../utils/errors.js'; import { emptyIcon, @@ -35,13 +32,10 @@ import { SettingScope } from '../../config/settings.js'; import { McpServerEnablementManager } from '../../config/mcp/mcpServerEnablement.js'; import { theme } from '../semantic-colors.js'; import { stat } from 'node:fs/promises'; -import { - ExtensionSettingScope, - getScopedEnvContents, - promptForSetting, - updateSetting, -} from '../../config/extensions/extensionSettings.js'; -import prompts from 'prompts'; +import { ExtensionSettingScope } from '../../config/extensions/extensionSettings.js'; +import { type ConfigLogger } from '../../commands/extensions/utils.js'; +import { ConfigExtensionDialog } from '../components/ConfigExtensionDialog.js'; +import React from 'react'; function showMessageIfNoExtensions( context: CommandContext, @@ -623,217 +617,34 @@ async function configAction(context: CommandContext, args: string) { } } - // Case 1: Configure specific setting for an extension - if (name && setting) { - await configureSpecificSetting(context, name, setting, scope); - } - // Case 2: Configure all settings for an extension - else if (name) { - await configureExtension(context, name, scope); - } - // Case 3: Configure all extensions - else { - await configureAllExtensions(context, scope); - } -} - -async function configureSpecificSetting( - context: CommandContext, - extensionName: string, - settingKey: string, - scope: ExtensionSettingScope, -) { const extensionManager = context.services.config?.getExtensionLoader(); - if (!(extensionManager instanceof ExtensionManager)) return; - - const extension = extensionManager - .getExtensions() - .find((e) => e.name === extensionName); - if (!extension) { - context.ui.addItem({ - type: MessageType.ERROR, - text: `Extension "${extensionName}" not found.`, - }); - return; - } - - const extensionConfig = await extensionManager.loadExtensionConfig( - extension.path, - ); - if (!extensionConfig) { - context.ui.addItem({ - type: MessageType.ERROR, - text: `Could not find configuration for extension "${extensionName}".`, - }); - return; - } - - await updateSetting( - extensionConfig, - extension.id, - settingKey, - promptForSetting, - scope, - process.cwd(), - ); - context.ui.addItem({ - type: MessageType.INFO, - text: `Setting "${settingKey}" updated.`, - }); -} - -async function configureExtension( - context: CommandContext, - extensionName: string, - scope: ExtensionSettingScope, -) { - const extensionManager = context.services.config?.getExtensionLoader(); - if (!(extensionManager instanceof ExtensionManager)) return; - - const extension = extensionManager - .getExtensions() - .find((e) => e.name === extensionName); - if (!extension) { - context.ui.addItem({ - type: MessageType.ERROR, - text: `Extension "${extensionName}" not found.`, - }); - return; - } - - const extensionConfig = await extensionManager.loadExtensionConfig( - extension.path, - ); - if ( - !extensionConfig || - !extensionConfig.settings || - extensionConfig.settings.length === 0 - ) { - context.ui.addItem({ - type: MessageType.INFO, - text: `Extension "${extensionName}" has no settings to configure.`, - }); - return; - } - - context.ui.addItem({ - type: MessageType.INFO, - text: `Configuring settings for "${extensionName}"...`, - }); - await configureExtensionSettings( - context, - extensionConfig, - extension.id, - scope, - ); - context.ui.addItem({ - type: MessageType.INFO, - text: `Configuration for "${extensionName}" completed.`, - }); -} - -async function configureAllExtensions( - context: CommandContext, - scope: ExtensionSettingScope, -) { - const extensionManager = context.services.config?.getExtensionLoader(); - if (!(extensionManager instanceof ExtensionManager)) return; - - const extensions = extensionManager.getExtensions(); - - if (extensions.length === 0) { - context.ui.addItem({ - type: MessageType.INFO, - text: 'No extensions installed.', - }); - return; - } - - for (const extension of extensions) { - const extensionConfig = await extensionManager.loadExtensionConfig( - extension.path, - ); - if ( - extensionConfig && - extensionConfig.settings && - extensionConfig.settings.length > 0 - ) { - context.ui.addItem({ - type: MessageType.INFO, - text: `\nConfiguring settings for "${extension.name}"...`, - }); - await configureExtensionSettings( - context, - extensionConfig, - extension.id, - scope, - ); - } - } - context.ui.addItem({ - type: MessageType.INFO, - text: 'All extensions configured.', - }); -} - -async function configureExtensionSettings( - context: CommandContext, - extensionConfig: ExtensionConfig, - extensionId: string, - scope: ExtensionSettingScope, -) { - const currentScopedSettings = await getScopedEnvContents( - extensionConfig, - extensionId, - scope, - process.cwd(), - ); - - let workspaceSettings: Record = {}; - if (scope === ExtensionSettingScope.USER) { - workspaceSettings = await getScopedEnvContents( - extensionConfig, - extensionId, - ExtensionSettingScope.WORKSPACE, - process.cwd(), + if (!(extensionManager instanceof ExtensionManager)) { + debugLogger.error( + `Cannot ${context.invocation?.name} extensions in this environment`, ); + return; } - if (!extensionConfig.settings) return; - - for (const setting of extensionConfig.settings) { - const currentValue = currentScopedSettings[setting.envVar]; - const workspaceValue = workspaceSettings[setting.envVar]; - - if (workspaceValue !== undefined) { - context.ui.addItem({ - type: MessageType.INFO, - text: `Note: Setting "${setting.name}" is already configured in the workspace scope.`, - }); - } - - if (currentValue !== undefined) { - const response = await prompts({ - type: 'confirm', - name: 'overwrite', - message: `Setting "${setting.name}" (${setting.envVar}) is already set. Overwrite?`, - initial: false, - }); - - if (!response.overwrite) { - continue; - } - } + const logger: ConfigLogger = { + log: (message: string) => { + context.ui.addItem({ type: MessageType.INFO, text: message.trim() }); + }, + error: (message: string) => + context.ui.addItem({ type: MessageType.ERROR, text: message }), + }; - await updateSetting( - extensionConfig, - extensionId, - setting.envVar, - promptForSetting, + return { + type: 'custom_dialog' as const, + component: React.createElement(ConfigExtensionDialog, { + extensionManager, + onClose: () => context.ui.removeComponent(), + extensionName: name, + settingKey: setting, scope, - process.cwd(), - ); - } + configureAll: !name && !setting, + loggerAdapter: logger, + }), + }; } /** diff --git a/packages/cli/src/ui/components/ConfigExtensionDialog.tsx b/packages/cli/src/ui/components/ConfigExtensionDialog.tsx new file mode 100644 index 00000000000..bbecf440f5d --- /dev/null +++ b/packages/cli/src/ui/components/ConfigExtensionDialog.tsx @@ -0,0 +1,343 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import type React from 'react'; +import { useEffect, useState, useRef, useCallback } from 'react'; +import { Box, Text } from 'ink'; +import { theme } from '../semantic-colors.js'; +import type { ExtensionManager } from '../../config/extension-manager.js'; +import { + configureExtension, + configureSpecificSetting, + configureAllExtensions, + type ConfigLogger, + type RequestSettingCallback, + type RequestConfirmationCallback, +} from '../../commands/extensions/utils.js'; +import { + ExtensionSettingScope, + type ExtensionSetting, +} from '../../config/extensions/extensionSettings.js'; +import { TextInput } from './shared/TextInput.js'; +import { useTextBuffer } from './shared/text-buffer.js'; +import { DialogFooter } from './shared/DialogFooter.js'; +import { type Key, useKeypress } from '../hooks/useKeypress.js'; + +export interface ConfigExtensionDialogProps { + extensionManager: ExtensionManager; + onClose: () => void; + extensionName?: string; + settingKey?: string; + scope?: ExtensionSettingScope; + configureAll?: boolean; + loggerAdapter: ConfigLogger; +} + +type DialogState = + | { type: 'IDLE' } + | { type: 'BUSY'; message?: string } + | { + type: 'ASK_SETTING'; + setting: ExtensionSetting; + resolve: (val: string) => void; + initialValue?: string; + } + | { + type: 'ASK_CONFIRMATION'; + message: string; + resolve: (val: boolean) => void; + } + | { type: 'DONE' } + | { type: 'ERROR'; error: Error }; + +export const ConfigExtensionDialog: React.FC = ({ + extensionManager, + onClose, + extensionName, + settingKey, + scope = ExtensionSettingScope.USER, + configureAll, + loggerAdapter, +}) => { + const [state, setState] = useState({ type: 'IDLE' }); + const [logMessages, setLogMessages] = useState([]); + + // Buffers for input + const settingBuffer = useTextBuffer({ + initialText: '', + viewport: { width: 80, height: 1 }, + singleLine: true, + isValidPath: () => true, + }); + + const mounted = useRef(true); + + useEffect(() => { + mounted.current = true; + return () => { + mounted.current = false; + }; + }, []); + + const addLog = useCallback( + (msg: string) => { + setLogMessages((prev) => [...prev, msg].slice(-5)); // Keep last 5 + loggerAdapter.log(msg); + }, + [loggerAdapter], + ); + + const requestSetting: RequestSettingCallback = useCallback( + async (setting) => + new Promise((resolve) => { + if (!mounted.current) return; + settingBuffer.setText(''); // Clear buffer + setState({ + type: 'ASK_SETTING', + setting, + resolve: (val) => { + resolve(val); + setState({ type: 'BUSY', message: 'Updating...' }); + }, + }); + }), + [settingBuffer], + ); + + const requestConfirmation: RequestConfirmationCallback = useCallback( + async (message) => + new Promise((resolve) => { + if (!mounted.current) return; + setState({ + type: 'ASK_CONFIRMATION', + message, + resolve: (val) => { + resolve(val); + setState({ type: 'BUSY', message: 'Processing...' }); + }, + }); + }), + [], + ); + + useEffect(() => { + async function run() { + try { + setState({ type: 'BUSY', message: 'Initializing...' }); + + // Wrap logger to capture logs locally too + const localLogger: ConfigLogger = { + log: (msg) => { + addLog(msg); + }, + error: (msg) => { + addLog('Error: ' + msg); + loggerAdapter.error(msg); + }, + }; + + if (configureAll) { + await configureAllExtensions( + extensionManager, + scope, + localLogger, + requestSetting, + requestConfirmation, + ); + } else if (extensionName && settingKey) { + await configureSpecificSetting( + extensionManager, + extensionName, + settingKey, + scope, + localLogger, + requestSetting, + ); + } else if (extensionName) { + await configureExtension( + extensionManager, + extensionName, + scope, + localLogger, + requestSetting, + requestConfirmation, + ); + } + + if (mounted.current) { + setState({ type: 'DONE' }); + // Delay close slightly to show done + setTimeout(onClose, 1000); + } + } catch (err: unknown) { + if (mounted.current) { + const error = err instanceof Error ? err : new Error(String(err)); + setState({ type: 'ERROR', error }); + loggerAdapter.error(error.message); + } + } + } + + // Only run once + if (state.type === 'IDLE') { + void run(); + } + }, [ + extensionManager, + extensionName, + settingKey, + scope, + configureAll, + loggerAdapter, + requestSetting, + requestConfirmation, + addLog, + onClose, + state.type, + ]); + + // Handle Input Submission + const handleSettingSubmit = (val: string) => { + if (state.type === 'ASK_SETTING') { + state.resolve(val); + } + }; + + // Handle Keys for Confirmation + useKeypress( + (key: Key) => { + if (state.type === 'ASK_CONFIRMATION') { + if (key.name === 'y' || key.name === 'return') { + state.resolve(true); + return true; + } + if (key.name === 'n' || key.name === 'escape') { + state.resolve(false); + return true; + } + } + if (state.type === 'DONE' || state.type === 'ERROR') { + if (key.name === 'return' || key.name === 'escape') { + onClose(); + return true; + } + } + return false; + }, + { + isActive: + state.type === 'ASK_CONFIRMATION' || + state.type === 'DONE' || + state.type === 'ERROR', + }, + ); + + if (state.type === 'BUSY' || state.type === 'IDLE') { + return ( + + + {state.type === 'BUSY' ? state.message : 'Starting...'} + + {logMessages.map((msg, i) => ( + {msg} + ))} + + ); + } + + if (state.type === 'ASK_SETTING') { + return ( + + + Configure {state.setting.name} + + + {state.setting.description || state.setting.envVar} + + + {'> '} + + + + + ); + } + + if (state.type === 'ASK_CONFIRMATION') { + return ( + + + Confirmation Required + + {state.message} + + + Press{' '} + + Y + {' '} + to confirm or{' '} + + N + {' '} + to cancel + + + + ); + } + + if (state.type === 'ERROR') { + return ( + + + Error + + {state.error.message} + + + ); + } + + return ( + + + Configuration Complete + + + + ); +}; From d582fbe5bac0e85aa9c793eefb479eeb367d3944 Mon Sep 17 00:00:00 2001 From: Christine Betts Date: Mon, 2 Feb 2026 12:15:38 -0500 Subject: [PATCH 3/4] fix: address code review comments for extensions command and settings - Fix environment variable injection vulnerability in extensionSettings.ts by validating keys and rejecting values with newlines. - Improve quoting logic for environment variable values. - Fix argument parsing for --scope flag in extensionsCommand.ts to handle both equals and space separators. - Add tests for security fixes in extensionSettings.test.ts. --- .../extensions/extensionSettings.test.ts | 69 +++++++++++++++++++ .../config/extensions/extensionSettings.ts | 14 +++- .../cli/src/ui/commands/extensionsCommand.ts | 33 ++++++--- 3 files changed, 104 insertions(+), 12 deletions(-) diff --git a/packages/cli/src/config/extensions/extensionSettings.test.ts b/packages/cli/src/config/extensions/extensionSettings.test.ts index 09ed586b821..bdfa896db61 100644 --- a/packages/cli/src/config/extensions/extensionSettings.test.ts +++ b/packages/cli/src/config/extensions/extensionSettings.test.ts @@ -804,5 +804,74 @@ describe('extensionSettings', () => { ); // Should complete without error }); + + it('should throw error if env var name contains invalid characters', async () => { + const securityConfig: ExtensionConfig = { + name: 'test-ext', + version: '1.0.0', + settings: [{ name: 's2', description: 'd2', envVar: 'VAR-BAD' }], + }; + mockRequestSetting.mockResolvedValue('value'); + + await expect( + updateSetting( + securityConfig, + '12345', + 'VAR-BAD', + mockRequestSetting, + ExtensionSettingScope.USER, + tempWorkspaceDir, + ), + ).rejects.toThrow(/Invalid environment variable name/); + }); + + it('should throw error if env var value contains newlines', async () => { + mockRequestSetting.mockResolvedValue('value\nwith\nnewlines'); + + await expect( + updateSetting( + config, + '12345', + 'VAR1', + mockRequestSetting, + ExtensionSettingScope.USER, + tempWorkspaceDir, + ), + ).rejects.toThrow(/Invalid environment variable value/); + }); + + it('should quote values with spaces', async () => { + mockRequestSetting.mockResolvedValue('value with spaces'); + + await updateSetting( + config, + '12345', + 'VAR1', + mockRequestSetting, + ExtensionSettingScope.USER, + tempWorkspaceDir, + ); + + const expectedEnvPath = path.join(extensionDir, '.env'); + const actualContent = await fsPromises.readFile(expectedEnvPath, 'utf-8'); + expect(actualContent).toContain('VAR1="value with spaces"'); + }); + + it('should escape quotes in values', async () => { + mockRequestSetting.mockResolvedValue('value with "quotes"'); + + await updateSetting( + config, + '12345', + 'VAR1', + mockRequestSetting, + ExtensionSettingScope.USER, + tempWorkspaceDir, + ); + + const expectedEnvPath = path.join(extensionDir, '.env'); + const actualContent = await fsPromises.readFile(expectedEnvPath, 'utf-8'); + expect(actualContent).toContain('VAR1="value with \\"quotes\\""'); + }); }); }); diff --git a/packages/cli/src/config/extensions/extensionSettings.ts b/packages/cli/src/config/extensions/extensionSettings.ts index 4ba7d34b354..b9384f22963 100644 --- a/packages/cli/src/config/extensions/extensionSettings.ts +++ b/packages/cli/src/config/extensions/extensionSettings.ts @@ -130,7 +130,19 @@ export async function maybePromptForSettings( function formatEnvContent(settings: Record): string { let envContent = ''; for (const [key, value] of Object.entries(settings)) { - const formattedValue = value.includes(' ') ? `"${value}"` : value; + if (!/^[a-zA-Z_][a-zA-Z0-9_]*$/.test(key)) { + throw new Error( + `Invalid environment variable name: "${key}". Must contain only alphanumeric characters and underscores.`, + ); + } + if (value.includes('\n') || value.includes('\r')) { + throw new Error( + `Invalid environment variable value for "${key}". Values cannot contain newlines.`, + ); + } + const formattedValue = value.includes(' ') + ? `"${value.replace(/"/g, '\\"')}"` + : value; envContent += `${key}=${formattedValue}\n`; } return envContent; diff --git a/packages/cli/src/ui/commands/extensionsCommand.ts b/packages/cli/src/ui/commands/extensionsCommand.ts index cd473020648..4cf48d7662d 100644 --- a/packages/cli/src/ui/commands/extensionsCommand.ts +++ b/packages/cli/src/ui/commands/extensionsCommand.ts @@ -590,20 +590,31 @@ async function uninstallAction(context: CommandContext, args: string) { async function configAction(context: CommandContext, args: string) { const parts = args.trim().split(/\s+/).filter(Boolean); let scope = ExtensionSettingScope.USER; - const otherArgs: string[] = []; - - for (const part of parts) { - if (part.startsWith('--scope=')) { - const scopeVal = part.split('=')[1]; - if (scopeVal === 'workspace') scope = ExtensionSettingScope.WORKSPACE; - else if (scopeVal === 'user') scope = ExtensionSettingScope.USER; - } else if (part === '--scope') { - // Handle next arg as scope value if needed, but simple parsing for now - } else { - otherArgs.push(part); + + const scopeEqIndex = parts.findIndex((p) => p.startsWith('--scope=')); + if (scopeEqIndex > -1) { + const scopeVal = parts[scopeEqIndex].split('=')[1]; + if (scopeVal === 'workspace') { + scope = ExtensionSettingScope.WORKSPACE; + } else if (scopeVal === 'user') { + scope = ExtensionSettingScope.USER; + } + parts.splice(scopeEqIndex, 1); + } else { + const scopeIndex = parts.indexOf('--scope'); + if (scopeIndex > -1) { + const scopeVal = parts[scopeIndex + 1]; + if (scopeVal === 'workspace' || scopeVal === 'user') { + scope = + scopeVal === 'workspace' + ? ExtensionSettingScope.WORKSPACE + : ExtensionSettingScope.USER; + parts.splice(scopeIndex, 2); + } } } + const otherArgs = parts; const name = otherArgs[0]; const setting = otherArgs[1]; From 4e98913f9372b296531d61225eb62fb864e10b00 Mon Sep 17 00:00:00 2001 From: Christine Betts Date: Mon, 2 Feb 2026 12:29:09 -0500 Subject: [PATCH 4/4] resolve issue --- packages/cli/src/config/extensions/extensionSettings.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/config/extensions/extensionSettings.ts b/packages/cli/src/config/extensions/extensionSettings.ts index b9384f22963..50a3bc81e95 100644 --- a/packages/cli/src/config/extensions/extensionSettings.ts +++ b/packages/cli/src/config/extensions/extensionSettings.ts @@ -141,7 +141,7 @@ function formatEnvContent(settings: Record): string { ); } const formattedValue = value.includes(' ') - ? `"${value.replace(/"/g, '\\"')}"` + ? `"${value.replace(/\\/g, '\\\\').replace(/"/g, '\\"')}"` : value; envContent += `${key}=${formattedValue}\n`; }