From 32c58be56a469b16d0422756444c52a74913a393 Mon Sep 17 00:00:00 2001 From: rechedev9 Date: Tue, 10 Mar 2026 00:20:32 +0100 Subject: [PATCH 1/2] fix: validate clientMetadataUrl at construction time for fail-fast behavior Move clientMetadataUrl validation to all SDK-controlled entry points (StreamableHTTP/SSE constructors, auth(), withOAuth factory) so invalid URLs are caught immediately rather than deep in the auth flow. Exports validateClientMetadataUrl() for consumers who implement OAuthClientProvider directly. Existing defense-in-depth validation in authInternal() is preserved. Closes #1159 --- packages/client/src/client/auth.ts | 26 +++++ packages/client/src/client/middleware.ts | 10 +- packages/client/src/client/sse.ts | 7 +- packages/client/src/client/streamableHttp.ts | 7 +- packages/client/test/client/auth.test.ts | 104 +++++++++++++++++- .../client/test/client/middleware.test.ts | 32 ++++++ packages/client/test/client/sse.test.ts | 61 ++++++++++ .../client/test/client/streamableHttp.test.ts | 71 ++++++++++++ 8 files changed, 312 insertions(+), 6 deletions(-) diff --git a/packages/client/src/client/auth.ts b/packages/client/src/client/auth.ts index 7f7f44019..e63ce2e98 100644 --- a/packages/client/src/client/auth.ts +++ b/packages/client/src/client/auth.ts @@ -404,6 +404,9 @@ export async function auth( fetchFn?: FetchLike; } ): Promise { + // Validate clientMetadataUrl before any network calls + validateClientMetadataUrl(provider.clientMetadataUrl); + try { return await authInternal(provider, options); } catch (error) { @@ -614,6 +617,29 @@ async function authInternal( return 'REDIRECT'; } +/** + * Validates that the given `clientMetadataUrl` is a valid HTTPS URL with a non-root pathname. + * + * This function is a no-op when `url` is `undefined` or empty (providers that do not use + * URL-based client IDs are unaffected). When the value is defined but invalid, it throws + * an {@linkcode OAuthError} with code {@linkcode OAuthErrorCode.InvalidClientMetadata}. + * + * SDK transports and the `auth()` function call this automatically. Consumers who implement + * {@linkcode OAuthClientProvider} directly can call this in their own constructors for + * early validation. + * + * @param url - The `clientMetadataUrl` value to validate (from `OAuthClientProvider.clientMetadataUrl`) + * @throws {OAuthError} When `url` is defined but is not a valid HTTPS URL with a non-root pathname + */ +export function validateClientMetadataUrl(url: string | undefined): void { + if (url && !isHttpsUrl(url)) { + throw new OAuthError( + OAuthErrorCode.InvalidClientMetadata, + `clientMetadataUrl must be a valid HTTPS URL with a non-root pathname, got: ${url}` + ); + } +} + /** * SEP-991: URL-based Client IDs * Validate that the `client_id` is a valid URL with `https` scheme diff --git a/packages/client/src/client/middleware.ts b/packages/client/src/client/middleware.ts index 749414441..2b10ed906 100644 --- a/packages/client/src/client/middleware.ts +++ b/packages/client/src/client/middleware.ts @@ -1,7 +1,7 @@ import type { FetchLike } from '@modelcontextprotocol/core'; import type { OAuthClientProvider } from './auth.js'; -import { auth, extractWWWAuthenticateParams, UnauthorizedError } from './auth.js'; +import { auth, extractWWWAuthenticateParams, UnauthorizedError, validateClientMetadataUrl } from './auth.js'; /** * Middleware function that wraps and enhances fetch functionality. @@ -36,8 +36,11 @@ export type Middleware = (next: FetchLike) => FetchLike; * @returns A fetch middleware function */ export const withOAuth = - (provider: OAuthClientProvider, baseUrl?: string | URL): Middleware => - next => { + (provider: OAuthClientProvider, baseUrl?: string | URL): Middleware => { + // Validate clientMetadataUrl at factory creation time (fail-fast) + validateClientMetadataUrl(provider.clientMetadataUrl); + + return next => { return async (input, init) => { const makeRequest = async (): Promise => { const headers = new Headers(init?.headers); @@ -95,6 +98,7 @@ export const withOAuth = return response; }; }; +}; /** * Logger function type for HTTP requests diff --git a/packages/client/src/client/sse.ts b/packages/client/src/client/sse.ts index 133aa0004..99302036a 100644 --- a/packages/client/src/client/sse.ts +++ b/packages/client/src/client/sse.ts @@ -4,7 +4,7 @@ import type { ErrorEvent, EventSourceInit } from 'eventsource'; import { EventSource } from 'eventsource'; import type { AuthResult, OAuthClientProvider } from './auth.js'; -import { auth, extractWWWAuthenticateParams, UnauthorizedError } from './auth.js'; +import { auth, extractWWWAuthenticateParams, UnauthorizedError, validateClientMetadataUrl } from './auth.js'; export class SseError extends Error { constructor( @@ -81,6 +81,11 @@ export class SSEClientTransport implements Transport { onmessage?: (message: JSONRPCMessage) => void; constructor(url: URL, opts?: SSEClientTransportOptions) { + // Validate clientMetadataUrl at construction time (fail-fast) + if (opts?.authProvider) { + validateClientMetadataUrl(opts.authProvider.clientMetadataUrl); + } + this._url = url; this._resourceMetadataUrl = undefined; this._scope = undefined; diff --git a/packages/client/src/client/streamableHttp.ts b/packages/client/src/client/streamableHttp.ts index dab9b37ab..7973d78f0 100644 --- a/packages/client/src/client/streamableHttp.ts +++ b/packages/client/src/client/streamableHttp.ts @@ -14,7 +14,7 @@ import { import { EventSourceParserStream } from 'eventsource-parser/stream'; import type { AuthResult, OAuthClientProvider } from './auth.js'; -import { auth, extractWWWAuthenticateParams, UnauthorizedError } from './auth.js'; +import { auth, extractWWWAuthenticateParams, UnauthorizedError, validateClientMetadataUrl } from './auth.js'; // Default reconnection options for StreamableHTTP connections const DEFAULT_STREAMABLE_HTTP_RECONNECTION_OPTIONS: StreamableHTTPReconnectionOptions = { @@ -147,6 +147,11 @@ export class StreamableHTTPClientTransport implements Transport { onmessage?: (message: JSONRPCMessage) => void; constructor(url: URL, opts?: StreamableHTTPClientTransportOptions) { + // Validate clientMetadataUrl at construction time (fail-fast) + if (opts?.authProvider) { + validateClientMetadataUrl(opts.authProvider.clientMetadataUrl); + } + this._url = url; this._resourceMetadataUrl = undefined; this._scope = undefined; diff --git a/packages/client/test/client/auth.test.ts b/packages/client/test/client/auth.test.ts index 9d8f5cf6b..34019babd 100644 --- a/packages/client/test/client/auth.test.ts +++ b/packages/client/test/client/auth.test.ts @@ -17,7 +17,8 @@ import { refreshAuthorization, registerClient, selectClientAuthMethod, - startAuthorization + startAuthorization, + validateClientMetadataUrl } from '../../src/client/auth.js'; import { createPrivateKeyJwtAuth } from '../../src/client/authExtensions.js'; @@ -1993,6 +1994,28 @@ describe('OAuth Authorization', () => { vi.clearAllMocks(); }); + it('throws OAuthError with InvalidClientMetadata before any network request when clientMetadataUrl is invalid', async () => { + const providerWithInvalidUrl: OAuthClientProvider = { + ...mockProvider, + clientMetadataUrl: 'http://invalid.example.com/metadata' + }; + + await expect( + auth(providerWithInvalidUrl, { + serverUrl: 'https://server.example.com' + }) + ).rejects.toThrow(OAuthError); + + await expect( + auth(providerWithInvalidUrl, { + serverUrl: 'https://server.example.com' + }) + ).rejects.toMatchObject({ code: OAuthErrorCode.InvalidClientMetadata }); + + // Verify no fetch was called + expect(mockFetch).not.toHaveBeenCalled(); + }); + it('performs client_credentials with private_key_jwt when provider has addClientAuthentication', async () => { // Arrange: metadata discovery for PRM and AS mockFetch.mockImplementation(url => { @@ -3406,6 +3429,85 @@ describe('OAuth Authorization', () => { }); }); + describe('validateClientMetadataUrl', () => { + it('passes for valid HTTPS URL with path', () => { + expect(() => validateClientMetadataUrl('https://client.example.com/.well-known/oauth-client')).not.toThrow(); + }); + + it('passes for valid HTTPS URL with multi-segment path', () => { + expect(() => validateClientMetadataUrl('https://example.com/clients/metadata.json')).not.toThrow(); + }); + + it('throws OAuthError for HTTP URL', () => { + expect(() => validateClientMetadataUrl('http://client.example.com/.well-known/oauth-client')).toThrow(OAuthError); + try { + validateClientMetadataUrl('http://client.example.com/.well-known/oauth-client'); + } catch (error) { + expect(error).toBeInstanceOf(OAuthError); + expect((error as OAuthError).code).toBe(OAuthErrorCode.InvalidClientMetadata); + expect((error as OAuthError).message).toContain('http://client.example.com/.well-known/oauth-client'); + } + }); + + it('throws OAuthError for non-URL string', () => { + expect(() => validateClientMetadataUrl('not-a-url')).toThrow(OAuthError); + try { + validateClientMetadataUrl('not-a-url'); + } catch (error) { + expect(error).toBeInstanceOf(OAuthError); + expect((error as OAuthError).code).toBe(OAuthErrorCode.InvalidClientMetadata); + expect((error as OAuthError).message).toContain('not-a-url'); + } + }); + + it('passes silently for empty string', () => { + // Empty string is falsy, so it passes through without throwing + expect(() => validateClientMetadataUrl('')).not.toThrow(); + }); + + it('throws OAuthError for root-path HTTPS URL with trailing slash', () => { + expect(() => validateClientMetadataUrl('https://client.example.com/')).toThrow(OAuthError); + try { + validateClientMetadataUrl('https://client.example.com/'); + } catch (error) { + expect(error).toBeInstanceOf(OAuthError); + expect((error as OAuthError).code).toBe(OAuthErrorCode.InvalidClientMetadata); + expect((error as OAuthError).message).toContain('https://client.example.com/'); + } + }); + + it('throws OAuthError for root-path HTTPS URL without trailing slash', () => { + expect(() => validateClientMetadataUrl('https://client.example.com')).toThrow(OAuthError); + try { + validateClientMetadataUrl('https://client.example.com'); + } catch (error) { + expect(error).toBeInstanceOf(OAuthError); + expect((error as OAuthError).code).toBe(OAuthErrorCode.InvalidClientMetadata); + expect((error as OAuthError).message).toContain('https://client.example.com'); + } + }); + + it('passes silently for undefined', () => { + expect(() => validateClientMetadataUrl(undefined)).not.toThrow(); + }); + + it('passes silently when called with no arguments', () => { + // eslint-disable-next-line unicorn/no-useless-undefined + expect(() => validateClientMetadataUrl(undefined)).not.toThrow(); + }); + + it('error message matches expected format', () => { + try { + validateClientMetadataUrl('http://example.com/path'); + } catch (error) { + expect(error).toBeInstanceOf(OAuthError); + expect((error as OAuthError).message).toBe( + 'clientMetadataUrl must be a valid HTTPS URL with a non-root pathname, got: http://example.com/path' + ); + } + }); + }); + describe('SEP-991: URL-based Client ID fallback logic', () => { const validClientMetadata = { redirect_uris: ['http://localhost:3000/callback'], diff --git a/packages/client/test/client/middleware.test.ts b/packages/client/test/client/middleware.test.ts index 64bbfa673..5c6b4aac9 100644 --- a/packages/client/test/client/middleware.test.ts +++ b/packages/client/test/client/middleware.test.ts @@ -1,4 +1,5 @@ import type { FetchLike } from '@modelcontextprotocol/core'; +import { OAuthError, OAuthErrorCode } from '@modelcontextprotocol/core'; import type { Mocked, MockedFunction, MockInstance } from 'vitest'; import type { OAuthClientProvider } from '../../src/client/auth.js'; @@ -44,6 +45,37 @@ describe('withOAuth', () => { mockFetch = vi.fn(); }); + describe('clientMetadataUrl validation', () => { + it('throws OAuthError at factory call when provider.clientMetadataUrl is an invalid HTTP URL', () => { + const providerWithInvalidUrl: Mocked = { + ...mockProvider, + clientMetadataUrl: 'http://example.com/metadata' + }; + + expect(() => withOAuth(providerWithInvalidUrl, 'https://server.example.com')).toThrow(OAuthError); + try { + withOAuth(providerWithInvalidUrl, 'https://server.example.com'); + } catch (error) { + expect(error).toBeInstanceOf(OAuthError); + expect((error as OAuthError).code).toBe(OAuthErrorCode.InvalidClientMetadata); + expect((error as OAuthError).message).toContain('http://example.com/metadata'); + } + }); + + it('succeeds when clientMetadataUrl is a valid HTTPS URL', () => { + const providerWithValidUrl: Mocked = { + ...mockProvider, + clientMetadataUrl: 'https://client.example.com/.well-known/oauth-client' + }; + + expect(() => withOAuth(providerWithValidUrl, 'https://server.example.com')).not.toThrow(); + }); + + it('succeeds when clientMetadataUrl is undefined', () => { + expect(() => withOAuth(mockProvider, 'https://server.example.com')).not.toThrow(); + }); + }); + it('should add Authorization header when tokens are available (with explicit baseUrl)', async () => { mockProvider.tokens.mockResolvedValue({ access_token: 'test-token', diff --git a/packages/client/test/client/sse.test.ts b/packages/client/test/client/sse.test.ts index 0b0aff67b..8a25c1d21 100644 --- a/packages/client/test/client/sse.test.ts +++ b/packages/client/test/client/sse.test.ts @@ -11,6 +11,67 @@ import type { OAuthClientProvider } from '../../src/client/auth.js'; import { UnauthorizedError } from '../../src/client/auth.js'; import { SSEClientTransport } from '../../src/client/sse.js'; +describe('SSEClientTransport clientMetadataUrl validation', () => { + it('throws OAuthError when authProvider.clientMetadataUrl is an invalid HTTP URL', () => { + const provider: OAuthClientProvider = { + get redirectUrl() { + return 'http://localhost/callback'; + }, + get clientMetadata() { + return { redirect_uris: ['http://localhost/callback'] }; + }, + clientMetadataUrl: 'http://example.com/metadata', + clientInformation: vi.fn(), + tokens: vi.fn(), + saveTokens: vi.fn(), + redirectToAuthorization: vi.fn(), + saveCodeVerifier: vi.fn(), + codeVerifier: vi.fn() + }; + + expect(() => new SSEClientTransport(new URL('https://server.example.com/sse'), { authProvider: provider })).toThrow(OAuthError); + }); + + it('succeeds when clientMetadataUrl is a valid HTTPS URL', () => { + const provider: OAuthClientProvider = { + get redirectUrl() { + return 'http://localhost/callback'; + }, + get clientMetadata() { + return { redirect_uris: ['http://localhost/callback'] }; + }, + clientMetadataUrl: 'https://client.example.com/.well-known/oauth-client', + clientInformation: vi.fn(), + tokens: vi.fn(), + saveTokens: vi.fn(), + redirectToAuthorization: vi.fn(), + saveCodeVerifier: vi.fn(), + codeVerifier: vi.fn() + }; + + expect(() => new SSEClientTransport(new URL('https://server.example.com/sse'), { authProvider: provider })).not.toThrow(); + }); + + it('succeeds when clientMetadataUrl is undefined', () => { + const provider: OAuthClientProvider = { + get redirectUrl() { + return 'http://localhost/callback'; + }, + get clientMetadata() { + return { redirect_uris: ['http://localhost/callback'] }; + }, + clientInformation: vi.fn(), + tokens: vi.fn(), + saveTokens: vi.fn(), + redirectToAuthorization: vi.fn(), + saveCodeVerifier: vi.fn(), + codeVerifier: vi.fn() + }; + + expect(() => new SSEClientTransport(new URL('https://server.example.com/sse'), { authProvider: provider })).not.toThrow(); + }); +}); + /** * Parses HTTP Basic auth from a request's Authorization header. * Returns the decoded client_id and client_secret, or undefined if the header is absent or malformed. diff --git a/packages/client/test/client/streamableHttp.test.ts b/packages/client/test/client/streamableHttp.test.ts index 0398964d3..877024fe0 100644 --- a/packages/client/test/client/streamableHttp.test.ts +++ b/packages/client/test/client/streamableHttp.test.ts @@ -7,6 +7,77 @@ import { UnauthorizedError } from '../../src/client/auth.js'; import type { StartSSEOptions, StreamableHTTPReconnectionOptions } from '../../src/client/streamableHttp.js'; import { StreamableHTTPClientTransport } from '../../src/client/streamableHttp.js'; +describe('StreamableHTTPClientTransport clientMetadataUrl validation', () => { + it('throws OAuthError when authProvider.clientMetadataUrl is an invalid HTTP URL', () => { + const provider: OAuthClientProvider = { + get redirectUrl() { + return 'http://localhost/callback'; + }, + get clientMetadata() { + return { redirect_uris: ['http://localhost/callback'] }; + }, + clientMetadataUrl: 'http://example.com/metadata', + clientInformation: vi.fn(), + tokens: vi.fn(), + saveTokens: vi.fn(), + redirectToAuthorization: vi.fn(), + saveCodeVerifier: vi.fn(), + codeVerifier: vi.fn() + }; + + expect(() => new StreamableHTTPClientTransport(new URL('https://server.example.com/mcp'), { authProvider: provider })).toThrow( + OAuthError + ); + }); + + it('succeeds when clientMetadataUrl is a valid HTTPS URL', () => { + const provider: OAuthClientProvider = { + get redirectUrl() { + return 'http://localhost/callback'; + }, + get clientMetadata() { + return { redirect_uris: ['http://localhost/callback'] }; + }, + clientMetadataUrl: 'https://client.example.com/.well-known/oauth-client', + clientInformation: vi.fn(), + tokens: vi.fn(), + saveTokens: vi.fn(), + redirectToAuthorization: vi.fn(), + saveCodeVerifier: vi.fn(), + codeVerifier: vi.fn() + }; + + expect( + () => new StreamableHTTPClientTransport(new URL('https://server.example.com/mcp'), { authProvider: provider }) + ).not.toThrow(); + }); + + it('succeeds when clientMetadataUrl is undefined', () => { + const provider: OAuthClientProvider = { + get redirectUrl() { + return 'http://localhost/callback'; + }, + get clientMetadata() { + return { redirect_uris: ['http://localhost/callback'] }; + }, + clientInformation: vi.fn(), + tokens: vi.fn(), + saveTokens: vi.fn(), + redirectToAuthorization: vi.fn(), + saveCodeVerifier: vi.fn(), + codeVerifier: vi.fn() + }; + + expect( + () => new StreamableHTTPClientTransport(new URL('https://server.example.com/mcp'), { authProvider: provider }) + ).not.toThrow(); + }); + + it('succeeds when no authProvider is provided', () => { + expect(() => new StreamableHTTPClientTransport(new URL('https://server.example.com/mcp'))).not.toThrow(); + }); +}); + describe('StreamableHTTPClientTransport', () => { let transport: StreamableHTTPClientTransport; let mockAuthProvider: Mocked; From 186e04082caa74133d6e5c90900d673f431408d0 Mon Sep 17 00:00:00 2001 From: rechedev9 Date: Tue, 10 Mar 2026 00:24:02 +0100 Subject: [PATCH 2/2] style: fix prettier formatting in middleware.ts --- packages/client/src/client/middleware.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/client/src/client/middleware.ts b/packages/client/src/client/middleware.ts index 2b10ed906..b0087085d 100644 --- a/packages/client/src/client/middleware.ts +++ b/packages/client/src/client/middleware.ts @@ -35,8 +35,7 @@ export type Middleware = (next: FetchLike) => FetchLike; * @param baseUrl - Base URL for OAuth server discovery (defaults to request URL domain) * @returns A fetch middleware function */ -export const withOAuth = - (provider: OAuthClientProvider, baseUrl?: string | URL): Middleware => { +export const withOAuth = (provider: OAuthClientProvider, baseUrl?: string | URL): Middleware => { // Validate clientMetadataUrl at factory creation time (fail-fast) validateClientMetadataUrl(provider.clientMetadataUrl);