fix(auth): allow-any redirect fallback broken by MCP SDK ≥1.29#37
Merged
Conversation
The allow-any redirect_uri fallback (used when MCP_REDIRECT_URIS is not set) was implemented as a Proxy faking only Array.prototype.includes(). SDK 1.29 changed the authorize handler's membership check from .includes() to .some(redirectUriMatches), which iterates the empty backing array — so every authorization request failed with 400 'Unregistered redirect_uri' (hit by claude.ai connectors on fresh 'npx openclaw-mcp@latest' installs, which resolve the ^1.x SDK range to 1.29+ regardless of our lockfile). - Replace the Proxy with an Array subclass overriding both includes() and some(), covering SDK <=1.28 and >=1.29 membership checks - Keep the backing list empty so an omitted redirect_uri now yields a clean validation error instead of a redirect to 'undefined' - Bump @modelcontextprotocol/sdk to ^1.29.0 so CI tests run against the SDK that fresh installs actually resolve - Add regression tests: claude.ai callback against the allow-any default, plus positive/negative coverage for an explicit MCP_REDIRECT_URIS allow-list
…ct_uri troubleshooting The MCP_REDIRECT_URIS example in .env.example pointed to https://claude.ai/oauth/callback, which is not the callback Claude.ai actually uses — anyone copying it would lock Claude.ai out. The real callback is https://claude.ai/api/mcp/auth_callback (claude.com variant included). - Fix the example URI in .env.example and recommend it in README, configuration.md, and the deployment security checklist - Add a redirect-URI allow-list section to configuration.md (exact matching, RFC 8252 loopback port relaxation) - Add a troubleshooting entry for 'Unregistered redirect_uri' covering both the wrong-URI case and the SDK 1.29 allow-any regression in bridge <=1.5.0 - Pass TRUST_PROXY through the deployment compose example and sample .env
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #36
Problem
With
MCP_REDIRECT_URISunset (the documented allow-any default), every Claude.ai OAuth attempt fails withinvalid_request/Unregistered redirect_urion fresh installs.The allow-any fallback was a
Proxyfaking onlyArray.prototype.includes(). MCP SDK 1.29 switched the authorize handler's membership check to.some(redirectUriMatches), which iterates the empty backing array — so the fallback silently rejected everything.npx openclaw-mcp@latestresolves the^1.xSDK range fresh (ignoring our lockfile, which pinned 1.25.3), so production installs broke while CI stayed green.Changes
src/auth/provider.ts— replace the Proxy with anArraysubclass overriding bothincludes()(SDK ≤1.28) andsome()(SDK ≥1.29). The backing list stays empty, so an omittedredirect_urinow yields a clean validation error instead of a redirect toundefined.package.json— bump@modelcontextprotocol/sdkfloor to^1.29.0so CI runs against what fresh installs actually resolve.https://claude.ai/api/mcp/auth_callback) against the allow-any default, plus positive/negative coverage for an explicitMCP_REDIRECT_URISallow-list. Verified the old Proxy fails these tests against SDK 1.29 (400instead of302— exactly the production symptom)..env.exampleshowedhttps://claude.ai/oauth/callbackas theMCP_REDIRECT_URISexample, which is not Claude's real callback and would itself lock Claude.ai out; corrected everywhere and added a troubleshooting entry forUnregistered redirect_uri.Compatibility
No behavior change for existing configurations:
MCP_REDIRECT_URISallow-lists validate exactly as beforeTest plan