Refreshable tokens: build-gated debug overrides for testing token expiry by babakks · Pull Request #14752 · cli/cli · GitHub
Skip to content

Refreshable tokens: build-gated debug overrides for testing token expiry - #14752

Merged
babakks merged 8 commits into
trunkfrom
babakks/refresh-token-test-env
Oct 21, 2026
Merged

babakks merged 8 commits into
trunkfrom
babakks/refresh-token-test-env

Conversation

@babakks

@babakks babakks commented Oct 19, 2026 •

Copy link
Copy Markdown
Member

Description

This PR sits on top of #14745 in the refreshable-token stack. Testing refresh behavior by hand is awkward because GitHub issues expiring access tokens with a lifetime of 8 hours and refresh tokens with a lifetime of 6 months, so there is no practical way to wait for a real expiry. Until now the stack relied on ad hoc os.Getenv hacks (GH_AT_EXPIRES_IN, GH_RT_EXPIRES_IN, and GH_DEBUG_REFRESH_TOKEN) that were compiled into every build, including release binaries.

This replaces those hacks with a small internal/debug package. debug.Config() returns a debug.DebugConfig value whose fields are Option values, one per override, read from the environment only when gh is built with the ghdebug build tag. An unset override is None, so an explicit zero or false is still distinguishable from no override. The tag is prefixed with gh so it cannot collide with a generic debug tag that a dependency or another tool might use. In a regular build the package compiles to a stub that always returns an empty DebugConfig with every override None, so the overrides do not exist in released binaries and cannot be switched on by setting an environment variable. The parsing itself lives in an unexported, untagged configFromEnv, so the regular test run covers it on every OS; only the ghdebug implementation of Config calls it, and the linker drops it from regular builds.

The overrides, all prefixed GH_INTERNAL_ to make it clear they are not a supported interface:

  • GH_INTERNAL_AT_EXPIRES_IN: access token lifetime in seconds, measured from when the token was issued. With 1, any stored access token is already treated as expired, so the next use triggers a refresh.
  • GH_INTERNAL_RT_EXPIRES_IN: refresh token lifetime in seconds, measured the same way. When it is exceeded, gh behaves as if the server rejected the refresh token: it clears the stored credential and asks the user to run gh auth login.
  • GH_INTERNAL_DEBUG_REFRESH_TOKEN: a boolean. By default the refresh exchange is logged as head lines only under GH_DEBUG=api; set it to true to log the full refresh request and response.

An invalid value panics in a debug build, so a typo cannot silently fall back to the real lifetime.

To make getting a debug binary easy, there is a new make debug target. It does the same as make but passes the ghdebug build tag, producing bin/gh with the overrides enabled. The build script now also compares the build tags embedded in an existing bin/gh with GO_BUILDTAGS, so switching between make and make debug always rebuilds, while repeating the same one still skips an up to date binary. CI also builds ./cmd/gh with -tags=ghdebug on the Ubuntu runner, so the debug variant cannot silently stop compiling.

A debug binary identifies itself in its version output, so it is easy to tell which kind of binary you are running:

$ bin/gh version
gh version 2.100.0-180-g76df8f52a (2026-10-19) [debug build]

The two guided e2e scripts (script/refreshable-token-e2e.sh and script/refreshable-token-gitcredential-e2e.sh) now use the GH_INTERNAL_ names, check gh version for the [debug build] marker and refuse to run against a regular binary, and point at make debug. Their own settings are renamed to match: GH_BIN, GH_E2E_HOST, and GH_E2E_ALLOW_VSCODE become GH_INTERNAL_E2E_BIN, GH_INTERNAL_E2E_HOST, and GH_INTERNAL_E2E_ALLOW_VSCODE.

The main e2e script also checks more of the behavior from earlier PRs in the stack. Every gh auth token step that prints a stored expiring token now asserts that the expiry warning appears on stderr, and the steps that print a non-expiring token (such as one from GH_TOKEN) assert that it does not. There is also a new gh auth refresh step without --expiring, next to the existing one with it, and both assert that gh asked for and received an expiring refreshable token.

