diff --git a/.changeset/probe-auth-status-not-era-evidence.md b/.changeset/probe-auth-status-not-era-evidence.md new file mode 100644 index 0000000000..7f2f75d42e --- /dev/null +++ b/.changeset/probe-auth-status-not-era-evidence.md @@ -0,0 +1,28 @@ +--- +'@modelcontextprotocol/client': patch +--- + +The version-negotiation probe no longer misclassifies auth-protected or +failing servers as legacy. Auth status is never era evidence: a 401 or 403 +rejection of the `server/discover` probe now surfaces as a typed +authorization failure — an `SdkHttpError` with code `ClientHttpAuthentication` +(401) or `ClientHttpForbidden` (403), carrying the HTTP status, reason +phrase, and response text — instead of triggering the legacy `initialize` +fallback (which put a doomed `initialize` on the wire) or, under `pin` mode, +the false "server did not offer pinned protocol version" diagnostic. The +codes are deliberately not `EraNegotiationFailed`, so era-recovery flows +keyed on that code cannot persist a verdict for an unauthorized exchange. A +5xx rejecting the probe is a server failure and now also rejects typed +(`SdkHttpError(EraNegotiationFailed)`) instead of demoting a mid-deploy +modern server to legacy — the legacy fallback now fires only on the 4xx +shapes the spec licenses. + +With an `authProvider`, a `401` (and a `403` `insufficient_scope` challenge) runs the transport's auth flow first — a plain `403` rejects the same as without a provider — and whatever +escapes it propagates unchanged, identity intact: the HTTP transports stamp +errors at their auth seams (the `token()` read, `onUnauthorized` including +custom callbacks, the 403 step-up flow, and their own auth-failure +constructions), so `UnauthorizedError` for `finishAuth()`, the flow's typed +failures (`OAuthError`, `InsufficientScopeError`, the +401-after-re-authentication diagnostic), and even an untyped `TypeError` +thrown inside the flow all reach the caller as thrown — never rewrapped, +never consumed by the probe's browser CORS heuristic as legacy-era evidence. diff --git a/docs/advanced/gateway.md b/docs/advanced/gateway.md index 708b7278dc..1aad441dc3 100644 --- a/docs/advanced/gateway.md +++ b/docs/advanced/gateway.md @@ -151,6 +151,8 @@ re-probed: 2026-07-28 Replace the persisted blob with the fresh `getDiscoverResult()` and the rest of the fleet recovers on its next read. +The `EraNegotiationFailed` filter above is deliberate and safe against auth walls: a `401`/`403` rejecting the probe carries `ClientHttpAuthentication`/`ClientHttpForbidden` instead, so an unauthorized exchange re-throws out of this recovery path and can never be persisted as an era verdict. + ## Skip the probe for a known-legacy server When out-of-band metadata already says the server is pre-2026 — a registry entry, an earlier connection's outcome — an `'auto'`-mode probe is a round trip that fails on every single connect. Supply the negative verdict instead: `PriorDiscovery`'s `{ kind: 'legacy' }` arm skips the probe and goes straight to the `initialize` handshake. diff --git a/docs/clients/oauth.md b/docs/clients/oauth.md index f30dcd9258..f713c2e04d 100644 --- a/docs/clients/oauth.md +++ b/docs/clients/oauth.md @@ -29,7 +29,7 @@ try { When the server requires authorization and the provider has no token, the SDK runs discovery against the server, registers (or looks up) your OAuth client, calls the provider's `redirectToAuthorization(url)`, and `connect()` throws `UnauthorizedError`. The end user finishes signing in out of band; your callback endpoint picks the flow back up below. ::: info -With protocol-version negotiation in play, the connect-time 401 can also surface as an `SdkError` carrying the `UnauthorizedError` at `error.data.cause` — see [Protocol versions](../protocol-versions.md). +With protocol-version negotiation in play (`versionNegotiation: { mode: 'auto' }` or a pin), the connect-time `UnauthorizedError` propagates unchanged from `connect()` — the same `instanceof` check works in every mode (older releases wrapped it as an `SdkError` with the error at `error.data.cause`). See [Protocol versions](../protocol-versions.md). ::: ## Implement OAuthClientProvider diff --git a/docs/migration/support-2026-07-28.md b/docs/migration/support-2026-07-28.md index 4ac41a75f2..13e869c0f2 100644 --- a/docs/migration/support-2026-07-28.md +++ b/docs/migration/support-2026-07-28.md @@ -71,7 +71,15 @@ infrastructure problems. Anything the probe does not positively recognize as mod falls back to the legacy era — provided the supported-versions list still contains a 2025-era revision; with a modern-only list `connect()` rejects with `SdkError(EraNegotiationFailed)` instead. A network outage rejects with a typed connect -error. Probe timeouts are **transport-aware**: on **stdio** a server that does not +error. Auth statuses are another exception: an HTTP `401` or `403` rejecting the probe +is never era evidence — `connect()` rejects with a typed authorization failure (an +`SdkHttpError` with code `ClientHttpAuthentication`/`ClientHttpForbidden` naming the +status, or the transport auth flow's own error propagated unchanged) instead of +falling back — see [Protocol versions](../protocol-versions.md). This holds even for +a front that answers the probe `403` but would pass `initialize`: relying on a legacy +fallback there is a deliberate non-goal — fix the auth wall (or pass credentials), +because auth status never selects an era. A `5xx` on the probe is a server failure, +also never era evidence: `connect()` rejects with `SdkHttpError(EraNegotiationFailed)`. Probe timeouts are **transport-aware**: on **stdio** a server that does not answer within `timeoutMs` is treated as legacy and the client falls back to `initialize` (some legacy servers never respond to unknown pre-`initialize` requests at all); on **HTTP** a probe timeout rejects with `SdkError(RequestTimeout)` — diff --git a/docs/migration/upgrade-to-v2.md b/docs/migration/upgrade-to-v2.md index 95dd040a07..19f4127733 100644 --- a/docs/migration/upgrade-to-v2.md +++ b/docs/migration/upgrade-to-v2.md @@ -1150,11 +1150,11 @@ try { } ``` -One qualification: this direct `instanceof` check applies under the default `'legacy'` -version negotiation. Under the probing modes (`versionNegotiation: { mode: 'auto' }`, -with or without a pin) the connect-time 401 currently surfaces wrapped as -`SdkError(SdkErrorCode.EraNegotiationFailed)` with the `UnauthorizedError` at -`error.data.cause` — unwrap before the check, as shown in the +This direct `instanceof` check works in every version-negotiation mode: under the +probing modes (`versionNegotiation: { mode: 'auto' }`, with or without a pin) the +connect-time `UnauthorizedError` also propagates unchanged from `connect()`. Older +releases wrapped it as `SdkError(SdkErrorCode.EraNegotiationFailed)` with the error at +`error.data.cause` — that unwrap is no longer needed. See the [client OAuth guide](../clients/oauth.md). #### `auth()` options are now `AuthOptions` diff --git a/docs/protocol-versions.md b/docs/protocol-versions.md index a304f4b604..ba1338b0d2 100644 --- a/docs/protocol-versions.md +++ b/docs/protocol-versions.md @@ -101,6 +101,8 @@ const cli = new Client( 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. +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 — and never as `EraNegotiationFailed`, so era-recovery flows keyed on that code (the [gateway guide](./advanced/gateway.md) recipe) cannot consume an auth wall. A plain `401` or `403` — with or without an `authProvider` — rejects `connect()` with an `SdkHttpError` carrying the status: code `ClientHttpAuthentication` for `401`, `ClientHttpForbidden` for `403`. One 403 shape is different: a `WWW-Authenticate` challenge with `error="insufficient_scope"` enters the Streamable HTTP transport's step-up flow regardless of provider, and with none it rejects with the flow's typed `InsufficientScopeError`. With a provider, a `401` (and a `403` `insufficient_scope` challenge) runs the auth flow first, and whatever escapes it reaches you as thrown, identity intact — the transport stamps errors at its auth seams (the `token()` read, `onUnauthorized` including custom callbacks, step-up), so `UnauthorizedError` for `finishAuth()`, the flow's typed failures (`OAuthError`, `InsufficientScopeError`, the 401-after-re-authentication diagnostic), and even an untyped crash inside a callback all propagate unchanged. A `5xx` rejecting the probe is a server failure, not era evidence: `connect()` rejects with `SdkHttpError(EraNegotiationFailed)` naming the status. These status-keyed rows read the transport's typed HTTP rejections — the SDK's Streamable HTTP transport surfaces them with the status attached; the legacy SSE client transport reports a non-2xx POST as a generic error, so over SSE a probe rejection surfaces as the generic `Version negotiation probe failed` connect error instead. Auth settles first, era second: a `401` never decides the era — the auth wall answers before the MCP layer ever sees `server/discover` — and the post-auth re-probe supplies the real era evidence. + 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. 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)`. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index bf96270c36..d9e6e7824d 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -75,6 +75,9 @@ With the global in place the [client OAuth](./clients/oauth.md) flows run unchan - `the connection closed during the server/discover probe (this transport probed in place — the disposable sibling probe requires the SDK's base StdioClientTransport)` — a subclass of `StdioClientTransport`, or a custom stdio-shaped transport, probed in place and met a server that exits on any pre-`initialize` request: use the base `StdioClientTransport` (which probes on a disposable sibling), or `mode: 'legacy'`. - `the transport was closed during the server/discover probe` — the caller closed the transport while the probe was in flight; the connect aborted deliberately and the session child was never spawned. - `Version negotiation probe failed: ...` — the probe hit a transport failure (network outage, HTTP connection drop): fix connectivity and retry. +- `the server answered the probe with HTTP 5xx` — the server or a proxy in front of it failed (mid-deploy, crashed backend); not era evidence, so no legacy fallback is attempted: retry once the deployment is healthy. + +A `401`/`403` probe rejection is **not** this code — see the next section. The pinned shape — `transport` here reaches a server still on the 2025 revisions ([Test a server](./testing.md) shows the in-memory wiring these outputs come from): @@ -112,6 +115,12 @@ legacy Keep `{ pin }` where a legacy connection is unacceptable and a hard failure is the behavior you want. [Protocol versions](./protocol-versions.md) defines the eras and what each negotiation mode offers. +## `Version negotiation failed: the server requires authorization (HTTP 401)` + +`SdkHttpError` with code `CLIENT_HTTP_AUTHENTICATION` (`error.status === 401`): the negotiation probe hit an auth wall and no `authProvider` is configured. Auth status is never protocol-era evidence, so `connect()` fails typed instead of guessing an era. Pass an `authProvider` — `{ token: async () => myApiKey }` for bearer tokens, or an `OAuthClientProvider` for OAuth flows (the connect-time `UnauthorizedError` → `finishAuth()` → reconnect cycle then works in every negotiation mode). + +The `403` sibling — `Version negotiation failed: the server denied access (HTTP 403)`, code `CLIENT_HTTP_FORBIDDEN` — means the server refused the request outright (IP allowlist, org policy, revoked key). Fix access; this is not a protocol mismatch. (A `403` whose `WWW-Authenticate` challenge carries `error="insufficient_scope"` instead rejects with `InsufficientScopeError` — request the challenged scope.) + ## `SdkError: METHOD_NOT_SUPPORTED_BY_PROTOCOL_VERSION` You sent a spec method the negotiated protocol era does not define. The SDK raises this locally — nothing reached the transport — and the message names the method, the negotiated revision, and the era-appropriate replacement. diff --git a/examples/cli-client/host/host.ts b/examples/cli-client/host/host.ts index 49165c9554..702f648032 100644 --- a/examples/cli-client/host/host.ts +++ b/examples/cli-client/host/host.ts @@ -17,7 +17,6 @@ import { Client, LOG_LEVEL_META_KEY, ProtocolError, - SdkError, StreamableHTTPClientTransport, SUPPORTED_PROTOCOL_VERSIONS, UnauthorizedError @@ -25,7 +24,6 @@ import { import { StdioClientTransport } from '@modelcontextprotocol/client/stdio'; import type { ChatMessage, ContentPart, GenerateResult, LLMProvider, ToolCall, ToolDefinition } from '../providers/provider'; -import { isRecord } from '../providers/provider'; import { completeAuthorizationWithBrowser, createOAuthProvider, findCallbackPort, isSafeBrowserUrl } from './auth'; import type { CliClientConfig, ServerConfig } from './config'; import { isHttpServer } from './config'; @@ -100,16 +98,6 @@ export function resolveVersionOptions(legacy: boolean, protocolVersion?: string) return { versionNegotiation: { mode: { pin: protocolVersion } } }; } -function unwrapUnauthorized(error: unknown): UnauthorizedError | undefined { - if (error instanceof UnauthorizedError) return error; - // Under versionNegotiation 'auto', a connect-time 401 surfaces as - // SdkError(EraNegotiationFailed) with the UnauthorizedError in error.data.cause. - if (error instanceof SdkError && isRecord(error.data) && error.data.cause instanceof UnauthorizedError) { - return error.data.cause; - } - return undefined; -} - function samplingContentToParts(content: CreateMessageRequest['params']['messages'][number]['content']): ContentPart[] { const blocks = Array.isArray(content) ? content : [content]; const parts: ContentPart[] = []; @@ -488,7 +476,9 @@ export class McpHost { try { await client.connect(httpTransport); } catch (error) { - if (!unwrapUnauthorized(error)) throw error; + // A connect-time 401 propagates UnauthorizedError unchanged + // in every negotiation mode, including the probing ones. + if (!(error instanceof UnauthorizedError)) throw error; const finishTransport = httpTransport; const authorized = await completeAuthorizationWithBrowser({ serverName: name, diff --git a/packages/client/src/client/authSeam.ts b/packages/client/src/client/authSeam.ts new file mode 100644 index 0000000000..fc93fd4522 --- /dev/null +++ b/packages/client/src/client/authSeam.ts @@ -0,0 +1,40 @@ +/** + * Auth-seam provenance stamp (internal — not part of the public API). + * + * Errors escaping a transport auth seam — the `authProvider.token()` read, + * an `onUnauthorized` invocation (SDK adapter or custom user callback), the + * 403 step-up flow, the transport's own auth-failure constructions — are + * stamped at the throw boundary. The version-negotiation probe routes stamped + * errors as auth outcomes; provenance is recorded where it is known, never + * reconstructed from error types downstream. + * + * `Symbol.for` uses the global symbol registry, so the stamp survives a + * duplicated SDK copy in one process (bundler double-install, version skew) + * by design: both copies resolve the same symbol. + */ +const AUTH_SEAM = Symbol.for('mcp.authSeamEscape'); + +/** + * Stamp `error` as an auth-seam escape and return it — identity-preserving + * (the same object flows on, `instanceof` and `.cause` chains intact). A + * frozen/sealed object is returned unstamped rather than replaced: identity + * outranks provenance. Primitive throws cannot carry the stamp. + */ +export function markAuthSeamEscape(error: T): T { + if ((typeof error === 'object' && error !== null) || typeof error === 'function') { + try { + Object.defineProperty(error, AUTH_SEAM, { value: true, configurable: true }); + } catch { + // Frozen/sealed: leave unstamped. + } + } + return error; +} + +/** Whether `error` escaped through a transport auth seam. */ +export function isAuthSeamEscape(error: unknown): boolean { + return ( + ((typeof error === 'object' && error !== null) || typeof error === 'function') && + (error as Record)[AUTH_SEAM] === true + ); +} diff --git a/packages/client/src/client/probeClassifier.ts b/packages/client/src/client/probeClassifier.ts index 863770acaf..e23d011a2a 100644 --- a/packages/client/src/client/probeClassifier.ts +++ b/packages/client/src/client/probeClassifier.ts @@ -7,8 +7,9 @@ * sibling), or a typed connect error. * * The classifier is deliberately conservative: anything it does not positively - * recognize as modern resolves to the legacy fallback, and a network outage is a - * typed connect error, never an era verdict. The verdicts apply to the + * recognize as modern resolves to the legacy fallback, and a network outage or + * an auth-status rejection (HTTP 401/403) is a typed connect error, never an + * era verdict. The verdicts apply to the * negotiation phase only — an established modern connection is never silently * demoted to `initialize` by a later failure. */ @@ -19,6 +20,7 @@ import { modernProtocolVersions, SdkError, SdkErrorCode, + SdkHttpError, UnsupportedProtocolVersionError } from '@modelcontextprotocol/core-internal'; @@ -46,10 +48,10 @@ export type ProbeOutcome = | { kind: 'result'; result: unknown } /** Answered with a JSON-RPC error (any HTTP status, including 200-bodied errors and stdio in-band errors). */ | { kind: 'rpc-error'; code: number; message: string; data?: unknown } - /** The HTTP layer rejected the probe POST (non-2xx); `body` is the raw response text, when available. */ - | { kind: 'http-error'; status: number; body?: string } + /** The HTTP layer rejected the probe POST (non-2xx); `body` is the raw response text and `statusText` the HTTP reason phrase, when available. */ + | { kind: 'http-error'; status: number; body?: string; statusText?: string } | { kind: 'network-error'; error: unknown } - /** The transport's auth flow challenged during the probe send (`UnauthorizedError`). */ + /** The transport's auth flow challenged or failed during the probe send — an error stamped at a transport auth seam, or an `UnauthorizedError` (the foreign-transport contract). `error` propagates unchanged. */ | { kind: 'auth-required'; error: Error } /** The transport reported close while the probe awaited its reply. */ | { kind: 'closed' } @@ -241,21 +243,70 @@ function classifyRpcError(outcome: { code: number; message: string; data?: unkno return { kind: 'legacy' }; } -function classifyHttpError(outcome: { status: number; body?: string }, context: ProbeClassifierContext): ProbeVerdict { +function classifyHttpError(outcome: { status: number; body?: string; statusText?: string }, context: ProbeClassifierContext): ProbeVerdict { + // Auth statuses are never era evidence, and a 401/403 body is the auth + // layer's, not the server/discover handler's — this row ranks above the + // JSON-RPC body parse. Parallel to the auth-required row: a typed failure + // naming the status (response text on `data.text`), never a legacy + // fallback. The codes are the auth ones, NOT EraNegotiationFailed: hosts + // key era-recovery recipes (e.g. the gateway guide's cached-verdict flow) + // on EraNegotiationFailed, and an auth wall must never enter era recovery + // — persisting a legacy verdict for it would recreate the silent-legacy + // bug at the fleet level. + if (outcome.status === 401 || outcome.status === 403) { + const isDenial = outcome.status === 403; + return { + kind: 'error', + error: new SdkHttpError( + isDenial ? SdkErrorCode.ClientHttpForbidden : SdkErrorCode.ClientHttpAuthentication, + `Version negotiation failed: ${isDenial ? 'the server denied access (HTTP 403)' : 'the server requires authorization (HTTP 401)'}`, + { + status: outcome.status, + statusText: outcome.statusText, + text: outcome.body + } + ) + }; + } + if (outcome.status >= 500) { + // A server failure is not era evidence, whatever the body says — this + // row too ranks above the JSON-RPC body parse (a 5xx body is the + // infrastructure's: a mid-deploy gateway's JSON error page, a crashed + // handler's -32603, neither a server/discover verdict). The spec keys + // the HTTP legacy signal to client-error rejections (a 2025 server + // answers the unknown probe 4xx), never to 5xx. + return { + kind: 'error', + error: new SdkHttpError( + SdkErrorCode.EraNegotiationFailed, + `Version negotiation failed: the server answered the probe with HTTP ${outcome.status}`, + { + status: outcome.status, + statusText: outcome.statusText, + text: outcome.body + } + ) + }; + } // HTTP-rejected probes carry their JSON-RPC error in the response body — classify it like an in-band error. const rpcError = parseJsonRpcErrorBody(outcome.body); if (rpcError !== undefined) { return classifyRpcError(rpcError, context); } - // Unparseable or unrecognized HTTP rejection: conservative legacy fallback. + // Unparseable or unrecognized 4xx rejection: conservative legacy fallback. + // With the auth and 5xx rows above, this row's legacy set now equals the + // spec-licensed set exactly — the 4xx a deployed 2025 server answers a + // request it does not recognize. return { kind: 'legacy' }; } function classifyNetworkError(error: unknown, context: ProbeClassifierContext): ProbeVerdict { if (context.environment === 'browser' && isOpaqueFetchTypeError(error)) { - // A browser CORS-preflight rejection against a deployed 2025 server is an - // opaque TypeError; the legacy fallback carries no custom headers (no - // preflight), so it can proceed where the probe could not. + // A browser CORS-preflight rejection against a deployed 2025 server is + // an opaque TypeError. The legacy fallback also preflights (its JSON + // Content-Type is non-simple), but carries only the pre-2026 headers a + // deployed server's CORS allowlist already covers — so it can proceed + // where the probe's 2026 headers could not. return { kind: 'legacy' }; } return { diff --git a/packages/client/src/client/sse.ts b/packages/client/src/client/sse.ts index 1c81928f10..0cc77d8f4f 100644 --- a/packages/client/src/client/sse.ts +++ b/packages/client/src/client/sse.ts @@ -23,6 +23,7 @@ import { } from './auth'; // eslint-disable-next-line @typescript-eslint/no-unused-vars -- referenced in JSDoc {@linkcode} import type { IssuerMismatchError } from './authErrors'; +import { markAuthSeamEscape } from './authSeam'; export class SseError extends Error { static { @@ -160,7 +161,14 @@ export class SSEClientTransport implements Transport { private async _commonHeaders(): Promise { const headers: RequestInit['headers'] & Record = {}; - const token = await this._authProvider?.token(); + let token: string | undefined; + try { + token = await this._authProvider?.token(); + } catch (error) { + // Auth-seam stamp: a throwing token() is an auth failure, never a + // network failure. + throw markAuthSeamEscape(error); + } if (token) { headers['Authorization'] = `Bearer ${token}`; } @@ -212,15 +220,18 @@ export class SSEClientTransport implements Transport { this._authProvider.onUnauthorized({ response, serverUrl: this._url, fetchFn: this._fetchWithInit }).then( // onUnauthorized succeeded → retry fresh. Its onerror handles its own onerror?.() + reject. () => this._startOrAuth().then(resolve, reject), - // onUnauthorized failed → not yet reported. - error => { - this.onerror?.(error); + // onUnauthorized failed → not yet reported. Auth-seam + // stamp: covers the SDK's OAuth flow and custom + // callbacks alike. + (error: unknown) => { + markAuthSeamEscape(error); + this.onerror?.(error as Error); reject(error); } ); return; } - const error = new UnauthorizedError(); + const error = markAuthSeamEscape(new UnauthorizedError()); reject(error); this.onerror?.(error); return; @@ -362,23 +373,31 @@ export class SSEClientTransport implements Transport { } if (this._authProvider.onUnauthorized && !isAuthRetry) { - await this._authProvider.onUnauthorized({ - response, - serverUrl: this._url, - fetchFn: this._fetchWithInit - }); + try { + await this._authProvider.onUnauthorized({ + response, + serverUrl: this._url, + fetchFn: this._fetchWithInit + }); + } catch (error) { + // Auth-seam stamp: covers the SDK's OAuth flow and + // custom onUnauthorized callbacks alike. + throw markAuthSeamEscape(error); + } await response.text?.().catch(() => {}); // Purposely _not_ awaited, so we don't call onerror twice return this._send(message, true); } await response.text?.().catch(() => {}); if (isAuthRetry) { - throw new SdkHttpError(SdkErrorCode.ClientHttpAuthentication, 'Server returned 401 after re-authentication', { - status: 401, - statusText: response.statusText - }); + throw markAuthSeamEscape( + new SdkHttpError(SdkErrorCode.ClientHttpAuthentication, 'Server returned 401 after re-authentication', { + status: 401, + statusText: response.statusText + }) + ); } - throw new UnauthorizedError(); + throw markAuthSeamEscape(new UnauthorizedError()); } const text = await response.text?.().catch(() => null); diff --git a/packages/client/src/client/streamableHttp.ts b/packages/client/src/client/streamableHttp.ts index 9067fd1ec4..ace0663158 100644 --- a/packages/client/src/client/streamableHttp.ts +++ b/packages/client/src/client/streamableHttp.ts @@ -34,10 +34,14 @@ import { // eslint-disable-next-line @typescript-eslint/no-unused-vars -- referenced via {@linkcode} in finishAuth JSDoc import type { IssuerMismatchError } from './authErrors'; import { InsufficientScopeError } from './authErrors'; +import { markAuthSeamEscape } from './authSeam'; /** Default cap on step-up re-authorization retries within a single send/stream-open. */ const DEFAULT_MAX_STEP_UP_RETRIES = 1; +/** The parsed 403 `insufficient_scope` challenge handed to the step-up flow. */ +type StepUpChallenge = { scope?: string; resourceMetadataUrl?: URL; errorDescription?: string; statusText?: string; text?: string | null }; + // Default reconnection options for StreamableHTTP connections const DEFAULT_STREAMABLE_HTTP_RECONNECTION_OPTIONS: StreamableHTTPReconnectionOptions = { initialReconnectionDelay: 1000, @@ -367,10 +371,18 @@ export class StreamableHTTPClientTransport implements Transport { * `_startOrAuthSse` path so both apply the same `'throw'` short-circuit, * the same superset-gated refresh bypass, and the same retry cap. */ - private async _stepUpAuthorize( - challenge: { scope?: string; resourceMetadataUrl?: URL; errorDescription?: string; statusText?: string; text?: string | null }, - stepUpRetries: number - ): Promise<'AUTHORIZED' | 'REDIRECT'> { + private async _stepUpAuthorize(challenge: StepUpChallenge, stepUpRetries: number): Promise<'AUTHORIZED' | 'REDIRECT'> { + // Auth-seam stamp, method-level: covers the InsufficientScopeError + // throws, the retry-limit SdkHttpError, the tokens() read, and every + // auth() escape (typed or not). + try { + return await this._stepUpAuthorizeInner(challenge, stepUpRetries); + } catch (error) { + throw markAuthSeamEscape(error); + } + } + + private async _stepUpAuthorizeInner(challenge: StepUpChallenge, stepUpRetries: number): Promise<'AUTHORIZED' | 'REDIRECT'> { if (this._onInsufficientScope === 'throw') { throw new InsufficientScopeError({ requiredScope: challenge.scope, @@ -422,7 +434,14 @@ export class StreamableHTTPClientTransport implements Transport { private async _commonHeaders(): Promise { const headers: RequestInit['headers'] & Record = {}; - const token = await this._authProvider?.token(); + let token: string | undefined; + try { + token = await this._authProvider?.token(); + } catch (error) { + // Auth-seam stamp: a throwing token() is an auth failure, never a + // network failure. + throw markAuthSeamEscape(error); + } if (token) { headers['Authorization'] = `Bearer ${token}`; } @@ -543,23 +562,31 @@ export class StreamableHTTPClientTransport implements Transport { } if (this._authProvider.onUnauthorized && !isAuthRetry) { - await this._authProvider.onUnauthorized({ - response, - serverUrl: this._url, - fetchFn: this._fetchWithInit - }); + try { + await this._authProvider.onUnauthorized({ + response, + serverUrl: this._url, + fetchFn: this._fetchWithInit + }); + } catch (error) { + // Auth-seam stamp: covers the SDK's OAuth flow and + // custom onUnauthorized callbacks alike. + throw markAuthSeamEscape(error); + } await response.text?.().catch(() => {}); // Purposely _not_ awaited, so we don't call onerror twice return this._startOrAuthSse(options, true, stepUpRetries); } await response.text?.().catch(() => {}); if (isAuthRetry) { - throw new SdkHttpError(SdkErrorCode.ClientHttpAuthentication, 'Server returned 401 after re-authentication', { - status: 401, - statusText: response.statusText - }); + throw markAuthSeamEscape( + new SdkHttpError(SdkErrorCode.ClientHttpAuthentication, 'Server returned 401 after re-authentication', { + status: 401, + statusText: response.statusText + }) + ); } - throw new UnauthorizedError(); + throw markAuthSeamEscape(new UnauthorizedError()); } if (response.status === 403) { @@ -571,7 +598,7 @@ export class StreamableHTTPClientTransport implements Transport { stepUpRetries ); if (result !== 'AUTHORIZED') { - throw new UnauthorizedError(); + throw markAuthSeamEscape(new UnauthorizedError()); } return this._startOrAuthSse(options, isAuthRetry, stepUpRetries + 1); } @@ -997,23 +1024,31 @@ export class StreamableHTTPClientTransport implements Transport { } if (this._authProvider.onUnauthorized && !isAuthRetry) { - await this._authProvider.onUnauthorized({ - response, - serverUrl: this._url, - fetchFn: this._fetchWithInit - }); + try { + await this._authProvider.onUnauthorized({ + response, + serverUrl: this._url, + fetchFn: this._fetchWithInit + }); + } catch (error) { + // Auth-seam stamp: covers the SDK's OAuth flow and + // custom onUnauthorized callbacks alike. + throw markAuthSeamEscape(error); + } await response.text?.().catch(() => {}); // Purposely _not_ awaited, so we don't call onerror twice return this._send(message, options, true, stepUpRetries); } await response.text?.().catch(() => {}); if (isAuthRetry) { - throw new SdkHttpError(SdkErrorCode.ClientHttpAuthentication, 'Server returned 401 after re-authentication', { - status: 401, - statusText: response.statusText - }); + throw markAuthSeamEscape( + new SdkHttpError(SdkErrorCode.ClientHttpAuthentication, 'Server returned 401 after re-authentication', { + status: 401, + statusText: response.statusText + }) + ); } - throw new UnauthorizedError(); + throw markAuthSeamEscape(new UnauthorizedError()); } const text = await response.text?.().catch(() => null); @@ -1027,7 +1062,7 @@ export class StreamableHTTPClientTransport implements Transport { stepUpRetries ); if (result !== 'AUTHORIZED') { - throw new UnauthorizedError(); + throw markAuthSeamEscape(new UnauthorizedError()); } return this._send(message, options, isAuthRetry, stepUpRetries + 1); } diff --git a/packages/client/src/client/versionNegotiation.ts b/packages/client/src/client/versionNegotiation.ts index 818ca2bb0d..9b89d962ba 100644 --- a/packages/client/src/client/versionNegotiation.ts +++ b/packages/client/src/client/versionNegotiation.ts @@ -26,6 +26,7 @@ import { } from '@modelcontextprotocol/core-internal'; import { UnauthorizedError } from './auth'; +import { isAuthSeamEscape } from './authSeam'; import type { ProbeEnvironment, ProbeOutcome, ProbeTransportKind, ProbeVerdict } from './probeClassifier'; import { classifyProbeOutcome } from './probeClassifier'; @@ -381,23 +382,33 @@ function normalizeReply(reply: RawProbeReply, timeoutMs: number): ProbeOutcome { } case 'send-error': { const error = reply.error; - if (error instanceof SdkHttpError) { - const text = (error.data as { text?: unknown } | undefined)?.text; - return { kind: 'http-error', status: error.data.status, body: typeof text === 'string' ? text : undefined }; - } - const isUnauthorized = + const isAuthOutcome = + // Provenance recorded at the throw boundary: the transport + // stamps every error escaping one of its auth seams (the + // token() read, onUnauthorized invocations — SDK flow and + // custom callbacks alike — the 403 step-up method, and its + // own auth-failure constructions). Never reconstructed here + // from error types. + isAuthSeamEscape(error) || + // The published contract for foreign transports: an + // UnauthorizedError by brand, or by name for one thrown by a + // differently bundled SDK copy at a skewed version or an auth + // middleware's own class. error instanceof UnauthorizedError || - // Name fallback for auth errors the brand cannot reach: an - // UnauthorizedError from a differently bundled SDK copy at a - // skewed version, or an auth middleware's own class. (error instanceof Error && error.name === 'UnauthorizedError'); - if (isUnauthorized) { - // Auth-gated server. (The pre-branding name-string check alone - // could never fire for the SDK's own class — it did not set - // `.name` — so these send failures fell through to the generic - // network-error wrap.) + if (isAuthOutcome) { + // Auth-gated server: propagate unchanged. return { kind: 'auth-required', error: error as Error }; } + if (error instanceof SdkHttpError) { + const text = (error.data as { text?: unknown } | undefined)?.text; + return { + kind: 'http-error', + status: error.data.status, + body: typeof text === 'string' ? text : undefined, + statusText: error.data.statusText + }; + } return { kind: 'network-error', error }; } case 'closed': { diff --git a/packages/client/test/client/probeAuthSeam.test.ts b/packages/client/test/client/probeAuthSeam.test.ts new file mode 100644 index 0000000000..a347d71cd5 --- /dev/null +++ b/packages/client/test/client/probeAuthSeam.test.ts @@ -0,0 +1,288 @@ +/** + * Fault injection per transport auth seam, adapted from the PR #2564 + * neutral-review measurement harness: each scenario drives negotiateEra() + * with a REAL StreamableHTTPClientTransport and a mocked fetch, so the full + * transport 401/403 handling and auth() flow execute for real. + * + * The invariant under test: any error escaping a stamped auth seam reaches + * the caller IDENTITY-PRESERVED (same object; instanceof and diagnostics + * intact) as an auth outcome — never a legacy verdict (the probe's browser + * CORS row must not consume auth-flow TypeErrors), never a rewrap. Errors + * thrown outside the stamped seams keep their pre-existing behavior. + */ +import { OAuthError, SdkError, SdkErrorCode, SdkHttpError } from '@modelcontextprotocol/core-internal'; +import { describe, expect, test } from 'vitest'; + +import type { AuthProvider, OAuthClientProvider } from '../../src/client/auth'; +import { UnauthorizedError } from '../../src/client/auth'; +import { StreamableHTTPClientTransport } from '../../src/client/streamableHttp'; +import { negotiateEra } from '../../src/client/versionNegotiation'; + +const SERVER = 'https://server.example.com/mcp'; +const AS = 'https://auth.example.com'; + +const AS_METADATA = { + issuer: AS, + authorization_endpoint: `${AS}/authorize`, + token_endpoint: `${AS}/token`, + registration_endpoint: `${AS}/register`, + response_types_supported: ['code'], + code_challenge_methods_supported: ['S256'] +}; + +function freshProvider(): OAuthClientProvider { + return { + get redirectUrl() { + return 'http://localhost:3000/callback'; + }, + get clientMetadata() { + return { redirect_uris: ['http://localhost:3000/callback'] }; + }, + clientInformation: () => undefined, + saveClientInformation: () => {}, + tokens: () => undefined, + saveTokens: () => {}, + redirectToAuthorization: () => {}, + saveCodeVerifier: () => {}, + codeVerifier: () => 'verifier' + }; +} + +/** fetch mock: 401 wall on the MCP endpoint, real discovery, DCR delegated. */ +function authWallFetch(onRegister: () => Promise): typeof fetch { + return (async (input: string | URL | Request, _init?: RequestInit): Promise => { + const url = new URL(String(input instanceof Request ? input.url : input)); + if (url.href === SERVER || url.pathname === '/mcp') { + return new Response('Unauthorized', { + status: 401, + headers: { + 'WWW-Authenticate': `Bearer resource_metadata="https://server.example.com/.well-known/oauth-protected-resource/mcp"` + } + }); + } + if (url.pathname.includes('oauth-protected-resource')) { + return Response.json({ resource: SERVER, authorization_servers: [AS] }); + } + if (url.pathname.includes('oauth-authorization-server') || url.pathname.includes('openid-configuration')) { + return Response.json(AS_METADATA); + } + if (url.pathname === '/register') { + return onRegister(); + } + return new Response('Not Found', { status: 404 }); + }) as typeof fetch; +} + +async function runProbe(transport: StreamableHTTPClientTransport, environment: 'node' | 'browser') { + return negotiateEra( + { kind: 'auto', modernVersions: ['2026-07-28'], fallbackAvailable: true, probe: {} }, + { + transport, + clientInfo: { name: 'seam-test', version: '0' }, + capabilities: {}, + environment, + transportKind: 'http', + defaultTimeoutMs: 3000 + } + ).then( + result => ({ settled: 'resolved' as const, result, error: undefined as unknown }), + (error: unknown) => ({ settled: 'rejected' as const, result: undefined, error }) + ); +} + +describe('stamped-seam fault injection (identity-preserving auth outcomes, never legacy)', () => { + test('DCR POST throws TypeError inside the SDK auth flow — browser: the raw TypeError propagates, no CORS-legacy verdict', async () => { + const dcrError = new TypeError('Failed to fetch'); + const transport = new StreamableHTTPClientTransport(new URL(SERVER), { + authProvider: freshProvider(), + fetch: authWallFetch(() => Promise.reject(dcrError)) + }); + const out = await runProbe(transport, 'browser'); + expect(out.settled).toBe('rejected'); + expect(out.error).toBe(dcrError); + }); + + test('DCR POST throws TypeError — node: same raw propagation, no network-error rewrap', async () => { + const dcrError = new TypeError('Failed to fetch'); + const transport = new StreamableHTTPClientTransport(new URL(SERVER), { + authProvider: freshProvider(), + fetch: authWallFetch(() => Promise.reject(dcrError)) + }); + const out = await runProbe(transport, 'node'); + expect(out.settled).toBe('rejected'); + expect(out.error).toBe(dcrError); + }); + + test("401 persists after re-auth: the transport's ClientHttpAuthentication diagnostic reaches the caller intact", async () => { + const transport = new StreamableHTTPClientTransport(new URL(SERVER), { + authProvider: { + token: async () => 'stale-token', + onUnauthorized: async () => {} + }, + fetch: authWallFetch(() => Promise.resolve(new Response(null, { status: 500 }))) + }); + const out = await runProbe(transport, 'node'); + expect(out.settled).toBe('rejected'); + expect(out.error).toBeInstanceOf(SdkHttpError); + expect((out.error as SdkHttpError).code).toBe(SdkErrorCode.ClientHttpAuthentication); + expect((out.error as Error).message).toContain('after re-authentication'); + }); + + test('403 insufficient_scope with no provider to drive step-up: the raw InsufficientScopeError propagates', async () => { + const fetchMock = (async (input: string | URL | Request): Promise => { + const url = new URL(String(input instanceof Request ? input.url : input)); + if (url.href === SERVER || url.pathname === '/mcp') { + return new Response('Forbidden', { + status: 403, + headers: { 'WWW-Authenticate': 'Bearer error="insufficient_scope", scope="mcp:tools"' } + }); + } + return new Response('Not Found', { status: 404 }); + }) as typeof fetch; + const transport = new StreamableHTTPClientTransport(new URL(SERVER), { fetch: fetchMock }); + const out = await runProbe(transport, 'node'); + expect(out.settled).toBe('rejected'); + expect((out.error as Error).name).toBe('InsufficientScopeError'); + expect((out.error as Error).message).toContain('mcp:tools'); + }); + + test('user callback (clientInformation) throws a custom class inside auth(): the raw instance propagates — instanceof works', async () => { + class UserDbError extends Error { + override readonly name = 'UserDbError'; + } + const userError = new UserDbError('user db exploded'); + const provider = freshProvider(); + provider.clientInformation = () => { + throw userError; + }; + const transport = new StreamableHTTPClientTransport(new URL(SERVER), { + authProvider: provider, + fetch: authWallFetch(() => Promise.resolve(new Response(null, { status: 500 }))) + }); + const out = await runProbe(transport, 'node'); + expect(out.settled).toBe('rejected'); + expect(out.error).toBe(userError); + expect(out.error).toBeInstanceOf(UserDbError); + }); + + test('token endpoint rejects with a non-recoverable OAuthError: the raw OAuthError (code intact) propagates', async () => { + const fetchMock = (async (input: string | URL | Request): Promise => { + const url = new URL(String(input instanceof Request ? input.url : input)); + if (url.href === SERVER || url.pathname === '/mcp') { + return new Response('Unauthorized', { + status: 401, + headers: { + 'WWW-Authenticate': `Bearer resource_metadata="https://server.example.com/.well-known/oauth-protected-resource/mcp"` + } + }); + } + if (url.pathname.includes('oauth-protected-resource')) { + return Response.json({ resource: SERVER, authorization_servers: [AS] }); + } + if (url.pathname.includes('oauth-authorization-server') || url.pathname.includes('openid-configuration')) { + return Response.json(AS_METADATA); + } + if (url.pathname === '/token') { + return Response.json({ error: 'invalid_scope', error_description: 'scope not granted' }, { status: 400 }); + } + return new Response('Not Found', { status: 404 }); + }) as typeof fetch; + const provider = freshProvider(); + provider.clientInformation = () => ({ client_id: 'abc', issuer: AS }); + provider.tokens = () => ({ access_token: 'tok', refresh_token: 'rt', token_type: 'Bearer', issuer: AS }); + const transport = new StreamableHTTPClientTransport(new URL(SERVER), { authProvider: provider, fetch: fetchMock }); + const out = await runProbe(transport, 'node'); + expect(out.settled).toBe('rejected'); + expect(out.error).toBeInstanceOf(OAuthError); + expect((out.error as OAuthError).code).toBe('invalid_scope'); + }); + + test('healthy flow ending in REDIRECT: UnauthorizedError propagates (the finishAuth contract)', async () => { + const transport = new StreamableHTTPClientTransport(new URL(SERVER), { + authProvider: freshProvider(), + fetch: authWallFetch(() => + Promise.resolve(Response.json({ client_id: 'abc', redirect_uris: ['http://localhost:3000/callback'] }, { status: 201 })) + ) + }); + const out = await runProbe(transport, 'node'); + expect(out.settled).toBe('rejected'); + expect(out.error).toBeInstanceOf(UnauthorizedError); + }); + + test('R6: token() throws at the _commonHeaders read — browser: raw TypeError propagates, never the CORS-legacy verdict', async () => { + const tokenError = new TypeError("Cannot read properties of undefined (reading 'access_token')"); + const provider: AuthProvider = { + token: () => { + throw tokenError; + } + }; + let fetched = 0; + const fetchMock = (async (): Promise => { + fetched++; + return new Response(null, { status: 500 }); + }) as typeof fetch; + const transport = new StreamableHTTPClientTransport(new URL(SERVER), { authProvider: provider, fetch: fetchMock }); + const out = await runProbe(transport, 'browser'); + expect(out.settled).toBe('rejected'); + expect(out.error).toBe(tokenError); + // The failure happened before any network I/O — nothing was fetched, + // and in particular no legacy initialize followed. + expect(fetched).toBe(0); + }); + + test('B12: a CUSTOM onUnauthorized callback throws TypeError — browser: raw propagation, never the CORS-legacy verdict', async () => { + const cbError = new TypeError('custom re-auth crashed'); + const transport = new StreamableHTTPClientTransport(new URL(SERVER), { + authProvider: { + token: async () => undefined, + onUnauthorized: async () => { + throw cbError; + } + }, + fetch: authWallFetch(() => Promise.resolve(new Response(null, { status: 500 }))) + }); + const out = await runProbe(transport, 'browser'); + expect(out.settled).toBe('rejected'); + expect(out.error).toBe(cbError); + }); + + test('B11 regression pin: on the NON-probe path a throwing custom onUnauthorized propagates the raw instance from send()', async () => { + class CallbackError extends Error { + override readonly name = 'CallbackError'; + } + const cbError = new CallbackError('boom'); + const transport = new StreamableHTTPClientTransport(new URL(SERVER), { + authProvider: { + token: async () => undefined, + onUnauthorized: async () => { + throw cbError; + } + }, + fetch: authWallFetch(() => Promise.resolve(new Response(null, { status: 500 }))) + }); + await transport.start(); + const rejection = await transport + .send({ jsonrpc: '2.0', id: 1, method: 'tools/list' }) + .then(() => { + throw new Error('send unexpectedly resolved'); + }) + .catch((e: unknown) => e); + // Identity preserved: the stamp is a non-enumerable marker, not a wrap. + expect(rejection).toBe(cbError); + expect(rejection).toBeInstanceOf(CallbackError); + await transport.close(); + }); + + test('control: an error thrown outside every auth seam (the probe POST itself failing) stays a typed negotiation error', async () => { + const netError = new Error('socket hang up'); + const fetchMock = (async (): Promise => { + throw netError; + }) as typeof fetch; + const transport = new StreamableHTTPClientTransport(new URL(SERVER), { fetch: fetchMock }); + const out = await runProbe(transport, 'node'); + expect(out.settled).toBe('rejected'); + expect(out.error).toBeInstanceOf(SdkError); + expect((out.error as SdkError).code).toBe(SdkErrorCode.EraNegotiationFailed); + expect(((out.error as SdkError).data as { cause?: unknown }).cause).toBe(netError); + }); +}); diff --git a/packages/client/test/client/probeFixtureCorpus.test.ts b/packages/client/test/client/probeFixtureCorpus.test.ts index d74ed0b5ad..8dd2eefb4f 100644 --- a/packages/client/test/client/probeFixtureCorpus.test.ts +++ b/packages/client/test/client/probeFixtureCorpus.test.ts @@ -19,7 +19,7 @@ * probe wire shape (string id, `server/discover` first, never a real request). */ import type { JSONRPCMessage, Transport } from '@modelcontextprotocol/core-internal'; -import { LATEST_PROTOCOL_VERSION, PROTOCOL_VERSION_META_KEY } from '@modelcontextprotocol/core-internal'; +import { LATEST_PROTOCOL_VERSION, PROTOCOL_VERSION_META_KEY, SdkErrorCode, SdkHttpError } from '@modelcontextprotocol/core-internal'; import { describe, expect, it } from 'vitest'; import { Client } from '../../src/client/client'; @@ -161,6 +161,56 @@ const CORPUS: CorpusRow[] = [ outcome: { kind: 'result', result: { content: [{ type: 'text', text: `supportedVersions: ["${MODERN}"]` }] } }, expected: 'legacy' }, + // --- Auth statuses are never era evidence (#2561): an auth-protected + // server is not a legacy server, whatever the body says — typed failure, + // never initialize (fallbackAvailable is true in every row here). + { + name: 'auth: HTTP 401 challenge (WWW-Authenticate rides the header; body carries the OAuth error JSON) → typed auth failure, never legacy', + outcome: { kind: 'http-error', status: 401, body: '{"error":"invalid_token","error_description":"Missing bearer token"}' }, + expected: 'error' + }, + { + name: 'auth: bare HTTP 401 (no body) → typed auth failure, never legacy', + outcome: { kind: 'http-error', status: 401 }, + expected: 'error' + }, + { + name: 'auth: bare HTTP 403 denial (no body) → typed auth failure, never legacy', + outcome: { kind: 'http-error', status: 403 }, + expected: 'error' + }, + { + 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', + outcome: { kind: 'http-error', status: 401, body: DEPLOYED_SESSION_REQUIRED_BODY }, + expected: 'error' + }, + // --- Server failures are never era evidence: the spec keys the HTTP + // legacy signal to a 4xx rejection, so a 5xx (mid-deploy proxy, crashed + // backend) rejects typed instead of demoting a modern server to legacy. + { + name: '5xx: HTTP 503 with an HTML error page (mid-deploy modern server) → typed error, never legacy', + outcome: { kind: 'http-error', status: 503, body: 'Service Unavailable' }, + expected: 'error' + }, + { + name: '5xx: bare HTTP 500 (no body) → typed error, never legacy', + outcome: { kind: 'http-error', status: 500 }, + expected: 'error' + }, + { + name: '5xx: HTTP 502 with a JSON (but not JSON-RPC) gateway body → typed error, never legacy', + outcome: { kind: 'http-error', status: 502, body: '{"message":"upstream connect error"}' }, + expected: 'error' + }, + { + name: '5xx: HTTP 500 whose body parses as a JSON-RPC error (-32603 from a crashed handler) is still a server failure — 5xx ranks above the body parse', + outcome: { + kind: 'http-error', + status: 500, + body: JSON.stringify({ jsonrpc: '2.0', id: null, error: { code: -32_603, message: 'Internal error' } }) + }, + expected: 'error' + }, // --- Q12 transport-aware timeout rows (stdio falls back, HTTP stays a typed error). { name: 'timeout on stdio → legacy fallback (the stdio backward-compatibility rule)', @@ -209,6 +259,40 @@ describe('T9/T11 merged probe fixture corpus (probe classifier)', () => { }); } + it('the 401/403 typed failures carry the auth codes (never EraNegotiationFailed — auth walls must not enter era-recovery flows) plus status, reason phrase, and response text', () => { + for (const [status, code, statusText, body] of [ + [401, SdkErrorCode.ClientHttpAuthentication, 'Unauthorized', '{"error":"invalid_token"}'], + [403, SdkErrorCode.ClientHttpForbidden, 'Forbidden', 'nope'] + ] as const) { + const verdict = classifyProbeOutcome({ kind: 'http-error', status, statusText, body }, baseContext); + expect(verdict.kind).toBe('error'); + if (verdict.kind === 'error') { + expect(verdict.error).toBeInstanceOf(SdkHttpError); + const error = verdict.error as SdkHttpError; + expect(error.code).toBe(code); + expect(error.status).toBe(status); + expect(error.statusText).toBe(statusText); + expect(error.data.text).toBe(body); + expect(error.message).toContain(String(status)); + } + } + }); + + it('the 5xx typed failure is EraNegotiationFailed (a genuine negotiation failure) carrying the status', () => { + const verdict = classifyProbeOutcome( + { kind: 'http-error', status: 503, statusText: 'Service Unavailable', body: 'down' }, + baseContext + ); + expect(verdict.kind).toBe('error'); + if (verdict.kind === 'error') { + expect(verdict.error).toBeInstanceOf(SdkHttpError); + const error = verdict.error as SdkHttpError; + expect(error.code).toBe(SdkErrorCode.EraNegotiationFailed); + expect(error.status).toBe(503); + expect(error.message).toContain('503'); + } + }); + it('a DiscoverResult with a mutual version is the only result shape that yields a modern verdict', () => { const verdict = classifyProbeOutcome( { diff --git a/packages/client/test/client/versionNegotiation.test.ts b/packages/client/test/client/versionNegotiation.test.ts index e197344b90..60c9a2aaba 100644 --- a/packages/client/test/client/versionNegotiation.test.ts +++ b/packages/client/test/client/versionNegotiation.test.ts @@ -11,14 +11,18 @@ import type { JSONRPCMessage, JSONRPCRequest, Transport } from '@modelcontextprotocol/core-internal'; import { isJSONRPCRequest, + OAuthError, PROTOCOL_VERSION_META_KEY, SdkError, SdkErrorCode, + SdkHttpError, UnsupportedProtocolVersionError } from '@modelcontextprotocol/core-internal'; import { describe, expect, test } from 'vitest'; import { UnauthorizedError } from '../../src/client/auth'; +import { InsufficientScopeError } from '../../src/client/authErrors'; +import { markAuthSeamEscape } from '../../src/client/authSeam'; import { Client } from '../../src/client/client'; import type { StreamableHTTPClientTransportOptions } from '../../src/client/streamableHttp'; import type { StdioServerParameters } from '../../src/client/stdio'; @@ -1103,6 +1107,53 @@ describe('probe send-error classification', () => { expect(requests(transport.sent).some(r => r.method === 'initialize')).toBe(false); }); + // Any error stamped at a transport auth seam propagates unchanged — + // identity preserved (same object, instanceof and diagnostics intact), + // never a legacy fallback, never a rewrap. Provenance comes from the + // stamp, not the error type: the rows deliberately span typed SDK + // errors, foreign classes, and a bare TypeError. + const seamEscapes: Array<[string, () => Error]> = [ + [ + "the transport's post-re-auth 401 (SdkHttpError ClientHttpAuthentication)", + () => + new SdkHttpError(SdkErrorCode.ClientHttpAuthentication, 'Server returned 401 after re-authentication', { + status: 401, + statusText: 'Unauthorized' + }) + ], + [ + 'an InsufficientScopeError from a step-up challenge with no provider', + () => new InsufficientScopeError({ requiredScope: 'mcp:tools' }) + ], + ['an OAuthError (invalid_grant on a revoked refresh token)', () => new OAuthError('invalid_grant', 'Refresh token was revoked')], + ['a bare TypeError from a DCR sub-fetch inside the auth flow', () => new TypeError('Failed to fetch')], + [ + 'a custom error class thrown by a user onUnauthorized callback', + () => { + class UserDbError extends Error { + override readonly name = 'UserDbError'; + } + return new UserDbError('user db exploded'); + } + ] + ]; + for (const [label, make] of seamEscapes) { + test(`stamped seam escape propagates unchanged: ${label}`, async () => { + const reason = markAuthSeamEscape(make()); + const transport = new AuthGatedTransport(reason); + const client = new Client({ name: 'c', version: '0' }, { versionNegotiation: { mode: 'auto' } }); + + const rejection = await client.connect(transport).then( + () => { + throw new Error('connect unexpectedly resolved'); + }, + (e: unknown) => e + ); + expect(rejection).toBe(reason); + expect(requests(transport.sent).some(r => r.method === 'initialize')).toBe(false); + }); + } + test('a plain send failure stays a typed negotiation error — no fallback runs', async () => { const transport = new AuthGatedTransport(new Error('connection refused')); const client = new Client({ name: 'c', version: '0' }, { versionNegotiation: { mode: 'auto' } }); diff --git a/packages/core-internal/src/errors/sdkErrors.ts b/packages/core-internal/src/errors/sdkErrors.ts index 21f42f6ef8..0bc8f9a1ad 100644 --- a/packages/core-internal/src/errors/sdkErrors.ts +++ b/packages/core-internal/src/errors/sdkErrors.ts @@ -65,16 +65,34 @@ export enum SdkErrorCode { /** * Protocol-era negotiation at connect time failed without producing either a * usable modern (2026-07-28+) era or a definitive legacy fallback signal — - * e.g. the negotiation mode forbids falling back (`pin`), or the probe hit a - * network failure (a typed connect error, never an era verdict). + * e.g. the negotiation mode forbids falling back (`pin`), the probe hit a + * network failure, or the server answered the probe with a 5xx (a typed + * connect error, never an era verdict). * - * Negotiation-phase only: this code is never used once an era is established. + * Negotiation-phase only: this code is never used once an era is + * established. Auth walls never carry it: a 401/403 rejecting the probe + * uses {@linkcode ClientHttpAuthentication} / {@linkcode ClientHttpForbidden} + * instead, so era-recovery flows keyed on this code (e.g. cached-verdict + * gateways) can never persist a verdict for an unauthorized exchange. */ EraNegotiationFailed = 'ERA_NEGOTIATION_FAILED', // Transport errors ClientHttpNotImplemented = 'CLIENT_HTTP_NOT_IMPLEMENTED', + /** + * HTTP 401 authentication failure: the transport's re-auth retry still got + * 401 (`Server returned 401 after re-authentication`), or the version + * negotiation probe was rejected 401 with no `authProvider` configured + * (`Version negotiation failed: the server requires authorization (HTTP 401)`). + * Carried on an {@linkcode SdkHttpError} with `status: 401`. + */ ClientHttpAuthentication = 'CLIENT_HTTP_AUTHENTICATION', + /** + * HTTP 403 denial: the step-up re-authorization retry limit was reached, + * or the version negotiation probe was rejected 403 + * (`Version negotiation failed: the server denied access (HTTP 403)`). + * Carried on an {@linkcode SdkHttpError} with `status: 403`. + */ ClientHttpForbidden = 'CLIENT_HTTP_FORBIDDEN', ClientHttpUnexpectedContent = 'CLIENT_HTTP_UNEXPECTED_CONTENT', ClientHttpFailedToOpenStream = 'CLIENT_HTTP_FAILED_TO_OPEN_STREAM', diff --git a/test/e2e/requirements.ts b/test/e2e/requirements.ts index 76b597c5eb..a3439e1ceb 100644 --- a/test/e2e/requirements.ts +++ b/test/e2e/requirements.ts @@ -2047,6 +2047,13 @@ export const REQUIREMENTS: Record = { transports: ['streamableHttp'], note: 'This exercises the HTTP hosting/auth layer and OAuth client; the matrix transport arg is ignored, so it runs as a single streamableHttp-labelled cell to avoid duplicate runs.' }, + 'client-auth:negotiation:auth-before-era': { + source: 'sdk', + behavior: + "An OAuth-protected legacy server is reachable under versionNegotiation mode 'auto': the connect-time probe's 401 propagates the auth challenge (UnauthorizedError) without deciding the era — auth settles first, era second — and after finishAuth the reconnect re-probes with the token, takes the legacy server's real server/discover rejection as the era evidence, completes the legacy initialize, and serves tools/call.", + transports: ['streamableHttp'], + note: "Wire-order pin: exactly two server/discover POSTs — the pre-auth one 401'd by the auth wall, the post-auth one answered by the legacy stack — then a single initialize, only after the second probe. Same single-cell setup as the rest of the client-auth family (self-contained body; the matrix transport arg is ignored)." + }, 'client-auth:403-scope-upgrade': { source: 'https://modelcontextprotocol.io/specification/2025-11-25/basic/authorization#step-up-authorization-flow', behavior: 'A 403 with WWW-Authenticate triggers a scope-upgrade authorization attempt; repeated 403s do not loop.', diff --git a/test/e2e/scenarios/client-auth.test.ts b/test/e2e/scenarios/client-auth.test.ts index 07d052c3b6..caa1d74fe4 100644 --- a/test/e2e/scenarios/client-auth.test.ts +++ b/test/e2e/scenarios/client-auth.test.ts @@ -348,6 +348,70 @@ verifies('client-auth:401-triggers-flow', async (_args: TestArgs) => { } }); +verifies('client-auth:negotiation:auth-before-era', async (_args: TestArgs) => { + const validToken = 'negotiated-token'; + const as = createMockAuthorizationServer({ tokenResponses: [{ access_token: validToken, token_type: 'Bearer' }] }); + const provider = new RecordingOAuthClientProvider(); + // The auth wall lives in createCombinedFetch (401 challenge without the + // token); behind it a plain legacy stack serves the tool. + const mcpHost = hostPerSession(() => { + const s = new McpServer({ name: 's', version: '0' }); + s.registerTool('probe', { inputSchema: z.object({}) }, () => ({ content: [{ type: 'text', text: 'ok' }] })); + return s; + }); + const baseFetch = createCombinedFetch({ as, mcpHost, validToken }); + + // Wire recorder for MCP-origin POSTs: JSON-RPC method, response status, auth presence. + const mcpPosts: Array<{ method: string; status: number; hasAuth: boolean }> = []; + const combinedFetch = async (url: URL | string, init?: RequestInit): Promise => { + const urlObj = typeof url === 'string' ? new URL(url) : url; + const isMcp = urlObj.origin !== ISSUER && !urlObj.pathname.includes('/.well-known/'); + const response = await baseFetch(url, init); + if (isMcp && init?.method === 'POST') { + const body = typeof init.body === 'string' ? (JSON.parse(init.body) as { method?: string }) : {}; + mcpPosts.push({ method: body.method ?? '?', status: response.status, hasAuth: new Headers(init.headers).has('authorization') }); + } + return response; + }; + + const client = new Client({ name: 'c', version: '0' }, { versionNegotiation: { mode: 'auto' } }); + const first = new StreamableHTTPClientTransport(new URL(MCP_URL), { authProvider: provider, fetch: combinedFetch }); + + try { + // Probe -> 401: the auth challenge propagates. The 401 never decides the + // era -- the auth wall answered before the MCP layer saw server/discover. + await expect(client.connect(first)).rejects.toThrow(UnauthorizedError); + expect(mcpPosts).toEqual([{ method: 'server/discover', status: 401, hasAuth: false }]); + expect(provider.redirectedTo).toHaveLength(1); + + // Complete the flow (the mock AS exchanges any code), then reconnect on + // a FRESH transport -- a started transport cannot be restarted. + await first.finishAuth('e2e-auth-code'); + expect(provider.saved.tokens?.access_token).toBe(validToken); + + const second = new StreamableHTTPClientTransport(new URL(MCP_URL), { authProvider: provider, fetch: combinedFetch }); + await client.connect(second); + + // The post-auth re-probe supplied the era evidence: two probes total -- + // the pre-auth one 401'd, the post-auth one answered by the legacy + // stack -- and initialize ran only after the second. + const probes = mcpPosts.filter(p => p.method === 'server/discover'); + expect(probes).toHaveLength(2); + expect(probes[1]).toMatchObject({ hasAuth: true }); + expect(probes[1]!.status).not.toBe(401); + const kinds = mcpPosts.map(p => p.method); + expect(kinds.filter(k => k === 'initialize')).toHaveLength(1); + expect(kinds.indexOf('initialize')).toBeGreaterThan(kinds.lastIndexOf('server/discover')); + expect(client.getNegotiatedProtocolVersion()).toBe('2025-11-25'); + + const result = await client.callTool({ name: 'probe', arguments: {} }); + expect(result.content).toEqual([{ type: 'text', text: 'ok' }]); + } finally { + await client.close(); + await mcpHost.close(); + } +}); + verifies('client-auth:401-after-auth-throws', async (_args: TestArgs) => { const as = createMockAuthorizationServer({ tokenResponses: [{ access_token: 'refreshed-access-token', token_type: 'Bearer' }] diff --git a/test/integration/test/client/versionNegotiation.test.ts b/test/integration/test/client/versionNegotiation.test.ts index 809994e3dd..c6e70446cd 100644 --- a/test/integration/test/client/versionNegotiation.test.ts +++ b/test/integration/test/client/versionNegotiation.test.ts @@ -22,9 +22,9 @@ import { createServer } from 'node:http'; import { tmpdir } from 'node:os'; import path from 'node:path'; -import { Client, StreamableHTTPClientTransport } from '@modelcontextprotocol/client'; +import { Client, StreamableHTTPClientTransport, UnauthorizedError } from '@modelcontextprotocol/client'; import { StdioClientTransport } from '@modelcontextprotocol/client/stdio'; -import { SdkError, SdkErrorCode } from '@modelcontextprotocol/core-internal'; +import { SdkError, SdkErrorCode, SdkHttpError } from '@modelcontextprotocol/core-internal'; import { NodeStreamableHTTPServerTransport } from '@modelcontextprotocol/node'; import { McpServer } from '@modelcontextprotocol/server'; import { listenOnRandomPort } from '@modelcontextprotocol/test-helpers'; @@ -230,6 +230,117 @@ describe('typed connect errors (Q12) over real sockets', () => { }, 15_000); }); +describe('auth-protected server (HTTP 401/403): typed auth failure, never a legacy verdict (#2561)', () => { + const cleanups: Array<() => Promise | void> = []; + afterEach(async () => { + while (cleanups.length > 0) await cleanups.pop()!(); + }); + + /** An OAuth-protected deployment shape: the auth layer rejects every tokenless request before any MCP handler runs. */ + async function startProtected(status: 401 | 403) { + const server = createServer((_req, res) => { + res.writeHead(status, { + 'WWW-Authenticate': 'Bearer realm="mcp", error="invalid_token"', + 'Content-Type': 'application/json' + }); + res.end('{"error":"invalid_token"}'); + }); + const url = await listenOnRandomPort(server); + cleanups.push(() => new Promise(resolve => server.close(() => resolve()))); + return url; + } + + it('auto mode, no authProvider: typed auth failure carrying the 401 — no initialize ever sent', async () => { + const url = await startProtected(401); + const { calls, fetchFn } = recordingFetch(); + const client = new Client({ name: 'neg-client', version: '1.0.0' }, { versionNegotiation: { mode: 'auto' } }); + const transport = new StreamableHTTPClientTransport(url, { fetch: fetchFn }); + + await expect(client.connect(transport)).rejects.toSatisfy( + error => + error instanceof SdkHttpError && + // The auth code, never EraNegotiationFailed: era-recovery flows + // keyed on that code must not consume auth walls. + error.code === SdkErrorCode.ClientHttpAuthentication && + error.status === 401 && + error.message.includes('401') + ); + + // The probe POST is the only wire traffic — the auth wall is not a + // legacy verdict, so no initialize follows it. + const posts = calls.filter(c => c.method === 'POST'); + expect(posts.length).toBeGreaterThan(0); + expect(posts.every(c => (c.body ?? '').includes('server/discover'))).toBe(true); + expect(calls.some(c => (c.body ?? '').includes('"initialize"'))).toBe(false); + }); + + it('auto mode, no authProvider: a 403 denial is a typed failure carrying the 403, not era evidence', async () => { + const url = await startProtected(403); + const { calls, fetchFn } = recordingFetch(); + const client = new Client({ name: 'neg-client', version: '1.0.0' }, { versionNegotiation: { mode: 'auto' } }); + + await expect(client.connect(new StreamableHTTPClientTransport(url, { fetch: fetchFn }))).rejects.toSatisfy( + error => error instanceof SdkHttpError && error.code === SdkErrorCode.ClientHttpForbidden && error.status === 403 + ); + expect(calls.some(c => (c.body ?? '').includes('"initialize"'))).toBe(false); + }); + + it('pin mode: the rejection names the auth status, never the did-not-offer-pinned-version text', async () => { + const url = await startProtected(401); + const client = new Client({ name: 'neg-client', version: '1.0.0' }, { versionNegotiation: { mode: { pin: '2026-07-28' } } }); + + await expect(client.connect(new StreamableHTTPClientTransport(url))).rejects.toSatisfy( + error => + error instanceof SdkError && + error.message.includes('401') && + !error.message.includes('did not offer pinned protocol version') + ); + }); + + it('with an authProvider the auth flow is unchanged: UnauthorizedError propagates for finishAuth, no fallback runs', async () => { + const url = await startProtected(401); + const { calls, fetchFn } = recordingFetch(); + const client = new Client({ name: 'neg-client', version: '1.0.0' }, { versionNegotiation: { mode: 'auto' } }); + const transport = new StreamableHTTPClientTransport(url, { + fetch: fetchFn, + // Token-only provider (no onUnauthorized) whose token the server + // rejects: the transport's 401 handling throws UnauthorizedError, + // and the probe propagates it unchanged — same as before this fix. + authProvider: { token: async () => 'rejected-token' } + }); + + await expect(client.connect(transport)).rejects.toSatisfy(error => error instanceof UnauthorizedError); + expect(calls.some(c => (c.body ?? '').includes('"initialize"'))).toBe(false); + }); + + it("onUnauthorized re-auth that does not help: the transport's typed 401-after-re-authentication failure propagates unchanged", async () => { + const url = await startProtected(401); + const { calls, fetchFn } = recordingFetch(); + const client = new Client({ name: 'neg-client', version: '1.0.0' }, { versionNegotiation: { mode: 'auto' } }); + let reauthRuns = 0; + const transport = new StreamableHTTPClientTransport(url, { + fetch: fetchFn, + authProvider: { + token: async () => 'still-rejected', + onUnauthorized: async () => { + reauthRuns++; + } + } + }); + + // First 401 runs onUnauthorized, the retry 401s again: the transport's + // own diagnostic must reach the caller, not an era-negotiation rewrap. + await expect(client.connect(transport)).rejects.toSatisfy( + error => + error instanceof SdkHttpError && + error.code === SdkErrorCode.ClientHttpAuthentication && + error.message.includes('after re-authentication') + ); + expect(reauthRuns).toBe(1); + expect(calls.some(c => (c.body ?? '').includes('"initialize"'))).toBe(false); + }); +}); + describe('stdio: silent legacy server (probe timeout fallback)', () => { // The stdio transport's backward-compatibility rule: a probe that gets no // response within a reasonable timeout indicates a legacy server — some