diff --git a/packages/cli/src/config/extension.test.ts b/packages/cli/src/config/extension.test.ts index 1de6eadd6a..915fd55f4a 100644 --- a/packages/cli/src/config/extension.test.ts +++ b/packages/cli/src/config/extension.test.ts @@ -373,6 +373,70 @@ describe('extension tests', () => { expect(serverConfig.env!.MISSING_VAR_BRACES).toBe('${ALSO_UNDEFINED}'); }); + it('should skip extensions with invalid JSON and log a warning', () => { + const consoleSpy = vi + .spyOn(console, 'error') + .mockImplementation(() => {}); + + // Good extension + createExtension({ + extensionsDir: userExtensionsDir, + name: 'good-ext', + version: '1.0.0', + }); + + // Bad extension + const badExtDir = path.join(userExtensionsDir, 'bad-ext'); + fs.mkdirSync(badExtDir); + const badConfigPath = path.join(badExtDir, EXTENSIONS_CONFIG_FILENAME); + fs.writeFileSync(badConfigPath, '{ "name": "bad-ext"'); // Malformed + + const extensions = loadExtensions(); + + expect(extensions).toHaveLength(1); + expect(extensions[0].config.name).toBe('good-ext'); + expect(consoleSpy).toHaveBeenCalledOnce(); + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringContaining( + `Warning: Skipping extension in ${badExtDir}: Failed to load extension config from ${badConfigPath}`, + ), + ); + + consoleSpy.mockRestore(); + }); + + it('should skip extensions with missing name and log a warning', () => { + const consoleSpy = vi + .spyOn(console, 'error') + .mockImplementation(() => {}); + + // Good extension + createExtension({ + extensionsDir: userExtensionsDir, + name: 'good-ext', + version: '1.0.0', + }); + + // Bad extension + const badExtDir = path.join(userExtensionsDir, 'bad-ext-no-name'); + fs.mkdirSync(badExtDir); + const badConfigPath = path.join(badExtDir, EXTENSIONS_CONFIG_FILENAME); + fs.writeFileSync(badConfigPath, JSON.stringify({ version: '1.0.0' })); + + const extensions = loadExtensions(); + + expect(extensions).toHaveLength(1); + expect(extensions[0].config.name).toBe('good-ext'); + expect(consoleSpy).toHaveBeenCalledOnce(); + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringContaining( + `Warning: Skipping extension in ${badExtDir}: Failed to load extension config from ${badConfigPath}: Invalid configuration in ${badConfigPath}: missing "name"`, + ), + ); + + consoleSpy.mockRestore(); + }); + it('should filter trust out of mcp servers', () => { createExtension({ extensionsDir: userExtensionsDir, @@ -591,15 +655,49 @@ describe('extension tests', () => { it('should throw an error and cleanup if gemini-extension.json is missing', async () => { const sourceExtDir = path.join(tempHomeDir, 'bad-extension'); fs.mkdirSync(sourceExtDir, { recursive: true }); + const configPath = path.join(sourceExtDir, EXTENSIONS_CONFIG_FILENAME); + + await expect( + installExtension({ source: sourceExtDir, type: 'local' }), + ).rejects.toThrow(`Configuration file not found at ${configPath}`); + + const targetExtDir = path.join(userExtensionsDir, 'bad-extension'); + expect(fs.existsSync(targetExtDir)).toBe(false); + }); + + it('should throw an error for invalid JSON in gemini-extension.json', async () => { + const sourceExtDir = path.join(tempHomeDir, 'bad-json-ext'); + fs.mkdirSync(sourceExtDir, { recursive: true }); + const configPath = path.join(sourceExtDir, EXTENSIONS_CONFIG_FILENAME); + fs.writeFileSync(configPath, '{ "name": "bad-json", "version": "1.0.0"'); // Malformed JSON await expect( installExtension({ source: sourceExtDir, type: 'local' }), ).rejects.toThrow( - `Invalid extension at ${sourceExtDir}. Please make sure it has a valid gemini-extension.json file.`, + new RegExp( + `^Failed to load extension config from ${configPath.replace( + /\\/g, + '\\\\', + )}`, + ), ); + }); - const targetExtDir = path.join(userExtensionsDir, 'bad-extension'); - expect(fs.existsSync(targetExtDir)).toBe(false); + it('should throw an error for missing name in gemini-extension.json', async () => { + const sourceExtDir = createExtension({ + extensionsDir: tempHomeDir, + name: 'missing-name-ext', + version: '1.0.0', + }); + const configPath = path.join(sourceExtDir, EXTENSIONS_CONFIG_FILENAME); + // Overwrite with invalid config + fs.writeFileSync(configPath, JSON.stringify({ version: '1.0.0' })); + + await expect( + installExtension({ source: sourceExtDir, type: 'local' }), + ).rejects.toThrow( + `Invalid configuration in ${configPath}: missing "name"`, + ); }); it('should install an extension from a git URL', async () => { diff --git a/packages/cli/src/config/extension.ts b/packages/cli/src/config/extension.ts index 857c034db0..28a8d62c6f 100644 --- a/packages/cli/src/config/extension.ts +++ b/packages/cli/src/config/extension.ts @@ -211,31 +211,11 @@ export function loadExtension(context: LoadExtensionContext): Extension | null { effectiveExtensionPath = installMetadata.source; } - const configFilePath = path.join( - effectiveExtensionPath, - EXTENSIONS_CONFIG_FILENAME, - ); - if (!fs.existsSync(configFilePath)) { - console.error( - `Warning: extension directory ${effectiveExtensionPath} does not contain a config file ${configFilePath}.`, - ); - return null; - } - try { - const configContent = fs.readFileSync(configFilePath, 'utf-8'); - let config = recursivelyHydrateStrings(JSON.parse(configContent), { - extensionPath: extensionDir, - workspacePath: workspaceDir, - '/': path.sep, - pathSeparator: path.sep, - }) as unknown as ExtensionConfig; - if (!config.name || !config.version) { - console.error( - `Invalid extension config in ${configFilePath}: missing name or version.`, - ); - return null; - } + let config = loadExtensionConfig({ + extensionDir: effectiveExtensionPath, + workspaceDir, + }); config = resolveEnvVarsInObject(config); @@ -262,7 +242,7 @@ export function loadExtension(context: LoadExtensionContext): Extension | null { }; } catch (e) { console.error( - `Warning: error parsing extension config in ${configFilePath}: ${getErrorMessage( + `Warning: Skipping extension in ${effectiveExtensionPath}: ${getErrorMessage( e, )}`, ); @@ -443,15 +423,10 @@ export async function installExtension( } try { - newExtensionConfig = await loadExtensionConfig({ + newExtensionConfig = loadExtensionConfig({ extensionDir: localSourcePath, workspaceDir: cwd, }); - if (!newExtensionConfig) { - throw new Error( - `Invalid extension at ${installMetadata.source}. Please make sure it has a valid gemini-extension.json file.`, - ); - } const newExtensionName = newExtensionConfig.name; const extensionStorage = new ExtensionStorage(newExtensionName); @@ -507,10 +482,14 @@ export async function installExtension( // Attempt to load config from the source path even if installation fails // to get the name and version for logging. if (!newExtensionConfig && localSourcePath) { - newExtensionConfig = await loadExtensionConfig({ - extensionDir: localSourcePath, - workspaceDir: cwd, - }); + try { + newExtensionConfig = loadExtensionConfig({ + extensionDir: localSourcePath, + workspaceDir: cwd, + }); + } catch { + // Ignore error, this is just for logging. + } } logger?.logExtensionInstallEvent( new ExtensionInstallEvent( @@ -560,13 +539,13 @@ async function requestConsent(extensionConfig: ExtensionConfig) { } } -export async function loadExtensionConfig( +export function loadExtensionConfig( context: LoadExtensionContext, -): Promise { +): ExtensionConfig { const { extensionDir, workspaceDir } = context; const configFilePath = path.join(extensionDir, EXTENSIONS_CONFIG_FILENAME); if (!fs.existsSync(configFilePath)) { - return null; + throw new Error(`Configuration file not found at ${configFilePath}`); } try { const configContent = fs.readFileSync(configFilePath, 'utf-8'); @@ -577,11 +556,17 @@ export async function loadExtensionConfig( pathSeparator: path.sep, }) as unknown as ExtensionConfig; if (!config.name || !config.version) { - return null; + throw new Error( + `Invalid configuration in ${configFilePath}: missing ${!config.name ? '"name"' : '"version"'}`, + ); } return config; - } catch (_) { - return null; + } catch (e) { + throw new Error( + `Failed to load extension config from ${configFilePath}: ${getErrorMessage( + e, + )}`, + ); } }