isNewerVersion split versions on "." and ran each component through
strconv.Atoi, discarding the error and substituting 0. For a git-describe
build the last component is "4-18-g8f70cce", which does not parse, so it
became 0 and every published release compared as newer. Running
v0.1.4-18-g8f70cce printed "Update available: v0.1.4" on every single
invocation, naming a version the binary was already 18 commits past.
Versions are now parsed properly: the "-<commits>-g<sha>" tail is recognised
and kept as a count of commits past the tag, and a version that cannot be
read in full returns false rather than being silently treated as 0. That
second part is the actual root cause — the comparison could not distinguish
"this component is zero" from "I could not read this component".
Ordering for a git-describe build is deliberately not semver, where a
prerelease sorts below its release. Such a build is commits AHEAD of its tag,
so v0.1.4-18-g8f70cce is newer than v0.1.4 and older than v0.1.4-20-gabc1234.
The function had no tests. Both failing cases are pinned along with the
ordinary release comparisons, so the git-describe handling cannot regress the
normal upgrade path.
Pre-existing at efabb677 rather than introduced by this range.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
The credential helper stored accounts keyed by handle, which broke on
handle renames and let the Docker `store` path (username+secret, no DID)
overwrite a good account with a DID-less one — how evan.jarrett.net on
buoy.cr ended up active with a blank DID.
Re-key everything by the stable DID, treating handle as a mutable display
label. DID is recovered client-side via standard AT-proto handle
resolution (DNS TXT _atproto.<handle> + HTTPS .well-known/atproto-did) —
no server change, no JWT, no auth, and no indigo pulled into the helper.
- resolve.go: stdlib handle->DID resolver
- config.go: v3 DID-keyed schema; find/activeAccount/upsert/rekey helpers;
upsert never blanks a known DID; migrateV2toV3 re-keys existing files in
place (no login lost; DID-less accounts stay provisional and self-heal)
- protocol.go: store resolves+preserves DID; get lazily backfills+re-keys;
list reports the active account's handle (was arbitrary map iteration)
- status/switch/logout: display handle, key/compare by DID
- config_test.go: migration, upsert-never-blanks-DID, rekey, find, list,
resolver parse helpers
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Evan JarrettandClaude Opus 5