feat(extension): resolve environment variables in extension configuration (#7213)

Co-authored-by: Tommaso Sciortino <sciortino@gmail.com>
Co-authored-by: Jacob Richman <jacob314@gmail.com>

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', () => {

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));

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<T>(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) {

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<string | number | ArrayWithCircularRef>;
+    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
+  });
+});

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<T>(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<T>(
+  obj: T,
+  visited: WeakSet<object>,
+): 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;
+}