Merge pull request #2371 from modelcontextprotocol/v2/fix/2297-401-response-body

fix: record an intercepted 401's status, headers and body in the network log
This commit is contained in:
Cliff Hall 2026-09-14 02:01:52 -04:00 • committed by GitHub
commit 48376cb5ca
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 570 additions and 31 deletions

View file

@ -1,5 +1,9 @@
import { describe, it, expect, vi } from "vitest";
import {
CHALLENGE_BODY_MAX_BYTES,
CHALLENGE_BODY_READ_MS,
TRUNCATED_BODY_SUFFIX,
WITHHELD_TRUNCATED_JSON_BODY,
createFetchTracker,
findHeader,
redactSensitiveHeaders,
@ -543,6 +547,281 @@ describe("createFetchTracker", () => {
});
});
describe("createFetchTracker — 401/403 challenge bodies (#2297)", () => {
function track(response: Response) {
const tracked: FetchRequestEntryBase[] = [];
const updateResponseBody = vi.fn();
const fetcher = createFetchTracker((async () => response) as typeof fetch, {
trackRequest: (entry) => tracked.push(entry),
updateResponseBody,
});
return { fetcher, tracked, updateResponseBody };
}
it("records the body on the entry itself, before returning", async () => {
const body = JSON.stringify({ error: "invalid_token" });
const { fetcher, tracked, updateResponseBody } = track(
new Response(body, {
status: 401,
headers: { "content-type": "application/json" },
}),
);
const res = await fetcher("https://mcp.example/mcp", { method: "POST" });
expect(tracked).toHaveLength(1);
expect(tracked[0]?.responseStatus).toBe(401);
expect(tracked[0]?.responseBody).toBe(body);
expect(updateResponseBody).not.toHaveBeenCalled();
// The caller still gets an unread body.
expect(await res.text()).toBe(body);
});
it("does the same for a 403", async () => {
const { fetcher, tracked } = track(
new Response("insufficient_scope", { status: 403 }),
);
await fetcher("https://mcp.example/mcp", { method: "POST" });
expect(tracked[0]?.responseBody).toBe("insufficient_scope");
});
it("stops at the byte cap and marks the body truncated", async () => {
const chunk = new Uint8Array(CHALLENGE_BODY_MAX_BYTES / 2).fill(97);
let pulls = 0;
const cancel = vi.fn();
const stream = new ReadableStream<Uint8Array>({
pull(controller) {
pulls += 1;
controller.enqueue(chunk);
},
cancel,
});
const { fetcher, tracked } = track(new Response(stream, { status: 401 }));
const res = await fetcher("https://mcp.example/mcp", { method: "POST" });
const recorded = tracked[0]?.responseBody ?? "";
expect(recorded.endsWith(TRUNCATED_BODY_SUFFIX)).toBe(true);
expect(recorded.length - TRUNCATED_BODY_SUFFIX.length).toBe(
CHALLENGE_BODY_MAX_BYTES,
);
// With the clone released, cancelling the caller's branch reaches the source.
await res.body?.cancel();
expect(cancel).toHaveBeenCalled();
expect(pulls).toBeLessThan(10);
});
it("gives up on a stalled body at the deadline and releases the source", async () => {
vi.useFakeTimers();
try {
const cancel = vi.fn();
const stream = new ReadableStream<Uint8Array>({
start(controller) {
controller.enqueue(new TextEncoder().encode("partial"));
},
cancel,
});
const { fetcher, tracked } = track(new Response(stream, { status: 401 }));
const pending = fetcher("https://mcp.example/mcp", { method: "POST" });
await vi.advanceTimersByTimeAsync(CHALLENGE_BODY_READ_MS);
const res = await pending;
expect(tracked[0]?.responseBody).toBe(`partial${TRUNCATED_BODY_SUFFIX}`);
await res.body?.cancel();
expect(cancel).toHaveBeenCalled();
} finally {
vi.useRealTimers();
}
});
it("records the entry with no body when the clone cannot be made", async () => {
const response = new Response("nope", { status: 401 });
vi.spyOn(response, "clone").mockImplementation(() => {
throw new Error("clone failed");
});
const { fetcher, tracked } = track(response);
await fetcher("https://mcp.example/mcp", { method: "POST" });
expect(tracked).toHaveLength(1);
expect(tracked[0]?.responseStatus).toBe(401);
expect(tracked[0]?.responseBody).toBeUndefined();
});
it("records the entry with no body when the read errors", async () => {
const stream = new ReadableStream<Uint8Array>({
pull(controller) {
controller.error(new Error("socket reset"));
},
});
const { fetcher, tracked } = track(new Response(stream, { status: 401 }));
await fetcher("https://mcp.example/mcp", { method: "POST" });
expect(tracked[0]?.responseStatus).toBe(401);
expect(tracked[0]?.responseBody).toBeUndefined();
});
});
describe("createFetchTracker — challenge body bounds and redaction (#2297)", () => {
function trackOne(response: Response) {
const tracked: FetchRequestEntryBase[] = [];
const fetcher = createFetchTracker((async () => response) as typeof fetch, {
trackRequest: (entry) => tracked.push(entry),
});
return { fetcher, tracked };
}
/** A body that delivers `prefix` and then never ends. */
function stalledBody(prefix: string): ReadableStream<Uint8Array> {
return new ReadableStream<Uint8Array>({
start(controller) {
controller.enqueue(new TextEncoder().encode(prefix));
},
});
}
async function withDeadlineElapsed<T>(run: () => Promise<T>): Promise<T> {
vi.useFakeTimers();
try {
const pending = run();
await vi.advanceTimersByTimeAsync(CHALLENGE_BODY_READ_MS);
return await pending;
} finally {
vi.useRealTimers();
}
}
it("caps a single chunk larger than the budget at the byte limit", async () => {
const oversized = new Uint8Array(1024 * 1024).fill(97);
const { fetcher, tracked } = trackOne(
new Response(
new ReadableStream<Uint8Array>({
start(controller) {
controller.enqueue(oversized);
controller.close();
},
}),
{ status: 401 },
),
);
await fetcher("https://mcp.example/mcp", { method: "POST" });
const recorded = tracked[0]?.responseBody ?? "";
expect(recorded.endsWith(TRUNCATED_BODY_SUFFIX)).toBe(true);
expect(recorded.length - TRUNCATED_BODY_SUFFIX.length).toBe(
CHALLENGE_BODY_MAX_BYTES,
);
});
it("records the body of a 401 to a long-lived-stream GET", async () => {
const { fetcher, tracked } = trackOne(
new Response("unauthorized", {
status: 401,
headers: { "content-type": "text/event-stream" },
}),
);
await fetcher("https://mcp.example/mcp", { method: "GET" });
expect(tracked[0]?.responseBody).toBe("unauthorized");
});
it("withholds a cut-off JSON body rather than logging an unredacted prefix", async () => {
const { fetcher, tracked } = trackOne(
new Response(stalledBody('{"access_token":"live-access","token_type'), {
status: 401,
headers: { "content-type": "application/json" },
}),
);
await withDeadlineElapsed(() =>
fetcher("https://mcp.example/token", { method: "POST" }),
);
expect(tracked[0]?.responseBody).toBe(WITHHELD_TRUNCATED_JSON_BODY);
expect(serializedWithoutId(tracked[0]!)).not.toContain("live-access");
});
it("withholds a cut-off body that sniffs as JSON with no content-type", async () => {
const { fetcher, tracked } = trackOne(
new Response(stalledBody(' [{"refresh_token":"live-refresh"'), {
status: 403,
}),
);
await withDeadlineElapsed(() =>
fetcher("https://mcp.example/token", { method: "POST" }),
);
expect(tracked[0]?.responseBody).toBe(WITHHELD_TRUNCATED_JSON_BODY);
});
it("still redacts a complete JSON challenge body", async () => {
const { fetcher, tracked } = trackOne(
new Response(JSON.stringify({ error: "x", access_token: "live" }), {
status: 401,
headers: { "content-type": "application/json" },
}),
);
await fetcher("https://mcp.example/token", { method: "POST" });
expect(tracked[0]?.responseBody).toBe(
JSON.stringify({ error: "x", access_token: REDACTED_VALUE }),
);
});
it("redacts a cut-off form body field by field", async () => {
const { fetcher, tracked } = trackOne(
new Response(stalledBody("error=invalid&access_token=live&scope=re"), {
status: 401,
headers: { "content-type": "application/x-www-form-urlencoded" },
}),
);
await withDeadlineElapsed(() =>
fetcher("https://mcp.example/token", { method: "POST" }),
);
const recorded = tracked[0]?.responseBody ?? "";
expect(recorded).not.toContain("live");
expect(recorded.endsWith(TRUNCATED_BODY_SUFFIX)).toBe(true);
});
});
describe("createFetchTracker — challenge body cancel rejection (#2297)", () => {
it("swallows the source's cancel() rejection after stopping early", async () => {
const chunk = new Uint8Array(CHALLENGE_BODY_MAX_BYTES).fill(97);
const stream = new ReadableStream<Uint8Array>({
pull(controller) {
controller.enqueue(chunk);
},
cancel() {
throw new Error("cancel failed");
},
});
const tracked: FetchRequestEntryBase[] = [];
const fetcher = createFetchTracker(
(async () => new Response(stream, { status: 401 })) as typeof fetch,
{ trackRequest: (entry) => tracked.push(entry) },
);
const res = await fetcher("https://mcp.example/mcp", { method: "POST" });
// Cancelling the caller's branch cancels the source, whose rejection then
// reaches the tracker's own cancel of its clone — which must swallow it.
await res.body?.cancel().catch(() => undefined);
await flush();
expect(tracked[0]?.responseStatus).toBe(401);
expect(tracked[0]?.responseBody?.endsWith(TRUNCATED_BODY_SUFFIX)).toBe(
true,
);
});
});
describe("redactUrlQuery", () => {
it("redacts sensitive params and keeps the path + other params", () => {
expect(

View file

@ -271,6 +271,90 @@ describe("Transport", () => {
}
});
it("records an intercepted 401's status, WWW-Authenticate and body (#2297)", async () => {
const challengeBody = JSON.stringify({
error: "invalid_token",
error_description: "token expired",
});
const wwwAuthenticate =
'Bearer resource_metadata="https://mcp.example/.well-known/oauth-protected-resource", scope="files:read"';
const fetchFn: typeof fetch = async () =>
new Response(challengeBody, {
status: 401,
statusText: "Unauthorized",
headers: {
"content-type": "application/json",
"www-authenticate": wwwAuthenticate,
},
});
const fetchRequests: FetchRequestEntryBase[] = [];
const onFetchResponseBody = vi.fn();
const result = createTransportNode(
{ type: "streamable-http", url: "https://mcp.example/mcp" },
{
fetchFn,
interceptAuthChallenges: true,
onFetchRequest: (entry) => fetchRequests.push(entry),
onFetchResponseBody,
},
);
const client = new Client(
{ name: "test-client", version: "1.0.0" },
{ capabilities: {} },
);
await expect(client.connect(result.transport)).rejects.toThrow(
"MCP auth challenge (401)",
);
// The body is on the entry by the time the challenge is thrown — not a
// later update — so a caller that stops listening at the throw has it.
expect(fetchRequests).toHaveLength(1);
const [entry] = fetchRequests;
expect(entry.error).toBeUndefined();
expect(entry.responseStatus).toBe(401);
expect(entry.responseHeaders?.["www-authenticate"]).toBe(wwwAuthenticate);
expect(entry.responseBody).toBe(challengeBody);
expect(onFetchResponseBody).not.toHaveBeenCalled();
});
it("still throws an intercepted 401 whose body never ends (#2297)", async () => {
const fetchFn: typeof fetch = async () =>
new Response(
new ReadableStream<Uint8Array>({
start(controller) {
controller.enqueue(new TextEncoder().encode("partial"));
},
}),
{
status: 401,
headers: { "www-authenticate": 'Bearer error="invalid_token"' },
},
);
const fetchRequests: FetchRequestEntryBase[] = [];
const result = createTransportNode(
{ type: "streamable-http", url: "https://mcp.example/mcp" },
{
fetchFn,
interceptAuthChallenges: true,
onFetchRequest: (entry) => fetchRequests.push(entry),
},
);
const client = new Client(
{ name: "test-client", version: "1.0.0" },
{ capabilities: {} },
);
await expect(client.connect(result.transport)).rejects.toThrow(
"MCP auth challenge (401)",
);
expect(fetchRequests[0]?.responseStatus).toBe(401);
// Cut off at the tracker's read deadline rather than buffered forever.
expect(fetchRequests[0]?.responseBody).toBe("partial\n[truncated]");
});
it("applies settings.headers to the outgoing streamable-http request", async () => {
const server = createTestServerHttp({
serverInfo: createTestServerInfo(),

View file

@ -193,6 +193,69 @@ describe("server.ts /api/mcp/* branch coverage", () => {
await new Promise<void>((r) => upstream.server.close(() => r()));
}
});
it("relays a streamable-http 401's status and body over the event stream (#2297)", async () => {
const upstream = await startUnauthorizedUpstream();
const events = new AbortController();
try {
const connected = (await (
await connect(h, { type: "streamable-http", url: upstream.url })
).json()) as { sessionId?: string };
expect(connected.sessionId).toBeDefined();
const stream = await fetch(
`${h.baseUrl}/api/mcp/events?sessionId=${connected.sessionId}`,
{ signal: events.signal },
);
const sent = await fetch(`${h.baseUrl}/api/mcp/send`, {
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({
sessionId: connected.sessionId,
message: {
jsonrpc: "2.0",
id: 1,
method: "initialize",
params: {
protocolVersion: "2025-06-18",
capabilities: {},
clientInfo: { name: "test", version: "1.0.0" },
},
},
}),
});
expect(await sent.json()).toMatchObject({
ok: false,
kind: "auth_challenge",
});
const reader = stream.body!.getReader();
const decoder = new TextDecoder();
const pattern = /data: (\{"type":"fetch_request",.*?\})\n/;
let text = "";
let match: RegExpMatchArray | null = null;
while (!(match = text.match(pattern))) {
const { done, value } = await reader.read();
if (done) break;
text += decoder.decode(value, { stream: true });
}
const event = JSON.parse(match![1]!) as {
data: {
responseStatus?: number;
responseBody?: string;
error?: string;
};
};
expect(event.data.error).toBeUndefined();
expect(event.data.responseStatus).toBe(401);
expect(event.data.responseBody).toBe(
JSON.stringify({ error: "unauthorized" }),
);
} finally {
events.abort();
await new Promise<void>((r) => upstream.server.close(() => r()));
}
});
});
describe("POST /api/mcp/send", () => {

View file

@ -150,6 +150,14 @@ function redactJsonValue(value: unknown): unknown {
* (and other binary/opaque bodies) are passed through verbatim — OAuth never uses
* multipart, so the risk is low; revisit if a multipart secret path appears.
*/
export function redactBody(
body: string,
contentType: string | null | undefined,
): string;
export function redactBody(
body: string | undefined,
contentType: string | null | undefined,
): string | undefined;
export function redactBody(
body: string | undefined,
contentType: string | null | undefined,
@ -232,6 +240,98 @@ export function isLongLivedStreamResponse(
);
}
/** Most of a 401/403 body the tracker will buffer before it stops reading. */
export const CHALLENGE_BODY_MAX_BYTES = 64 * 1024;
/** How long the tracker waits for a 401/403 body before giving up on the rest. */
export const CHALLENGE_BODY_READ_MS = 2_000;
/** Appended to a 401/403 body the tracker stopped reading early. */
export const TRUNCATED_BODY_SUFFIX = "\n[truncated]";
/**
* Recorded in place of a 401/403 body that was cut off and looks like JSON.
* {@link redactBody} can only mask fields in JSON that parses, so a cut-off
* prefix such as `{"access_token":"…",` would otherwise reach the log verbatim.
*/
export const WITHHELD_TRUNCATED_JSON_BODY =
"[truncated JSON body withheld: an incomplete body cannot be redacted]";
interface ChallengeBodyRead {
text: string;
truncated: boolean;
}
/**
* The recorded form of a 401/403 body: redacted, marked when cut off, and never
* a cut-off JSON prefix. Form-encoded bodies still redact when cut, since each
* surviving `key=value` pair is parsed on its own; plain text has nothing to
* redact either way.
*/
function recordableChallengeBody(
read: ChallengeBodyRead,
contentType: string | null,
): string {
if (!read.truncated) return redactBody(read.text, contentType);
const looksLikeJson =
(contentType ?? "").toLowerCase().includes("json") ||
/^\s*[{[]/.test(read.text);
if (looksLikeJson) return WITHHELD_TRUNCATED_JSON_BODY;
return redactBody(read.text, contentType) + TRUNCATED_BODY_SUFFIX;
}
/**
* Read a 401/403 response body from a clone, bounded by
* {@link CHALLENGE_BODY_MAX_BYTES} and {@link CHALLENGE_BODY_READ_MS}.
*
* Unlike every other status, a challenge body is read *before* the entry is
* recorded (#2297). An auth-challenge interceptor above the tracker cancels the
* response and throws as soon as the tracker returns, and on the web backend
* that ends the connect request — so a body delivered later, as a separate
* update, arrives after anything could still carry it to the log. The bound is
* what makes waiting safe: a body that never ends is cut off, and cancelling the
* clone's reader lets the interceptor's cancel of the original release the
* connection instead of leaving the clone draining it.
*/
async function readChallengeBody(
response: Response,
): Promise<ChallengeBodyRead | undefined> {
let reader: ReadableStreamDefaultReader<Uint8Array>;
try {
const body = response.clone().body;
/* v8 ignore next -- the caller checked response.body, and a clone of a response with a body has one */
if (!body) return undefined;
reader = body.getReader();
} catch {
return undefined;
}
const decoder = new TextDecoder();
let text = "";
let bytes = 0;
let timer: ReturnType<typeof setTimeout> | undefined;
const deadline = new Promise<"timeout">((resolve) => {
timer = setTimeout(() => resolve("timeout"), CHALLENGE_BODY_READ_MS);
});
try {
while (bytes < CHALLENGE_BODY_MAX_BYTES) {
const next = await Promise.race([reader.read(), deadline]);
if (next === "timeout") break;
if (next.done) return { text: text + decoder.decode(), truncated: false };
// A stream may hand back one chunk far larger than the budget, so the cap
// is applied to the bytes, not to the chunk count.
const chunk = next.value.subarray(0, CHALLENGE_BODY_MAX_BYTES - bytes);
bytes += chunk.byteLength;
text += decoder.decode(chunk, { stream: true });
}
// Stopped early. Not awaited: cancel() adopts the source's promise, which a
// stalled body may never settle.
reader.cancel().catch(() => {});
return { text: text + decoder.decode(), truncated: true };
} catch {
return undefined;
} finally {
clearTimeout(timer);
}
}
export interface FetchTrackingCallbacks {
trackRequest?: (entry: FetchRequestEntryBase) => void;
/**
@ -578,6 +678,22 @@ export function createFetchTracker(
duration,
};
// Not gated on isLongLivedStream: that exclusion guards an unbounded read,
// and this one is bounded — so a challenge to an SSE GET is recorded too.
if (
(responseStatus === 401 || responseStatus === 403) &&
response.body &&
!response.bodyUsed
) {
const read = await readChallengeBody(response);
entry.responseBody =
read === undefined
? undefined
: recordableChallengeBody(read, response.headers.get("content-type"));
callbacks.trackRequest?.(entry);
return response;
}
callbacks.trackRequest?.(entry);
// A long-lived stream is watched instead of read: its body never ends,

View file

@ -24,8 +24,11 @@ export function createAuthChallengeInterceptFetch(
}
// Release the connection before throwing so the SDK transport is not left
// with a half-read 401/403 body on streamable HTTP.
await response.body?.cancel().catch(() => {});
// with a half-read 401/403 body on streamable HTTP. Not awaited: when the
// network tracker sits below this wrapper (#2297) the body is one branch of
// a tee, and cancel() adopts the source's promise, which a stalled body may
// never settle — so awaiting it could hang the throw.
void response.body?.cancel().catch(() => {});
throw new AuthChallengeError(
challenge,

View file

@ -74,12 +74,29 @@ export function createTransportNode(
onAuthChallengeObserved
? createAuthChallengeObserverFetch(inner, onAuthChallengeObserved)
: inner;
const withTracking = (inner: typeof fetch): typeof fetch =>
wantsFetchTracking
? createFetchTracker(inner, {
trackRequest: onFetchRequest,
updateResponseBody: onFetchResponseBody,
updateStream: onFetchStreamUpdate,
})
: inner;
// The observer sits *under* the interceptor so it still reports the
// challenge on the path where interception throws — and, more to the point,
// on the legacy first-auth path where interception is off entirely.
//
// The network tracker sits under the interceptor too (#2297). Above it, the
// tracker never saw the 401/403 at all: the interceptor cancels the body and
// throws, so the entry was recorded from the thrown error alone — no status,
// no `WWW-Authenticate`, and no response body, which is exactly the detail
// someone debugging an OAuth challenge needs. Below it, the tracker records
// the real response and reads its body (bounded) before the interceptor
// cancels it.
const trackedBase = withTracking(withChallengeObserver(baseFetch));
const fetchWithOptionalAuthIntercept = interceptAuthChallenges
? createAuthChallengeInterceptFetch(withChallengeObserver(baseFetch))
: withChallengeObserver(baseFetch);
? createAuthChallengeInterceptFetch(trackedBase)
: trackedBase;
if (serverType === "stdio") {
const stdioConfig = config as StdioServerConfig;
@ -118,16 +135,9 @@ export function createTransportNode(
const configuredSseFetch = sseConfig.eventSourceInit?.fetch as
| typeof fetch
| undefined;
const sseFetch = configuredSseFetch
? withChallengeObserver(configuredSseFetch)
const trackedFetch = configuredSseFetch
? withTracking(withChallengeObserver(configuredSseFetch))
: fetchWithOptionalAuthIntercept;
const trackedFetch = wantsFetchTracking
? createFetchTracker(sseFetch, {
trackRequest: onFetchRequest,
updateResponseBody: onFetchResponseBody,
updateStream: onFetchStreamUpdate,
})
: sseFetch;
const headers = headersFromSettings(settings);
@ -142,19 +152,11 @@ export function createTransportNode(
...(headers && { headers }),
};
const postFetch = wantsFetchTracking
? createFetchTracker(fetchWithOptionalAuthIntercept, {
trackRequest: onFetchRequest,
updateResponseBody: onFetchResponseBody,
updateStream: onFetchStreamUpdate,
})
: fetchWithOptionalAuthIntercept;
const transport = new SSEClientTransport(url, {
authProvider,
eventSourceInit,
requestInit,
fetch: postFetch,
fetch: fetchWithOptionalAuthIntercept,
});
return { transport };
@ -170,18 +172,10 @@ export function createTransportNode(
...(headers && { headers }),
};
const transportFetch = wantsFetchTracking
? createFetchTracker(fetchWithOptionalAuthIntercept, {
trackRequest: onFetchRequest,
updateResponseBody: onFetchResponseBody,
updateStream: onFetchStreamUpdate,
})
: fetchWithOptionalAuthIntercept;
const transport = new StreamableHTTPClientTransport(url, {
authProvider,
requestInit,
fetch: transportFetch,
fetch: fetchWithOptionalAuthIntercept,
// SEP-2350: how the transport reacts to a `403 insufficient_scope`
// challenge. Defaults to the SDK's `reauthorize` when unset.
...(settings?.oauthOnInsufficientScope && {