fix(auth): stop configure-docker from replacing other credential helpers

Sylvia Shenrequested to merge
sshen/fix-docker-cred-helper into main

glab auth configure-docker silently replaces other tools' Docker credential helpers and writes its shim non-atomically, and both it and glab auth docker-helper report configuration read failures as missing configuration.

They surfaced while adding the glab artifact-registry commands, which touch the same credential-helper code: asking whether registering glab as a Docker credential helper conflicts with an existing docker login led to reading Docker's credential-resolution order, and review of the equivalent new code turned up the same swallowed read errors in the shipped code. The defects themselves predate that work.

Replacing another credential helper

configureDocker wrote credHelpers[<domain>] = "glab" without looking at the existing value. In docker/cli, getConfiguredCredentialStore returns credHelpers[domain] when present and GetAuthConfig goes entirely through the credential store, so:

  • the entry is authoritative for that registry, and the key holds exactly one helper — overwriting it takes the registry away from whichever tool owned it (ecr-login, gcloud, a per-registry osxkeychain), and Save discards the old value with no way back;
  • the auths entry that docker login writes is only consulted when no helper is configured, so an existing login for that registry stops being used.

Registration now checks every domain before writing anything and fails with the conflicting domains and the config.json holding them named. Checking all of them up front means a conflict on the third domain cannot leave the first two applied. Shadowing a docker login is a warning rather than an error, since that credential is bypassed, not destroyed.

This is a behaviour change: configure-docker overwrites silently today and will now refuse. Refusing rather than warning because the overwrite is not recoverable.

Read failures reported as missing configuration

findAssociatedHostname and configureDocker both discarded the error from GetWithSource, so a structurally broken host entry or a locked keyring was indistinguishable from "key not set". Users got no hostname associated with registryURL: … or no hosts were configured - ensure you've logged in via oauth2 and configured at least one container registry domain for a host, both of which point at configuration that exists but could not be read.

Read errors are now collected and reported only when no host matched, so one broken entry cannot hide a host that does list the domain.

Torn shim

The shim was written with os.WriteFile, so a docker-credential-glab invocation racing a concurrent configure-docker could observe a partially written script. It now goes through fsx.WriteExecutable (atomic temp file plus rename on POSIX, landing the file at its final mode as part of the same rename so it is never observed non-executable at its final path).

Structure

The shim bytes, the helper names, the platform check, and the credHelpers write move into internal/dockercredhelper, so the conflict guard exists once. The Docker config directory is resolved there rather than by passing "" to dockerconfig.Load, because dockerconfig.Dir memoizes its result in a process-wide sync.Once that the first caller in the process would lock in — which also makes it testable.

Testing

internal/dockercredhelper and internal/commands/auth/docker are covered, including the conflict refusal, the check-all-before-writing ordering, re-registering glab being idempotent, and shadowed-login detection both with a plaintext auths entry and under a configured credsStore (where docker login leaves an empty marker entry and keeps the secret in the OS keychain, so the code checks whether the key exists rather than what it holds).

configureDocker had no tests before this change.

Related

Edited by Sylvia Shen

Merge request reports