Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changeset/fix-validate-client-metadata-url.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'@modelcontextprotocol/client': patch
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The changeset declares patch for @modelcontextprotocol/client, but this PR adds a new publicly-exported symbol (validateClientMetadataUrl) to packages/client/src/index.ts. Per semver, adding new backwards-compatible functionality is a minor change. The changeset should declare minor instead of patch.

Extended reasoning...

What the bug is

The changeset file .changeset/fix-validate-client-metadata-url.md uses patch as the bump type for @modelcontextprotocol/client. However, the PR adds validateClientMetadataUrl as a new named export in packages/client/src/index.ts (line 41). Semver defines a minor bump as any backwards-compatible addition of new functionality, and a new public export is exactly that.

The specific code path

The new function validateClientMetadataUrl is defined in packages/client/src/client/auth.ts and explicitly added to the public surface in packages/client/src/index.ts:

export {
    ...
    UnauthorizedError,
    validateClientMetadataUrl   // <-- new public export
} from ./client/auth.js;

This is unambiguously a new API addition visible to all downstream consumers.

Why existing patterns contradict this choice

The repo has a clear, consistent convention for when to use minor vs patch in changesets. Examining .changeset/ history: expose-auth-server-discovery.md uses minor for adding discoverOAuthServerInfo, OAuthDiscoveryState, and OAuthServerInfo; token-provider-composable-auth.md uses minor for adding AuthProvider, adaptOAuthProvider, handleOAuthUnauthorized, etc. Conversely, patch changesets are reserved for bug fixes and behavioral tweaks (fix-oauth-5xx-discovery.md, oauth-error-http200.md, finish-sdkerror-capability.md). This PR breaks that pattern by using patch for an additive API change.

Proof by example

Step-by-step: (1) The PR adds export function validateClientMetadataUrl(...) in auth.ts. (2) It is listed in the named exports in index.ts. (3) The changeset declares this as patch. (4) Per semver §7, a MINOR version increment is required when "new, backwards compatible functionality is introduced to the public API". (5) A patch increment is only correct for "backwards compatible bug fixes" (semver §6). Since validateClientMetadataUrl is purely additive and introduces no behavioral change to existing code, it does not qualify as a bug fix — it is new functionality.

Impact and fix

The functional impact is nil — consumers using ^x.y.z ranges will receive the new export regardless of whether it ships as a patch or minor bump. However, this violates both the strict semver specification and the repo's own established convention, which can confuse consumers tracking API surface changes across release boundaries. The fix is a one-character change in the changeset file: replace patch with minor.

---

Add `validateClientMetadataUrl()` utility for early validation of `clientMetadataUrl`

Exports a `validateClientMetadataUrl()` function that `OAuthClientProvider` implementations
can call in their constructors to fail fast on invalid URL-based client IDs, instead of
discovering the error deep in the auth flow.
4 changes: 4 additions & 0 deletions examples/client/src/simpleOAuthClientProvider.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { OAuthClientInformationMixed, OAuthClientMetadata, OAuthClientProvider, OAuthTokens } from '@modelcontextprotocol/client';
import { validateClientMetadataUrl } from '@modelcontextprotocol/client';

/**
* In-memory OAuth client provider for demonstration purposes
Expand All @@ -15,6 +16,9 @@ export class InMemoryOAuthClientProvider implements OAuthClientProvider {
onRedirect?: (url: URL) => void,
public readonly clientMetadataUrl?: string
) {
// Validate clientMetadataUrl at construction time (fail-fast)
validateClientMetadataUrl(clientMetadataUrl);

this._onRedirect =
onRedirect ||
(url => {
Expand Down
22 changes: 22 additions & 0 deletions packages/client/src/client/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -804,6 +804,28 @@ async function authInternal(
return 'REDIRECT';
}

/**
* Validates that the given `clientMetadataUrl` is a valid HTTPS URL with a non-root pathname.
*
* 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, throws an {@linkcode OAuthError}
* with code {@linkcode OAuthErrorCode.InvalidClientMetadata}.
*
* {@linkcode OAuthClientProvider} implementations that accept a `clientMetadataUrl` should
* call this in their 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}`
);
}
}

/**
Comment thread
claude[bot] marked this conversation as resolved.
* SEP-991: URL-based Client IDs
* Validate that the `client_id` is a valid URL with `https` scheme
Expand Down
3 changes: 2 additions & 1 deletion packages/client/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ export {
selectClientAuthMethod,
selectResourceURL,
startAuthorization,
UnauthorizedError
UnauthorizedError,
validateClientMetadataUrl
} from './client/auth.js';
export type {
AssertionCallback,
Expand Down
77 changes: 76 additions & 1 deletion packages/client/test/client/auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ import {
refreshAuthorization,
registerClient,
selectClientAuthMethod,
startAuthorization
startAuthorization,
validateClientMetadataUrl
} from '../../src/client/auth.js';
import { createPrivateKeyJwtAuth } from '../../src/client/authExtensions.js';

Expand Down Expand Up @@ -3833,6 +3834,80 @@ 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', () => {
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('error message matches expected format', () => {
expect(() => validateClientMetadataUrl('http://example.com/path')).toThrow(OAuthError);
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('determineScope', () => {
const baseClientMetadata = {
redirect_uris: ['http://localhost:3000/callback'],
Expand Down
Loading