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

103 lines
2.3 KiB
Go

package credhelper
import (
"fmt"
"os"
"sort"
"github.com/charmbracelet/huh"
"github.com/spf13/cobra"
)
func newLogoutCmd() *cobra.Command {
return &cobra.Command{
Use: "logout [registry]",
Short: "Remove account credentials",
Long: "Remove stored credentials for an account.\nDefault registry: " + cfg.DefaultRegistry,
Args: cobra.MaximumNArgs(1),
RunE: runLogout,
}
}
func runLogout(cmd *cobra.Command, args []string) error {
serverURL := cfg.DefaultRegistry
if len(args) > 0 {
serverURL = args[0]
}
appViewURL := buildAppViewURL(serverURL)
sc, err := loadConfigForWrite()
if err != nil {
return err
}
reg := sc.findRegistry(appViewURL)
if reg == nil || len(reg.Accounts) == 0 {
fmt.Fprintf(os.Stderr, "No accounts configured for %s.\n", serverURL)
return nil
}
// Determine which account to remove
var target *Account
if len(reg.Accounts) == 1 {
for _, acct := range reg.Accounts {
target = acct
}
} else {
// Multiple accounts — select which to remove. Options display the
// handle but carry the account's key (DID) as the value.
accts := make([]*Account, 0, len(reg.Accounts))
for _, acct := range reg.Accounts {
accts = append(accts, acct)
}
sort.Slice(accts, func(i, j int) bool { return accts[i].Handle < accts[j].Handle })
active := reg.activeAccount()
var options []huh.Option[string]
for _, acct := range accts {
label := acct.Handle
if acct == active {
label += " (active)"
}
options = append(options, huh.NewOption(label, acct.key()))
}
var selected string
err := huh.NewSelect[string]().
Title("Which account to remove?").
Options(options...).
Value(&selected).
Run()
if err != nil {
return err
}
target = reg.find(selected)
}
if target == nil {
fmt.Fprintf(os.Stderr, "No account selected.\n")
return nil
}
// Confirm
var confirm bool
err = huh.NewConfirm().
Title(fmt.Sprintf("Remove %s from %s?", target.Handle, serverURL)).
Value(&confirm).
Run()
if err != nil || !confirm {
fmt.Fprintf(os.Stderr, "Cancelled.\n")
return nil
}
sc.removeAccount(appViewURL, target.key())
if err := sc.save(); err != nil {
return fmt.Errorf("saving config: %w", err)
}
fmt.Printf("Removed %s from %s\n", target.Handle, serverURL)
return nil
}