Skip to content

Inspector client: Added proxy fetch for use by auth (to avoid CORS issues)#1047

Open
BobDickinson wants to merge 8 commits intomodelcontextprotocol:mainfrom
BobDickinson:web-client-oauth-proxy-fetch
Open

Inspector client: Added proxy fetch for use by auth (to avoid CORS issues)#1047
BobDickinson wants to merge 8 commits intomodelcontextprotocol:mainfrom
BobDickinson:web-client-oauth-proxy-fetch

Conversation

@BobDickinson
Copy link
Copy Markdown
Contributor

@BobDickinson BobDickinson commented Jan 31, 2026

There are many types of failures in the auth process due to CORS when authenticating from a browser environment, including response header stripping, failure to access server or auth server endpoints (including, but not limited to, server metadata endpoints), failure in token exchange, and I'm sure others.

Issue #995 and the associated PR #996 raised a similar (maybe the same?) issue.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • Test updates

Changes Made

This PR takes a different approach than #996, which is that we remote only the fetch function to the proxy server (so it can run from a Node environment instead of the browser). When in "proxy" mode we use a client side proxyFetch function, which we pass to all auth functions (which were designed for this exact situation, such that they all take an optional fetchFn param). This prevents CORS issue from impacting auth in any way, while still relying on the auth SDK functions for all auth logic.

This change is fairly lightweight (< 100 lines of non-test code).

Related Issues

This PR is related to #995 and the associated PR #996, but takes a different approach. I am interested to learn whether this approach also solves the issues targeted there (I have tested my use cases, but not the use cases from the issue/PR or mentioned in the comments there).

Testing

  • Tested in UI mode
  • Tested in CLI mode
  • Tested with STDIO transport
  • Tested with SSE transport
  • Tested with Streamable HTTP transport
  • Added/updated automated tests
  • Manual testing performed

Test Results and/or Instructions

Tested with:

  • https://example-server.modelcontextprotocol.io/mcp - Works, as before
  • https://api.githubcopilot.com/mcp/ - Previously failed discovery due to CORS, now fails because DCR not supported, but demonstrates that this fix works

Checklist

  • Code follows the style guidelines (ran npm run prettier-fix)
  • Self-review completed
  • Code is commented where necessary
  • Documentation updated (README, comments, etc.)

@BobDickinson BobDickinson marked this pull request as ready for review January 31, 2026 00:48
@BobDickinson
Copy link
Copy Markdown
Contributor Author

Would appreciate you testing this in your use cases @asoorm and @jstjoe.

@BobDickinson BobDickinson deleted the web-client-oauth-proxy-fetch branch March 7, 2026 21:57
@BobDickinson BobDickinson restored the web-client-oauth-proxy-fetch branch March 13, 2026 00:12
@BobDickinson
Copy link
Copy Markdown
Contributor Author

PR closed by accident when client branch inadvertently cleaned up.

@BobDickinson BobDickinson reopened this Mar 13, 2026
@cliffhall
Copy link
Copy Markdown
Member

cliffhall commented Mar 18, 2026

Connects in Direct Mode / Fails in Via Proxy

FAIL-web-client-oauth-proxy-fetch.mov

Resource

curl -D - https://example.com/mcp
HTTP/2 401 
date: Wed, 18 Mar 2026 23:17:33 GMT
content-type: application/json
content-length: 79
cf-ray: 9de7f3951fd89324-ATL
www-authenticate: Bearer realm="OAuth", error="invalid_token", error_description="Missing or invalid access token", resource_metadata="https://example.com/.well-known/oauth-protected-resource"
server: cloudflare

{"error":"invalid_token","error_description":"Missing or invalid access token"}

Protected Resource Metadata

curl -D - https://example.com/.well-known/oauth-protected-resource
HTTP/2 200 
date: Wed, 18 Mar 2026 23:18:34 GMT
content-type: application/json
content-length: 172
cf-ray: 9de7f5127d98cc22-ATL
server: cloudflare

