Files
Evan JarrettandClaude Opus 5 c615d7253b credhelper: make config writes non-destructive
Follow-up to 8a556a5. v3 keys accounts by DID and, unlike v2, rewrites the
config during ordinary `docker pull` traffic (migration on first read, DID
backfill after a successful get) rather than only on explicit login. That moved
three latent ways to lose a device.json onto the hot path. A user cannot recover
from any of them: once the file is gone they no longer know which accounts they
had.

save() is now atomic. It writes a temp file in the same directory, fsyncs, and
renames, so the config always holds either the previous contents or the complete
new contents. The truncate-then-write it replaced left a zero-length window that
Ctrl-C, Docker reaping the helper, or a suspend could land in. The fsync matters
on its own: on ext4/XFS a rename can become durable while the new file's data
blocks are not, which resurrects the empty-file case. The rename also replaces
the destination's mode, which incidentally repairs a config restored from a
backup as world-readable (os.WriteFile's perm argument only applies at creation).

An unreadable config no longer degrades into an empty one that the next write
commits. loadConfig returns a usable empty config so read paths can still print
something helpful, but it now marks the unrecoverable cases with
errConfigUnusable, and loadConfigForWrite refuses on that sentinel. This is the
downgrade path: the previous binary hard-gates on Version == 2, so without the
guard a reinstall of an older helper would wipe a v3 file on the first
`docker login`. loadConfig also stops explicitly on a version newer than it
understands instead of falling through the legacy probes to the same empty
config. All six write paths take the guard — get, store, erase, login, logout,
switch. login had the same warn-and-continue-then-save shape as store.

The guard keys on the sentinel rather than on any error, deliberately. The
v2 -> v3 migration returns a fully populated config alongside a "saving migrated
config" error when the directory is not writable, and every pre-v3 user passes
through that path on their next invocation; refusing there would hard-break
`docker pull` for exactly the population that is migrating.

migrateV2toV3 merges DID collisions deterministically. Two v2 entries collapse
onto one v3 key when a handle was renamed and the old entry was never cleaned up,
which is the case v3 exists to fix. The previous loop wrote both into the same
map slot in randomized iteration order, so which account survived varied run to
run, and when the stale one won, get would fail validation and remove it — the
user ended up with no account at all. Iteration is now sorted and collisions
resolve through v2EntryBeats (active entry, then the one holding a secret, then
the smaller key), with the loser donating its secret if the winner has none. The
"No account is dropped" comment was false and is now accurate.

Also deterministic: find() and upsert() resolve their scans through a shared
scanFor helper that prefers DID matches and breaks ties on the smallest key, so
two entries sharing a handle can no longer hand Docker a different secret on each
invocation. upsert additionally refuses to match a handle that already belongs to
a different DID, which previously let it overwrite an unrelated account's DID and
destroy that account's credentials. Nil map entries are tolerated throughout
rather than panicking on Docker's credential path.

resolveHandleDNS is bounded at 2s. It runs synchronously on every store and on
every get for a DID-less account, and net.LookupTXT applies no deadline of its
own, so a blackholing resolver (captive portal, split-horizon VPN) stalled
docker login and docker pull on each invocation.

Tests cover the paths that can lose credentials. Each was checked against the
pre-fix code: the collision, null-entry, DID-theft, find-determinism,
permissions, and out-of-place-write tests all fail or panic without their fix.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-02 19:19:40 -05:00

271 lines
7.8 KiB
Go

package credhelper
import (
"encoding/json"
"fmt"
"os"
"strings"
"github.com/spf13/cobra"
)
// Credentials represents docker credentials (Docker credential helper protocol)
type Credentials struct {
ServerURL string `json:"ServerURL,omitempty"`
Username string `json:"Username,omitempty"`
Secret string `json:"Secret,omitempty"`
}
func newGetCmd() *cobra.Command {
return &cobra.Command{
Use: "get",
Short: "Get credentials for a registry (Docker protocol)",
Hidden: true,
RunE: runGet,
}
}
func newStoreCmd() *cobra.Command {
return &cobra.Command{
Use: "store",
Short: "Store credentials (Docker protocol)",
Hidden: true,
RunE: runStore,
}
}
func newEraseCmd() *cobra.Command {
return &cobra.Command{
Use: "erase",
Short: "Erase credentials (Docker protocol)",
Hidden: true,
RunE: runErase,
}
}
func newListCmd() *cobra.Command {
return &cobra.Command{
Use: "list",
Short: "List all credentials (Docker protocol extension)",
Hidden: true,
RunE: runList,
}
}
func runGet(cmd *cobra.Command, args []string) error {
// If stdin is a terminal, the user ran this directly (not Docker calling us)
if isTerminal(os.Stdin) {
fmt.Fprintf(os.Stderr, "The 'get' command is part of the Docker credential helper protocol.\n")
fmt.Fprintf(os.Stderr, "It should not be run directly.\n\n")
fmt.Fprintf(os.Stderr, "To authenticate with a registry, run:\n")
fmt.Fprintf(os.Stderr, " %s login\n\n", cfg.BinaryName)
fmt.Fprintf(os.Stderr, "To check your accounts:\n")
fmt.Fprintf(os.Stderr, " %s status\n", cfg.BinaryName)
return fmt.Errorf("not a pipe")
}
// Docker sends the server URL as a plain string on stdin (not JSON)
var serverURL string
if _, err := fmt.Fscanln(os.Stdin, &serverURL); err != nil {
return fmt.Errorf("reading server URL: %w", err)
}
appViewURL := buildAppViewURL(serverURL)
// `get` can rewrite the config (DID backfill, or removing an account whose
// credentials no longer validate), so it must not proceed on a config it
// failed to parse — the empty fallback would be saved over the real one.
// Returned, not printed: Run's top-level handler already prints it.
sc, err := loadConfigForWrite()
if err != nil {
return err
}
acct, err := sc.resolveAccount(appViewURL, serverURL)
if err != nil {
return err
}
// Validate credentials
result := validateCredentials(appViewURL, acct.Handle, acct.DeviceSecret)
if !result.Valid {
if result.OAuthSessionExpired {
loginURL := result.LoginURL
if loginURL == "" {
loginURL = appViewURL + "/auth/oauth/login"
}
fmt.Fprintf(os.Stderr, "OAuth session expired for %s.\n", acct.Handle)
fmt.Fprintf(os.Stderr, "Please visit: %s\n", loginURL)
fmt.Fprintf(os.Stderr, "Then retry your docker command.\n")
return fmt.Errorf("oauth session expired")
}
// Generic auth failure — remove the bad account
fmt.Fprintf(os.Stderr, "Credentials for %s are invalid.\n", acct.Handle)
fmt.Fprintf(os.Stderr, "Run: %s login\n", cfg.BinaryName)
sc.removeAccount(appViewURL, acct.key())
sc.save() //nolint:errcheck
return fmt.Errorf("invalid credentials")
}
// Credentials are valid. If this account is still keyed by handle (e.g.
// created via Docker `store`, which has no DID), backfill its DID by
// resolving the handle and re-key it. Best-effort: resolution failure is
// non-fatal and the secret works regardless.
if acct.DID == "" {
if did, resolveErr := resolveHandleDID(acct.Handle); resolveErr == nil {
if reg := sc.findRegistry(appViewURL); reg != nil {
reg.rekey(acct, did)
sc.save() //nolint:errcheck
}
}
}
// Check for updates (cached, non-blocking)
checkAndNotifyUpdate()
// Return credentials for Docker
creds := Credentials{
ServerURL: serverURL,
Username: acct.Handle,
Secret: acct.DeviceSecret,
}
return json.NewEncoder(os.Stdout).Encode(creds)
}
func runStore(cmd *cobra.Command, args []string) error {
var creds Credentials
if err := json.NewDecoder(os.Stdin).Decode(&creds); err != nil {
return fmt.Errorf("decoding credentials: %w", err)
}
// Only store if the secret looks like one of our device secrets
if !strings.HasPrefix(creds.Secret, cfg.SecretPrefix) {
// Not our device secret — ignore (e.g., docker login with app-password)
return nil
}
appViewURL := buildAppViewURL(creds.ServerURL)
// Refuse to write when the existing config couldn't be read. Warning and
// continuing would save the empty fallback config, replacing every stored
// account with just this one. That is the downgrade path: an older build
// cannot parse a v3 file, so the first `docker login` would wipe it.
// Returned, not printed: Run's top-level handler already prints it.
sc, err := loadConfigForWrite()
if err != nil {
return err
}
// Docker's store protocol only supplies {username, secret} — no DID. Resolve
// the handle to its DID (best-effort) so the account keys by the stable DID.
// addAccount/upsert preserves any existing DID rather than blanking it.
did, _ := resolveHandleDID(creds.Username)
sc.addAccount(appViewURL, &Account{
Handle: creds.Username,
DID: did,
DeviceSecret: creds.Secret,
})
return sc.save()
}
func runErase(cmd *cobra.Command, args []string) error {
var serverURL string
if _, err := fmt.Fscanln(os.Stdin, &serverURL); err != nil {
return fmt.Errorf("reading server URL: %w", err)
}
appViewURL := buildAppViewURL(serverURL)
// Erase writes, so an unreadable config means we do nothing rather than
// save the empty fallback over it. Reported as success: Docker surfaces a
// failing `logout` as a hard error, and there is nothing the user can act on.
sc, err := loadConfigForWrite()
if err != nil {
fmt.Fprintf(os.Stderr, "Warning: %v\n", err)
return nil
}
reg := sc.findRegistry(appViewURL)
if reg == nil {
return nil
}
// Erase the active account (or sole account)
handle := reg.Active
if handle == "" && len(reg.Accounts) == 1 {
for h := range reg.Accounts {
handle = h
}
}
if handle == "" {
return nil
}
sc.removeAccount(appViewURL, handle)
return sc.save()
}
func runList(cmd *cobra.Command, args []string) error {
sc, err := loadConfig()
if err != nil {
// Return empty object
fmt.Println("{}")
return nil
}
// Docker list protocol: {"ServerURL": "Username", ...}
// Report exactly one username per registry: the active account (or the
// sole account if none is explicitly active), matching what `get` hands
// Docker via resolveAccount. The map is keyed by DID, but the protocol
// value must be the handle/username, so read acct.Handle.
result := make(map[string]string)
for url, reg := range sc.Registries {
// Strip scheme for Docker compatibility
host := strings.TrimPrefix(url, "https://")
host = strings.TrimPrefix(host, "http://")
acct := reg.activeAccount()
if acct == nil {
continue
}
result[host] = acct.Handle
}
return json.NewEncoder(os.Stdout).Encode(result)
}
// checkAndNotifyUpdate checks for updates in the background and notifies the user
func checkAndNotifyUpdate() {
cache := loadUpdateCheckCache()
if cache != nil && cache.Current == cfg.Version {
// Cache is fresh and for current version
if isNewerVersion(cache.Latest, cfg.Version) {
fmt.Fprintf(os.Stderr, "\nUpdate available: %s (current: %s)\n", cache.Latest, cfg.Version)
fmt.Fprintf(os.Stderr, "Run: %s update\n\n", cfg.BinaryName)
}
// Check if cache is still fresh (24h)
if cache.CheckedAt.Add(updateCheckCacheTTL).After(timeNow()) {
return
}
}
latest, err := fetchLatestVersion()
if err != nil {
return // Silently fail
}
saveUpdateCheckCache(&UpdateCheckCache{
CheckedAt: timeNow(),
Latest: latest,
Current: cfg.Version,
})
if isNewerVersion(latest, cfg.Version) {
fmt.Fprintf(os.Stderr, "\nUpdate available: %s (current: %s)\n", latest, cfg.Version)
fmt.Fprintf(os.Stderr, "Run: %s update\n\n", cfg.BinaryName)
}
}