fix(auth): check credentials can be saved before refreshing OAuth token

Description

A refresh is irreversible in a way most writes are not. GitLab invalidates the single-use refresh token as soon as it answers, and only then does refreshLocked try to persist the rotated one. When that write fails, the old token is spent and the new one is lost, so the session is unrecoverable.

The refreshing command does report the write failure. But every command after it fails with:

Oauth2: "invalid_grant" "The provided authorization grant is invalid, expired, revoked, ..."

which says nothing about a write. The loud error and the symptom you actually notice are different commands, which is why it reads as credentials expiring on their own, and why affected users end up re-running glab auth login daily.

Fix

config.CredentialWriteProbe reports whether credentials for a host could be persisted right now, without writing one. refreshLocked calls it immediately before the network request, so a host whose credentials cannot be saved fails while its refresh token is still usable. Recovery is then fixing the permission, not re-authenticating.

Every OAuth2 caller reaches the token endpoint through that one function, so the API client, the Docker credential helper, and the git-credential helper are all covered without changes of their own.

The probe mirrors what a refresh actually writes:

  • is_oauth2 and oauth2_expiry_date always land in the configuration file, so the directory is always checked, by way of the same writeConfigFile path a real write uses.
  • token and oauth2_refresh_token land in the keyring only in keyring mode, so the keyring is checked only for a keyring-mode host. This matters: headless Linux and CI runners have no keyring backend at all, and an unconditional check would stop those users refreshing entirely.

It reuses the existing KeyringAvailable() sentinel write/delete rather than adding a second probe, and needs no change to the config.Config interface.

The cost is one sentinel write per refresh, not per command.

Context in #8262 (closed). That issue is closed and stays closed: !3728 (merged), !3729 (merged), !3730 (merged), and !3732 (merged) delivered its documentation and observability half. This is the ordering change identified in its discussion, which none of those four covered.

How has this been tested?

Seven new cases. Both failure-path tests fail without the probe.

internal/oauth2:

  • The token endpoint is never called when the write would fail, and oauth2_refresh_token is still unspent afterwards.
  • A refresh that would have succeeded still succeeds, and the sentinel does not survive it.

internal/config:

  • Passes for a writable directory and leaves nothing behind.
  • Fails for an unwritable directory.
  • Passes for an in-memory config, whose Write() is already a no-op, so the probe cannot invent a failure the real write would not hit.
  • Fails for a keyring-mode host whose keyring rejects writes, even though its directory is fine.
  • Passes for a file-mode host whose keyring is unavailable.

Unwritability is simulated by giving the directory a regular file as its parent, so MkdirAll fails with ENOTDIR. chmod is ignored when tests run as root, which is common in CI containers; this is deterministic.

Full suite green: 4609 tests, 9 skipped. golangci-lint clean and the pre-push suite passed.

Merge request reports

Loading
Loading