From d1fcbca63b66dd55008d87b10d4e1b7355b918d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 25 Oct 2026 12:55:36 +0100 Subject: [PATCH] fix(mcp): name the host an authorization goes to on the consent page Dynamic client registration is open by specification and the client names itself, so anyone could register under a well-known assistant's name with a callback on a host of their own and send a signed-in person the authorize link. The consent page showed the borrowed name and "Unverified application", never where the code would be sent. Every consent page now says where the access goes: the callback's host, or an application on this device for a loopback callback, and the Allow button names the host. For a self-registered client whose callback leaves the device, a warning that names the host leads the page, above the self-reported name. Registration stays open: the hosted assistants register this way with their own https callback. --- .../api/mcp/oauth/__tests__/authorize.test.ts | 56 +++++++++++++++++++ src/app/api/mcp/oauth/authorize/route.ts | 54 +++++++++++++++++- 2 files changed, 109 insertions(+), 1 deletion(-) diff --git a/src/app/api/mcp/oauth/__tests__/authorize.test.ts b/src/app/api/mcp/oauth/__tests__/authorize.test.ts index 24f12bb88..4b62e7bf3 100644 --- a/src/app/api/mcp/oauth/__tests__/authorize.test.ts +++ b/src/app/api/mcp/oauth/__tests__/authorize.test.ts @@ -1,6 +1,7 @@ /** * Authorize endpoint — consent gate + audience + PKCE + redirect-URI binding. */ +import { NextRequest } from "next/server"; import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; process.env.API_TOKEN_HMAC_KEY = "x".repeat(48); @@ -496,3 +497,58 @@ describe("authorize CSP — the consent form may lead only to the validated redi expect(csp(unavailable)).toContain("default-src 'none'"); }); }); + +/** + * Dynamic registration lets anyone register "Claude" with a callback on a + * host of their own and send a signed-in person the authorize link. The + * consent page used to show the borrowed name and "Unverified application", + * never where the code would go. + */ +describe("GET /authorize — where the access goes (M3)", () => { + it("names the redirect host for a self-registered client, on the page and on Allow", async () => { + const phish = registerDcrClient({ + clientName: "Claude", + redirectUris: ["https://collector.example.net/cb"], + }); + signedIn(); + const body = await ( + await GET( + new NextRequest( + authorizeUrl({ + client_id: phish.clientId, + redirect_uri: "https://collector.example.net/cb", + }), + ), + ) + ).text(); + expect(body).toMatch(/Check where this access goes/); + expect(body).toMatch( + /Access goes to:<\/strong> collector\.example\.net<\/code>/, + ); + expect(body).toMatch( + /value="allow">Allow access for collector\.example\.net { + const desktop = registerDcrClient({ + clientName: "Desktop client", + redirectUris: ["http://127.0.0.1:4000/callback"], + }); + signedIn(); + const body = await ( + await GET( + new NextRequest( + authorizeUrl({ + client_id: desktop.clientId, + redirect_uri: "http://127.0.0.1:4000/callback", + }), + ), + ) + ).text(); + expect(body).toMatch(/an application running on this device/); + expect(body).not.toMatch(/Check where this access goes/); + }); +}); diff --git a/src/app/api/mcp/oauth/authorize/route.ts b/src/app/api/mcp/oauth/authorize/route.ts index deb3d29c0..1748a85ae 100644 --- a/src/app/api/mcp/oauth/authorize/route.ts +++ b/src/app/api/mcp/oauth/authorize/route.ts @@ -420,6 +420,7 @@ async function authorizeGet(request: NextRequest): Promise { v.clientSource === "cimd" ? `

Verified origin: ${htmlEscape(v.clientOrigin)}

` : `

Unverified application (dynamically registered, the name above is self-reported and not verified).

`; + const destination = consentDestination(v.redirectUri, v.clientSource); // Plain-language access summary. When write is granted, say so explicitly — // the app will be able to READ and LOG/WRITE health data, not just read. const writeGranted = v.scope.split(/\s+/).includes(SCOPE_HEALTH_WRITE); @@ -436,8 +437,10 @@ async function authorizeGet(request: NextRequest): Promise { html( `

Authorize access

+ ${destination.warning}

${htmlEscape(v.clientName)} is requesting access to your HealthLog data.

${provenance} + ${destination.line}

Scope: ${htmlEscape(v.scope)}

${accessSummary} ${readSummary} @@ -450,7 +453,7 @@ async function authorizeGet(request: NextRequest): Promise { ${hidden("scope", v.scope)} ${hidden("resource", v.resource)} ${v.state !== undefined ? hidden("state", v.state) : ""} - +
`, @@ -460,6 +463,55 @@ async function authorizeGet(request: NextRequest): Promise { }); } +/** + * Where an Allow sends the access, said on the consent page in words a person + * checks: the redirect URI's host, or "an application on this device" for a + * loopback redirect. + * + * Dynamic registration is open by specification and the client names itself, + * so anyone can register "Claude" with a redirect to a host of their own and + * send a signed-in person the authorize link; the page used to show the + * borrowed name and never the host the code would go to. The host is now on + * every consent page and on the Allow button. For an unverified client whose + * redirect leaves this device, a warning leads the page. + * + * Registration itself stays open: the hosted assistants register this way + * with their own https callback, so closing it or allowlisting hosts would + * break them on every instance, and a switch left at its default would + * protect nobody. Naming where the access goes is what a phished person can + * act on. + */ +function consentDestination( + redirectUri: string, + clientSource: string, +): { warning: string; line: string; allowLabel: string } { + let url: URL; + try { + url = new URL(redirectUri); + } catch { + return { warning: "", line: "", allowLabel: "Allow" }; + } + const loopback = + url.protocol === "http:" && + ["localhost", "127.0.0.1", "[::1]", "::1"].includes(url.hostname); + if (loopback) { + return { + warning: "", + line: `

Access goes to: an application running on this device (${htmlEscape(url.host)}).

`, + allowLabel: "Allow", + }; + } + const host = htmlEscape(url.host); + return { + warning: + clientSource === "cimd" + ? "" + : `

Check where this access goes before you allow it. Whoever runs ${host} will be able to use it. Allow only if you started this connection from ${host} yourself; the application's name below is its own claim.

`, + line: `

Access goes to: ${host}

`, + allowLabel: `Allow access for ${host}`, + }; +} + async function authorizePost(request: NextRequest): Promise { return withBackgroundEvent("mcp.oauth.authorize", async () => { if (!(await surfaceAvailable())) {