From 01a7a66ca0d82acf2843f942713edb2551920f0f Mon Sep 17 00:00:00 2001 From: Trung Dang Date: Wed, 23 Apr 2025 14:13:04 +0700 Subject: [PATCH] feat: add `injectEnv` util, support env ref in mcp config (#2679) * feat: support environment variables reference in mcp `env` config * tests(src/utils/config): add test for `injectEnv` * fix(injectEnv): use `env == null` and `??` check instead of `!env`, `||` * refactor: remove unnecessary type declare * chore!: simplify regexp, remove replacement for env vars with dots --- src/services/mcp/McpHub.ts | 3 +- src/utils/__tests__/config.test.ts | 100 +++++++++++++++++++++++++++++ src/utils/config.ts | 25 ++++++++ 3 files changed, 127 insertions(+), 1 deletion(-) create mode 100644 src/utils/__tests__/config.test.ts create mode 100644 src/utils/config.ts diff --git a/src/services/mcp/McpHub.ts b/src/services/mcp/McpHub.ts index 31d0dd8020..59349ffdbc 100644 --- a/src/services/mcp/McpHub.ts +++ b/src/services/mcp/McpHub.ts @@ -30,6 +30,7 @@ import { } from "../../shared/mcp" import { fileExistsAtPath } from "../../utils/fs" import { arePathsEqual } from "../../utils/path" +import { injectEnv } from "../../utils/config" export type McpConnection = { server: McpServer @@ -452,7 +453,7 @@ export class McpHub { args: config.args, cwd: config.cwd, env: { - ...config.env, + ...(config.env ? await injectEnv(config.env) : {}), ...(process.env.PATH ? { PATH: process.env.PATH } : {}), }, stderr: "pipe", diff --git a/src/utils/__tests__/config.test.ts b/src/utils/__tests__/config.test.ts new file mode 100644 index 0000000000..c5e9e2ba88 --- /dev/null +++ b/src/utils/__tests__/config.test.ts @@ -0,0 +1,100 @@ +import { injectEnv } from "../config" + +describe("injectEnv", () => { + const originalEnv = process.env + + beforeEach(() => { + // Assign a new / reset process.env before each test + jest.resetModules() + process.env = { ...originalEnv } + }) + + afterAll(() => { + // Restore original process.env after all tests + process.env = originalEnv + }) + + it("should replace env variables in a string", async () => { + process.env.TEST_VAR = "testValue" + const configString = "Hello ${env:TEST_VAR}" + const expectedString = "Hello testValue" + const result = await injectEnv(configString) + expect(result).toBe(expectedString) + }) + + it("should replace env variables in an object", async () => { + process.env.API_KEY = "12345" + process.env.ENDPOINT = "https://example.com" + const configObject = { + key: "${env:API_KEY}", + url: "${env:ENDPOINT}", + nested: { + value: "Keep this ${env:API_KEY}", + }, + } + const expectedObject = { + key: "12345", + url: "https://example.com", + nested: { + value: "Keep this 12345", + }, + } + const result = await injectEnv(configObject) + expect(result).toEqual(expectedObject) + }) + + it("should use notFoundValue for missing env variables", async () => { + const consoleWarnSpy = jest.spyOn(console, "warn").mockImplementation() + process.env.EXISTING_VAR = "exists" + const configString = "Value: ${env:EXISTING_VAR}, Missing: ${env:MISSING_VAR}" + const expectedString = "Value: exists, Missing: NOT_FOUND" + const result = await injectEnv(configString, "NOT_FOUND") + expect(result).toBe(expectedString) + expect(consoleWarnSpy).toHaveBeenCalledWith( + "[injectEnv] env variable MISSING_VAR referenced but not found in process.env", + ) + consoleWarnSpy.mockRestore() + }) + + it("should use default empty string for missing env variables if notFoundValue is not provided", async () => { + const consoleWarnSpy = jest.spyOn(console, "warn").mockImplementation() + const configString = "Missing: ${env:ANOTHER_MISSING}" + const expectedString = "Missing: " + const result = await injectEnv(configString) + expect(result).toBe(expectedString) + expect(consoleWarnSpy).toHaveBeenCalledWith( + "[injectEnv] env variable ANOTHER_MISSING referenced but not found in process.env", + ) + consoleWarnSpy.mockRestore() + }) + + it("should handle strings without env variables", async () => { + const configString = "Just a regular string" + const result = await injectEnv(configString) + expect(result).toBe(configString) + }) + + it("should handle objects without env variables", async () => { + const configObject = { key: "value", number: 123 } + const result = await injectEnv(configObject) + expect(result).toEqual(configObject) + }) + + it("should not mutate the original object", async () => { + process.env.MUTATE_TEST = "mutated" + const originalObject = { value: "${env:MUTATE_TEST}" } + const copyOfOriginal = { ...originalObject } // Shallow copy for comparison + await injectEnv(originalObject) + expect(originalObject).toEqual(copyOfOriginal) // Check if the original object remains unchanged + }) + + it("should handle empty string input", async () => { + const result = await injectEnv("") + expect(result).toBe("") + }) + + it("should handle empty object input", async () => { + const result = await injectEnv({}) + expect(result).toEqual({}) + }) +}) diff --git a/src/utils/config.ts b/src/utils/config.ts new file mode 100644 index 0000000000..24bf9cbdec --- /dev/null +++ b/src/utils/config.ts @@ -0,0 +1,25 @@ +/** + * Deeply injects environment variables into a configuration object/string/json + * + * Uses VSCode env:name pattern: https://code.visualstudio.com/docs/reference/variables-reference#_environment-variables + * + * Does not mutate original object + */ +export async function injectEnv(config: string | Record, notFoundValue: any = "") { + // Use simple regex replace for now, will see if object traversal and recursion is needed here (e.g: for non-serializable objects) + + const isObject = typeof config === "object" + let _config = isObject ? JSON.stringify(config) : config + + _config = _config.replace(/\$\{env:([\w]+)\}/g, (_, name) => { + // Check if null or undefined + // intentionally using == to match null | undefined + // eslint-disable-next-line eqeqeq + if (process.env[name] == null) + console.warn(`[injectEnv] env variable ${name} referenced but not found in process.env`) + + return process.env[name] ?? notFoundValue + }) + + return isObject ? JSON.parse(_config) : _config +}