Skip to content

fix(login): use isolated HTTP mux, eagerly bind port, and return errors from oauth2login. Fixes #312#317

Open
rootp1 wants to merge 8 commits intomicrocks:masterfrom
rootp1:fix/issue-312
Open

fix(login): use isolated HTTP mux, eagerly bind port, and return errors from oauth2login. Fixes #312#317
rootp1 wants to merge 8 commits intomicrocks:masterfrom
rootp1:fix/issue-312

Conversation

@rootp1
Copy link
Copy Markdown

@rootp1 rootp1 commented May 7, 2026

Description

  • Replaced http.HandleFunc (process-global http.DefaultServeMux) with an isolated http.NewServeMux per oauth2login call, eliminating handler pollution across repeated invocations and race conditions on the shared global mux
  • Changed oauth2login to eagerly bind the callback port via net.Listen before opening the browser, so port hijack attempts are detected immediately with a clear error message instead of crashing inside a goroutine after the OAuth flow has already started
  • Reordered server startup: the callback HTTP server now starts before ssoAuthFlow opens the browser, guaranteeing the server is ready when the browser redirects back
  • Changed oauth2login signature from (string, string) to (string, string, error), replacing log.Fatal/log.Fatalf calls with returned errors so the caller controls error handling and the function is testable
  • Added srv.Shutdown on the error path before returning, ensuring the callback server port is released even when the OAuth flow fails
  • Removed the time.Sleep(1 * time.Second) hack that previously tried to paper over the race between server startup and browser redirect

Reproduction results

Before fix - Port hijack:
A rogue process listening on port 58085 before login --sso causes log.Fatalf inside a goroutine after the browser is already open. The user sees a confusing crash and the OAuth callback is silently routed to the attacker.

After fix - Port hijack detected early:
oauth2login returns error: "failed to bind callback port localhost:58085: listen tcp 127.0.0.1:58085: bind: address already in use (is another process using it?)" immediately - no browser opened, no OAuth flow started.

Before fix - Global mux pollution:
http.HandleFunc("/auth/callback", ...) registers the route on http.DefaultServeMux permanently. A second login --sso call overwrites the first handler. In the same binary, this route interferes with any other HTTP serving.

After fix - Isolated mux:
Each oauth2login call creates its own http.NewServeMux. /auth/callback is never registered on http.DefaultServeMux. Concurrent and sequential calls are fully independent.

Before fix - Server port leaked on error:
When the callback receives an error (e.g. access_denied), log.Fatal is called without shutting down the server, leaving the port bound until the process exits.

After fix - Clean shutdown on error:
srv.Shutdown is called before returning the error, releasing the port immediately.

Test suite (5 tests, all passing):

Test Verifies
TestPortHijackDetection Rogue listener → clear early abort with descriptive error
TestGlobalMuxNotPolluted /auth/callback not in DefaultServeMux after call
TestEagerPortBindBeforeBrowser Port bound before browser would open
TestServerShutdownOnErrorPath Port released after error callback
TestConcurrentIsolatedMuxCalls Two concurrent calls bind independent ports

Related issue(s)

Fixes race condition, global mux pollution, and port hijack vulnerability in OAuth2 SSO callback server as described in issue
Fixes #312 .

BREAKING CHANGE

oauth2login now returns (string, string, error) instead of (string, string). Any callers of this function must handle the returned error.

@rootp1
Copy link
Copy Markdown
Author

rootp1 commented May 7, 2026

oauth2login now returns (string, string, error) instead of (string, string). Any callers of this function must handle the returned error.

Hey @Harsh4902, @lbroudoux @yada,
It would be great to know your views on this change

@rootp1 rootp1 changed the title fix(import-url): validate artifact URLs to prevent SSRF. Fixes #312 fix(login): use isolated HTTP mux, eagerly bind port, and return errors from oauth2login. Fixes #312 May 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Auth2 SSO Callback Server Is a Global Singleton, Race Condition & Port Hijack

1 participant