fix(transport): return 404 from /register so MCP SDK falls through cleanly#83
Conversation
…eanly PR #38 (51cbdb8) added OAuth discovery stubs to fix HTML-404 parse errors, but chose 501 for POST /register. Claude Code's MCP SDK treats 501 as a fatal "Dynamic client registration is not supported" error and surfaces: SDK auth failed: Dynamic client registration is not supported by this MCP server. Auth is tunnel-delegated — connect without OAuth. The user's ~/.claude.json has the server as { type: "http", url: ... } with no auth, so the SDK should fall through to unauthenticated when discovery returns "no auth." A 404 reads as "no such endpoint, fall through"; a 501 reads as "DCR explicitly rejected — fatal." The .well-known/* stubs already return 404 via a shared oauthNotSupported handler. Route /register through the same handler — single source of truth for "no OAuth, anywhere," and the SDK probe completes cleanly. Info-disclosure hardening preserved: mcp_auth: "tunnel-delegated" debug field only ships on localhost requests; the public Cloudflare-tunneled response stays generic. Tests: src/__tests__/HttpTransport.oauth_stubs.test.ts covers all four endpoints, both the 404-shape contract and the localhost-only debug field hardening (9 cases). Full suite: 404/404 passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
User does not have a PR Review subscription. Go to Team management and add this email to the PR Review subscription. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR unifies the ChangesOAuth Stub Behavior Alignment
🎯 3 (Moderate) | ⏱️ ~20 minutes
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modifies the HttpTransport to return a 404 status code for the /register endpoint, replacing the previous 501 response to allow the MCP SDK to fall back to unauthenticated connections. It also introduces a new test suite to verify OAuth stubs and info-disclosure hardening. Feedback includes a security recommendation to use req.ip instead of req.hostname for local request verification to prevent spoofing, and a suggestion to define shared metadata strings as constants to maintain consistency across the codebase.
| }) | ||
| } | ||
| }) | ||
| this.app.post('/register', oauthNotSupported) |
There was a problem hiding this comment.
The oauthNotSupported handler (and the isLocalRequest helper it uses) relies on req.hostname to determine if a request is local for info-disclosure hardening. Since req.hostname is derived from the client-provided Host header (or X-Forwarded-Host when trust proxy is enabled), it can be spoofed by external attackers to leak internal auth metadata. Consider using req.ip or req.socket.remoteAddress for a more reliable local request check.
| error: 'oauth_not_supported', | ||
| mcp_auth: 'tunnel-delegated' |
There was a problem hiding this comment.
The error string 'oauth_not_supported' and the debug field value 'tunnel-delegated' are hardcoded in both the implementation and the tests. Per the project's general rules, these shared metadata keys should be defined as constants in a common location to serve as a single source of truth for handoff contracts between producers and consumers, ensuring consistency and avoiding bugs from typos.
References
- Define shared transient metadata keys as constants in a common location to serve as a single source of truth for handoff contracts between producers and consumers, ensuring consistency and avoiding bugs from typos.
1 participant
Summary
SDK auth failed: Dynamic client registration is not supported by this MCP server. Auth is tunnel-delegated — connect without OAuth.when connecting to the local KMS athttp://localhost:8180/mcp.oauthNotSupportedhandler the .well-known endpoints already use. Single source of truth for "no OAuth, anywhere," returning a JSON 404 the SDK treats as "fall through."What changed
src/transport/HttpTransport.ts:this.app.post('/register', oauthNotSupported).src/__tests__/HttpTransport.oauth_stubs.test.ts(new, 9 cases):error: "oauth_not_supported"error_description(string)mcp_auth: "tunnel-delegated"debug field.mcp_auth— info-disclosure hardening preserved.What this doesn't solve
The Claude Code SDK arguably shouldn't probe DCR after the .well-known endpoints all return 404 — that's an upstream concern. The 404 workaround is the pragmatic server-side fix and unblocks the user immediately. After merge & rebuild, Rich needs to click "Reconnect" in the Claude Code MCP UI to pick up the change.
Test plan
npx tsc --noEmit— cleannpm test— 404/404 passing (19 suites, 9 new test cases)npm run build— clean (dist diff matches source change exactly)POST /registercurrently returns 501 against the live daemoncurl -s -o /dev/null -w "HTTP %{http_code}\n" -X POST http://localhost:8180/register -H "Content-Type: application/json" -d '{}'should returnHTTP 404Constraints honored
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests