modelcontextprotocol/inspector
Watch
1
0
Fork
You've already forked inspector
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

fix(auth): mirror the SDK's candidate policy in the compat probe loop

Browse source 
The probe loop advanced to the next OIDC candidate on any failure, which meant
it could promote a later candidate over an earlier one the SDK would have
stopped at — turning a failure into a success, a worse defect than the one
being worked around. Four distinct cases: a 500 (an outage the SDK surfaces), a
2xx body that will not parse (terminal for the SDK), a network error (which the
SDK propagates outside the browser's CORS case), and a non-JSON media type.

It now continues only where the SDK's own loop does — a 4xx or 502 — and
otherwise returns the original response, leaving the SDK to make the same
request and reach its own verdict. The `content-type` gate is gone entirely,
because the SDK parses a 2xx discovery body whatever media type it carries, so
gating could skip a genuine OpenID provider document and substitute an
RFC-8414-only one from a lower-priority candidate in its place.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UL9retfvAXRgvi6EWk4SY4
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-08-28 01:59:04 -04:00
commit 89d6af4436
2 changed files with 58 additions and 32 deletions
Download patch file Download diff file Expand all files Collapse all files

48
clients/web/src/test/core/auth/oidcDiscoveryCompat.test.ts
View file

@ -203,17 +203,17 @@ describe("withRfc8414OidcCompat", () => {
expect(base).toHaveBeenCalledTimes(2);
});
it("skips a candidate that is not JSON and keeps looking", async () => {
it("accepts a 2xx candidate whatever media type it carries", async () => {
// The SDK parses a 2xx discovery body regardless of `content-type`, so
// gating on it here could skip a document the SDK would have used (Copilot).
vi.spyOn(console, "warn").mockImplementation(() => {});
const base = vi.fn<typeof fetch>(async (input) => {
const url = String(input);
if (url === OIDC_SUFFIXED) {
return new Response("<html>login</html>", {
if (String(input) === OIDC_SUFFIXED) {
return new Response(JSON.stringify(RFC8414_DOC), {
status: 200,
headers: { "content-type": "text/html" },
headers: { "content-type": "text/plain" },
});
}
if (url === OIDC_APPENDED) return json(RFC8414_DOC);
return notFound();
});
const wrapped = withRfc8414OidcCompat(base);
@ -223,10 +223,12 @@ describe("withRfc8414OidcCompat", () => {
);
});
it("skips a candidate whose JSON body will not parse", async () => {
it("stops at a candidate whose body will not parse", async () => {
// Terminal for the SDK, so a later candidate must not be promoted over it
// — that would turn a failure into a success (Copilot).
const original = notFound();
const base = vi.fn<typeof fetch>(async (input) => {
if (String(input).includes("openid-configuration")) {
if (String(input) === OIDC_SUFFIXED) {
return new Response("{ not json", {
status: 200,
headers: { "content-type": "application/json" },
@ -237,10 +239,12 @@ describe("withRfc8414OidcCompat", () => {
const wrapped = withRfc8414OidcCompat(base);
await expect(wrapped(RFC8414_URL)).resolves.toBe(original);
expect(base).toHaveBeenCalledTimes(3);
// The failed original and the first candidate only — the second is not
// reached.
expect(base).toHaveBeenCalledTimes(2);
});
it("skips a candidate that itself fails", async () => {
it("walks past a candidate that 404s, as the SDK does", async () => {
const original = notFound();
const base = vi.fn<typeof fetch>().mockResolvedValue(original);
const wrapped = withRfc8414OidcCompat(base);
@ -249,7 +253,27 @@ describe("withRfc8414OidcCompat", () => {
expect(base).toHaveBeenCalledTimes(3);
});
it("treats a probe that throws as a candidate to skip", async () => {
it("stops at a candidate returning a status the SDK would not walk past", async () => {
// A 500 on the first OIDC candidate is an outage the SDK surfaces; hiding
// it behind a second candidate's document would be worse than the bug
// being worked around (Copilot).
const original = notFound();
const base = vi.fn<typeof fetch>(async (input) => {
const url = String(input);
if (url === OIDC_SUFFIXED) return new Response("boom", { status: 500 });
if (url === OIDC_APPENDED) return json(RFC8414_DOC);
return original;
});
const wrapped = withRfc8414OidcCompat(base);
await expect(wrapped(RFC8414_URL)).resolves.toBe(original);
expect(base).toHaveBeenCalledTimes(2);
});
it("stops when a probe throws", async () => {
// The SDK swallows only the browser's CORS `TypeError`; elsewhere a network
// error propagates. Leave it to the SDK to make the same request and reach
// its own verdict (Copilot).
const original = notFound();
const base = vi.fn<typeof fetch>(async (input) => {
if (String(input).includes("openid-configuration")) {
@ -260,7 +284,7 @@ describe("withRfc8414OidcCompat", () => {
const wrapped = withRfc8414OidcCompat(base);
await expect(wrapped(RFC8414_URL)).resolves.toBe(original);
expect(base).toHaveBeenCalledTimes(3);
expect(base).toHaveBeenCalledTimes(2);
});
it("probes on a 502, which the SDK also walks past", async () => {

42
core/auth/oidcDiscoveryCompat.ts
View file

@ -160,19 +160,6 @@ function continuesDiscovery(status: number): boolean {
return (status >= 400 && status < 500) || status === 502;
}
/**
* Whether a `content-type` names a whole JSON document.
*
* An exact media-type test rather than a substring search for "json", for the
* same reason `endpointOverrides.ts` uses one: a streaming media type would be
* read to completion and never resolve.
*/
function isJsonDocumentResponse(contentType: string | null): boolean {
if (!contentType) return false;
const mediaType = contentType.split(";")[0].trim().toLowerCase();
return mediaType === "application/json" || mediaType.endsWith("+json");
}
/**
* Whether a parsed body is RFC 8414 authorization-server metadata that is *not*
* a valid OpenID provider document — the exact shape the upstream schema
@ -252,26 +239,41 @@ export function withRfc8414OidcCompat(fetchFn: typeof fetch): typeof fetch {
if (candidates.length === 0) return response;
const headers = discoveryHeaders(input, init);
// The loop advances to the next candidate only where the SDK's own loop
// would. Anywhere else it hands the original response back and lets
// discovery run its normal course, because *promoting a later candidate
// over an earlier one the SDK would have stopped at* would turn a failure
// into a success — a much worse defect than the one being worked around
// (Copilot). Concretely: a 500 on the first OIDC candidate is an outage the
// SDK surfaces, a malformed JSON body there is terminal for it, and a
// network error propagates outside the browser's CORS case. In each of
// those, returning `response` leaves the SDK to make the same request and
// reach the same verdict it always would.
for (const candidate of candidates) {
let probe: Response;
try {
probe = await fetchFn(candidate, { headers });
} catch {
// A network or CORS failure on a URL the SDK had not asked for yet is
// not this wrapper's to report — the SDK will make the same request and
// handle it. Try the next candidate.
continue;
return response;
}
if (!probe.ok) {
if (continuesDiscovery(probe.status)) continue;
return response;
}
if (!probe.ok) continue;
if (!isJsonDocumentResponse(probe.headers.get("content-type"))) continue;
// Deliberately not gated on `content-type`: the SDK parses a 2xx
// discovery body whatever media type it carries, so gating here could
// skip a genuine OIDC document served with an odd one and substitute a
// later RFC-8414-only document in its place. A body that will not parse
// falls through to `return response` below, where the SDK re-fetches it
// and raises its own parse error.
let parsed: unknown;
let body: string;
try {
body = await probe.text();
parsed = JSON.parse(body);
} catch {
continue;
return response;
}
if (!isRfc8414OnlyMetadata(parsed)) {
// Either not metadata at all, or a genuine OpenID provider document the