安全研究员让我关闭 OAuth 注册端点，我拒绝了

Last week I got an unsolicited security report about my QR code service. It was polite, it was accurate about the facts, and both of its headline recommendations would have broken the product. I want to walk through it, because "the report was right and the fix was wrong" is a situation you will hit if you ship anything with an MCP server on it, and because the actual exposure turned out to be somewhere the report never looked. The finding POST /api/oauth/register accepts a registration from anyone, with any HTTPS redirect address, and saves it. No credentials, no review, no allowlist. All true. Here is the thing: that is the endpoint working as designed. Open Dynamic Client Registration is one of the two modes RFC 7591 defines, and the Model Context Protocol relies on it. When you paste a remote MCP server URL into Claude or ChatGPT, the client has to become an OAuth client of a server it has never met, in a few hundred milliseconds, with nobody available to approve a form. There is no human in that loop by design. Every real client I have - Claude, Glama, Oasis, Rayrun - arrived through exactly that path with no credentials. So when the report said require an Initial Access Token or manual app review before registering, what it was really proposing was: turn off the MCP server. The second suggestion was impossible, and the control already existed Enforce a redirect_uri allowlist. You cannot allowlist redirect addresses for applications that do not exist yet. That is the whole point of dynamic registration. What you can do - and what any correct implementation already does - is refuse any redirect at authorization time that the client did not register for itself. Mine checks it on the authorize call and re-checks it at the token exchange: if (!redirectAllowed(client, redirectUri)) throw new OAuthError("invalid_request", "The redirect address is not registered for this app."); Enter fullscreen mode Exit fullscreen mode Registering a client with your own redirect gets you a client that redirects to you. It does not get you anyone else's authorization code. There is no open redirect here, and the report's mental model - "open registration means open redirects" - conflated two different things. Also already in place, and worth stating because it is the control that makes the rest survivable: PKCE with S256 is mandatory for every dynamically registered client. Not optional, not plain: if (client.dynamic && (!codeChallenge || method !== "S256")) throw new OAuthError("invalid_request", "PKCE with code_challenge_method=S256 is required."); Enter fullscreen mode Exit fullscreen mode An intercepted authorization code is worthless on its own. The hole the report missed Here is the part that kept me up, and it is not in the endpoint at all. It is on the consent screen. client_name is a free-text field supplied by whoever registers. My consent screen rendered it directly: Connect {client_name} to your QRFLOW.codes account? In my own brand color. With the actual destination in small print underneath. So anyone could have registered a client called "QRFLOW Official Support", pointed the redirect at their own server, and sent people a link to a consent screen on my real domain, with my real TLS certificate, asking them to connect QRFLOW Official Support to their QRFLOW account. Nothing in that flow is a vulnerability in the usual sense. Every component behaves exactly as specified. The attack is that I was rendering attacker-controlled text as though it were established fact. Open registration means the name field is attacker-controlled. If you render it, you are part of the attack. What I actually shipped Three changes, none of which touch the ability to register. 1. Recognize apps by redirect host, never by name. const KNOWN_CLIENT_HOSTS = new Set([ "claude.ai", "claude.com", "chatgpt.com", "chat.openai.com", "platform.openai.com", "cursor.com", "cursor.sh", "glama.ai", "www.canva.com", ]); Enter fullscreen mode Exit fullscreen mode The host is the one part of a registration an attacker cannot fake, because they have to actually receive the callback there. The name is the part they control completely. So the host decides, and the name is only ever displayed. I kept this as a hardcoded list rather than a database table someone can edit from an admin screen. It changes a few times a year, and a security control you can edit at runtime is a bigger target than one that needs a deploy. 2. An unrecognized app gets a warning that names the destination. Before, the amber warning only appeared for loopback clients - apps on your own machine. An unknown app at a remote address got nothing. Now anything off the list gets the banner, the destination host is stated inside it rather than in a footnote, and the name is phrased as a claim: It calls itself X and will send you to example.com. "Calls itself" is doing real work in that sentence. The reason the known list exists at all is so the warning stays rare. Warn on every screen and you have trained everyone to click through it. 3. Refuse the impersonation at the door. // Nobody self-registers as us. if (/qrflow/i.test(rawName)) throw new OAuthError("invalid_client_metadata", 'Client names may not contain "QRFLOW".'); Enter fullscreen mode Exit fullscreen mode One line. It removes the most convincing version of the attack, which is the one that uses my own brand against me. The rate limit, and a serverless trap The report's third suggestion was the good one: nothing stopped you creating unlimited rows in oauth_clients. Not a break-in, but a junk-data and cost problem. The trap is where you put the counter. I already had an in-memory Map rate-limiting something else in this codebase, and reaching for it here would have been the obvious move. On serverless, an in-memory rate limit is decorative. Vercel runs many instances; a counter in one process is bypassed by the load balancer handing the next request to a different one. It is not a weak control, it is close to no control, and it looks exactly like a real one in code review. So the counter went into Postgres, mirroring the function already behind my API key limiter: const { data: bump } = await supabaseAdmin.rpc("oauth_register_bump", { p_ip: ip }); if (typeof bump === "number" && bump > REGISTRATIONS_PER_IP_PER_HOUR) { console.warn("[oauth/register] rate limited", { ip, count: bump }); throw new RateLimited(3600 - (Math.floor(Date.now() / 1000) % 3600)); } Enter fullscreen mode Exit fullscreen mode Ten per address per hour. I sized that off real traffic rather than instinct: the heaviest legitimate burst in my whole history was Claude registering six times in a few minutes during my own testing. Ten leaves headroom. And every refusal is logged, because the failure mode of a rate limit is silently blocking someone real. What I would take from this Open DCR is a feature. Do not let a scanner talk you out of it. If your MCP server requires manual approval to register, it does not work with the clients people actually use. The spec tells you what to verify, not what to render.RFC 7591 says nothing about how to display client_name, so everyone displays it, and that is where the attack lives. Sort every field into "they control this" and "they cannot fake this." The redirect host is in the second bucket because they must receive traffic there. Almost everything else in a registration payload is in the first. A report can be entirely correct and still propose a fix that ships you backwards. The facts were right. The recommendations were written by someone who had not asked what the endpoint was for. The consent screen change is the one I would do first if I were starting again, and it is the one nobody flagged. I build QRFLOW.codes, a QR code service with an MCP server so you can make and re-point codes by asking an assistant. The developer docs cover the OAuth flow, the REST API and the MCP tools.