From ea844857a2fb3d8f38f4b6a140fdee19a7bde67e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Albert=20Mul=C3=A0?= Date: Fri, 29 Aug 2025 19:53:39 +0200 Subject: [PATCH] feat(extension): resolve environment variables in extension configuration (#7213) Co-authored-by: Tommaso Sciortino Co-authored-by: Jacob Richman --- packages/cli/src/config/extension.test.ts | 94 ++++++ packages/cli/src/config/extension.ts | 5 +- packages/cli/src/config/settings.ts | 44 +-- packages/cli/src/utils/envVarResolver.test.ts | 297 ++++++++++++++++++ packages/cli/src/utils/envVarResolver.ts | 112 +++++++ 5 files changed, 509 insertions(+), 43 deletions(-) create mode 100644 packages/cli/src/utils/envVarResolver.test.ts create mode 100644 packages/cli/src/utils/envVarResolver.ts diff --git a/packages/cli/src/config/extension.test.ts b/packages/cli/src/config/extension.test.ts index 707cd23543..6db44665e2 100644 --- a/packages/cli/src/config/extension.test.ts +++ b/packages/cli/src/config/extension.test.ts @@ -216,6 +216,100 @@ describe('loadExtensions', () => { ); expect(loadedConfig.mcpServers?.['test-server'].cwd).toBe(expectedCwd); }); + + it('should resolve environment variables in extension configuration', () => { + process.env.TEST_API_KEY = 'test-api-key-123'; + process.env.TEST_DB_URL = 'postgresql://localhost:5432/testdb'; + + try { + const workspaceExtensionsDir = path.join( + tempWorkspaceDir, + EXTENSIONS_DIRECTORY_NAME, + ); + fs.mkdirSync(workspaceExtensionsDir, { recursive: true }); + + const extDir = path.join(workspaceExtensionsDir, 'test-extension'); + fs.mkdirSync(extDir); + + // Write config to a separate file for clarity and good practices + const configPath = path.join(extDir, EXTENSIONS_CONFIG_FILENAME); + const extensionConfig = { + name: 'test-extension', + version: '1.0.0', + mcpServers: { + 'test-server': { + command: 'node', + args: ['server.js'], + env: { + API_KEY: '$TEST_API_KEY', + DATABASE_URL: '${TEST_DB_URL}', + STATIC_VALUE: 'no-substitution', + }, + }, + }, + }; + fs.writeFileSync(configPath, JSON.stringify(extensionConfig)); + + const extensions = loadExtensions(tempWorkspaceDir); + + expect(extensions).toHaveLength(1); + const extension = extensions[0]; + expect(extension.config.name).toBe('test-extension'); + expect(extension.config.mcpServers).toBeDefined(); + + const serverConfig = extension.config.mcpServers?.['test-server']; + expect(serverConfig).toBeDefined(); + expect(serverConfig?.env).toBeDefined(); + expect(serverConfig?.env?.API_KEY).toBe('test-api-key-123'); + expect(serverConfig?.env?.DATABASE_URL).toBe( + 'postgresql://localhost:5432/testdb', + ); + expect(serverConfig?.env?.STATIC_VALUE).toBe('no-substitution'); + } finally { + delete process.env.TEST_API_KEY; + delete process.env.TEST_DB_URL; + } + }); + + it('should handle missing environment variables gracefully', () => { + const workspaceExtensionsDir = path.join( + tempWorkspaceDir, + EXTENSIONS_DIRECTORY_NAME, + ); + fs.mkdirSync(workspaceExtensionsDir, { recursive: true }); + + const extDir = path.join(workspaceExtensionsDir, 'test-extension'); + fs.mkdirSync(extDir); + + const extensionConfig = { + name: 'test-extension', + version: '1.0.0', + mcpServers: { + 'test-server': { + command: 'node', + args: ['server.js'], + env: { + MISSING_VAR: '$UNDEFINED_ENV_VAR', + MISSING_VAR_BRACES: '${ALSO_UNDEFINED}', + }, + }, + }, + }; + + fs.writeFileSync( + path.join(extDir, EXTENSIONS_CONFIG_FILENAME), + JSON.stringify(extensionConfig), + ); + + const extensions = loadExtensions(tempWorkspaceDir); + + expect(extensions).toHaveLength(1); + const extension = extensions[0]; + const serverConfig = extension.config.mcpServers!['test-server']; + expect(serverConfig.env).toBeDefined(); + expect(serverConfig.env!.MISSING_VAR).toBe('$UNDEFINED_ENV_VAR'); + expect(serverConfig.env!.MISSING_VAR_BRACES).toBe('${ALSO_UNDEFINED}'); + }); }); describe('annotateActiveExtensions', () => { diff --git a/packages/cli/src/config/extension.ts b/packages/cli/src/config/extension.ts index e87eda4c68..f8bd250d87 100644 --- a/packages/cli/src/config/extension.ts +++ b/packages/cli/src/config/extension.ts @@ -17,6 +17,7 @@ import { SettingScope, loadSettings } from '../config/settings.js'; import { getErrorMessage } from '../utils/errors.js'; import { recursivelyHydrateStrings } from './extensions/variables.js'; import { isWorkspaceTrusted } from './trustedFolders.js'; +import { resolveEnvVarsInObject } from '../utils/envVarResolver.js'; export const EXTENSIONS_DIRECTORY_NAME = path.join(GEMINI_DIR, 'extensions'); @@ -184,7 +185,7 @@ export function loadExtension(extensionDir: string): Extension | null { try { const configContent = fs.readFileSync(configFilePath, 'utf-8'); - const config = recursivelyHydrateStrings(JSON.parse(configContent), { + let config = recursivelyHydrateStrings(JSON.parse(configContent), { extensionPath: extensionDir, '/': path.sep, pathSeparator: path.sep, @@ -196,6 +197,8 @@ export function loadExtension(extensionDir: string): Extension | null { return null; } + config = resolveEnvVarsInObject(config); + const contextFiles = getContextFileNames(config) .map((contextFileName) => path.join(extensionDir, contextFileName)) .filter((contextFilePath) => fs.existsSync(contextFilePath)); diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts index 1fde0c0f8b..572c86111d 100644 --- a/packages/cli/src/config/settings.ts +++ b/packages/cli/src/config/settings.ts @@ -8,6 +8,7 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import { homedir, platform } from 'node:os'; import * as dotenv from 'dotenv'; +import process from 'node:process'; import { GEMINI_CONFIG_DIR as GEMINI_DIR, getErrorMessage, @@ -18,6 +19,7 @@ import { DefaultLight } from '../ui/themes/default-light.js'; import { DefaultDark } from '../ui/themes/default.js'; import { isWorkspaceTrusted } from './trustedFolders.js'; import type { Settings, MemoryImportFormat } from './settingsSchema.js'; +import { resolveEnvVarsInObject } from '../utils/envVarResolver.js'; import { mergeWith } from 'lodash-es'; export type { Settings, MemoryImportFormat }; @@ -462,48 +464,6 @@ export class LoadedSettings { } } -function resolveEnvVarsInString(value: string): string { - const envVarRegex = /\$(?:(\w+)|{([^}]+)})/g; // Find $VAR_NAME or ${VAR_NAME} - return value.replace(envVarRegex, (match, varName1, varName2) => { - const varName = varName1 || varName2; - if (process && process.env && typeof process.env[varName] === 'string') { - return process.env[varName]!; - } - return match; - }); -} - -function resolveEnvVarsInObject(obj: T): T { - if ( - obj === null || - obj === undefined || - typeof obj === 'boolean' || - typeof obj === 'number' - ) { - return obj; - } - - if (typeof obj === 'string') { - return resolveEnvVarsInString(obj) as unknown as T; - } - - if (Array.isArray(obj)) { - return obj.map((item) => resolveEnvVarsInObject(item)) as unknown as T; - } - - if (typeof obj === 'object') { - const newObj = { ...obj } as T; - for (const key in newObj) { - if (Object.prototype.hasOwnProperty.call(newObj, key)) { - newObj[key] = resolveEnvVarsInObject(newObj[key]); - } - } - return newObj; - } - - return obj; -} - function findEnvFile(startDir: string): string | null { let currentDir = path.resolve(startDir); while (true) { diff --git a/packages/cli/src/utils/envVarResolver.test.ts b/packages/cli/src/utils/envVarResolver.test.ts new file mode 100644 index 0000000000..a45b65b1b9 --- /dev/null +++ b/packages/cli/src/utils/envVarResolver.test.ts @@ -0,0 +1,297 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { + resolveEnvVarsInString, + resolveEnvVarsInObject, +} from './envVarResolver.js'; + +describe('resolveEnvVarsInString', () => { + let originalEnv: NodeJS.ProcessEnv; + + beforeEach(() => { + originalEnv = { ...process.env }; + }); + + afterEach(() => { + process.env = originalEnv; + }); + + it('should resolve $VAR_NAME format', () => { + process.env['TEST_VAR'] = 'test-value'; + + const result = resolveEnvVarsInString('Value is $TEST_VAR'); + + expect(result).toBe('Value is test-value'); + }); + + it('should resolve ${VAR_NAME} format', () => { + process.env['TEST_VAR'] = 'test-value'; + + const result = resolveEnvVarsInString('Value is ${TEST_VAR}'); + + expect(result).toBe('Value is test-value'); + }); + + it('should resolve multiple variables in the same string', () => { + process.env['HOST'] = 'localhost'; + process.env['PORT'] = '3000'; + + const result = resolveEnvVarsInString('URL: http://$HOST:${PORT}/api'); + + expect(result).toBe('URL: http://localhost:3000/api'); + }); + + it('should leave undefined variables unchanged', () => { + const result = resolveEnvVarsInString('Value is $UNDEFINED_VAR'); + + expect(result).toBe('Value is $UNDEFINED_VAR'); + }); + + it('should leave undefined variables with braces unchanged', () => { + const result = resolveEnvVarsInString('Value is ${UNDEFINED_VAR}'); + + expect(result).toBe('Value is ${UNDEFINED_VAR}'); + }); + + it('should handle empty string', () => { + const result = resolveEnvVarsInString(''); + + expect(result).toBe(''); + }); + + it('should handle string without variables', () => { + const result = resolveEnvVarsInString('No variables here'); + + expect(result).toBe('No variables here'); + }); + + it('should handle mixed defined and undefined variables', () => { + process.env['DEFINED'] = 'value'; + + const result = resolveEnvVarsInString('$DEFINED and $UNDEFINED mixed'); + + expect(result).toBe('value and $UNDEFINED mixed'); + }); +}); + +describe('resolveEnvVarsInObject', () => { + let originalEnv: NodeJS.ProcessEnv; + + beforeEach(() => { + originalEnv = { ...process.env }; + }); + + afterEach(() => { + process.env = originalEnv; + }); + + it('should resolve variables in nested objects', () => { + process.env['API_KEY'] = 'secret-123'; + process.env['DB_URL'] = 'postgresql://localhost/test'; + + const config = { + server: { + auth: { + key: '$API_KEY', + }, + database: '${DB_URL}', + }, + port: 3000, + }; + + const result = resolveEnvVarsInObject(config); + + expect(result).toEqual({ + server: { + auth: { + key: 'secret-123', + }, + database: 'postgresql://localhost/test', + }, + port: 3000, + }); + }); + + it('should resolve variables in arrays', () => { + process.env['ENV'] = 'production'; + process.env['VERSION'] = '1.0.0'; + + const config = { + tags: ['$ENV', 'app', '${VERSION}'], + metadata: { + env: '$ENV', + }, + }; + + const result = resolveEnvVarsInObject(config); + + expect(result).toEqual({ + tags: ['production', 'app', '1.0.0'], + metadata: { + env: 'production', + }, + }); + }); + + it('should preserve non-string types', () => { + const config = { + enabled: true, + count: 42, + value: null, + data: undefined, + tags: ['item1', 'item2'], + }; + + const result = resolveEnvVarsInObject(config); + + expect(result).toEqual(config); + }); + + it('should handle MCP server config structure', () => { + process.env['API_TOKEN'] = 'token-123'; + process.env['SERVER_PORT'] = '8080'; + + const extensionConfig = { + name: 'test-extension', + version: '1.0.0', + mcpServers: { + 'test-server': { + command: 'node', + args: ['server.js', '--port', '${SERVER_PORT}'], + env: { + API_KEY: '$API_TOKEN', + STATIC_VALUE: 'unchanged', + }, + timeout: 5000, + }, + }, + }; + + const result = resolveEnvVarsInObject(extensionConfig); + + expect(result).toEqual({ + name: 'test-extension', + version: '1.0.0', + mcpServers: { + 'test-server': { + command: 'node', + args: ['server.js', '--port', '8080'], + env: { + API_KEY: 'token-123', + STATIC_VALUE: 'unchanged', + }, + timeout: 5000, + }, + }, + }); + }); + + it('should handle empty and null values', () => { + const config = { + empty: '', + nullValue: null, + undefinedValue: undefined, + zero: 0, + false: false, + }; + + const result = resolveEnvVarsInObject(config); + + expect(result).toEqual(config); + }); + + it('should handle circular references in objects without infinite recursion', () => { + process.env['TEST_VAR'] = 'resolved-value'; + + type ConfigWithCircularRef = { + name: string; + value: number; + self?: ConfigWithCircularRef; + }; + + const config: ConfigWithCircularRef = { + name: '$TEST_VAR', + value: 42, + }; + // Create circular reference + config.self = config; + + const result = resolveEnvVarsInObject(config); + + expect(result.name).toBe('resolved-value'); + expect(result.value).toBe(42); + expect(result.self).toBeDefined(); + expect(result.self?.name).toBe('$TEST_VAR'); // Circular reference should be shallow copied + expect(result.self?.value).toBe(42); + // Verify it doesn't create infinite recursion by checking it's not the same object + expect(result.self).not.toBe(result); + }); + + it('should handle circular references in arrays without infinite recursion', () => { + process.env['ARRAY_VAR'] = 'array-value'; + + type ArrayWithCircularRef = Array; + const arr: ArrayWithCircularRef = ['$ARRAY_VAR', 123]; + // Create circular reference + arr.push(arr); + + const result = resolveEnvVarsInObject(arr) as ArrayWithCircularRef; + + expect(result[0]).toBe('array-value'); + expect(result[1]).toBe(123); + expect(Array.isArray(result[2])).toBe(true); + const subArray = result[2] as ArrayWithCircularRef; + expect(subArray[0]).toBe('$ARRAY_VAR'); // Circular reference should be shallow copied + expect(subArray[1]).toBe(123); + // Verify it doesn't create infinite recursion + expect(result[2]).not.toBe(result); + }); + + it('should handle complex nested circular references', () => { + process.env['NESTED_VAR'] = 'nested-resolved'; + + type ObjWithRef = { + name: string; + id: number; + ref?: ObjWithRef; + }; + + const obj1: ObjWithRef = { name: '$NESTED_VAR', id: 1 }; + const obj2: ObjWithRef = { name: 'static', id: 2 }; + + // Create cross-references + obj1.ref = obj2; + obj2.ref = obj1; + + const config = { + primary: obj1, + secondary: obj2, + value: '$NESTED_VAR', + }; + + const result = resolveEnvVarsInObject(config); + + expect(result.value).toBe('nested-resolved'); + expect(result.primary.name).toBe('nested-resolved'); + expect(result.primary.id).toBe(1); + expect(result.secondary.name).toBe('static'); + expect(result.secondary.id).toBe(2); + + // Check that circular references are handled (shallow copied) + expect(result.primary.ref).toBeDefined(); + expect(result.secondary.ref).toBeDefined(); + expect(result.primary.ref?.name).toBe('static'); // Should be shallow copy + expect(result.secondary.ref?.name).toBe('nested-resolved'); // The shallow copy still gets processed + + // Most importantly: verify no infinite recursion by checking objects are different + expect(result.primary.ref).not.toBe(result.secondary); + expect(result.secondary.ref).not.toBe(result.primary); + expect(result.primary).not.toBe(obj1); // New object created + expect(result.secondary).not.toBe(obj2); // New object created + }); +}); diff --git a/packages/cli/src/utils/envVarResolver.ts b/packages/cli/src/utils/envVarResolver.ts new file mode 100644 index 0000000000..d6d50d0206 --- /dev/null +++ b/packages/cli/src/utils/envVarResolver.ts @@ -0,0 +1,112 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Resolves environment variables in a string. + * Replaces $VAR_NAME and ${VAR_NAME} with their corresponding environment variable values. + * If the environment variable is not defined, the original placeholder is preserved. + * + * @param value - The string that may contain environment variable placeholders + * @returns The string with environment variables resolved + * + * @example + * resolveEnvVarsInString("Token: $API_KEY") // Returns "Token: secret-123" + * resolveEnvVarsInString("URL: ${BASE_URL}/api") // Returns "URL: https://api.example.com/api" + * resolveEnvVarsInString("Missing: $UNDEFINED_VAR") // Returns "Missing: $UNDEFINED_VAR" + */ +export function resolveEnvVarsInString(value: string): string { + const envVarRegex = /\$(?:(\w+)|{([^}]+)})/g; // Find $VAR_NAME or ${VAR_NAME} + return value.replace(envVarRegex, (match, varName1, varName2) => { + const varName = varName1 || varName2; + if (process && process.env && typeof process.env[varName] === 'string') { + return process.env[varName]!; + } + return match; + }); +} + +/** + * Recursively resolves environment variables in an object of any type. + * Handles strings, arrays, nested objects, and preserves other primitive types. + * Protected against circular references using a WeakSet to track visited objects. + * + * @param obj - The object to process for environment variable resolution + * @returns A new object with environment variables resolved + * + * @example + * const config = { + * server: { + * host: "$HOST", + * port: "${PORT}", + * enabled: true, + * tags: ["$ENV", "api"] + * } + * }; + * const resolved = resolveEnvVarsInObject(config); + */ +export function resolveEnvVarsInObject(obj: T): T { + return resolveEnvVarsInObjectInternal(obj, new WeakSet()); +} + +/** + * Internal implementation of resolveEnvVarsInObject with circular reference protection. + * + * @param obj - The object to process + * @param visited - WeakSet to track visited objects and prevent circular references + * @returns A new object with environment variables resolved + */ +function resolveEnvVarsInObjectInternal( + obj: T, + visited: WeakSet, +): T { + if ( + obj === null || + obj === undefined || + typeof obj === 'boolean' || + typeof obj === 'number' + ) { + return obj; + } + + if (typeof obj === 'string') { + return resolveEnvVarsInString(obj) as unknown as T; + } + + if (Array.isArray(obj)) { + // Check for circular reference + if (visited.has(obj)) { + // Return a shallow copy to break the cycle + return [...obj] as unknown as T; + } + + visited.add(obj); + const result = obj.map((item) => + resolveEnvVarsInObjectInternal(item, visited), + ) as unknown as T; + visited.delete(obj); + return result; + } + + if (typeof obj === 'object') { + // Check for circular reference + if (visited.has(obj as object)) { + // Return a shallow copy to break the cycle + return { ...obj } as T; + } + + visited.add(obj as object); + const newObj = { ...obj } as T; + for (const key in newObj) { + if (Object.prototype.hasOwnProperty.call(newObj, key)) { + newObj[key] = resolveEnvVarsInObjectInternal(newObj[key], visited); + } + } + visited.delete(obj as object); + return newObj; + } + + return obj; +}