From 15557fb885bd71b6de11f7654d5f68ae87501397 Mon Sep 17 00:00:00 2001 From: HwangJohn Date: Mon, 27 Jul 2026 01:25:22 +0900 Subject: [PATCH] fix: reject malformed mcp header templates --- src/connectors/mcp-client.ts | 36 ++++++++- test/mcp-client.test.ts | 139 ++++++++++++++++++++++++++++++++++- 2 files changed, 169 insertions(+), 6 deletions(-) diff --git a/src/connectors/mcp-client.ts b/src/connectors/mcp-client.ts index ef5b9c62..92fd9db1 100644 --- a/src/connectors/mcp-client.ts +++ b/src/connectors/mcp-client.ts @@ -810,16 +810,44 @@ async function resolveTemplateEnvReferences( return value; } - let resolvedValue = value; - const envRefs = value.matchAll(/\$\{([A-Z_][A-Z0-9_]*)\}/gu); + let resolvedValue = ""; + let cursor = 0; + let resolvedAnyReference = false; + const envRefs = value.matchAll(/\$\{([^}]*)\}/gu); for (const match of envRefs) { + resolvedAnyReference = true; + const matchIndex = match.index; + const literalValue = value.slice(cursor, matchIndex); + validateHeaderTemplateLiteral(literalValue, key); + const envKey = match[1]; const envValue = await resolveHeaderEnvReference(envKey); - resolvedValue = resolvedValue.replace(match[0], envValue); + resolvedValue += literalValue + envValue; + cursor = matchIndex + match[0].length; + } + + const remainingValue = value.slice(cursor); + validateHeaderTemplateLiteral(remainingValue, key); + + if (!resolvedAnyReference) { + throw new Error(`Header ${key} contains a malformed environment template.`); } - return resolvedValue; + const finalValue = resolvedValue + remainingValue; + if (finalValue.includes("${") || finalValue.includes("}")) { + throw new Error( + `Header ${key} resolved to a value with unresolved template fragments.`, + ); + } + + return finalValue; +} + +function validateHeaderTemplateLiteral(value: string, key: string): void { + if (value.includes("${") || value.includes("}")) { + throw new Error(`Header ${key} contains a malformed environment template.`); + } } async function resolveHeaderEnvReference(envKey: string): Promise { diff --git a/test/mcp-client.test.ts b/test/mcp-client.test.ts index 44cc55e5..958e1622 100644 --- a/test/mcp-client.test.ts +++ b/test/mcp-client.test.ts @@ -1,5 +1,51 @@ -import { afterEach, beforeEach, describe, expect, test } from "vitest"; -import { buildChildEnv } from "../src/connectors/mcp-client.ts"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; +import { buildChildEnv, listMcpTools } from "../src/connectors/mcp-client.ts"; + +function stubHttpMcpFetch(): ReturnType { + const fetchMock = vi.fn( + ( + _input: Parameters[0], + init?: Parameters[1], + ): Promise => { + if (typeof init?.body !== "string") { + return Promise.reject(new Error("Expected MCP request body.")); + } + + const body = JSON.parse(init.body) as { + id?: number; + method?: string; + }; + + if (body.method === "notifications/initialized") { + return Promise.resolve(new Response("", { status: 202 })); + } + + return Promise.resolve( + new Response( + JSON.stringify({ + id: body.id, + jsonrpc: "2.0", + result: body.method === "tools/list" ? { tools: [] } : {}, + }), + { headers: { "content-type": "application/json" } }, + ), + ); + }, + ); + vi.stubGlobal("fetch", fetchMock); + + return fetchMock; +} + +function mcpConfigWithHeaders(headers: Record) { + return { + transport: { + headers, + type: "http" as const, + url: "https://mcp.example.test/rpc", + }, + }; +} describe("buildChildEnv", () => { const SECRET_KEYS = [ @@ -68,3 +114,92 @@ describe("buildChildEnv", () => { ); }); }); + +describe("HTTP MCP header env templates", () => { + const HEADER_ENV_KEYS = ["MCP_HEADER_TOKEN", "MCP_HEADER_MISSING"] as const; + const saved: Record = {}; + + beforeEach(() => { + for (const key of HEADER_ENV_KEYS) { + saved[key] = process.env[key]; + } + process.env.MCP_HEADER_TOKEN = "declared-header-token"; + delete process.env.MCP_HEADER_MISSING; + }); + + afterEach(() => { + for (const [key, value] of Object.entries(saved)) { + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } + vi.unstubAllGlobals(); + }); + + test("resolves valid env template headers before HTTP requests", async () => { + const fetchMock = stubHttpMcpFetch(); + + await listMcpTools( + mcpConfigWithHeaders({ + Authorization: "Bearer ${MCP_HEADER_TOKEN}", + "X-Trace": "trace-${MCP_HEADER_TOKEN}-done", + }), + ); + + const [, init] = fetchMock.mock.calls[0] as [ + string, + { headers: Record }, + ]; + expect(init.headers.Authorization).toBe("Bearer declared-header-token"); + expect(init.headers["X-Trace"]).toBe("trace-declared-header-token-done"); + }); + + test("still rejects literal secret-like header values", async () => { + await expect( + listMcpTools( + mcpConfigWithHeaders({ + Authorization: "Bearer literal-token", + }), + ), + ).rejects.toThrow( + /Header Authorization must reference credentials with \$\{ENV_VAR\}/u, + ); + }); + + test.each([ + ["missing closing brace", "Bearer ${MCP_HEADER_TOKEN", /malformed/u], + [ + "lowercase env reference", + "Bearer ${mcp_header_token}", + /Invalid env var reference/u, + ], + ["extra closing brace", "Bearer ${MCP_HEADER_TOKEN}}", /malformed/u], + [ + "missing env value", + "Bearer ${MCP_HEADER_MISSING}", + /MCP_HEADER_MISSING is required/u, + ], + ])("rejects %s", async (_caseName, headerValue, errorPattern) => { + await expect( + listMcpTools( + mcpConfigWithHeaders({ + Authorization: headerValue, + }), + ), + ).rejects.toThrow(errorPattern); + }); + + test("rejects template fragments remaining after resolution", async () => { + process.env.MCP_HEADER_TOKEN = "still-${UNRESOLVED}"; + + await expect( + listMcpTools( + mcpConfigWithHeaders({ + Authorization: "Bearer ${MCP_HEADER_TOKEN}", + }), + ), + ).rejects.toThrow(/unresolved template fragments/u); + }); +});