Repository navigation
Conversation
Honor a GH_USER environment variable in ActiveToken: when set (and GH_TOKEN is not), resolve the token for that already-authenticated account from the keyring, instead of the stored active account. This lets concurrent shells and scripted/ agent invocations act as different accounts from one shared config, without gh auth switch mutating global state. Scoped to token resolution so it never leaks into the stored active user that switch/login/logout read and write. GH_TOKEN still takes precedence; an unauthenticated GH_USER yields no token rather than falling back to another account. Re: cli#12145 Signed-off-by: 1fanwang <[email protected]>
github-actions
Bot
added
external
|
Thanks for your pull request! Unfortunately, it doesn't meet the requirements for review:
Please update your PR to address the above. This PR will be automatically closed in 4 days if these requirements are not met. Full contribution requirements
|
There was a problem hiding this comment.
Pull request overview
This pull request updates GitHub CLI’s authentication token resolution so a caller can select an already-authenticated keyring account per invocation via GH_USER, without mutating the stored “active account” state in config.
Changes:
- Teach
AuthConfig.ActiveTokento honorGH_USER(when no token is provided via env/config), resolving the selected user’s keyring token instead of the stored active user. - Add unit tests covering
GH_USERselection behavior, precedence withGH_TOKEN, and non-mutation of stored active user.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/config/config.go | Adds GH_USER handling to ActiveToken to resolve a selected authenticated user’s keyring token without changing stored active account state. |
| internal/config/auth_config_test.go | Adds unit tests validating GH_USER token selection, precedence rules, and that stored active user is unaffected. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if envUser := os.Getenv("GH_USER"); envUser != "" { | ||
| if t, err := c.TokenFromKeyringForUser(hostname, envUser); err == nil { | ||
| return t, "keyring" | ||
| } | ||
| return "", "" | ||
| } |
There was a problem hiding this comment.
Good catch — fixed in fe91757. The legacy unkeyed-token fallback is now preserved, but only when GH_USER equals the stored active user, so a different requested account never receives the active user's token. Added regression tests for both the fallback and the no-leak case.
When GH_USER's per-user keyring lookup misses, fall back to the legacy unkeyed keyring slot only when GH_USER equals the stored active user, so legacy keyrings keep working without ever handing the active user's token to a different account. Adds regression tests for both the fallback and the no-leak case. Addresses review feedback on cli#13984. Signed-off-by: 1fanwang <[email protected]>
Restructure ActiveToken so GH_USER overrides the stored active account's config or keyring token (an environment GH_TOKEN/GITHUB_TOKEN still wins), resolving the selected account per host from the keyring. When GH_USER is the active user and has no keyring entry, fall through to normal resolution (config or legacy unkeyed token); when it names a different account with no token, return nothing rather than another account's token — so it never leaks across accounts or hosts. Extends coverage to 14 tests: multi-host resolution and cross-host no-leak, same-username-per-host, mixed secure/insecure storage, three-account selection, empty-string GH_USER, HasActiveToken, and the legacy-keyring fallback. Signed-off-by: 1fanwang <[email protected]>
|
Hey @1fanwang, I'm happy to let you know that I'm finally getting round to giving this feature some further report. You'll find a bunch of previously closed PRs for the same idea:
I think there's a few more I can't find. The main issue with the approach outlined here (with no alternative approach) is that while it works for There's also these that are loosely related to the same problem:
So I appreciate your trying to help out here (and reading the contribution guide re: |
Why
Multiple authenticated accounts on one host is a long-standing pain (#12145, #12885, #326, #9111, #11938). Switching means
gh auth switch, which mutates the shared active-account state, so it races across concurrent shells, scripts, and agent runs. There's no per-invocation way to pick an account.#12145 asks for a
GH_USERenv var to select an account. This is the token-resolution half of it.What
ActiveTokenhonorsGH_USER: when it's set, resolve the selected account's token instead of the stored active account, per host, without changing what any other shell sees.GH_USER=work gh pr createacts aswork.GH_TOKEN/GITHUB_TOKENstill wins;GH_USERoverrides only the stored active account's config or keyring token.GH_USERthat exists on one host doesn't leak another host's active token.GH_USERis the active user with no keyring entry, it falls through to normal resolution (config token, or a legacy unkeyed keyring token).Complementary to the
--userflag discussed in #12145: env var for interactive sessions, flag for scripted/agent callers.Testing
Unit,
internal/config(14 tests): non-active selection, stored-active untouched,GH_TOKENprecedence, unauthenticated yields no token,switchunaffected, empty-string ignored, three-account selection,HasActiveToken, legacy-keyring fallback and its no-leak case, config-token override for mixed secure/insecure storage, multi-host resolution with cross-host no-leak, and same-username-per-host. Full package passes.Manual, two real accounts on github.com:
GH_USER=<other> gh api userreturns the other account; noGH_USER(and emptyGH_USER) returns the stored active;GH_TOKENoverridesGH_USER; an unknownGH_USERreturns auth-required.Process
I know #12145 isn't
help wantedyet. This is a reference implementation to make the design concrete, not to bypass the process — happy to hold or reshape until it's labelled. Re: #12145, #12885.