fix(mcp): close OAuth callback port races — closures, reuse_address, TOCTOU (salvage of #44872 + #22161) - #65622
Merged
Merged
Conversation
…#44588, #44590) Two related OAuth fixes: 1. Replace module-level _redirect_handler with _make_redirect_handler() closure factory that closes over the resolved port. This prevents cross-server state pollution when multiple MCP servers run OAuth concurrently (#44588). 2. Set server.allow_reuse_address = True on the ephemeral callback HTTPServer so the socket doesn't stay in TIME_WAIT after the flow completes. This prevents 'Address already in use' errors on the next OAuth flow for the same port (#44590). Fixes #44588 Fixes #44590
_find_free_port() closed its probe socket before HTTPServer re-bound the port minutes later, leaving a window where another process could steal it (#22161 by @amathxbt). _reserve_callback_port() now keeps the selected socket bound (bounded FIFO pool) until _wait_for_callback adopts it via bind_and_activate=False. Also sets allow_reuse_address BEFORE binding — the cherry-picked #44872 set it after the constructor had already bound, where it is a no-op. Also updates the three #57836 non-interactive-guard tests to the closure-factory API from #44872.
alt-glitch
added
type/bug
Collaborator
This was referenced Jul 16, 2026
tonydwb
reviewed
Jul 16, 2026
tonydwb
left a comment
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Comment
Scope
- 4 files (tools/mcp_oauth.py, tools/mcp_oauth_manager.py, tests/tools/test_mcp_oauth.py, scripts/release.py), +376/-11 lines
- Fixes OAuth callback port races: closures, reuse_address, TOCTOU.
Quality
- Multiple concurrent callback port openings could race; this fixes the race conditions.
- Based on prior PR context (#44872, #22161 salvage).
Looks Good
- Technical fix with multi-file impact.
Reviewed by Hermes Agent
teknium1
added a commit
that referenced
this pull request
Jul 16, 2026
…cross ports _wait_for_callback still read the legacy module-level _oauth_port, so with two concurrent OAuth flows, flow A's callback wait bound flow B's port while A's redirect URI pointed at A's port — the callback-side half of the cross-flow collision that #65622 fixed on the redirect side. _make_callback_waiter(port) closes over each flow's resolved port; both provider construction sites (build_oauth_auth and MCPOAuthManager._build_provider) now wire per-flow waiters. The legacy _wait_for_callback delegates for backwards compatibility. Direction credit to @LeonSGP43 (#34280) and the #34260 analysis.
teknium1
added a commit
that referenced
this pull request
Jul 16, 2026
…cross ports _wait_for_callback still read the legacy module-level _oauth_port, so with two concurrent OAuth flows, flow A's callback wait bound flow B's port while A's redirect URI pointed at A's port — the callback-side half of the cross-flow collision that #65622 fixed on the redirect side. _make_callback_waiter(port) closes over each flow's resolved port; both provider construction sites (build_oauth_auth and MCPOAuthManager._build_provider) now wire per-flow waiters. The legacy _wait_for_callback delegates for backwards compatibility. Direction credit to @LeonSGP43 (#34280) and the #34260 analysis.
This was referenced Jul 16, 2026
76 tasks
Gravezzz
pushed a commit
to Gravezzz/hermes-agent
that referenced
this pull request
Jul 21, 2026
…cross ports _wait_for_callback still read the legacy module-level _oauth_port, so with two concurrent OAuth flows, flow A's callback wait bound flow B's port while A's redirect URI pointed at A's port — the callback-side half of the cross-flow collision that NousResearch#65622 fixed on the redirect side. _make_callback_waiter(port) closes over each flow's resolved port; both provider construction sites (build_oauth_auth and MCPOAuthManager._build_provider) now wire per-flow waiters. The legacy _wait_for_callback delegates for backwards compatibility. Direction credit to @LeonSGP43 (NousResearch#34280) and the NousResearch#34260 analysis.
Contributor
|
This PR addresses the root cause of #40656 (Desktop remote gateway destabilizing self-hosted host when used concurrently with TUI/gateway). |
randlee
pushed a commit
to randlee/hermes-agent
that referenced
this pull request
Aug 11, 2026
…cross ports _wait_for_callback still read the legacy module-level _oauth_port, so with two concurrent OAuth flows, flow A's callback wait bound flow B's port while A's redirect URI pointed at A's port — the callback-side half of the cross-flow collision that NousResearch#65622 fixed on the redirect side. _make_callback_waiter(port) closes over each flow's resolved port; both provider construction sites (build_oauth_auth and MCPOAuthManager._build_provider) now wire per-flow waiters. The legacy _wait_for_callback delegates for backwards compatibility. Direction credit to @LeonSGP43 (NousResearch#34280) and the NousResearch#34260 analysis.
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
5 participants
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.
Infographic
Summary
Closes three races in the MCP OAuth callback flow: cross-server port pollution on concurrent flows, TIME_WAIT lockout on sequential flows, and the select-to-bind TOCTOU on the callback port.
Salvage of #44872 by @Code-suphub (cherry-picked, authorship preserved) with the TOCTOU fix from #22161 by @amathxbt folded in. The same closure-factory direction was independently submitted in #5345 by @caseyg (earliest), #44607 (@LeonSGP43), and #44685 (@HrushiYadav); the
allow_reuse_addresshalf also in #44611 (@LeonSGP43). Credit to all six contributors.Fixes #44588, fixes #44590, fixes #5344.
Changes
tools/mcp_oauth.py:_make_redirect_handler(port)closure factory replaces the module-level handler reading the shared_oauth_portglobal — concurrent OAuth flows no longer print each other's ports (OAuth redirect_handler/callback_handler use shared globals, causing cross-server state pollution #44588). The [Bug]: Headless MCP OAuth blocks gateway startup with stale cached tokens #57836 non-interactive fail-fast guard is preserved inside the closure._reserve_callback_port(): ephemeral callback ports keep their socket bound (bounded FIFO pool) from selection until_wait_for_callbackadopts it viabind_and_activate=False— no process can steal the port in between (fix: TOCTOU port race between _find_free_port() and HTTPServer bind #22161).allow_reuse_addressset BEFORE binding (the original fix(mcp): use per-provider closures and allow_reuse_address for OAuth #44872 set it after the constructor had bound, a no-op) so TIME_WAIT from a previous flow can't block the next (OAuth: ephemeral HTTPServer per flow causes OSError Address already in use #44590).tools/mcp_oauth_manager.py: consume the closure factory.tests/tools/test_mcp_oauth.py:TestCallbackPortReservation(steal-attempt rejected, pool bounded, pinned port untouched, live adopt + callback round-trip); [Bug]: Headless MCP OAuth blocks gateway startup with stale cached tokens #57836 guard tests updated to the factory API.Validation (live E2E, real sockets)
test_mcp_oauth.py+test_mcp_oauth_manager.py