fix(auth): treat null optional fields in token responses as absent

[claude]

Requested by Den Delimarsky · Slack thread

Before: connecting to an MCP server whose authorization server returns "refresh_token": null (or "scope": null / "id_token": null) fails with a Zod validation error ("expected string, received null") inside exchangeAuthorization/refreshAuthorization, before any tokens are saved. Worse, "expires_in": null passed validation but silently coerced to 0 (Number(null) === 0), producing a token the client treats as already expired.

After: those responses are accepted, and the null-valued fields are treated exactly as if they had been omitted. expires_in: null parses as undefined, not 0.

How: the exported OAuthTokensSchema is left completely untouched (still a plain ZodObject; class, .shape, .extend, z.input, and the emitted .d.ts are identical to v1.x, verified on zod 3.25.x and 4.x). Null normalization instead happens at the SDK's own response-parsing layer: a new exported OAuthTokenResponseSchema in src/shared/auth.ts wraps OAuthTokensSchema in a z.preprocess that removes null-valued optional members before validation (matching the existing ElicitResult null-leniency idiom in src/types.ts). The optional keys are derived from the schema's shape rather than hardcoded, so future optional members are covered automatically. The SDK's parse sites — executeTokenRequest in src/client/auth.ts and both token exchanges in ProxyOAuthServerProvider — now use the new schema. access_token: null and a missing token_type are still rejected, and refreshAuthorization's refresh-token preservation is hardened to { ...tokens, refresh_token: tokens.refresh_token ?? refreshToken } so it holds regardless of how the key is serialized. A changeset (patch) is included.

Note

Revised after an adversarial review: the previous approach wrapped OAuthTokensSchema itself in an object-level z.preprocess, which changed the exported symbol's class (ZodObject → ZodEffects/ZodPipe), breaking downstream consumers that call .extend() or read .shape — unacceptable in a patch release on the stable 1.x line. It also hardcoded the optional-field list.

Tests (test/shared/auth.test.ts, test/client/auth.test.ts): per-field null-as-absent with strict key-absence assertions, all-optionals-null toStrictEqual, expires_in: null never becomes 0, string expires_in still coerces, null access_token / missing token_type still rejected, a regression test pinning that OAuthTokensSchema keeps rejecting nulls and remains usable with .shape/.extend, a drift guard walking every optional member of the shape, plus e2e exchangeAuthorization (all-null optionals) and refreshAuthorization (server returns refresh_token: null, original refresh token preserved).

Counterpart for the main (v2) line: #2462. RFC 6749 doesn't sanction null for absent members, but real-world servers emit it anyway — see #754 for the same null-emitting-server pattern (Ory Hydra) hitting the client registration response schema (that schema is intentionally left out of scope here).

Scope of the normalization: this change deliberately covers only RFC 6749 §5.1 token responses at the SDK's token parse sites. The OAuth error-response schema (OAuthErrorResponseSchema, used by parseErrorResponse in src/client/auth.ts) intentionally remains strict about null members — consistent with the #754 registration-schema carve-out above — and can be revisited if field evidence of null-emitting servers turns up for error responses.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KXnNnp3fxQYR5HF9BUVhYP

---

Generated by Claude Code

[changeset-bot]

🦋 Changeset detected

Latest commit: eb19889

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@modelcontextprotocol/sdk@2461

commit: eb19889

[claude]

I didn't find any bugs, but this changes OAuth token-response validation behavior and adds a new public export (OAuthTokenResponseSchema) on the stable 1.x line, so a maintainer should weigh the API surface and leniency policy.

What was reviewed:

• The z.preprocess normalization: null is only stripped from members whose field schema accepts undefined, so access_token: null and missing token_type still reject; expires_in: null is deleted before reaching z.coerce.number(), so it can no longer coerce to 0.
• Confirmed the exported OAuthTokensSchema symbol is untouched (still a plain ZodObject; .shape/.extend intact, pinned by a regression test).
• The refreshAuthorization spread reorder (tokens.refresh_token ?? refreshToken) is behavior-preserving given the new schema strips null keys.
• Ran both changed test files (164 tests) and npm run typecheck locally — all pass.

Extended reasoning...

Overview

This PR makes the SDK tolerant of authorization servers that serialize absent optional members of OAuth token responses as JSON null (nonconformant with RFC 6749 §5.1 but seen in the wild, e.g. Ory Hydra-style servers). It adds a new exported OAuthTokenResponseSchema in src/shared/auth.ts that wraps OAuthTokensSchema in a z.preprocess step deleting null-valued optional members before validation, and switches the SDK's three token-response parse sites (executeTokenRequest in src/client/auth.ts, and both token exchanges in ProxyOAuthServerProvider) to use it. It also hardens refreshAuthorization's refresh-token preservation and includes a patch changeset plus thorough tests.