{"resource":"https://example.com/mcp","authorization_servers":["https://example.com"],"bearer_methods_supported":["header"]}% 

@cliffhall
Copy link
Copy Markdown
Member

cliffhall commented Mar 19, 2026

@BobDickinson also a some auth-related tests are failing ...

…en because the TLS verification changed. Without pinned deps (package lock) it's hard to track down the dep change actually caused this - fix was to use http instead of https example.com test urls.
…which should fix the failures reported with proxy auth. I have not yet verified that locally (pending repro details).
… on error string text matching to detect an OAuth 401)
@BobDickinson
Copy link
Copy Markdown
Contributor Author

Proxy mode OAuth: example-server vs Guildbridge

The problem where example-server auth worked in proxy mode and guildbridge did not was not related to the proxy fetch solution from this PR (it existed as a general issue when connecting to a serving requiring auth over the proxy). So if this PR is not merged for whatever reason, the previous change should still be ported to main.

Issue: In proxy mode, OAuth recovery ran for example-server but not Guildbridge, even though both return a normal 401 with WWW-Authenticate (and resource_metadata) plus invalid_token JSON—the HTTP challenge was fine for both. Recovery was gated on is401Error, which treated Error.message.includes("Missing Authorization header") as a signal to run handleAuthError. That phrase appears in example-server’s JSON body; Guildbridge uses Missing or invalid access token, which did not match any of the substring checks, so is401Error stayed false and handleAuthError never ran.

TL;DR

Before the fix, two things broke proxy OAuth relative to direct mode:

  1. First-hop 401: the proxy answered res.status(401).json(error) on a thrown Error, so the browser lost upstream WWW-Authenticate and the real JSON body—the SDK no longer saw the same HTTP 401 as in direct mode.
  2. Session path: mcpProxy failures became McpError. The McpError message is MCP error <code>: <inner text> and does not contain the substring "401", so error.message.includes("401") was false. (For StreamableHTTPError, the 401 is on .code; the message is Streamable HTTP error: Error POSTing to endpoint: <body> and also does not reliably contain the characters 401.) On that path, example-server still tripped message.includes("Missing Authorization header") because that text is in the upstream 401 body and was embedded in the error string. Guildbridge’s body is Missing or invalid access token, which matched none of the is401Error substring checks, so is401Error was false and handleAuthError never ran.

After the fix: the proxy forwards 401 headers/body on the HTTP response where it can, puts upstream401 / httpStatus in JSON-RPC error.data, and the client uses isConnectionAuthError (client/src/lib/connectionAuthErrors.ts) on types and structured data, not vendor error_description strings in message.


Unauthenticated POST to /mcp (initialize-style body)

Request (same for both):

POST /mcp HTTP/2
Content-Type: application/json
Accept: application/json, text/event-stream
{
  "jsonrpc": "2.0",
  "id": 1,
  "method": "initialize",
  "params": {
    "protocolVersion": "2024-11-05",
    "capabilities": {},
    "clientInfo": { "name": "curl", "version": "1.0" }
  }
}

example-server — https://example-server.modelcontextprotocol.io/mcp

Status: 401

Response headers (representative):

HTTP/2 401
www-authenticate: Bearer error="invalid_token", error_description="Missing Authorization header", resource_metadata="https://example-server.modelcontextprotocol.io/.well-known/oauth-protected-resource"
content-type: application/json; charset=utf-8

Response body:

{
  "error": "invalid_token",
  "error_description": "Missing Authorization header"
}

With Origin: http://localhost:6274 (inspector client origin), the same endpoint also returns CORS headers such as access-control-allow-origin: http://localhost:6274 (exact values may vary by deployment).


Guildbridge — https://guildbridge.modelcontextprotocol.io/mcp

Note: The hostname is modelcontextprotocol, not modelcontect.

Status: 401

