From 1b90c96d11fd17016d2977cae9dd661de3fb84df Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Mon, 6 Jul 2026 22:00:16 +0800 Subject: [PATCH 1/4] fix(packaging): expose Ajv from CJS validator subpaths (#2431) --- .changeset/cjs-ajv-validator-subpath.md | 6 ++++++ packages/client/test/client/barrelClean.test.ts | 13 +++++++++++++ .../core-internal/src/validators/ajvProvider.ts | 8 +++++--- packages/server/test/server/barrelClean.test.ts | 13 +++++++++++++ 4 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 .changeset/cjs-ajv-validator-subpath.md diff --git a/.changeset/cjs-ajv-validator-subpath.md b/.changeset/cjs-ajv-validator-subpath.md new file mode 100644 index 0000000000..ac4c48c0b6 --- /dev/null +++ b/.changeset/cjs-ajv-validator-subpath.md @@ -0,0 +1,6 @@ +--- +'@modelcontextprotocol/server': patch +'@modelcontextprotocol/client': patch +--- + +Fix the CommonJS `validators/ajv` subpath so reading the exported `Ajv` class no longer throws `ReferenceError: import_ajv is not defined`. The subpath now re-exports the bundled provider's concrete `Ajv` value in CJS output, matching the existing ESM behavior. diff --git a/packages/client/test/client/barrelClean.test.ts b/packages/client/test/client/barrelClean.test.ts index 741528b6f6..a0f716581e 100644 --- a/packages/client/test/client/barrelClean.test.ts +++ b/packages/client/test/client/barrelClean.test.ts @@ -1,5 +1,6 @@ import { execFileSync } from 'node:child_process'; import { existsSync, readFileSync } from 'node:fs'; +import { createRequire } from 'node:module'; import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -7,6 +8,7 @@ import { beforeAll, describe, expect, test } from 'vitest'; const pkgDir = join(dirname(fileURLToPath(import.meta.url)), '../..'); const distDir = join(pkgDir, 'dist'); +const requireDist = createRequire(join(pkgDir, 'package.json')); const NODE_ONLY = /\b(child_process|cross-spawn|node:stream|node:child_process)\b/; // Anchored at start-of-line so JSDoc-example `from 'ajv'` strings in vendored chunks don't match. const VALIDATOR_BACKEND_IMPORT = /^import[^\n]*?from\s+["'](?:ajv|ajv-formats|@cfworker\/json-schema)["']/m; @@ -67,4 +69,15 @@ describe('@modelcontextprotocol/client root entry is browser-safe', () => { } } }); + + test('CJS AJV validator subpath exposes Ajv at runtime', () => { + const { Ajv, AjvJsonSchemaValidator, addFormats } = requireDist( + './dist/validators/ajv.cjs' + ) as typeof import('../../src/validators/ajv'); + + const ajv = new Ajv({ strict: false, allErrors: true }); + addFormats(ajv); + + expect(new AjvJsonSchemaValidator(ajv)).toBeInstanceOf(AjvJsonSchemaValidator); + }); }); diff --git a/packages/core-internal/src/validators/ajvProvider.ts b/packages/core-internal/src/validators/ajvProvider.ts index 2d1d948721..0a24fe4533 100644 --- a/packages/core-internal/src/validators/ajvProvider.ts +++ b/packages/core-internal/src/validators/ajvProvider.ts @@ -2,6 +2,7 @@ * AJV-based JSON Schema validator provider */ +import { Ajv as Draft7Ajv } from 'ajv'; import { Ajv2020 } from 'ajv/dist/2020.js'; import _addFormats from 'ajv-formats'; @@ -31,6 +32,7 @@ interface AjvValidateFunction { errors?: any; } +/** `ajv-formats` default export, normalised through the CJS/ESM interop wrapper. */ const addFormats = _addFormats as unknown as typeof _addFormats.default; function createDefaultAjvInstance(): AjvLike { @@ -152,6 +154,6 @@ export class AjvJsonSchemaValidator implements jsonSchemaValidator { * declaration bundling — see #2339). To construct a custom 2020-12 instance, add `ajv` to your own * dependencies (matching the SDK's pinned version) and `import { Ajv2020 } from 'ajv/dist/2020.js'`. */ -export { Ajv } from 'ajv'; -/** `ajv-formats` default export, normalised through the CJS/ESM interop wrapper. */ -export { addFormats }; +const Ajv = Draft7Ajv; + +export { addFormats, Ajv }; diff --git a/packages/server/test/server/barrelClean.test.ts b/packages/server/test/server/barrelClean.test.ts index bf3924e484..184a590d10 100644 --- a/packages/server/test/server/barrelClean.test.ts +++ b/packages/server/test/server/barrelClean.test.ts @@ -1,5 +1,6 @@ import { execFileSync } from 'node:child_process'; import { existsSync, readFileSync } from 'node:fs'; +import { createRequire } from 'node:module'; import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -7,6 +8,7 @@ import { beforeAll, describe, expect, test } from 'vitest'; const pkgDir = join(dirname(fileURLToPath(import.meta.url)), '../..'); const distDir = join(pkgDir, 'dist'); +const requireDist = createRequire(join(pkgDir, 'package.json')); const NODE_ONLY = /\b(child_process|cross-spawn|node:stream|node:child_process)\b/; // Anchored at start-of-line so JSDoc-example `from 'ajv'` strings in vendored chunks don't match. const VALIDATOR_BACKEND_IMPORT = /^import[^\n]*?from\s+["'](?:ajv|ajv-formats|@cfworker\/json-schema)["']/m; @@ -68,4 +70,15 @@ describe('@modelcontextprotocol/server root entry is browser-safe', () => { } } }); + + test('CJS AJV validator subpath exposes Ajv at runtime', () => { + const { Ajv, AjvJsonSchemaValidator, addFormats } = requireDist( + './dist/validators/ajv.cjs' + ) as typeof import('../../src/validators/ajv'); + + const ajv = new Ajv({ strict: false, allErrors: true }); + addFormats(ajv); + + expect(new AjvJsonSchemaValidator(ajv)).toBeInstanceOf(AjvJsonSchemaValidator); + }); }); From e6cc5dc633bf40811935d078c9c056b67da4de9b Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 6 Jul 2026 14:04:08 +0000 Subject: [PATCH 2/4] chore(deps): bump actions/checkout from 6 to 7 (#2338) --- .github/workflows/claude.yml | 2 +- .github/workflows/conformance.yml | 4 ++-- .github/workflows/deploy-docs.yml | 2 +- .github/workflows/examples.yml | 2 +- .github/workflows/main.yml | 8 ++++---- .github/workflows/publish.yml | 2 +- .github/workflows/release.yml | 4 ++-- .github/workflows/update-spec-types.yml | 2 +- 8 files changed, 13 insertions(+), 13 deletions(-) diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml index 58d7921b96..5e48e2c5ea 100644 --- a/.github/workflows/claude.yml +++ b/.github/workflows/claude.yml @@ -27,7 +27,7 @@ jobs: actions: read steps: - name: Checkout repository - uses: actions/checkout@v6 + uses: actions/checkout@v7 with: fetch-depth: 1 diff --git a/.github/workflows/conformance.yml b/.github/workflows/conformance.yml index 719e41f697..bb2dbef504 100644 --- a/.github/workflows/conformance.yml +++ b/.github/workflows/conformance.yml @@ -17,7 +17,7 @@ jobs: client-conformance: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - name: Install pnpm uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v5.0.0 with: @@ -35,7 +35,7 @@ jobs: server-conformance: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - name: Install pnpm uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v5.0.0 with: diff --git a/.github/workflows/deploy-docs.yml b/.github/workflows/deploy-docs.yml index 32e8a76c1a..68ed84f8e6 100644 --- a/.github/workflows/deploy-docs.yml +++ b/.github/workflows/deploy-docs.yml @@ -25,7 +25,7 @@ jobs: url: ${{ steps.deployment.outputs.page_url }} steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - name: Install pnpm uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v5.0.0 diff --git a/.github/workflows/examples.yml b/.github/workflows/examples.yml index 197041e07f..e00c0c289c 100644 --- a/.github/workflows/examples.yml +++ b/.github/workflows/examples.yml @@ -24,7 +24,7 @@ jobs: name: examples (build + e2e) runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - name: Install pnpm uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v5.0.0 diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 1b47f899c6..089bbe113d 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -15,7 +15,7 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - name: Install pnpm uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v5.0.0 @@ -41,7 +41,7 @@ jobs: node-version: [20, 22, 24] steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - name: Install pnpm uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v5.0.0 @@ -67,7 +67,7 @@ jobs: node-version: [20, 22, 24] steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - name: Install pnpm uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v5.0.0 @@ -95,7 +95,7 @@ jobs: - runtime: deno version: v2.x steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - name: Install pnpm uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v5.0.0 with: diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 829b634bdc..d8db820e13 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -16,7 +16,7 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - name: Install pnpm uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v5.0.0 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index c2d5da1bf7..3682c55480 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -17,7 +17,7 @@ jobs: outputs: hasChangesets: ${{ steps.changesets.outputs.hasChangesets }} steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - name: Install pnpm uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v5.0.0 @@ -50,7 +50,7 @@ jobs: contents: write id-token: write steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - name: Install pnpm uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v5.0.0 diff --git a/.github/workflows/update-spec-types.yml b/.github/workflows/update-spec-types.yml index d9bb15969c..c62aca8907 100644 --- a/.github/workflows/update-spec-types.yml +++ b/.github/workflows/update-spec-types.yml @@ -26,7 +26,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout repository - uses: actions/checkout@v6 + uses: actions/checkout@v7 - name: Install pnpm uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v5.0.0 From ce2f65db0e019506f4d2526466ec8cc7106de98e Mon Sep 17 00:00:00 2001 From: Felix Weinberger <3823880+felixweinberger@users.noreply.github.com> Date: Mon, 6 Jul 2026 15:53:12 +0100 Subject: [PATCH 3/4] fix(core): make instanceof on SDK error classes work across bundled copies (#2384) --- .changeset/cross-bundle-error-instanceof.md | 6 + docs/migration/upgrade-to-v2.md | 25 ++- docs/servers/errors.md | 4 +- examples/oauth/client.ts | 11 +- packages/client/src/client/auth.ts | 30 +++ packages/client/src/client/authErrors.ts | 48 +++++ packages/client/src/client/probeClassifier.ts | 11 + packages/client/src/client/sse.ts | 31 ++- .../client/src/client/versionNegotiation.ts | 17 +- .../test/client/errorBrandConformance.test.ts | 130 ++++++++++++ .../client/test/client/sseErrorBrand.test.ts | 26 +++ .../test/client/versionNegotiation.test.ts | 76 +++++++ packages/core-internal/src/auth/errors.ts | 28 +++ .../src/errors/crossBundleBrand.ts | 117 +++++++++++ .../core-internal/src/errors/sdkErrors.ts | 33 +++ packages/core-internal/src/index.ts | 1 + packages/core-internal/src/types/errors.ts | 60 +++++- .../test/errors/crossBundleBrand.test.ts | 197 ++++++++++++++++++ .../shared/abortReasonPassthrough.test.ts | 67 ++++++ .../test/types/errorSurfacePins.test.ts | 34 ++- .../missingClientCapabilityError.test.ts | 5 +- .../test/server/errorBrandConformance.test.ts | 89 ++++++++ 22 files changed, 1020 insertions(+), 26 deletions(-) create mode 100644 .changeset/cross-bundle-error-instanceof.md create mode 100644 packages/client/test/client/errorBrandConformance.test.ts create mode 100644 packages/client/test/client/sseErrorBrand.test.ts create mode 100644 packages/core-internal/src/errors/crossBundleBrand.ts create mode 100644 packages/core-internal/test/errors/crossBundleBrand.test.ts create mode 100644 packages/core-internal/test/shared/abortReasonPassthrough.test.ts create mode 100644 packages/server/test/server/errorBrandConformance.test.ts diff --git a/.changeset/cross-bundle-error-instanceof.md b/.changeset/cross-bundle-error-instanceof.md new file mode 100644 index 0000000000..940ca66efc --- /dev/null +++ b/.changeset/cross-bundle-error-instanceof.md @@ -0,0 +1,6 @@ +--- +'@modelcontextprotocol/client': patch +'@modelcontextprotocol/server': patch +--- + +`instanceof` on the SDK error classes (`ProtocolError` and its typed subclasses, `SdkError`/`SdkHttpError`, `OAuthError`, and the client's `SseError`, `UnauthorizedError`, and OAuth-client-flow error family — `OAuthClientFlowError` and its subclasses) now works across separately bundled copies of the SDK. The classes match by a stable brand (via `Symbol.hasInstance` and a registry symbol) instead of prototype identity, so a process that uses both `@modelcontextprotocol/client` and `@modelcontextprotocol/server` - a gateway, host, or in-process test - can check errors constructed by either package against the class re-exported by the other. Ordinary prototype-based `instanceof` is preserved as a fallback; user-defined subclasses keep plain prototype semantics. Notes: cross-bundle matching requires both copies to be at or after this release; brands assert identity, not field shape, across versions - keep reading fields defensively. As a side effect, a foreign-bundle `SdkError` used as an abort reason is now rethrown as-is instead of being wrapped as a `RequestTimeout`. Branded hierarchies additionally expose an explicit static guard, `X.isInstance(value)`, that reads the same brand and narrows in TypeScript — an alternative for codebases that prefer predicate-style checks over `instanceof`. Also: `UnauthorizedError` now sets `error.name` to `'UnauthorizedError'` (previously `'Error'`), and per-package conformance tests enforce that every exported error class participates in branding. Version-negotiation probing now recognizes `UnauthorizedError` (previously a dead name-string check) and propagates it unchanged, so `connect()` on an auth-gated server rejects with the original `UnauthorizedError` (previously wrapped as the `cause` of an `SdkError(EraNegotiationFailed)`) — run `finishAuth()` and reconnect, and the retry probes with the token. diff --git a/docs/migration/upgrade-to-v2.md b/docs/migration/upgrade-to-v2.md index a149e5b407..e8714d7487 100644 --- a/docs/migration/upgrade-to-v2.md +++ b/docs/migration/upgrade-to-v2.md @@ -825,6 +825,28 @@ The SDK now distinguishes three error kinds: 3. **`SdkHttpError`** (extends `SdkError`) — HTTP transport errors with typed `.status` and `.statusText`. +These classes (and `OAuthError`, the client's `SseError`, `UnauthorizedError`, and the +OAuth-client-flow error family) brand-match under `instanceof`, so checks work across +separately bundled copies of the SDK — e.g. a process using both +`@modelcontextprotocol/client` and `@modelcontextprotocol/server`. Each branded +hierarchy also exposes the same check as an explicit static guard +(`SdkError.isInstance(err)`, `ProtocolError.isInstance(err)`, …) that narrows in +TypeScript — use whichever style your codebase prefers; both read the same brand. +Fine print (applies equally to `instanceof` and `isInstance`): + +- **Version skew** — matching needs *both* copies at a brand-aware release; against an + older copy, behavior degrades to plain prototype `instanceof` (false across bundles). + During mixed-version rollouts, recognize errors without class identity: match + `error.name` plus the class's discriminant field (`code`, `status`), or reconstruct + typed protocol errors with `ProtocolError.fromError(code, message, data)`. +- **Worker boundaries** — `structuredClone`/`postMessage` drop the (symbol-keyed) brand, + so a rehydrated error no longer brand-matches; recognize forwarded errors by + `code`/`data` instead. +- **Brands assert identity, not shape** — a matched instance from another SDK version + may lack newer fields; read fields defensively. +- **Re-bundling with property mangling** (`mangle.props` and similar) breaks the brand + statics; default esbuild/webpack/terser settings are safe. + The codemod renames `McpError` → `ProtocolError`, `ErrorCode` → `ProtocolErrorCode` (routing `RequestTimeout` / `ConnectionClosed` to `SdkErrorCode`), and `StreamableHTTPError` → `SdkHttpError`. After the codemod runs, your `instanceof` @@ -982,7 +1004,8 @@ peers as `-32602` — a server can no longer emit `-32002` on the wire. `ProtocolErrorCode.ResourceNotFound` (`-32002`) stays importable as receive-tolerated vocabulary — accept both `-32602` and `-32002` from peers. `ProtocolError.fromError(code, message, data)` reconstructs the typed subclass from -code + data alone, so it works across bundle boundaries where `instanceof` doesn't. +code + data alone — the version-agnostic path: it also works on plain wire shapes and +against SDK copies that predate brand-matched `instanceof`. The default message text changed alongside: v1's unknown-resource error read `Resource not found`; v2's `ResourceNotFoundError` default is `Resource not found: ` (the code is unchanged). Tests pinning the exact string diff --git a/docs/servers/errors.md b/docs/servers/errors.md index b2201ce05d..53f8b8355e 100644 --- a/docs/servers/errors.md +++ b/docs/servers/errors.md @@ -179,7 +179,7 @@ Three more subclasses cover the other structured protocol errors: - `UnsupportedProtocolVersionError({ supported, requested })` — `-32022`; `data.supported` lets the peer pick a version and retry. - `MissingRequiredClientCapabilityError({ requiredCapabilities })` — `-32021`; `data.requiredCapabilities` names exactly what the client must declare. -Match these by `code` and `data` shape, not by `instanceof` — `instanceof` fails across separately bundled copies of the SDK. +Match these by `code` and `data` shape when peers may run pre-brand SDK copies or hand you plain wire shapes; on brand-aware releases `instanceof` also matches across separately bundled copies of the SDK. The same check is available as an explicit static guard — `ProtocolError.isInstance(err)`, `ResourceNotFoundError.isInstance(err)` — which narrows in TypeScript and reads the same brand. ## Look up a protocol error code @@ -205,5 +205,5 @@ Match these by `code` and `data` shape, not by `instanceof` — `instanceof` fai - A tool handler that throws produces the same `isError: true` result; the exception's `message` becomes the `content` text. - A tool handler cannot produce a protocol error — only `UrlElicitationRequiredError` escapes. - `ProtocolError` and its subclasses, thrown from resource, prompt, and completion callbacks, become JSON-RPC error responses the model never sees. -- `ResourceNotFoundError` and the other subclasses pick the code and pack structured `data`; match them by `code` and `data`, not `instanceof`. +- `ResourceNotFoundError` and the other subclasses pick the code and pack structured `data`; match them by `code` and `data` — or, on brand-aware releases, by `instanceof`. - The table above lists every `ProtocolErrorCode` member. diff --git a/examples/oauth/client.ts b/examples/oauth/client.ts index 4da94f4b5d..60ffab7739 100644 --- a/examples/oauth/client.ts +++ b/examples/oauth/client.ts @@ -113,11 +113,12 @@ let challenged = false; try { await client.connect(firstTransport); } catch (error) { - // Under `--legacy` the transport surfaces `UnauthorizedError` directly; - // under `mode: 'auto'` the version-negotiation probe is what got 401'd - // and wraps it in an EraNegotiationFailed `SdkError` whose `data.cause` - // is the original `UnauthorizedError`. Either way the auth driver has - // already run by the time we land here — DCR done, auth URL captured. + // Both `--legacy` and `mode: 'auto'` surface the original + // `UnauthorizedError` directly (the negotiation probe propagates it + // unchanged; older releases wrapped it as the `data.cause` of an + // EraNegotiationFailed `SdkError`, which the unwrap below still + // tolerates). Either way the auth driver has already run by the time we + // land here — DCR done, auth URL captured. const root = error instanceof UnauthorizedError ? error : (error as { data?: { cause?: unknown } }).data?.cause; if (!(root instanceof UnauthorizedError)) throw error; challenged = true; diff --git a/packages/client/src/client/auth.ts b/packages/client/src/client/auth.ts index a4d5b14c62..25bed6d2f5 100644 --- a/packages/client/src/client/auth.ts +++ b/packages/client/src/client/auth.ts @@ -13,7 +13,9 @@ import type { StoredOAuthTokens } from '@modelcontextprotocol/core-internal'; import { + brandedHasInstance, checkResourceAllowed, + stampErrorBrands, LATEST_PROTOCOL_VERSION, OAuthClientInformationFullSchema, OAuthError, @@ -486,8 +488,36 @@ export interface OAuthDiscoveryState extends OAuthServerInfo { export type AuthResult = 'AUTHORIZED' | 'REDIRECT'; export class UnauthorizedError extends Error { + static { + Object.defineProperty(this, 'mcpBrand', { value: 'mcp.UnauthorizedError' }); + } + + static override [Symbol.hasInstance](value: unknown): boolean { + return brandedHasInstance(this, value); + } + + /** + * Brand-based type guard: equivalent to `value instanceof this`, as an + * explicit static predicate (the axios/AWS-SDK `isInstance` style). Reads + * the caller's own brand via `this`, so every branded subclass gets a + * correctly-scoped guard by inheritance. Must be invoked on the class — + * in callback position write `v => SdkError.isInstance(v)`, not + * `.filter(SdkError.isInstance)` (detached calls throw rather than + * silently matching nothing). + */ + static isInstance unknown>(this: T, value: unknown): value is InstanceType { + if (typeof this !== 'function') { + throw new TypeError( + 'isInstance must be called on the class (e.g. `SdkError.isInstance(value)`); for callbacks use `v => SdkError.isInstance(v)`' + ); + } + return brandedHasInstance(this, value); + } + constructor(message?: string) { super(message ?? 'Unauthorized'); + this.name = 'UnauthorizedError'; + stampErrorBrands(this, new.target); } } diff --git a/packages/client/src/client/authErrors.ts b/packages/client/src/client/authErrors.ts index 896eab6e36..e8925b2f86 100644 --- a/packages/client/src/client/authErrors.ts +++ b/packages/client/src/client/authErrors.ts @@ -7,6 +7,7 @@ */ import type { OAuthClientMetadata } from '@modelcontextprotocol/core-internal'; +import { brandedHasInstance, stampErrorBrands } from '@modelcontextprotocol/core-internal'; /** * Base class for the OAuth-client-flow error family. Concrete subclasses are @@ -19,9 +20,36 @@ import type { OAuthClientMetadata } from '@modelcontextprotocol/core-internal'; * hook and will not match anything until the first behavior change ships. */ export class OAuthClientFlowError extends Error { + static { + Object.defineProperty(this, 'mcpBrand', { value: 'mcp.OAuthClientFlowError' }); + } + + static override [Symbol.hasInstance](value: unknown): boolean { + return brandedHasInstance(this, value); + } + + /** + * Brand-based type guard: equivalent to `value instanceof this`, as an + * explicit static predicate (the axios/AWS-SDK `isInstance` style). Reads + * the caller's own brand via `this`, so every branded subclass gets a + * correctly-scoped guard by inheritance. Must be invoked on the class — + * in callback position write `v => SdkError.isInstance(v)`, not + * `.filter(SdkError.isInstance)` (detached calls throw rather than + * silently matching nothing). + */ + static isInstance unknown>(this: T, value: unknown): value is InstanceType { + if (typeof this !== 'function') { + throw new TypeError( + 'isInstance must be called on the class (e.g. `SdkError.isInstance(value)`); for callbacks use `v => SdkError.isInstance(v)`' + ); + } + return brandedHasInstance(this, value); + } + constructor(message: string) { super(message); this.name = new.target.name; + stampErrorBrands(this, new.target); } } @@ -45,6 +73,10 @@ export class OAuthClientFlowError extends Error { * end users. The values are JSON-encoded in the message to neutralize log-injection. */ export class IssuerMismatchError extends OAuthClientFlowError { + static { + Object.defineProperty(this, 'mcpBrand', { value: 'mcp.IssuerMismatchError' }); + } + /** Which check failed — metadata echo (RFC 8414 §3.3) or authorization-response `iss` (RFC 9207). */ readonly kind: 'metadata' | 'authorization_response'; /** The issuer the client expected (from validated metadata / discovery input). */ @@ -79,6 +111,10 @@ export class IssuerMismatchError extends OAuthClientFlowError { * path. */ export class RegistrationRejectedError extends OAuthClientFlowError { + static { + Object.defineProperty(this, 'mcpBrand', { value: 'mcp.RegistrationRejectedError' }); + } + /** HTTP status code returned by the registration endpoint. */ public readonly status: number; /** Raw response body text (typically an RFC 7591 error JSON document). */ @@ -102,6 +138,10 @@ export class RegistrationRejectedError extends OAuthClientFlowError { * of falling through to a fresh `/authorize` redirect. */ export class InsecureTokenEndpointError extends OAuthClientFlowError { + static { + Object.defineProperty(this, 'mcpBrand', { value: 'mcp.InsecureTokenEndpointError' }); + } + /** The token endpoint URL that was rejected. */ public readonly tokenEndpoint: string; @@ -145,6 +185,10 @@ export class InsecureTokenEndpointError extends OAuthClientFlowError { * client credentials are protected structurally by the `issuer` stamp instead. */ export class AuthorizationServerMismatchError extends OAuthClientFlowError { + static { + Object.defineProperty(this, 'mcpBrand', { value: 'mcp.AuthorizationServerMismatchError' }); + } + constructor( /** The issuer recorded in `discoveryState()` when the authorization redirect was issued. */ public readonly recordedIssuer: string, @@ -160,6 +204,10 @@ export class AuthorizationServerMismatchError extends OAuthClientFlowError { } export class InsufficientScopeError extends OAuthClientFlowError { + static { + Object.defineProperty(this, 'mcpBrand', { value: 'mcp.InsufficientScopeError' }); + } + /** The `scope` value from the `WWW-Authenticate` challenge — the scopes the resource server says are required. */ readonly requiredScope?: string; /** The `resource_metadata` URL from the `WWW-Authenticate` challenge, if present. */ diff --git a/packages/client/src/client/probeClassifier.ts b/packages/client/src/client/probeClassifier.ts index 37c2f65701..bcc4ff5fca 100644 --- a/packages/client/src/client/probeClassifier.ts +++ b/packages/client/src/client/probeClassifier.ts @@ -46,6 +46,8 @@ export type ProbeOutcome = /** The HTTP layer rejected the probe POST (non-2xx); `body` is the raw response text, when available. */ | { kind: 'http-error'; status: number; body?: string } | { kind: 'network-error'; error: unknown } + /** The transport's auth flow challenged during the probe send (`UnauthorizedError`). */ + | { kind: 'auth-required'; error: Error } /** No response arrived within the probe timeout. */ | { kind: 'timeout'; timeoutMs: number }; @@ -114,6 +116,15 @@ export function classifyProbeOutcome(outcome: ProbeOutcome, context: ProbeClassi case 'network-error': { return classifyNetworkError(outcome.error, context); } + case 'auth-required': { + // Not era evidence: propagate the auth challenge unchanged so the + // caller can run finishAuth() and reconnect — the reconnect probes + // again with the token and settles the era with real evidence. + // Converting to a legacy fallback here would re-run the auth flow + // inside the same connect (a second authorization prompt) and then + // handshake an auth-gated modern server as legacy. + return { kind: 'error', error: outcome.error }; + } case 'timeout': { if (context.transportKind === 'stdio') { // Per the stdio transport's backward-compatibility rule, a probe diff --git a/packages/client/src/client/sse.ts b/packages/client/src/client/sse.ts index b983fcbcb3..1c81928f10 100644 --- a/packages/client/src/client/sse.ts +++ b/packages/client/src/client/sse.ts @@ -1,11 +1,13 @@ import type { FetchLike, JSONRPCMessage, Transport } from '@modelcontextprotocol/core-internal'; import { + brandedHasInstance, createFetchWithInit, JSONRPCMessageSchema, normalizeHeaders, SdkError, SdkErrorCode, - SdkHttpError + SdkHttpError, + stampErrorBrands } from '@modelcontextprotocol/core-internal'; import type { ErrorEvent, EventSourceInit } from 'eventsource'; import { EventSource } from 'eventsource'; @@ -23,12 +25,39 @@ import { import type { IssuerMismatchError } from './authErrors'; export class SseError extends Error { + static { + Object.defineProperty(this, 'mcpBrand', { value: 'mcp.SseError' }); + } + + static override [Symbol.hasInstance](value: unknown): boolean { + return brandedHasInstance(this, value); + } + + /** + * Brand-based type guard: equivalent to `value instanceof this`, as an + * explicit static predicate (the axios/AWS-SDK `isInstance` style). Reads + * the caller's own brand via `this`, so every branded subclass gets a + * correctly-scoped guard by inheritance. Must be invoked on the class — + * in callback position write `v => SdkError.isInstance(v)`, not + * `.filter(SdkError.isInstance)` (detached calls throw rather than + * silently matching nothing). + */ + static isInstance unknown>(this: T, value: unknown): value is InstanceType { + if (typeof this !== 'function') { + throw new TypeError( + 'isInstance must be called on the class (e.g. `SdkError.isInstance(value)`); for callbacks use `v => SdkError.isInstance(v)`' + ); + } + return brandedHasInstance(this, value); + } + constructor( public readonly code: number | undefined, message: string | undefined, public readonly event: ErrorEvent ) { super(`SSE error: ${message}`); + stampErrorBrands(this, new.target); } } diff --git a/packages/client/src/client/versionNegotiation.ts b/packages/client/src/client/versionNegotiation.ts index bced563626..d438ff7a81 100644 --- a/packages/client/src/client/versionNegotiation.ts +++ b/packages/client/src/client/versionNegotiation.ts @@ -25,6 +25,7 @@ import { SUPPORTED_MODERN_PROTOCOL_VERSIONS } from '@modelcontextprotocol/core-internal'; +import { UnauthorizedError } from './auth'; import type { ProbeEnvironment, ProbeOutcome, ProbeTransportKind, ProbeVerdict } from './probeClassifier'; import { classifyProbeOutcome } from './probeClassifier'; @@ -293,10 +294,18 @@ function normalizeReply(reply: RawProbeReply, timeoutMs: number): ProbeOutcome { const text = (error.data as { text?: unknown } | undefined)?.text; return { kind: 'http-error', status: error.data.status, body: typeof text === 'string' ? text : undefined }; } - if (error instanceof Error && error.name === 'UnauthorizedError') { - // Auth-gated server: not era evidence — the conservative legacy - // fallback re-runs the auth flow through the plain connect path. - return { kind: 'http-error', status: 401 }; + const isUnauthorized = + 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.) + return { kind: 'auth-required', error: error as Error }; } return { kind: 'network-error', error }; } diff --git a/packages/client/test/client/errorBrandConformance.test.ts b/packages/client/test/client/errorBrandConformance.test.ts new file mode 100644 index 0000000000..7265b8c29b --- /dev/null +++ b/packages/client/test/client/errorBrandConformance.test.ts @@ -0,0 +1,130 @@ +import { describe, expect, it, vi } from 'vitest'; + +import * as barrel from '../../src/index'; + +/** + * Enforces the cross-bundle branding participation criterion from + * `core-internal/src/errors/crossBundleBrand.ts`: every error class exported + * from this package's public surface must carry an own `mcpBrand` static and + * resolve `instanceof` through a branded `Symbol.hasInstance`. + * + * This is the infrastructure that replaces "remember the static block": adding + * a new exported error class without branding turns this test red naming the + * class, instead of shipping a subclass whose cross-bundle `instanceof` + * silently returns false while its parent still matches. + */ + +/** Error classes exported on purpose without a brand. Justify every entry. */ +const UNBRANDED_ALLOWLIST: ReadonlySet = new Set(); + +function exportedErrorClasses(mod: Record): Array<[string, Function]> { + return Object.entries(mod) + .filter((entry): entry is [string, Function] => { + const v = entry[1]; + return typeof v === 'function' && v !== Error && v.prototype instanceof Error; + }) + .sort(([a], [b]) => a.localeCompare(b)); +} + +describe('error brand conformance (client export surface)', () => { + const classes = exportedErrorClasses(barrel as Record); + + it('finds the export surface (guards against a broken walker)', () => { + expect(classes.length).toBeGreaterThanOrEqual(15); + expect(classes.map(([n]) => n)).toContain('ProtocolError'); + expect(classes.map(([n]) => n)).toContain('OAuthClientFlowError'); + }); + + it('every exported error class is branded (or explicitly allowlisted)', () => { + const unbranded = classes + .filter(([name]) => !UNBRANDED_ALLOWLIST.has(name)) + .filter(([, cls]) => !Object.prototype.hasOwnProperty.call(cls, 'mcpBrand')) + .map(([name]) => name); + expect( + unbranded, + `unbranded exported error classes (add the static mcpBrand block, or allowlist with a justification): ${unbranded.join(', ')}` + ).toEqual([]); + }); + + it('every branded class resolves instanceof through a branded hasInstance', () => { + const missing = classes + .filter(([, cls]) => Object.prototype.hasOwnProperty.call(cls, 'mcpBrand')) + .filter(([, cls]) => (cls as never as Record)[Symbol.hasInstance] === Function.prototype[Symbol.hasInstance]) + .map(([name]) => name); + expect(missing, `branded classes whose hierarchy root does not install brandedHasInstance: ${missing.join(', ')}`).toEqual([]); + }); + + // Core-internal brand strings are pinned once, in core-internal's + // errorSurfacePins.test.ts — this test only asserts the core re-export SET + // so a rename there is not double-maintained here. + const CORE_PINNED = new Set([ + 'MissingRequiredClientCapabilityError', + 'OAuthError', + 'ProtocolError', + 'ResourceNotFoundError', + 'SdkError', + 'SdkHttpError', + 'UnsupportedProtocolVersionError', + 'UrlElicitationRequiredError' + ]); + + it('pins every client-local brand string (renaming one severs cross-version matching — must be deliberate)', () => { + const brands = Object.fromEntries( + classes + .filter(([name, cls]) => !CORE_PINNED.has(name) && Object.prototype.hasOwnProperty.call(cls, 'mcpBrand')) + .map(([name, cls]) => [name, (cls as never as { mcpBrand: string }).mcpBrand]) + ); + expect(brands).toEqual({ + AuthorizationServerMismatchError: 'mcp.AuthorizationServerMismatchError', + InsecureTokenEndpointError: 'mcp.InsecureTokenEndpointError', + InsufficientScopeError: 'mcp.InsufficientScopeError', + IssuerMismatchError: 'mcp.IssuerMismatchError', + OAuthClientFlowError: 'mcp.OAuthClientFlowError', + RegistrationRejectedError: 'mcp.RegistrationRejectedError', + SseError: 'mcp.SseError', + UnauthorizedError: 'mcp.UnauthorizedError' + }); + }); + + it('every branded class exposes an isInstance guard that agrees with instanceof', () => { + const branded = classes.filter(([, cls]) => Object.prototype.hasOwnProperty.call(cls, 'mcpBrand')); + const missing = branded.filter(([, cls]) => typeof (cls as { isInstance?: unknown }).isInstance !== 'function').map(([n]) => n); + expect(missing, `branded classes without a static isInstance guard: ${missing.join(', ')}`).toEqual([]); + for (const [, cls] of branded) { + const guard = (cls as unknown as { isInstance: (v: unknown) => boolean }).isInstance; + for (const v of [new Error('plain'), null, undefined, 0, '', {}]) { + expect(guard.call(cls, v)).toBe(v instanceof (cls as never as new () => unknown)); + } + } + }); + + it('core re-exported error classes are exactly the set pinned in errorSurfacePins.test.ts', () => { + const coreExported = classes.map(([name]) => name).filter(name => CORE_PINNED.has(name)); + expect(coreExported).toEqual([...CORE_PINNED].sort((a, b) => a.localeCompare(b))); + }); + + it('the OAuth flow family and UnauthorizedError match across module copies', async () => { + vi.resetModules(); + const foreignAuthErrors = await import('../../src/client/authErrors'); + const foreignAuth = await import('../../src/client/auth'); + // Guard the premise: a cached module would make every check below pass + // trivially through prototype identity. + expect(foreignAuthErrors.IssuerMismatchError).not.toBe(barrel.IssuerMismatchError); + expect(foreignAuth.UnauthorizedError).not.toBe(barrel.UnauthorizedError); + + const issuer = new foreignAuthErrors.IssuerMismatchError('metadata', 'a', 'b'); + expect(issuer instanceof barrel.IssuerMismatchError).toBe(true); + expect(issuer instanceof barrel.OAuthClientFlowError).toBe(true); + expect(issuer instanceof barrel.UnauthorizedError).toBe(false); + + const unauthorized = new foreignAuth.UnauthorizedError('nope'); + expect(unauthorized instanceof barrel.UnauthorizedError).toBe(true); + expect(unauthorized.name).toBe('UnauthorizedError'); + + // the isInstance guards read the same brands: cross-copy agreement + expect(barrel.IssuerMismatchError.isInstance(issuer)).toBe(true); + expect(barrel.OAuthClientFlowError.isInstance(issuer)).toBe(true); + expect(barrel.UnauthorizedError.isInstance(issuer)).toBe(false); + expect(barrel.UnauthorizedError.isInstance(unauthorized)).toBe(true); + }); +}); diff --git a/packages/client/test/client/sseErrorBrand.test.ts b/packages/client/test/client/sseErrorBrand.test.ts new file mode 100644 index 0000000000..a806accc7f --- /dev/null +++ b/packages/client/test/client/sseErrorBrand.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { SseError } from '../../src/client/sse'; + +describe('SseError cross-bundle instanceof', () => { + it('same-bundle instanceof and fields keep working', () => { + const err = new SseError(401, 'unauthorized', {} as never); + expect(err instanceof SseError).toBe(true); + expect(err instanceof Error).toBe(true); + expect(err.code).toBe(401); + }); + + it('an instance from a second module copy satisfies instanceof against this copy', async () => { + vi.resetModules(); + const copy2 = await import('../../src/client/sse'); + expect(copy2.SseError).not.toBe(SseError); + const foreign = new copy2.SseError(403, 'forbidden', {} as never); + expect(foreign instanceof SseError).toBe(true); + }); + + it('stays disjoint from the SdkError hierarchy', async () => { + const { SdkError } = await import('@modelcontextprotocol/core-internal'); + const err = new SseError(500, 'boom', {} as never); + expect((err as unknown) instanceof SdkError).toBe(false); + }); +}); diff --git a/packages/client/test/client/versionNegotiation.test.ts b/packages/client/test/client/versionNegotiation.test.ts index 7a41347ae1..86d901e24a 100644 --- a/packages/client/test/client/versionNegotiation.test.ts +++ b/packages/client/test/client/versionNegotiation.test.ts @@ -18,6 +18,7 @@ import { } from '@modelcontextprotocol/core-internal'; import { describe, expect, test } from 'vitest'; +import { UnauthorizedError } from '../../src/client/auth'; import { Client } from '../../src/client/client'; import type { StreamableHTTPClientTransportOptions } from '../../src/client/streamableHttp'; import type { StdioServerParameters } from '../../src/client/stdio'; @@ -706,3 +707,78 @@ describe('era scope discipline', () => { expect(noCachedEra).toBe(true); }); }); + +/* ------------------------------------------------------------------------- * + * Probe send-error classification: auth-gated servers propagate the + * UnauthorizedError unchanged (finishAuth() + reconnect probes again); other + * send failures stay typed negotiation errors. + * ------------------------------------------------------------------------- */ + +describe('probe send-error classification', () => { + /** Rejects the probe send with `probeError`, then serves legacy initialize. */ + class AuthGatedTransport extends ScriptedTransport { + constructor(private readonly probeError: Error) { + super(legacyServerScript); + } + + override async send(message: JSONRPCMessage): Promise { + if (isJSONRPCRequest(message) && message.method === 'server/discover') { + throw this.probeError; + } + await super.send(message); + } + } + + test('UnauthorizedError from the probe send propagates unchanged — no fallback, no second auth round (finishAuth + reconnect probes again)', async () => { + const reason = new UnauthorizedError(); + 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 + ); + + // The original error, unwrapped: callers dispatch finishAuth() on it. + expect(rejection).toBe(reason); + // No initialize fallback ran — that would re-trigger the transport's + // auth flow (a second authorization prompt) and pin an auth-gated + // modern server to the legacy era. + expect(requests(transport.sent).some(r => r.method === 'initialize')).toBe(false); + }); + + test("a foreign auth error matching only by name === 'UnauthorizedError' propagates the same way", async () => { + class ForeignUnauthorizedError extends Error { + override readonly name = 'UnauthorizedError'; + } + const reason = new ForeignUnauthorizedError('401 from middleware'); + 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' } }); + + const rejection = await client.connect(transport).then( + () => { + throw new Error('connect unexpectedly resolved'); + }, + (e: unknown) => e + ); + expect(rejection).toBeInstanceOf(SdkError); + expect((rejection as SdkError).code).toBe(SdkErrorCode.EraNegotiationFailed); + expect(requests(transport.sent).some(r => r.method === 'initialize')).toBe(false); + }); +}); diff --git a/packages/core-internal/src/auth/errors.ts b/packages/core-internal/src/auth/errors.ts index 6977ae0e57..13f0558fad 100644 --- a/packages/core-internal/src/auth/errors.ts +++ b/packages/core-internal/src/auth/errors.ts @@ -1,3 +1,4 @@ +import { brandedHasInstance, stampErrorBrands } from '../errors/crossBundleBrand'; import type { OAuthErrorResponse } from '../shared/auth'; /** @@ -103,6 +104,32 @@ export enum OAuthErrorCode { * OAuth error class for all OAuth-related errors. */ export class OAuthError extends Error { + static { + Object.defineProperty(this, 'mcpBrand', { value: 'mcp.OAuthError' }); + } + + static override [Symbol.hasInstance](value: unknown): boolean { + return brandedHasInstance(this, value); + } + + /** + * Brand-based type guard: equivalent to `value instanceof this`, as an + * explicit static predicate (the axios/AWS-SDK `isInstance` style). Reads + * the caller's own brand via `this`, so every branded subclass gets a + * correctly-scoped guard by inheritance. Must be invoked on the class — + * in callback position write `v => SdkError.isInstance(v)`, not + * `.filter(SdkError.isInstance)` (detached calls throw rather than + * silently matching nothing). + */ + static isInstance unknown>(this: T, value: unknown): value is InstanceType { + if (typeof this !== 'function') { + throw new TypeError( + 'isInstance must be called on the class (e.g. `SdkError.isInstance(value)`); for callbacks use `v => SdkError.isInstance(v)`' + ); + } + return brandedHasInstance(this, value); + } + constructor( public readonly code: OAuthErrorCode | string, message: string, @@ -110,6 +137,7 @@ export class OAuthError extends Error { ) { super(message); this.name = 'OAuthError'; + stampErrorBrands(this, new.target); } /** diff --git a/packages/core-internal/src/errors/crossBundleBrand.ts b/packages/core-internal/src/errors/crossBundleBrand.ts new file mode 100644 index 0000000000..1422a092f7 --- /dev/null +++ b/packages/core-internal/src/errors/crossBundleBrand.ts @@ -0,0 +1,117 @@ +/** + * Cross-bundle `instanceof` support for the SDK error classes. + * + * `@modelcontextprotocol/client` and `@modelcontextprotocol/server` each bundle their + * own copy of `core-internal`, so an error constructed by one package fails a + * prototype-identity `instanceof` against the same class re-exported by the other — + * exactly the check a dual-role process (gateway, host, in-process test) writes. + * + * Instead of prototype identity, branded classes stamp every instance with the brand + * strings of its class chain under a registry symbol (`Symbol.for`, shared across + * bundles and realms), and resolve `instanceof` via `Symbol.hasInstance` against the + * brand set. Ordinary prototype-based `instanceof` is kept as a fallback so behavior + * is unchanged for anything unbranded. + * + * A class participates by defining an **own** `mcpBrand` static (via a `static {}` + * block, so nothing reaches the declaration files — a declared `protected static` + * field would make the constructor types nominally incompatible across the bundled + * copies) and (for hierarchy roots) installing {@linkcode brandedHasInstance} as + * `Symbol.hasInstance`. User-defined subclasses that do not declare their own brand + * keep plain prototype semantics — a foreign base-class instance never satisfies + * `instanceof UserSubclass`. + * + * Prior art — the same stamp-and-hook shape ships at scale elsewhere: Node core + * (stream.Writable since 2017, Console via a marker symbol, diagnostics_channel), + * undici's whole error hierarchy (vendored into Node as fetch), googleapis/gaxios + * (GaxiosError, Symbol.for marker), AWS SDK v3's ServiceException (which pairs a + * Symbol.hasInstance override with a static isInstance guard, as we do), and zod v4 + * (Symbol.hasInstance on every schema class for cross-version interop). + * + * Contract notes: + * - Participation criterion: **every error class exported from a public package that + * callers are documented to `instanceof` must be branded.** The per-package + * errorBrandConformance tests walk the export surfaces and fail naming any + * exported Error subclass that has not opted in. + * - Brands assert **identity, not shape**: brand strings are version-less, so an + * instance from one SDK version matches the class of another. Members added to a + * branded class in a later version may be absent on a matched instance — read + * fields defensively, and treat branded classes as additive-only. The escape + * hatch when a release must break a branded class's read contract: change that + * class's brand string in the same release, which cleanly severs cross-version + * matching for that class. The per-package brand pins make the rename + * deliberate: errorSurfacePins.test.ts owns the core-internal brands, and each + * package's errorBrandConformance test pins its package-local ones. + * - Cross-bundle matching requires **both** copies to be at or after the release + * that introduced branding; against an older copy, behavior degrades to plain + * prototype `instanceof` in both directions. + * - A consumer re-bundling the SDK with property mangling (`mangle.props`) would + * break the brand statics; default esbuild/webpack/terser settings do not. + */ + +/** Registry symbol — identical across bundled copies and realms. */ +const BRANDS: unique symbol = Symbol.for('mcp.sdk.errorBrands') as never; + +interface BrandCarrier { + [BRANDS]?: { has(brand: string): boolean }; +} + +interface BrandedConstructor { + mcpBrand?: string; +} + +/** + * Stamp `instance` with the brand of every class in `ctor`'s chain that declares an + * own `mcpBrand`. Call once from the hierarchy root's constructor with `new.target` — + * subclasses inherit the stamping without touching their constructors. + * + * Constructor-time only: never stamp arbitrary objects. A stamped non-instance would + * satisfy `instanceof` while lacking the prototype members (getters like `.status`) + * that callers reach for after the check. + */ +export function stampErrorBrands(instance: object, ctor: unknown): void { + const brands = new Set(); + let current: unknown = ctor; + while (typeof current === 'function') { + const brand = (current as BrandedConstructor).mcpBrand; + if (Object.prototype.hasOwnProperty.call(current, 'mcpBrand') && typeof brand === 'string') { + brands.add(brand); + } + current = Object.getPrototypeOf(current); + } + if (brands.size === 0) return; + // configurable so userland instrumentation that clones/replaces error objects + // (e.g. wrapping via Object.defineProperties) is not broken by a frozen stamp; + // deleting the stamp merely degrades that instance to prototype `instanceof`. + Object.defineProperty(instance, BRANDS, { value: brands, enumerable: false, configurable: true }); +} + +/** + * `Symbol.hasInstance` implementation for branded hierarchy roots. Matches when the + * value carries the **own** brand of the class being tested against (cross-bundle + * path), falling back to ordinary prototype-based `instanceof` otherwise. + */ +export function brandedHasInstance(cls: object, value: unknown): boolean { + try { + if ( + typeof value === 'object' && + value !== null && + Object.prototype.hasOwnProperty.call(cls, 'mcpBrand') && + typeof (cls as BrandedConstructor).mcpBrand === 'string' && + // Own-property only: a brand inherited via the prototype chain is never + // honored, so polluting Object.prototype with the registry symbol cannot + // make arbitrary objects satisfy instanceof (real instances are stamped + // with an own property by stampErrorBrands). + Object.prototype.hasOwnProperty.call(value, BRANDS) + ) { + const carried = (value as BrandCarrier)[BRANDS]; + if (carried && typeof carried.has === 'function' && carried.has((cls as BrandedConstructor).mcpBrand!)) { + return true; + } + } + } catch { + // A hostile Proxy trap or throwing accessor must not make `instanceof` + // throw where it previously returned false — fall through to the + // ordinary prototype check. + } + return Function.prototype[Symbol.hasInstance].call(cls, value); +} diff --git a/packages/core-internal/src/errors/sdkErrors.ts b/packages/core-internal/src/errors/sdkErrors.ts index b808d98877..21f42f6ef8 100644 --- a/packages/core-internal/src/errors/sdkErrors.ts +++ b/packages/core-internal/src/errors/sdkErrors.ts @@ -1,3 +1,5 @@ +import { brandedHasInstance, stampErrorBrands } from './crossBundleBrand'; + /** * Error codes for SDK errors (local errors that never cross the wire). * Unlike {@linkcode ProtocolErrorCode} which uses numeric JSON-RPC codes, `SdkErrorCode` uses @@ -98,6 +100,32 @@ export enum SdkErrorCode { * ``` */ export class SdkError extends Error { + static { + Object.defineProperty(this, 'mcpBrand', { value: 'mcp.SdkError' }); + } + + static override [Symbol.hasInstance](value: unknown): boolean { + return brandedHasInstance(this, value); + } + + /** + * Brand-based type guard: equivalent to `value instanceof this`, as an + * explicit static predicate (the axios/AWS-SDK `isInstance` style). Reads + * the caller's own brand via `this`, so every branded subclass gets a + * correctly-scoped guard by inheritance. Must be invoked on the class — + * in callback position write `v => SdkError.isInstance(v)`, not + * `.filter(SdkError.isInstance)` (detached calls throw rather than + * silently matching nothing). + */ + static isInstance unknown>(this: T, value: unknown): value is InstanceType { + if (typeof this !== 'function') { + throw new TypeError( + 'isInstance must be called on the class (e.g. `SdkError.isInstance(value)`); for callbacks use `v => SdkError.isInstance(v)`' + ); + } + return brandedHasInstance(this, value); + } + constructor( public readonly code: SdkErrorCode, message: string, @@ -105,6 +133,7 @@ export class SdkError extends Error { ) { super(message); this.name = 'SdkError'; + stampErrorBrands(this, new.target); } } @@ -134,6 +163,10 @@ export interface SdkHttpErrorData { * ``` */ export class SdkHttpError extends SdkError { + static { + Object.defineProperty(this, 'mcpBrand', { value: 'mcp.SdkHttpError' }); + } + declare readonly data: SdkHttpErrorData; constructor(code: SdkErrorCode, message: string, data: SdkHttpErrorData) { diff --git a/packages/core-internal/src/index.ts b/packages/core-internal/src/index.ts index ab66a038b9..50238566cf 100644 --- a/packages/core-internal/src/index.ts +++ b/packages/core-internal/src/index.ts @@ -1,4 +1,5 @@ export * from './auth/errors'; +export * from './errors/crossBundleBrand'; export * from './errors/sdkErrors'; export * from './shared/auth'; export * from './shared/authUtils'; diff --git a/packages/core-internal/src/types/errors.ts b/packages/core-internal/src/types/errors.ts index 8f268c3a8c..b079b8faa2 100644 --- a/packages/core-internal/src/types/errors.ts +++ b/packages/core-internal/src/types/errors.ts @@ -1,3 +1,4 @@ +import { brandedHasInstance, stampErrorBrands } from '../errors/crossBundleBrand'; import { ProtocolErrorCode } from './enums'; import type { ClientCapabilities, @@ -9,8 +10,39 @@ import type { /** * Protocol errors are JSON-RPC errors that cross the wire as error responses. * They use numeric error codes from the {@linkcode ProtocolErrorCode} enum. + * + * `instanceof` on this class (and its subclasses) is brand-matched, so it works + * across separately bundled copies of the SDK — e.g. an error constructed by + * `@modelcontextprotocol/client` matches the class re-exported by + * `@modelcontextprotocol/server` in the same process. */ export class ProtocolError extends Error { + static { + Object.defineProperty(this, 'mcpBrand', { value: 'mcp.ProtocolError' }); + } + + static override [Symbol.hasInstance](value: unknown): boolean { + return brandedHasInstance(this, value); + } + + /** + * Brand-based type guard: equivalent to `value instanceof this`, as an + * explicit static predicate (the axios/AWS-SDK `isInstance` style). Reads + * the caller's own brand via `this`, so every branded subclass gets a + * correctly-scoped guard by inheritance. Must be invoked on the class — + * in callback position write `v => SdkError.isInstance(v)`, not + * `.filter(SdkError.isInstance)` (detached calls throw rather than + * silently matching nothing). + */ + static isInstance unknown>(this: T, value: unknown): value is InstanceType { + if (typeof this !== 'function') { + throw new TypeError( + 'isInstance must be called on the class (e.g. `SdkError.isInstance(value)`); for callbacks use `v => SdkError.isInstance(v)`' + ); + } + return brandedHasInstance(this, value); + } + constructor( public readonly code: number, message: string, @@ -18,6 +50,7 @@ export class ProtocolError extends Error { ) { super(message); this.name = 'ProtocolError'; + stampErrorBrands(this, new.target); } /** @@ -86,11 +119,14 @@ export class ProtocolError extends Error { * accept `-32002` as resource not found — earlier SDK builds emitted that * code, and {@linkcode ProtocolError.fromError} reconstructs this class for * either code **when `error.data` carries `uri`** (a bare `-32002` without - * `data.uri` stays a generic {@linkcode ProtocolError}). Do not rely on - * `instanceof` — it does not work across separately bundled copies of the - * SDK. + * `data.uri` stays a generic {@linkcode ProtocolError}). `instanceof` checks + * are brand-matched and work across separately bundled copies of the SDK. */ export class ResourceNotFoundError extends ProtocolError { + static { + Object.defineProperty(this, 'mcpBrand', { value: 'mcp.ResourceNotFoundError' }); + } + constructor(uri: string, message: string = `Resource not found: ${uri}`) { super(ProtocolErrorCode.InvalidParams, message, { uri }); } @@ -106,6 +142,10 @@ export class ResourceNotFoundError extends ProtocolError { * This makes it nicer for the client to handle since there is specific data to work with instead of just a code to check against. */ export class UrlElicitationRequiredError extends ProtocolError { + static { + Object.defineProperty(this, 'mcpBrand', { value: 'mcp.UrlElicitationRequiredError' }); + } + constructor(elicitations: ElicitRequestURLParams[], message: string = `URL elicitation${elicitations.length > 1 ? 's' : ''} required`) { super(ProtocolErrorCode.UrlElicitationRequired, message, { elicitations: elicitations @@ -127,6 +167,10 @@ export class UrlElicitationRequiredError extends ProtocolError { * version that was requested (`requested`). */ export class UnsupportedProtocolVersionError extends ProtocolError { + static { + Object.defineProperty(this, 'mcpBrand', { value: 'mcp.UnsupportedProtocolVersionError' }); + } + constructor(data: UnsupportedProtocolVersionErrorData, message: string = `Unsupported protocol version: ${data.requested}`) { super(ProtocolErrorCode.UnsupportedProtocolVersion, message, data); } @@ -156,11 +200,15 @@ export class UnsupportedProtocolVersionError extends ProtocolError { * have to declare for the request to be served. On HTTP, the response status * is `400 Bad Request`. * - * Recognize this error by its code and `data.requiredCapabilities` rather than - * by class identity (`instanceof` does not work across separately bundled - * copies of the SDK). + * Recognize this error by its code and `data.requiredCapabilities`, or by + * `instanceof` — checks are brand-matched and work across separately bundled + * copies of the SDK. */ export class MissingRequiredClientCapabilityError extends ProtocolError { + static { + Object.defineProperty(this, 'mcpBrand', { value: 'mcp.MissingRequiredClientCapabilityError' }); + } + constructor( data: MissingRequiredClientCapabilityErrorData, message: string = `Missing required client capabilities: ${Object.keys(data.requiredCapabilities).join(', ')}` diff --git a/packages/core-internal/test/errors/crossBundleBrand.test.ts b/packages/core-internal/test/errors/crossBundleBrand.test.ts new file mode 100644 index 0000000000..ed9c42507b --- /dev/null +++ b/packages/core-internal/test/errors/crossBundleBrand.test.ts @@ -0,0 +1,197 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { OAuthError, OAuthErrorCode } from '../../src/auth/errors'; +import { SdkError, SdkErrorCode, SdkHttpError } from '../../src/errors/sdkErrors'; +import { ProtocolErrorCode } from '../../src/types/enums'; +import { + MissingRequiredClientCapabilityError, + ProtocolError, + ResourceNotFoundError, + UnsupportedProtocolVersionError, + UrlElicitationRequiredError +} from '../../src/types/errors'; + +// `@modelcontextprotocol/client` and `@modelcontextprotocol/server` each bundle their +// own copy of core-internal; `vi.resetModules()` + a dynamic import reproduces that +// dual-copy situation faithfully (separate class objects, shared registry symbol). +async function loadForeignCopies() { + vi.resetModules(); + const sdkErrors = await import('../../src/errors/sdkErrors'); + const protocolErrors = await import('../../src/types/errors'); + const authErrors = await import('../../src/auth/errors'); + return { sdkErrors, protocolErrors, authErrors }; +} + +describe('cross-bundle error instanceof (branded)', () => { + it('same-bundle instanceof keeps working for every class', () => { + const http = new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, 'boom', { status: 404 }); + expect(http instanceof SdkHttpError).toBe(true); + expect(http instanceof SdkError).toBe(true); + expect(http instanceof Error).toBe(true); + + const proto = new ProtocolError(ProtocolErrorCode.InvalidParams, 'bad'); + expect(proto instanceof ProtocolError).toBe(true); + + const rnf = new ResourceNotFoundError('file:///missing'); + expect(rnf instanceof ResourceNotFoundError).toBe(true); + expect(rnf instanceof ProtocolError).toBe(true); + + const oauth = new OAuthError(OAuthErrorCode.InvalidToken, 'nope'); + expect(oauth instanceof OAuthError).toBe(true); + }); + + it('hierarchies stay disjoint', () => { + const sdk = new SdkError(SdkErrorCode.NotConnected, 'x'); + const proto = new ProtocolError(ProtocolErrorCode.InvalidParams, 'x'); + expect(sdk instanceof ProtocolError).toBe(false); + expect(proto instanceof SdkError).toBe(false); + expect(proto instanceof OAuthError).toBe(false); + + const rnf = new ResourceNotFoundError('file:///missing'); + expect(rnf instanceof UrlElicitationRequiredError).toBe(false); + expect(rnf instanceof UnsupportedProtocolVersionError).toBe(false); + expect(rnf instanceof MissingRequiredClientCapabilityError).toBe(false); + }); + + it('an instance from a second module copy satisfies instanceof against this copy (and vice versa)', async () => { + const { sdkErrors, protocolErrors, authErrors } = await loadForeignCopies(); + + // Sanity: these really are different class objects. + expect(sdkErrors.SdkHttpError).not.toBe(SdkHttpError); + expect(protocolErrors.ProtocolError).not.toBe(ProtocolError); + + const foreignHttp = new sdkErrors.SdkHttpError(sdkErrors.SdkErrorCode.ClientHttpAuthentication, '401', { status: 401 }); + expect(foreignHttp instanceof SdkHttpError).toBe(true); + expect(foreignHttp instanceof SdkError).toBe(true); + expect(foreignHttp instanceof ProtocolError).toBe(false); + + const localHttp = new SdkHttpError(SdkErrorCode.ClientHttpAuthentication, '401', { status: 401 }); + expect(localHttp instanceof sdkErrors.SdkHttpError).toBe(true); + expect(localHttp instanceof sdkErrors.SdkError).toBe(true); + + const foreignRnf = new protocolErrors.ResourceNotFoundError('file:///missing'); + expect(foreignRnf instanceof ResourceNotFoundError).toBe(true); + expect(foreignRnf instanceof ProtocolError).toBe(true); + expect(foreignRnf instanceof UrlElicitationRequiredError).toBe(false); + + const foreignPlainProto = new protocolErrors.ProtocolError(ProtocolErrorCode.InvalidParams, 'x'); + expect(foreignPlainProto instanceof ProtocolError).toBe(true); + expect(foreignPlainProto instanceof ResourceNotFoundError).toBe(false); + + const foreignOauth = new authErrors.OAuthError(authErrors.OAuthErrorCode.InvalidGrant, 'x'); + expect(foreignOauth instanceof OAuthError).toBe(true); + expect(foreignOauth instanceof SdkError).toBe(false); + }); + + it('ProtocolError.fromError from a second copy reconstructs instances this copy recognizes', async () => { + const { protocolErrors } = await loadForeignCopies(); + const reconstructed = protocolErrors.ProtocolError.fromError(ProtocolErrorCode.InvalidParams, 'gone', { + uri: 'file:///missing' + }); + expect(reconstructed instanceof ResourceNotFoundError).toBe(true); + expect(reconstructed instanceof ProtocolError).toBe(true); + }); + + it('isInstance guards agree with instanceof, per class, including across module copies', async () => { + const http = new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, 'boom', { status: 404 }); + expect(SdkHttpError.isInstance(http)).toBe(true); + expect(SdkError.isInstance(http)).toBe(true); + // inherited guard is scoped to the caller: an SdkError is NOT an SdkHttpError + expect(SdkHttpError.isInstance(new SdkError(SdkErrorCode.ConnectionClosed, 'x'))).toBe(false); + + const foreign = await loadForeignCopies(); + const foreignHttp = new foreign.sdkErrors.SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, 'boom', { status: 500 }); + expect(SdkHttpError.isInstance(foreignHttp)).toBe(true); + expect(SdkError.isInstance(foreignHttp)).toBe(true); + const foreignRnf = new foreign.protocolErrors.ResourceNotFoundError('file:///missing'); + expect(ResourceNotFoundError.isInstance(foreignRnf)).toBe(true); + expect(ProtocolError.isInstance(foreignRnf)).toBe(true); + expect(SdkError.isInstance(foreignRnf)).toBe(false); + + // detached calls fail loud instead of silently matching nothing + // (the cast models the callback-position escape TS cannot flag, e.g. + // `errors.filter(SdkError.isInstance)`) + const detached = SdkError.isInstance as (value: unknown) => boolean; + expect(() => detached(new SdkError(SdkErrorCode.ConnectionClosed, 'x'))).toThrow(TypeError); + + // agreement with the operator on negatives + for (const cls of [ProtocolError, SdkError, SdkHttpError, OAuthError]) { + for (const v of [new Error('plain'), null, undefined, 42, 'x', {}]) { + expect(cls.isInstance(v)).toBe(v instanceof (cls as never as new () => unknown)); + } + } + }); + + it('user-defined subclasses keep plain prototype semantics (no cross-class false positives)', () => { + class MyError extends ProtocolError {} + const mine = new MyError(ProtocolErrorCode.InternalError, 'mine'); + expect(mine instanceof MyError).toBe(true); + expect(mine instanceof ProtocolError).toBe(true); + + // A base-class instance — same or foreign bundle — must never satisfy the subclass. + const base = new ProtocolError(ProtocolErrorCode.InternalError, 'base'); + expect(base instanceof MyError).toBe(false); + }); + + it('a foreign base instance does not satisfy a user-defined subclass of this copy', async () => { + const { protocolErrors } = await loadForeignCopies(); + class MyError extends ProtocolError {} + const foreignBase = new protocolErrors.ProtocolError(ProtocolErrorCode.InternalError, 'x'); + expect(foreignBase instanceof MyError).toBe(false); + }); + + it('does not leak the brand through enumeration or serialization', () => { + const err = new SdkHttpError(SdkErrorCode.ClientHttpForbidden, 'nope', { status: 403 }); + expect(Object.keys(err).some(k => k.includes('mcp'))).toBe(false); + const brandKey = Symbol.for('mcp.sdk.errorBrands'); + expect(Object.prototype.propertyIsEnumerable.call(err, brandKey)).toBe(false); + const json = JSON.stringify({ ...err }); + expect(json).not.toContain('mcp.Sdk'); + }); + + it('ignores a brand inherited via the prototype chain (prototype-pollution hardening)', () => { + const BRANDS = Symbol.for('mcp.sdk.errorBrands'); + // A brand reachable only through the prototype chain must not count — + // otherwise polluting a shared prototype would make arbitrary objects + // satisfy instanceof against every branded class. + const polluted = Object.create({ [BRANDS]: new Set(['mcp.SdkError', 'mcp.SdkHttpError']) }) as object; + expect(polluted instanceof SdkError).toBe(false); + expect(polluted instanceof SdkHttpError).toBe(false); + + // An own-property carrier (what stampErrorBrands produces) is honored. + const own = Object.create(Error.prototype) as object; + Object.defineProperty(own, BRANDS, { value: new Set(['mcp.SdkError']), enumerable: false }); + expect(own instanceof SdkError).toBe(true); + }); + + it('matches across versions by identity, not shape (contract lock)', () => { + // Brand strings are version-less by design: an instance constructed by a + // different SDK version cross-matches. instanceof implies identity, not + // field shape - consumers read fields defensively. + const BRANDS = Symbol.for('mcp.sdk.errorBrands'); + const olderAlpha = Object.create(Error.prototype) as { status?: number }; + Object.defineProperty(olderAlpha, BRANDS, { value: new Set(['mcp.SdkError', 'mcp.SdkHttpError']), enumerable: false }); + expect((olderAlpha as unknown) instanceof SdkHttpError).toBe(true); + expect(olderAlpha.status).toBeUndefined(); + }); + + it('does not throw on hostile proxies (falls back to prototype check)', () => { + const hostile = new Proxy( + {}, + { + getOwnPropertyDescriptor() { + throw new Error('trap'); + } + } + ); + expect((hostile as unknown) instanceof SdkError).toBe(false); + }); + + it('tolerates primitives, null, and brandless objects', () => { + for (const value of [null, undefined, 42, 'x', Symbol('s'), {}, new Error('plain')]) { + expect((value as unknown) instanceof SdkError).toBe(false); + expect((value as unknown) instanceof ProtocolError).toBe(false); + expect((value as unknown) instanceof OAuthError).toBe(false); + } + }); +}); diff --git a/packages/core-internal/test/shared/abortReasonPassthrough.test.ts b/packages/core-internal/test/shared/abortReasonPassthrough.test.ts new file mode 100644 index 0000000000..3277edef27 --- /dev/null +++ b/packages/core-internal/test/shared/abortReasonPassthrough.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, it, vi } from 'vitest'; + +import type { BaseContext } from '../../src/shared/protocol'; +import { Protocol } from '../../src/shared/protocol'; +import { SdkError, SdkErrorCode } from '../../src/errors/sdkErrors'; +import { InMemoryTransport } from '../../src/util/inMemory'; + +class TestProtocolImpl extends Protocol { + protected assertCapabilityForMethod(): void {} + protected assertNotificationCapability(): void {} + protected assertRequestHandlerCapability(): void {} + protected buildContext(ctx: BaseContext): BaseContext { + return ctx; + } +} + +/** + * Pins the abort-reason behavior the branding changeset announces: an + * `SdkError` used as an abort reason is rethrown as-is — including one + * constructed by a foreign bundled copy of the SDK, which only matches + * `reason instanceof SdkError` through the cross-bundle brand. + */ +describe('request() abort-reason passthrough', () => { + async function connectedProtocol(): Promise { + const protocol = new TestProtocolImpl(); + const [transport] = InMemoryTransport.createLinkedPair(); + await protocol.connect(transport); + return protocol; + } + + it('rethrows a same-bundle SdkError abort reason as the same object', async () => { + const protocol = await connectedProtocol(); + const reason = new SdkError(SdkErrorCode.ConnectionClosed, 'caller closed'); + const controller = new AbortController(); + controller.abort(reason); + + await expect(protocol.request({ method: 'ping' }, { signal: controller.signal })).rejects.toBe(reason); + }); + + it('rethrows a foreign-bundle SdkError abort reason as the same object (brand-matched)', async () => { + vi.resetModules(); + const foreign = await import('../../src/errors/sdkErrors'); + expect(foreign.SdkError).not.toBe(SdkError); + + const protocol = await connectedProtocol(); + const reason = new foreign.SdkError(foreign.SdkErrorCode.ConnectionClosed, 'foreign caller closed'); + const controller = new AbortController(); + controller.abort(reason); + + await expect(protocol.request({ method: 'ping' }, { signal: controller.signal })).rejects.toBe(reason); + }); + + it('wraps a non-SdkError abort reason in SdkError(RequestTimeout)', async () => { + const protocol = await connectedProtocol(); + const controller = new AbortController(); + controller.abort(new Error('plain')); + + const rejection = await protocol.request({ method: 'ping' }, { signal: controller.signal }).then( + () => { + throw new Error('request unexpectedly resolved'); + }, + (e: unknown) => e + ); + expect(rejection).toBeInstanceOf(SdkError); + expect((rejection as SdkError).code).toBe(SdkErrorCode.RequestTimeout); + }); +}); diff --git a/packages/core-internal/test/types/errorSurfacePins.test.ts b/packages/core-internal/test/types/errorSurfacePins.test.ts index 59a207791f..cc01cf4c57 100644 --- a/packages/core-internal/test/types/errorSurfacePins.test.ts +++ b/packages/core-internal/test/types/errorSurfacePins.test.ts @@ -2,8 +2,9 @@ * Behavior-surface pins: error codes, error classes, and version constants. * * Consumers match SDK errors by literal numeric code, `error.name`, and message - * text — not only by enum member or `instanceof` (which breaks across bundled - * package boundaries). These tests pin the literal values so that a renumber, + * text — not only by enum member or `instanceof` (brand-matched `instanceof` + * needs both bundled copies at a brand-aware release; the literal values are + * the version-agnostic contract). These tests pin the literal values so that a renumber, * rename, or membership change turns CI red instead of landing silently. A * failing pin here means the change is deliberate: update the pin in the same * change, together with a changeset and a migration-doc entry. @@ -90,11 +91,34 @@ describe('SdkErrorCode', () => { }); }); +describe('cross-bundle brand strings', () => { + test('pins every core error brand (renaming one severs cross-version matching — must be deliberate)', async () => { + const { OAuthError } = await import('../../src/auth/errors'); + const { SdkError, SdkHttpError } = await import('../../src/errors/sdkErrors'); + const { + MissingRequiredClientCapabilityError, + ProtocolError: PE, + ResourceNotFoundError, + UnsupportedProtocolVersionError, + UrlElicitationRequiredError + } = await import('../../src/types/errors'); + const brand = (cls: unknown): unknown => (cls as { mcpBrand?: string }).mcpBrand; + expect(brand(PE)).toBe('mcp.ProtocolError'); + expect(brand(ResourceNotFoundError)).toBe('mcp.ResourceNotFoundError'); + expect(brand(UrlElicitationRequiredError)).toBe('mcp.UrlElicitationRequiredError'); + expect(brand(UnsupportedProtocolVersionError)).toBe('mcp.UnsupportedProtocolVersionError'); + expect(brand(MissingRequiredClientCapabilityError)).toBe('mcp.MissingRequiredClientCapabilityError'); + expect(brand(SdkError)).toBe('mcp.SdkError'); + expect(brand(SdkHttpError)).toBe('mcp.SdkHttpError'); + expect(brand(OAuthError)).toBe('mcp.OAuthError'); + }); +}); + describe('ProtocolError', () => { test('sets error.name, carries code/data, and leaves the message verbatim', () => { - // Consumers classify errors via err.name (instanceof breaks when core is - // bundled into both the client and server dists), and read .code/.data as - // a duck shape. The constructor must not decorate the message. + // Consumers classify errors via err.name (`instanceof` brand-matches + // across bundles only when both copies are brand-aware), and read + // .code/.data as a duck shape. The constructor must not decorate the message. const error = new ProtocolError(ProtocolErrorCode.InvalidParams, 'oops', { extra: 1 }); expect(error.name).toBe('ProtocolError'); expect(error.code).toBe(-32602); diff --git a/packages/core-internal/test/types/missingClientCapabilityError.test.ts b/packages/core-internal/test/types/missingClientCapabilityError.test.ts index e9a97375ca..c9270cdd6e 100644 --- a/packages/core-internal/test/types/missingClientCapabilityError.test.ts +++ b/packages/core-internal/test/types/missingClientCapabilityError.test.ts @@ -2,8 +2,9 @@ * The `-32021` MissingRequiredClientCapability typed error. * * Recognition is data-parse based: a peer (or another bundled copy of the SDK) - * is recognized by the error code plus the `data.requiredCapabilities` shape, - * never by `instanceof` across bundles. + * is recognized by the error code plus the `data.requiredCapabilities` shape — + * the version-agnostic path that also covers plain wire shapes and pre-brand + * SDK copies (branded `instanceof` needs both copies brand-aware). */ import { describe, expect, test } from 'vitest'; diff --git a/packages/server/test/server/errorBrandConformance.test.ts b/packages/server/test/server/errorBrandConformance.test.ts new file mode 100644 index 0000000000..8f8445d437 --- /dev/null +++ b/packages/server/test/server/errorBrandConformance.test.ts @@ -0,0 +1,89 @@ +import { describe, expect, it } from 'vitest'; + +import * as barrel from '../../src/index'; + +/** + * Server-side twin of the client's errorBrandConformance test: every error + * class exported from this package's public surface must carry an own + * `mcpBrand` static and resolve `instanceof` through a branded + * `Symbol.hasInstance`, so a new exported error class cannot silently regress + * to prototype-identity `instanceof` across bundled copies. See + * `core-internal/src/errors/crossBundleBrand.ts` for the participation + * criterion. + */ + +/** Error classes exported on purpose without a brand. Justify every entry. */ +const UNBRANDED_ALLOWLIST: ReadonlySet = new Set([]); + +function exportedErrorClasses(mod: Record): Array<[string, Function]> { + return Object.entries(mod) + .filter((entry): entry is [string, Function] => { + const v = entry[1]; + return typeof v === 'function' && v !== Error && v.prototype instanceof Error; + }) + .sort(([a], [b]) => a.localeCompare(b)); +} + +describe('error brand conformance (server export surface)', () => { + const classes = exportedErrorClasses(barrel as Record); + + it('finds the export surface (guards against a broken walker)', () => { + expect(classes.length).toBeGreaterThanOrEqual(5); + expect(classes.map(([n]) => n)).toContain('ProtocolError'); + }); + + it('every exported error class is branded (or explicitly allowlisted)', () => { + const unbranded = classes + .filter(([name]) => !UNBRANDED_ALLOWLIST.has(name)) + .filter(([, cls]) => !Object.prototype.hasOwnProperty.call(cls, 'mcpBrand')) + .map(([name]) => name); + expect( + unbranded, + `unbranded exported error classes (add the static mcpBrand block, or allowlist with a justification): ${unbranded.join(', ')}` + ).toEqual([]); + }); + + it('every branded class resolves instanceof through a branded hasInstance', () => { + const missing = classes + .filter(([, cls]) => Object.prototype.hasOwnProperty.call(cls, 'mcpBrand')) + .filter(([, cls]) => (cls as never as Record)[Symbol.hasInstance] === Function.prototype[Symbol.hasInstance]) + .map(([name]) => name); + expect(missing, `branded classes whose hierarchy root does not install brandedHasInstance: ${missing.join(', ')}`).toEqual([]); + }); + + it('every branded class exposes an isInstance guard that agrees with instanceof', () => { + const branded = classes.filter(([, cls]) => Object.prototype.hasOwnProperty.call(cls, 'mcpBrand')); + const missing = branded.filter(([, cls]) => typeof (cls as { isInstance?: unknown }).isInstance !== 'function').map(([n]) => n); + expect(missing, `branded classes without a static isInstance guard: ${missing.join(', ')}`).toEqual([]); + for (const [, cls] of branded) { + const guard = (cls as unknown as { isInstance: (v: unknown) => boolean }).isInstance; + for (const v of [new Error('plain'), null, undefined, 0, '', {}]) { + expect(guard.call(cls, v)).toBe(v instanceof (cls as never as new () => unknown)); + } + } + }); + + // Core-internal brand strings are pinned in core-internal's + // errorSurfacePins.test.ts. A server-local branded error class must add its + // brand string here so a rename cannot land silently. + const CORE_PINNED = new Set([ + 'MissingRequiredClientCapabilityError', + 'OAuthError', + 'ProtocolError', + 'ResourceNotFoundError', + 'SdkError', + 'SdkHttpError', + 'UnsupportedProtocolVersionError', + 'UrlElicitationRequiredError' + ]); + const SERVER_LOCAL_BRANDS: Record = {}; + + it('pins every server-local brand string (add new entries here — renames must be deliberate)', () => { + const brands = Object.fromEntries( + classes + .filter(([name, cls]) => !CORE_PINNED.has(name) && Object.prototype.hasOwnProperty.call(cls, 'mcpBrand')) + .map(([name, cls]) => [name, (cls as never as { mcpBrand: string }).mcpBrand]) + ); + expect(brands).toEqual(SERVER_LOCAL_BRANDS); + }); +}); From 61866d7a5ff4475663ceb525c88447c497c1b92a Mon Sep 17 00:00:00 2001 From: Felix Weinberger <3823880+felixweinberger@users.noreply.github.com> Date: Mon, 6 Jul 2026 18:02:06 +0100 Subject: [PATCH 4/4] feat(server): runtime-neutral OAuth discovery serving for web-standard hosts (#2422) --- .changeset/web-standard-oauth-metadata.md | 17 ++ docs/migration/upgrade-to-v2.md | 4 +- docs/serving/authorization.md | 10 + .../guides/serving/authorization.examples.ts | 10 + .../express/src/auth/metadataRouter.ts | 99 +++------- packages/server/src/index.ts | 8 + .../src/server/middleware/bearerAuth.ts | 4 +- .../middleware/oauthMetadata.examples.ts | 23 +++ .../src/server/middleware/oauthMetadata.ts | 180 +++++++++++++++++ .../server/test/server/oauthMetadata.test.ts | 186 ++++++++++++++++++ 10 files changed, 465 insertions(+), 76 deletions(-) create mode 100644 .changeset/web-standard-oauth-metadata.md create mode 100644 packages/server/src/server/middleware/oauthMetadata.examples.ts create mode 100644 packages/server/src/server/middleware/oauthMetadata.ts create mode 100644 packages/server/test/server/oauthMetadata.test.ts diff --git a/.changeset/web-standard-oauth-metadata.md b/.changeset/web-standard-oauth-metadata.md new file mode 100644 index 0000000000..32b960fa44 --- /dev/null +++ b/.changeset/web-standard-oauth-metadata.md @@ -0,0 +1,17 @@ +--- +'@modelcontextprotocol/server': minor +'@modelcontextprotocol/express': patch +--- + +Add runtime-neutral OAuth discovery serving to `@modelcontextprotocol/server`: +`oauthMetadataResponse` serves the RFC 9728 Protected Resource Metadata and +RFC 8414 Authorization Server metadata documents from web-standard +`fetch(request)` hosts, built on the exported +`buildOAuthProtectedResourceMetadata`, with +`getOAuthProtectedResourceMetadataUrl` now defined here. The Express metadata +router adapts the same core and is unchanged in behavior; the insecure-issuer +escape hatch is an explicit `dangerouslyAllowInsecureIssuerUrl` option in the +neutral core instead of a module-scope environment read. The web-standard +matcher validates lazily so unmatched traffic always falls through, tolerates +a trailing slash, supports HEAD, and marks reflected CORS preflights with +`Vary`. diff --git a/docs/migration/upgrade-to-v2.md b/docs/migration/upgrade-to-v2.md index e8714d7487..c3ca2b062b 100644 --- a/docs/migration/upgrade-to-v2.md +++ b/docs/migration/upgrade-to-v2.md @@ -397,7 +397,9 @@ A few transports need a decision the codemod can't make: `mcpAuthMetadataRouter`, `getOAuthProtectedResourceMetadataUrl`, `OAuthTokenVerifier`) → `@modelcontextprotocol/express`; the runtime-neutral core (`requireBearerAuth` for web-standard `fetch` hosts, `verifyBearerToken`, `bearerAuthChallengeResponse`, - `OAuthTokenVerifier`) is also exported from `@modelcontextprotocol/server`. Authorization Server helpers (`mcpAuthRouter`, + `OAuthTokenVerifier`, and the discovery serving `oauthMetadataResponse` / + `buildOAuthProtectedResourceMetadata` / `getOAuthProtectedResourceMetadataUrl`) + is also exported from `@modelcontextprotocol/server`. Authorization Server helpers (`mcpAuthRouter`, `OAuthServerProvider`, `ProxyOAuthServerProvider`, `allowedMethods`, `authenticateClient`, `metadataHandler`, `createOAuthMetadata`, `authorizationHandler` / `tokenHandler` / `revocationHandler` / diff --git a/docs/serving/authorization.md b/docs/serving/authorization.md index 5dbaa60a3f..7a0389c77c 100644 --- a/docs/serving/authorization.md +++ b/docs/serving/authorization.md @@ -89,6 +89,16 @@ app.use(mcpAuthMetadataRouter({ oauthMetadata, resourceServerUrl: mcpServerUrl } The router mounts two well-known routes: `/.well-known/oauth-protected-resource/mcp` — the path-aware RFC 9728 location, the same string `getOAuthProtectedResourceMetadataUrl(mcpServerUrl)` put into the challenge — and `/.well-known/oauth-authorization-server`, a mirror of `oauthMetadata` for clients that probe your origin directly. An unauthenticated client follows `401` → `resource_metadata` → `authorization_servers` to find your AS, obtains a token, and retries. +On a web-standard host, `oauthMetadataResponse` from `@modelcontextprotocol/server` serves the same two documents from a `fetch(request)` handler — it returns the matched document `Response` (with permissive CORS and `405` handling) or `undefined` to fall through to your own routing: + +```ts source="../../examples/guides/serving/authorization.examples.ts#oauthMetadataResponse_webStandard" +import { oauthMetadataResponse } from '@modelcontextprotocol/server'; + +async function webStandardFetch(request: Request): Promise { + return oauthMetadataResponse(request, { oauthMetadata, resourceServerUrl: mcpServerUrl }) ?? serveMcp(request); +} +``` + ## Read the caller in your handlers `requireBearerAuth` attaches the verified `AuthInfo` to `req.auth`, `toNodeHandler` forwards it, and tool handlers inside `buildServer` read it as `ctx.http.authInfo` — the exact object your verifier returned. diff --git a/examples/guides/serving/authorization.examples.ts b/examples/guides/serving/authorization.examples.ts index 60dbc41605..bc6887d9e0 100644 --- a/examples/guides/serving/authorization.examples.ts +++ b/examples/guides/serving/authorization.examples.ts @@ -83,3 +83,13 @@ function buildServer(): McpServer { return server; } + +//#region oauthMetadataResponse_webStandard +import { oauthMetadataResponse } from '@modelcontextprotocol/server'; + +async function webStandardFetch(request: Request): Promise { + return oauthMetadataResponse(request, { oauthMetadata, resourceServerUrl: mcpServerUrl }) ?? serveMcp(request); +} +//#endregion oauthMetadataResponse_webStandard + +declare function serveMcp(request: Request): Promise; diff --git a/packages/middleware/express/src/auth/metadataRouter.ts b/packages/middleware/express/src/auth/metadataRouter.ts index 7913c88171..8bf8faf8a6 100644 --- a/packages/middleware/express/src/auth/metadataRouter.ts +++ b/packages/middleware/express/src/auth/metadataRouter.ts @@ -1,5 +1,9 @@ -import type { OAuthMetadata, OAuthProtectedResourceMetadata } from '@modelcontextprotocol/server'; -import { OAuthError, OAuthErrorCode } from '@modelcontextprotocol/server'; +import type { + AuthMetadataOptions as NeutralAuthMetadataOptions, + OAuthMetadata, + OAuthProtectedResourceMetadata +} from '@modelcontextprotocol/server'; +import { buildOAuthProtectedResourceMetadata, OAuthError, OAuthErrorCode } from '@modelcontextprotocol/server'; import cors from 'cors'; import type { RequestHandler, Router } from 'express'; import express from 'express'; @@ -12,19 +16,6 @@ if (allowInsecureIssuerUrl) { console.warn('MCP_DANGEROUSLY_ALLOW_INSECURE_ISSUER_URL is enabled - HTTP issuer URLs are allowed. Do not use in production.'); } -function checkIssuerUrl(issuer: URL): void { - // RFC 8414 technically does not permit a localhost HTTPS exemption, but it is necessary for local testing. - if (issuer.protocol !== 'https:' && issuer.hostname !== 'localhost' && issuer.hostname !== '127.0.0.1' && !allowInsecureIssuerUrl) { - throw new Error('Issuer URL must be HTTPS'); - } - if (issuer.hash) { - throw new Error(`Issuer URL must not have a fragment: ${issuer}`); - } - if (issuer.search) { - throw new Error(`Issuer URL must not have a query string: ${issuer}`); - } -} - /** * Express middleware that rejects HTTP methods not in the supplied allow-list * with a 405 Method Not Allowed and an OAuth-style error body. Used by @@ -60,39 +51,12 @@ export function metadataHandler(metadata: OAuthMetadata | OAuthProtectedResource } /** - * Options for {@link mcpAuthMetadataRouter}. + * Options for {@link mcpAuthMetadataRouter}: the runtime-neutral + * `AuthMetadataOptions` from `@modelcontextprotocol/server`. The + * insecure-issuer escape hatch can also be enabled here by the + * `MCP_DANGEROUSLY_ALLOW_INSECURE_ISSUER_URL` environment variable. */ -export interface AuthMetadataOptions { - /** - * Authorization Server metadata (RFC 8414) for the AS this MCP server - * relies on. Served at `/.well-known/oauth-authorization-server` so - * legacy clients that probe the resource origin still discover the AS. - */ - oauthMetadata: OAuthMetadata; - - /** - * The public URL of this MCP server, used as the `resource` value in the - * Protected Resource Metadata document. Any path component is reflected - * in the well-known route per RFC 9728. - */ - resourceServerUrl: URL; - - /** - * Optional documentation URL advertised as `resource_documentation`. - */ - serviceDocumentationUrl?: URL; - - /** - * Optional list of scopes this MCP server understands, advertised as - * `scopes_supported`. - */ - scopesSupported?: string[]; - - /** - * Optional human-readable name advertised as `resource_name`. - */ - resourceName?: string; -} +export type { AuthMetadataOptions } from '@modelcontextprotocol/server'; /** * Builds an Express router that serves the two OAuth discovery documents an @@ -101,7 +65,7 @@ export interface AuthMetadataOptions { * - `/.well-known/oauth-protected-resource[/]` — RFC 9728 Protected * Resource Metadata, derived from the supplied options. * - `/.well-known/oauth-authorization-server` — RFC 8414 Authorization - * Server Metadata, passed through verbatim from {@link AuthMetadataOptions.oauthMetadata}. + * Server Metadata, passed through verbatim from the supplied `oauthMetadata`. * * Mount this router at the application root: * @@ -113,19 +77,21 @@ export interface AuthMetadataOptions { * `getOAuthProtectedResourceMetadataUrl` as its `resourceMetadataUrl` * so unauthenticated clients can discover the AS from the 401 challenge. */ -export function mcpAuthMetadataRouter(options: AuthMetadataOptions): Router { - checkIssuerUrl(new URL(options.oauthMetadata.issuer)); +export function mcpAuthMetadataRouter(options: NeutralAuthMetadataOptions): Router { + if (options.dangerouslyAllowInsecureIssuerUrl && !allowInsecureIssuerUrl) { + // The env-var path warns at module load; enabling via the option is + // equally loud so an insecure issuer can never be allowed silently. + // eslint-disable-next-line no-console + console.warn('dangerouslyAllowInsecureIssuerUrl is enabled - HTTP issuer URLs are allowed. Do not use in production.'); + } + const protectedResourceMetadata = buildOAuthProtectedResourceMetadata({ + ...options, + // The env var and the option are both honored; either enables it. + dangerouslyAllowInsecureIssuerUrl: allowInsecureIssuerUrl || options.dangerouslyAllowInsecureIssuerUrl + }); const router = express.Router(); - const protectedResourceMetadata: OAuthProtectedResourceMetadata = { - resource: options.resourceServerUrl.href, - authorization_servers: [options.oauthMetadata.issuer], - scopes_supported: options.scopesSupported, - resource_name: options.resourceName, - resource_documentation: options.serviceDocumentationUrl?.href - }; - // Serve PRM at the path-aware URL per RFC 9728 §3.1. const rsPath = new URL(options.resourceServerUrl.href).pathname; router.use(`/.well-known/oauth-protected-resource${rsPath === '/' ? '' : rsPath}`, metadataHandler(protectedResourceMetadata)); @@ -136,18 +102,5 @@ export function mcpAuthMetadataRouter(options: AuthMetadataOptions): Router { return router; } -/** - * Builds the RFC 9728 Protected Resource Metadata URL for a given MCP server - * URL by inserting `/.well-known/oauth-protected-resource` ahead of the path. - * - * @example - * ```ts - * getOAuthProtectedResourceMetadataUrl(new URL('https://api.example.com/mcp')) - * // → 'https://api.example.com/.well-known/oauth-protected-resource/mcp' - * ``` - */ -export function getOAuthProtectedResourceMetadataUrl(serverUrl: URL): string { - const u = new URL(serverUrl.href); - const rsPath = u.pathname && u.pathname !== '/' ? u.pathname : ''; - return new URL(`/.well-known/oauth-protected-resource${rsPath}`, u).href; -} +// Re-exported from the runtime-neutral home in @modelcontextprotocol/server. +export { getOAuthProtectedResourceMetadataUrl } from '@modelcontextprotocol/server'; diff --git a/packages/server/src/index.ts b/packages/server/src/index.ts index 1c48319bc0..dd720eb954 100644 --- a/packages/server/src/index.ts +++ b/packages/server/src/index.ts @@ -39,6 +39,14 @@ export type { BearerAuthOptions, OAuthTokenVerifier } from './server/middleware/ export { bearerAuthChallengeResponse, requireBearerAuth, verifyBearerToken } from './server/middleware/bearerAuth'; export type { HostHeaderValidationResult } from './server/middleware/hostHeaderValidation'; export { hostHeaderValidationResponse, localhostAllowedHostnames, validateHostHeader } from './server/middleware/hostHeaderValidation'; +// OAuth discovery documents (RFC 9728 / RFC 8414) for web-standard hosts; the +// Express metadata router in @modelcontextprotocol/express adapts the same core. +export type { AuthMetadataOptions } from './server/middleware/oauthMetadata'; +export { + buildOAuthProtectedResourceMetadata, + getOAuthProtectedResourceMetadataUrl, + oauthMetadataResponse +} from './server/middleware/oauthMetadata'; export type { OriginValidationResult } from './server/middleware/originValidation'; export { localhostAllowedOrigins, originValidationResponse, validateOriginHeader } from './server/middleware/originValidation'; export type { PerRequestHTTPServerTransportOptions, PerRequestMessageExtra, PerRequestResponseMode } from './server/perRequestTransport'; diff --git a/packages/server/src/server/middleware/bearerAuth.ts b/packages/server/src/server/middleware/bearerAuth.ts index c91b0c0922..1169e21336 100644 --- a/packages/server/src/server/middleware/bearerAuth.ts +++ b/packages/server/src/server/middleware/bearerAuth.ts @@ -48,8 +48,8 @@ export interface BearerAuthOptions { * `WWW-Authenticate` header on 401/403 responses, per * {@link https://datatracker.ietf.org/doc/html/rfc9728 | RFC 9728}. * - * Typically built with `getOAuthProtectedResourceMetadataUrl` from - * `@modelcontextprotocol/express`. + * Typically built with `getOAuthProtectedResourceMetadataUrl`, exported + * from this package. */ resourceMetadataUrl?: string; } diff --git a/packages/server/src/server/middleware/oauthMetadata.examples.ts b/packages/server/src/server/middleware/oauthMetadata.examples.ts new file mode 100644 index 0000000000..9aff9f11c7 --- /dev/null +++ b/packages/server/src/server/middleware/oauthMetadata.examples.ts @@ -0,0 +1,23 @@ +/** + * Type-checked examples for `oauthMetadata.ts`. + * + * These examples are synced into JSDoc comments via the sync-snippets script. + * Each function's region markers define the code snippet that appears in the docs. + * + * @module + */ + +import type { AuthMetadataOptions } from './oauthMetadata'; +import { oauthMetadataResponse } from './oauthMetadata'; + +/** + * Example: serving the discovery documents from a fetch handler. + */ +function oauthMetadataResponse_fetchHandler(options: AuthMetadataOptions, serveMcp: (request: Request) => Promise) { + //#region oauthMetadataResponse_fetchHandler + async function fetchHandler(request: Request): Promise { + return oauthMetadataResponse(request, options) ?? serveMcp(request); + } + //#endregion oauthMetadataResponse_fetchHandler + return fetchHandler; +} diff --git a/packages/server/src/server/middleware/oauthMetadata.ts b/packages/server/src/server/middleware/oauthMetadata.ts new file mode 100644 index 0000000000..fa5ac5f455 --- /dev/null +++ b/packages/server/src/server/middleware/oauthMetadata.ts @@ -0,0 +1,180 @@ +import type { OAuthMetadata, OAuthProtectedResourceMetadata } from '@modelcontextprotocol/core-internal'; +import { OAuthError, OAuthErrorCode } from '@modelcontextprotocol/core-internal'; + +/** + * Options for {@link oauthMetadataResponse} and + * {@link buildOAuthProtectedResourceMetadata}. + */ +export interface AuthMetadataOptions { + /** + * Authorization Server metadata (RFC 8414) for the AS this MCP server + * relies on. Served at `/.well-known/oauth-authorization-server` so + * legacy clients that probe the resource origin still discover the AS. + */ + oauthMetadata: OAuthMetadata; + + /** + * The public URL of this MCP server, used as the `resource` value in the + * Protected Resource Metadata document. Any path component is reflected + * in the well-known route per RFC 9728. + */ + resourceServerUrl: URL; + + /** + * Optional documentation URL advertised as `resource_documentation`. + */ + serviceDocumentationUrl?: URL; + + /** + * Optional list of scopes this MCP server understands, advertised as + * `scopes_supported`. + */ + scopesSupported?: string[]; + + /** + * Optional human-readable name advertised as `resource_name`. + */ + resourceName?: string; + + /** + * Allow a non-HTTPS issuer URL. Local testing only — never enable in + * production. The Express adapter maps its + * `MCP_DANGEROUSLY_ALLOW_INSECURE_ISSUER_URL` environment variable here. + */ + dangerouslyAllowInsecureIssuerUrl?: boolean; +} + +function checkIssuerUrl(issuer: URL, allowInsecure: boolean | undefined): void { + // RFC 8414 technically does not permit a localhost HTTPS exemption, but it is necessary for local testing. + if (issuer.protocol !== 'https:' && issuer.hostname !== 'localhost' && issuer.hostname !== '127.0.0.1' && !allowInsecure) { + throw new Error('Issuer URL must be HTTPS'); + } + if (issuer.hash) { + throw new Error(`Issuer URL must not have a fragment: ${issuer}`); + } + if (issuer.search) { + throw new Error(`Issuer URL must not have a query string: ${issuer}`); + } +} + +/** + * Derive the RFC 9728 Protected Resource Metadata document from + * {@link AuthMetadataOptions}, validating the Authorization Server issuer URL + * (HTTPS required outside localhost) in the process. + * + * `oauthMetadataResponse` and the Express `mcpAuthMetadataRouter` both build + * on this; use it directly when serving the document through your own + * routing — or call it once at startup to fail fast on a misconfigured + * issuer before any request arrives. + */ +export function buildOAuthProtectedResourceMetadata(options: AuthMetadataOptions): OAuthProtectedResourceMetadata { + checkIssuerUrl(new URL(options.oauthMetadata.issuer), options.dangerouslyAllowInsecureIssuerUrl); + return { + resource: options.resourceServerUrl.href, + authorization_servers: [options.oauthMetadata.issuer], + scopes_supported: options.scopesSupported, + resource_name: options.resourceName, + resource_documentation: options.serviceDocumentationUrl?.href + }; +} + +/** + * Builds the RFC 9728 Protected Resource Metadata URL for a given MCP server + * URL by inserting `/.well-known/oauth-protected-resource` ahead of the path. + * + * @example + * ```ts + * getOAuthProtectedResourceMetadataUrl(new URL('https://api.example.com/mcp')) + * // → 'https://api.example.com/.well-known/oauth-protected-resource/mcp' + * ``` + */ +export function getOAuthProtectedResourceMetadataUrl(serverUrl: URL): string { + return new URL(protectedResourceMetadataPath(serverUrl), serverUrl).href; +} + +/** The RFC 9728 path-aware well-known path for a resource URL. */ +function protectedResourceMetadataPath(resourceServerUrl: URL): string { + // Normalized like the request path in `oauthMetadataResponse`: a resource + // URL with a trailing slash must not make its own PRM route unreachable. + const rsPath = stripTrailingSlash(resourceServerUrl.pathname); + return `/.well-known/oauth-protected-resource${rsPath === '/' ? '' : rsPath}`; +} + +function stripTrailingSlash(path: string): string { + return path.length > 1 && path.endsWith('/') ? path.slice(0, -1) : path; +} + +const ALLOWED_METHODS = 'GET, HEAD, OPTIONS'; + +function metadataDocumentResponse(request: Request, metadata: OAuthMetadata | OAuthProtectedResourceMetadata): Response { + // Metadata documents must be fetchable from web-based MCP clients on any + // origin, so every response carries permissive CORS headers. + if (request.method === 'OPTIONS') { + const requestedHeaders = request.headers.get('access-control-request-headers'); + return new Response(null, { + status: 204, + headers: { + 'Access-Control-Allow-Origin': '*', + 'Access-Control-Allow-Methods': ALLOWED_METHODS, + // The reflected allow-list makes the response vary by request: + // without this a shared cache would replay one preflight's + // allow-list against another's headers. + ...(requestedHeaders === null + ? {} + : { 'Access-Control-Allow-Headers': requestedHeaders, Vary: 'Access-Control-Request-Headers' }) + } + }); + } + if (request.method !== 'GET' && request.method !== 'HEAD') { + const error = new OAuthError(OAuthErrorCode.MethodNotAllowed, `The method ${request.method} is not allowed for this endpoint`); + return Response.json(error.toResponseObject(), { + status: 405, + headers: { Allow: ALLOWED_METHODS, 'Access-Control-Allow-Origin': '*' } + }); + } + const response = Response.json(metadata, { headers: { 'Access-Control-Allow-Origin': '*' } }); + // RFC 9110: HEAD is GET without the body, same headers. + return request.method === 'HEAD' ? new Response(null, { status: response.status, headers: response.headers }) : response; +} + +/** + * Serve the two OAuth discovery documents an MCP server acting as a Resource + * Server exposes, from a web-standard `fetch(request)` handler: + * + * - `/.well-known/oauth-protected-resource[/]` — RFC 9728 Protected + * Resource Metadata, derived from the supplied options (path-aware: the + * resource URL's path is reflected in the route). + * - `/.well-known/oauth-authorization-server` — RFC 8414 Authorization + * Server Metadata, passed through verbatim. + * + * Returns the matched document `Response` (JSON with permissive CORS, `405` + * with an `Allow` header for non-GET methods, `204` for CORS preflight), or + * `undefined` when the request path is neither well-known route — fall + * through to your own routing. The framework-free counterpart of + * `mcpAuthMetadataRouter` from `@modelcontextprotocol/express`; pair it with + * `requireBearerAuth` and `getOAuthProtectedResourceMetadataUrl` so + * unauthenticated clients can discover the AS from the `401` challenge. + * + * @example + * ```ts source="./oauthMetadata.examples.ts#oauthMetadataResponse_fetchHandler" + * async function fetchHandler(request: Request): Promise { + * return oauthMetadataResponse(request, options) ?? serveMcp(request); + * } + * ``` + */ +export function oauthMetadataResponse(request: Request, options: AuthMetadataOptions): Response | undefined { + // Match before building: unmatched traffic must fall through untouched, + // even when the options are misconfigured — a bad issuer surfaces on the + // discovery routes (or at startup via buildOAuthProtectedResourceMetadata), + // never on the host's own traffic. + // Tolerate a single trailing slash, as path-mounted routers do. + const requestPath = stripTrailingSlash(new URL(request.url).pathname); + if (requestPath === protectedResourceMetadataPath(options.resourceServerUrl)) { + return metadataDocumentResponse(request, buildOAuthProtectedResourceMetadata(options)); + } + if (requestPath === '/.well-known/oauth-authorization-server') { + buildOAuthProtectedResourceMetadata(options); // issuer validation + return metadataDocumentResponse(request, options.oauthMetadata); + } + return undefined; +} diff --git a/packages/server/test/server/oauthMetadata.test.ts b/packages/server/test/server/oauthMetadata.test.ts new file mode 100644 index 0000000000..fe3b3c4e20 --- /dev/null +++ b/packages/server/test/server/oauthMetadata.test.ts @@ -0,0 +1,186 @@ +import type { OAuthMetadata } from '@modelcontextprotocol/core-internal'; +import { describe, expect, it } from 'vitest'; + +import type { AuthMetadataOptions } from '../../src/server/middleware/oauthMetadata'; +import { + buildOAuthProtectedResourceMetadata, + getOAuthProtectedResourceMetadataUrl, + oauthMetadataResponse +} from '../../src/server/middleware/oauthMetadata'; + +const oauthMetadata: OAuthMetadata = { + issuer: 'https://auth.example.com/', + authorization_endpoint: 'https://auth.example.com/authorize', + token_endpoint: 'https://auth.example.com/token', + response_types_supported: ['code'] +}; + +const options: AuthMetadataOptions = { + oauthMetadata, + resourceServerUrl: new URL('https://api.example.com/mcp'), + scopesSupported: ['mcp'], + resourceName: 'Example Server', + serviceDocumentationUrl: new URL('https://docs.example.com/') +}; + +describe('buildOAuthProtectedResourceMetadata', () => { + it('derives the RFC 9728 document', () => { + const prm = buildOAuthProtectedResourceMetadata(options); + expect(prm).toEqual({ + resource: 'https://api.example.com/mcp', + authorization_servers: ['https://auth.example.com/'], + scopes_supported: ['mcp'], + resource_name: 'Example Server', + resource_documentation: 'https://docs.example.com/' + }); + }); + + it('rejects a non-HTTPS issuer', () => { + expect(() => + buildOAuthProtectedResourceMetadata({ ...options, oauthMetadata: { ...oauthMetadata, issuer: 'http://auth.example.com/' } }) + ).toThrow('Issuer URL must be HTTPS'); + }); + + it('exempts localhost and honors the insecure escape hatch', () => { + expect(() => + buildOAuthProtectedResourceMetadata({ ...options, oauthMetadata: { ...oauthMetadata, issuer: 'http://localhost:9000/' } }) + ).not.toThrow(); + expect(() => + buildOAuthProtectedResourceMetadata({ + ...options, + dangerouslyAllowInsecureIssuerUrl: true, + oauthMetadata: { ...oauthMetadata, issuer: 'http://auth.internal/' } + }) + ).not.toThrow(); + }); + + it('rejects issuer URLs with fragments or query strings', () => { + expect(() => + buildOAuthProtectedResourceMetadata({ + ...options, + oauthMetadata: { ...oauthMetadata, issuer: 'https://auth.example.com/#frag' } + }) + ).toThrow('fragment'); + expect(() => + buildOAuthProtectedResourceMetadata({ + ...options, + oauthMetadata: { ...oauthMetadata, issuer: 'https://auth.example.com/?x=1' } + }) + ).toThrow('query string'); + }); +}); + +describe('getOAuthProtectedResourceMetadataUrl', () => { + it('is path-aware', () => { + expect(getOAuthProtectedResourceMetadataUrl(new URL('https://api.example.com/mcp'))).toBe( + 'https://api.example.com/.well-known/oauth-protected-resource/mcp' + ); + }); + + it('omits the trailing path for a root resource URL', () => { + expect(getOAuthProtectedResourceMetadataUrl(new URL('https://api.example.com/'))).toBe( + 'https://api.example.com/.well-known/oauth-protected-resource' + ); + }); +}); + +describe('oauthMetadataResponse', () => { + it('serves the path-aware PRM document with permissive CORS', async () => { + const response = oauthMetadataResponse(new Request('https://api.example.com/.well-known/oauth-protected-resource/mcp'), options); + expect(response?.status).toBe(200); + expect(response?.headers.get('Access-Control-Allow-Origin')).toBe('*'); + expect(await response?.json()).toMatchObject({ + resource: 'https://api.example.com/mcp', + authorization_servers: ['https://auth.example.com/'] + }); + }); + + it('serves the PRM at the bare well-known path for a root resource URL', () => { + const rootOptions = { ...options, resourceServerUrl: new URL('https://api.example.com/') }; + const response = oauthMetadataResponse(new Request('https://api.example.com/.well-known/oauth-protected-resource'), rootOptions); + expect(response?.status).toBe(200); + }); + + it('mirrors the AS metadata document', async () => { + const response = oauthMetadataResponse(new Request('https://api.example.com/.well-known/oauth-authorization-server'), options); + expect(response?.status).toBe(200); + expect(await response?.json()).toMatchObject({ issuer: 'https://auth.example.com/' }); + }); + + it('answers 405 with an Allow header and OAuth error body for non-GET methods', async () => { + const response = oauthMetadataResponse( + new Request('https://api.example.com/.well-known/oauth-authorization-server', { method: 'POST' }), + options + ); + expect(response?.status).toBe(405); + expect(response?.headers.get('Allow')).toBe('GET, HEAD, OPTIONS'); + expect(await response?.json()).toMatchObject({ error: 'method_not_allowed' }); + }); + + it('answers CORS preflight with 204 and reflected request headers', () => { + const response = oauthMetadataResponse( + new Request('https://api.example.com/.well-known/oauth-authorization-server', { + method: 'OPTIONS', + headers: { 'Access-Control-Request-Headers': 'authorization, mcp-protocol-version' } + }), + options + ); + expect(response?.status).toBe(204); + expect(response?.headers.get('Access-Control-Allow-Origin')).toBe('*'); + expect(response?.headers.get('Access-Control-Allow-Methods')).toBe('GET, HEAD, OPTIONS'); + expect(response?.headers.get('Access-Control-Allow-Headers')).toBe('authorization, mcp-protocol-version'); + }); + + it('returns undefined for unmatched paths so the host falls through', () => { + expect(oauthMetadataResponse(new Request('https://api.example.com/mcp'), options)).toBeUndefined(); + expect(oauthMetadataResponse(new Request('https://api.example.com/.well-known/other'), options)).toBeUndefined(); + }); +}); + +describe('review-hardened contracts', () => { + const badIssuer = { ...options, oauthMetadata: { ...oauthMetadata, issuer: 'http://auth.internal/' } }; + + it('never throws for unmatched paths, even with a misconfigured issuer', () => { + expect(oauthMetadataResponse(new Request('https://api.example.com/mcp'), badIssuer)).toBeUndefined(); + }); + + it('surfaces the issuer misconfiguration on the discovery routes only', () => { + expect(() => + oauthMetadataResponse(new Request('https://api.example.com/.well-known/oauth-protected-resource/mcp'), badIssuer) + ).toThrow('Issuer URL must be HTTPS'); + }); + + it('serves the PRM when the resource URL itself has a trailing slash', () => { + const slashOptions = { ...options, resourceServerUrl: new URL('https://api.example.com/mcp/') }; + for (const path of ['/.well-known/oauth-protected-resource/mcp', '/.well-known/oauth-protected-resource/mcp/']) { + const response = oauthMetadataResponse(new Request(`https://api.example.com${path}`), slashOptions); + expect(response?.status).toBe(200); + } + }); + + it('tolerates a single trailing slash like path-mounted routers', () => { + const response = oauthMetadataResponse(new Request('https://api.example.com/.well-known/oauth-protected-resource/mcp/'), options); + expect(response?.status).toBe(200); + }); + + it('supports HEAD with the same headers and no body', async () => { + const response = oauthMetadataResponse( + new Request('https://api.example.com/.well-known/oauth-authorization-server', { method: 'HEAD' }), + options + ); + expect(response?.status).toBe(200); + expect(response?.headers.get('Access-Control-Allow-Origin')).toBe('*'); + expect(await response?.text()).toBe(''); + }); + + it('marks the reflected preflight allow-list as varying', () => { + const response = oauthMetadataResponse( + new Request('https://api.example.com/.well-known/oauth-protected-resource/mcp', { + method: 'OPTIONS', + headers: { 'Access-Control-Request-Headers': 'authorization' } + }), + options + ); + expect(response?.headers.get('Vary')).toBe('Access-Control-Request-Headers'); + }); +});