hermes - 💡(How to fix) Fix MCP OAuth callback: module-level port global causes port collisions and structural weaknesses vs upstream [1 pull requests]

Summary

The MCP OAuth callback flow in tools/mcp_oauth.py has a cluster of related issues rooted in the module-level _oauth_port global and the single-request HTTP server design. The codebase already documents the port collision as issue #5344 but marks it as out of scope — filing this to track the full set of problems together.

Core issue: _oauth_port module-level global

_oauth_port (mcp_oauth.py:94) is a plain module-level global. This causes two failure modes:

1. TOCTOU race between port discovery and bind

_find_free_port() binds to port 0, gets the OS-assigned port, then closes the socket (releasing the port). The port is stored in _oauth_port and baked into client_metadata.redirect_uris. Much later, when the OAuth flow fires, _wait_for_callback() tries to HTTPServer(("127.0.0.1", _oauth_port), ...) — but the port may have been grabbed by another process in the interim.

When this happens, OSError is raised at line 484, which raises OAuthNonInteractiveError before the paste fallback thread starts — so the user gets no recovery path at all.

2. Concurrent flow port collision

register_mcp_servers connects all servers in parallel via asyncio.gather. Multiple OAuth servers all call _configure_callback_port, each overwriting the same _oauth_port global. The last writer wins; earlier flows get a redirect to a port with a different server's callback listener (or no listener at all).

Already documented as the root cause of #5344 in the code comment at line 652-654.

Secondary issues in the callback handler

Single-request HTTP server

_wait_for_callback uses server.handle_request() which processes one HTTP request then exits. Any non-callback request (e.g. /favicon.ico, browser preflight, stray connection) consumes this slot, and the actual OAuth callback is never served.

Upstream (Claude Code) uses createServer() — a persistent HTTP server that handles unlimited requests.

No path validation

The do_GET handler at line 367 processes any GET request regardless of path. A request to /favicon.ico is treated identically to /callback?code=.... Combined with the single-request server, this means a non-callback request silently consumes the only handler slot.

Upstream checks parsedUrl.pathname === '/callback' before processing.

Paste reader is one-shot

_paste_callback_reader reads one line from stdin (line 554), then the thread exits. If the user pastes an invalid URL or makes a typo, there's no retry — they're stuck waiting for the HTTP callback or the 300-second timeout.

Upstream uses a callback-based design where the user can paste multiple times until a valid URL is accepted.

No OAuth state validation in callback

Neither the HTTP handler nor the paste reader validate the OAuth state parameter — it's passed through blindly to the SDK. The SDK may or may not validate it downstream.

Upstream validates state !== oauthState in both the HTTP handler and the paste handler, rejecting CSRF mismatches immediately with a clear error.

Suggested fix direction

The upstream approach (Claude Code src/services/mcp/auth.ts:959-1196) avoids all of these by:

1. Binding the server first — server.listen(port, '127.0.0.1', callback) — then starting the SDK auth flow only after the server is confirmed listening. No TOCTOU gap.
2. Per-flow port — port is local to the auth function, not a module global.
3. Persistent server with path checking — createServer handles multiple requests, only processes pathname === '/callback'.
4. Callback-based paste — invalid pastes are ignored, user can retry.

Files involved

• tools/mcp_oauth.py — _oauth_port, _find_free_port, _wait_for_callback, _paste_callback_reader, _make_callback_handler
• tools/mcp_oauth_manager.py — _build_provider calls _configure_callback_port
• tools/mcp_tool.py — register_mcp_servers runs parallel asyncio.gather over OAuth flows