From 0a8424699a37fa80fb110d008f72fe6d7877e8fa Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 21 Jul 2026 14:51:42 +0000 Subject: [PATCH] status: fix DCR check for the hosted-redirect allow-list; monitor the guard The OAuth Dynamic Client Registration check registered with a hosted redirect (https://example.org/callback) and expected a client_id. Since the allow-list shipped (#69), that hosted redirect is correctly rejected with 400 invalid_redirect_uri, so the check went red for a working server. - oauth-register: register a LOOPBACK redirect instead (always permitted; the allow-list exempts loopback), which still confirms a client can self-register and get a client_id, the case DCR is for. - oauth-register-allowlist (new): register a non-allow-listed hosted redirect and assert it's rejected with 400 invalid_redirect_uri, so the dashboard positively monitors that the CWE-601 guard stays enforced. - test harness: stubFetch now accepts a function route so a single endpoint can answer by request body; the /oauth/register mock mirrors the allow-list (201 for loopback, 400 for a hosted non-allow-listed redirect). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_014T9N8USDfNK5yzznPGg7Ym --- monitoring/mcp-status/checks.js | 50 +++++++++++++++- monitoring/mcp-status/checks.test.js | 85 +++++++++++++++++++++++++--- 2 files changed, 124 insertions(+), 11 deletions(-) diff --git a/monitoring/mcp-status/checks.js b/monitoring/mcp-status/checks.js index 2904a90..b44735f 100644 --- a/monitoring/mcp-status/checks.js +++ b/monitoring/mcp-status/checks.js @@ -382,7 +382,10 @@ export const checkMcpEndpoints = async (mcpOrigin, timeoutMs) => { }); } - // 5. Dynamic Client Registration (RFC 7591) must mint a client_id. + // 5. Dynamic Client Registration (RFC 7591) must mint a client_id. Registered + // with a LOOPBACK redirect: loopback is always permitted (the hosted-redirect + // allow-list exempts it), so this is the dependency-free way to confirm a + // client can self-register without manual approval, which is what DCR is for. { const url = `${mcpOrigin}/oauth/register`; const r = await probe(url, { @@ -391,7 +394,7 @@ export const checkMcpEndpoints = async (mcpOrigin, timeoutMs) => { headers: { "content-type": "application/json" }, body: JSON.stringify({ client_name: "imcp-status-dashboard", - redirect_uris: ["https://example.org/callback"], + redirect_uris: ["http://127.0.0.1:8765/callback"], token_endpoint_auth_method: "none", grant_types: ["authorization_code"], response_types: ["code"], @@ -406,7 +409,7 @@ export const checkMcpEndpoints = async (mcpOrigin, timeoutMs) => { id: "oauth-register", label: "OAuth Dynamic Client Registration", description: - "Confirms Dynamic Client Registration (RFC 7591) issues a client_id, so MCP clients can self-register without manual setup.", + "Confirms Dynamic Client Registration (RFC 7591) issues a client_id for a loopback client, so native/CLI MCP clients can self-register without manual setup.", target: `POST ${url}`, expected: "200/201 JSON with client_id", status: pass ? "pass" : "fail", @@ -420,6 +423,47 @@ export const checkMcpEndpoints = async (mcpOrigin, timeoutMs) => { }); } + // 5b. The hosted-redirect allow-list must REJECT a hosted redirect_uri on a + // domain that isn't approved (CWE-601 auth-code phishing guard). A + // non-allow-listed hosted redirect is refused with 400 invalid_redirect_uri + // before any client_id is issued; loopback (above) stays exempt. The probe + // uses a reserved `.invalid` host (RFC 2606) that can never be legitimately + // allow-listed by a deployment, so this can't false-alert if the allow-list + // is widened via OAUTH_ALLOWED_REDIRECT_DOMAINS. + { + const url = `${mcpOrigin}/oauth/register`; + const r = await probe(url, { + timeoutMs, + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + client_name: "imcp-status-dashboard-probe", + redirect_uris: ["https://not-allowlisted.invalid/callback"], + token_endpoint_auth_method: "none", + grant_types: ["authorization_code"], + response_types: ["code"], + }), + }); + const json = tryJson(r.bodyText); + const pass = r.status === 400 && json && json.error === "invalid_redirect_uri"; + checks.push({ + id: "oauth-register-allowlist", + label: "OAuth redirect allow-list enforced", + description: + "Confirms a hosted redirect_uri on a non-allow-listed domain is rejected (400 invalid_redirect_uri), closing the open-registration auth-code phishing vector (CWE-601). Loopback redirects stay exempt.", + target: `POST ${url}`, + expected: "400 invalid_redirect_uri", + status: pass ? "pass" : "fail", + httpStatus: r.status, + latencyMs: r.latencyMs, + detail: r.error + ? `request failed: ${r.error.message}` + : pass + ? "rejected non-allow-listed hosted redirect_uri" + : `expected 400 invalid_redirect_uri, got ${r.status}: ${r.bodyText.slice(0, 120)}`, + }); + } + // 6. Authorization endpoint liveness: rejects malformed input with 4xx // (rather than 5xx / connection error). It is interactive, so we only // assert it is alive and validating, not a full successful redirect. diff --git a/monitoring/mcp-status/checks.test.js b/monitoring/mcp-status/checks.test.js index 19a4966..69be626 100644 --- a/monitoring/mcp-status/checks.test.js +++ b/monitoring/mcp-status/checks.test.js @@ -34,14 +34,17 @@ const resp = (status, { headers = {}, body = "" } = {}) => ({ }); /** - * Install a fetch stub that dispatches on "METHOD url". - * @param {Record>} routes + * Install a fetch stub that dispatches on "METHOD url". A route value is either a + * `resp(...)` object or a function `(init) => resp(...)` that answers by request + * (used to mirror the /oauth/register allow-list, which branches on the body). + * @param {Record | ((init: RequestInit) => ReturnType)>} routes */ const stubFetch = (routes) => { const original = globalThis.fetch; globalThis.fetch = async (url, init = {}) => { const key = `${init.method ?? "GET"} ${url.split("?")[0]}`; - if (routes[key]) return routes[key]; + const route = routes[key]; + if (route) return typeof route === "function" ? route(init) : route; throw new Error(`unexpected fetch: ${key}`); }; return () => { @@ -49,6 +52,33 @@ const stubFetch = (routes) => { }; }; +/** + * `/oauth/register` mock mirroring the hosted-redirect allow-list: a loopback + * redirect mints a client_id (201); a non-allow-listed hosted redirect is + * rejected (400 invalid_redirect_uri). + */ +const registerRoute = (init) => { + const ru = (JSON.parse(init.body ?? "{}").redirect_uris ?? [])[0] ?? ""; + let loopback = false; + try { + const u = new URL(ru); + // Match the server's is_loopback_redirect: http scheme, NO userinfo, and a + // loopback HOSTNAME (so a look-alike like http://localhost.evil.com, or an + // authority trick like http://user@127.0.0.1, is NOT loopback). URL.hostname + // yields the bracketed [::1]; accept the bare ::1 too for robustness. + loopback = + u.protocol === "http:" && + u.username === "" && + u.password === "" && + ["localhost", "127.0.0.1", "[::1]", "::1"].includes(u.hostname); + } catch { + loopback = false; + } + return loopback + ? resp(201, { body: JSON.stringify({ client_id: "client-123" }) }) + : resp(400, { body: JSON.stringify({ error: "invalid_redirect_uri" }) }); +}; + test("worstStatus picks the most severe status", () => { assert.equal(worstStatus(["pass", "pass"]), "pass"); assert.equal(worstStatus(["pass", "warn", "pass"]), "warn"); @@ -155,9 +185,7 @@ test("checkMcpEndpoints passes for a well-behaved server", async () => { }, body: JSON.stringify({ error: "invalid_token" }), }), - [`POST ${origin}/oauth/register`]: resp(201, { - body: JSON.stringify({ client_id: "client-123" }), - }), + [`POST ${origin}/oauth/register`]: registerRoute, [`GET ${origin}/oauth/authorize`]: resp(400, { body: "missing client_id" }), [`POST ${origin}/oauth/token`]: resp(400, { body: JSON.stringify({ error: "invalid_grant" }), @@ -172,6 +200,7 @@ test("checkMcpEndpoints passes for a well-behaved server", async () => { assert.equal(byId(section, "metadata-consistency").status, "pass"); assert.equal(byId(section, "mcp-challenge").status, "pass"); assert.equal(byId(section, "oauth-register").status, "pass"); + assert.equal(byId(section, "oauth-register-allowlist").status, "pass"); assert.equal(byId(section, "oauth-authorize").status, "pass"); assert.equal(byId(section, "oauth-token").status, "pass"); // Deployment facts are captured and a GitHub commit link is derived. @@ -213,8 +242,45 @@ test("checkMcpEndpoints flags a missing OAuth challenge", async () => { }), // 200 instead of a 401 challenge → wrong contract. [`POST ${origin}/mcp`]: resp(200, { body: "{}" }), + [`POST ${origin}/oauth/register`]: registerRoute, + [`GET ${origin}/oauth/authorize`]: resp(400), + [`POST ${origin}/oauth/token`]: resp(400, { + body: JSON.stringify({ error: "invalid_grant" }), + }), + }); + try { + const { section } = await checkMcpEndpoints(origin, 2000); + assert.equal(byId(section, "mcp-challenge").status, "fail"); + } finally { + restore(); + } +}); + +test("checkMcpEndpoints flags a missing hosted-redirect allow-list", async () => { + const origin = "https://mcp.beta.test"; + const restore = stubFetch({ + [`GET ${origin}/`]: resp(200, { headers: { "content-type": "text/html" } }), + [`GET ${origin}/version`]: resp(404), + [`GET ${origin}/.well-known/oauth-protected-resource`]: resp(200, { + body: JSON.stringify({ authorization_servers: [origin], resource: `${origin}/mcp` }), + }), + [`GET ${origin}/.well-known/oauth-authorization-server`]: resp(200, { + body: JSON.stringify({ + issuer: origin, + authorization_endpoint: `${origin}/oauth/authorize`, + token_endpoint: `${origin}/oauth/token`, + registration_endpoint: `${origin}/oauth/register`, + }), + }), + [`POST ${origin}/mcp`]: resp(401, { + headers: { + "www-authenticate": `Bearer resource_metadata="${origin}/.well-known/oauth-protected-resource"`, + }, + body: JSON.stringify({ error: "invalid_token" }), + }), + // Guard MISSING: the server accepts ANY redirect (even a hosted one) with 201. [`POST ${origin}/oauth/register`]: resp(201, { - body: JSON.stringify({ client_id: "x" }), + body: JSON.stringify({ client_id: "leaked" }), }), [`GET ${origin}/oauth/authorize`]: resp(400), [`POST ${origin}/oauth/token`]: resp(400, { @@ -223,7 +289,10 @@ test("checkMcpEndpoints flags a missing OAuth challenge", async () => { }); try { const { section } = await checkMcpEndpoints(origin, 2000); - assert.equal(byId(section, "mcp-challenge").status, "fail"); + // DCR still mints a client_id for the loopback probe, but the allow-list + // probe (a hosted redirect) wrongly succeeds, so the guard check goes red. + assert.equal(byId(section, "oauth-register").status, "pass"); + assert.equal(byId(section, "oauth-register-allowlist").status, "fail"); } finally { restore(); }