Response headers (representative):

HTTP/2 401
www-authenticate: Bearer realm="OAuth", error="invalid_token", error_description="Missing or invalid access token", resource_metadata="https://guildbridge.modelcontextprotocol.io/.well-known/oauth-protected-resource"
content-type: application/json

Response body:

{
  "error": "invalid_token",
  "error_description": "Missing or invalid access token"
}

With Origin: http://localhost:6274, Guildbridge may still return access-control-allow-origin: http://localhost:6274 on this MCP response.


Where the bug was: is401Error string matching on Error.message

handleAuthError only ran when is401Error(error) was true. Before the fix, is401Error returned true if any of these held:

  • SseError with code === 401
  • StreamableHTTPError with code === 401
  • Error.message.includes("401")
  • Error.message.includes("Unauthorized")
  • Error.message.includes("Missing Authorization header")

On the proxy session path, the thrown value was McpError, not StreamableHTTPError, so the instanceof StreamableHTTPError && code === 401 branch did not apply. McpError.message does not include "401" as text. None of the remaining checks matched Guildbridge’s upstream JSON. The includes("Missing Authorization header") check did match example-server, because that exact string is in the 401 response body and ended up inside the error message. That single substring check is what made OAuth recovery work for one host and not the other.


What we changed (high level)

Layer Change
Proxy → browser (HTTP) On upstream 401, forward WWW-Authenticate and raw body (see server/src/index.ts createCustomFetch / sendProxiedUnauthorized). Expose WWW-Authenticate for CORS where needed.
Proxy → browser (JSON-RPC) On failed forward after a session exists, server/src/mcpProxy.ts returns JSON-RPC code -32099 with serializable error.data including upstream401 and/or httpStatus when known.
Client isConnectionAuthError (client/src/lib/connectionAuthErrors.ts) treats SseError / StreamableHTTPError 401, UnauthorizedError, and McpError with -32099 plus auth-shaped data—no dependency on a single vendor’s error_description string.

Hook-level tests live in client/src/lib/hooks/__tests__/useConnection.test.tsx under “Inspector proxy McpError auth recovery”.


Takeaway

  • Do not infer missing OAuth from curl: both hosts return a Bearer challenge with resource_metadata and invalid_token JSON.
  • Do treat proxy fidelity (HTTP headers/body vs JSON-RPC data) and structured client checks as the contract for “run OAuth recovery,” not message substrings tied to one server’s wording.

@cliffhall
Copy link
Copy Markdown
Member

cliffhall commented Apr 7, 2026

Via Proxy is working...

via-proxy-working.mov

@cliffhall
Copy link
Copy Markdown
Member

@claude review

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 7, 2026

Claude finished @cliffhall's task in 2m 17s —— View job


PR Review: Inspector client proxy fetch for OAuth/CORS

Tasks:

  • Gather context from PR description and comments
  • Read changed files
  • Analyze implementation for correctness and security
  • Review tests
  • Post comprehensive review

Overview

This PR fixes a real, well-documented problem: OAuth auth flows in proxy mode were broken by CORS restrictions and fragile string-matching in is401Error. The approach — adding a /fetch endpoint on the proxy and a createProxyFetch factory on the client — is clean and correct in concept. The structural error data in serializeProxyTransportError replacing substring heuristics is a particularly good fix.

A few issues are worth addressing before merge, ranging from a functional memoization bug to a security concern.


Issues

1. fetchFn unstable reference defeats useMemo in AuthDebugger.tsxfunctional bug

client/src/components/AuthDebugger.tsx:110-115

// Bug: called on every render, new function reference every time
const fetchFn =
  connectionType === "proxy" && config ? createProxyFetch(config) : undefined;

const stateMachine = useMemo(
  () => new OAuthStateMachine(serverUrl, updateAuthState, fetchFn),
  [serverUrl, updateAuthState, fetchFn],  // fetchFn is always new → memo never hits
);

