From 8a556a589323bc6c47bf99e1b60263c976682b45 Mon Sep 17 00:00:00 2001 From: Evan Jarrett Date: Sun, 5 Jul 2026 22:23:54 -0500 Subject: [PATCH] credhelper: key accounts by DID (v3 config) with lossless migration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. + 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) --- pkg/credhelper/cmd_logout.go | 41 +++--- pkg/credhelper/cmd_status.go | 18 +-- pkg/credhelper/cmd_switch.go | 39 +++--- pkg/credhelper/config.go | 242 +++++++++++++++++++++++++++++----- pkg/credhelper/config_test.go | 219 ++++++++++++++++++++++++++++++ pkg/credhelper/helpers.go | 17 ++- pkg/credhelper/protocol.go | 31 ++++- pkg/credhelper/resolve.go | 100 ++++++++++++++ 8 files changed, 626 insertions(+), 81 deletions(-) create mode 100644 pkg/credhelper/config_test.go create mode 100644 pkg/credhelper/resolve.go diff --git a/pkg/credhelper/cmd_logout.go b/pkg/credhelper/cmd_logout.go index e6b7960..b994903 100644 --- a/pkg/credhelper/cmd_logout.go +++ b/pkg/credhelper/cmd_logout.go @@ -39,43 +39,52 @@ func runLogout(cmd *cobra.Command, args []string) error { } // Determine which account to remove - var handle string + var target *Account if len(reg.Accounts) == 1 { - for h := range reg.Accounts { - handle = h + for _, acct := range reg.Accounts { + target = acct } } else { - // Multiple accounts — select which to remove - var handles []string - for h := range reg.Accounts { - handles = append(handles, h) + // 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.Strings(handles) + sort.Slice(accts, func(i, j int) bool { return accts[i].Handle < accts[j].Handle }) + active := reg.activeAccount() var options []huh.Option[string] - for _, h := range handles { - label := h - if h == reg.Active { + for _, acct := range accts { + label := acct.Handle + if acct == active { label += " (active)" } - options = append(options, huh.NewOption(label, h)) + options = append(options, huh.NewOption(label, acct.key())) } + var selected string err := huh.NewSelect[string](). Title("Which account to remove?"). Options(options...). - Value(&handle). + 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?", handle, serverURL)). + Title(fmt.Sprintf("Remove %s from %s?", target.Handle, serverURL)). Value(&confirm). Run() if err != nil || !confirm { @@ -83,11 +92,11 @@ func runLogout(cmd *cobra.Command, args []string) error { return nil } - sc.removeAccount(appViewURL, handle) + 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", handle, serverURL) + fmt.Printf("Removed %s from %s\n", target.Handle, serverURL) return nil } diff --git a/pkg/credhelper/cmd_status.go b/pkg/credhelper/cmd_status.go index c39320e..46350a3 100644 --- a/pkg/credhelper/cmd_status.go +++ b/pkg/credhelper/cmd_status.go @@ -39,24 +39,24 @@ func runStatus(cmd *cobra.Command, args []string) error { reg := sc.Registries[url] fmt.Printf("%s\n", url) - // Sort handles for stable output - var handles []string - for h := range reg.Accounts { - handles = append(handles, h) + // Sort accounts by handle for stable output (the map is keyed by DID). + accts := make([]*Account, 0, len(reg.Accounts)) + for _, acct := range reg.Accounts { + accts = append(accts, acct) } - sort.Strings(handles) + sort.Slice(accts, func(i, j int) bool { return accts[i].Handle < accts[j].Handle }) - for _, handle := range handles { - acct := reg.Accounts[handle] + active := reg.activeAccount() + for _, acct := range accts { marker := " " - if handle == reg.Active { + if acct == active { marker = "* " } did := "" if acct.DID != "" { did = fmt.Sprintf(" (%s)", acct.DID) } - fmt.Printf(" %s%s%s\n", marker, handle, did) + fmt.Printf(" %s%s%s\n", marker, acct.Handle, did) } fmt.Println() } diff --git a/pkg/credhelper/cmd_switch.go b/pkg/credhelper/cmd_switch.go index 34b2abd..71da231 100644 --- a/pkg/credhelper/cmd_switch.go +++ b/pkg/credhelper/cmd_switch.go @@ -40,40 +40,43 @@ func runSwitch(cmd *cobra.Command, args []string) error { } if len(reg.Accounts) == 1 { - for h := range reg.Accounts { - fmt.Fprintf(os.Stderr, "Only one account (%s) — nothing to switch.\n", h) + for _, acct := range reg.Accounts { + fmt.Fprintf(os.Stderr, "Only one account (%s) — nothing to switch.\n", acct.Handle) } return nil } // For exactly 2 accounts, just toggle if len(reg.Accounts) == 2 { - for h := range reg.Accounts { - if h != reg.Active { - reg.Active = h + active := reg.activeAccount() + for _, acct := range reg.Accounts { + if acct != active { + reg.setActive(acct) if err := sc.save(); err != nil { return fmt.Errorf("saving config: %w", err) } - fmt.Printf("Switched to %s on %s\n", h, serverURL) + fmt.Printf("Switched to %s on %s\n", acct.Handle, serverURL) return nil } } } - // 3+ accounts: interactive select - var handles []string - for h := range reg.Accounts { - handles = append(handles, h) + // 3+ accounts: interactive select. 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.Strings(handles) + sort.Slice(accts, func(i, j int) bool { return accts[i].Handle < accts[j].Handle }) + active := reg.activeAccount() var options []huh.Option[string] - for _, h := range handles { - label := h - if h == reg.Active { + for _, acct := range accts { + label := acct.Handle + if acct == active { label += " (current)" } - options = append(options, huh.NewOption(label, h)) + options = append(options, huh.NewOption(label, acct.key())) } var selected string @@ -91,6 +94,10 @@ func runSwitch(cmd *cobra.Command, args []string) error { return fmt.Errorf("saving config: %w", err) } - fmt.Printf("Switched to %s on %s\n", selected, serverURL) + label := selected + if acct := reg.find(selected); acct != nil { + label = acct.Handle + } + fmt.Printf("Switched to %s on %s\n", label, serverURL) return nil } diff --git a/pkg/credhelper/config.go b/pkg/credhelper/config.go index 0750826..1bd0c80 100644 --- a/pkg/credhelper/config.go +++ b/pkg/credhelper/config.go @@ -7,26 +7,46 @@ import ( "time" ) -// StoredConfig is the on-disk credential helper state (v2). Distinct from +// configVersion is the current on-disk format version. v3 keys the accounts +// map (and Active) by DID, the stable identity; v2 keyed them by handle, which +// broke on handle renames. The struct shape is identical between v2 and v3 — +// only the map keys and version number differ — so [loadConfig] migrates v2 +// files in place on first read. +const configVersion = 3 + +// StoredConfig is the on-disk credential helper state (v3). Distinct from // the per-binary [Config] passed to [Run]. type StoredConfig struct { Version int `json:"version"` Registries map[string]*RegistryConfig `json:"registries"` } -// RegistryConfig holds accounts for a single registry. +// RegistryConfig holds accounts for a single registry. Accounts is keyed by +// DID when known, else by handle as a provisional key that is re-keyed to the +// DID once resolved (see [RegistryConfig.rekey]). Active holds the key of the +// active account. type RegistryConfig struct { Active string `json:"active"` Accounts map[string]*Account `json:"accounts"` } -// Account holds credentials for a single identity on a registry. +// Account holds credentials for a single identity on a registry. DID is the +// stable identity; Handle is a mutable display label. type Account struct { Handle string `json:"handle"` DID string `json:"did,omitempty"` DeviceSecret string `json:"device_secret"` } +// key returns the map key for this account: the DID when known, else the +// handle as a provisional key. +func (a *Account) key() string { + if a.DID != "" { + return a.DID + } + return a.Handle +} + // UpdateCheckCache stores the last update check result. type UpdateCheckCache struct { CheckedAt time.Time `json:"checked_at"` @@ -46,10 +66,20 @@ func loadConfig() (*StoredConfig, error) { return newStoredConfig(), err } - // Try v2 format first + // v3 (current) and v2 share the same struct shape — v2 keyed accounts by + // handle, v3 by DID. Unmarshal once, then branch on the version. var sc StoredConfig - if err := json.Unmarshal(data, &sc); err == nil && sc.Version == 2 && sc.Registries != nil { - return &sc, nil + if err := json.Unmarshal(data, &sc); err == nil && sc.Registries != nil { + switch sc.Version { + case configVersion: + return &sc, nil + case 2: + migrated := migrateV2toV3(&sc) + if err := migrated.save(); err != nil { + return migrated, fmt.Errorf("saving migrated config: %w", err) + } + return migrated, nil + } } // Try current multi-registry format: {"credentials": {"url": {...}}} @@ -68,15 +98,14 @@ func loadConfig() (*StoredConfig, error) { if handle == "" { continue } - registryURL := appViewURL - reg := migrated.getOrCreateRegistry(registryURL) - reg.Accounts[handle] = &Account{ + reg := migrated.getOrCreateRegistry(appViewURL) + stored := reg.upsert(&Account{ Handle: handle, DID: cred.DID, DeviceSecret: cred.DeviceSecret, - } + }) if reg.Active == "" { - reg.Active = handle + reg.setActive(stored) } } if err := migrated.save(); err != nil { @@ -99,11 +128,11 @@ func loadConfig() (*StoredConfig, error) { registryURL = "https://" + cfg.DefaultRegistry } reg := migrated.getOrCreateRegistry(registryURL) - reg.Accounts[handle] = &Account{ + stored := reg.upsert(&Account{ Handle: handle, DeviceSecret: legacy.DeviceSecret, - } - reg.Active = handle + }) + reg.setActive(stored) if err := migrated.save(); err != nil { return migrated, fmt.Errorf("saving migrated config: %w", err) } @@ -115,7 +144,7 @@ func loadConfig() (*StoredConfig, error) { func newStoredConfig() *StoredConfig { return &StoredConfig{ - Version: 2, + Version: configVersion, Registries: make(map[string]*RegistryConfig), } } @@ -147,9 +176,129 @@ func (c *StoredConfig) findRegistry(registryURL string) *RegistryConfig { return c.Registries[registryURL] } +// find returns the account matching identity, which may be a DID or a handle. +// It checks the map key first (DID or provisional handle key) then scans by +// DID/Handle, so an image ref by handle resolves even when the map is +// DID-keyed. Returns nil if no account matches. +func (r *RegistryConfig) find(identity string) *Account { + if identity == "" { + return nil + } + if acct, ok := r.Accounts[identity]; ok { + return acct + } + for _, acct := range r.Accounts { + if acct.DID == identity || acct.Handle == identity { + return acct + } + } + return nil +} + +// activeAccount returns the active account, falling back to the sole account +// when Active is unset or dangling and exactly one account exists. +func (r *RegistryConfig) activeAccount() *Account { + if acct := r.find(r.Active); acct != nil { + return acct + } + if len(r.Accounts) == 1 { + for _, acct := range r.Accounts { + return acct + } + } + return nil +} + +// setActive points Active at acct's key (DID when known, else handle). +func (r *RegistryConfig) setActive(acct *Account) { + if acct != nil { + r.Active = acct.key() + } +} + +// upsert inserts or updates an account, keyed by DID when known. It locates any +// existing entry by DID first, then by handle, and updates it in place — +// never blanking a known DID with an empty one. When a provisional +// handle-keyed entry gains a DID, it is re-keyed to the DID (and Active +// repointed). Returns the stored account. +func (r *RegistryConfig) upsert(acct *Account) *Account { + if r.Accounts == nil { + r.Accounts = make(map[string]*Account) + } + + var existing *Account + var existingKey string + if acct.DID != "" { + if e, ok := r.Accounts[acct.DID]; ok { + existing, existingKey = e, acct.DID + } + } + if existing == nil && acct.Handle != "" { + for k, e := range r.Accounts { + if e.Handle == acct.Handle || (acct.DID != "" && e.DID == acct.DID) { + existing, existingKey = e, k + break + } + } + } + + if existing == nil { + r.Accounts[acct.key()] = acct + return acct + } + + if acct.DID != "" { + existing.DID = acct.DID + } + if acct.Handle != "" { + existing.Handle = acct.Handle + } + if acct.DeviceSecret != "" { + existing.DeviceSecret = acct.DeviceSecret + } + if newKey := existing.key(); newKey != existingKey { + delete(r.Accounts, existingKey) + r.Accounts[newKey] = existing + if r.Active == existingKey { + r.Active = newKey + } + } + return existing +} + +// rekey assigns did to a provisional (DID-less) account and moves it under its +// DID key, merging into any existing DID-keyed entry and repointing Active. No +// op if the DID is already set or empty. +func (r *RegistryConfig) rekey(acct *Account, did string) { + if acct == nil || did == "" || acct.DID != "" { + return + } + oldKey := acct.key() // handle + acct.DID = did + newKey := acct.key() // did + if newKey == oldKey { + return + } + delete(r.Accounts, oldKey) + if existing, ok := r.Accounts[newKey]; ok { + // A DID-keyed entry already exists (e.g. a prior login) — merge into it. + if acct.DeviceSecret != "" { + existing.DeviceSecret = acct.DeviceSecret + } + if acct.Handle != "" { + existing.Handle = acct.Handle + } + } else { + r.Accounts[newKey] = acct + } + if r.Active == oldKey { + r.Active = newKey + } +} + // resolveAccount determines which account to use for a given registry. // Priority: -// 1. Identity detected from parent process command line +// 1. Identity detected from parent process command line (handle or DID) // 2. Active account (set by `switch`) // 3. Sole account (if only one exists) // 4. Error @@ -160,19 +309,16 @@ func (c *StoredConfig) resolveAccount(registryURL, serverURL string) (*Account, } // 1. Try to detect identity from parent process - ref := detectImageRef(serverURL) - if ref != nil && ref.Identity != "" { - if acct, ok := reg.Accounts[ref.Identity]; ok { + if ref := detectImageRef(serverURL); ref != nil && ref.Identity != "" { + if acct := reg.find(ref.Identity); acct != nil { return acct, nil } // Identity detected but no matching account — fall through to active } // 2. Active account - if reg.Active != "" { - if acct, ok := reg.Accounts[reg.Active]; ok { - return acct, nil - } + if acct := reg.find(reg.Active); acct != nil { + return acct, nil } // 3. Sole account @@ -189,25 +335,31 @@ func (c *StoredConfig) resolveAccount(registryURL, serverURL string) (*Account, // addAccount adds or updates an account in a registry and sets it active. func (c *StoredConfig) addAccount(registryURL string, acct *Account) { reg := c.getOrCreateRegistry(registryURL) - reg.Accounts[acct.Handle] = acct - reg.Active = acct.Handle + stored := reg.upsert(acct) + reg.setActive(stored) } -// removeAccount removes an account from a registry. -// If it was the active account, clears active (or sets to remaining account if exactly one left). -func (c *StoredConfig) removeAccount(registryURL, handle string) { +// removeAccount removes an account (identity may be a DID or handle) from a +// registry. If it was the active account, clears active (or sets to the +// remaining account if exactly one left). +func (c *StoredConfig) removeAccount(registryURL, identity string) { reg := c.findRegistry(registryURL) if reg == nil { return } - delete(reg.Accounts, handle) + acct := reg.find(identity) + if acct == nil { + return + } + k := acct.key() + delete(reg.Accounts, k) - if reg.Active == handle { + if reg.Active == k || reg.Active == identity { reg.Active = "" if len(reg.Accounts) == 1 { - for h := range reg.Accounts { - reg.Active = h + for _, a := range reg.Accounts { + reg.Active = a.key() } } } @@ -218,6 +370,32 @@ func (c *StoredConfig) removeAccount(registryURL, handle string) { } } +// migrateV2toV3 re-keys a v2 (handle-keyed) config to v3 (DID-keyed) in place. +// Accounts with a stored DID move under their DID key; DID-less accounts keep +// the handle as a provisional key and self-heal on the next successful get. +// No account is dropped. +func migrateV2toV3(sc *StoredConfig) *StoredConfig { + for _, reg := range sc.Registries { + oldActive := reg.Active + newAccounts := make(map[string]*Account, len(reg.Accounts)) + var activeAcct *Account + for oldKey, acct := range reg.Accounts { + if acct.Handle == "" { + acct.Handle = oldKey // v2 map key was the handle + } + newAccounts[acct.key()] = acct + if oldKey == oldActive || acct.Handle == oldActive { + activeAcct = acct + } + } + reg.Accounts = newAccounts + reg.Active = "" + reg.setActive(activeAcct) + } + sc.Version = configVersion + return sc +} + // getUpdateCheckCachePath returns the path to the update check cache file func getUpdateCheckCachePath() string { homeDir, err := os.UserHomeDir() diff --git a/pkg/credhelper/config_test.go b/pkg/credhelper/config_test.go new file mode 100644 index 0000000..85f5dcf --- /dev/null +++ b/pkg/credhelper/config_test.go @@ -0,0 +1,219 @@ +package credhelper + +import ( + "encoding/json" + "io" + "os" + "path/filepath" + "testing" +) + +// setupConfigDir points the config helpers at a temp dir and installs a minimal +// cfg for the duration of the test. +func setupConfigDir(t *testing.T) string { + t.Helper() + dir := t.TempDir() + oldCfg, oldOverride := cfg, configDirOverride + cfg = Config{BinaryName: "docker-credential-test", DefaultRegistry: "atcr.io"} + configDirOverride = dir + t.Cleanup(func() { cfg, configDirOverride = oldCfg, oldOverride }) + return dir +} + +// writeConfigFile writes raw JSON to the active config path. +func writeConfigFile(t *testing.T, dir, contents string) { + t.Helper() + if err := os.WriteFile(filepath.Join(dir, "device.json"), []byte(contents), 0600); err != nil { + t.Fatal(err) + } +} + +func TestMigrateV2toV3_PreservesAndReKeys(t *testing.T) { + dir := setupConfigDir(t) + // v2 file: one account WITH a DID (should re-key to DID), one WITHOUT + // (stays under its handle as a provisional key). Active is a handle. + writeConfigFile(t, dir, `{ + "version": 2, + "registries": { + "https://atcr.io": { + "active": "evan.jarrett.net", + "accounts": { + "evan.jarrett.net": {"handle": "evan.jarrett.net", "device_secret": "atcr_device_A"}, + "tnybot.jarrett.app": {"handle": "tnybot.jarrett.app", "did": "did:plc:tny", "device_secret": "atcr_device_B"} + } + } + } + }`) + + sc, err := loadConfig() + if err != nil { + t.Fatalf("loadConfig: %v", err) + } + if sc.Version != configVersion { + t.Fatalf("version = %d, want %d", sc.Version, configVersion) + } + + reg := sc.Registries["https://atcr.io"] + if reg == nil { + t.Fatal("registry missing after migration") + } + if len(reg.Accounts) != 2 { + t.Fatalf("accounts = %d, want 2 (none dropped)", len(reg.Accounts)) + } + + // Account with a DID is keyed by DID. + if a := reg.Accounts["did:plc:tny"]; a == nil || a.DeviceSecret != "atcr_device_B" { + t.Errorf("did-keyed account not found or wrong secret: %+v", a) + } + // DID-less account survives under its handle key. + if a := reg.Accounts["evan.jarrett.net"]; a == nil || a.DeviceSecret != "atcr_device_A" { + t.Errorf("provisional handle-keyed account not preserved: %+v", a) + } + // Active carried over and still resolves to the same handle. + if act := reg.activeAccount(); act == nil || act.Handle != "evan.jarrett.net" { + t.Errorf("active = %+v, want handle evan.jarrett.net", act) + } + + // Migration persisted to disk as v3. + data, _ := os.ReadFile(filepath.Join(dir, "device.json")) + var onDisk StoredConfig + if err := json.Unmarshal(data, &onDisk); err != nil { + t.Fatal(err) + } + if onDisk.Version != configVersion { + t.Errorf("on-disk version = %d, want %d", onDisk.Version, configVersion) + } +} + +func TestUpsert_MergesAndNeverBlanksDID(t *testing.T) { + reg := &RegistryConfig{Accounts: map[string]*Account{}} + + // First a login-created account with a real DID. + reg.upsert(&Account{Handle: "evan.jarrett.net", DID: "did:plc:evan", DeviceSecret: "secret1"}) + reg.setActive(reg.Accounts["did:plc:evan"]) + + // Then a Docker `store` for the same handle with NO DID and a new secret — + // this is the regression: it must NOT blank the existing DID. + reg.upsert(&Account{Handle: "evan.jarrett.net", DeviceSecret: "secret2"}) + + if len(reg.Accounts) != 1 { + t.Fatalf("accounts = %d, want 1 (merged, not duplicated)", len(reg.Accounts)) + } + a := reg.Accounts["did:plc:evan"] + if a == nil { + t.Fatal("account no longer keyed by DID after store") + } + if a.DID != "did:plc:evan" { + t.Errorf("DID blanked/changed: %q", a.DID) + } + if a.DeviceSecret != "secret2" { + t.Errorf("secret not updated: %q", a.DeviceSecret) + } + if reg.Active != "did:plc:evan" { + t.Errorf("Active = %q, want did:plc:evan", reg.Active) + } +} + +func TestRekey_BackfillsProvisionalAccount(t *testing.T) { + reg := &RegistryConfig{Accounts: map[string]*Account{}} + // Provisional handle-keyed account (e.g. from store when offline). + reg.upsert(&Account{Handle: "evan.jarrett.net", DeviceSecret: "s"}) + reg.setActive(reg.Accounts["evan.jarrett.net"]) + + acct := reg.Accounts["evan.jarrett.net"] + reg.rekey(acct, "did:plc:evan") + + if _, stillHandle := reg.Accounts["evan.jarrett.net"]; stillHandle { + t.Error("old handle key not removed after rekey") + } + if a := reg.Accounts["did:plc:evan"]; a == nil || a.Handle != "evan.jarrett.net" { + t.Errorf("account not re-keyed to DID: %+v", a) + } + if reg.Active != "did:plc:evan" { + t.Errorf("Active not repointed: %q", reg.Active) + } +} + +func TestFind_ByDIDAndHandle(t *testing.T) { + reg := &RegistryConfig{Accounts: map[string]*Account{}} + reg.upsert(&Account{Handle: "evan.jarrett.net", DID: "did:plc:evan", DeviceSecret: "s"}) + + if a := reg.find("did:plc:evan"); a == nil { + t.Error("find by DID failed") + } + // Image refs use the handle even though the map is DID-keyed. + if a := reg.find("evan.jarrett.net"); a == nil { + t.Error("find by handle failed") + } + if a := reg.find("nobody.example"); a != nil { + t.Errorf("find matched nonexistent identity: %+v", a) + } +} + +func TestRunList_ReportsActiveHandle(t *testing.T) { + dir := setupConfigDir(t) + // Multi-account registry: list must report the ACTIVE account's handle, not + // an arbitrary one. Written as v3 (DID-keyed). + writeConfigFile(t, dir, `{ + "version": 3, + "registries": { + "https://buoy.cr": { + "active": "did:plc:evan", + "accounts": { + "did:plc:evan": {"handle": "evan.jarrett.net", "did": "did:plc:evan", "device_secret": "atcr_device_A"}, + "did:plc:tny": {"handle": "tnybot.jarrett.app", "did": "did:plc:tny", "device_secret": "atcr_device_B"} + } + } + } + }`) + + out := captureStdout(t, func() { + if err := runList(nil, nil); err != nil { + t.Fatalf("runList: %v", err) + } + }) + + var got map[string]string + if err := json.Unmarshal([]byte(out), &got); err != nil { + t.Fatalf("decoding list output %q: %v", out, err) + } + if got["buoy.cr"] != "evan.jarrett.net" { + t.Errorf("list reported %q for buoy.cr, want the active account evan.jarrett.net", got["buoy.cr"]) + } +} + +func TestResolveHelpers_Parse(t *testing.T) { + if !looksLikeDID("did:plc:abc") { + t.Error("did:plc:abc should look like a DID") + } + if looksLikeDID("evan.jarrett.net") { + t.Error("handle should not look like a DID") + } + if got := normalizeHandle("@Evan.Jarrett.NET/"); got != "evan.jarrett.net" { + t.Errorf("normalizeHandle = %q", got) + } + txt := []string{"some other record", "did=did:plc:xyz"} + if got := parseDIDFromTXT(txt); got != "did:plc:xyz" { + t.Errorf("parseDIDFromTXT = %q, want did:plc:xyz", got) + } + if got := parseDIDFromTXT([]string{"did=not-a-did", "nope"}); got != "" { + t.Errorf("parseDIDFromTXT accepted invalid DID: %q", got) + } +} + +// captureStdout redirects os.Stdout for the duration of fn and returns what was +// written. +func captureStdout(t *testing.T, fn func()) string { + t.Helper() + old := os.Stdout + r, w, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + os.Stdout = w + fn() + w.Close() + os.Stdout = old + data, _ := io.ReadAll(r) + return string(data) +} diff --git a/pkg/credhelper/helpers.go b/pkg/credhelper/helpers.go index 15506f6..206ceca 100644 --- a/pkg/credhelper/helpers.go +++ b/pkg/credhelper/helpers.go @@ -172,16 +172,23 @@ func isTerminal(f *os.File) bool { return (stat.Mode() & os.ModeCharDevice) != 0 } +// configDirOverride, when non-empty, replaces the default $HOME/ +// config directory. Test-only seam; unset in normal operation. +var configDirOverride string + // getConfigDir returns the per-brand config directory under $HOME, creating // it if needed. The directory name comes from cfg.ConfigDirName. func getConfigDir() string { - homeDir, err := os.UserHomeDir() - if err != nil { - fmt.Fprintf(os.Stderr, "Error getting home directory: %v\n", err) - os.Exit(1) + dir := configDirOverride + if dir == "" { + homeDir, err := os.UserHomeDir() + if err != nil { + fmt.Fprintf(os.Stderr, "Error getting home directory: %v\n", err) + os.Exit(1) + } + dir = filepath.Join(homeDir, cfg.ConfigDirName) } - dir := filepath.Join(homeDir, cfg.ConfigDirName) if err := os.MkdirAll(dir, 0700); err != nil { fmt.Fprintf(os.Stderr, "Error creating %s directory: %v\n", cfg.ConfigDirName, err) os.Exit(1) diff --git a/pkg/credhelper/protocol.go b/pkg/credhelper/protocol.go index 5587c6f..e9ca38f 100644 --- a/pkg/credhelper/protocol.go +++ b/pkg/credhelper/protocol.go @@ -99,11 +99,24 @@ func runGet(cmd *cobra.Command, args []string) error { // 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.Handle) + 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() @@ -136,8 +149,13 @@ func runStore(cmd *cobra.Command, args []string) error { fmt.Fprintf(os.Stderr, "Warning: config load error: %v\n", 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, }) @@ -186,14 +204,21 @@ func runList(cmd *cobra.Command, args []string) error { } // 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://") - for _, acct := range reg.Accounts { - result[host] = acct.Handle + + acct := reg.activeAccount() + if acct == nil { + continue } + result[host] = acct.Handle } return json.NewEncoder(os.Stdout).Encode(result) diff --git a/pkg/credhelper/resolve.go b/pkg/credhelper/resolve.go new file mode 100644 index 0000000..d18c34b --- /dev/null +++ b/pkg/credhelper/resolve.go @@ -0,0 +1,100 @@ +package credhelper + +import ( + "fmt" + "io" + "net" + "strings" + "time" +) + +// resolveHandleDID resolves an AT-proto handle to its DID using the two +// standard resolution methods, no server or auth required: +// +// 1. DNS: a TXT record at _atproto. whose value is "did=". +// 2. HTTPS: GET https:///.well-known/atproto-did returning the bare DID. +// +// DNS is tried first (cheaper, no TLS), HTTPS second. If the input already +// looks like a DID it is returned as-is. Returns an error if neither method +// yields a DID — callers treat resolution as best-effort (a failure just means +// the account keeps its handle key until the next attempt). +func resolveHandleDID(handle string) (string, error) { + h := normalizeHandle(handle) + if h == "" { + return "", fmt.Errorf("empty handle") + } + if looksLikeDID(h) { + return h, nil + } + + if did := resolveHandleDNS(h); did != "" { + return did, nil + } + if did := resolveHandleHTTPS(h); did != "" { + return did, nil + } + return "", fmt.Errorf("could not resolve handle %q to a DID", handle) +} + +// normalizeHandle lowercases and strips an optional leading "@" and any scheme +// or trailing slash a caller may have passed through. +func normalizeHandle(handle string) string { + h := strings.TrimSpace(handle) + h = strings.TrimPrefix(h, "@") + h = strings.TrimPrefix(h, "https://") + h = strings.TrimPrefix(h, "http://") + h = strings.TrimSuffix(h, "/") + return strings.ToLower(h) +} + +// looksLikeDID reports whether s is syntactically a DID (did:method:id). +func looksLikeDID(s string) bool { + return strings.HasPrefix(s, "did:") && strings.Count(s, ":") >= 2 +} + +// resolveHandleDNS looks up the _atproto. TXT record and extracts the +// DID. Returns "" on any failure. +func resolveHandleDNS(handle string) string { + records, err := net.LookupTXT("_atproto." + handle) + if err != nil { + return "" + } + return parseDIDFromTXT(records) +} + +// parseDIDFromTXT returns the first "did=" value found in a set of TXT +// records, or "" if none is a valid DID. +func parseDIDFromTXT(records []string) string { + for _, rec := range records { + val := strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(rec), "did=")) + if val != rec && looksLikeDID(val) { + return val + } + } + return "" +} + +// resolveHandleHTTPS fetches https:///.well-known/atproto-did and +// returns the DID in the body. Returns "" on any failure. +func resolveHandleHTTPS(handle string) string { + client := httpClientWithTimeout(5*time.Second, nil) + resp, err := client.Get("https://" + handle + "/.well-known/atproto-did") + if err != nil { + return "" + } + defer resp.Body.Close() + if resp.StatusCode != 200 { + return "" + } + // The well-known document is a single line containing just the DID; cap the + // read so a misbehaving host can't stream us an unbounded body. + body, err := io.ReadAll(io.LimitReader(resp.Body, 2048)) + if err != nil { + return "" + } + did := strings.TrimSpace(string(body)) + if !looksLikeDID(did) { + return "" + } + return did +}