diff --git a/client/src/__tests__/proxyFetchEndpoint.test.ts b/client/src/__tests__/proxyFetchEndpoint.test.ts index 14b6ec8e..4be75f6c 100644 --- a/client/src/__tests__/proxyFetchEndpoint.test.ts +++ b/client/src/__tests__/proxyFetchEndpoint.test.ts @@ -3,13 +3,21 @@ * Spawns the server and hits it like any other HTTP client would. */ import { spawn, type ChildProcess } from "child_process"; -import { createServer, type Server } from "http"; +import { + createServer, + type IncomingMessage, + type Server, + type ServerResponse, +} from "http"; import { resolve } from "path"; const TEST_PORT = 16321; const TEST_TOKEN = "test-proxy-token-12345"; const SERVER_PATH = resolve(__dirname, "../../../server/build/index.js"); +/** Placeholder URL for tests where auth fails before the proxy fetches (no network). */ +const UNUSED_UPSTREAM_URL = "http://127.0.0.1:1/unused"; + async function waitForServer(baseUrl: string, maxWaitMs = 5000): Promise { const start = Date.now(); while (Date.now() - start < maxWaitMs) { @@ -23,6 +31,36 @@ async function waitForServer(baseUrl: string, maxWaitMs = 5000): Promise { throw new Error("Server did not become ready"); } +/** + * Runs `fn` with a local HTTP server on 127.0.0.1:ephemeral-port. + * `origin` is `http://127.0.0.1:` (no trailing path). + */ +async function withLocalUpstream( + onRequest: (req: IncomingMessage, res: ServerResponse) => void, + fn: (origin: string) => Promise, +): Promise { + const upstream: Server = createServer(onRequest); + + await new Promise((resolve, reject) => { + upstream.once("error", reject); + upstream.listen(0, "127.0.0.1", () => resolve()); + }); + + const addr = upstream.address(); + if (!addr || typeof addr === "string") { + upstream.close(); + throw new Error("Expected TCP listen address"); + } + + const origin = `http://127.0.0.1:${addr.port}`; + + try { + await fn(origin); + } finally { + await new Promise((r) => upstream.close(() => r())); + } +} + describe("POST /fetch endpoint", () => { let server: ChildProcess; const baseUrl = `http://localhost:${TEST_PORT}`; @@ -48,7 +86,7 @@ describe("POST /fetch endpoint", () => { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ - url: "http://example.com/", + url: UNUSED_UPSTREAM_URL, init: { method: "GET" }, }), }); @@ -65,14 +103,14 @@ describe("POST /fetch endpoint", () => { "X-MCP-Proxy-Auth": "Bearer wrong-token", }, body: JSON.stringify({ - url: "http://example.com/", + url: UNUSED_UPSTREAM_URL, init: { method: "GET" }, }), }); expect(res.status).toBe(401); }); - it("forwards request when auth token is valid", async () => { + it("returns 400 for non-http(s) URL when auth token is valid", async () => { const res = await fetch(`${baseUrl}/fetch`, { method: "POST", headers: { @@ -80,59 +118,117 @@ describe("POST /fetch endpoint", () => { "X-MCP-Proxy-Auth": `Bearer ${TEST_TOKEN}`, }, body: JSON.stringify({ - url: "http://example.com/", + url: "file:///etc/passwd", init: { method: "GET" }, }), }); - expect(res.status).toBe(200); - const body = await res.json(); - expect(body.ok).toBe(true); - expect(body.status).toBe(200); - expect(body.body).toBeDefined(); + expect(res.status).toBe(400); + const body = (await res.json()) as { error: string }; + expect(body.error).toBe("Only http/https URLs are allowed"); + }); + + it("returns 400 for invalid URL string when auth token is valid", async () => { + const res = await fetch(`${baseUrl}/fetch`, { + method: "POST", + headers: { + "Content-Type": "application/json", + "X-MCP-Proxy-Auth": `Bearer ${TEST_TOKEN}`, + }, + body: JSON.stringify({ + url: "not a valid url", + init: { method: "GET" }, + }), + }); + expect(res.status).toBe(400); + const body = (await res.json()) as { error: string }; + expect(body.error).toBe("Invalid URL"); + }); + + it("returns 400 when url is missing when auth token is valid", async () => { + const res = await fetch(`${baseUrl}/fetch`, { + method: "POST", + headers: { + "Content-Type": "application/json", + "X-MCP-Proxy-Auth": `Bearer ${TEST_TOKEN}`, + }, + body: JSON.stringify({ init: { method: "GET" } }), + }); + expect(res.status).toBe(400); + const body = (await res.json()) as { error: string }; + expect(body.error).toBe("Missing or invalid url"); + }); + + it("forwards request when auth token is valid", async () => { + const upstreamPayload = JSON.stringify({ hello: "proxy-fetch-test" }); + + await withLocalUpstream( + (req, res) => { + res.writeHead(200, { "Content-Type": "application/json" }); + res.end(upstreamPayload); + }, + async (origin) => { + const upstreamUrl = `${origin}/ok`; + + const res = await fetch(`${baseUrl}/fetch`, { + method: "POST", + headers: { + "Content-Type": "application/json", + "X-MCP-Proxy-Auth": `Bearer ${TEST_TOKEN}`, + }, + body: JSON.stringify({ + url: upstreamUrl, + init: { method: "GET" }, + }), + }); + + expect(res.status).toBe(200); + const body = (await res.json()) as { + ok: boolean; + status: number; + statusText: string; + body: string; + headers: Record; + }; + expect(body.ok).toBe(true); + expect(body.status).toBe(200); + expect(body.statusText).toBe("OK"); + expect(body.body).toBe(upstreamPayload); + expect(body.headers["content-type"]).toMatch(/application\/json/i); + }, + ); }); it("mirrors upstream 404 (non-2xx) when auth token is valid", async () => { - const upstream: Server = createServer((req, res) => { - res.writeHead(404, { "Content-Type": "application/json" }); - res.end('{"error":"not_found"}'); - }); + await withLocalUpstream( + (req, res) => { + res.writeHead(404, { "Content-Type": "application/json" }); + res.end('{"error":"not_found"}'); + }, + async (origin) => { + const upstreamUrl = `${origin}/missing`; - await new Promise((resolve, reject) => { - upstream.once("error", reject); - upstream.listen(0, "127.0.0.1", () => resolve()); - }); + const res = await fetch(`${baseUrl}/fetch`, { + method: "POST", + headers: { + "Content-Type": "application/json", + "X-MCP-Proxy-Auth": `Bearer ${TEST_TOKEN}`, + }, + body: JSON.stringify({ + url: upstreamUrl, + init: { method: "GET" }, + }), + }); - const addr = upstream.address(); - if (!addr || typeof addr === "string") { - upstream.close(); - throw new Error("Expected TCP listen address"); - } - const upstreamUrl = `http://127.0.0.1:${addr.port}/missing`; - - try { - const res = await fetch(`${baseUrl}/fetch`, { - method: "POST", - headers: { - "Content-Type": "application/json", - "X-MCP-Proxy-Auth": `Bearer ${TEST_TOKEN}`, - }, - body: JSON.stringify({ - url: upstreamUrl, - init: { method: "GET" }, - }), - }); - - expect(res.status).toBe(404); - const body = (await res.json()) as { - ok: boolean; - status: number; - body: string; - }; - expect(body.ok).toBe(false); - expect(body.status).toBe(404); - expect(JSON.parse(body.body)).toEqual({ error: "not_found" }); - } finally { - await new Promise((r) => upstream.close(() => r())); - } + expect(res.status).toBe(404); + const body = (await res.json()) as { + ok: boolean; + status: number; + body: string; + }; + expect(body.ok).toBe(false); + expect(body.status).toBe(404); + expect(JSON.parse(body.body)).toEqual({ error: "not_found" }); + }, + ); }); }); diff --git a/client/src/components/AuthDebugger.tsx b/client/src/components/AuthDebugger.tsx index cd1c7e22..376817d6 100644 --- a/client/src/components/AuthDebugger.tsx +++ b/client/src/components/AuthDebugger.tsx @@ -108,8 +108,13 @@ const AuthDebugger = ({ }); }, [serverUrl, updateAuthState]); - const fetchFn = - connectionType === "proxy" && config ? createProxyFetch(config) : undefined; + const fetchFn = useMemo( + () => + connectionType === "proxy" && config + ? createProxyFetch(config) + : undefined, + [connectionType, config], + ); const stateMachine = useMemo( () => new OAuthStateMachine(serverUrl, updateAuthState, fetchFn), diff --git a/client/src/lib/__tests__/mcpProxyTransportErrorCode.test.ts b/client/src/lib/__tests__/mcpProxyTransportErrorCode.test.ts new file mode 100644 index 00000000..7e0a33ac --- /dev/null +++ b/client/src/lib/__tests__/mcpProxyTransportErrorCode.test.ts @@ -0,0 +1,24 @@ +/** + * Compare client vs server numeric literal without importing server/mcpProxy.ts: + * that module pulls in the SDK graph and breaks Jest (e.g. optional peer deps). + */ +import { readFileSync } from "fs"; +import { resolve } from "path"; +import { MCP_PROXY_TRANSPORT_ERROR_CODE as clientCode } from "../constants"; + +const EXPORT_RE = + /export const MCP_PROXY_TRANSPORT_ERROR_CODE\s*=\s*(-?\d+)\s*;/; + +describe("MCP_PROXY_TRANSPORT_ERROR_CODE", () => { + it("matches server/src/mcpProxy.ts (avoid silent drift between client and server)", () => { + const serverSrcPath = resolve( + __dirname, + "../../../../server/src/mcpProxy.ts", + ); + const serverSrc = readFileSync(serverSrcPath, "utf-8"); + const match = serverSrc.match(EXPORT_RE); + expect(match).not.toBeNull(); + const serverCode = Number(match![1]); + expect(clientCode).toBe(serverCode); + }); +}); diff --git a/client/src/lib/__tests__/proxyFetch.test.ts b/client/src/lib/__tests__/proxyFetch.test.ts index c67f4384..5ac239eb 100644 --- a/client/src/lib/__tests__/proxyFetch.test.ts +++ b/client/src/lib/__tests__/proxyFetch.test.ts @@ -221,4 +221,24 @@ describe("createProxyFetch", () => { const callBody = JSON.parse(mockFetch.mock.calls[0][1].body); expect(callBody.url).toBe("https://example.com/discovery"); }); + + it("uses Request.url when input is a Request (not [object Request])", async () => { + mockFetch.mockResolvedValue({ + ok: true, + json: () => + Promise.resolve({ + ok: true, + status: 200, + statusText: "OK", + headers: {}, + body: "", + }), + }); + + const fetchFn = createProxyFetch(configWithProxy); + await fetchFn(new Request("https://example.com/from-request")); + + const callBody = JSON.parse(mockFetch.mock.calls[0][1].body); + expect(callBody.url).toBe("https://example.com/from-request"); + }); }); diff --git a/client/src/lib/constants.ts b/client/src/lib/constants.ts index 20979d93..d986d380 100644 --- a/client/src/lib/constants.ts +++ b/client/src/lib/constants.ts @@ -41,7 +41,8 @@ export const DEFAULT_MCP_PROXY_LISTEN_PORT = "6277"; * JSON-RPC error code when the inspector proxy cannot complete a forward to the upstream MCP server. * **-32099** — high end of JSON-RPC’s -32000..-32099 server-error band to avoid MCP-assigned codes * (see comment on `server/src/mcpProxy.ts` `MCP_PROXY_TRANSPORT_ERROR_CODE`). Duplicated like - * `DEFAULT_MCP_PROXY_LISTEN_PORT`. + * `DEFAULT_MCP_PROXY_LISTEN_PORT`. Keep in sync with the server value; + * `src/lib/__tests__/mcpProxyTransportErrorCode.test.ts` fails if they drift. */ export const MCP_PROXY_TRANSPORT_ERROR_CODE = -32099; diff --git a/client/src/lib/hooks/useConnection.ts b/client/src/lib/hooks/useConnection.ts index 80739e66..016f8aa4 100644 --- a/client/src/lib/hooks/useConnection.ts +++ b/client/src/lib/hooks/useConnection.ts @@ -379,8 +379,6 @@ export function useConnection({ } }; - const is401Error = isConnectionAuthError; - const isProxyAuthError = (error: unknown): boolean => { return ( error instanceof Error && @@ -389,7 +387,7 @@ export function useConnection({ }; const handleAuthError = async (error: unknown) => { - if (is401Error(error)) { + if (isConnectionAuthError(error)) { let scope = oauthScope?.trim(); const fetchFn = connectionType === "proxy" ? createProxyFetch(config) : undefined; @@ -820,7 +818,7 @@ export function useConnection({ if (shouldRetry) { return connect(undefined, retryCount + 1); } - if (is401Error(error)) { + if (isConnectionAuthError(error)) { // Don't set error state if we're about to redirect for auth return; diff --git a/client/src/lib/proxyFetch.ts b/client/src/lib/proxyFetch.ts index f7017d05..a0ff5977 100644 --- a/client/src/lib/proxyFetch.ts +++ b/client/src/lib/proxyFetch.ts @@ -92,7 +92,12 @@ export function createProxyFetch(config: InspectorConfig): typeof fetch { input: RequestInfo | URL, init?: RequestInit, ): Promise => { - const url = typeof input === "string" ? input : input.toString(); + const url = + typeof input === "string" + ? input + : input instanceof Request + ? input.url + : input.toString(); // Serialize body for JSON transport. URLSearchParams and similar don't // JSON-serialize (they become {}), so we must convert to string first. diff --git a/server/src/index.ts b/server/src/index.ts index d3214cd4..e6af55e5 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -844,6 +844,24 @@ app.post( try { const { url, init } = req.body as { url: string; init?: RequestInit }; + if (typeof url !== "string" || url.length === 0) { + res.status(400).json({ error: "Missing or invalid url" }); + return; + } + + let parsedUrl: URL; + try { + parsedUrl = new URL(url); + } catch { + res.status(400).json({ error: "Invalid URL" }); + return; + } + + if (!["http:", "https:"].includes(parsedUrl.protocol)) { + res.status(400).json({ error: "Only http/https URLs are allowed" }); + return; + } + const response = await fetch(url, { method: init?.method ?? "GET", headers: (init?.headers as Record) ?? {}, diff --git a/server/src/mcpProxy.ts b/server/src/mcpProxy.ts index bf2d5c87..3195553e 100644 --- a/server/src/mcpProxy.ts +++ b/server/src/mcpProxy.ts @@ -12,6 +12,9 @@ import { isJSONRPCRequest } from "@modelcontextprotocol/sdk/types.js"; * the band are assigned over time — e.g. **-32002** is used for **ResourceNotFound** in parts of * the MCP ecosystem). We use **-32099** so this inspector-only bridge error stays clear of those * registered meanings; the client still keys off `error.data` shape, not the number alone. + * + * Keep in sync with `client/src/lib/constants.ts`; drift is caught by + * `client/src/lib/__tests__/mcpProxyTransportErrorCode.test.ts`. */ export const MCP_PROXY_TRANSPORT_ERROR_CODE = -32099; @@ -62,8 +65,8 @@ export function serializeProxyTransportError( } else if (error instanceof SseError) { if (error.code !== undefined) attachHttpStatusIfValid(error.code); } - const u = headerHolder?.lastUpstream401; - if (u && headerHolder) { + if (headerHolder?.lastUpstream401) { + const u = headerHolder.lastUpstream401; data.upstream401 = { wwwAuthenticate: u.wwwAuthenticate, body: u.body,