Trying it out

Build a debug binary and log in with an expiring token:

make debug
bin/gh auth login --expiring

Then force the access token to count as expired and make any API call with GH_DEBUG=api:

GH_INTERNAL_AT_EXPIRES_IN=1 GH_DEBUG=api bin/gh api user --jq .login

A refresh shows up in stderr as a request to the OAuth token endpoint, before the api/user request:

* Request to https://github.com/login/oauth/access_token

Running the same command without GH_INTERNAL_AT_EXPIRES_IN shows no such line, because the freshly refreshed token is still valid. bin/gh auth status also shows the new expiry time. Add GH_INTERNAL_DEBUG_REFRESH_TOKEN=true to see the full refresh exchange instead of just its head lines.

To see what happens when the refresh token itself has expired (this logs you out of the host in gh):

GH_INTERNAL_AT_EXPIRES_IN=1 GH_INTERNAL_RT_EXPIRES_IN=1 bin/gh api user

For a guided walkthrough of every scenario, run script/refreshable-token-e2e.sh or script/refreshable-token-gitcredential-e2e.sh after make debug.

How did you test this change?

I ran make and make debug and compared the two binaries. bin/gh version printed gh version 2.100.0-180-g76df8f52a (2026-10-19) for the regular build and the same line with a trailing [debug build] for the debug build, and gh --version matched in both cases. go version -m bin/gh reported -tags=ghdebug for the debug build. I also ran make debug, make, make, make debug, make debug in sequence: every switch between the two rebuilt with the right tags, and every repeat reported bin/gh as up to date. I also built regular and ghdebug binaries for linux/amd64, windows/amd64, windows/arm64, darwin/amd64, and darwin/arm64: no regular build contains any GH_INTERNAL_ string or the configFromEnv and lifetimeFromEnv symbols, while every ghdebug build contains all of them.

I ran script/refreshable-token-e2e.sh against the debug binary and all scenarios passed, including the new gh auth token warning checks and both gh auth refresh steps, with and without --expiring.

I ran script/refreshable-token-gitcredential-e2e.sh against the debug binary in an Ubuntu 25.10 container with git 2.51.0, so the git 2.46+ authtype credential capability path was exercised, and all scenarios passed.

Key points

  • The overrides are compiled out rather than hidden behind an undocumented variable, so there is no way to enable them in a release build. For the same reason the debug check is a version marker rather than something like a GH_INTERNAL_IS_DEBUG variable: a release binary has no code that reads GH_INTERNAL_* variables, so it could not respond to one without adding such code to release builds.
  • The lifetime overrides are measured from issuance, not from now. GH_INTERNAL_AT_EXPIRES_IN=3600 therefore means "pretend the token was issued with a one hour lifetime", which keeps the behavior consistent across repeated invocations.
  • The simulated refresh token expiry follows the same path as a real server rejection, including only clearing the stored credential when the refresh lock is held.
  • make debug writes to the same bin/gh as make, so the two builds replace each other rather than coexisting.

Notes for reviewers

Start with internal/debug/config.go (the DebugConfig type and the untagged configFromEnv parser) and its two build-tagged implementations (config_enabled.go and config_disabled.go, which pick what Config returns and define debug.Enabled), then pkg/cmd/version/version.go for the version marker, then the call sites in internal/config/refreshable.go (accessTokenNeedsRefresh and simulateRefreshTokenExpired) and pkg/cmd/factory/default.go (tokenRefresherHttpClientFunc). In the scripts, the debug check is require_gh, and apart from the renames the only new logic is the auth token warning and auth refresh assertions in script/refreshable-token-e2e.sh.

Authorship and follow-up

Who wrote this:

  • A human wrote it.
  • An agent wrote it under close human direction.
  • An agent wrote it independently, and no human has guided the implementation beyond the initial prompt.

Who answers review comments:

  • @babakks will read and reply directly.
  • An agent will draft replies and @babakks will read them before they are posted.
  • Nobody has explicitly committed to replying.

