diff --git a/README.md b/README.md index 9f1a9a0..57f64f1 100644 --- a/README.md +++ b/README.md @@ -232,13 +232,43 @@ navigates on its own — the SDK can demand authorization from inside a running tool call, and evicting the user then would discard the run. The URL is recorded on `provider.authorizationUrl` and the app decides when to use it. -Two things to check on the **server** side, both invisible until they bite: +### CORS: what the server must send -- CORS must allow `Authorization` and expose `WWW-Authenticate` and - `mcp-session-id` (`Access-Control-Expose-Headers`). Without the first, the - browser cannot read the 401 challenge and discovery never starts. -- The authorization server's `/register`, metadata and token endpoints must be - CORS-enabled too — a browser client calls them directly. +A browser tells JavaScript nothing about why it blocked a request — a CORS +rejection and a server that is down both arrive as `TypeError: Failed to fetch`, +with the real reason printed only to the devtools console. So these are worth +checking first rather than last: + +``` +Access-Control-Allow-Origin: +Access-Control-Allow-Methods: GET, POST, DELETE, OPTIONS +Access-Control-Allow-Headers: Content-Type, Authorization, mcp-session-id, + MCP-Protocol-Version, Last-Event-ID +Access-Control-Expose-Headers: mcp-session-id, WWW-Authenticate +``` + +Each line earns its place: + +- **`MCP-Protocol-Version` in Allow-Headers** is the one that bites hardest. The + spec has required that header on every request *after* `initialize` since + 2025-06-18, and CORS lists written before that date omit it. The first request + does not carry it, so discovery, the whole OAuth dance and `initialize` all + succeed — and then everything afterwards is blocked. It reads exactly like the + consent step failing, and sends you to debug the client. +- **`WWW-Authenticate` in Expose-Headers**, or the browser hides the 401 + challenge from JS and OAuth discovery never starts. +- **`Last-Event-ID`** for resuming a dropped SSE stream. +- The authorization server's `/register`, metadata and token endpoints need CORS + too, `OPTIONS` included — a browser client calls them directly. +- If the server uses the MCP SDK's DNS-rebinding protection, its `allowedOrigins` + must contain your origin. Left empty, it answers **403 `Origin not allowed`** + to any request that carries an `Origin` header — which is every request a + browser makes. + +When a connection does fail this way, `connectMcpHttp` no longer just repeats the +browser's silence: it re-probes the endpoint with a plain request and, if that +gets through, says which header is being refused. `diagnoseMcpCors(url)` is +exported so a UI can show the same sentence. ## Configuration (highlights) diff --git a/src/mcp/http.ts b/src/mcp/http.ts index 3734265..93b4e7d 100644 --- a/src/mcp/http.ts +++ b/src/mcp/http.ts @@ -238,6 +238,68 @@ const refreshIfExpired = async ( } } +/** + * A fetch the browser refused for CORS reasons is indistinguishable, from + * JavaScript, from a server that is simply down: both surface as a bare + * `TypeError`. The reason is deliberately withheld, and the browser prints it + * only to the devtools console — which is why this failure is usually read as a + * client bug and debugged in the wrong place for a long time. + */ +const isNetworkLevelFailure = (err: unknown): boolean => + err instanceof TypeError || /failed to fetch|networkerror|load failed/i.test(String(err)) + +/** + * Work out WHY the browser blocked us, by asking a question it will answer. + * + * A request carrying only `content-type` needs the same preflight as the real + * one but asks for nothing else. So if the plain request gets through while the + * transport's did not, the origin is fine and a *header* is the problem — and + * the header the transport adds is `MCP-Protocol-Version`, which the spec has + * required on every post-initialize request since 2025-06-18 and which server + * CORS lists written before that date do not include. That is the shape of the + * bug that lets OAuth and `initialize` succeed and then kills the connection + * moments later, looking for all the world like the consent step failed. + * + * Exported because a UI wants to say this too, not just log it. + */ +export const diagnoseMcpCors = async ( + url: string, + fetchFn: FetchLike = globalThis.fetch, +): Promise => { + let plain: Response + try { + plain = await fetchFn(url, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ jsonrpc: '2.0', id: 0, method: 'ping' }), + }) + } catch { + return ( + `the browser blocked every request to ${url}, including a plain one. ` + + 'The server is unreachable, or it does not answer the CORS preflight for this ' + + 'origin: it must return Access-Control-Allow-Origin (and handle OPTIONS) on the ' + + 'MCP endpoint, the OAuth endpoints and the /.well-known documents.' + ) + } + const hints = [ + `a plain request to ${url} succeeded (HTTP ${plain.status}) but the transport's did not, ` + + 'so the origin is allowed and a request HEADER is being refused. Add ' + + 'MCP-Protocol-Version (required on every request after initialize) and Last-Event-ID ' + + "to the server's Access-Control-Allow-Headers, alongside Content-Type, Authorization " + + 'and mcp-session-id.', + ] + // A 401 whose challenge JS cannot read means OAuth discovery cannot start + // from a browser at all, so say so while we are here. + if (plain.status === 401 && !plain.headers.get('www-authenticate')) { + hints.push( + 'The server also answers 401 without exposing WWW-Authenticate: add it to ' + + 'Access-Control-Expose-Headers, or a browser client cannot read the challenge ' + + 'and OAuth discovery never starts.', + ) + } + return hints.join(' ') +} + const openConnection = async ( name: string, cfg: McpHttpServerConfig, @@ -295,7 +357,13 @@ const openConnection = async ( // lifetime of the tab. if (client) await client.close().catch(() => {}) const needsAuthorization = err instanceof UnauthorizedError - const message = err instanceof Error ? err.message : String(err) + let message = err instanceof Error ? err.message : String(err) + // Turn "TypeError: Failed to fetch" into the sentence the reader needs. + // Only on the failure path, so the extra request costs nothing in normal use. + if (!needsAuthorization && isNetworkLevelFailure(err)) { + const hint = await diagnoseMcpCors(cfg.url, cfg.fetch).catch(() => undefined) + if (hint) message = `${message} - ${hint}` + } log( 'error', needsAuthorization diff --git a/src/mcp/index.ts b/src/mcp/index.ts index 957d815..5de1afe 100644 --- a/src/mcp/index.ts +++ b/src/mcp/index.ts @@ -1,4 +1,4 @@ -export { connectMcpHttp, flattenContent } from './http.js' +export { connectMcpHttp, diagnoseMcpCors, flattenContent } from './http.js' // Re-exported so a host can identify an authorization failure by identity. // The SDK's class does not set `name`, so `err.name === 'UnauthorizedError'` // is always false — string sniffing is not an option, and a host that only diff --git a/tests/mcp.test.ts b/tests/mcp.test.ts index 404881e..bd5e3f0 100644 --- a/tests/mcp.test.ts +++ b/tests/mcp.test.ts @@ -5,6 +5,7 @@ import { beginMcpOAuth, BrowserOAuthProvider, connectMcpHttp, + diagnoseMcpCors, finishMcpOAuth, flattenContent, MemoryOAuthStorage, @@ -867,3 +868,82 @@ test('the subpath re-exports UnauthorizedError so a host can identify auth failu assert.ok(err instanceof UnauthorizedError) assert.ok(err instanceof Error) }) + +// ── The failure a browser refuses to explain ──────────────────────────────── +// A CORS rejection reaches JavaScript as a bare TypeError, identical to a +// server being down. The interesting case is the one that looks like the OAuth +// consent step broke: the origin is allowed, so discovery, the token exchange +// and `initialize` all succeed, and then the transport adds the +// MCP-Protocol-Version header that the spec requires afterwards — and a server +// whose CORS list predates that requirement blocks every request from there on. + +test('diagnoseMcpCors: names the refused header when a plain request gets through', async () => { + const seen: string[][] = [] + const hint = await diagnoseMcpCors(MCP_URL, async (_url, init) => { + seen.push(Object.keys((init?.headers ?? {}) as Record)) + return new Response('{}', { status: 200 }) + }) + assert.match(hint ?? '', /MCP-Protocol-Version/) + assert.match(hint ?? '', /Access-Control-Allow-Headers/) + // The probe must ask for nothing beyond content-type, or it proves nothing. + assert.deepEqual(seen, [['content-type']]) +}) + +test('diagnoseMcpCors: a plain request that is also blocked points at the origin', async () => { + const hint = await diagnoseMcpCors(MCP_URL, async () => { + throw new TypeError('Failed to fetch') + }) + assert.match(hint ?? '', /Access-Control-Allow-Origin/) + assert.doesNotMatch(hint ?? '', /MCP-Protocol-Version/) +}) + +test('diagnoseMcpCors: an unreadable 401 challenge is called out as well', async () => { + const hint = await diagnoseMcpCors( + MCP_URL, + async () => new Response('{}', { status: 401 }), // no WWW-Authenticate exposed + ) + assert.match(hint ?? '', /WWW-Authenticate/) + assert.match(hint ?? '', /Access-Control-Expose-Headers/) +}) + +test('diagnoseMcpCors: stays quiet about the challenge when it is readable', async () => { + const hint = await diagnoseMcpCors( + MCP_URL, + async () => + new Response('{}', { + status: 401, + headers: { 'www-authenticate': 'Bearer resource_metadata="https://x.example.test/rm"' }, + }), + ) + assert.doesNotMatch(hint ?? '', /Expose-Headers/) +}) + +test('connectMcpHttp: a blocked fetch is reported with the CORS diagnosis attached', async () => { + let calls = 0 + const mcp = await connectMcpHttp({ + docs: { + url: MCP_URL, + fetch: async () => { + calls += 1 + // The transport's own requests are blocked; the diagnostic probe is not. + if (calls === 1) throw new TypeError('Failed to fetch') + return new Response('{}', { status: 200 }) + }, + }, + }) + const docs = mcp.results.find((r) => r.name === 'docs') + assert.equal(docs?.connected, false) + assert.match(docs?.error ?? '', /Failed to fetch/) + assert.match(docs?.error ?? '', /MCP-Protocol-Version/) + await mcp.close() +}) + +test('connectMcpHttp: an ordinary server error is not dressed up as a CORS problem', async () => { + const mcp = await connectMcpHttp({ + docs: { url: MCP_URL, fetch: async () => new Response('boom', { status: 500 }) }, + }) + const docs = mcp.results.find((r) => r.name === 'docs') + assert.equal(docs?.connected, false) + assert.doesNotMatch(docs?.error ?? '', /Access-Control/) + await mcp.close() +})