diff --git a/packages/adapters/src/mcp-connector.test.ts b/packages/adapters/src/mcp-connector.test.ts index 60eaab04b..8e0fd43b2 100644 --- a/packages/adapters/src/mcp-connector.test.ts +++ b/packages/adapters/src/mcp-connector.test.ts @@ -23,10 +23,14 @@ const ASSIGNMENT = { server: SERVER, }; -function mcpFetch(state: { failNext: boolean; initializations: number }) { +function mcpFetch( + state: { failNext: boolean; initializations: number; headers?: Record[] }, + expectedUrl = "https://mcp.example.test/mcp", +) { return vi.fn(async (input: string | URL | Request, init?: RequestInit) => { const request = input instanceof Request ? input : new Request(input, init); - if (new URL(request.url).href !== "https://mcp.example.test/mcp") + state.headers?.push(Object.fromEntries(request.headers.entries())); + if (new URL(request.url).href !== expectedUrl) throw new Error(`Unexpected request: ${request.url}`); if (request.method !== "POST") return new Response(null, { status: 405 }); if (state.failNext) return new Response("boom", { status: 500 }); @@ -62,6 +66,63 @@ function mcpFetch(state: { failNext: boolean; initializations: number }) { } describe("MCP connector session cache", () => { + it("connects to an explicitly configured localhost HTTP server", async () => { + const state = { failNext: false, initializations: 0 }; + const localAssignment = { + ...ASSIGNMENT, + server: { ...SERVER, endpoint: "http://localhost:8123/api/mcp" }, + }; + vi.stubGlobal("fetch", mcpFetch(state, "http://localhost:8123/api/mcp")); + const prisma = { + botMcpServer: { findMany: vi.fn().mockResolvedValue([localAssignment]) }, + }; + const connector = new McpConnector(prisma as never, {} as never); + + const tools = await connector.discoverTools({ + workspaceId: "w1", + userId: "u1", + botId: "bot-1", + signal: new AbortController().signal, + } as never); + + expect(tools.map((tool) => tool.name)).toEqual(["mcp__demo__echo"]); + await connector.close(); + }); + + it("does not send stored credentials to a localhost HTTP server", async () => { + const state = { failNext: false, initializations: 0, headers: [] as Record[] }; + const localAssignment = { + ...ASSIGNMENT, + server: { ...SERVER, endpoint: "http://localhost:8123/api/mcp", secretId: "secret-1" }, + }; + vi.stubGlobal("fetch", mcpFetch(state, "http://localhost:8123/api/mcp")); + const prisma = { + botMcpServer: { findMany: vi.fn().mockResolvedValue([localAssignment]) }, + secret: { findFirst: vi.fn().mockResolvedValue({ id: "secret-1", ciphertext: "encrypted" }) }, + }; + const connector = new McpConnector( + prisma as never, + { + load: vi + .fn() + .mockReturnValue( + JSON.stringify({ secret: "local-token", headers: { "X-Api-Key": "local-key" } }), + ), + } as never, + ); + + await connector.discoverTools({ + workspaceId: "w1", + userId: "u1", + botId: "bot-1", + signal: new AbortController().signal, + } as never); + + expect(state.headers[0]?.authorization).toBeUndefined(); + expect(state.headers[0]?.["x-api-key"]).toBeUndefined(); + await connector.close(); + }); + it("evicts a session after a failed call so the next call reconnects instead of reusing a dead session", async () => { const state = { failNext: false, initializations: 0 }; vi.stubGlobal("fetch", mcpFetch(state)); diff --git a/packages/adapters/src/mcp-connector.ts b/packages/adapters/src/mcp-connector.ts index c7f0d24f7..470764f3c 100644 --- a/packages/adapters/src/mcp-connector.ts +++ b/packages/adapters/src/mcp-connector.ts @@ -5,6 +5,7 @@ import type { ConnectorProvider, ConnectorTool, } from "@rakazo/adapter-kit"; +import { isLocalMcpHost } from "@rakazo/contracts"; import type { McpServer, PrismaClient } from "@rakazo/db"; import type { McpOAuthBroker, OAuthMaterial } from "./mcp-oauth.js"; import { McpSession } from "./mcp-transport.js"; @@ -193,9 +194,12 @@ export class McpConnector implements ConnectorProvider { }); } else { if (!server.endpoint) throw new Error("MCP endpoint is required"); - const authProvider = this.oauth - ? await this.oauth.providerFor(server, context, loaded) - : undefined; + const endpoint = new URL(server.endpoint); + const localHttp = endpoint.protocol === "http:" && isLocalMcpHost(endpoint.hostname); + const authProvider = + !localHttp && this.oauth + ? await this.oauth.providerFor(server, context, loaded) + : undefined; const staticToken = material.secret ? material.secret.startsWith("Bearer ") ? material.secret @@ -207,9 +211,10 @@ export class McpConnector implements ConnectorProvider { }; await session.connectRemote({ url: server.endpoint, + urlPolicy: { allowHttpLocalhost: true }, transport: server.transport === "sse" ? "sse" : "streamable-http", allowLegacySse: server.transport === "sse", - headerPolicy: { headers }, + headerPolicy: localHttp ? undefined : { headers }, fallbackToSse: false, authProvider, network: this.options.network, diff --git a/packages/adapters/src/mcp-transport.test.ts b/packages/adapters/src/mcp-transport.test.ts index 39bbb4d8b..dea721627 100644 --- a/packages/adapters/src/mcp-transport.test.ts +++ b/packages/adapters/src/mcp-transport.test.ts @@ -3,7 +3,12 @@ import { createServer } from "node:http"; import type { AddressInfo } from "node:net"; import { afterEach, describe, expect, it, vi } from "vitest"; import { StoredMcpOAuthProvider } from "./mcp-oauth.js"; -import { McpSession, validateUrl, withEndpointOriginFallback } from "./mcp-transport.js"; +import { + McpSession, + secureFetch, + validateUrl, + withEndpointOriginFallback, +} from "./mcp-transport.js"; afterEach(() => vi.unstubAllGlobals()); @@ -299,6 +304,64 @@ describe("MCP transport seam", () => { await expect(direct.json()).resolves.toEqual({ authorization: "Bearer secret" }); }); + it("strips configured credentials from localhost HTTP requests", async () => { + let seen: Record = {}; + const safeFetch = secureFetch( + new URL("http://localhost:8123/mcp"), + { allowHttpLocalhost: true }, + { + allowedHeaders: ["authorization", "x-api-key"], + headers: { Authorization: "Bearer stored", "X-Api-Key": "stored-key" }, + }, + { + fetch: vi.fn(async (input: string | URL | Request, init?: RequestInit) => { + const request = input instanceof Request ? input : new Request(input, init); + seen = Object.fromEntries(request.headers.entries()); + return Response.json({ ok: true }); + }), + }, + ); + + await safeFetch("http://localhost:8123/mcp", { + method: "POST", + headers: { + Authorization: "Bearer sdk", + "X-Api-Key": "sdk-key", + "Content-Type": "application/json", + }, + body: "{}", + }); + + expect(seen.authorization).toBeUndefined(); + expect(seen["x-api-key"]).toBeUndefined(); + expect(seen["content-type"]).toBe("application/json"); + }); + + it("keeps configured credentials for HTTPS requests", async () => { + let seen: Record = {}; + const safeFetch = secureFetch( + new URL("https://mcp.example.test/mcp"), + {}, + { + allowedHeaders: ["authorization", "x-api-key"], + headers: { Authorization: "Bearer stored", "X-Api-Key": "stored-key" }, + }, + { + resolveHostname: TEST_NETWORK.resolveHostname, + fetch: vi.fn(async (input: string | URL | Request, init?: RequestInit) => { + const request = input instanceof Request ? input : new Request(input, init); + seen = Object.fromEntries(request.headers.entries()); + return Response.json({ ok: true }); + }), + }, + ); + + await safeFetch("https://mcp.example.test/mcp", { method: "POST", body: "{}" }); + + expect(seen.authorization).toBe("Bearer stored"); + expect(seen["x-api-key"]).toBe("stored-key"); + }); + it("never retries a failed write against the endpoint origin", async () => { const inner = vi.fn(async () => { throw new TypeError("fetch failed"); diff --git a/packages/adapters/src/mcp-transport.ts b/packages/adapters/src/mcp-transport.ts index 12973f758..5eb6ab69b 100644 --- a/packages/adapters/src/mcp-transport.ts +++ b/packages/adapters/src/mcp-transport.ts @@ -8,6 +8,7 @@ import { import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js"; import type { Transport } from "@modelcontextprotocol/sdk/shared/transport.js"; import type { CallToolResult, ListToolsResult } from "@modelcontextprotocol/sdk/types.js"; +import { isLocalMcpHost } from "@rakazo/contracts"; import { combineSignals } from "./connector-safety.js"; import { createSafeRemoteFetch, @@ -75,8 +76,7 @@ function validateUrl(raw: string | URL, policy: McpUrlPolicy = {}): URL { if (url.toString().length > max) throw new Error(`MCP URL exceeds ${max} characters`); if (url.username || url.password || url.hash) throw new Error("MCP URL must not contain credentials or a fragment"); - const local = - url.hostname === "localhost" || url.hostname === "127.0.0.1" || url.hostname === "[::1]"; + const local = isLocalMcpHost(url.hostname); if ( url.protocol !== "https:" && !(url.protocol === "http:" && policy.allowHttpLocalhost === true && local) @@ -101,6 +101,15 @@ export function secureFetch( const configured = Object.entries(headerPolicy.headers ?? {}).filter(([name]) => allowed.has(name.toLowerCase()), ); + const configuredNames = new Set( + Object.keys(headerPolicy.headers ?? {}).map((name) => name.toLowerCase()), + ); + const localCredentialHeaders = new Set([ + ...configuredNames, + "authorization", + "cookie", + "proxy-authorization", + ]); const safeRemoteFetch = createSafeRemoteFetch( network.fetch ?? globalThis.fetch, network.resolveHostname, @@ -109,10 +118,17 @@ export function secureFetch( const source = input instanceof Request ? input : new Request(input, init); const url = validateUrl(source.url, urlPolicy); const headers = new Headers(source.headers); - if (url.origin === resourceUrl.origin) { + const localHttp = url.protocol === "http:" && isLocalMcpHost(url.hostname); + if (localHttp) { + for (const name of [...headers.keys()]) { + if (localCredentialHeaders.has(name.toLowerCase())) headers.delete(name); + } + } + if (!localHttp && url.origin === resourceUrl.origin) { for (const [name, value] of configured) headers.set(name, value); } for (const [name, value] of new Headers(init?.headers)) { + if (localHttp && localCredentialHeaders.has(name.toLowerCase())) continue; if (allowed.has(name.toLowerCase())) headers.set(name, value); } // Buffer the body: a re-wrapped Request body is a stream without a replayable @@ -127,7 +143,6 @@ export function secureFetch( redirect: "manual", signal: source.signal, } satisfies RequestInit; - const localHttp = url.protocol === "http:"; const response = localHttp ? await (network.fetch ?? globalThis.fetch)(url, requestInit) : await safeRemoteFetch(url, requestInit); diff --git a/packages/contracts/src/index.test.ts b/packages/contracts/src/index.test.ts index 19be58776..b529e5f5a 100644 --- a/packages/contracts/src/index.test.ts +++ b/packages/contracts/src/index.test.ts @@ -170,7 +170,7 @@ describe("contracts", () => { ).toBe(false); }); - it("rejects non-HTTPS MCP endpoints before storage", () => { + it("allows localhost HTTP MCP endpoints and rejects other non-HTTPS URLs before storage", () => { const base = { slug: "demo", name: "Demo", @@ -179,6 +179,17 @@ describe("contracts", () => { }; expect( McpServerConfigInput.safeParse({ ...base, endpoint: "http://127.0.0.1:3000/mcp" }).success, + ).toBe(true); + expect( + McpServerConfigInput.safeParse({ ...base, endpoint: "http://localhost:8123/api/mcp" }) + .success, + ).toBe(true); + expect( + McpServerConfigInput.safeParse({ ...base, endpoint: "http://localhost:8123/api/mcp#" }) + .success, + ).toBe(false); + expect( + McpServerConfigInput.safeParse({ ...base, endpoint: "http://example.test/mcp" }).success, ).toBe(false); expect( McpServerConfigInput.safeParse({ ...base, endpoint: "https://mcp.example.test/mcp" }).success, diff --git a/packages/contracts/src/mcp.ts b/packages/contracts/src/mcp.ts index 24daf754d..d7c99c900 100644 --- a/packages/contracts/src/mcp.ts +++ b/packages/contracts/src/mcp.ts @@ -3,18 +3,30 @@ import * as z from "zod"; export const McpTransportSchema = z.enum(["streamable_http", "sse", "stdio"]); export type McpTransport = z.infer; +export function isLocalMcpHost(hostname: string): boolean { + return ( + hostname === "localhost" || + hostname === "127.0.0.1" || + hostname === "[::1]" || + hostname === "::1" + ); +} + export const McpRemoteEndpointSchema = z .string() .max(2048) .url() .refine((value) => { try { + if (value.endsWith("#")) return false; const url = new URL(value); - return url.protocol === "https:" && !url.username && !url.password && !url.hash; + if (url.username || url.password || url.hash) return false; + if (url.protocol === "https:") return true; + return url.protocol === "http:" && isLocalMcpHost(url.hostname); } catch { return false; } - }, "MCP remote endpoint must be an HTTPS URL without credentials or a fragment"); + }, "MCP remote endpoint must be an HTTPS URL without credentials or a fragment (HTTP is allowed only for localhost)"); export const McpHeadersSchema = z .record(z.string().regex(/^[A-Za-z0-9-]+$/), z.string().max(4096))