Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 63 additions & 2 deletions packages/adapters/src/mcp-connector.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,14 @@ const ASSIGNMENT = {
server: SERVER,
};

function mcpFetch(state: { failNext: boolean; initializations: number }) {
function mcpFetch(
state: { failNext: boolean; initializations: number; headers?: Record<string, string>[] },
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 });
Expand Down Expand Up @@ -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<string, string>[] };
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));
Expand Down
13 changes: 9 additions & 4 deletions packages/adapters/src/mcp-connector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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;
Comment thread
luinbytes marked this conversation as resolved.
const staticToken = material.secret
? material.secret.startsWith("Bearer ")
? material.secret
Expand All @@ -207,9 +211,10 @@ export class McpConnector implements ConnectorProvider {
};
await session.connectRemote({
url: server.endpoint,
urlPolicy: { allowHttpLocalhost: true },
Comment thread
luinbytes marked this conversation as resolved.
transport: server.transport === "sse" ? "sse" : "streamable-http",
allowLegacySse: server.transport === "sse",
headerPolicy: { headers },
headerPolicy: localHttp ? undefined : { headers },
fallbackToSse: false,
authProvider,
network: this.options.network,
Expand Down
65 changes: 64 additions & 1 deletion packages/adapters/src/mcp-transport.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand Down Expand Up @@ -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<string, string> = {};
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<string, string> = {};
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");
Expand Down
23 changes: 19 additions & 4 deletions packages/adapters/src/mcp-transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand All @@ -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,
Expand All @@ -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
Expand All @@ -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);
Expand Down
13 changes: 12 additions & 1 deletion packages/contracts/src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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,
Expand Down
16 changes: 14 additions & 2 deletions packages/contracts/src/mcp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,30 @@ import * as z from "zod";
export const McpTransportSchema = z.enum(["streamable_http", "sse", "stdio"]);
export type McpTransport = z.infer<typeof McpTransportSchema>;

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);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
} 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))
Expand Down
Loading