Skip to content

Commit a22fe96

Browse files
fix(client): treat HTTP 401/403 on the negotiation probe as auth failures, not legacy evidence
A 401 (without an authProvider) or 403 rejection of the connect-time server/discover probe fell into the classifier's conservative legacy fallback: auto mode sent a doomed legacy initialize, and pin mode reported "the server did not offer pinned protocol version ..." for a server that was never asked. Auth status is not era evidence. classifyHttpError now has an explicit 401/403 row ahead of the JSON-RPC body parse: a typed SdkHttpError (EraNegotiationFailed) carrying the HTTP status, reason phrase, and response text — never a legacy verdict. The probe's send-error normalization also propagates the transport auth flow's own typed failures unchanged (UnauthorizedError for finishAuth, InsufficientScopeError, the 401-after-re-authentication diagnostic) instead of flattening them into the generic HTTP row. Fixes #2561
1 parent f130e1a commit a22fe96

7 files changed

Lines changed: 261 additions & 20 deletions

File tree

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
'@modelcontextprotocol/client': patch
3+
---
4+
5+
The version-negotiation probe no longer misclassifies auth-protected servers
6+
(HTTP 401/403) as legacy. A 401 or 403 rejection of the `server/discover`
7+
probe now surfaces as a typed authorization failure — an `SdkHttpError` with
8+
code `EraNegotiationFailed` carrying the HTTP status, reason phrase, and
9+
response text — instead of triggering the legacy `initialize` fallback (which
10+
put a doomed `initialize` on the wire) or, under `pin` mode, the false
11+
"server did not offer pinned protocol version" diagnostic. With an
12+
`authProvider` the transport's auth flow still runs first and its outcome now
13+
always propagates unchanged: `UnauthorizedError` for `finishAuth()` as
14+
before, and the flow's own typed failures (`InsufficientScopeError`, the
15+
401-after-re-authentication diagnostic) instead of a legacy verdict or a
16+
generic negotiation wrap.

docs/protocol-versions.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,8 @@ const cli = new Client(
101101

102102
A probe timeout is transport-aware. On stdio a silent server is a legacy server, so `connect()` falls back to `initialize`; on HTTP silence is an outage, so `connect()` rejects with `SdkError(RequestTimeout)` instead of misreporting a dead server as legacy. One browser exception: an opaque CORS `TypeError` during the probe falls back to the legacy era, because deployed 2025 servers commonly have allow-lists that predate the 2026 headers.
103103

104+
Auth statuses are not era evidence either. An HTTP `401` or `403` rejecting the probe surfaces as a typed authorization failure, never the legacy fallback: with no `authProvider`, `connect()` rejects with an `SdkHttpError(EraNegotiationFailed)` naming the status; with one, the transport's auth flow runs first and its outcome propagates unchanged — `UnauthorizedError` for `finishAuth()`, or the flow's own typed failure (an `InsufficientScopeError`, the 401-after-re-authentication diagnostic).
105+
104106
On the SDK's own stdio transport (exactly `StdioClientTransport` — subclasses, like custom stdio-shaped transports, probe in place) the probe runs on a short-lived **sibling process** spawned from the same parameters — some stdio servers exit on any pre-`initialize` request (servers built on the official Rust SDK, rmcp, behave this way), so the probe must not spend the caller's one child process. The sibling is invisible infrastructure: its stderr is discarded and it is reaped once the era is known; the caller's transport spawns exactly once, afterwards, and its wire never carries `server/discover`. A child that exits on the probe is simply a legacy server (its exit must close the child's stdio pipes to register — an exit hidden behind a helper process holding them open falls to the probe-timeout path). Closing the caller's transport during the probe aborts `connect()` with a typed `SdkError(EraNegotiationFailed)` and the session child is never spawned. On HTTP — and on custom stdio-shaped transports, which probe in place — a mid-probe connection close rejects with the same typed error as any probe transport failure.
105107

106108
The client's `supportedProtocolVersions` option shapes the probe: its 2026+ entries are the versions the probe offers, and the legacy fallback stays available only while the list keeps a pre-2026 entry. A list with no pre-2026 entry removes the fallback — against a 2025-only server, `connect()` rejects with `SdkError(EraNegotiationFailed)`.

