diff --git a/clients/web/src/test/core/client/config.test.ts b/clients/web/src/test/core/client/config.test.ts index 3b6e2663..a3d94239 100644 --- a/clients/web/src/test/core/client/config.test.ts +++ b/clients/web/src/test/core/client/config.test.ts @@ -335,7 +335,27 @@ describe("client config", () => { ); }); - it("does not widen the exemption beyond the three literals", () => { + it("accepts alternate spellings that canonicalize to the same addresses", () => { + // The check runs against the WHATWG-canonicalized `URL.hostname`, so the + // exemption covers these three *addresses*, not three input strings. Each + // of these reaches the loopback interface, which is what the security + // argument is about — and the SDK canonicalizes identically, so an alias + // accepted here is one it accepts for a token endpoint too. + for (const value of [ + "http://127.1:8090/client-metadata.json", + "http://2130706433:8090/client-metadata.json", + "http://0x7f.0.0.1:8090/client-metadata.json", + "http://[0:0:0:0:0:0:0:1]:8090/client-metadata.json", + // WHATWG strips a root-anchored dot from an IP literal but not from a + // name, which is why this passes where `localhost.` below does not. + "http://127.0.0.1.:8090/client-metadata.json", + "http://LOCALHOST:8090/client-metadata.json", + ]) { + expect(getCimdClientMetadataUrlError(value)).toBeUndefined(); + } + }); + + it("does not widen the exemption beyond those three addresses", () => { // `localhost.` and `*.localhost` resolve to loopback on every resolver, // but they are outside the SDK's allow-list — so they are outside ours // too, deliberately, rather than the two disagreeing about one URL. diff --git a/core/client/config-parse.ts b/core/client/config-parse.ts index fe783664..48f762d4 100644 --- a/core/client/config-parse.ts +++ b/core/client/config-parse.ts @@ -83,19 +83,34 @@ export const CIMD_METADATA_URL_HTTPS_ERROR = * validation would not let anyone enter, and CIMD could not be driven against * any fixture in this repo (#2305). * - * ⚠️ This is the SDK's *three-literal* list, deliberately, and not the broader - * `isLoopbackHost` in `core/node/hostUrl.ts` — which imports `node:net` and so - * cannot be reached from this browser-safe module. Matching the SDK literal for - * literal also keeps the two allow-lists from disagreeing about the same URL. - * `::1` arrives from `URL.hostname` bracketed, which is the form stored here. + * ⚠️ This is the SDK's own list, matched member for member — not the broader + * `isLoopbackHost` in `core/node/hostUrl.ts`, which imports `node:net` and so + * cannot be reached from this browser-safe module. The SDK's + * `assertSecureTokenEndpoint` tests `url.hostname` against exactly + * `localhost` / `127.0.0.1` / `[::1]` (plus a `::1` arm `URL.hostname` never + * produces, since it brackets IPv6 literals). Keeping the two identical is what + * stops them disagreeing about one URL. + * + * ⚠️ **The comparison is against the WHATWG-canonicalized `URL.hostname`, so it + * covers alternate spellings of these three *addresses*, not three input + * strings.** `http://127.1/…`, `http://2130706433/…`, `http://0x7f.0.0.1/…` and + * `http://[0:0:0:0:0:0:0:1]/…` all canonicalize into the set and are accepted; + * so is a root-anchored `http://127.0.0.1./…`, which WHATWG strips the dot from + * for IP literals but *not* for `localhost.`. That is deliberate on both counts: + * the security argument is about the address the request actually reaches, and + * every one of these is the loopback interface. It is also exactly what the SDK + * does — it canonicalizes through `new URL` the same way — so an alias accepted + * here is an alias the SDK accepts too. */ const CIMD_HTTP_EXEMPT_HOSTS = new Set(["localhost", "127.0.0.1", "[::1]"]); /** * True when a plain-`http:` CIMD metadata URL on this host is acceptable. Takes - * the bare host (`URL.hostname`), never a `host:port`. Not exported: the only - * caller is the validator below, and the exemption is exercised through it - * rather than through a second public surface that could drift from it. + * the already-canonicalized `URL.hostname`, never a raw authority or a + * `host:port` — passing an unparsed string would compare the wrong thing and + * silently reject every alias above. Not exported: the only caller is the + * validator below, and the exemption is exercised through it rather than + * through a second public surface that could drift from it. */ function isCimdHttpExemptHost(hostname: string): boolean { return CIMD_HTTP_EXEMPT_HOSTS.has(hostname); diff --git a/specification/v2_auth_smoke_testing.md b/specification/v2_auth_smoke_testing.md index c3168e16..340e1b84 100644 --- a/specification/v2_auth_smoke_testing.md +++ b/specification/v2_auth_smoke_testing.md @@ -513,10 +513,14 @@ Same MCP URL (`stytch-as-demo.val.run/mcp` or `mcp.stytch.dev/mcp`), leave CIMD For offline CIMD regression — or a **strict** fail-with-CIMD-off / succeed-with-CIMD-on pair without Stytch’s DCR fallback — use the shipped `oauth-cimd-http.json` fixture, which sets `supportCIMD: true` and **`supportDCR: false`** and hosts the client metadata document itself at `/client-metadata.json`. ```bash -cd clients/web && npm run test-servers:build +# The build script lives in clients/web; both paths below are repo-root-relative, +# so come back up before starting the server. +(cd clients/web && npm run test-servers:build) node test-servers/build/server-composable.js --config test-servers/configs/oauth-cimd-http.json ``` +The server announces its URL on **stderr**, and walks to the next free port on `EADDRINUSE` — read the announced URL rather than assuming 8092. + Since [#2305](https://github.com/modelcontextprotocol/inspector/issues/2305) a loopback `http://` metadata URL is a **valid persisted `client.json` setting and a valid entry in the settings form**, not a test-only affordance — `getCimdClientMetadataUrlError` exempts `localhost`, `127.0.0.1` and `[::1]`, the same three literals the SDK exempts for token endpoints. So point `clientMetadataUrl` straight at the URL the fixture announced (`http://127.0.0.1:8092/client-metadata.json` on its configured port) with no HTTPS listener. Before that change the exemption existed only in `ensureCimdClientRegistration`, for an already-stored `client_id`, and driving this by hand needed a self-signed HTTPS listener plus `NODE_TLS_REJECT_UNAUTHORIZED=0`. With CIMD off in Client Settings, connect + authenticate should **fail**; with CIMD on and that local metadata URL, it should **succeed**. `docs/test-servers.md` has the full walkthrough, including the `redirect_uris` port trap.