diff --git a/src/features/skill-mcp-manager/env-cleaner.test.ts b/src/features/skill-mcp-manager/env-cleaner.test.ts index 75cfe348e..0a0b4f6df 100644 --- a/src/features/skill-mcp-manager/env-cleaner.test.ts +++ b/src/features/skill-mcp-manager/env-cleaner.test.ts @@ -112,8 +112,8 @@ describe("createCleanMcpEnvironment", () => { process.env.PATH = "/usr/bin" process.env.NPM_CONFIG_REGISTRY = "https://private.registry.com" const customEnv = { - MCP_API_KEY: "secret-key", - CUSTOM_VAR: "custom-value", + SAFE_CUSTOM_VAR: "custom-value", + ANOTHER_SAFE_VAR: "another-value", } // when @@ -122,8 +122,8 @@ describe("createCleanMcpEnvironment", () => { // then expect(cleanEnv.PATH).toBe("/usr/bin") expect(cleanEnv.NPM_CONFIG_REGISTRY).toBeUndefined() - expect(cleanEnv.MCP_API_KEY).toBe("secret-key") - expect(cleanEnv.CUSTOM_VAR).toBe("custom-value") + expect(cleanEnv.SAFE_CUSTOM_VAR).toBe("custom-value") + expect(cleanEnv.ANOTHER_SAFE_VAR).toBe("another-value") }) it("custom env can override process.env values", () => { @@ -139,6 +139,25 @@ describe("createCleanMcpEnvironment", () => { // then expect(cleanEnv.NODE_ENV).toBe("production") }) + + it("filters secret keys from customEnv that would bypass process.env filtering", () => { + // given - customEnv tries to inject secrets that should be filtered + process.env.PATH = "/usr/bin" + const customEnv = { + MCP_API_KEY: "secret-key-that-should-be-filtered", + CUSTOM_SECRET: "another-secret", + SAFE_VAR: "safe-value", + } + + // when + const cleanEnv = createCleanMcpEnvironment(customEnv) + + // then - secret keys from customEnv are filtered despite not being in process.env + expect(cleanEnv.MCP_API_KEY).toBeUndefined() + expect(cleanEnv.CUSTOM_SECRET).toBeUndefined() + expect(cleanEnv.SAFE_VAR).toBe("safe-value") + expect(cleanEnv.PATH).toBe("/usr/bin") + }) }) describe("undefined value handling", () => { diff --git a/src/features/skill-mcp-manager/env-cleaner.ts b/src/features/skill-mcp-manager/env-cleaner.ts index 9c6ebe1aa..b5281c88a 100644 --- a/src/features/skill-mcp-manager/env-cleaner.ts +++ b/src/features/skill-mcp-manager/env-cleaner.ts @@ -28,18 +28,22 @@ export const EXCLUDED_ENV_PATTERNS: RegExp[] = [ export function createCleanMcpEnvironment( customEnv: Record = {} ): Record { - const cleanEnv: Record = {} + const mergedEnv: Record = {} for (const [key, value] of Object.entries(process.env)) { if (value === undefined) continue + mergedEnv[key] = value + } + Object.assign(mergedEnv, customEnv) + + const cleanEnv: Record = {} + for (const [key, value] of Object.entries(mergedEnv)) { const shouldExclude = EXCLUDED_ENV_PATTERNS.some((pattern) => pattern.test(key)) if (!shouldExclude) { cleanEnv[key] = value } } - Object.assign(cleanEnv, customEnv) - return cleanEnv }