createProxyFetch(config) is called on every render outside of useMemo, so fetchFn gets a new function reference each render, which means stateMachine is also reconstructed each render, defeating the memo. Fix: wrap fetchFn in its own useMemo:

const fetchFn = useMemo(
  () => connectionType === "proxy" && config ? createProxyFetch(config) : undefined,
  [connectionType, config],
);

Fix this →


2. Request object URL extraction is broken in proxyFetch.tslatent bug

client/src/lib/proxyFetch.ts:95

const url = typeof input === "string" ? input : input.toString();

When input is a Request object (which is valid per RequestInfo | URL), input.toString() returns "[object Request]", not the URL. Should be:

const url =
  typeof input === "string"
    ? input
    : input instanceof Request
      ? input.url
      : input.toString();

In practice the auth SDK doesn't appear to pass Request objects, so this doesn't break today — but it's a latent footgun if SDK internals change. No test covers this case.

Fix this →


3. /fetch endpoint has no URL scheme validation — security concern

server/src/index.ts:845-851

const { url, init } = req.body as { url: string; init?: RequestInit };
const response = await fetch(url, { ... });

The endpoint proxies any URL without validating that it's http:// or https://. While auth middleware and origin validation significantly limit who can call this, consider adding explicit validation:

const parsedUrl = new URL(url); // throws on invalid URLs
if (!["http:", "https:"].includes(parsedUrl.protocol)) {
  return res.status(400).json({ error: "Only http/https URLs are allowed" });
}

This prevents accidental or intentional use of file://, ftp://, or internal-metadata endpoints (e.g. http://169.254.169.254/... in cloud environments). The token-protection makes this low-risk today, but a defense-in-depth note or URL allow-pattern would be good practice for a proxy endpoint.

Fix this →


4. Integration test hits real network — flakiness concern

client/src/__tests__/proxyFetchEndpoint.test.ts:75-92

it("forwards request when auth token is valid", async () => {
  const res = await fetch(`${baseUrl}/fetch`, {
    // ...
    body: JSON.stringify({ url: "http://example.com/", ... }),
  });
  expect(res.status).toBe(200);
  expect(body.ok).toBe(true);

This test makes a real outbound network call to http://example.com/. The existing non-2xx test (line 94+) correctly spins up a local server — that pattern should be used here too. CI runners without external internet access will have flaky results.

Fix this →


Minor notes

MCP_PROXY_TRANSPORT_ERROR_CODE duplication — The constant -32099 is defined in both server/src/mcpProxy.ts and client/src/lib/constants.ts. The comment acknowledges this ("Duplicated like DEFAULT_MCP_PROXY_LISTEN_PORT"), which is a reasonable stance for a monorepo without a shared package. A simple test asserting client_constant === server_constant would prevent silent drift.

is401Error alias in useConnection.ts (line 382):

const is401Error = isConnectionAuthError;

The alias is harmless but isConnectionAuthError could just be used directly.

Redundant double-guard in serializeProxyTransportError (mcpProxy.ts:65-66):

const u = headerHolder?.lastUpstream401;
if (u && headerHolder) {   // if u is truthy, headerHolder must be defined

if (u) is sufficient, or if (headerHolder?.lastUpstream401).


What's good

  • Replacing fragile substring matching (error.message.includes("Missing Authorization header")) with structured error.data checks is a meaningful correctness improvement that will generalize to any MCP server, not just example-server.
  • Capturing lastUpstream401 in createCustomFetch and forwarding the real WWW-Authenticate header + body to the browser is the right approach — the SDK's auth machinery needs to see the actual HTTP 401.
  • The proxyFetch.ts design is clean: validate the response shape rather than blindly trusting it, distinguish infra errors from mirrored upstream responses, reconstruct a Response object so callers get standard fetch semantics.
  • Test coverage for both the client-side createProxyFetch logic and the server endpoint itself is solid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants