From 18bda28ce1bdcc108b0c9a852eb7739ee2b104d1 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Wed, 9 Sep 2026 01:21:15 -0400 Subject: [PATCH 1/2] feat(test-servers): a CIMD showcase fixture that serves its own client metadata document MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In CIMD (SEP-991) the `client_id` *is* a URL the authorization server dereferences to learn the client's metadata. Nothing in this repo served such a document, so a fixture could advertise `client_id_metadata_document_supported` and still be unusable — exercising CIMD meant standing up a second host by hand. That is why #2242 shipped verified by its tests alone, and why the v2.6.0 release smoke recorded it as the one contribution with an observable UI surface that could not be reached. The composable server now hosts the document itself when `oauth.clientMetadata` is set, gated on `supportCIMD` — advertising a client the server would then refuse to honour is a worse fixture than serving nothing. The document's `client_id` is derived from the **request** rather than from a configured issuer, so it stays correct when the harness picks the port, as the integration test does. `supportDCR: false` in the fixture is load-bearing rather than incidental. With DCR available a CIMD failure silently succeeds via dynamic registration and the reproduction proves nothing — during development a misconfigured run connected happily and reported `Dynamic (DCR)` with a `test_client_…` id, which reads as success until you check the client id. With DCR off, CIMD is the only path that can complete, so reaching a connected state is itself the assertion; the test pins the absent `registration_endpoint` so it stays that way. Verified end to end against the fixture: Connection Info read `Client registration — Client ID Metadata (CIMD)` with the client id equal to the metadata URL. That drive needed a self-signed HTTPS listener, because the Inspector requires the CIMD metadata URL to be HTTPS with no loopback exemption — filed separately as #2305, since it is a validation gap rather than a fixture one. Closes #2306 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01RsmR1iQstcrEzJFmgZXGLi Signed-off-by: cliffhall --- .../mcp/oauth-cimd-fixture.test.ts | 138 ++++++++++++++++++ docs/test-servers.md | 55 +++++++ test-servers/configs/oauth-cimd-http.json | 37 +++++ test-servers/src/composable-test-server.ts | 28 ++++ test-servers/src/load-config.ts | 12 ++ test-servers/src/test-server-oauth.ts | 32 ++++ 6 files changed, 302 insertions(+) create mode 100644 clients/web/src/test/integration/mcp/oauth-cimd-fixture.test.ts create mode 100644 test-servers/configs/oauth-cimd-http.json diff --git a/clients/web/src/test/integration/mcp/oauth-cimd-fixture.test.ts b/clients/web/src/test/integration/mcp/oauth-cimd-fixture.test.ts new file mode 100644 index 000000000..e5621830c --- /dev/null +++ b/clients/web/src/test/integration/mcp/oauth-cimd-fixture.test.ts @@ -0,0 +1,138 @@ +import { describe, it, expect, afterEach } from "vitest"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { + createTestServerHttp, + type TestServerHttp, + createTestServerInfo, + loadConfig, + resolveConfig, +} from "@modelcontextprotocol/inspector-test-server"; + +/** + * Live coverage of `test-servers/configs/oauth-cimd-http.json` — the fixture + * that makes #2242 reproducible by hand. + * + * #2242 (CIMD provenance surviving the SDK's issuer-binding write) shipped + * verified by its own end-to-end tests, because nothing in this repo served a + * **client metadata document**: in CIMD the `client_id` is a URL the + * authorization server dereferences, so exercising it meant standing up a + * second host. The v2.6.0 release ledger recorded that as the one row with an + * observable UI surface and no way to reach it. + * + * What this file protects is the fixture itself, not the fix. The manual + * reproduction depends on three things being true of the served document, and + * each of them is the kind of thing that breaks silently: + * + * - the AS advertises `client_id_metadata_document_supported`, or the + * Inspector's CIMD pre-registration bails out before storing anything; + * - it advertises **no** `registration_endpoint`, or a CIMD failure quietly + * falls back to DCR and the repro passes while proving nothing (this is + * exactly how the fixture first fooled its own author); + * - the document's `client_id` equals the URL it was fetched from, which is + * what makes it a legal CIMD client id rather than an arbitrary blob. + * + * The document is served over plain HTTP here. That is deliberate and is *not* + * a usable `clientMetadataUrl` for the Inspector, which requires HTTPS with no + * loopback exemption (#2305) — driving the UI needs a self-signed HTTPS + * listener, as `docs/test-servers.md` describes. This endpoint exists for the + * authorization-server side of the flow and for exactly these assertions. + */ +describe("CIMD showcase fixture (#2242)", () => { + let server: TestServerHttp | null = null; + + const configPath = path.resolve( + path.dirname(fileURLToPath(import.meta.url)), + "../../../../../../test-servers/configs/oauth-cimd-http.json", + ); + + afterEach(async () => { + if (server) { + try { + await server.stop(); + } catch { + // ignore + } + server = null; + } + }); + + /** + * Boot the showcase config on a harness-chosen port, so this cannot collide + * with a showcase server someone is running by hand. + */ + async function startShowcase(): Promise { + const resolved = resolveConfig(loadConfig(configPath)); + const started = createTestServerHttp({ + ...resolved, + serverInfo: createTestServerInfo("oauth-cimd-test", "1.0.0"), + port: undefined, + }); + await started.start(); + server = started; + return started; + } + + /** The MCP endpoint's origin, which is also the AS and the document host. */ + function originOf(started: TestServerHttp): string { + return new URL(started.url).origin; + } + + it("resolves the config with CIMD on and DCR deliberately off", () => { + const resolved = resolveConfig(loadConfig(configPath)); + expect(resolved.oauth?.supportCIMD).toBe(true); + // Not an oversight: with DCR available a CIMD failure silently succeeds + // via dynamic registration, and the repro stops proving anything. + expect(resolved.oauth?.supportDCR).toBe(false); + expect(resolved.oauth?.clientMetadata?.redirectUris).toContain( + "http://127.0.0.1:6276/oauth/callback", + ); + }); + + it("advertises CIMD support and no registration endpoint", async () => { + const started = await startShowcase(); + const res = await fetch( + `${originOf(started)}/.well-known/oauth-authorization-server`, + ); + expect(res.ok).toBe(true); + const metadata = await res.json(); + + expect(metadata.client_id_metadata_document_supported).toBe(true); + // The half that keeps the repro honest. + expect(metadata.registration_endpoint).toBeUndefined(); + }); + + it("serves a client metadata document whose client_id is its own URL", async () => { + const started = await startShowcase(); + const documentUrl = `${originOf(started)}/client-metadata.json`; + + const res = await fetch(documentUrl); + expect(res.ok).toBe(true); + const doc = await res.json(); + + // A CIMD client id IS the document's URL. Deriving it from the request + // rather than from a configured issuer is what keeps this true when the + // harness picks the port, as it does here. + expect(doc.client_id).toBe(documentUrl); + expect(doc.redirect_uris).toContain("http://127.0.0.1:6276/oauth/callback"); + // CIMD clients are public; the server's own CIMD branch issues no secret. + expect(doc.token_endpoint_auth_method).toBe("none"); + }); + + it("does not serve the document when CIMD is switched off", async () => { + const resolved = resolveConfig(loadConfig(configPath)); + const started = createTestServerHttp({ + ...resolved, + oauth: { ...resolved.oauth!, supportCIMD: false }, + serverInfo: createTestServerInfo("oauth-cimd-off-test", "1.0.0"), + port: undefined, + }); + await started.start(); + server = started; + + const res = await fetch(`${originOf(started)}/client-metadata.json`); + // Advertising a client this server would then refuse to honour is a worse + // fixture than serving nothing at all. + expect(res.status).toBe(404); + }); +}); diff --git a/docs/test-servers.md b/docs/test-servers.md index 4b5681620..2b2575905 100644 --- a/docs/test-servers.md +++ b/docs/test-servers.md @@ -55,6 +55,7 @@ as a missing capability rather than an error. | `oauth-revocation-http.json` / `oauth-no-revocation-http.json` **(legacy era)** | RFC 7009 token revocation on clear, with and without a `revocation_endpoint` | [#2144](https://github.com/modelcontextprotocol/inspector/issues/2144) | | `oauth-rfc8414-at-oidc-path-http.json` **(legacy era)** | Plain OAuth 2.0 AS metadata served at the OIDC well-known path | [#2172](https://github.com/modelcontextprotocol/inspector/issues/2172) | | `oauth-insecure-token-endpoint-http.json` **(legacy era)** | A token endpoint the SDK refuses to post credentials to | [#2280](https://github.com/modelcontextprotocol/inspector/issues/2280) | +| `oauth-cimd-http.json` **(legacy era)** | URL-based client IDs (CIMD / SEP-991), DCR deliberately off | [#2242](https://github.com/modelcontextprotocol/inspector/issues/2242) | | `logging-{legacy,modern}-http.json` **(era per file)** | Logging, both eras | [#1629](https://github.com/modelcontextprotocol/inspector/issues/1629) | | `subscriptions-{legacy,modern}-http.json` **(era per file)** | Resource subscriptions, both eras | [#1630](https://github.com/modelcontextprotocol/inspector/issues/1630) | | `subscriptions-never-acknowledged-http.json` **(modern era)** | A `subscriptions/listen` answered with a bare result | [#2097](https://github.com/modelcontextprotocol/inspector/issues/2097) | @@ -534,6 +535,60 @@ On the broken build you got a **"Re-authentication required"** banner with a **R Note that the fix here is presentational only. Making a `*.localhost` token endpoint actually **work** has to land in the SDK — the assertion runs inside `executeTokenRequest`, takes no options, and there is no hook the Inspector could reach. +## URL-based client IDs (CIMD / SEP-991) + +`oauth-cimd-http.json` is a combined AS + resource server that advertises +`client_id_metadata_document_supported: true` and — the part that makes it usable — **hosts the client +metadata document itself**, at `/client-metadata.json`. Plain streamable-HTTP; connect with the +**default (legacy)** protocol era. + +Hosting the document is the whole reason this fixture exists. In CIMD the `client_id` *is* a URL that +the authorization server dereferences to learn the client's metadata, so a server that merely +advertises support is only half a fixture: exercising it still meant standing up a second host by +hand. That is why [#2242](https://github.com/modelcontextprotocol/inspector/issues/2242) shipped +verified by its tests alone, and the v2.6.0 release ledger recorded it as the one row that had an +observable UI surface but no way to reach it. + +**`supportDCR` is `false` on purpose.** With both registration paths available a successful connection +proves nothing about which one ran — precisely the confusion #2242 was about, where Connection Info +reported `Dynamic (DCR)` for a connection that never issued a `POST /oauth/register`. With DCR off, +CIMD is the only way the flow can complete, so reaching a connected state *is* the assertion. + +⚠️ **CIMD is configured install-wide, not per server.** It lives in `client.json` +(`~/.mcp-inspector/storage/client.json`) as `cimd: { enabled: true, clientMetadataUrl }`, reachable +from **Client settings**, not from a server's own OAuth settings. A `clientMetadataUrl` written into a +catalog entry's `oauth` block is silently ignored — and with `supportDCR: true` the connection then +succeeds *via DCR*, which looks like CIMD working until you read the client id. + +⚠️ **The Inspector requires that URL to be HTTPS, and there is no loopback exemption** +(`getCimdClientMetadataUrlError` in `core/client/config-parse.ts`, applied to `client.json` on disk as +well as to the settings form). So this server's own `http://` document is **not** usable as a +`clientMetadataUrl`: it exists for the authorization-server side of the flow and for tests that drive +the AS directly. To drive the Inspector end to end you need the document served over HTTPS — +`https://127.0.0.1:8443/client-metadata.json` from a throwaway self-signed listener works, with +`NODE_TLS_REJECT_UNAUTHORIZED=0` in the *test server's* environment so its own fetch of that document +succeeds. That asymmetry is tracked in +[#2305](https://github.com/modelcontextprotocol/inspector/issues/2305); it is the same over-narrow +allow-list shape as the token-endpoint exemption above. + +With that in place: set the metadata URL in Client settings, connect, and open **Connection Info**. +It should read `Client registration — Client ID Metadata (CIMD)` with the **client id equal to the +metadata URL**, which is what CIMD means and what distinguishes it from a DCR-issued +`test_client_…`. On the broken build it read `Dynamic (DCR)` for exactly this flow +([#2242](https://github.com/modelcontextprotocol/inspector/issues/2242)). + +⚠️ **`clientMetadata.redirectUris` must list the callback for the port you are running.** The +Inspector's browser redirect is `/oauth/callback` and the CLI/TUI's is +`http://127.0.0.1:6276/oauth/callback`; the authorization server checks the incoming `redirect_uri` +against this list. The shipped fixture lists **6274** (the default), **6330** and **6276**; on any +other port the flow fails with `Invalid redirect_uri`, which reads like a CIMD problem and is not one. +Add your port to the config rather than debugging the registration path. + +⚠️ **The same fixed-`issuerUrl` hazard as the fixture above applies**, for the same reason: the +`client_id` this server publishes is derived from its issuer URL, so a server that walked to another +port on `EADDRINUSE` publishes a `client_id` pointing at whatever process holds 8092. Check with +`lsof -nP -iTCP:8092 -sTCP:LISTEN` before believing a failure. + ## Revoking tokens on clear (RFC 7009) `oauth-revocation-http.json` and `oauth-no-revocation-http.json` are the same OAuth-protected server (combined AS + resource, DCR, refresh tokens) differing in one thing: the first advertises a `revocation_endpoint`, the second advertises none. Plain streamable-HTTP — connect with the **default (legacy)** protocol era. diff --git a/test-servers/configs/oauth-cimd-http.json b/test-servers/configs/oauth-cimd-http.json new file mode 100644 index 000000000..01c6afccf --- /dev/null +++ b/test-servers/configs/oauth-cimd-http.json @@ -0,0 +1,37 @@ +{ + "serverInfo": { + "name": "oauth-cimd-showcase", + "version": "1.0.0" + }, + "tools": [ + { + "preset": "echo" + } + ], + "oauth": { + "enabled": true, + "mode": "combined", + "requireAuth": true, + "scopesSupported": [ + "mcp" + ], + "supportCIMD": true, + "supportDCR": false, + "supportRefreshTokens": true, + "clientMetadata": { + "clientName": "MCP Inspector (CIMD test fixture)", + "scope": "mcp", + "redirectUris": [ + "http://127.0.0.1:6274/oauth/callback", + "http://localhost:6274/oauth/callback", + "http://127.0.0.1:6330/oauth/callback", + "http://localhost:6330/oauth/callback", + "http://127.0.0.1:6276/oauth/callback" + ] + } + }, + "transport": { + "type": "streamable-http", + "port": 8092 + } +} diff --git a/test-servers/src/composable-test-server.ts b/test-servers/src/composable-test-server.ts index 10e895d91..2bdf78e94 100644 --- a/test-servers/src/composable-test-server.ts +++ b/test-servers/src/composable-test-server.ts @@ -734,6 +734,34 @@ export interface ServerConfig { */ supportCIMD?: boolean; + /** + * Serve a CIMD client metadata document from this server, so a CIMD + * fixture is self-contained. + * + * CIMD makes the `client_id` a URL that the authorization server fetches + * to learn the client's metadata (SEP-991). Nothing in this repo served + * such a document, so exercising CIMD meant standing up a second host by + * hand — which is why #2242 shipped verified only by its tests. With this + * set, the server hosts the document at `clientMetadataPath` (default + * `/client-metadata.json`) and that URL is a usable `client_id`. + * + * `redirectUris` MUST list the Inspector's callback for the port you run + * it on (`/oauth/callback`) — the authorization server checks + * the incoming `redirect_uri` against this list, and a mismatch fails the + * flow with `Invalid redirect_uri` rather than anything CIMD-specific. + * + * Only served when `supportCIMD` is true: a document advertising a client + * the server would then refuse is a worse fixture than none. + */ + clientMetadata?: { + redirectUris: string[]; + clientName?: string; + scope?: string; + }; + + /** Where to serve `clientMetadata` (default `/client-metadata.json`). */ + clientMetadataPath?: string; + /** * Token expiration time in seconds (default: 3600) */ diff --git a/test-servers/src/load-config.ts b/test-servers/src/load-config.ts index 3b32862f0..2b4f2e83c 100644 --- a/test-servers/src/load-config.ts +++ b/test-servers/src/load-config.ts @@ -37,6 +37,18 @@ export interface ConfigFileOAuth { }>; supportDCR?: boolean; supportCIMD?: boolean; + /** + * Serve a CIMD client metadata document, making a CIMD fixture + * self-contained. `redirectUris` must list the Inspector callback for the + * web port under test. See the field's doc comment in + * `composable-test-server.ts`. + */ + clientMetadata?: { + redirectUris: string[]; + clientName?: string; + scope?: string; + }; + clientMetadataPath?: string; tokenExpirationSeconds?: number; supportRefreshTokens?: boolean; /** RFC 7009 revocation endpoint; default true (#2144). */ diff --git a/test-servers/src/test-server-oauth.ts b/test-servers/src/test-server-oauth.ts index e73e0e7a1..1ca7fa2da 100644 --- a/test-servers/src/test-server-oauth.ts +++ b/test-servers/src/test-server-oauth.ts @@ -283,6 +283,38 @@ function setupMetadataEndpoints( }); } + // CIMD client metadata document (SEP-991). The `client_id` in a CIMD flow is + // a URL the authorization server dereferences, so a fixture that advertises + // `client_id_metadata_document_supported` without hosting a document + // anywhere is only half a fixture — it needs a second host to be usable at + // all. Serving it here makes a CIMD run self-contained. + // + // Gated on `supportCIMD` as well as on the document's presence: advertising + // a client this server would then refuse to honour is worse than serving + // nothing. + if (config.supportCIMD && config.clientMetadata) { + const doc = config.clientMetadata; + const metadataPath = config.clientMetadataPath ?? "/client-metadata.json"; + app.get(metadataPath, (req: Request, res: Response) => { + // Derived from the request rather than from `issuerUrl`, so the + // document's own `client_id` always equals the URL it was fetched from + // — which is what CIMD requires, and what stays true if the server + // walked to another port on EADDRINUSE. + const requestBaseUrl = `${req.protocol}://${req.get("host")}`; + res.json({ + client_id: new URL(metadataPath, requestBaseUrl).href, + client_name: doc.clientName ?? "MCP Inspector (CIMD test fixture)", + redirect_uris: doc.redirectUris, + // CIMD clients are public and authenticate with nothing; the server's + // own CIMD branch assumes exactly this (no client_secret is issued). + token_endpoint_auth_method: "none", + grant_types: ["authorization_code", "refresh_token"], + response_types: ["code"], + ...(doc.scope ? { scope: doc.scope } : {}), + }); + }); + } + // OAuth Protected Resource Metadata. `resourceMetadataPath` moves the // document off the well-known path entirely (rather than serving both), so // a client that ignores the advertised `resource_metadata` URL gets a 404 From a8cbc506ab16824c59dd9c5b51de4b53221b504a Mon Sep 17 00:00:00 2001 From: cliffhall Date: Thu, 10 Sep 2026 22:49:36 -0400 Subject: [PATCH 2/2] fix: address Copilot review round 1 on #2306 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Validate `oauth.clientMetadataPath` the way the other two metadata paths are validated — in `loadConfig` for the JSON route and again at server setup for a `ServerConfig` built in code. This path is not merely advertised: it becomes the served document's own `client_id`, so an off-origin or query-bearing value would publish a client id this server cannot honour. Derive that `client_id` from `req.originalUrl` rather than from the registered route, so a document fetched with a query string answers with a `client_id` byte-identical to the URL it was fetched from — the equality CIMD turns on. Correct the docs' fixed-port warning: this fixture configures no `issuerUrl` and derives the `client_id` from the request, which is exactly why the integration test can drive it on a harness-chosen port. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UWGH2ytPzSPxnp4cibaV3Z Signed-off-by: cliffhall --- .../mcp/oauth-cimd-fixture.test.ts | 56 +++++++++++++++++++ docs/test-servers.md | 9 +-- test-servers/src/composable-test-server.ts | 11 +++- test-servers/src/load-config.ts | 11 ++++ test-servers/src/test-server-oauth.ts | 31 +++++++++- 5 files changed, 111 insertions(+), 7 deletions(-) diff --git a/clients/web/src/test/integration/mcp/oauth-cimd-fixture.test.ts b/clients/web/src/test/integration/mcp/oauth-cimd-fixture.test.ts index e5621830c..759d1d2a3 100644 --- a/clients/web/src/test/integration/mcp/oauth-cimd-fixture.test.ts +++ b/clients/web/src/test/integration/mcp/oauth-cimd-fixture.test.ts @@ -1,4 +1,6 @@ import { describe, it, expect, afterEach } from "vitest"; +import fs from "node:fs"; +import os from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; import { @@ -119,6 +121,60 @@ describe("CIMD showcase fixture (#2242)", () => { expect(doc.token_endpoint_auth_method).toBe("none"); }); + it("preserves a query-bearing document URL in the client_id it publishes", async () => { + const started = await startShowcase(); + const documentUrl = `${originOf(started)}/client-metadata.json?profile=a`; + + const res = await fetch(documentUrl); + expect(res.ok).toBe(true); + const doc = await res.json(); + + // CIMD turns on the document's `client_id` being the URL it was fetched + // from. Answering `?profile=a` with the bare route would publish a + // document that fails that equality for a client id the server just + // served (Copilot). + expect(doc.client_id).toBe(documentUrl); + }); + + it("rejects a clientMetadataPath that would publish a foreign client_id", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "cimd-config-")); + const badPath = path.join(dir, "bad-cimd.json"); + const base = JSON.parse(fs.readFileSync(configPath, "utf8")); + fs.writeFileSync( + badPath, + JSON.stringify({ + ...base, + oauth: { ...base.oauth, clientMetadataPath: "//other-host/doc" }, + }), + ); + + try { + // The path is not merely advertised: it becomes the document's own + // `client_id`, so an off-origin value publishes a client id naming a + // host this server does not serve. + expect(() => loadConfig(badPath)).toThrow(/clientMetadataPath/); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it("rejects the same path built programmatically rather than from JSON", async () => { + const resolved = resolveConfig(loadConfig(configPath)); + const started = createTestServerHttp({ + ...resolved, + oauth: { ...resolved.oauth!, clientMetadataPath: "/doc?version=1" }, + serverInfo: createTestServerInfo("oauth-cimd-badpath-test", "1.0.0"), + port: undefined, + }); + server = started; + + // `loadConfig` covers the JSON route only, so the server-setup check is + // what catches a `ServerConfig` assembled in code — the same split the + // two existing metadata paths have. + await expect(started.start()).rejects.toThrow(/clientMetadataPath/); + server = null; + }); + it("does not serve the document when CIMD is switched off", async () => { const resolved = resolveConfig(loadConfig(configPath)); const started = createTestServerHttp({ diff --git a/docs/test-servers.md b/docs/test-servers.md index 2b2575905..55afa07a1 100644 --- a/docs/test-servers.md +++ b/docs/test-servers.md @@ -584,10 +584,11 @@ against this list. The shipped fixture lists **6274** (the default), **6330** an other port the flow fails with `Invalid redirect_uri`, which reads like a CIMD problem and is not one. Add your port to the config rather than debugging the registration path. -⚠️ **The same fixed-`issuerUrl` hazard as the fixture above applies**, for the same reason: the -`client_id` this server publishes is derived from its issuer URL, so a server that walked to another -port on `EADDRINUSE` publishes a `client_id` pointing at whatever process holds 8092. Check with -`lsof -nP -iTCP:8092 -sTCP:LISTEN` before believing a failure. +This fixture has **no** fixed-`issuerUrl` hazard, unlike several of the ones above: it configures no +`issuerUrl`, and the document's `client_id` is derived from the request it was fetched over — query +string included — so a server that walked to another port on `EADDRINUSE` still publishes a +`client_id` equal to the URL you fetched, and the integration test drives it on a harness-chosen port +for exactly that reason. The fixed-port dependency that *does* bite is `redirect_uris`, above. ## Revoking tokens on clear (RFC 7009) diff --git a/test-servers/src/composable-test-server.ts b/test-servers/src/composable-test-server.ts index 2bdf78e94..2c094fc9e 100644 --- a/test-servers/src/composable-test-server.ts +++ b/test-servers/src/composable-test-server.ts @@ -759,7 +759,16 @@ export interface ServerConfig { scope?: string; }; - /** Where to serve `clientMetadata` (default `/client-metadata.json`). */ + /** + * Where to serve `clientMetadata` (default `/client-metadata.json`). + * + * Must be origin-relative with no query or fragment, and is validated as + * such — both by `loadConfig` and again at server setup for a config built + * in code. Unlike the other metadata paths this one is not merely + * advertised: it becomes the document's own `client_id`, so an off-origin + * or query-bearing value would publish a client id this server cannot + * honour (Copilot). + */ clientMetadataPath?: string; /** diff --git a/test-servers/src/load-config.ts b/test-servers/src/load-config.ts index 2b4f2e83c..865fdaabb 100644 --- a/test-servers/src/load-config.ts +++ b/test-servers/src/load-config.ts @@ -48,6 +48,11 @@ export interface ConfigFileOAuth { clientName?: string; scope?: string; }; + /** + * Where to serve `clientMetadata` (default `/client-metadata.json`); + * validated as an origin-relative path, since it becomes the document's own + * `client_id`. See `composable-test-server.ts`. + */ clientMetadataPath?: string; tokenExpirationSeconds?: number; supportRefreshTokens?: boolean; @@ -260,6 +265,12 @@ function validateConfig( `Invalid config in ${filePath}: oauth.asMetadataPath must be an origin-relative path (e.g. "/.well-known/openid-configuration") — a value such as "//host/doc" would move the document off this server entirely`, ); } + const cimdPath = oauth.clientMetadataPath; + if (cimdPath !== undefined && !isOriginRelativePath(cimdPath)) { + throw new Error( + `Invalid config in ${filePath}: oauth.clientMetadataPath must be an origin-relative path (e.g. "/client-metadata.json") — this path becomes the document's own client_id, so a value such as "//host/doc" would publish a client id naming a host this server does not serve`, + ); + } if (transportType === "stdio" && oauth.enabled === true) { throw new Error( `Invalid config in ${filePath}: oauth requires streamable-http or sse transport`, diff --git a/test-servers/src/test-server-oauth.ts b/test-servers/src/test-server-oauth.ts index 1ca7fa2da..19c716ca8 100644 --- a/test-servers/src/test-server-oauth.ts +++ b/test-servers/src/test-server-oauth.ts @@ -101,6 +101,27 @@ function asMetadataPath(config: OAuthConfig): string { return path; } +/** + * Where the CIMD client metadata document is served from, validated the same + * way the two metadata paths above are — and for a sharper reason than either. + * This path is not merely advertised: it becomes the document's own + * `client_id`, so a value such as `//other-host/doc` would publish a client id + * naming a host this server does not control, and `/doc?version=1` would + * publish one that cannot reach the route Express registered (Copilot). + */ +function clientMetadataPath(config: OAuthConfig): string { + const path = config.clientMetadataPath; + if (path === undefined) { + return "/client-metadata.json"; + } + if (!isOriginRelativePath(path)) { + throw new Error( + `oauth.clientMetadataPath must be an origin-relative path (got ${JSON.stringify(path)})`, + ); + } + return path; +} + /** * The `WWW-Authenticate` challenge sent with every 401. * @@ -294,15 +315,21 @@ function setupMetadataEndpoints( // nothing. if (config.supportCIMD && config.clientMetadata) { const doc = config.clientMetadata; - const metadataPath = config.clientMetadataPath ?? "/client-metadata.json"; + const metadataPath = clientMetadataPath(config); app.get(metadataPath, (req: Request, res: Response) => { // Derived from the request rather than from `issuerUrl`, so the // document's own `client_id` always equals the URL it was fetched from // — which is what CIMD requires, and what stays true if the server // walked to another port on EADDRINUSE. + // + // `originalUrl` rather than the registered route, so a client id that + // carries a query string (`/client-metadata.json?profile=a`) still gets + // a document whose `client_id` is byte-identical to the URL that was + // fetched. Answering with the bare route instead would hand back a + // document that fails the very equality CIMD turns on (Copilot). const requestBaseUrl = `${req.protocol}://${req.get("host")}`; res.json({ - client_id: new URL(metadataPath, requestBaseUrl).href, + client_id: new URL(req.originalUrl, requestBaseUrl).href, client_name: doc.clientName ?? "MCP Inspector (CIMD test fixture)", redirect_uris: doc.redirectUris, // CIMD clients are public and authenticate with nothing; the server's