isAllowedRedirectURI in internal/handlers/github_oauth.go trusts every localhost/127.0.0.1 origin unconditionally in production, unlike its own already-fixed preview-wildcard branch

[Jagadeeshftw]

Description

isAllowedRedirectURI gates OAuth redirect_uri values used by LoginStart and CallbackUnified (three call sites: lines ~159, ~318, ~336 of internal/handlers/github_oauth.go) to prevent open-redirect abuse of the login flow:

func isAllowedRedirectURI(redirectURI string, cfg config.Config) bool {
	parsedURL, err := url.Parse(redirectURI)
	...
	origin := parsedURL.Scheme + "://" + parsedURL.Host

	// Always allow localhost origins for development
	if strings.HasPrefix(origin, "http://localhost:") ||
		strings.HasPrefix(origin, "http://127.0.0.1:") ||
		strings.HasPrefix(origin, "https://localhost:") ||
		strings.HasPrefix(origin, "https://127.0.0.1:") {
		return true
	}

	// Allow Vercel and 0xo preview deployments (*.vercel.app, *.0xo.in) only if CORSAllowPreview is enabled
	if cfg.CORSAllowPreview {
		if strings.HasSuffix(origin, ".vercel.app") || strings.HasSuffix(origin, ".0xo.in") {
			return true
		}
	}
	...

The comment says "for development," but the localhost/127.0.0.1 branch is unconditional — it does not check cfg.IsDev(). This repo's own sibling implementation, CORSOriginPolicy.Allows in internal/api/cors.go, gets this right for the exact same concern:

return CORSOriginPolicy{
    allowDevLocalhost:     cfg.IsDev(),
    allowPreviewWildcards: cfg.CORSAllowPreview,
    ...
}
...
if p.allowDevLocalhost && isLocalhostOrigin(origin) {
    return true
}

isAllowedRedirectURI's preview-wildcard branch is correctly gated behind cfg.CORSAllowPreview (this matches the fix already applied for the previously-flagged unconditional .vercel.app/.0xo.in trust in this same function), but the localhost branch was missed and still trusts http://localhost:<any-port> / http://127.0.0.1:<any-port> as a valid post-login OAuth redirect target even when APP_ENV is a production/staging environment.

Requirements

• The localhost/127.0.0.1 branch in isAllowedRedirectURI must only return true when cfg.IsDev() is true, mirroring internal/api/cors.go's allowDevLocalhost gate.
• Non-dev environments must reject redirect_uri/redirect_uri_from_state values on localhost/127.0.0.1 unless they are also covered by CORSOrigins or FrontendBaseURL.
• Existing dev-environment behavior (APP_ENV=dev) must be unchanged.

Suggested execution

1. In internal/handlers/github_oauth.go, wrap the localhost/127.0.0.1 prefix check in if cfg.IsDev() { ... }, matching the existing pattern already used for cfg.CORSAllowPreview.
2. Consider factoring the localhost-matching logic out into a shared helper reused by both internal/api/cors.go and internal/handlers/github_oauth.go so the two allowlists cannot drift again.
3. Add a test in internal/handlers/github_oauth_test.go (or the existing CSRF test file) asserting isAllowedRedirectURI("http://localhost:5173/cb", cfg) returns false when cfg.Env is a non-dev value and true when cfg.Env == "dev".

Acceptance criteria

• isAllowedRedirectURI rejects localhost/127.0.0.1 redirect URIs outside of cfg.IsDev().
• A regression test locks in the non-dev rejection and the dev-mode acceptance.
• LoginStart and CallbackUnified behavior in dev is unchanged.

Security notes

In a production deployment, a caller-supplied redirect_uri=http://localhost:<port>/... currently passes this allowlist unconditionally. While this isn't a classic remote open-redirect (the victim's own browser must have something listening on that local port), it does mean the production OAuth flow will happily redirect a logged-in victim's browser to an attacker-chosen localhost port, which can be combined with a locally-running malicious or vulnerable service on the victim's machine to exfiltrate the OAuth success payload/query parameters. This should be closed the same way the .vercel.app/.0xo.in branch already was.

Guidelines

• Minimum 95% test coverage
• Timeframe: 96 hours