feat(oauth): add stdio OAuth 2.1 login core library (1/4)#2704
Merged
Conversation
Introduce internal/oauth, a self-contained library that performs the user-facing GitHub OAuth login the stdio server uses to obtain a token without a pre-provisioned PAT. It is independent of MCP: client concerns (elicitation) sit behind the Prompter interface so the flows are testable without a live session. What it provides: - Authorization-code + PKCE flow with a local loopback callback server, state/CSRF validation, and XSS-safe result pages. - Device-authorization flow as a fallback (headless, containers). - A Manager that selects the most secure available channel (browser auto-open -> URL elicitation -> last-resort user action), runs a single flow at a time, and exposes a refreshing token source. Both GitHub OAuth Apps and GitHub Apps are supported without special casing: the token is modeled as an x/oauth2 refreshing TokenSource, so expiring GitHub App user tokens are renewed transparently (the gap that made a stored-token approach silently die after ~8h). When a client lacks secure URL elicitation and the flow falls back to a tool-response message, the message advises the user that their agent/CLI/ IDE does not appear to support URL elicitation and suggests requesting it for improved security. Tests exercise real protocol behavior against an httptest GitHub stand-in: PKCE challenge/verifier, GitHub App refresh-on-expiry, device polling, URL elicitation, declined prompts, the last-resort action with advisory, and single-flight concurrency. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new internal/oauth core library that implements GitHub OAuth (PKCE + loopback callback, with device-flow fallback) for future stdio-mode authentication, including a Manager that orchestrates flow selection and exposes a refreshing oauth2.TokenSource. This PR is intentionally not wired into the server yet.
Changes:
- Introduces the OAuth core library (
internal/oauth) with PKCE + device-flow support and a user-prompt abstraction (Prompter). - Adds embedded HTML templates for local callback success/error pages.
- Adds unit tests using an
httptestfake GitHub OAuth server and prompter fakes; addsgolang.org/x/oauth2as a direct dependency.
Show a summary per file
| File | Description |
|---|---|
| internal/oauth/oauth.go | Defines OAuth config, GitHub endpoint construction, and host normalization helpers. |
| internal/oauth/oauth_test.go | Tests host normalization, endpoint construction, and random state generation. |
| internal/oauth/prompter.go | Introduces Prompter interface and prompt data model for secure out-of-band UX. |
| internal/oauth/manager.go | Adds Manager that single-flights auth flows and provides refreshed access tokens. |
| internal/oauth/manager_test.go | Behavioral tests for browser PKCE, URL elicitation, declines, device flow, and single-flight. |
| internal/oauth/flow.go | Implements flow selection plus PKCE/device flow preparation and user-action fallback messaging. |
| internal/oauth/env.go | Adds browser-opening and Docker-detection utilities used by the manager. |
| internal/oauth/callback.go | Implements the local loopback callback server with embedded templates and state validation. |
| internal/oauth/callback_test.go | Tests callback handler outcomes and loopback binding for random ports. |
| internal/oauth/testutil_test.go | Test utilities: fake GitHub OAuth endpoints, fake prompter, browser simulation, polling helper. |
| internal/oauth/templates/success.html | HTML template shown after successful authorization redirect. |
| internal/oauth/templates/error.html | HTML template shown after failed authorization redirect (auto-escaped error). |
| go.mod | Adds golang.org/x/oauth2 as a direct dependency. |
Copilot's findings
- Files reviewed: 13/13 changed files
- Comments generated: 4
Comment on lines
+32
to
+35
| cmd.Stdout = io.Discard | ||
| cmd.Stderr = io.Discard | ||
| return cmd.Start() | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
Good catch — fixed in f707dba. openBrowser now reaps the launcher asynchronously (go func() { _ = cmd.Wait() }()) so it can't linger as a zombie for the life of the server. The launcher (xdg-open/open/rundll32) exits as soon as it hands off to the browser, so this is cheap.
Comment on lines
+37
to
+55
| // listenCallback binds the local callback listener. | ||
| // | ||
| // A random port (port == 0) binds to 127.0.0.1 only: the redirect target is | ||
| // loopback and never reachable off-host. A fixed port binds to all interfaces | ||
| // because Docker's published-port DNAT delivers traffic to the container's eth0 | ||
| // rather than to loopback; exposure is still constrained by the host-side | ||
| // publish (e.g. -p 127.0.0.1:8085:8085). | ||
| func listenCallback(port int) (net.Listener, error) { | ||
| host := "127.0.0.1" | ||
| if port > 0 { | ||
| host = "0.0.0.0" | ||
| } | ||
| addr := fmt.Sprintf("%s:%d", host, port) | ||
| listener, err := net.Listen("tcp", addr) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("starting callback listener on %s: %w", addr, err) | ||
| } | ||
| return listener, nil | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
Fixed in f707dba. listenCallback now takes an explicit bindAll flag and binds to 0.0.0.0 only when running inside a container; beginPKCE passes m.inDocker(). Native runs — even with a fixed callback port — stay on 127.0.0.1. (PKCE in a container only happens with a fixed port, since a random port there falls back to device flow, so this is exactly the publish-via-eth0 case that needs all-interfaces.) Call site and test updated accordingly.
Comment on lines
+57
to
+60
| listener, err := listenCallback(m.config.CallbackPort) | ||
| if err != nil { | ||
| return nil, err | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
Done — beginPKCE now calls listenCallback(m.config.CallbackPort, m.inDocker()), so bindAll is driven by container detection as you suggested.
Comment on lines
+73
to
+76
| func TestListenCallbackRandomPortIsLoopback(t *testing.T) { | ||
| listener, err := listenCallback(0) | ||
| require.NoError(t, err) | ||
| defer listener.Close() |
Collaborator
Author
There was a problem hiding this comment.
Updated — the test helper now calls listenCallback(0, false), and I added TestListenCallbackBindAllForContainer to assert the bindAll path binds all interfaces.
Address code review: - openBrowser: reap the launcher process asynchronously so it does not linger as a zombie for the lifetime of the server. - listenCallback: take an explicit bindAll flag and bind to all interfaces only inside a container (where the published port arrives via eth0). A native run, even with a fixed callback port, now stays on 127.0.0.1 instead of 0.0.0.0. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
A fixed --oauth-callback-port is registered with the OAuth app and chosen deliberately, so a bind failure means another process holds the port and could intercept the authorization redirect. Treat that as fatal instead of silently downgrading to the device flow, which would mask the conflict. Also warn, when binding the callback inside a container, that the listener is on all interfaces and should be published to loopback only so the authorization code is not exposed on the container network. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ow when headless Addresses pre-merge review of the OAuth stdio core: - Log a one-time warning when token refresh fails instead of silently returning an empty access token, so a forced re-login isn't a surprise. - Bound each background token refresh with a 30s HTTP client timeout so a stalled GitHub token endpoint can't block tool calls indefinitely. - On a headless host (no display server) with a random callback port, fall back to the device-code flow — the only channel reachable from a browser on another machine — instead of dead-ending on an unreachable localhost redirect. A generic browser-open failure still offers the manual URL. - Mark the callback bind failure with a sentinel so the fixed-port-busy fatal path can't misreport an unrelated error as a port conflict. - Export NormalizeHost so callers can recognize the default github.com host (consumed by the build-time baked-in credential guard). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
4 tasks
omgitsads
previously approved these changes
Jun 26, 2026
* feat(oauth): wire stdio OAuth 2.1 login into the server Connect the internal/oauth core library to the stdio MCP server so users can authenticate with an OAuth App or GitHub App client ID instead of a static personal access token. - BearerAuthTransport gains a TokenProvider that is consulted per request, letting the lazily-acquired, auto-refreshing OAuth token take effect without rebuilding the client. - createGitHubClients uses BearerAuthTransport (and skips go-github's WithAuthToken, which would pin a static token) when a TokenProvider is set. - RunStdioServer starts without a token and installs receiving middleware that runs the authorization flow on the first tool call, surfacing the auth URL or device code via elicitation (or a tool result as a fallback). - Tool filtering uses the requested OAuth scopes; the default supported set hides nothing, while a narrower --oauth-scopes both narrows the grant and filters tools accordingly. - A sessionPrompter adapts the MCP server session to oauth.Prompter, keeping the authorization URL off the model's context. - New stdio flags: --oauth-client-id/-client-secret/-scopes/-callback-port. This is stdio-only and deliberately does not touch MCP-HTTP auth. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor(oauth): address review — omit empty bearer header, guard token/oauth - BearerAuthTransport omits the Authorization header entirely when the token is empty (pre-authorization) rather than sending an empty "Bearer " value. - RunStdioServer rejects the ambiguous combination of a static Token and an OAuthManager up front, enforcing the documented mutual exclusivity. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(oauth): clarify SupportedScopes is the stdio default and tool filter Document that stdio OAuth login requests these scopes by default and then filters the exposed tools to the scopes actually granted, so a tool whose required scope is absent from this list is hidden under default OAuth even though a PAT carrying that scope would expose it. Keep the list in sync with tool scope requirements when scopes change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Distinguish undeliverable auth prompts from user declines An elicitation prompt that the client cannot deliver (a transport or protocol failure) was treated the same as a user actively declining: any display error cancelled the flow. That conflated a system failure with a deliberate "no", so a client that advertised URL elicitation but failed to deliver it would hard-fail the login instead of degrading. Add an ErrPromptUnavailable sentinel alongside ErrPromptDeclined and have the MCP adapter return it when Elicit fails at the transport level. The manager now falls back to the manual user-action channel on an undeliverable prompt (keeping the background flow alive so the user can still authorize out of band), while a genuine decline still aborts. A context-cancelled prompt is checked first so an ending flow is never misread as a transport failure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * build(oauth): bake in default OAuth credentials for official releases (3/4) (#2711) * build(oauth): bake in default OAuth credentials via build-time ldflags Inject the public OAuth client credentials (stored as the OAUTH_CLIENT_ID and OAUTH_CLIENT_SECRET repo secrets) at build time via -ldflags so official binaries and images ship a working default app for zero-config login. Security relies on PKCE, not on the secret. Local/dev builds leave the values empty and continue to require an explicit token or --oauth-client-id. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(oauth): recognize github.com host aliases for the baked-in client Match the default host via oauth.NormalizeHost instead of only an empty host string, so an explicit GITHUB_HOST=github.com (or api.github.com) still counts as the default and keeps zero-config baked-in login working. GHES and ghe.com users continue to bring their own --oauth-client-id. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(oauth): document stdio OAuth login; make PAT optional in install config (#2717) Add a dedicated Local Server OAuth Login guide (docs/oauth-login.md) covering the PKCE/device flows, display channels and the URL-elicitation security advisory, scope-based tool filtering, the fixed-port Docker recipe and its loopback/port-safety behavior, bringing your own OAuth or GitHub App, and the GitHub Enterprise Server / ghe.com requirement to register an app on that host (custom --gh-host directs login at that instance's authorization server). Reflect that the local server now logs in with OAuth by default on github.com: - README: make the stdio Docker install badges OAuth-first (fixed callback port 8085 published to loopback), drop the PAT prompt, and reframe the PAT as an optional alternative with a pointer to the new guide. - server.json: make GITHUB_PERSONAL_ACCESS_TOKEN optional and publish the OAuth callback port so the registry default works without a token. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
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.
OAuth for stdio — PR 1 of 4
This is the first PR in a stack that replaces #1836 with smaller, focused changes. It adds only the OAuth core library (
internal/oauth); nothing is wired into the server yet, so this PR is inert on its own and safe to review in isolation.Stack
internal/oauthcore library ← this PRWhat this adds
A self-contained library that performs the user-facing GitHub OAuth login the stdio server will use to obtain a token without a pre-provisioned PAT. It depends only on
golang.org/x/oauth2and the standard library; MCP concerns (elicitation) sit behind a smallPrompterinterface so the flows are fully testable without a live client.state/CSRF validation,ReadHeaderTimeout, and XSS-safe HTML result pages.Managerthat picks the most secure available channel — browser auto-open → URL elicitation → last-resort tool-response action — runs a single flow at a time (single-flight), and exposes a token.Why a refreshing token source (the key correctness point)
The token is modeled as an
x/oauth2refreshingTokenSource, so both OAuth Apps and GitHub Apps work without special-casing. GitHub App user tokens expire (~8h) and carry a refresh token; this renews them transparently. (A stored-token approach — as in the original PR — silently dies when the token expires.)URL-elicitation security advisory
When a client lacks secure URL elicitation and we fall back to returning the auth URL/device code as a tool response, the message also advises the user that their agent/CLI/IDE does not appear to support URL elicitation and suggests requesting it for improved security — keeping the auth URL out of model context where possible.
Tests
Behavioral tests run against an
httptestGitHub stand-in and assert real protocol behavior rather than mirroring the implementation: PKCES256challenge/verifier round-trip, GitHub App refresh-on-expiry, device polling (incl.authorization_pending), URL elicitation, declined prompts, the last-resort action + advisory, and single-flight concurrency.go test -race,golangci-lint, and the license check all pass.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com
Update — pre-merge review fixes (commit 64afe52)
Rubber-duck review of the full stack surfaced a few hardening items, all in this core lib:
AccessTokenlogs a one-time warning when theTokenSourcerefresh fails instead of silently returning an empty token (the forced re-login was otherwise a surprise).TestAuthenticateHeadlessPrefersDeviceFlow.NormalizeHostso the build-time credential guard (3/4) can recognize the default github.com host.