Copilot AI balanced review requested due to automatic review settings October 19, 2026 14:49
@babakks
babakks requested a review from a team as a code owner October 19, 2026 14:49
@babakks
babakks requested review from sergiou87 and removed request for a team October 19, 2026 14:49
@babakks
babakks removed the request for review from sergiou87 October 19, 2026 14:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Debug-only behavior needs HTTP-client logging regression tests and a tagged CI test invocation.

2 open issues
What changed in this PR

Moves refreshable-token testing overrides behind the ghdebug build tag, keeping them unavailable in regular builds.

Changes:

  • Adds optional debug configuration and a make debug target.
  • Marks debug binaries in version output.
  • Updates guided scripts and expands refresh assertions.
File Description
script/​refreshable-token-gitcredential-e2e.sh Requires debug builds and renames overrides.
script/​refreshable-token-e2e.sh Updates overrides and adds warning/refresh checks.
pkg/​cmd/​version/​version.go Marks debug builds.
pkg/​cmd/​version/​version_test.go Tests both version formats.
pkg/​cmd/​factory/​default.go Uses build-gated refresh logging configuration.
Makefile Adds debug build target.
internal/​debug/​config.go Defines optional overrides.
internal/​debug/​config_enabled.go Parses debug-only environment settings.
internal/​debug/​config_enabled_test.go Tests parsing and invalid values.
internal/​debug/​config_disabled.go Disables overrides in regular builds.
internal/​debug/​config_disabled_test.go Tests environment isolation.
internal/​config/​refreshable.go Applies build-gated expiry overrides.

Review effort: Balanced

Comment thread internal/debug/config_enabled_test.go
Comment thread pkg/cmd/factory/default.go
@babakks
babakks added this pull request to stack #14457 October 19, 2026 15:00
@babakks
babakks requested a review from williammartin October 19, 2026 15:00

@williammartin williammartin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @babakks,

Sorry, I have to leave as my day is over. This is a slightly sloppy review starting point 😅

I feel 3/10 strongly about the testing comments. However, I feel strongly about the rebuild decision because plain make should reliably produce a regular build.

The PR description is very helpful, especially the walkthrough and reviewer notes.

Comment thread Makefile

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion

Non blocking, and I feel 3/10 strongly about this. Could we run the ghdebug variant in CI so the tagged implementation cannot silently stop compiling?

$ git show origin/pr-14752:.github/workflows/go.yml | grep 'go test'
        run: go test -race -tags=integration ./...

The regular run should stay so we also cover the disabled implementation. What do you think?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, thanks! 🙏 I took a slightly different route that makes a tagged test run unnecessary.

In 8f43bec I moved the environment parsing out of config_enabled.go into an unexported configFromEnv in the untagged config.go. That way config_test.go covers every parsing and panic case in the regular test run, on every OS. What is left behind the tag is just the choice of what Config returns:

  • ghdebug: return configFromEnv()
  • !ghdebug: return DebugConfig{}, which TestConfigIgnoresEnvironment already asserts in the regular run

Since the release Config never references configFromEnv, the linker drops it from regular builds. I checked the resulting binaries for linux/amd64, windows/amd64, windows/arm64, darwin/amd64, and darwin/arm64: every regular build has no GH_INTERNAL_ strings and no parser symbols at all (only the Config stub), while every ghdebug build has all three variable names along with configFromEnv and lifetimeFromEnv.

So the only way the debug variant can break without the regular run noticing is by failing to compile, and d5d4d81 covers that with a go build -tags=ghdebug ./cmd/gh step on the Ubuntu runner. Running the whole suite again with the tag would buy us nothing extra, and since Go caches test results per tag set it would rerun every package on every OS.

Comment thread pkg/cmd/factory/default.go
@babakks
babakks requested review from williammartin and a balanced review from Copilot October 19, 2026 15:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

Static review found no blocking defects in the build gating, refresh integration, or script changes.

No open issues 🎉

2 resolved since last review

Review effort: Balanced

