mcp_oauth: configurable redirect_host for WAF-safe OAuth redirect URIs - #63889
mcp_oauth: configurable redirect_host for WAF-safe OAuth redirect URIs#63889Peterskaronis wants to merge 1 commit into
Conversation
Reclaim.ai's AWS API Gateway WAF 403s any /oauth2/authorize request whose query string contains a literal 127.0.0.1, so the SDK's hardcoded redirect_uri made the browser flow impossible. New optional oauth config key redirect_host (default 127.0.0.1, unchanged behavior) lets a server entry use localhost instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Duplicate of #61755 (earlier, open) — both add a configurable |
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: LGTM (token read-only, submitted as COMMENT)
Adds a configurable redirect_host to MCP OAuth client metadata so WAFs that reject 127.0.0.1 in authorize queries (cloudfront/apigateway style) can use localhost. The redirect listener still binds 127.0.0.1 either way, so no security boundary changes — only the URI string the provider sees.
Notes:
- Defaults to
127.0.0.1for backward compatibility. - Docstring names the specific failure mode (Reclaim.ai AWS API Gateway FORBIDDEN).
- Single-file, single-line semantics change, low risk.
Reviewed by Hermes Agent batch cron (token read-only).
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating a real compatibility problem: current main still advertises 127.0.0.1 at tools/mcp_oauth.py:849.
Problems
- The PR changes only
_build_client_metadata; pre-registered clients still writehttp://127.0.0.1:{port}/callbackattools/mcp_oauth.py:876. Configurations usingoauth.client_idtherefore do not receive the requested override. redirect_hostis accepted without validation, while the callback listener remains bound to127.0.0.1attools/mcp_oauth.py:655. An arbitrary configured hostname can advertise a callback Hermes cannot receive.- The diff has no regression tests. Existing metadata tests start at
tests/tools/test_mcp_oauth.py:712and do not cover configured hosts or the pre-registered path.
Suggested changes
- Share one validated effective redirect-URI helper between
_build_client_metadataand_maybe_preregister_client. - Add dynamic-registration and
client_idtests for the configured host, and align the SSH/docs messaging with the effective URI.
This is an automated hermes-sweeper review.
| # returning ``{"message":"Forbidden"}``. ``redirect_host: localhost`` in | ||
| # the server's oauth config works around that; the callback listener | ||
| # still binds 127.0.0.1 either way. | ||
| redirect_host = cfg.get("redirect_host", "127.0.0.1") |
There was a problem hiding this comment.
redirect_host is unrestricted here, but the callback listener remains fixed to 127.0.0.1 (tools/mcp_oauth.py:655). Please restrict this to hosts the listener actually serves, or bind the listener consistently; otherwise a configured non-loopback host advertises a callback Hermes cannot receive.
|
Merged via PR #65610 with your commit cherry-picked onto current main and your authorship preserved in git log — thanks for tracking down the Reclaim.ai WAF behavior! Your |
Upstream hat NousResearch#47755 (configurable redirect_uri) via NousResearch#65610 gemerged und dabei umgebaut: _make_redirect_handler-Closure statt functools.partial (NousResearch#44588/NousResearch#44590), _resolve_redirect_uri mit neuer redirect_host-Praezedenz (NousResearch#63889), plus eine TOCTOU-Portreservierung (NousResearch#22161). Konflikte in tools/mcp_oauth.py und tests/tools/test_mcp_oauth.py zugunsten der Upstream-Seite aufgeloest; unser oauth.redirect_bind neu darauf aufgesetzt: - _oauth_bind_host global (Default Loopback) — jetzt explizit abgegrenzt gegen upstreams redirect_host (das nur die *angekuendigte* URI umschreibt) - _configure_callback_port loest redirect_bind auf, VOR der Portwahl - _reserve_callback_port bindet auf _oauth_bind_host (NEUE zweite Bindstelle aus NousResearch#22161 — sonst waeren Reservierung und HTTPServer uneinig) - HTTPServer bindet (_oauth_bind_host, port) - _find_free_port(host=None) faellt auf _oauth_bind_host zurueck Tests als TestRedirectBind neu aufgesetzt (die alte Datei war upstream klassenbasiert reorganisiert), inkl. Regressionstest fuer die Reservierung. 5 passed. Der eine Fehlschlag in test_mcp_oauth.py (test_build_oauth_auth_preserves_server_url_path) ist vorbestehend und umgebungsbedingt — er faellt auf unveraendertem upstream/main identisch aus (mcp-SDK im Devcontainer nicht installiert). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Problem
Some MCP providers sit behind WAFs that reject any
/authorizerequest whose query string contains a literal127.0.0.1. Reclaim.ai's MCP (https://mcp.reclaim.ai, AWS API Gateway) is a live example: every authorize attempt returns{"message":"Forbidden"}before reaching the OAuth application, because_build_client_metadatahardcodesClients that use
localhostin the redirect URI (e.g. Claude Code) pass the same WAF fine, so the failure looks provider-side and is painful to diagnose — the browser shows Forbidden even for a logged-in user, for every client_id (valid, freshly-registered, or bogus).Fix
Add an optional
redirect_hostkey to a server'soauthconfig block, defaulting to127.0.0.1(behavior unchanged for existing configs):The callback listener still binds
127.0.0.1either way (localhostresolves there), so only the registered/requested redirect URI string changes.Verified
Live against Reclaim.ai's MCP: with
redirect_host: localhost, DCR + authorize + token exchange +tools/listcomplete end-to-end (28 tools). With the default127.0.0.1, the same flow is 403-blocked at the edge.🤖 Generated with Claude Code