diff --git a/docs/cloudflare/oauth-architecture.md b/docs/cloudflare/oauth-architecture.md index a52da228f..92622e226 100644 --- a/docs/cloudflare/oauth-architecture.md +++ b/docs/cloudflare/oauth-architecture.md @@ -215,6 +215,14 @@ upstream option, and run The regression exercises the production provider with blocked metadata, token exchange and refresh, and checks that OAuth validation remains enforced. +The patch also validates CIMD client ID syntax before authorization looks up +metadata, using the provider's existing validator. Invalid identifiers produce +`AuthorizationError` and a local `400` without redirecting. Fetch failures and +invalid metadata for syntactically valid URLs still propagate as `CimdFetchError`. +Dynamic client registration and the token endpoint retain their existing behavior. +Run `src/server/oauth/client-id-validation.integration.test.ts` with the command +above when changing this patch. + ### 2. API Handler The `apiHandler` is a protected endpoint that requires valid OAuth tokens: diff --git a/packages/mcp-cloudflare/src/server/oauth/client-id-validation.integration.test.ts b/packages/mcp-cloudflare/src/server/oauth/client-id-validation.integration.test.ts new file mode 100644 index 000000000..557731257 --- /dev/null +++ b/packages/mcp-cloudflare/src/server/oauth/client-id-validation.integration.test.ts @@ -0,0 +1,68 @@ +import { createExecutionContext, env } from "cloudflare:test"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { PKCE_CODE_CHALLENGE } from "../../test-utils/pkce"; +import handler from "../index"; +import type { Env } from "../types"; + +const ORIGIN = "https://mcp.sentry.dev"; +const workerEnv = { + ...(env as Record), + CF_VERSION_METADATA: { id: "test-version-id" }, +} as Env; + +function createAuthorizationRequest(clientId: string) { + const url = new URL("/oauth/authorize", ORIGIN); + url.search = new URLSearchParams({ + response_type: "code", + client_id: clientId, + redirect_uri: "http://127.0.0.1:43127/callback", + scope: "org:read", + state: "test-state", + resource: `${ORIGIN}/mcp`, + code_challenge: PKCE_CODE_CHALLENGE, + code_challenge_method: "S256", + }).toString(); + return new Request(url); +} + +describe("authorization client ID validation", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it.each([ + ["whitespace", "https://example.com/client meta.json"], + ["invalid percent encoding", "https://example.com/client%2"], + ["encoded dot segments", "https://example.com/a/%2e%2e/client.json"], + ["userinfo", "https://user:password@example.com/client.json"], + ["fragment", "https://example.com/client.json#fragment"], + ])("rejects a CIMD client ID containing %s locally", async (_, clientId) => { + const fetchSpy = vi.spyOn(globalThis, "fetch"); + const response = await handler.fetch!( + createAuthorizationRequest(clientId), + workerEnv, + createExecutionContext(), + ); + + expect(response.status).toBe(400); + expect(response.headers.get("Location")).toBeNull(); + expect(await response.text()).toBe("Invalid client_id"); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it("preserves an upstream metadata failure for a valid client ID", async () => { + const fetchSpy = vi.spyOn(globalThis, "fetch"); + const response = await handler.fetch!( + createAuthorizationRequest( + "https://chatgpt.com/oauth/codex/client.json?other-client=1", + ), + workerEnv, + createExecutionContext(), + ); + + expect(response.status).toBe(500); + expect(response.headers.get("Location")).toBeNull(); + expect(await response.text()).toBe("Internal Server Error"); + expect(fetchSpy).toHaveBeenCalled(); + }); +}); diff --git a/patches/@cloudflare__workers-oauth-provider@0.10.3.patch b/patches/@cloudflare__workers-oauth-provider@0.10.3.patch index 7b5bf4843..98f282e0f 100644 --- a/patches/@cloudflare__workers-oauth-provider@0.10.3.patch +++ b/patches/@cloudflare__workers-oauth-provider@0.10.3.patch @@ -1,5 +1,5 @@ diff --git a/dist/oauth-provider.d.ts b/dist/oauth-provider.d.ts -index 01141b6b4e708a7f40b32667ed8dd9cf33592888..33c099916cedc3666a5330675f1b3dc88d60a5c1 100644 +index 01141b6b4e708a7f40b32667ed8dd9cf33592888..8778203e3cfdf4c996c3155c67babcfd1956527b 100644 --- a/dist/oauth-provider.d.ts +++ b/dist/oauth-provider.d.ts @@ -684,6 +684,12 @@ interface OAuthProviderOptions { @@ -16,7 +16,7 @@ index 01141b6b4e708a7f40b32667ed8dd9cf33592888..33c099916cedc3666a5330675f1b3dc8 * When true, requested-vs-granted resource validation compares origins only * (scheme + host + port) instead of exact URIs. This allows an origin-only diff --git a/dist/oauth-provider.js b/dist/oauth-provider.js -index c15348c226f1064f6c277f45f10740b9266219e9..43b4d2bc3fca7aa844722c57c895c68951b76d55 100644 +index c15348c226f1064f6c277f45f10740b9266219e9..9182e6421c801f03b0b93d543afab5d733e4b2e7 100644 --- a/dist/oauth-provider.js +++ b/dist/oauth-provider.js @@ -2806,6 +2806,11 @@ var OAuthProviderImpl = class { @@ -31,3 +31,18 @@ index c15348c226f1064f6c277f45f10740b9266219e9..43b4d2bc3fca7aa844722c57c895c689 return await fetchClientIdMetadataDocument(clientId, this.serverCapabilities); } catch (error) { console.warn(`CIMD fetch failed for ${clientId}:`, error instanceof Error ? error.message : error); +@@ -3491,6 +3496,14 @@ var OAuthHelpersImpl = class { + const resourceParams = url.searchParams.getAll("resource"); + const resourceParam = resourceParams.length > 0 ? resourceParams.length === 1 ? resourceParams[0] : resourceParams : void 0; + if (!clientId) throw new AuthorizationError("invalid_request", { description: "client_id is required" }); ++ if (this.provider.options.clientIdMetadataDocumentEnabled && this.provider.isClientMetadataUrl(clientId)) { ++ // Invalid identifiers are request errors, not metadata resolution failures. ++ try { ++ validateClientIdentifierUrl(clientId); ++ } catch { ++ throw new AuthorizationError("invalid_request", { description: "Invalid client_id" }); ++ } ++ } + const clientInfo = await this.lookupClient(clientId); + if (!clientInfo) throw new AuthorizationError("invalid_request", { description: "Invalid client_id" }); + try { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index fd9ce6293..ac860d64e 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -191,7 +191,7 @@ overrides: patchedDependencies: '@ai-sdk/provider-utils@2.2.8': 4649a74595006c4a262d453549e091db331ade06b4f4320957af1b2c659b8ee3 - '@cloudflare/workers-oauth-provider@0.10.3': 34910b67929c1888b1492384c514d6baeedc4a439f66e21375dee5af404834f8 + '@cloudflare/workers-oauth-provider@0.10.3': 5ae7b25fa331991a0de83fbf6c9fb42b6ffad3076ecbab027f33eabb13724573 '@sentry/core@10.63.0': e663994979ff877a26ab6d4dea5968fbaee4ccdfdeb85623983535a572678940 '@sentry/node-core@10.63.0': 0a5f08471bd72cb833a2b36220c96001d966e5c390d71aaf598fdef75b552640 '@stricli/core@1.2.8': cbf047581d38be04108d3921321c14a778272c24004da8110d4498d4f2a6b2e1 @@ -571,7 +571,7 @@ importers: version: 3.0.232(react@19.2.8)(zod@4.4.3) '@cloudflare/workers-oauth-provider': specifier: 'catalog:' - version: 0.10.3(patch_hash=34910b67929c1888b1492384c514d6baeedc4a439f66e21375dee5af404834f8) + version: 0.10.3(patch_hash=5ae7b25fa331991a0de83fbf6c9fb42b6ffad3076ecbab027f33eabb13724573) '@modelcontextprotocol/sdk': specifier: 1.32.0 version: 1.32.0(@cfworker/json-schema@4.1.1)(zod@4.4.3) @@ -10277,7 +10277,7 @@ snapshots: '@cloudflare/workerd-windows-64@1.20261019.1': optional: true - '@cloudflare/workers-oauth-provider@0.10.3(patch_hash=34910b67929c1888b1492384c514d6baeedc4a439f66e21375dee5af404834f8)': {} + '@cloudflare/workers-oauth-provider@0.10.3(patch_hash=5ae7b25fa331991a0de83fbf6c9fb42b6ffad3076ecbab027f33eabb13724573)': {} '@cloudflare/workers-types@4.20260405.1': {}