Security risks

The change is in the OAuth client/proxy token-parsing path, which is inherently security-sensitive. The specific normalization is low-risk: it only converts null → absent for members that are already optional, derived from the schema shape rather than a hardcoded list, and required members still reject null (verified by tests). The most meaningful semantic subtlety — that stripping scope: null turns an undefined-semantics response into RFC 6749's "granted scope equals requested scope" assertion — is explicitly documented in the JSDoc and changeset, with the note that the SDK never uses tokens.scope for authorization decisions. I found no injection, bypass, or data-exposure vector. Still, loosening validation of security-relevant responses is a policy decision.

Level of scrutiny

High: this is production auth code in a widely-used SDK, changes what the client accepts from authorization servers, and adds a new public export to src/shared/auth.ts on the stable 1.x line (with a v2 counterpart in #2462). Per my guidelines, auth-touching changes and new public API surface should not be shadow-approved regardless of how clean the implementation is.

Other factors

Verification performed: I ran npx vitest run test/shared/auth.test.ts test/client/auth.test.ts (164 tests pass) and npm run typecheck (clean) on the PR branch. The test coverage is strong — per-field null-as-absent with key-absence assertions, a drift guard over every optional member of the shape, a regression test pinning that OAuthTokensSchema remains a usable ZodObject, and e2e exchange/refresh tests. I also checked the preprocess edge cases: non-object input passes through unchanged, the input object is shallow-copied rather than mutated, and expires_in's .optional() short-circuits before z.coerce.number() so the optionality detection (safeParse(undefined)) is correct for coerced fields. No human reviewer has looked at the PR yet; the design tradeoff (new exported schema vs. modifying the existing one) is well-reasoned in the description but deserves maintainer sign-off.

[claude]

I didn't find any bugs, but this changes OAuth token-response validation semantics in both the client auth flow and the proxy server provider on the stable 1.x line, so it deserves a maintainer's sign-off on the leniency decision itself.

What was reviewed:

• The z.preprocess normalization: only null-valued members whose field schema accepts undefined are stripped, so access_token: null and missing token_type still reject; non-object input passes through to fail normally.
• All three SDK token parse sites (executeTokenRequest, both ProxyOAuthServerProvider exchanges) now use the new schema — no strict parse site was left behind.
• The refreshAuthorization spread reorder: tokens.refresh_token ?? refreshToken correctly preserves the prior refresh token whether the key is absent or stripped-from-null.
• Ran test/shared/auth.test.ts, test/client/auth.test.ts, and test/server/auth/providers/proxyProvider.test.ts at the PR head — all 198 tests pass.

Extended reasoning...

Overview

The PR adds a new exported OAuthTokenResponseSchema in src/shared/auth.ts that wraps the unchanged OAuthTokensSchema in a z.preprocess step stripping null-valued optional members before validation, and switches the SDK's three token-response parse sites (executeTokenRequest in src/client/auth.ts, and exchangeAuthorizationCode/exchangeRefreshToken in ProxyOAuthServerProvider) to use it. It also hardens refreshAuthorization's refresh-token preservation from a spread-order trick to an explicit ?? fallback. Extensive tests and a patch changeset are included.

Security risks

This is auth code, and the change deliberately relaxes validation of authorization-server token responses. The relaxation is narrow and well-reasoned: only optional members (derived from the schema shape, not hardcoded) are normalized, required members (access_token, token_type) still reject null, and the fix removes a real footgun where expires_in: null coerced to 0 (an instantly-expired token). The changeset itself flags the one semantic subtlety — a stripped scope: null becomes indistinguishable from an omitted scope, which RFC 6749 §5.1 defines as "granted scope equals requested scope" — and correctly notes the SDK never uses tokens.scope for authorization decisions. I found no injection, bypass, or exposure vector, but whether the SDK should accept nonconformant server responses at all is a policy call.

Level of scrutiny

High: this is OAuth client/server code on the stable 1.x release line, and the approval guidelines exclude security-sensitive auth code from bot approval regardless of implementation quality. The implementation is clean and the API-compatibility concern (keeping OAuthTokensSchema a plain ZodObject so .shape/.extend keep working downstream) was explicitly handled and regression-tested, but the leniency-vs-strictness tradeoff and the new exported symbol on a patch release warrant maintainer judgment.

Other factors

Test coverage is thorough — per-field null-as-absent with strict key-absence assertions, a drift guard over all optional shape members, negative tests pinning that required fields still reject null, and e2e tests for both exchange and refresh flows. I ran the three touched test files at the PR head and all 198 tests pass. I also verified via grep that no OAuthTokensSchema.parse call site in src/ was left on the strict schema. The bug hunting system reported no findings, and there are no prior human reviews on the thread.