🤖 fix: refactor & harden OAuth desktop flows#2248
Merged
ibetitsmike merged 22 commits intomainfrom Feb 15, 2026
Merged
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ef2cdc075
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
ibetitsmike
force-pushed
the
auth-7ete
branch
from
2ef2cdc to
669c59d
Compare
Contributor
Author
|
@codex review Addressed the review feedback:
Please take another look. |
|
Codex Review: Didn't find any major issues. 👍 ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
ibetitsmike
changed the title
ibetitsmike
changed the title
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
added 22 commits
February 15, 2026 19:54
Create 3 new shared modules for the duplicated OAuth infrastructure found across 5 service files (codexOauthService, copilotOauthService, muxGatewayOauthService, muxGovernorOauthService, mcpOauthService): - oauthUtils.ts: createDeferred, closeServer, escapeHtml, renderOAuthCallbackHtml — verbatim-duplicated utilities - oauthLoopbackServer.ts: startLoopbackServer — shared local HTTP server pattern for receiving OAuth code redirects - oauthFlowManager.ts: OAuthFlowManager class — shared desktop flow lifecycle (register/waitFor/cancel/finish/shutdownAll) No existing files are modified. Follow-up work will wire each service to use these shared modules.
Replace the inline deferred-promise pattern with the shared createDeferred<T>() utility extracted into oauthUtils.
Replace duplicated OAuth infrastructure with the new shared utilities: - Import createDeferred, renderOAuthCallbackHtml from oauthUtils - Replace http.createServer block with startLoopbackServer - Replace manual desktopFlows Map + lifecycle methods with OAuthFlowManager - Remove local closeServer, createDeferred, escapeHtml helpers - Remove DesktopFlow interface and handleDesktopCallback method - Remove finishDesktopFlow (delegated to OAuthFlowManager.finish) - Simplify dispose() to use shutdownAll() - Preserve Gateway-specific branding via renderHtml + extraHead Public API unchanged. File reduced from 443 to 239 lines (-46%).
Replace duplicated OAuth infrastructure with the new shared modules: - Import createDeferred, renderOAuthCallbackHtml from oauthUtils - Replace manual http.createServer with startLoopbackServer - Replace desktopFlows Map + lifecycle methods with OAuthFlowManager - Remove local closeServer, escapeHtml, createDeferred functions - Remove DesktopFlow interface, handleDesktopCallback, finishDesktopFlow Governor-specific behavior preserved: - Dynamic governorOrigin from user input for authorize/exchange URLs - Custom 'Enrollment complete/failed' branding via renderHtml option - policyService.refreshNow() callback after successful enrollment - ServerFlow pattern (startServerFlow, handleServerCallbackAndExchange) left unchanged as it doesn't use loopback All public method signatures remain identical. Reduces file from 484 to 293 lines (-40%).
Replace local duplicated helpers with imports from @/node/utils/oauthUtils: - createDeferred (~7 lines) - closeServer (~4 lines) - escapeHtml (~8 lines) - Inline HTML template in handleDesktopCallback → renderOAuthCallbackHtml MCP's unique patterns (DesktopFlow lifecycle, two-phase callback handling, state-mismatch flow termination, telemetry) are preserved unchanged. Net reduction: 26 lines (1492 → 1466).
- Replace local createDeferred, closeServer, escapeHtml, isLoopbackAddress with imports from shared oauthUtils/oauthLoopbackServer - Replace DesktopFlow interface + Map with OAuthFlowManager - Replace manual http.createServer with startLoopbackServer - Remove handleDesktopCallback (logic moved to background task in startDesktopFlow) - Remove finishDesktopFlow (delegated to OAuthFlowManager.finish) - Simplify dispose() with shutdownAll() - Keep DeviceFlow and all device flow methods unchanged 931 → 749 lines (-182)
Add 3 test files covering the shared OAuth utilities: - oauthUtils.test.ts (21 tests): createDeferred, closeServer, escapeHtml, renderOAuthCallbackHtml (XSS escaping, auto-close, extraHead) - oauthLoopbackServer.test.ts (9 tests): server start, valid callback, state mismatch, error params, missing code, 404, cancel, custom path/renderer - oauthFlowManager.test.ts (13 tests): register/get/has, waitFor with resolve/timeout/missing, cancel, finish (idempotent, null server, timeout cleanup), shutdownAll Uses real HTTP requests for loopback server tests and mock http.Server for flow manager tests.
…flow timeouts P1a (XSS regression): renderOAuthCallbackHtml now escapes the message unconditionally for both success and failure paths. Previously, failure messages (which can contain provider error_description values) were rendered as raw HTML, allowing injection in the local callback page. Also remove the now-redundant escapeHtml() wrapper in mcpOauthService's handleDesktopCallback — renderOAuthCallbackHtml handles escaping itself. P1b (leaked flows): Gateway, Governor, and Codex desktop flow registrations now set a server-side timeout at register time. Previously, the timeout only existed inside waitFor(), so flows started without a corresponding waitFor call (e.g. MuxGatewaySessionExpiredDialog) would leak the loopback server and map entry indefinitely.
Preserve existing OAuth flow test behavior while resolving lint/type failures in the new gateway and governor test files.
Bump pending-detection race timeout from 25ms to 100ms in new MuxGateway/MuxGovernor OAuth service tests. Document that RenderOAuthCallbackHtmlOptions.extraHead is injected unescaped and must be trusted static HTML.
… cancel Three fixes: 1. ProvidersSection: change useEffect dep from [api] to [] with apiRef so the cleanup only fires on true unmount, not on api identity changes (e.g. IPC reconnection). This was the root cause of the first flow being cancelled before it could succeed. 2. ProvidersSection: pre-emptively cancel previous copilot flow before starting a new one (matching the existing Codex pattern). 3. copilotOauthService: skip cancelDeviceFlow log+resolve when the flow already completed, avoiding misleading 'cancelled' logs after success.
- ProvidersSection: cancel Copilot device flow when start result returns for a stale attempt. - OAuthFlowManager: on duplicate register(flowId), best-effort cleanup the previous active entry (timeout, deferred, server) before storing the new one. - CodexOauthService: only log cancel when the desktop flow exists to avoid misleading logs.
- OAuthFlowManager: regression test for duplicate flowId register cleanup. - oauthLoopbackServer: regression test asserting validateLoopback rejects non-loopback clients with 403.
ibetitsmike
force-pushed
the
auth-7ete
branch
from
193addc to
ebf95f1
Compare
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
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
1 participant
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.
Summary
Extracts duplicated OAuth infrastructure (loopback server, flow lifecycle, cancellation, HTML rendering) from 5 service files into shared, tested utilities. Fixes several real-world edge cases found during testing.
Why
Each OAuth service (Gateway, Governor, Codex, Copilot, MCP) had its own copy of loopback server setup, callback HTML, state validation, and flow cancellation — ~775 lines of near-identical code with inconsistent error handling. None of it had tests.
What changed
Shared utilities (
src/node/utils/oauth*.ts):oauthLoopbackServer— configurable localhost callback server (port, host, state validation, deferred responses)oauthFlowManager— flow registration, timeout, cancellation, completed-flow TTL for late waitersoauthUtils—createDeferred,closeServer,escapeHtml, callback HTML renderingService simplification: Gateway, Governor, and Codex desktop flows now delegate to the shared utilities. MCP partially migrated (uses helpers but keeps its own server due to
@ai-sdk/mcpintegration).Bug fixes:
statereturns 400 without killing the flow (keeps listening for the real callback)useEffectdep fix)Line count
Production code is net negative (−161). The entire +1,344 increase is test coverage that didn't exist before.
Generated with
mux• Model:anthropic:claude-opus-4-6• Thinking:xhigh• Cost:$12.84