From df5cbb715bacf9d239467676fdd2fa73639669df Mon Sep 17 00:00:00 2001 From: Alec McLeod Date: Mon, 8 Jun 2026 14:41:44 -0300 Subject: [PATCH] Drop platform_overrides from resolved config; honour empty-string overrides MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getMcpConfigForManifest spreads the base mcp_config (including platform_overrides) and never removes it, so the full platform_overrides map — other platforms' commands/env included — leaked into the resolved runtime config handed to the host. Delete it after resolving. Also switch the command/args override merge from `||` to explicit `!== undefined` checks so a deliberate empty-string command (or empty args) override is honoured instead of silently falling back to the base value. Fixes #265 (empty-string override is item 1 of #267) --- src/shared/config.ts | 13 +++++++-- test/config.test.ts | 64 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 2 deletions(-) diff --git a/src/shared/config.ts b/src/shared/config.ts index 7073f58..cdc9ea7 100644 --- a/src/shared/config.ts +++ b/src/shared/config.ts @@ -115,12 +115,21 @@ export async function getMcpConfigForManifest( if (process.platform in baseConfig.platform_overrides) { const platformConfig = baseConfig.platform_overrides[process.platform]; - result.command = platformConfig.command || result.command; - result.args = platformConfig.args || result.args; + // Use explicit presence checks so a deliberate empty-string command (or + // empty args) override is honoured rather than falling back to the base. + result.command = + platformConfig.command !== undefined + ? platformConfig.command + : result.command; + result.args = + platformConfig.args !== undefined ? platformConfig.args : result.args; result.env = platformConfig.env ? { ...result.env, ...platformConfig.env } : result.env; } + // platform_overrides is a build-time directive; it must not leak into the + // resolved runtime mcp_config handed to the host. + delete result.platform_overrides; } // Check if required configuration is missing diff --git a/test/config.test.ts b/test/config.test.ts index 7e29f69..f0cf755 100644 --- a/test/config.test.ts +++ b/test/config.test.ts @@ -200,6 +200,70 @@ describe("getMcpConfigForManifest", () => { expect(result?.command).toBe("platform-command"); expect(result?.args).toEqual(["platform-specific"]); + // platform_overrides must not leak into the resolved runtime config. + expect(result?.platform_overrides).toBeUndefined(); + }); + + it("should not leak platform_overrides into the resolved config", async () => { + const manifest: McpbManifestAny = { + ...baseManifest, + server: { + type: "node", + entry_point: "server.js", + mcp_config: { + command: "default-command", + args: ["default"], + platform_overrides: { + // An override for a platform that is NOT the current one. + [process.platform === "win32" ? "darwin" : "win32"]: { + command: "other-platform-command", + }, + }, + }, + }, + }; + + const result = await getMcpConfigForManifest({ + manifest, + extensionPath: "/ext/path", + systemDirs: mockSystemDirs, + userConfig: {}, + pathSeparator: "/", + logger: mockLogger, + }); + + expect(result?.command).toBe("default-command"); + expect(result?.platform_overrides).toBeUndefined(); + }); + + it("should honour an empty-string command platform override", async () => { + const manifest: McpbManifestAny = { + ...baseManifest, + server: { + type: "node", + entry_point: "server.js", + mcp_config: { + command: "default-command", + args: ["default"], + platform_overrides: { + [process.platform]: { + command: "", + }, + }, + }, + }, + }; + + const result = await getMcpConfigForManifest({ + manifest, + extensionPath: "/ext/path", + systemDirs: mockSystemDirs, + userConfig: {}, + pathSeparator: "/", + logger: mockLogger, + }); + + expect(result?.command).toBe(""); }); it("should handle user config variable replacement with defaults", async () => {