@babakks
babakks force-pushed the babakks/refresh-token-test-env branch 12 times, most recently from 3423139 to d1792c3 Compare October 21, 2026 11:45
Base automatically changed from babakks/refresh-token-relative-expiry to trunk October 21, 2026 11:56
babakks and others added 8 commits October 21, 2026 12:56
Add an internal/debug package that exposes Config, which returns a
DebugConfig holding internal debugging overrides as Option values, and
Enabled, which reports whether gh was built with the ghdebug build tag.
In ghdebug builds Config reads GH_INTERNAL_AT_EXPIRES_IN and
GH_INTERNAL_RT_EXPIRES_IN as token lifetimes in seconds and
GH_INTERNAL_DEBUG_REFRESH_TOKEN as a boolean, leaves unset overrides as
None, and panics on invalid values. In regular builds every override is
always None, so the overrides cannot be enabled in released binaries.
The tag is prefixed with gh to avoid colliding with a generic debug tag.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ac178b5a-5b62-49bd-9e83-4796c7ddec46
Add "make debug", which builds bin/gh like "make" but with the ghdebug
build tag so the internal debug overrides are available. It removes
bin/gh first because script/build skips rebuilding when no source file
has changed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ac178b5a-5b62-49bd-9e83-4796c7ddec46
Append "[debug build]" to the first line of gh version and gh --version
when gh is built with the ghdebug build tag, so a debug binary is easy
to tell apart from a regular one. Regular builds are unchanged.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ac178b5a-5b62-49bd-9e83-4796c7ddec46
Replace the ad hoc os.Getenv testing hacks with the build-gated
internal/debug configuration. The access and refresh token lifetime
overrides in internal/config and the verbose refresh logging switch in
the factory now come from GH_INTERNAL_AT_EXPIRES_IN,
GH_INTERNAL_RT_EXPIRES_IN, and GH_INTERNAL_DEBUG_REFRESH_TOKEN, and have
no effect in builds without the ghdebug tag.

Update the refreshable-token e2e scripts to use the new variable names,
rename their own settings to GH_INTERNAL_E2E_BIN, GH_INTERNAL_E2E_HOST,
and GH_INTERNAL_E2E_ALLOW_VSCODE, and refuse to run unless gh version
reports a debug build, pointing at "make debug" otherwise.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ac178b5a-5b62-49bd-9e83-4796c7ddec46
Check that gh auth token prints the expiry warning on stderr whenever it
prints a stored expiring token, and that the warning is absent when no
stored expiring token is printed, such as with GH_TOKEN. Also run gh
auth refresh both with and without --expiring and assert that each
receives an expiring refreshable token.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ac178b5a-5b62-49bd-9e83-4796c7ddec46
script/build skipped rebuilding bin/gh whenever no source file was newer than the binary, regardless of build tags. A plain `make` straight after `make debug` therefore kept the debug binary.

Compare the tags recorded in the existing binary's build info with GO_BUILDTAGS and rebuild when they differ. This also lets `make debug` drop its `rm -f bin/gh` workaround.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ac178b5a-5b62-49bd-9e83-4796c7ddec46
Move the environment parsing into an unexported configFromEnv in the untagged config.go. The ghdebug implementation of Config now just calls it, while the release implementation still returns an empty DebugConfig.

This lets the regular test run cover all parsing and panic cases on every OS without the ghdebug tag. The tagged test only checks that Config reads the real environment. Release builds never reference configFromEnv, so the linker still drops the parser and the variable names from them.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ac178b5a-5b62-49bd-9e83-4796c7ddec46
The ghdebug implementation of internal/debug is not compiled by the regular test run or build, so it could silently stop compiling. Build ./cmd/gh with the tag on the Ubuntu runner to catch that.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ac178b5a-5b62-49bd-9e83-4796c7ddec46
@babakks
babakks force-pushed the babakks/refresh-token-test-env branch from d1792c3 to a1d1667 Compare October 21, 2026 11:56
@babakks
babakks merged commit ae7b971 into trunk Oct 21, 2026
11 checks passed
@babakks
babakks deleted the babakks/refresh-token-test-env branch October 21, 2026 12:06
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.

3 participants