packages/client/src/client/probeClassifier.ts

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,9 @@
77
* sibling), or a typed connect error.
88
*
99
* The classifier is deliberately conservative: anything it does not positively
10-
* recognize as modern resolves to the legacy fallback, and a network outage is a
11-
* typed connect error, never an era verdict. The verdicts apply to the
10+
* recognize as modern resolves to the legacy fallback, and a network outage or
11+
* an auth-status rejection (HTTP 401/403) is a typed connect error, never an
12+
* era verdict. The verdicts apply to the
1213
* negotiation phase only — an established modern connection is never silently
1314
* demoted to `initialize` by a later failure.
1415
*/
@@ -19,6 +20,7 @@ import {
1920
modernProtocolVersions,
2021
SdkError,
2122
SdkErrorCode,
23+
SdkHttpError,
2224
UnsupportedProtocolVersionError
2325
} from '@modelcontextprotocol/core-internal';
2426

@@ -46,10 +48,10 @@ export type ProbeOutcome =
4648
| { kind: 'result'; result: unknown }
4749
/** Answered with a JSON-RPC error (any HTTP status, including 200-bodied errors and stdio in-band errors). */
4850
| { kind: 'rpc-error'; code: number; message: string; data?: unknown }
49-
/** The HTTP layer rejected the probe POST (non-2xx); `body` is the raw response text, when available. */
50-
| { kind: 'http-error'; status: number; body?: string }
51+
/** The HTTP layer rejected the probe POST (non-2xx); `body` is the raw response text and `statusText` the HTTP reason phrase, when available. */
52+
| { kind: 'http-error'; status: number; body?: string; statusText?: string }
5153
| { kind: 'network-error'; error: unknown }
52-
/** The transport's auth flow challenged during the probe send (`UnauthorizedError`). */
54+
/** The transport's auth flow challenged or failed during the probe send (`UnauthorizedError`, an `OAuthClientFlowError`, or the transport's own re-auth/step-up diagnostics) — `error` propagates unchanged. */
5355
| { kind: 'auth-required'; error: Error }
5456
/** The transport reported close while the probe awaited its reply. */
5557
| { kind: 'closed' }
@@ -241,7 +243,24 @@ function classifyRpcError(outcome: { code: number; message: string; data?: unkno
241243
return { kind: 'legacy' };
242244
}
243245

244-
function classifyHttpError(outcome: { status: number; body?: string }, context: ProbeClassifierContext): ProbeVerdict {
246+
function classifyHttpError(outcome: { status: number; body?: string; statusText?: string }, context: ProbeClassifierContext): ProbeVerdict {
247+
// Auth statuses are never era evidence, and a 401/403 body is the auth
248+
// layer's, not the server/discover handler's — this row ranks above the
249+
// JSON-RPC body parse. Parallel to the auth-required row: a typed failure
250+
// naming the status (response text on `data.text`), never a legacy
251+
// fallback. This matches the non-probing legacy-mode client, which
252+
// surfaces the same server as an `SdkHttpError` carrying this status.
253+
if (outcome.status === 401 || outcome.status === 403) {
254+
const detail = outcome.status === 401 ? 'the server requires authorization (HTTP 401)' : 'the server denied access (HTTP 403)';
255+
return {
256+
kind: 'error',
257+
error: new SdkHttpError(SdkErrorCode.EraNegotiationFailed, `Version negotiation failed: ${detail}`, {
258+
status: outcome.status,
259+
statusText: outcome.statusText,
260+
text: outcome.body
261+
})
262+
};
263+
}
245264
// HTTP-rejected probes carry their JSON-RPC error in the response body — classify it like an in-band error.
246265
const rpcError = parseJsonRpcErrorBody(outcome.body);
247266
if (rpcError !== undefined) {

packages/client/src/client/versionNegotiation.ts

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
} from '@modelcontextprotocol/core-internal';
2727

2828
import { UnauthorizedError } from './auth';
29+
import { OAuthClientFlowError } from './authErrors';
2930
import type { ProbeEnvironment, ProbeOutcome, ProbeTransportKind, ProbeVerdict } from './probeClassifier';
3031
import { classifyProbeOutcome } from './probeClassifier';
3132

@@ -381,23 +382,38 @@ function normalizeReply(reply: RawProbeReply, timeoutMs: number): ProbeOutcome {
381382
}
382383
case 'send-error': {
383384
const error = reply.error;
384-
if (error instanceof SdkHttpError) {
385-
const text = (error.data as { text?: unknown } | undefined)?.text;
386-
return { kind: 'http-error', status: error.data.status, body: typeof text === 'string' ? text : undefined };
387-
}
388-
const isUnauthorized =
385+
const isAuthFlowError =
389386
error instanceof UnauthorizedError ||
390387
// Name fallback for auth errors the brand cannot reach: an
391388
// UnauthorizedError from a differently bundled SDK copy at a
392389
// skewed version, or an auth middleware's own class.
393-
(error instanceof Error && error.name === 'UnauthorizedError');
394-
if (isUnauthorized) {
395-
// Auth-gated server. (The pre-branding name-string check alone
396-
// could never fire for the SDK's own class — it did not set
397-
// `.name` — so these send failures fell through to the generic
398-
// network-error wrap.)
390+
(error instanceof Error && error.name === 'UnauthorizedError') ||
391+
// The transport's OAuth flow failing with its own typed error
392+
// (e.g. InsufficientScopeError when a 403 insufficient_scope
393+
// challenge has no provider to drive step-up).
394+
error instanceof OAuthClientFlowError ||
395+
// The transport's auth flow exhausting itself: 401 after the
396+
// onUnauthorized re-auth retry, 403 after the step-up retry
397+
// limit. Flattening these to the http-error row would swallow
398+
// their codes and diagnostics.
399+
(error instanceof SdkHttpError &&
400+
(error.code === SdkErrorCode.ClientHttpAuthentication || error.code === SdkErrorCode.ClientHttpForbidden));
401+
if (isAuthFlowError) {
402+
// Auth-gated server: propagate unchanged. (The pre-branding
403+
// name-string check alone could never fire for the SDK's own
404+
// class — it did not set `.name` — so these send failures fell
405+
// through to the generic network-error wrap.)
399406
return { kind: 'auth-required', error: error as Error };
400407
}
408+
if (error instanceof SdkHttpError) {
409+
const text = (error.data as { text?: unknown } | undefined)?.text;
410+
return {
411+
kind: 'http-error',
412+
status: error.data.status,
413+
body: typeof text === 'string' ? text : undefined,
414+
statusText: error.data.statusText
415+
};
416+
}
401417
return { kind: 'network-error', error };
402418
}
403419
case 'closed': {

packages/client/test/client/probeFixtureCorpus.test.ts

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
* probe wire shape (string id, `server/discover` first, never a real request).
2020
*/
2121
import type { JSONRPCMessage, Transport } from '@modelcontextprotocol/core-internal';
22-
import { LATEST_PROTOCOL_VERSION, PROTOCOL_VERSION_META_KEY } from '@modelcontextprotocol/core-internal';
22+
import { LATEST_PROTOCOL_VERSION, PROTOCOL_VERSION_META_KEY, SdkErrorCode, SdkHttpError } from '@modelcontextprotocol/core-internal';
2323
import { describe, expect, it } from 'vitest';
2424

2525
import { Client } from '../../src/client/client';
@@ -161,6 +161,29 @@ const CORPUS: CorpusRow[] = [
161161
outcome: { kind: 'result', result: { content: [{ type: 'text', text: `supportedVersions: ["${MODERN}"]` }] } },
162162
expected: 'legacy'
163163
},
164+
// --- Auth statuses are never era evidence (#2561): an auth-protected
165+
// server is not a legacy server, whatever the body says — typed failure,
166+
// never initialize (fallbackAvailable is true in every row here).
167+
{
168+
name: 'auth: HTTP 401 challenge (WWW-Authenticate rides the header; body carries the OAuth error JSON) → typed auth failure, never legacy',
169+
outcome: { kind: 'http-error', status: 401, body: '{"error":"invalid_token","error_description":"Missing bearer token"}' },
170+
expected: 'error'
171+
},
172+
{
173+
name: 'auth: bare HTTP 401 (no body) → typed auth failure, never legacy',
174+
outcome: { kind: 'http-error', status: 401 },
175+
expected: 'error'
176+
},
177+
{
178+
name: 'auth: bare HTTP 403 denial (no body) → typed auth failure, never legacy',
179+
outcome: { kind: 'http-error', status: 403 },
180+
expected: 'error'
181+
},
182+
{
183+
name: 'auth: a 401 whose body parses as a JSON-RPC error is still an auth failure — the auth layer wrote that body, not server/discover',
184+
outcome: { kind: 'http-error', status: 401, body: DEPLOYED_SESSION_REQUIRED_BODY },
185+
expected: 'error'
186+
},
164187
// --- Q12 transport-aware timeout rows (stdio falls back, HTTP stays a typed error).
165188
{
166189
name: 'timeout on stdio → legacy fallback (the stdio backward-compatibility rule)',
@@ -209,6 +232,25 @@ describe('T9/T11 merged probe fixture corpus (probe classifier)', () => {
209232
});
210233
}
211234

235+
it('the 401/403 typed failures name the auth status and carry it with the response text and reason phrase', () => {
236+
for (const [status, statusText, body] of [
237+
[401, 'Unauthorized', '{"error":"invalid_token"}'],
238+
[403, 'Forbidden', 'nope']
239+
] as const) {
240+
const verdict = classifyProbeOutcome({ kind: 'http-error', status, statusText, body }, baseContext);
241+
expect(verdict.kind).toBe('error');
242+
if (verdict.kind === 'error') {
243+
expect(verdict.error).toBeInstanceOf(SdkHttpError);
244+
const error = verdict.error as SdkHttpError;
245+
expect(error.code).toBe(SdkErrorCode.EraNegotiationFailed);
246+
expect(error.status).toBe(status);
247+
expect(error.statusText).toBe(statusText);
248+
expect(error.data.text).toBe(body);
249+
expect(error.message).toContain(String(status));
250+
}
251+
}
252+
});
253+
212254
it('a DiscoverResult with a mutual version is the only result shape that yields a modern verdict', () => {
213255
const verdict = classifyProbeOutcome(
214256
{

packages/client/test/client/versionNegotiation.test.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,13 @@ import {
1414
PROTOCOL_VERSION_META_KEY,
1515
SdkError,
1616
SdkErrorCode,
17+
SdkHttpError,
1718
UnsupportedProtocolVersionError
1819
} from '@modelcontextprotocol/core-internal';
1920
import { describe, expect, test } from 'vitest';
2021

2122
import { UnauthorizedError } from '../../src/client/auth';
23+
import { InsufficientScopeError } from '../../src/client/authErrors';
2224
import { Client } from '../../src/client/client';
2325
import type { StreamableHTTPClientTransportOptions } from '../../src/client/streamableHttp';
2426
import type { StdioServerParameters } from '../../src/client/stdio';
@@ -1103,6 +1105,41 @@ describe('probe send-error classification', () => {
11031105
expect(requests(transport.sent).some(r => r.method === 'initialize')).toBe(false);
11041106
});
11051107

1108+
test("the transport's post-re-auth 401 (SdkHttpError ClientHttpAuthentication) propagates unchanged — not flattened to the http-error row", async () => {
1109+
const reason = new SdkHttpError(SdkErrorCode.ClientHttpAuthentication, 'Server returned 401 after re-authentication', {
1110+
status: 401,
1111+
statusText: 'Unauthorized'
1112+
});
1113+
const transport = new AuthGatedTransport(reason);
1114+
const client = new Client({ name: 'c', version: '0' }, { versionNegotiation: { mode: 'auto' } });
1115+
1116+
const rejection = await client.connect(transport).then(
1117+
() => {
1118+
throw new Error('connect unexpectedly resolved');
1119+
},
1120+
(e: unknown) => e
1121+
);
1122+
// The same object, code and diagnostic intact — rebuilding it from the
1123+
// status alone would swallow the re-auth context.
1124+
expect(rejection).toBe(reason);
1125+
expect(requests(transport.sent).some(r => r.method === 'initialize')).toBe(false);
1126+
});
1127+
1128+
test('an OAuth flow error (InsufficientScopeError from a step-up challenge with no provider) propagates unchanged — no fallback, no network-error wrap', async () => {
1129+
const reason = new InsufficientScopeError({ requiredScope: 'mcp:tools' });
1130+
const transport = new AuthGatedTransport(reason);
1131+
const client = new Client({ name: 'c', version: '0' }, { versionNegotiation: { mode: 'auto' } });
1132+
1133+
const rejection = await client.connect(transport).then(
1134+
() => {
1135+
throw new Error('connect unexpectedly resolved');
1136+
},
1137+
(e: unknown) => e
1138+
);
1139+
expect(rejection).toBe(reason);
1140+
expect(requests(transport.sent).some(r => r.method === 'initialize')).toBe(false);
1141+
});
1142+
11061143
test('a plain send failure stays a typed negotiation error — no fallback runs', async () => {
11071144
const transport = new AuthGatedTransport(new Error('connection refused'));
11081145
const client = new Client({ name: 'c', version: '0' }, { versionNegotiation: { mode: 'auto' } });

0 commit comments

Comments
 (0)