From a0cf253177ba1e10dcd226d9658328bc23c4e6ae Mon Sep 17 00:00:00 2001
From: euxaristia <25621994+euxaristia@users.noreply.github.com>
Date: Tue, 14 Jul 2026 04:36:21 -0400
Subject: [PATCH 01/17] fix(oauth): store keyring tokens as one entry per
 provider

The keyring backend combined every provider and MCP token into a single
JSON blob under one keyring entry. On macOS, add-generic-password now
writes through security -i, whose command parser caps a single write at
4095 bytes (#574). The combined blob has no such bound: three or more
logged-in providers routinely exceeds it, so Set() starts failing for
every provider, not just the one that pushed it over.

Split storage into one keyring entry per token key, plus a small index
entry listing which keys exist (KeyringClient has no list operation).
Each write is now bounded to a single token's size, well under the
line cap regardless of how many providers are logged in. Installs on
the old combined-entry format keep reading correctly via a legacy
fallback, and get migrated to per-key entries on the next save.
---
 internal/oauth/store.go              | 145 +++++++++++++++++++++++++--
 internal/oauth/store_keyring_test.go | 112 ++++++++++++++++++++-
 2 files changed, 244 insertions(+), 13 deletions(-)

diff --git a/internal/oauth/store.go b/internal/oauth/store.go
index 951e9616f..f111c8a4e 100644
--- a/internal/oauth/store.go
+++ b/internal/oauth/store.go
@@ -106,10 +106,23 @@ type KeyringClient interface {
 	Delete(service, account string) (bool, error)
 }
 
-// Keyring storage stores the whole token blob under one fixed entry.
+// Keyring storage splits the token blob into one keyring entry per token key,
+// plus a small index entry listing which keys exist. A single combined entry
+// (the original design) grows with every additional provider/MCP login and,
+// on macOS, add-generic-password now goes through `security -i`'s line-based
+// command parser (see internal/keyring), which caps a single write at 4095
+// bytes; three or more logged-in providers routinely exceeds that. Splitting
+// by key bounds each write to one token, which stays well under the cap
+// regardless of how many providers are logged in.
 const (
 	keyringService = "zero"
-	keyringAccount = "oauth-tokens"
+	// keyringLegacyAccount held the whole blob as one entry in the original
+	// design. New writes never use it; it is only read once, to migrate
+	// existing installs into the per-key format.
+	keyringLegacyAccount = "oauth-tokens"
+	// keyringIndexAccount holds a JSON array of the token keys that currently
+	// have their own keyring entry, since KeyringClient has no "list" operation.
+	keyringIndexAccount = "oauth-tokens-index"
 )
 
 // Store persists OAuth tokens (provider + MCP namespaces) as one JSON blob,
@@ -203,7 +216,7 @@ func NewStore(options StoreOptions) (*Store, error) {
 		if storePath, perr := ResolveStorePath(options.Env); perr == nil {
 			lockPath = filepath.Join(filepath.Dir(storePath), "oauth-keyring.lockfile")
 		}
-		return &Store{blob: keyringBlob{kr: kr, service: keyringService, account: keyringAccount, lockPath: lockPath}, now: now}, nil
+		return &Store{blob: keyringBlob{kr: kr, service: keyringService, legacyAccount: keyringLegacyAccount, indexAccount: keyringIndexAccount, lockPath: lockPath}, now: now}, nil
 	default:
 		return nil, fmt.Errorf("oauth: unknown storage %q (want \"file\", \"encrypted-file\", or \"keyring\")", storage)
 	}
@@ -431,19 +444,70 @@ func (b fileBlob) withLock(now func() time.Time, fn func() error) error {
 
 func (b fileBlob) location() string { return b.path }
 
-// keyringBlob persists the blob in the OS keyring as a single base64 entry
-// (base64 keeps the multi-line JSON a single, control-character-free value).
+// keyringBlob persists tokens in the OS keyring as one base64 entry per token
+// key (account = key), plus an index entry listing which keys exist (base64
+// keeps every value a single, control-character-free string; see keyringService
+// for why a single combined entry doesn't work). read/write still present the
+// same whole-blob shape (a marshaled storeFile) that Store expects, fanning it
+// out to/in from the individual entries internally.
 type keyringBlob struct {
 	kr      KeyringClient
 	service string
-	account string
+	// legacyAccount is the pre-migration whole-blob entry; read only, to pick up
+	// tokens saved by older versions the first time this runs.
+	legacyAccount string
+	indexAccount  string
 	// lockPath, when set, is a cross-process lock file serializing the keyring's
 	// read-modify-write so concurrent processes don't clobber each other's tokens.
 	lockPath string
 }
 
 func (b keyringBlob) read() ([]byte, bool, error) {
-	enc, ok, err := b.kr.Get(b.service, b.account)
+	indexEnc, ok, err := b.kr.Get(b.service, b.indexAccount)
+	if err != nil {
+		return nil, false, err
+	}
+	if !ok {
+		return b.readLegacy()
+	}
+	keys, err := decodeKeyIndex(indexEnc)
+	if err != nil {
+		return nil, false, fmt.Errorf("oauth: decode keyring token index: %w", err)
+	}
+	tokens := make(map[string]Token, len(keys))
+	for _, key := range keys {
+		enc, ok, err := b.kr.Get(b.service, key)
+		if err != nil {
+			return nil, false, err
+		}
+		if !ok {
+			// Index and entries fell out of sync (e.g. a killed process between
+			// writing an entry and updating the index); skip rather than fail the
+			// whole read, since the next Save/Delete will reconcile the index.
+			continue
+		}
+		raw, err := base64.StdEncoding.DecodeString(strings.TrimSpace(enc))
+		if err != nil {
+			return nil, false, fmt.Errorf("oauth: decode keyring token entry %q: %w", key, err)
+		}
+		var token Token
+		if err := json.Unmarshal(raw, &token); err != nil {
+			return nil, false, fmt.Errorf("oauth: invalid keyring token entry %q: %w", key, err)
+		}
+		tokens[key] = token
+	}
+	data, err := json.Marshal(storeFile{SchemaVersion: storeSchemaVersion, Tokens: tokens})
+	if err != nil {
+		return nil, false, err
+	}
+	return data, true, nil
+}
+
+// readLegacy reads the pre-migration whole-blob entry, for installs that
+// haven't written since upgrading. The next write() migrates them: it writes
+// per-key entries and an index, then deletes this entry.
+func (b keyringBlob) readLegacy() ([]byte, bool, error) {
+	enc, ok, err := b.kr.Get(b.service, b.legacyAccount)
 	if err != nil || !ok {
 		return nil, ok, err
 	}
@@ -455,7 +519,70 @@ func (b keyringBlob) read() ([]byte, bool, error) {
 }
 
 func (b keyringBlob) write(data []byte) error {
-	return b.kr.Set(b.service, b.account, base64.StdEncoding.EncodeToString(data))
+	var state storeFile
+	if err := json.Unmarshal(data, &state); err != nil {
+		return fmt.Errorf("oauth: encode keyring token blob: %w", err)
+	}
+	priorKeys, err := b.indexedKeys()
+	if err != nil {
+		return err
+	}
+	keys := make([]string, 0, len(state.Tokens))
+	for key, token := range state.Tokens {
+		raw, err := json.Marshal(token)
+		if err != nil {
+			return err
+		}
+		if err := b.kr.Set(b.service, key, base64.StdEncoding.EncodeToString(raw)); err != nil {
+			return err
+		}
+		keys = append(keys, key)
+	}
+	sort.Strings(keys)
+	indexData, err := json.Marshal(keys)
+	if err != nil {
+		return err
+	}
+	if err := b.kr.Set(b.service, b.indexAccount, base64.StdEncoding.EncodeToString(indexData)); err != nil {
+		return err
+	}
+	for _, key := range priorKeys {
+		if _, ok := state.Tokens[key]; !ok {
+			if _, err := b.kr.Delete(b.service, key); err != nil {
+				return err
+			}
+		}
+	}
+	// The index now exists and is authoritative; drop the legacy entry so a
+	// future read never falls back to it.
+	_, _ = b.kr.Delete(b.service, b.legacyAccount)
+	return nil
+}
+
+// indexedKeys returns the keys currently listed in the index, or nil if there
+// is no index yet (first write, or still on the legacy format).
+func (b keyringBlob) indexedKeys() ([]string, error) {
+	enc, ok, err := b.kr.Get(b.service, b.indexAccount)
+	if err != nil || !ok {
+		return nil, err
+	}
+	keys, err := decodeKeyIndex(enc)
+	if err != nil {
+		return nil, fmt.Errorf("oauth: decode keyring token index: %w", err)
+	}
+	return keys, nil
+}
+
+func decodeKeyIndex(enc string) ([]string, error) {
+	data, err := base64.StdEncoding.DecodeString(strings.TrimSpace(enc))
+	if err != nil {
+		return nil, err
+	}
+	var keys []string
+	if err := json.Unmarshal(data, &keys); err != nil {
+		return nil, err
+	}
+	return keys, nil
 }
 
 // withLock serializes the keyring's read-modify-write. Store.mu covers the
@@ -473,7 +600,7 @@ func (b keyringBlob) withLock(now func() time.Time, fn func() error) error {
 	return fn()
 }
 
-func (b keyringBlob) location() string { return "keyring:" + b.service + "/" + b.account }
+func (b keyringBlob) location() string { return "keyring:" + b.service + "/" + b.indexAccount }
 
 // FormatStatuses renders a human-readable status table without leaking token
 // material.
diff --git a/internal/oauth/store_keyring_test.go b/internal/oauth/store_keyring_test.go
index 8931dc6de..d79294be0 100644
--- a/internal/oauth/store_keyring_test.go
+++ b/internal/oauth/store_keyring_test.go
@@ -1,6 +1,8 @@
 package oauth
 
 import (
+	"encoding/base64"
+	"encoding/json"
 	"strings"
 	"testing"
 )
@@ -49,13 +51,17 @@ func TestStoreKeyringBackendRoundTrip(t *testing.T) {
 		t.Fatalf("Load = %#v", got)
 	}
 
-	// The blob is stored base64-encoded, so the raw JSON field names never appear.
-	raw := kr.data[keyringService+"/"+keyringAccount]
+	// The token lives under its own entry (account = key), not one combined
+	// blob, and is base64-encoded so the raw JSON field names never appear.
+	raw := kr.data[keyringService+"/"+ProviderKey("demo")]
 	if raw == "" {
-		t.Fatal("nothing stored in keyring")
+		t.Fatal("nothing stored under the token's own keyring entry")
 	}
 	if strings.Contains(raw, "access_token") {
-		t.Fatalf("keyring blob is not encoded: %s", raw)
+		t.Fatalf("keyring entry is not encoded: %s", raw)
+	}
+	if raw := kr.data[keyringService+"/"+keyringLegacyAccount]; raw != "" {
+		t.Fatalf("legacy combined entry should not be written by new code: %s", raw)
 	}
 
 	removed, err := s.Delete(ProviderKey("demo"))
@@ -65,6 +71,104 @@ func TestStoreKeyringBackendRoundTrip(t *testing.T) {
 	if _, ok, _ := s.Load(ProviderKey("demo")); ok {
 		t.Fatal("token still present after delete")
 	}
+	// Delete must also drop the now-unused entry, not just remove it from the
+	// index, or a stale keyring item accumulates for every logout.
+	if _, ok := kr.data[keyringService+"/"+ProviderKey("demo")]; ok {
+		t.Fatal("deleted token's keyring entry was not removed")
+	}
+}
+
+// TestStoreKeyringManyProvidersStayUnderEntryLimit is the regression test for
+// the bug this backend originally shipped with: every provider's tokens were
+// combined into one keyring entry, and on macOS that entry is written through
+// `security -i`, whose command parser caps a single write around 4KB. Three or
+// more logged-in providers routinely exceeded it, so Set() would start failing
+// for every provider, not just the one pushing it over. Splitting into one
+// entry per key bounds each individual write to a single token regardless of
+// how many providers are logged in.
+func TestStoreKeyringManyProvidersStayUnderEntryLimit(t *testing.T) {
+	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
+	kr := newFakeKR()
+	s, err := NewStore(StoreOptions{Storage: "keyring", Keyring: kr})
+	if err != nil {
+		t.Fatal(err)
+	}
+	// A realistically large single token: JWT-shaped access/ID tokens plus an
+	// opaque refresh token, comparable to what OIDC providers actually issue.
+	big := Token{
+		AccessToken:  "eyJhbGciOiJSUzI1NiJ9." + strings.Repeat("QUJDRA", 60) + ".sig",
+		RefreshToken: "rt_" + strings.Repeat("x", 80),
+		TokenType:    "Bearer",
+		Scopes:       []string{"openid", "profile", "email", "offline_access"},
+		Account:      "user@example.com",
+		IDToken:      "eyJhbGciOiJSUzI1NiJ9." + strings.Repeat("QUJDRA", 70) + ".sig",
+	}
+	providers := []string{"anthropic", "openai", "minimax", "zai", "google"}
+	for _, name := range providers {
+		if err := s.Save(ProviderKey(name), big); err != nil {
+			t.Fatalf("Save(%s): %v", name, err)
+		}
+	}
+	// Each individual keyring value must stay small even with 5 providers
+	// logged in: no entry aggregates more than one provider's tokens.
+	const singleTokenCeiling = 3000 // generous margin under the ~4095-byte line cap
+	for k, v := range kr.data {
+		if len(v) > singleTokenCeiling {
+			t.Fatalf("keyring entry %q is %d bytes, want < %d (aggregation regression)", k, len(v), singleTokenCeiling)
+		}
+	}
+	for _, name := range providers {
+		got, ok, err := s.Load(ProviderKey(name))
+		if err != nil || !ok {
+			t.Fatalf("Load(%s): ok=%v err=%v", name, ok, err)
+		}
+		if got.AccessToken != big.AccessToken {
+			t.Fatalf("Load(%s) = %#v", name, got)
+		}
+	}
+}
+
+// TestStoreKeyringMigratesLegacyCombinedEntry ensures installs upgrading from
+// the original single-blob format keep reading their existing tokens, and get
+// migrated to per-key entries (with the legacy entry removed) the next time
+// anything is saved.
+func TestStoreKeyringMigratesLegacyCombinedEntry(t *testing.T) {
+	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
+	kr := newFakeKR()
+	legacy := storeFile{SchemaVersion: storeSchemaVersion, Tokens: map[string]Token{
+		ProviderKey("demo"): {AccessToken: "legacy-a", RefreshToken: "legacy-r"},
+	}}
+	data, err := json.Marshal(legacy)
+	if err != nil {
+		t.Fatal(err)
+	}
+	kr.data[keyringService+"/"+keyringLegacyAccount] = base64.StdEncoding.EncodeToString(data)
+
+	s, err := NewStore(StoreOptions{Storage: "keyring", Keyring: kr})
+	if err != nil {
+		t.Fatal(err)
+	}
+	got, ok, err := s.Load(ProviderKey("demo"))
+	if err != nil || !ok {
+		t.Fatalf("Load legacy token: ok=%v err=%v", ok, err)
+	}
+	if got.AccessToken != "legacy-a" {
+		t.Fatalf("Load = %#v", got)
+	}
+
+	// Saving a second provider must migrate: the legacy entry is dropped, and
+	// both tokens end up as their own entries.
+	if err := s.Save(ProviderKey("other"), Token{AccessToken: "other-a"}); err != nil {
+		t.Fatal(err)
+	}
+	if _, ok := kr.data[keyringService+"/"+keyringLegacyAccount]; ok {
+		t.Fatal("legacy combined entry should be removed after migration")
+	}
+	for _, name := range []string{"demo", "other"} {
+		if _, ok, err := s.Load(ProviderKey(name)); err != nil || !ok {
+			t.Fatalf("Load(%s) after migration: ok=%v err=%v", name, ok, err)
+		}
+	}
 }
 
 func TestNewStoreStorageSelection(t *testing.T) {

From 5bfbd1676baa62cd7f2a993c60f6a8ac2c4cb256 Mon Sep 17 00:00:00 2001
From: euxaristia <25621994+euxaristia@users.noreply.github.com>
Date: Tue, 14 Jul 2026 11:38:24 -0400
Subject: [PATCH 02/17] fix(oauth): lock keyring reads against concurrent
 Save/Delete

Store.Load and Status read the keyring blob via several separate Get
calls (index, then each entry), not one atomic snapshot, but only
Save/Delete ran that read-modify-write under blob.withLock. An
unlocked Load/Status could run concurrently with another process's
Save/Delete mid write and observe a torn state. Route both through
withLock like Save/Delete already do.

Also add coverage for read()'s index/entry desync recovery: a key
listed in the index whose own entry is missing must be skipped, not
fail the whole read.
---
 internal/oauth/store.go              | 22 ++++++++++--
 internal/oauth/store_keyring_test.go | 50 ++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/internal/oauth/store.go b/internal/oauth/store.go
index f111c8a4e..ac985d957 100644
--- a/internal/oauth/store.go
+++ b/internal/oauth/store.go
@@ -269,7 +269,18 @@ func (s *Store) Load(key string) (Token, bool, error) {
 	}
 	s.mu.Lock()
 	defer s.mu.Unlock()
-	state, err := s.readState()
+	// Through blob.withLock, like Save/Delete: the keyring backend's read is
+	// several separate Get calls (index, then each entry), not one atomic
+	// snapshot, so an unlocked Load can run concurrently with another
+	// process's Save/Delete mid write and observe a torn state (e.g. an index
+	// already updated but an entry not yet written). The lock keeps this read
+	// from overlapping any other process's read-modify-write cycle.
+	var state storeFile
+	err := s.blob.withLock(s.now, func() error {
+		var readErr error
+		state, readErr = s.readState()
+		return readErr
+	})
 	if err != nil {
 		return Token{}, false, err
 	}
@@ -305,7 +316,14 @@ func (s *Store) Delete(key string) (bool, error) {
 func (s *Store) Status(prefix string) ([]Status, error) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
-	state, err := s.readState()
+	// Same reasoning as Load: run the read under blob.withLock so it can't
+	// observe another process's Save/Delete mid write.
+	var state storeFile
+	err := s.blob.withLock(s.now, func() error {
+		var readErr error
+		state, readErr = s.readState()
+		return readErr
+	})
 	if err != nil {
 		return nil, err
 	}
diff --git a/internal/oauth/store_keyring_test.go b/internal/oauth/store_keyring_test.go
index d79294be0..34bf945a4 100644
--- a/internal/oauth/store_keyring_test.go
+++ b/internal/oauth/store_keyring_test.go
@@ -171,6 +171,56 @@ func TestStoreKeyringMigratesLegacyCombinedEntry(t *testing.T) {
 	}
 }
 
+// TestStoreKeyringSkipsIndexedKeyMissingItsEntry covers read()'s recovery from
+// an index/entry desync: a key listed in the index whose own entry is
+// missing (e.g. a process killed between writing the entry and updating the
+// index, or between updating the index and deleting a removed entry). read()
+// must skip that key rather than fail the whole read, since the next
+// Save/Delete reconciles the index against what's actually there.
+func TestStoreKeyringSkipsIndexedKeyMissingItsEntry(t *testing.T) {
+	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
+	kr := newFakeKR()
+
+	present := Token{AccessToken: "present-a", RefreshToken: "present-r"}
+	raw, err := json.Marshal(present)
+	if err != nil {
+		t.Fatal(err)
+	}
+	kr.data[keyringService+"/"+ProviderKey("present")] = base64.StdEncoding.EncodeToString(raw)
+
+	// The index references both keys, but "missing"'s own entry was never
+	// written (or was already deleted) — the desync this test targets.
+	index, err := json.Marshal([]string{ProviderKey("missing"), ProviderKey("present")})
+	if err != nil {
+		t.Fatal(err)
+	}
+	kr.data[keyringService+"/"+keyringIndexAccount] = base64.StdEncoding.EncodeToString(index)
+
+	s, err := NewStore(StoreOptions{Storage: "keyring", Keyring: kr})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if _, ok, err := s.Load(ProviderKey("missing")); err != nil || ok {
+		t.Fatalf("Load(missing): ok=%v err=%v, want ok=false err=nil", ok, err)
+	}
+	got, ok, err := s.Load(ProviderKey("present"))
+	if err != nil || !ok {
+		t.Fatalf("Load(present): ok=%v err=%v", ok, err)
+	}
+	if got.AccessToken != present.AccessToken {
+		t.Fatalf("Load(present) = %#v", got)
+	}
+
+	statuses, err := s.Status("")
+	if err != nil {
+		t.Fatalf("Status: %v", err)
+	}
+	if len(statuses) != 1 || statuses[0].Key != ProviderKey("present") {
+		t.Fatalf("Status = %#v, want only the present key", statuses)
+	}
+}
+
 func TestNewStoreStorageSelection(t *testing.T) {
 	// Unknown storage is rejected (fail closed).
 	if _, err := NewStore(StoreOptions{Storage: "bogus"}); err == nil {

From f2e37bd6fcb9ff80a603f974c75a101754dc8539 Mon Sep 17 00:00:00 2001
From: euxaristia <25621994+euxaristia@users.noreply.github.com>
Date: Wed, 15 Jul 2026 12:52:14 -0400
Subject: [PATCH 03/17] fix(oauth): make the keyring token store bounded,
 recoverable, and mixed-version safe

- The key index is chunked: continuation entries hold overflow keys and are
  written before the header that references them, so every index entry stays
  under the macOS security -i 4095-byte line cap regardless of how many
  providers are logged in, and a torn chunk write is skipped on read like a
  missing token entry.
- Writes follow a recoverable ordering: the union of the prior and new key
  sets is published first, token entries are written next, removed entries
  are deleted while the index still lists them, and only then does the index
  shrink. Every token entry that exists at any instant is listed in the
  published index, so an interrupted login/logout can never strand an
  invisible credential in the OS keychain; the next write reconciles.
- A held lock's mtime is refreshed every 10s while the multi-command keyring
  operation runs, so the 30s stale-reclaim threshold only ever fires for a
  genuinely crashed holder, not a legitimately slow healthy one.
- A legacy combined entry that reappears after migration was written by an
  old binary still running; its unseen keys are merged before the entry is
  deleted, so mixed old/new versions during an upgrade no longer lose
  freshly saved tokens.
- Read-side locking is scoped to the keyring backend via a new
  blob.withReadLock: file-backend reads stay lock-free (writes are atomic
  renames), restoring crash tolerance when a writer dies holding the lock.
- The cross-process lock path falls back to the OS temp directory when no
  config location resolves, instead of silently degrading to in-process
  serialization only.
---
 internal/oauth/store.go              | 299 ++++++++++++++++++++----
 internal/oauth/store_keyring_test.go | 326 +++++++++++++++++++++++++++
 2 files changed, 578 insertions(+), 47 deletions(-)

diff --git a/internal/oauth/store.go b/internal/oauth/store.go
index ac985d957..496062e67 100644
--- a/internal/oauth/store.go
+++ b/internal/oauth/store.go
@@ -210,9 +210,12 @@ func NewStore(options StoreOptions) (*Store, error) {
 			kr = osKeyring
 		}
 		// Serialize the keyring's read-modify-write across processes with a lock
-		// file beside where the file backend would live. Best-effort: if no config
-		// location resolves, fall back to in-process serialization only.
-		lockPath := ""
+		// file beside where the file backend would live. Cross-process exclusion
+		// must not silently disappear when no config location resolves (withLock
+		// would be a no-op and a concurrent save could delete another process's
+		// newly written entry), so fall back to the OS temp directory, which
+		// always exists, rather than to in-process serialization only.
+		lockPath := filepath.Join(os.TempDir(), "zero-oauth-keyring.lockfile")
 		if storePath, perr := ResolveStorePath(options.Env); perr == nil {
 			lockPath = filepath.Join(filepath.Dir(storePath), "oauth-keyring.lockfile")
 		}
@@ -269,14 +272,15 @@ func (s *Store) Load(key string) (Token, bool, error) {
 	}
 	s.mu.Lock()
 	defer s.mu.Unlock()
-	// Through blob.withLock, like Save/Delete: the keyring backend's read is
-	// several separate Get calls (index, then each entry), not one atomic
-	// snapshot, so an unlocked Load can run concurrently with another
-	// process's Save/Delete mid write and observe a torn state (e.g. an index
-	// already updated but an entry not yet written). The lock keeps this read
-	// from overlapping any other process's read-modify-write cycle.
+	// Through blob.withReadLock: the keyring backend's read is several
+	// separate Get calls (index, then each entry), not one atomic snapshot,
+	// so an unguarded Load could run concurrently with another process's
+	// Save/Delete mid write and observe a torn state. The file backend's
+	// withReadLock is a no-op: its writes are atomic renames, so lock-free
+	// reads keep their crash tolerance (a crashed writer's fresh lock file
+	// must not block reads of the last complete file).
 	var state storeFile
-	err := s.blob.withLock(s.now, func() error {
+	err := s.blob.withReadLock(s.now, func() error {
 		var readErr error
 		state, readErr = s.readState()
 		return readErr
@@ -316,10 +320,11 @@ func (s *Store) Delete(key string) (bool, error) {
 func (s *Store) Status(prefix string) ([]Status, error) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
-	// Same reasoning as Load: run the read under blob.withLock so it can't
-	// observe another process's Save/Delete mid write.
+	// Same reasoning as Load: run the read under blob.withReadLock so the
+	// keyring's multi-entry read can't observe another process's Save/Delete
+	// mid write, while file-backend reads stay lock-free.
 	var state storeFile
-	err := s.blob.withLock(s.now, func() error {
+	err := s.blob.withReadLock(s.now, func() error {
 		var readErr error
 		state, readErr = s.readState()
 		return readErr
@@ -417,6 +422,13 @@ type blobStore interface {
 	// (a lock file for the file backend; none for the keyring, which is the
 	// authoritative store and is serialized within the process by Store.mu).
 	withLock(now func() time.Time, fn func() error) error
+	// withReadLock guards a read-only pass. The file backend's writes are
+	// atomic renames, so its reads stay lock-free: a crashed writer's fresh
+	// lock file must not turn into ~30s of read failures when the last
+	// complete file is perfectly readable. The keyring backend's read is
+	// several separate Get calls (index, then each entry), not one atomic
+	// snapshot, so it takes the same cross-process lock as its writes.
+	withReadLock(now func() time.Time, fn func() error) error
 	// location is a human-readable identifier for diagnostics/errors.
 	location() string
 }
@@ -460,6 +472,14 @@ func (b fileBlob) withLock(now func() time.Time, fn func() error) error {
 	return fn()
 }
 
+// withReadLock is deliberately lock-free: write() replaces the file with an
+// atomic rename, so a reader always sees a complete file, and a crashed
+// writer's leftover lock file must not turn readable state into ~30 seconds
+// of Load/Status failures while the stale threshold runs out.
+func (b fileBlob) withReadLock(now func() time.Time, fn func() error) error {
+	return fn()
+}
+
 func (b fileBlob) location() string { return b.path }
 
 // keyringBlob persists tokens in the OS keyring as one base64 entry per token
@@ -481,17 +501,13 @@ type keyringBlob struct {
 }
 
 func (b keyringBlob) read() ([]byte, bool, error) {
-	indexEnc, ok, err := b.kr.Get(b.service, b.indexAccount)
+	keys, ok, _, err := b.readKeyIndex()
 	if err != nil {
 		return nil, false, err
 	}
 	if !ok {
 		return b.readLegacy()
 	}
-	keys, err := decodeKeyIndex(indexEnc)
-	if err != nil {
-		return nil, false, fmt.Errorf("oauth: decode keyring token index: %w", err)
-	}
 	tokens := make(map[string]Token, len(keys))
 	for _, key := range keys {
 		enc, ok, err := b.kr.Get(b.service, key)
@@ -536,34 +552,87 @@ func (b keyringBlob) readLegacy() ([]byte, bool, error) {
 	return data, true, nil
 }
 
+// write replaces the keyring's token entries with state, ordered so that
+// every interruption boundary leaves a recoverable store. The invariant is
+// that any token entry existing in the keyring at any instant is listed in
+// the published index: the union index is published before entries are
+// written, entries are deleted before the index shrinks, and the index
+// header is only updated after the chunks it references exist. A crash at
+// any step therefore leaves either an index over-listing keys whose entries
+// are missing (read() already skips those) or entries that a later
+// read/write can still see and reconcile, never an invisible credential
+// stranded in the OS keychain.
 func (b keyringBlob) write(data []byte) error {
 	var state storeFile
 	if err := json.Unmarshal(data, &state); err != nil {
 		return fmt.Errorf("oauth: encode keyring token blob: %w", err)
 	}
-	priorKeys, err := b.indexedKeys()
+	priorKeys, indexExisted, priorChunks, err := b.readKeyIndex()
 	if err != nil {
 		return err
 	}
-	keys := make([]string, 0, len(state.Tokens))
-	for key, token := range state.Tokens {
-		raw, err := json.Marshal(token)
-		if err != nil {
-			return err
-		}
-		if err := b.kr.Set(b.service, key, base64.StdEncoding.EncodeToString(raw)); err != nil {
-			return err
+	prior := make(map[string]bool, len(priorKeys))
+	for _, key := range priorKeys {
+		prior[key] = true
+	}
+
+	// An older binary running alongside this one still reads and writes only
+	// the legacy combined entry. If that entry exists even though the index
+	// has already been published, it was recreated by such a binary after
+	// migration: merge any key the indexed schema has never seen before the
+	// legacy entry is deleted below, or that binary's freshly saved token
+	// would be silently lost. Keys already in the prior index are not merged;
+	// their absence from state means this write deliberately removed them.
+	if indexExisted {
+		if legacyData, ok, legacyErr := b.readLegacy(); legacyErr == nil && ok {
+			var legacyState storeFile
+			if json.Unmarshal(legacyData, &legacyState) == nil {
+				for key, token := range legacyState.Tokens {
+					if _, exists := state.Tokens[key]; exists || prior[key] || ValidateKey(key) != nil {
+						continue
+					}
+					state.Tokens[key] = token
+				}
+			}
 		}
+	}
+
+	keys := make([]string, 0, len(state.Tokens))
+	for key := range state.Tokens {
 		keys = append(keys, key)
 	}
 	sort.Strings(keys)
-	indexData, err := json.Marshal(keys)
+
+	// 1. Publish the union of the prior and new key sets first, so every
+	// entry that exists at any point during this update is indexed.
+	union := keys
+	if len(priorKeys) > 0 {
+		merged := make(map[string]bool, len(keys)+len(priorKeys))
+		for _, key := range append(append([]string{}, keys...), priorKeys...) {
+			merged[key] = true
+		}
+		union = make([]string, 0, len(merged))
+		for key := range merged {
+			union = append(union, key)
+		}
+		sort.Strings(union)
+	}
+	unionChunks, err := b.writeKeyIndex(union, priorChunks)
 	if err != nil {
 		return err
 	}
-	if err := b.kr.Set(b.service, b.indexAccount, base64.StdEncoding.EncodeToString(indexData)); err != nil {
-		return err
+	// 2. Write each token entry.
+	for _, key := range keys {
+		raw, err := json.Marshal(state.Tokens[key])
+		if err != nil {
+			return err
+		}
+		if err := b.kr.Set(b.service, key, base64.StdEncoding.EncodeToString(raw)); err != nil {
+			return err
+		}
 	}
+	// 3. Delete removed entries while the union index still lists them, so a
+	// failed Delete leaves a visible (re-deletable) entry, never an orphan.
 	for _, key := range priorKeys {
 		if _, ok := state.Tokens[key]; !ok {
 			if _, err := b.kr.Delete(b.service, key); err != nil {
@@ -571,41 +640,154 @@ func (b keyringBlob) write(data []byte) error {
 			}
 		}
 	}
+	// 4. Shrink the index to the exact new key set.
+	if _, err := b.writeKeyIndex(keys, unionChunks); err != nil {
+		return err
+	}
 	// The index now exists and is authoritative; drop the legacy entry so a
-	// future read never falls back to it.
+	// future read never falls back to it (its fresh writes were merged above).
 	_, _ = b.kr.Delete(b.service, b.legacyAccount)
 	return nil
 }
 
-// indexedKeys returns the keys currently listed in the index, or nil if there
-// is no index yet (first write, or still on the legacy format).
-func (b keyringBlob) indexedKeys() ([]string, error) {
+// maxKeyringIndexChunkBytes bounds one index chunk's raw JSON payload so its
+// base64 encoding plus command framing stays well under the macOS
+// `security -i` 4095-byte line cap (see internal/keyring): 2700 raw bytes
+// expand to 3600 base64 bytes, leaving ~490 bytes for the add-generic-password
+// syntax, service, and account. The old single-entry index hit that cap at
+// roughly 22 maximum-length keys even when every token was tiny.
+const maxKeyringIndexChunkBytes = 2700
+
+// keyIndexHeader is chunk 0 of the key index. Chunks 1..Chunks-1 live under
+// "<indexAccount>-<n>" as plain JSON string arrays. The pre-chunking format
+// (a bare JSON array at indexAccount) is still read transparently.
+type keyIndexHeader struct {
+	Version int      `json:"v"`
+	Chunks  int      `json:"chunks"`
+	Keys    []string `json:"keys"`
+}
+
+func (b keyringBlob) chunkAccount(index int) string {
+	return fmt.Sprintf("%s-%d", b.indexAccount, index)
+}
+
+// readKeyIndex returns the indexed keys, whether an index exists at all, and
+// how many chunk entries it currently occupies. A chunk listed by the header
+// but missing from the keyring (a torn write) is skipped, mirroring how
+// read() skips an indexed key whose entry is missing.
+func (b keyringBlob) readKeyIndex() ([]string, bool, int, error) {
 	enc, ok, err := b.kr.Get(b.service, b.indexAccount)
-	if err != nil || !ok {
-		return nil, err
+	if err != nil {
+		return nil, false, 0, err
+	}
+	if !ok {
+		return nil, false, 0, nil
 	}
-	keys, err := decodeKeyIndex(enc)
+	raw, err := base64.StdEncoding.DecodeString(strings.TrimSpace(enc))
 	if err != nil {
-		return nil, fmt.Errorf("oauth: decode keyring token index: %w", err)
+		return nil, false, 0, fmt.Errorf("oauth: decode keyring token index: %w", err)
+	}
+	trimmed := strings.TrimSpace(string(raw))
+	if strings.HasPrefix(trimmed, "[") {
+		var keys []string
+		if err := json.Unmarshal(raw, &keys); err != nil {
+			return nil, false, 0, fmt.Errorf("oauth: decode keyring token index: %w", err)
+		}
+		return keys, true, 1, nil
+	}
+	var header keyIndexHeader
+	if err := json.Unmarshal(raw, &header); err != nil {
+		return nil, false, 0, fmt.Errorf("oauth: decode keyring token index: %w", err)
+	}
+	keys := header.Keys
+	for i := 1; i < header.Chunks; i++ {
+		chunkEnc, ok, err := b.kr.Get(b.service, b.chunkAccount(i))
+		if err != nil {
+			return nil, false, 0, err
+		}
+		if !ok {
+			continue
+		}
+		chunkRaw, err := base64.StdEncoding.DecodeString(strings.TrimSpace(chunkEnc))
+		if err != nil {
+			return nil, false, 0, fmt.Errorf("oauth: decode keyring token index chunk %d: %w", i, err)
+		}
+		var more []string
+		if err := json.Unmarshal(chunkRaw, &more); err != nil {
+			return nil, false, 0, fmt.Errorf("oauth: decode keyring token index chunk %d: %w", i, err)
+		}
+		keys = append(keys, more...)
+	}
+	chunks := header.Chunks
+	if chunks < 1 {
+		chunks = 1
 	}
-	return keys, nil
+	return keys, true, chunks, nil
 }
 
-func decodeKeyIndex(enc string) ([]string, error) {
-	data, err := base64.StdEncoding.DecodeString(strings.TrimSpace(enc))
+// writeKeyIndex persists keys as a chunked index and reports how many chunk
+// entries it used. Continuation chunks are written before the header that
+// references them, so the authoritative chunk 0 never advertises a chunk that
+// does not exist yet; stale chunks from a previously larger index are removed
+// only after the header stops referencing them (best-effort: an unreferenced
+// chunk is never read).
+func (b keyringBlob) writeKeyIndex(keys []string, priorChunks int) (int, error) {
+	chunks := chunkIndexKeys(keys)
+	for i := 1; i < len(chunks); i++ {
+		chunkData, err := json.Marshal(chunks[i])
+		if err != nil {
+			return 0, err
+		}
+		if err := b.kr.Set(b.service, b.chunkAccount(i), base64.StdEncoding.EncodeToString(chunkData)); err != nil {
+			return 0, err
+		}
+	}
+	headerData, err := json.Marshal(keyIndexHeader{Version: 1, Chunks: len(chunks), Keys: chunks[0]})
 	if err != nil {
-		return nil, err
+		return 0, err
 	}
-	var keys []string
-	if err := json.Unmarshal(data, &keys); err != nil {
-		return nil, err
+	if err := b.kr.Set(b.service, b.indexAccount, base64.StdEncoding.EncodeToString(headerData)); err != nil {
+		return 0, err
+	}
+	for i := len(chunks); i < priorChunks; i++ {
+		_, _ = b.kr.Delete(b.service, b.chunkAccount(i))
 	}
-	return keys, nil
+	return len(chunks), nil
 }
 
+// chunkIndexKeys packs keys into chunks whose marshaled JSON stays under
+// maxKeyringIndexChunkBytes. Always returns at least one (possibly empty)
+// chunk.
+func chunkIndexKeys(keys []string) [][]string {
+	chunks := [][]string{{}}
+	size := 0
+	for _, key := range keys {
+		// Per-key JSON cost: quotes, comma, and headroom for escaping.
+		cost := len(key) + 8
+		if size+cost > maxKeyringIndexChunkBytes && len(chunks[len(chunks)-1]) > 0 {
+			chunks = append(chunks, []string{})
+			size = 0
+		}
+		chunks[len(chunks)-1] = append(chunks[len(chunks)-1], key)
+		size += cost
+	}
+	return chunks
+}
+
+// fileLockRefreshInterval is how often a held keyring lock's mtime is
+// refreshed while its critical section runs. It must stay comfortably under
+// fileLockStaleAfter (30s): one external keyring command may legitimately
+// take up to its 10s timeout and a multi-entry pass runs several, so without
+// refreshing, a healthy slow holder would look stale and another process
+// could reclaim the live lock and resume the token-loss race the lock
+// exists to prevent. A var so tests can shorten it.
+var fileLockRefreshInterval = 10 * time.Second
+
 // withLock serializes the keyring's read-modify-write. Store.mu covers the
 // in-process case; lockPath (when set) adds cross-process exclusion so two
 // processes can't both read the blob, modify, and write — dropping a token.
+// While fn runs, the lock file's mtime is refreshed so the stale-reclaim
+// threshold only ever expires for a genuinely crashed holder.
 func (b keyringBlob) withLock(now func() time.Time, fn func() error) error {
 	if b.lockPath == "" {
 		return fn()
@@ -615,7 +797,30 @@ func (b keyringBlob) withLock(now func() time.Time, fn func() error) error {
 		return err
 	}
 	defer unlock()
-	return fn()
+	stop := make(chan struct{})
+	done := make(chan struct{})
+	go func() {
+		defer close(done)
+		ticker := time.NewTicker(fileLockRefreshInterval)
+		defer ticker.Stop()
+		for {
+			select {
+			case <-stop:
+				return
+			case <-ticker.C:
+				at := now()
+				_ = os.Chtimes(b.lockPath, at, at)
+			}
+		}
+	}()
+	err = fn()
+	close(stop)
+	<-done
+	return err
+}
+
+func (b keyringBlob) withReadLock(now func() time.Time, fn func() error) error {
+	return b.withLock(now, fn)
 }
 
 func (b keyringBlob) location() string { return "keyring:" + b.service + "/" + b.indexAccount }
diff --git a/internal/oauth/store_keyring_test.go b/internal/oauth/store_keyring_test.go
index 34bf945a4..6d6eaf096 100644
--- a/internal/oauth/store_keyring_test.go
+++ b/internal/oauth/store_keyring_test.go
@@ -3,8 +3,11 @@ package oauth
 import (
 	"encoding/base64"
 	"encoding/json"
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
+	"time"
 )
 
 // fakeKR is an in-memory KeyringClient for exercising the keyring backend
@@ -221,6 +224,258 @@ func TestStoreKeyringSkipsIndexedKeyMissingItsEntry(t *testing.T) {
 	}
 }
 
+// failingKR wraps fakeKR and fails the Nth mutating operation (Set/Delete),
+// for exercising every interruption boundary of the multi-step write.
+type failingKR struct {
+	*fakeKR
+	failAt int // 1-based mutating-operation number to fail; 0 disables
+	ops    int
+}
+
+func (f *failingKR) Set(service, account, secret string) error {
+	f.ops++
+	if f.failAt != 0 && f.ops == f.failAt {
+		return errKRInjected
+	}
+	return f.fakeKR.Set(service, account, secret)
+}
+
+func (f *failingKR) Delete(service, account string) (bool, error) {
+	f.ops++
+	if f.failAt != 0 && f.ops == f.failAt {
+		return false, errKRInjected
+	}
+	return f.fakeKR.Delete(service, account)
+}
+
+var errKRInjected = errKR("injected keyring failure")
+
+type errKR string
+
+func (e errKR) Error() string { return string(e) }
+
+// indexedKeysOf parses the (possibly chunked) index in kr and returns every
+// listed key.
+func indexedKeysOf(t *testing.T, kr *fakeKR) map[string]bool {
+	t.Helper()
+	blob := keyringBlob{kr: kr, service: keyringService, legacyAccount: keyringLegacyAccount, indexAccount: keyringIndexAccount}
+	keys, _, _, err := blob.readKeyIndex()
+	if err != nil {
+		t.Fatalf("readKeyIndex: %v", err)
+	}
+	out := make(map[string]bool, len(keys))
+	for _, k := range keys {
+		out[k] = true
+	}
+	return out
+}
+
+// TestStoreKeyringIndexStaysUnderEntryLimit is the regression test for the
+// index itself hitting the same macOS `security -i` line cap the per-token
+// split fixed for token entries: with enough maximum-length keys, a single
+// index entry base64-expands past 4095 bytes even when every token is tiny.
+// The index must therefore be bounded per entry (chunked) like everything
+// else, and still round-trip.
+func TestStoreKeyringIndexStaysUnderEntryLimit(t *testing.T) {
+	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
+	kr := newFakeKR()
+	s, err := NewStore(StoreOptions{Storage: "keyring", Keyring: kr})
+	if err != nil {
+		t.Fatal(err)
+	}
+	// 40 keys near ValidateKey's cap: an unchunked index of these would
+	// serialize to ~5.5KB before base64.
+	names := make([]string, 0, 40)
+	for i := 0; i < 40; i++ {
+		names = append(names, strings.Repeat("p", 100)+"-"+strings.Repeat("0123456789", 2)+string(rune('a'+i%26))+string(rune('a'+i/26)))
+	}
+	for _, name := range names {
+		if err := s.Save(ProviderKey(name), Token{AccessToken: "a"}); err != nil {
+			t.Fatalf("Save(%s): %v", name, err)
+		}
+	}
+	// Every keyring value, index entries included, must stay under the cap
+	// with generous framing margin.
+	const entryCeiling = 3800
+	for k, v := range kr.data {
+		if len(v) > entryCeiling {
+			t.Fatalf("keyring entry %q is %d bytes, want <= %d (index cap regression)", k, len(v), entryCeiling)
+		}
+	}
+	// The index actually chunked (otherwise the ceiling check proves nothing).
+	if _, ok := kr.data[keyringService+"/"+keyringIndexAccount+"-1"]; !ok {
+		t.Fatal("expected the index to split into continuation chunks")
+	}
+	for _, name := range names {
+		if _, ok, err := s.Load(ProviderKey(name)); err != nil || !ok {
+			t.Fatalf("Load(%s): ok=%v err=%v", name, ok, err)
+		}
+	}
+	// Shrinking back to one token must also shrink the index and drop the
+	// stale continuation chunks.
+	for _, name := range names[1:] {
+		if _, err := s.Delete(ProviderKey(name)); err != nil {
+			t.Fatalf("Delete(%s): %v", name, err)
+		}
+	}
+	if _, ok := kr.data[keyringService+"/"+keyringIndexAccount+"-1"]; ok {
+		t.Fatal("stale index continuation chunk left behind after shrink")
+	}
+}
+
+// TestStoreKeyringWriteInterruptionsLeaveNoInvisibleTokens drives a write
+// through an injected failure at every mutating operation in turn and checks
+// the recoverable-store invariant at each boundary: every token entry present
+// in the keyring is listed in the published index (so no credential is ever
+// stranded invisibly), and a subsequent unimpeded write fully reconciles.
+func TestStoreKeyringWriteInterruptionsLeaveNoInvisibleTokens(t *testing.T) {
+	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
+	for failAt := 1; ; failAt++ {
+		kr := &failingKR{fakeKR: newFakeKR()}
+		s, err := NewStore(StoreOptions{Storage: "keyring", Keyring: kr})
+		if err != nil {
+			t.Fatal(err)
+		}
+		// Seed two tokens cleanly, then fail the Nth mutating operation of a
+		// write that both adds a token and (via the later delete pass of a
+		// Delete call) removes one.
+		if err := s.Save(ProviderKey("alpha"), Token{AccessToken: "a"}); err != nil {
+			t.Fatal(err)
+		}
+		if err := s.Save(ProviderKey("beta"), Token{AccessToken: "b"}); err != nil {
+			t.Fatal(err)
+		}
+		kr.ops = 0
+		kr.failAt = failAt
+		saveErr := s.Save(ProviderKey("gamma"), Token{AccessToken: "c"})
+		opsUsed := kr.ops
+		kr.failAt = 0
+
+		// Invariant at the interruption boundary: nothing invisible.
+		indexed := indexedKeysOf(t, kr.fakeKR)
+		for entry := range kr.data {
+			account := strings.TrimPrefix(entry, keyringService+"/")
+			if account == keyringIndexAccount || strings.HasPrefix(account, keyringIndexAccount+"-") || account == keyringLegacyAccount {
+				continue
+			}
+			if !indexed[account] {
+				t.Fatalf("failAt=%d: token entry %q exists but is not listed in the index (invisible credential)", failAt, account)
+			}
+		}
+
+		// A later unimpeded write must reconcile completely.
+		if err := s.Save(ProviderKey("gamma"), Token{AccessToken: "c"}); err != nil {
+			t.Fatalf("failAt=%d: reconciling Save: %v", failAt, err)
+		}
+		for _, name := range []string{"alpha", "beta", "gamma"} {
+			if _, ok, err := s.Load(ProviderKey(name)); err != nil || !ok {
+				t.Fatalf("failAt=%d: Load(%s) after reconcile: ok=%v err=%v", failAt, name, ok, err)
+			}
+		}
+		// saveErr itself is not asserted: most boundaries surface the injected
+		// failure, but the final legacy-entry delete is deliberately
+		// best-effort, so its failure is swallowed by design. The invariant
+		// and the reconcile above are the actual contract.
+		_ = saveErr
+		if opsUsed < failAt {
+			// The write used fewer mutating ops than failAt, so the injection
+			// never fired and every boundary has been covered.
+			break
+		}
+	}
+}
+
+// TestStoreKeyringDeleteInterruptionsLeaveNoInvisibleTokens is the Delete
+// counterpart: a logout interrupted at any boundary must not leave a
+// logged-out credential invisibly resident in the OS keychain (the index is
+// only shrunk after the entry deletion), and a repeated delete reconciles.
+func TestStoreKeyringDeleteInterruptionsLeaveNoInvisibleTokens(t *testing.T) {
+	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
+	for failAt := 1; ; failAt++ {
+		kr := &failingKR{fakeKR: newFakeKR()}
+		s, err := NewStore(StoreOptions{Storage: "keyring", Keyring: kr})
+		if err != nil {
+			t.Fatal(err)
+		}
+		if err := s.Save(ProviderKey("alpha"), Token{AccessToken: "a"}); err != nil {
+			t.Fatal(err)
+		}
+		if err := s.Save(ProviderKey("beta"), Token{AccessToken: "b"}); err != nil {
+			t.Fatal(err)
+		}
+		kr.ops = 0
+		kr.failAt = failAt
+		_, _ = s.Delete(ProviderKey("beta"))
+		opsUsed := kr.ops
+		kr.failAt = 0
+
+		indexed := indexedKeysOf(t, kr.fakeKR)
+		for entry := range kr.data {
+			account := strings.TrimPrefix(entry, keyringService+"/")
+			if account == keyringIndexAccount || strings.HasPrefix(account, keyringIndexAccount+"-") || account == keyringLegacyAccount {
+				continue
+			}
+			if !indexed[account] {
+				t.Fatalf("failAt=%d: token entry %q exists but is not listed in the index (invisible credential)", failAt, account)
+			}
+		}
+
+		// Retrying the delete must fully reconcile: beta gone from both the
+		// index and the keyring, alpha intact.
+		if _, err := s.Delete(ProviderKey("beta")); err != nil {
+			t.Fatalf("failAt=%d: reconciling Delete: %v", failAt, err)
+		}
+		if _, ok := kr.data[keyringService+"/"+ProviderKey("beta")]; ok {
+			t.Fatalf("failAt=%d: logged-out credential still resident after reconcile", failAt)
+		}
+		if _, ok, err := s.Load(ProviderKey("alpha")); err != nil || !ok {
+			t.Fatalf("failAt=%d: Load(alpha): ok=%v err=%v", failAt, ok, err)
+		}
+		if opsUsed < failAt {
+			break
+		}
+	}
+}
+
+// TestStoreKeyringMergesFreshLegacyWriteFromOldBinary covers the mixed-version
+// window: after migration to the indexed format, an old binary still running
+// can save a token into the legacy combined entry. The next new-binary write
+// must merge that fresh token instead of deleting the legacy entry over it.
+func TestStoreKeyringMergesFreshLegacyWriteFromOldBinary(t *testing.T) {
+	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
+	kr := newFakeKR()
+	s, err := NewStore(StoreOptions{Storage: "keyring", Keyring: kr})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := s.Save(ProviderKey("alpha"), Token{AccessToken: "a"}); err != nil {
+		t.Fatal(err)
+	}
+
+	// An old binary saves token "carol" through the legacy combined entry.
+	legacy := storeFile{SchemaVersion: storeSchemaVersion, Tokens: map[string]Token{
+		ProviderKey("carol"): {AccessToken: "c", RefreshToken: "cr"},
+	}}
+	data, err := json.Marshal(legacy)
+	if err != nil {
+		t.Fatal(err)
+	}
+	kr.data[keyringService+"/"+keyringLegacyAccount] = base64.StdEncoding.EncodeToString(data)
+
+	// The next new-binary save must keep carol, not silently lose it.
+	if err := s.Save(ProviderKey("beta"), Token{AccessToken: "b"}); err != nil {
+		t.Fatal(err)
+	}
+	for _, name := range []string{"alpha", "beta", "carol"} {
+		if _, ok, err := s.Load(ProviderKey(name)); err != nil || !ok {
+			t.Fatalf("Load(%s): ok=%v err=%v (fresh legacy write lost)", name, ok, err)
+		}
+	}
+	if _, ok := kr.data[keyringService+"/"+keyringLegacyAccount]; ok {
+		t.Fatal("legacy entry should be removed once its fresh writes are merged")
+	}
+}
+
 func TestNewStoreStorageSelection(t *testing.T) {
 	// Unknown storage is rejected (fail closed).
 	if _, err := NewStore(StoreOptions{Storage: "bogus"}); err == nil {
@@ -247,6 +502,77 @@ func TestNewStoreStorageSelection(t *testing.T) {
 	}
 }
 
+// TestStoreKeyringWithLockRefreshesLease guards the stale-reclaim race: one
+// keyring command can take up to 10s and a multi-entry pass runs several, so
+// a lock held for a legitimately slow operation can outlive the fixed 30s
+// stale threshold. withLock must keep the lock file's mtime fresh while its
+// critical section runs, so only a genuinely crashed holder ever looks stale.
+func TestStoreKeyringWithLockRefreshesLease(t *testing.T) {
+	lockPath := filepath.Join(t.TempDir(), "oauth-keyring.lockfile")
+	blob := keyringBlob{kr: newFakeKR(), service: "zero-test", indexAccount: "idx", lockPath: lockPath}
+
+	previous := fileLockRefreshInterval
+	fileLockRefreshInterval = 20 * time.Millisecond
+	defer func() { fileLockRefreshInterval = previous }()
+
+	var first, second time.Time
+	err := blob.withLock(time.Now, func() error {
+		info, err := os.Stat(lockPath)
+		if err != nil {
+			return err
+		}
+		first = info.ModTime()
+		time.Sleep(150 * time.Millisecond)
+		info, err = os.Stat(lockPath)
+		if err != nil {
+			return err
+		}
+		second = info.ModTime()
+		return nil
+	})
+	if err != nil {
+		t.Fatalf("withLock: %v", err)
+	}
+	if !second.After(first) {
+		t.Fatalf("lock mtime was not refreshed during the critical section: %v then %v", first, second)
+	}
+	if _, err := os.Stat(lockPath); !os.IsNotExist(err) {
+		t.Fatalf("lock file not released: %v", err)
+	}
+}
+
+// TestStoreFileLoadToleratesCrashedWriterLock: file-backend reads must stay
+// lock-free. A writer that crashed after taking the lock leaves a fresh lock
+// file behind; the store file itself is always complete (writes are atomic
+// renames), so Load must read it rather than waiting out the lock and
+// failing for the ~30 seconds the stale threshold takes to expire.
+func TestStoreFileLoadToleratesCrashedWriterLock(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "oauth-tokens.json")
+	s, err := NewStore(StoreOptions{FilePath: path})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := s.Save(ProviderKey("demo"), Token{AccessToken: "a"}); err != nil {
+		t.Fatal(err)
+	}
+	// Simulate the crashed writer: a fresh, never-released lock file.
+	if err := os.WriteFile(path+".lockfile", []byte("someone-else"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	start := time.Now()
+	got, ok, err := s.Load(ProviderKey("demo"))
+	if err != nil || !ok || got.AccessToken != "a" {
+		t.Fatalf("Load behind a crashed writer's lock: ok=%v err=%v token=%#v", ok, err, got)
+	}
+	if elapsed := time.Since(start); elapsed > 2*time.Second {
+		t.Fatalf("Load waited on the write lock (%v); reads must be lock-free", elapsed)
+	}
+	statuses, err := s.Status("")
+	if err != nil || len(statuses) != 1 {
+		t.Fatalf("Status behind a crashed writer's lock: %v (%d entries)", err, len(statuses))
+	}
+}
+
 func TestStoreKeyringStatus(t *testing.T) {
 	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
 	kr := newFakeKR()

From 44aff1af5e41612270222037c6289cdf7ad34e24 Mon Sep 17 00:00:00 2001
From: euxaristia <25621994+euxaristia@users.noreply.github.com>
Date: Wed, 15 Jul 2026 14:50:09 -0400
Subject: [PATCH 04/17] test(oauth): assert Status also stays lock-free behind
 a crashed writer's lock

---
 internal/oauth/store_keyring_test.go | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/internal/oauth/store_keyring_test.go b/internal/oauth/store_keyring_test.go
index 6d6eaf096..2229f659f 100644
--- a/internal/oauth/store_keyring_test.go
+++ b/internal/oauth/store_keyring_test.go
@@ -567,10 +567,14 @@ func TestStoreFileLoadToleratesCrashedWriterLock(t *testing.T) {
 	if elapsed := time.Since(start); elapsed > 2*time.Second {
 		t.Fatalf("Load waited on the write lock (%v); reads must be lock-free", elapsed)
 	}
+	statusStart := time.Now()
 	statuses, err := s.Status("")
 	if err != nil || len(statuses) != 1 {
 		t.Fatalf("Status behind a crashed writer's lock: %v (%d entries)", err, len(statuses))
 	}
+	if elapsed := time.Since(statusStart); elapsed > 2*time.Second {
+		t.Fatalf("Status waited on the write lock (%v); reads must be lock-free", elapsed)
+	}
 }
 
 func TestStoreKeyringStatus(t *testing.T) {

From 2a27743689c997e80abd55f935188ec63b71e587 Mon Sep 17 00:00:00 2001
From: euxaristia <25621994+euxaristia@users.noreply.github.com>
Date: Thu, 16 Jul 2026 21:55:58 -0400
Subject: [PATCH 05/17] fix(oauth): recover legacy tokens across an interrupted
 keyring migration

During the initial legacy->indexed migration, write() publishes the index
before the per-key entries and deletes the legacy combined entry only as the
final step. A crash after the index appears but before an entry is written
previously left that pre-existing credential unreadable: the index listed the
key, but its own entry did not exist yet.

read() now falls back to the still-present legacy blob for any indexed key
whose own entry is missing, so a migration interrupted at any point keeps every
token readable, and a following unimpeded save completes the migration. write()
also reconciles a concurrent old-binary refresh: when the legacy blob holds a
strictly later expiry for an already-indexed key, that fresher value wins
instead of being overwritten by the stale indexed copy and then deleted.
---
 internal/oauth/store.go              |  96 ++++++++++++++++++------
 internal/oauth/store_keyring_test.go | 107 +++++++++++++++++++++++++++
 2 files changed, 182 insertions(+), 21 deletions(-)

diff --git a/internal/oauth/store.go b/internal/oauth/store.go
index 496062e67..8c5d4635c 100644
--- a/internal/oauth/store.go
+++ b/internal/oauth/store.go
@@ -508,6 +508,14 @@ func (b keyringBlob) read() ([]byte, bool, error) {
 	if !ok {
 		return b.readLegacy()
 	}
+	// The legacy combined entry is consulted lazily (below) only when an indexed
+	// key's own entry is missing. write() publishes the index before the per-key
+	// entries and deletes the legacy blob only after every entry is written, so a
+	// crash partway through the initial legacy->indexed migration can leave a
+	// pre-existing credential readable solely in the still-present legacy blob.
+	// In steady state (all entries present) the legacy blob is never read.
+	var legacyTokens map[string]Token
+	legacyLoaded := false
 	tokens := make(map[string]Token, len(keys))
 	for _, key := range keys {
 		enc, ok, err := b.kr.Get(b.service, key)
@@ -515,9 +523,18 @@ func (b keyringBlob) read() ([]byte, bool, error) {
 			return nil, false, err
 		}
 		if !ok {
-			// Index and entries fell out of sync (e.g. a killed process between
-			// writing an entry and updating the index); skip rather than fail the
-			// whole read, since the next Save/Delete will reconcile the index.
+			// The index lists this key but its own entry is missing. Recover it
+			// from the legacy blob when a migration is still in flight; otherwise
+			// (a steady-state index/entry desync whose legacy blob is already
+			// gone) skip rather than fail the whole read, since the next
+			// Save/Delete will reconcile the index.
+			if !legacyLoaded {
+				legacyTokens = b.readLegacyTokens()
+				legacyLoaded = true
+			}
+			if token, has := legacyTokens[key]; has {
+				tokens[key] = token
+			}
 			continue
 		}
 		raw, err := base64.StdEncoding.DecodeString(strings.TrimSpace(enc))
@@ -552,6 +569,32 @@ func (b keyringBlob) readLegacy() ([]byte, bool, error) {
 	return data, true, nil
 }
 
+// readLegacyTokens returns the tokens held in the legacy combined entry, or an
+// empty map when there is no readable legacy blob. It is a best-effort recovery
+// source (read() falls back to it, write() reconciles against it), so a missing
+// or malformed legacy entry is reported as "no tokens" rather than a hard error.
+func (b keyringBlob) readLegacyTokens() map[string]Token {
+	data, ok, err := b.readLegacy()
+	if err != nil || !ok {
+		return nil
+	}
+	var legacyState storeFile
+	if json.Unmarshal(data, &legacyState) != nil {
+		return nil
+	}
+	return legacyState.Tokens
+}
+
+// legacyIsFresher reports whether the legacy copy of an already-indexed key
+// should win over the indexed copy. An old binary running alongside the new one
+// refreshes tokens only in the legacy combined entry, and a refresh pushes the
+// expiry later, so a strictly later, non-zero expiry on the legacy side is the
+// signal that it holds a newer credential. A zero (unknown) expiry on either
+// side is not evidence of freshness, so the indexed value is kept.
+func legacyIsFresher(legacy, current Token) bool {
+	return !legacy.ExpiresAt.IsZero() && !current.ExpiresAt.IsZero() && legacy.ExpiresAt.After(current.ExpiresAt)
+}
+
 // write replaces the keyring's token entries with state, ordered so that
 // every interruption boundary leaves a recoverable store. The invariant is
 // that any token entry existing in the keyring at any instant is listed in
@@ -559,9 +602,11 @@ func (b keyringBlob) readLegacy() ([]byte, bool, error) {
 // written, entries are deleted before the index shrinks, and the index
 // header is only updated after the chunks it references exist. A crash at
 // any step therefore leaves either an index over-listing keys whose entries
-// are missing (read() already skips those) or entries that a later
-// read/write can still see and reconcile, never an invisible credential
-// stranded in the OS keychain.
+// are missing (read() recovers those from the legacy blob during a migration,
+// or skips them once it is gone) or entries that a later read/write can still
+// see and reconcile, never an invisible credential stranded in the OS keychain.
+// The legacy combined entry is the durable fallback for the initial migration
+// and is deleted only as the final step, after every per-key entry is written.
 func (b keyringBlob) write(data []byte) error {
 	var state storeFile
 	if err := json.Unmarshal(data, &state); err != nil {
@@ -576,24 +621,33 @@ func (b keyringBlob) write(data []byte) error {
 		prior[key] = true
 	}
 
-	// An older binary running alongside this one still reads and writes only
-	// the legacy combined entry. If that entry exists even though the index
-	// has already been published, it was recreated by such a binary after
-	// migration: merge any key the indexed schema has never seen before the
-	// legacy entry is deleted below, or that binary's freshly saved token
-	// would be silently lost. Keys already in the prior index are not merged;
-	// their absence from state means this write deliberately removed them.
+	// An older binary running alongside this one still reads and writes only the
+	// legacy combined entry. If that entry exists even though the index has
+	// already been published, an old binary wrote it after migration, so
+	// reconcile it into state before it is deleted below rather than blindly
+	// overwriting it:
+	//   - a key the indexed schema has never seen is a fresh old-binary login;
+	//     merge it so it is not lost;
+	//   - a key already present in state that the legacy blob refreshed (a
+	//     strictly later expiry) takes the legacy value, so a concurrent
+	//     old-binary refresh is not discarded in favor of the stale indexed one;
+	//   - a key that was in the prior index but is absent from this write was
+	//     deliberately removed (a logout); it is left removed, not resurrected.
 	if indexExisted {
-		if legacyData, ok, legacyErr := b.readLegacy(); legacyErr == nil && ok {
-			var legacyState storeFile
-			if json.Unmarshal(legacyData, &legacyState) == nil {
-				for key, token := range legacyState.Tokens {
-					if _, exists := state.Tokens[key]; exists || prior[key] || ValidateKey(key) != nil {
-						continue
-					}
-					state.Tokens[key] = token
+		for key, legacyToken := range b.readLegacyTokens() {
+			if ValidateKey(key) != nil {
+				continue
+			}
+			if current, exists := state.Tokens[key]; exists {
+				if legacyIsFresher(legacyToken, current) {
+					state.Tokens[key] = legacyToken
 				}
+				continue
+			}
+			if prior[key] {
+				continue
 			}
+			state.Tokens[key] = legacyToken
 		}
 	}
 
diff --git a/internal/oauth/store_keyring_test.go b/internal/oauth/store_keyring_test.go
index 2229f659f..7072bf23a 100644
--- a/internal/oauth/store_keyring_test.go
+++ b/internal/oauth/store_keyring_test.go
@@ -595,3 +595,110 @@ func TestStoreKeyringStatus(t *testing.T) {
 		t.Fatalf("status = %#v", statuses)
 	}
 }
+
+// TestStoreKeyringMigrationInterruptionsPreserveLegacyTokens drives the initial
+// legacy->indexed migration through an injected failure at every mutating
+// operation and checks that no pre-existing legacy credential is ever lost.
+// write() publishes the index before the per-key entries, so a crash after the
+// index appears but before an entry is written must still leave that token
+// readable in the not-yet-deleted legacy blob; read() recovers it, and a
+// following unimpeded save completes the migration.
+func TestStoreKeyringMigrationInterruptionsPreserveLegacyTokens(t *testing.T) {
+	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
+	seeded := map[string]Token{
+		ProviderKey("demo"):  {AccessToken: "demo-a", RefreshToken: "demo-r"},
+		ProviderKey("other"): {AccessToken: "other-a"},
+	}
+	for failAt := 1; ; failAt++ {
+		kr := &failingKR{fakeKR: newFakeKR()}
+		// A legacy-only install: one combined entry, no index yet.
+		legacyData, err := json.Marshal(storeFile{SchemaVersion: storeSchemaVersion, Tokens: seeded})
+		if err != nil {
+			t.Fatal(err)
+		}
+		kr.data[keyringService+"/"+keyringLegacyAccount] = base64.StdEncoding.EncodeToString(legacyData)
+
+		s, err := NewStore(StoreOptions{Storage: "keyring", Keyring: kr})
+		if err != nil {
+			t.Fatal(err)
+		}
+		kr.ops = 0
+		kr.failAt = failAt
+		_ = s.Save(ProviderKey("new"), Token{AccessToken: "new-c"})
+		opsUsed := kr.ops
+		kr.failAt = 0
+
+		// Regardless of where the migration was interrupted, a subsequent
+		// unimpeded save must complete it with every token intact.
+		if err := s.Save(ProviderKey("new"), Token{AccessToken: "new-c"}); err != nil {
+			t.Fatalf("failAt=%d: reconciling Save: %v", failAt, err)
+		}
+		for key, want := range seeded {
+			got, ok, err := s.Load(key)
+			if err != nil || !ok {
+				t.Fatalf("failAt=%d: Load(%s) after migration: ok=%v err=%v (legacy token lost)", failAt, key, ok, err)
+			}
+			if got.AccessToken != want.AccessToken {
+				t.Fatalf("failAt=%d: Load(%s) = %q, want %q", failAt, key, got.AccessToken, want.AccessToken)
+			}
+		}
+		if got, ok, err := s.Load(ProviderKey("new")); err != nil || !ok || got.AccessToken != "new-c" {
+			t.Fatalf("failAt=%d: Load(new): ok=%v err=%v token=%#v", failAt, ok, err, got)
+		}
+		// The completed migration drops the legacy entry.
+		if _, ok := kr.data[keyringService+"/"+keyringLegacyAccount]; ok {
+			t.Fatalf("failAt=%d: legacy entry not removed after migration completed", failAt)
+		}
+		if opsUsed < failAt {
+			break
+		}
+	}
+}
+
+// TestStoreKeyringMergesFreshLegacyRefreshOfIndexedKey covers the mixed-version
+// window for a key that already exists in the index: an old binary refreshes
+// provider:alpha in the legacy combined entry (a strictly later expiry). The
+// next new-binary save must keep that fresher refresh instead of overwriting it
+// with the stale indexed value and then deleting the legacy entry.
+func TestStoreKeyringMergesFreshLegacyRefreshOfIndexedKey(t *testing.T) {
+	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
+	kr := newFakeKR()
+	s, err := NewStore(StoreOptions{Storage: "keyring", Keyring: kr})
+	if err != nil {
+		t.Fatal(err)
+	}
+	stale := time.Now().Add(1 * time.Hour)
+	if err := s.Save(ProviderKey("alpha"), Token{AccessToken: "a-old", RefreshToken: "r-old", ExpiresAt: stale}); err != nil {
+		t.Fatal(err)
+	}
+
+	// An old binary refreshes alpha through the legacy combined entry, pushing
+	// the expiry later than the indexed copy.
+	fresh := stale.Add(1 * time.Hour)
+	legacy := storeFile{SchemaVersion: storeSchemaVersion, Tokens: map[string]Token{
+		ProviderKey("alpha"): {AccessToken: "a-new", RefreshToken: "r-new", ExpiresAt: fresh},
+	}}
+	legacyData, err := json.Marshal(legacy)
+	if err != nil {
+		t.Fatal(err)
+	}
+	kr.data[keyringService+"/"+keyringLegacyAccount] = base64.StdEncoding.EncodeToString(legacyData)
+
+	// A new-binary save of an unrelated key must reconcile alpha, not clobber it.
+	if err := s.Save(ProviderKey("beta"), Token{AccessToken: "b"}); err != nil {
+		t.Fatal(err)
+	}
+	got, ok, err := s.Load(ProviderKey("alpha"))
+	if err != nil || !ok {
+		t.Fatalf("Load(alpha): ok=%v err=%v", ok, err)
+	}
+	if got.AccessToken != "a-new" || got.RefreshToken != "r-new" {
+		t.Fatalf("Load(alpha) = %#v, want the refreshed legacy value (fresh refresh discarded)", got)
+	}
+	if _, ok, _ := s.Load(ProviderKey("beta")); !ok {
+		t.Fatal("Load(beta): not stored")
+	}
+	if _, ok := kr.data[keyringService+"/"+keyringLegacyAccount]; ok {
+		t.Fatal("legacy entry should be removed once its refresh is merged")
+	}
+}

From 527846e8c92bdff31d30f7acf0c206c2b50c12af Mon Sep 17 00:00:00 2001
From: euxaristia <25621994+euxaristia@users.noreply.github.com>
Date: Thu, 16 Jul 2026 21:56:11 -0400
Subject: [PATCH 06/17] fix(oauth): lease the keyring lock with wall-clock time

The lock lease renewal stamped the live lock file using the injectable
StoreOptions.Now, but a caller may fix or freeze that clock (for example to
drive token-expiry tests). acquireFileLock judges lock staleness with real
time.Since(mtime), so leasing with a frozen or backdated clock let another
process treat a held lock as stale and reclaim it mid-operation, reviving the
token-loss race the lease exists to prevent. Lease with time.Now() regardless
of the store clock.
---
 internal/oauth/store.go              |  7 +++++-
 internal/oauth/store_keyring_test.go | 34 ++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/internal/oauth/store.go b/internal/oauth/store.go
index 8c5d4635c..02a9a0b1f 100644
--- a/internal/oauth/store.go
+++ b/internal/oauth/store.go
@@ -862,7 +862,12 @@ func (b keyringBlob) withLock(now func() time.Time, fn func() error) error {
 			case <-stop:
 				return
 			case <-ticker.C:
-				at := now()
+				// Lease with wall-clock time, never the injectable now: acquireFileLock
+				// judges staleness with real time.Since(mtime), so a fixed or stale
+				// StoreOptions.Now would stamp the live lock with an old mtime that
+				// another process would immediately reclaim, reviving the token-loss
+				// race this lease prevents.
+				at := time.Now()
 				_ = os.Chtimes(b.lockPath, at, at)
 			}
 		}
diff --git a/internal/oauth/store_keyring_test.go b/internal/oauth/store_keyring_test.go
index 7072bf23a..9a8372cf7 100644
--- a/internal/oauth/store_keyring_test.go
+++ b/internal/oauth/store_keyring_test.go
@@ -702,3 +702,37 @@ func TestStoreKeyringMergesFreshLegacyRefreshOfIndexedKey(t *testing.T) {
 		t.Fatal("legacy entry should be removed once its refresh is merged")
 	}
 }
+
+// TestStoreKeyringLeaseUsesWallClockNotStoreClock guards the lock lease against
+// a fixed or stale StoreOptions.Now. acquireFileLock judges staleness with real
+// time.Since(mtime), so the lease must stamp the live lock with wall-clock time;
+// leasing with an old injectable clock would let a peer immediately reclaim the
+// held lock and re-enter the keyring read-modify-write concurrently.
+func TestStoreKeyringLeaseUsesWallClockNotStoreClock(t *testing.T) {
+	lockPath := filepath.Join(t.TempDir(), "oauth-keyring.lockfile")
+	blob := keyringBlob{kr: newFakeKR(), service: "zero-test", indexAccount: "idx", lockPath: lockPath}
+
+	previous := fileLockRefreshInterval
+	fileLockRefreshInterval = 20 * time.Millisecond
+	defer func() { fileLockRefreshInterval = previous }()
+
+	// A deliberately stale, fixed clock: if the lease used it, the lock mtime
+	// would land decades in the past and look stale immediately.
+	fixed := time.Date(2000, 1, 1, 0, 0, 0, 0, time.UTC)
+	var mtime time.Time
+	err := blob.withLock(func() time.Time { return fixed }, func() error {
+		time.Sleep(150 * time.Millisecond)
+		info, statErr := os.Stat(lockPath)
+		if statErr != nil {
+			return statErr
+		}
+		mtime = info.ModTime()
+		return nil
+	})
+	if err != nil {
+		t.Fatalf("withLock: %v", err)
+	}
+	if time.Since(mtime) > fileLockStaleAfter {
+		t.Fatalf("lease stamped the lock with the store clock (%v); a peer would reclaim the live lock", mtime)
+	}
+}

From cedc33e8a2a935cb6e6ca69884b5d298202a62aa Mon Sep 17 00:00:00 2001
From: euxaristia <25621994+euxaristia@users.noreply.github.com>
Date: Thu, 16 Jul 2026 21:56:22 -0400
Subject: [PATCH 07/17] fix(oauth): bound the keyring index chunk count before
 reading

readKeyIndex trusted the stored header's advertised chunk count and issued one
keyring lookup per chunk. A corrupt or hostile header claiming a huge count
(for example {"v":1,"chunks":1000000000}) would fan out into that many blocking
keyring lookups, each up to the command timeout, while holding the store lock,
wedging every Load/Status/Save/Delete instead of failing promptly. Reject an
unsupported index version or an out-of-range chunk count (1..128) up front,
before the read loop.
---
 internal/oauth/store.go              | 24 ++++++++++++----
 internal/oauth/store_keyring_test.go | 43 ++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 5 deletions(-)

diff --git a/internal/oauth/store.go b/internal/oauth/store.go
index 02a9a0b1f..cd0250a32 100644
--- a/internal/oauth/store.go
+++ b/internal/oauth/store.go
@@ -712,6 +712,14 @@ func (b keyringBlob) write(data []byte) error {
 // roughly 22 maximum-length keys even when every token was tiny.
 const maxKeyringIndexChunkBytes = 2700
 
+// maxKeyringIndexChunks caps how many chunk entries a stored index header may
+// claim before readKeyIndex issues one OS-keyring lookup per chunk. Each chunk
+// holds up to maxKeyringIndexChunkBytes of keys (dozens to ~150 keys), so this
+// bound admits far more logins than any real install while refusing to fan a
+// corrupt header (e.g. {"v":1,"chunks":1000000000}) out into a billion blocking
+// lookups that would wedge every OAuth operation under the store lock.
+const maxKeyringIndexChunks = 128
+
 // keyIndexHeader is chunk 0 of the key index. Chunks 1..Chunks-1 live under
 // "<indexAccount>-<n>" as plain JSON string arrays. The pre-chunking format
 // (a bare JSON array at indexAccount) is still read transparently.
@@ -753,6 +761,16 @@ func (b keyringBlob) readKeyIndex() ([]string, bool, int, error) {
 	if err := json.Unmarshal(raw, &header); err != nil {
 		return nil, false, 0, fmt.Errorf("oauth: decode keyring token index: %w", err)
 	}
+	// Reject an unsupported or corrupt header before looping: an out-of-range
+	// Chunks would otherwise drive up to that many blocking keyring lookups
+	// (each up to the 10s command timeout) while the store lock is held, wedging
+	// every Load/Status/Save/Delete instead of failing promptly.
+	if header.Version != 1 {
+		return nil, false, 0, fmt.Errorf("oauth: unsupported keyring token index version %d", header.Version)
+	}
+	if header.Chunks < 1 || header.Chunks > maxKeyringIndexChunks {
+		return nil, false, 0, fmt.Errorf("oauth: keyring token index advertises %d chunks (want 1..%d)", header.Chunks, maxKeyringIndexChunks)
+	}
 	keys := header.Keys
 	for i := 1; i < header.Chunks; i++ {
 		chunkEnc, ok, err := b.kr.Get(b.service, b.chunkAccount(i))
@@ -772,11 +790,7 @@ func (b keyringBlob) readKeyIndex() ([]string, bool, int, error) {
 		}
 		keys = append(keys, more...)
 	}
-	chunks := header.Chunks
-	if chunks < 1 {
-		chunks = 1
-	}
-	return keys, true, chunks, nil
+	return keys, true, header.Chunks, nil
 }
 
 // writeKeyIndex persists keys as a chunked index and reports how many chunk
diff --git a/internal/oauth/store_keyring_test.go b/internal/oauth/store_keyring_test.go
index 9a8372cf7..a079e3ab1 100644
--- a/internal/oauth/store_keyring_test.go
+++ b/internal/oauth/store_keyring_test.go
@@ -736,3 +736,46 @@ func TestStoreKeyringLeaseUsesWallClockNotStoreClock(t *testing.T) {
 		t.Fatalf("lease stamped the lock with the store clock (%v); a peer would reclaim the live lock", mtime)
 	}
 }
+
+// countingKR counts Get calls so a test can prove a corrupt index is rejected
+// before it fans out into a keyring lookup per advertised chunk.
+type countingKR struct {
+	*fakeKR
+	gets int
+}
+
+func (c *countingKR) Get(service, account string) (string, bool, error) {
+	c.gets++
+	return c.fakeKR.Get(service, account)
+}
+
+// TestStoreKeyringReadIndexRejectsCorruptHeader is the regression test for an
+// index header whose advertised chunk count is unbounded: readKeyIndex must
+// reject an out-of-range or unsupported header up front rather than issue up to
+// that many blocking keyring lookups while holding the store lock.
+func TestStoreKeyringReadIndexRejectsCorruptHeader(t *testing.T) {
+	ckr := &countingKR{fakeKR: newFakeKR()}
+	blob := keyringBlob{kr: ckr, service: keyringService, legacyAccount: keyringLegacyAccount, indexAccount: keyringIndexAccount}
+
+	oversized, err := json.Marshal(keyIndexHeader{Version: 1, Chunks: 1_000_000_000, Keys: []string{ProviderKey("demo")}})
+	if err != nil {
+		t.Fatal(err)
+	}
+	ckr.data[keyringService+"/"+keyringIndexAccount] = base64.StdEncoding.EncodeToString(oversized)
+	ckr.gets = 0
+	if _, _, _, err := blob.readKeyIndex(); err == nil {
+		t.Fatal("expected an oversized chunk count to be rejected")
+	}
+	if ckr.gets != 1 {
+		t.Fatalf("readKeyIndex issued %d keyring gets on a corrupt header; it must reject before fanning out over chunks", ckr.gets)
+	}
+
+	unsupported, err := json.Marshal(keyIndexHeader{Version: 2, Chunks: 1, Keys: []string{}})
+	if err != nil {
+		t.Fatal(err)
+	}
+	ckr.data[keyringService+"/"+keyringIndexAccount] = base64.StdEncoding.EncodeToString(unsupported)
+	if _, _, _, err := blob.readKeyIndex(); err == nil {
+		t.Fatal("expected an unsupported index version to be rejected")
+	}
+}

From 08ed2c6c316ff1ae34e106533fd7da59bf2e0fb6 Mon Sep 17 00:00:00 2001
From: euxaristia <25621994+euxaristia@users.noreply.github.com>
Date: Thu, 16 Jul 2026 21:56:40 -0400
Subject: [PATCH 08/17] fix(oauth): scope the keyring fallback lock to a
 per-user path

When no config location resolves, the keyring lock fell back to a single shared
${TMPDIR}/zero-oauth-keyring.lockfile. On a multi-user host any other account
could pre-create or keep refreshing that path and time out the victim's
Load/Status/Save/Delete, even though each user has a separate OS keychain.
Prefer the per-user OS cache directory, and scope the last-resort temp file by
uid so two different users never collide on one lock path.
---
 internal/oauth/store.go              | 30 +++++++++++++++++++++++++---
 internal/oauth/store_keyring_test.go | 25 +++++++++++++++++++++++
 2 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/internal/oauth/store.go b/internal/oauth/store.go
index cd0250a32..f1a669a18 100644
--- a/internal/oauth/store.go
+++ b/internal/oauth/store.go
@@ -213,9 +213,9 @@ func NewStore(options StoreOptions) (*Store, error) {
 		// file beside where the file backend would live. Cross-process exclusion
 		// must not silently disappear when no config location resolves (withLock
 		// would be a no-op and a concurrent save could delete another process's
-		// newly written entry), so fall back to the OS temp directory, which
-		// always exists, rather than to in-process serialization only.
-		lockPath := filepath.Join(os.TempDir(), "zero-oauth-keyring.lockfile")
+		// newly written entry), so fall back to a per-user location that always
+		// exists, rather than to in-process serialization only.
+		lockPath := keyringFallbackLockPath()
 		if storePath, perr := ResolveStorePath(options.Env); perr == nil {
 			lockPath = filepath.Join(filepath.Dir(storePath), "oauth-keyring.lockfile")
 		}
@@ -244,6 +244,30 @@ func resolveStoreFilePath(options StoreOptions) (string, error) {
 	return filepath.Clean(filePath), nil
 }
 
+// keyringFallbackLockPath returns a per-user location for the keyring lock when
+// no config location resolves. A single shared ${TMPDIR}/zero-oauth-keyring.lockfile
+// let any other account on a multi-user host pre-create or keep refreshing the
+// victim's lock and time out their Load/Status/Save/Delete, even though each user
+// has a separate OS keychain. Prefer the per-user OS cache directory (created
+// 0700 by acquireFileLock); only if that cannot be resolved fall back to a temp
+// file scoped by uid so two different users never collide on one path.
+func keyringFallbackLockPath() string {
+	if dir, err := os.UserCacheDir(); err == nil && strings.TrimSpace(dir) != "" {
+		return filepath.Join(dir, "zero", "oauth-keyring.lockfile")
+	}
+	return filepath.Join(os.TempDir(), keyringTempLockName())
+}
+
+// keyringTempLockName names the last-resort temp lock file, scoping it by uid so
+// concurrently running different users do not share one path. os.Getuid returns
+// -1 where uids do not apply (Windows), where os.TempDir is already per-user.
+func keyringTempLockName() string {
+	if uid := os.Getuid(); uid >= 0 {
+		return fmt.Sprintf("zero-oauth-keyring-%d.lockfile", uid)
+	}
+	return "zero-oauth-keyring.lockfile"
+}
+
 // FilePath returns the resolved token store location (a path for the file
 // backend, or a "keyring:..." identifier for the keyring backend).
 func (s *Store) FilePath() string { return s.blob.location() }
diff --git a/internal/oauth/store_keyring_test.go b/internal/oauth/store_keyring_test.go
index a079e3ab1..e615a18e3 100644
--- a/internal/oauth/store_keyring_test.go
+++ b/internal/oauth/store_keyring_test.go
@@ -3,6 +3,7 @@ package oauth
 import (
 	"encoding/base64"
 	"encoding/json"
+	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
@@ -779,3 +780,27 @@ func TestStoreKeyringReadIndexRejectsCorruptHeader(t *testing.T) {
 		t.Fatal("expected an unsupported index version to be rejected")
 	}
 }
+
+// TestKeyringFallbackLockPathIsPerUser covers the fallback taken when no config
+// location resolves. It must not be the single shared temp path that any account
+// on a multi-user host could pre-create or hold, and the last-resort temp name
+// must be scoped by uid so different users never collide on one lock file.
+func TestKeyringFallbackLockPathIsPerUser(t *testing.T) {
+	got := keyringFallbackLockPath()
+	if got == filepath.Join(os.TempDir(), "zero-oauth-keyring.lockfile") {
+		t.Fatalf("fallback lock path is the shared temp path %q; a co-tenant could grief it", got)
+	}
+	if cache, err := os.UserCacheDir(); err == nil && strings.TrimSpace(cache) != "" {
+		if want := filepath.Join(cache, "zero", "oauth-keyring.lockfile"); got != want {
+			t.Fatalf("fallback = %q, want per-user cache path %q", got, want)
+		}
+	}
+	name := keyringTempLockName()
+	if uid := os.Getuid(); uid >= 0 {
+		if !strings.Contains(name, fmt.Sprintf("%d", uid)) {
+			t.Fatalf("temp lock name %q is not scoped by uid %d", name, uid)
+		}
+	} else if name == "" {
+		t.Fatal("temp lock name is empty")
+	}
+}

From 73f6da823fc9aba483d4efcf026b1a8c98676cf8 Mon Sep 17 00:00:00 2001
From: euxaristia <25621994+euxaristia@users.noreply.github.com>
Date: Sat, 18 Jul 2026 17:04:01 -0400
Subject: [PATCH 09/17] fix(oauth): fail logout on legacy-blob delete failure;
 cap index chunks on write
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two review findings:

- write() swallowed the final legacy combined-entry delete error, so a
  logout could report success while the secret stayed resident, and the
  next save would classify the leftover legacy key as a fresh old-binary
  login and silently log the user back in. The delete now runs while the
  union index still lists removed keys and its failure propagates, so a
  retried logout reconciles instead of resurrecting the credential.

- writeKeyIndex could publish a header with more chunks than
  maxKeyringIndexChunks, which readKeyIndex then refuses — bricking every
  later Load/Status/Save/Delete. The write side now enforces the same cap
  before publishing anything.

Adds failure-injection coverage for the legacy-delete boundary and a
write-side cap regression test; the write-interruption test now asserts
every mutating boundary surfaces its injected failure.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 internal/oauth/store.go              | 24 ++++++--
 internal/oauth/store_keyring_test.go | 91 ++++++++++++++++++++++++++--
 2 files changed, 105 insertions(+), 10 deletions(-)

diff --git a/internal/oauth/store.go b/internal/oauth/store.go
index f1a669a18..b40e75218 100644
--- a/internal/oauth/store.go
+++ b/internal/oauth/store.go
@@ -630,7 +630,9 @@ func legacyIsFresher(legacy, current Token) bool {
 // or skips them once it is gone) or entries that a later read/write can still
 // see and reconcile, never an invisible credential stranded in the OS keychain.
 // The legacy combined entry is the durable fallback for the initial migration
-// and is deleted only as the final step, after every per-key entry is written.
+// and is deleted only after every per-key entry is written, while the union
+// index still lists removed keys; a failure of that delete is returned so a
+// logout is never reported successful with the stale blob still resident.
 func (b keyringBlob) write(data []byte) error {
 	var state storeFile
 	if err := json.Unmarshal(data, &state); err != nil {
@@ -718,13 +720,19 @@ func (b keyringBlob) write(data []byte) error {
 			}
 		}
 	}
-	// 4. Shrink the index to the exact new key set.
+	// 4. Drop the legacy entry: the index now exists and is authoritative,
+	// and its fresh writes were merged above. This must happen while the
+	// union index still lists any removed keys and its failure must surface:
+	// if a stale legacy blob survived a logout whose index shrink already
+	// completed, the next save would classify its keys as fresh old-binary
+	// logins and silently resurrect the logged-out credential.
+	if _, err := b.kr.Delete(b.service, b.legacyAccount); err != nil {
+		return err
+	}
+	// 5. Shrink the index to the exact new key set.
 	if _, err := b.writeKeyIndex(keys, unionChunks); err != nil {
 		return err
 	}
-	// The index now exists and is authoritative; drop the legacy entry so a
-	// future read never falls back to it (its fresh writes were merged above).
-	_, _ = b.kr.Delete(b.service, b.legacyAccount)
 	return nil
 }
 
@@ -825,6 +833,12 @@ func (b keyringBlob) readKeyIndex() ([]string, bool, int, error) {
 // chunk is never read).
 func (b keyringBlob) writeKeyIndex(keys []string, priorChunks int) (int, error) {
 	chunks := chunkIndexKeys(keys)
+	// Refuse to publish an index the reader would reject: readKeyIndex caps
+	// headers at maxKeyringIndexChunks, and a header beyond it would make
+	// every later Load/Status/Save/Delete fail before it could recover.
+	if len(chunks) > maxKeyringIndexChunks {
+		return 0, fmt.Errorf("oauth: keyring key index needs %d chunks, over the %d-chunk cap readers accept; too many stored credentials", len(chunks), maxKeyringIndexChunks)
+	}
 	for i := 1; i < len(chunks); i++ {
 		chunkData, err := json.Marshal(chunks[i])
 		if err != nil {
diff --git a/internal/oauth/store_keyring_test.go b/internal/oauth/store_keyring_test.go
index e615a18e3..7e5d41c28 100644
--- a/internal/oauth/store_keyring_test.go
+++ b/internal/oauth/store_keyring_test.go
@@ -373,11 +373,12 @@ func TestStoreKeyringWriteInterruptionsLeaveNoInvisibleTokens(t *testing.T) {
 				t.Fatalf("failAt=%d: Load(%s) after reconcile: ok=%v err=%v", failAt, name, ok, err)
 			}
 		}
-		// saveErr itself is not asserted: most boundaries surface the injected
-		// failure, but the final legacy-entry delete is deliberately
-		// best-effort, so its failure is swallowed by design. The invariant
-		// and the reconcile above are the actual contract.
-		_ = saveErr
+		// Every mutating boundary of the write path now surfaces its failure,
+		// including the legacy-entry delete (a swallowed failure there could
+		// let a later save resurrect logged-out credentials).
+		if opsUsed >= failAt && saveErr == nil {
+			t.Fatalf("failAt=%d: injected keyring failure was swallowed", failAt)
+		}
 		if opsUsed < failAt {
 			// The write used fewer mutating ops than failAt, so the injection
 			// never fired and every boundary has been covered.
@@ -804,3 +805,83 @@ func TestKeyringFallbackLockPathIsPerUser(t *testing.T) {
 		t.Fatal("temp lock name is empty")
 	}
 }
+
+// legacyDeleteFailKR fails Delete for the legacy combined entry only, to
+// exercise the boundary where a logout has already rewritten the indexed
+// state but the stale legacy blob cannot be removed.
+type legacyDeleteFailKR struct {
+	*fakeKR
+	fail bool
+}
+
+func (f *legacyDeleteFailKR) Delete(service, account string) (bool, error) {
+	if f.fail && account == keyringLegacyAccount {
+		return false, errKRInjected
+	}
+	return f.fakeKR.Delete(service, account)
+}
+
+// TestStoreKeyringLogoutSurfacesLegacyBlobDeleteFailure: when the final
+// legacy-blob delete fails during a logout, the operation must report the
+// failure (not success with the secret still resident), and after a clean
+// retry the logged-out credential must not be resurrected by a later save.
+func TestStoreKeyringLogoutSurfacesLegacyBlobDeleteFailure(t *testing.T) {
+	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
+	kr := &legacyDeleteFailKR{fakeKR: newFakeKR()}
+	s, err := NewStore(StoreOptions{Storage: "keyring", Keyring: kr})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := s.Save(ProviderKey("alpha"), Token{AccessToken: "a"}); err != nil {
+		t.Fatal(err)
+	}
+	// A leftover legacy blob still carries alpha (e.g. written by an old
+	// binary during the upgrade window).
+	legacy := storeFile{SchemaVersion: storeSchemaVersion, Tokens: map[string]Token{
+		ProviderKey("alpha"): {AccessToken: "stale-a"},
+	}}
+	data, err := json.Marshal(legacy)
+	if err != nil {
+		t.Fatal(err)
+	}
+	kr.data[keyringService+"/"+keyringLegacyAccount] = base64.StdEncoding.EncodeToString(data)
+
+	kr.fail = true
+	if _, err := s.Delete(ProviderKey("alpha")); err == nil {
+		t.Fatal("Delete reported success although the stale legacy blob could not be removed")
+	}
+
+	// A clean retry succeeds, and a later save must not classify the stale
+	// legacy alpha as a fresh old-binary login.
+	kr.fail = false
+	if _, err := s.Delete(ProviderKey("alpha")); err != nil {
+		t.Fatalf("retried Delete: %v", err)
+	}
+	if err := s.Save(ProviderKey("beta"), Token{AccessToken: "b"}); err != nil {
+		t.Fatal(err)
+	}
+	if _, ok, err := s.Load(ProviderKey("alpha")); err != nil || ok {
+		t.Fatalf("logged-out credential resurrected: ok=%v err=%v", ok, err)
+	}
+}
+
+// TestStoreKeyringWriteIndexRejectsOverCapChunks: writeKeyIndex must refuse
+// to publish an index header that readKeyIndex would reject, instead of
+// persisting a store no later operation can open.
+func TestStoreKeyringWriteIndexRejectsOverCapChunks(t *testing.T) {
+	kr := newFakeKR()
+	b := keyringBlob{kr: kr, service: keyringService, legacyAccount: keyringLegacyAccount, indexAccount: keyringIndexAccount}
+	// Each key exceeds the per-chunk byte budget on its own, forcing one
+	// chunk per key.
+	long := strings.Repeat("k", maxKeyringIndexChunkBytes)
+	keys := make([]string, maxKeyringIndexChunks+1)
+	for i := range keys {
+		keys[i] = fmt.Sprintf("%s-%d", long, i)
+	}
+	if _, err := b.writeKeyIndex(keys, 0); err == nil {
+		t.Fatal("writeKeyIndex published an index readKeyIndex would refuse")
+	}
+	if len(kr.data) != 0 {
+		t.Fatalf("over-cap index write must publish nothing, found %d entries", len(kr.data))
+	}
+}

From ecca0c2fbaa4faed6ae8b3de5068d432a5831280 Mon Sep 17 00:00:00 2001
From: euxaristia <25621994+euxaristia@users.noreply.github.com>
Date: Sun, 19 Jul 2026 04:49:46 -0400
Subject: [PATCH 10/17] fix(oauth): use wall clock for lock timeout and bound
 keyring index

acquireFileLock computed its deadline using the injectable clock, so
a fixed test clock could make lock contention retry forever instead
of timing out. Compute the deadline against wall-clock time instead,
keeping the injectable clock for token generation only. Cap the
number of keys a single keyring index chunk can claim, so a corrupted
index can no longer drive an unbounded lookup fan-out while holding
the store lock. Strengthen several existing tests that were marked
addressed but weren't fully.
---
 internal/oauth/lock.go               | 10 +++-
 internal/oauth/store.go              | 28 +++++++++
 internal/oauth/store_keyring_test.go | 90 +++++++++++++++++++++++++++-
 3 files changed, 123 insertions(+), 5 deletions(-)

diff --git a/internal/oauth/lock.go b/internal/oauth/lock.go
index d1dd344d2..0d192a2ed 100644
--- a/internal/oauth/lock.go
+++ b/internal/oauth/lock.go
@@ -23,6 +23,12 @@ var lockSeq atomic.Uint64
 // file is older than fileLockStaleAfter (so a crashed holder cannot deadlock the
 // store). Release is ownership-aware: it removes the lock only if it still holds
 // our token, so a stale-broken holder cannot delete a newer holder's lock.
+//
+// The acquisition deadline is always measured against the real wall clock, never
+// the now parameter: now is StoreOptions.Now, which callers may legitimately fix
+// (e.g. a test or an embedded clock), and deadline := now().Add(fileLockTimeout)
+// followed by now().After(deadline) would then never become true, turning lock
+// contention into an infinite retry loop instead of a timeout error.
 func acquireFileLock(lockPath string, now func() time.Time) (func(), error) {
 	if now == nil {
 		now = time.Now
@@ -31,7 +37,7 @@ func acquireFileLock(lockPath string, now func() time.Time) (func(), error) {
 		return nil, err
 	}
 	token := fmt.Sprintf("%d-%d-%d", os.Getpid(), now().UnixNano(), lockSeq.Add(1))
-	deadline := now().Add(fileLockTimeout)
+	deadline := time.Now().Add(fileLockTimeout)
 	for {
 		f, err := os.OpenFile(lockPath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o600)
 		if err == nil {
@@ -88,7 +94,7 @@ func acquireFileLock(lockPath string, now func() time.Time) (func(), error) {
 			// Lost the reclaim race (or it was actually fresh) — fall through to the
 			// bounded wait rather than hot-spinning on a reclaim that never wins.
 		}
-		if now().After(deadline) {
+		if time.Now().After(deadline) {
 			return nil, fmt.Errorf("oauth: timed out acquiring token lock %s", filepath.Base(lockPath))
 		}
 		time.Sleep(10 * time.Millisecond)
diff --git a/internal/oauth/store.go b/internal/oauth/store.go
index b40e75218..13ff4fde5 100644
--- a/internal/oauth/store.go
+++ b/internal/oauth/store.go
@@ -752,6 +752,25 @@ const maxKeyringIndexChunkBytes = 2700
 // lookups that would wedge every OAuth operation under the store lock.
 const maxKeyringIndexChunks = 128
 
+// maxKeyringIndexKeys bounds how many keys readKeyIndex will ever return, across
+// the header and every chunk (and the legacy bare-array format), before read()
+// and write() fan them out into one kr.Get per key while holding the store
+// lock. maxKeyringIndexChunks only bounds the number of chunk entries fetched;
+// it does not bound how many keys a single chunk's JSON can claim, so a
+// corrupted index with an oversized keys array (or many chunks each stuffed
+// with keys) could still drive an unbounded number of blocking lookups. The
+// bound here is generous relative to what chunkIndexKeys ever legitimately
+// produces (short namespaced keys cost at least ~18 bytes each, so one
+// maxKeyringIndexChunkBytes chunk holds on the order of a hundred, times
+// maxKeyringIndexChunks) while still rejecting a damaged index promptly.
+const maxKeyringIndexKeys = maxKeyringIndexChunks * 200
+
+// errKeyringIndexTooManyKeys is returned when a decoded index (or one of its
+// chunks) claims more keys than maxKeyringIndexKeys.
+func errKeyringIndexTooManyKeys(count int) error {
+	return fmt.Errorf("oauth: keyring token index lists %d keys, over the %d-key cap", count, maxKeyringIndexKeys)
+}
+
 // keyIndexHeader is chunk 0 of the key index. Chunks 1..Chunks-1 live under
 // "<indexAccount>-<n>" as plain JSON string arrays. The pre-chunking format
 // (a bare JSON array at indexAccount) is still read transparently.
@@ -787,6 +806,9 @@ func (b keyringBlob) readKeyIndex() ([]string, bool, int, error) {
 		if err := json.Unmarshal(raw, &keys); err != nil {
 			return nil, false, 0, fmt.Errorf("oauth: decode keyring token index: %w", err)
 		}
+		if len(keys) > maxKeyringIndexKeys {
+			return nil, false, 0, errKeyringIndexTooManyKeys(len(keys))
+		}
 		return keys, true, 1, nil
 	}
 	var header keyIndexHeader
@@ -803,6 +825,9 @@ func (b keyringBlob) readKeyIndex() ([]string, bool, int, error) {
 	if header.Chunks < 1 || header.Chunks > maxKeyringIndexChunks {
 		return nil, false, 0, fmt.Errorf("oauth: keyring token index advertises %d chunks (want 1..%d)", header.Chunks, maxKeyringIndexChunks)
 	}
+	if len(header.Keys) > maxKeyringIndexKeys {
+		return nil, false, 0, errKeyringIndexTooManyKeys(len(header.Keys))
+	}
 	keys := header.Keys
 	for i := 1; i < header.Chunks; i++ {
 		chunkEnc, ok, err := b.kr.Get(b.service, b.chunkAccount(i))
@@ -820,6 +845,9 @@ func (b keyringBlob) readKeyIndex() ([]string, bool, int, error) {
 		if err := json.Unmarshal(chunkRaw, &more); err != nil {
 			return nil, false, 0, fmt.Errorf("oauth: decode keyring token index chunk %d: %w", i, err)
 		}
+		if len(keys)+len(more) > maxKeyringIndexKeys {
+			return nil, false, 0, errKeyringIndexTooManyKeys(len(keys) + len(more))
+		}
 		keys = append(keys, more...)
 	}
 	return keys, true, header.Chunks, nil
diff --git a/internal/oauth/store_keyring_test.go b/internal/oauth/store_keyring_test.go
index 7e5d41c28..f6db1c2c2 100644
--- a/internal/oauth/store_keyring_test.go
+++ b/internal/oauth/store_keyring_test.go
@@ -364,6 +364,15 @@ func TestStoreKeyringWriteInterruptionsLeaveNoInvisibleTokens(t *testing.T) {
 			}
 		}
 
+		// The tokens this write didn't touch must stay readable at the
+		// interruption boundary itself, not just after a later reconciling
+		// write papers over an incorrect intermediate state.
+		for _, name := range []string{"alpha", "beta"} {
+			if _, ok, err := s.Load(ProviderKey(name)); err != nil || !ok {
+				t.Fatalf("failAt=%d: Load(%s) before reconcile: ok=%v err=%v", failAt, name, ok, err)
+			}
+		}
+
 		// A later unimpeded write must reconcile completely.
 		if err := s.Save(ProviderKey("gamma"), Token{AccessToken: "c"}); err != nil {
 			t.Fatalf("failAt=%d: reconciling Save: %v", failAt, err)
@@ -407,7 +416,7 @@ func TestStoreKeyringDeleteInterruptionsLeaveNoInvisibleTokens(t *testing.T) {
 		}
 		kr.ops = 0
 		kr.failAt = failAt
-		_, _ = s.Delete(ProviderKey("beta"))
+		_, deleteErr := s.Delete(ProviderKey("beta"))
 		opsUsed := kr.ops
 		kr.failAt = 0
 
@@ -433,6 +442,12 @@ func TestStoreKeyringDeleteInterruptionsLeaveNoInvisibleTokens(t *testing.T) {
 		if _, ok, err := s.Load(ProviderKey("alpha")); err != nil || !ok {
 			t.Fatalf("failAt=%d: Load(alpha): ok=%v err=%v", failAt, ok, err)
 		}
+		// Every mutating boundary of the delete path must surface its failure,
+		// mirroring the Save-interruption assertion: a swallowed error here
+		// would let a caller believe a logout succeeded when it didn't.
+		if opsUsed >= failAt && deleteErr == nil {
+			t.Fatalf("failAt=%d: injected keyring failure was swallowed", failAt)
+		}
 		if opsUsed < failAt {
 			break
 		}
@@ -473,6 +488,11 @@ func TestStoreKeyringMergesFreshLegacyWriteFromOldBinary(t *testing.T) {
 			t.Fatalf("Load(%s): ok=%v err=%v (fresh legacy write lost)", name, ok, err)
 		}
 	}
+	// Presence alone doesn't rule out the merge corrupting carol's credential
+	// material; check the actual values survived the legacy->indexed merge.
+	if got, _, err := s.Load(ProviderKey("carol")); err != nil || got.AccessToken != "c" || got.RefreshToken != "cr" {
+		t.Fatalf("Load(carol) = %#v, err=%v, want the legacy access/refresh tokens intact", got, err)
+	}
 	if _, ok := kr.data[keyringService+"/"+keyringLegacyAccount]; ok {
 		t.Fatal("legacy entry should be removed once its fresh writes are merged")
 	}
@@ -694,8 +714,8 @@ func TestStoreKeyringMergesFreshLegacyRefreshOfIndexedKey(t *testing.T) {
 	if err != nil || !ok {
 		t.Fatalf("Load(alpha): ok=%v err=%v", ok, err)
 	}
-	if got.AccessToken != "a-new" || got.RefreshToken != "r-new" {
-		t.Fatalf("Load(alpha) = %#v, want the refreshed legacy value (fresh refresh discarded)", got)
+	if got.AccessToken != "a-new" || got.RefreshToken != "r-new" || !got.ExpiresAt.Equal(fresh) {
+		t.Fatalf("Load(alpha) = %#v, want the refreshed legacy value (tokens and expiry) with ExpiresAt=%v", got, fresh)
 	}
 	if _, ok, _ := s.Load(ProviderKey("beta")); !ok {
 		t.Fatal("Load(beta): not stored")
@@ -705,6 +725,31 @@ func TestStoreKeyringMergesFreshLegacyRefreshOfIndexedKey(t *testing.T) {
 	}
 }
 
+// TestAcquireFileLockDeadlineUsesWallClockNotInjectedClock guards the other
+// half of the same hazard as the lease test below: acquireFileLock's own
+// acquisition deadline must be measured against the real wall clock, not the
+// injectable now parameter. StoreOptions.Now may legitimately be a fixed
+// clock (as this test uses), and deadline := now().Add(fileLockTimeout)
+// followed by now().After(deadline) would then never become true, so a
+// contested lock would retry forever instead of returning a timeout error.
+func TestAcquireFileLockDeadlineUsesWallClockNotInjectedClock(t *testing.T) {
+	lockPath := filepath.Join(t.TempDir(), "test.lockfile")
+	// A fresh (non-stale) lock held by someone else: acquireFileLock must not
+	// reclaim it, only wait out fileLockTimeout and report a timeout.
+	if err := os.WriteFile(lockPath, []byte("someone-else"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	fixed := time.Date(2000, 1, 1, 0, 0, 0, 0, time.UTC)
+	start := time.Now()
+	_, err := acquireFileLock(lockPath, func() time.Time { return fixed })
+	if err == nil {
+		t.Fatal("expected a timeout error acquiring an already-held, non-stale lock")
+	}
+	if elapsed := time.Since(start); elapsed > 10*time.Second {
+		t.Fatalf("acquireFileLock took %v with a fixed clock; the deadline must use the wall clock, not now()", elapsed)
+	}
+}
+
 // TestStoreKeyringLeaseUsesWallClockNotStoreClock guards the lock lease against
 // a fixed or stale StoreOptions.Now. acquireFileLock judges staleness with real
 // time.Since(mtime), so the lease must stamp the live lock with wall-clock time;
@@ -777,9 +822,48 @@ func TestStoreKeyringReadIndexRejectsCorruptHeader(t *testing.T) {
 		t.Fatal(err)
 	}
 	ckr.data[keyringService+"/"+keyringIndexAccount] = base64.StdEncoding.EncodeToString(unsupported)
+	ckr.gets = 0
 	if _, _, _, err := blob.readKeyIndex(); err == nil {
 		t.Fatal("expected an unsupported index version to be rejected")
 	}
+	if ckr.gets != 1 {
+		t.Fatalf("readKeyIndex issued %d gets for an unsupported version; want header lookup only", ckr.gets)
+	}
+}
+
+// TestStoreKeyringReadIndexRejectsOversizedKeyList regresses a corrupt index
+// that claims more keys than maxKeyringIndexKeys: maxKeyringIndexChunks alone
+// bounds only how many chunk entries are fetched, not how many keys a single
+// chunk's JSON (or the legacy bare-array format) can claim, so without this
+// check readKeyIndex would hand read()/write() an oversized key list to fan
+// out into one blocking kr.Get per key while holding the store lock.
+func TestStoreKeyringReadIndexRejectsOversizedKeyList(t *testing.T) {
+	ckr := &countingKR{fakeKR: newFakeKR()}
+	blob := keyringBlob{kr: ckr, service: keyringService, legacyAccount: keyringLegacyAccount, indexAccount: keyringIndexAccount}
+
+	tooMany := make([]string, maxKeyringIndexKeys+1)
+	for i := range tooMany {
+		tooMany[i] = ProviderKey(fmt.Sprintf("p%d", i))
+	}
+
+	header, err := json.Marshal(keyIndexHeader{Version: 1, Chunks: 1, Keys: tooMany})
+	if err != nil {
+		t.Fatal(err)
+	}
+	ckr.data[keyringService+"/"+keyringIndexAccount] = base64.StdEncoding.EncodeToString(header)
+	if _, _, _, err := blob.readKeyIndex(); err == nil {
+		t.Fatal("expected an oversized key list in a chunk-0 header to be rejected")
+	}
+
+	// The pre-chunking bare-array format must be capped the same way.
+	legacyArray, err := json.Marshal(tooMany)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ckr.data[keyringService+"/"+keyringIndexAccount] = base64.StdEncoding.EncodeToString(legacyArray)
+	if _, _, _, err := blob.readKeyIndex(); err == nil {
+		t.Fatal("expected an oversized legacy-format key array to be rejected")
+	}
 }
 
 // TestKeyringFallbackLockPathIsPerUser covers the fallback taken when no config

From 19a14ae7b39c9526aee31d613454aef5bb164305 Mon Sep 17 00:00:00 2001
From: euxaristia <25621994+euxaristia@users.noreply.github.com>
Date: Sun, 19 Jul 2026 07:25:56 -0400
Subject: [PATCH 11/17] fix(oauth): refuse keyring indexes over the reader key
 cap on write

writeKeyIndex already refused over-cap chunk counts, but a large set of
short keys can stay under the chunk limit while exceeding
maxKeyringIndexKeys. Cap the key count before publishing so Save cannot
persist an index that readKeyIndex then rejects. Cover the write-side
path and multi-chunk accumulation on the reader.
---
 internal/oauth/store.go              | 11 +++++--
 internal/oauth/store_keyring_test.go | 49 ++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/internal/oauth/store.go b/internal/oauth/store.go
index 13ff4fde5..9dc76d67f 100644
--- a/internal/oauth/store.go
+++ b/internal/oauth/store.go
@@ -860,10 +860,15 @@ func (b keyringBlob) readKeyIndex() ([]string, bool, int, error) {
 // only after the header stops referencing them (best-effort: an unreferenced
 // chunk is never read).
 func (b keyringBlob) writeKeyIndex(keys []string, priorChunks int) (int, error) {
+	// Refuse to publish an index the reader would reject: readKeyIndex caps both
+	// total keys and chunk count, and a header beyond either would make every
+	// later Load/Status/Save/Delete fail before it could recover. Check the key
+	// count before chunking so a large set of short keys that still fit under
+	// maxKeyringIndexChunks cannot strand the store unreadable.
+	if len(keys) > maxKeyringIndexKeys {
+		return 0, errKeyringIndexTooManyKeys(len(keys))
+	}
 	chunks := chunkIndexKeys(keys)
-	// Refuse to publish an index the reader would reject: readKeyIndex caps
-	// headers at maxKeyringIndexChunks, and a header beyond it would make
-	// every later Load/Status/Save/Delete fail before it could recover.
 	if len(chunks) > maxKeyringIndexChunks {
 		return 0, fmt.Errorf("oauth: keyring key index needs %d chunks, over the %d-chunk cap readers accept; too many stored credentials", len(chunks), maxKeyringIndexChunks)
 	}
diff --git a/internal/oauth/store_keyring_test.go b/internal/oauth/store_keyring_test.go
index f6db1c2c2..826c6e82a 100644
--- a/internal/oauth/store_keyring_test.go
+++ b/internal/oauth/store_keyring_test.go
@@ -851,9 +851,13 @@ func TestStoreKeyringReadIndexRejectsOversizedKeyList(t *testing.T) {
 		t.Fatal(err)
 	}
 	ckr.data[keyringService+"/"+keyringIndexAccount] = base64.StdEncoding.EncodeToString(header)
+	ckr.gets = 0
 	if _, _, _, err := blob.readKeyIndex(); err == nil {
 		t.Fatal("expected an oversized key list in a chunk-0 header to be rejected")
 	}
+	if ckr.gets != 1 {
+		t.Fatalf("readKeyIndex issued %d gets for an oversized header keys list; want header lookup only", ckr.gets)
+	}
 
 	// The pre-chunking bare-array format must be capped the same way.
 	legacyArray, err := json.Marshal(tooMany)
@@ -861,9 +865,34 @@ func TestStoreKeyringReadIndexRejectsOversizedKeyList(t *testing.T) {
 		t.Fatal(err)
 	}
 	ckr.data[keyringService+"/"+keyringIndexAccount] = base64.StdEncoding.EncodeToString(legacyArray)
+	ckr.gets = 0
 	if _, _, _, err := blob.readKeyIndex(); err == nil {
 		t.Fatal("expected an oversized legacy-format key array to be rejected")
 	}
+	if ckr.gets != 1 {
+		t.Fatalf("readKeyIndex issued %d gets for an oversized legacy array; want header lookup only", ckr.gets)
+	}
+
+	// Accumulation across continuation chunks must hit the same total cap: a
+	// small header plus an oversized chunk-1 would otherwise fan out past the
+	// bound after the per-header check has already passed.
+	headerOK, err := json.Marshal(keyIndexHeader{Version: 1, Chunks: 2, Keys: []string{ProviderKey("seed")}})
+	if err != nil {
+		t.Fatal(err)
+	}
+	chunk1, err := json.Marshal(tooMany)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ckr.data[keyringService+"/"+keyringIndexAccount] = base64.StdEncoding.EncodeToString(headerOK)
+	ckr.data[keyringService+"/"+keyringIndexAccount+"-1"] = base64.StdEncoding.EncodeToString(chunk1)
+	ckr.gets = 0
+	if _, _, _, err := blob.readKeyIndex(); err == nil {
+		t.Fatal("expected an oversized key list accumulated across chunks to be rejected")
+	}
+	if ckr.gets != 2 {
+		t.Fatalf("readKeyIndex issued %d gets for a header+oversize-chunk; want header and chunk-1 only", ckr.gets)
+	}
 }
 
 // TestKeyringFallbackLockPathIsPerUser covers the fallback taken when no config
@@ -969,3 +998,23 @@ func TestStoreKeyringWriteIndexRejectsOverCapChunks(t *testing.T) {
 		t.Fatalf("over-cap index write must publish nothing, found %d entries", len(kr.data))
 	}
 }
+
+// TestStoreKeyringWriteIndexRejectsOverCapKeys: short keys can still fit under
+// the chunk-count cap while exceeding maxKeyringIndexKeys. writeKeyIndex must
+// refuse that set before publishing, matching the reader-side total key cap.
+func TestStoreKeyringWriteIndexRejectsOverCapKeys(t *testing.T) {
+	kr := newFakeKR()
+	b := keyringBlob{kr: kr, service: keyringService, legacyAccount: keyringLegacyAccount, indexAccount: keyringIndexAccount}
+	keys := make([]string, maxKeyringIndexKeys+1)
+	for i := range keys {
+		// Short keys pack densely into chunks so the chunk-count check alone
+		// would not catch this over-cap set.
+		keys[i] = fmt.Sprintf("p%d", i)
+	}
+	if _, err := b.writeKeyIndex(keys, 0); err == nil {
+		t.Fatal("writeKeyIndex published a key count readKeyIndex would refuse")
+	}
+	if len(kr.data) != 0 {
+		t.Fatalf("over-cap key write must publish nothing, found %d entries", len(kr.data))
+	}
+}

From 189527643b91fa7e55fea415be7ba6ee424c1bbd Mon Sep 17 00:00:00 2001
From: euxaristia <25621994+euxaristia@users.noreply.github.com>
Date: Wed, 22 Jul 2026 03:41:39 -0400
Subject: [PATCH 12/17] fix(oauth): derive the keyring lock path from keyring
 identity, not file config

The cross-process lock guarding the keyring token index was keyed off
ResolveStorePath (ZERO_OAUTH_TOKENS_PATH / XDG_CONFIG_HOME), not off the
keyring's own identity (service + index account). Two zero processes with
different config roots but pointed at the same underlying OS keyring entry
got different lock files, so they could race a read-modify-write on the
shared index and silently drop one process's token write.

keyringLockPath now derives the lock file name from the keyring service and
account instead, so the lock always matches the entry actually being
touched regardless of caller config.
---
 internal/oauth/store.go              | 70 +++++++++++++++++++---------
 internal/oauth/store_keyring_test.go | 68 +++++++++++++++++++++------
 2 files changed, 102 insertions(+), 36 deletions(-)

diff --git a/internal/oauth/store.go b/internal/oauth/store.go
index 9dc76d67f..f536212dc 100644
--- a/internal/oauth/store.go
+++ b/internal/oauth/store.go
@@ -210,15 +210,14 @@ func NewStore(options StoreOptions) (*Store, error) {
 			kr = osKeyring
 		}
 		// Serialize the keyring's read-modify-write across processes with a lock
-		// file beside where the file backend would live. Cross-process exclusion
-		// must not silently disappear when no config location resolves (withLock
-		// would be a no-op and a concurrent save could delete another process's
-		// newly written entry), so fall back to a per-user location that always
-		// exists, rather than to in-process serialization only.
-		lockPath := keyringFallbackLockPath()
-		if storePath, perr := ResolveStorePath(options.Env); perr == nil {
-			lockPath = filepath.Join(filepath.Dir(storePath), "oauth-keyring.lockfile")
-		}
+		// file keyed off the keyring identity itself (service + index account),
+		// never off the file-backend's path config: two processes with different
+		// ZERO_OAUTH_TOKENS_PATH / XDG_CONFIG_HOME roots but pointed at the SAME
+		// OS keyring entry (the service/account is fixed per binary, not per
+		// config root) must still serialize against each other, or they can race
+		// a read-modify-write on the shared keyring index and silently drop one
+		// process's token write.
+		lockPath := keyringLockPath(keyringService, keyringIndexAccount)
 		return &Store{blob: keyringBlob{kr: kr, service: keyringService, legacyAccount: keyringLegacyAccount, indexAccount: keyringIndexAccount, lockPath: lockPath}, now: now}, nil
 	default:
 		return nil, fmt.Errorf("oauth: unknown storage %q (want \"file\", \"encrypted-file\", or \"keyring\")", storage)
@@ -244,28 +243,55 @@ func resolveStoreFilePath(options StoreOptions) (string, error) {
 	return filepath.Clean(filePath), nil
 }
 
-// keyringFallbackLockPath returns a per-user location for the keyring lock when
-// no config location resolves. A single shared ${TMPDIR}/zero-oauth-keyring.lockfile
-// let any other account on a multi-user host pre-create or keep refreshing the
-// victim's lock and time out their Load/Status/Save/Delete, even though each user
-// has a separate OS keychain. Prefer the per-user OS cache directory (created
-// 0700 by acquireFileLock); only if that cannot be resolved fall back to a temp
-// file scoped by uid so two different users never collide on one path.
-func keyringFallbackLockPath() string {
+// keyringLockPath returns the cross-process lock file location for the
+// keyring backend's read-modify-write, derived from the keyring identity
+// itself (the service/account the index is stored under) rather than from
+// the unrelated file-backend path config (ZERO_OAUTH_TOKENS_PATH /
+// XDG_CONFIG_HOME): the file backend's location has nothing to do with which
+// OS keyring entry a process is about to read-modify-write, so a lock keyed
+// off it let two processes with different config roots but the SAME keyring
+// entry race the shared index and silently drop one process's token write.
+// A single shared ${TMPDIR}/zero-oauth-keyring.lockfile would also let any
+// other account on a multi-user host pre-create or keep refreshing the
+// victim's lock and time out their Load/Status/Save/Delete, even though each
+// user has a separate OS keychain, so this prefers the per-user OS cache
+// directory (created 0700 by acquireFileLock); only if that cannot be
+// resolved does it fall back to a temp file scoped by uid so two different
+// users never collide on one path.
+func keyringLockPath(service, account string) string {
+	name := keyringLockFileName(service, account)
 	if dir, err := os.UserCacheDir(); err == nil && strings.TrimSpace(dir) != "" {
-		return filepath.Join(dir, "zero", "oauth-keyring.lockfile")
+		return filepath.Join(dir, "zero", name)
 	}
-	return filepath.Join(os.TempDir(), keyringTempLockName())
+	return filepath.Join(os.TempDir(), keyringTempLockName(service, account))
+}
+
+// keyringLockFileName names the lock file after the keyring identity it
+// guards, so distinct (service, account) pairs never share a lock and the
+// same pair always resolves to the same lock regardless of caller config.
+func keyringLockFileName(service, account string) string {
+	return fmt.Sprintf("oauth-keyring-%s-%s.lockfile", sanitizeLockComponent(service), sanitizeLockComponent(account))
+}
+
+// lockComponentSafe keeps a service/account string safe as one path segment:
+// alphanumerics, dot, underscore, and hyphen pass through; anything else
+// (a path separator, especially) is replaced so a crafted identity can never
+// escape the lock directory.
+var lockComponentSafe = regexp.MustCompile(`[^A-Za-z0-9._-]+`)
+
+func sanitizeLockComponent(s string) string {
+	return lockComponentSafe.ReplaceAllString(s, "_")
 }
 
 // keyringTempLockName names the last-resort temp lock file, scoping it by uid so
 // concurrently running different users do not share one path. os.Getuid returns
 // -1 where uids do not apply (Windows), where os.TempDir is already per-user.
-func keyringTempLockName() string {
+func keyringTempLockName(service, account string) string {
+	name := keyringLockFileName(service, account)
 	if uid := os.Getuid(); uid >= 0 {
-		return fmt.Sprintf("zero-oauth-keyring-%d.lockfile", uid)
+		return fmt.Sprintf("zero-%d-%s", uid, name)
 	}
-	return "zero-oauth-keyring.lockfile"
+	return "zero-" + name
 }
 
 // FilePath returns the resolved token store location (a path for the file
diff --git a/internal/oauth/store_keyring_test.go b/internal/oauth/store_keyring_test.go
index 826c6e82a..1821c2859 100644
--- a/internal/oauth/store_keyring_test.go
+++ b/internal/oauth/store_keyring_test.go
@@ -895,30 +895,70 @@ func TestStoreKeyringReadIndexRejectsOversizedKeyList(t *testing.T) {
 	}
 }
 
-// TestKeyringFallbackLockPathIsPerUser covers the fallback taken when no config
-// location resolves. It must not be the single shared temp path that any account
-// on a multi-user host could pre-create or hold, and the last-resort temp name
-// must be scoped by uid so different users never collide on one lock file.
-func TestKeyringFallbackLockPathIsPerUser(t *testing.T) {
-	got := keyringFallbackLockPath()
-	if got == filepath.Join(os.TempDir(), "zero-oauth-keyring.lockfile") {
-		t.Fatalf("fallback lock path is the shared temp path %q; a co-tenant could grief it", got)
+// TestKeyringLockPathIsPerUser covers the lock path used for every keyring
+// Store regardless of file-backend config. It must not be the single shared
+// temp path that any account on a multi-user host could pre-create or hold,
+// and the last-resort temp name must be scoped by uid so different users
+// never collide on one lock file.
+func TestKeyringLockPathIsPerUser(t *testing.T) {
+	got := keyringLockPath(keyringService, keyringIndexAccount)
+	name := keyringLockFileName(keyringService, keyringIndexAccount)
+	if got == filepath.Join(os.TempDir(), "zero-"+name) {
+		t.Fatalf("lock path is the shared temp path %q; a co-tenant could grief it", got)
 	}
 	if cache, err := os.UserCacheDir(); err == nil && strings.TrimSpace(cache) != "" {
-		if want := filepath.Join(cache, "zero", "oauth-keyring.lockfile"); got != want {
-			t.Fatalf("fallback = %q, want per-user cache path %q", got, want)
+		if want := filepath.Join(cache, "zero", name); got != want {
+			t.Fatalf("lock path = %q, want per-user cache path %q", got, want)
 		}
 	}
-	name := keyringTempLockName()
+	tempName := keyringTempLockName(keyringService, keyringIndexAccount)
 	if uid := os.Getuid(); uid >= 0 {
-		if !strings.Contains(name, fmt.Sprintf("%d", uid)) {
-			t.Fatalf("temp lock name %q is not scoped by uid %d", name, uid)
+		if !strings.Contains(tempName, fmt.Sprintf("%d", uid)) {
+			t.Fatalf("temp lock name %q is not scoped by uid %d", tempName, uid)
 		}
-	} else if name == "" {
+	} else if tempName == "" {
 		t.Fatal("temp lock name is empty")
 	}
 }
 
+// TestKeyringLockPathDerivedFromKeyringIdentityNotFileConfig is the
+// regression test for the cross-process lock racing bug: the lock guarding
+// the shared keyring index must be keyed off the keyring's own identity
+// (service + index account), never off the file-backend path config
+// (ZERO_OAUTH_TOKENS_PATH / XDG_CONFIG_HOME). Two zero processes with
+// different config roots but pointed at the SAME keyring entry (the service
+// and account are fixed per binary, not per config root) must resolve to the
+// identical lock path, or they can race a read-modify-write on the shared
+// index and silently drop one process's token write.
+func TestKeyringLockPathDerivedFromKeyringIdentityNotFileConfig(t *testing.T) {
+	dirA := filepath.Join(t.TempDir(), "config-root-a")
+	dirB := filepath.Join(t.TempDir(), "config-root-b")
+
+	storeA, err := NewStore(StoreOptions{Storage: "keyring", Keyring: newFakeKR(), Env: map[string]string{"XDG_CONFIG_HOME": dirA}})
+	if err != nil {
+		t.Fatal(err)
+	}
+	storeB, err := NewStore(StoreOptions{Storage: "keyring", Keyring: newFakeKR(), Env: map[string]string{"XDG_CONFIG_HOME": dirB}})
+	if err != nil {
+		t.Fatal(err)
+	}
+	blobA, okA := storeA.blob.(keyringBlob)
+	blobB, okB := storeB.blob.(keyringBlob)
+	if !okA || !okB {
+		t.Fatal("Store.blob is not keyringBlob")
+	}
+	if blobA.lockPath == "" || blobB.lockPath == "" {
+		t.Fatal("lockPath should never be empty for the keyring backend")
+	}
+	if blobA.lockPath != blobB.lockPath {
+		t.Fatalf("two processes on the SAME keyring entry got different lock paths for different config roots: %q vs %q (they can race the shared keyring index)", blobA.lockPath, blobB.lockPath)
+	}
+	// And it must not be derived from either config root's resolved store path.
+	if strings.Contains(blobA.lockPath, dirA) || strings.Contains(blobA.lockPath, dirB) {
+		t.Fatalf("lock path %q is still derived from file-backend config, not the keyring identity", blobA.lockPath)
+	}
+}
+
 // legacyDeleteFailKR fails Delete for the legacy combined entry only, to
 // exercise the boundary where a logout has already rewritten the indexed
 // state but the stale legacy blob cannot be removed.

From 1d6a8bf3940f5b17c18ea761d3f9ffa938e0c087 Mon Sep 17 00:00:00 2001
From: euxaristia <25621994+euxaristia@users.noreply.github.com>
Date: Wed, 22 Jul 2026 03:43:11 -0400
Subject: [PATCH 13/17] fix(oauth): refuse to delete the legacy keyring blob on
 a transient read error

readLegacyTokens collapsed every failure reading the legacy combined
keyring entry (a transient keyring error, undecodable base64, invalid
JSON) into the same "no tokens" result as the entry genuinely not
existing. During mixed-version reconciliation, write() used that result
to decide what to merge before unconditionally deleting the legacy blob,
so a transient read failure looked identical to "nothing to reconcile"
and the blob got deleted anyway, permanently losing a credential that
belonged to an older, still-installed zero binary.

readLegacyTokens now returns an explicit error distinct from "not
found." read()'s recovery fallback stays best-effort (a failure there
just skips recovering one desynced key, since it doesn't delete
anything), but write() now aborts the whole write and surfaces the
error instead of reconciling against an empty map when the legacy blob
can't actually be read.
---
 internal/oauth/store.go              | 50 ++++++++++++-----
 internal/oauth/store_keyring_test.go | 80 ++++++++++++++++++++++++++++
 2 files changed, 118 insertions(+), 12 deletions(-)

diff --git a/internal/oauth/store.go b/internal/oauth/store.go
index f536212dc..a3bb315ed 100644
--- a/internal/oauth/store.go
+++ b/internal/oauth/store.go
@@ -579,7 +579,15 @@ func (b keyringBlob) read() ([]byte, bool, error) {
 			// gone) skip rather than fail the whole read, since the next
 			// Save/Delete will reconcile the index.
 			if !legacyLoaded {
-				legacyTokens = b.readLegacyTokens()
+				// A best-effort recovery source for a read: a transient failure
+				// here must not fail the whole Load/Status, only skip recovering
+				// this particular desynced key. The next Save/Delete will
+				// reconcile it once the legacy blob is legible again (and,
+				// unlike here, will refuse to delete the legacy blob until it
+				// can actually read it — see write()).
+				if lt, lerr := b.readLegacyTokens(); lerr == nil {
+					legacyTokens = lt
+				}
 				legacyLoaded = true
 			}
 			if token, has := legacyTokens[key]; has {
@@ -619,20 +627,28 @@ func (b keyringBlob) readLegacy() ([]byte, bool, error) {
 	return data, true, nil
 }
 
-// readLegacyTokens returns the tokens held in the legacy combined entry, or an
-// empty map when there is no readable legacy blob. It is a best-effort recovery
-// source (read() falls back to it, write() reconciles against it), so a missing
-// or malformed legacy entry is reported as "no tokens" rather than a hard error.
-func (b keyringBlob) readLegacyTokens() map[string]Token {
+// readLegacyTokens returns the tokens held in the legacy combined entry. A
+// nil map with a nil error means the entry genuinely does not exist (readLegacy
+// returned ok=false, err=nil) — the one case callers may treat as "no tokens"
+// and proceed. Any other failure (a transient keyring read error, undecodable
+// base64, invalid JSON) is returned as err and must NOT be collapsed into "no
+// tokens": write() deletes the legacy blob once it believes reconciliation is
+// complete, so mistaking a transient read failure for an empty blob would
+// delete a still-unread, still-live credential that belongs to an older,
+// still-installed zero binary.
+func (b keyringBlob) readLegacyTokens() (map[string]Token, error) {
 	data, ok, err := b.readLegacy()
-	if err != nil || !ok {
-		return nil
+	if err != nil {
+		return nil, err
+	}
+	if !ok {
+		return nil, nil
 	}
 	var legacyState storeFile
-	if json.Unmarshal(data, &legacyState) != nil {
-		return nil
+	if err := json.Unmarshal(data, &legacyState); err != nil {
+		return nil, fmt.Errorf("oauth: invalid legacy keyring token blob: %w", err)
 	}
-	return legacyState.Tokens
+	return legacyState.Tokens, nil
 }
 
 // legacyIsFresher reports whether the legacy copy of an already-indexed key
@@ -686,7 +702,17 @@ func (b keyringBlob) write(data []byte) error {
 	//   - a key that was in the prior index but is absent from this write was
 	//     deliberately removed (a logout); it is left removed, not resurrected.
 	if indexExisted {
-		for key, legacyToken := range b.readLegacyTokens() {
+		// Unlike read()'s best-effort fallback, a failure here must abort the
+		// whole write rather than proceed as though the legacy blob were empty:
+		// step 4 below deletes it, and a transient read error (the legacy blob
+		// genuinely exists but couldn't be read right now) must never be
+		// mistaken for "nothing to reconcile," or a still-live credential from
+		// an older, still-installed zero binary is destroyed irrecoverably.
+		legacyTokens, err := b.readLegacyTokens()
+		if err != nil {
+			return fmt.Errorf("oauth: read legacy keyring token blob for reconciliation: %w", err)
+		}
+		for key, legacyToken := range legacyTokens {
 			if ValidateKey(key) != nil {
 				continue
 			}
diff --git a/internal/oauth/store_keyring_test.go b/internal/oauth/store_keyring_test.go
index 1821c2859..6b09c99c1 100644
--- a/internal/oauth/store_keyring_test.go
+++ b/internal/oauth/store_keyring_test.go
@@ -1058,3 +1058,83 @@ func TestStoreKeyringWriteIndexRejectsOverCapKeys(t *testing.T) {
 		t.Fatalf("over-cap key write must publish nothing, found %d entries", len(kr.data))
 	}
 }
+
+// legacyGetFailKR fails Get for the legacy combined entry only, simulating a
+// transient keyring read error (as opposed to the entry genuinely not
+// existing, which fakeKR's normal Get reports as ok=false, err=nil).
+type legacyGetFailKR struct {
+	*fakeKR
+	fail bool
+}
+
+func (f *legacyGetFailKR) Get(service, account string) (string, bool, error) {
+	if f.fail && account == keyringLegacyAccount {
+		return "", false, errKRInjected
+	}
+	return f.fakeKR.Get(service, account)
+}
+
+// TestStoreKeyringWriteRefusesToDeleteLegacyBlobOnTransientReadError is the
+// regression test for finding #2: during mixed-version reconciliation (an old
+// zero binary's legacy blob alongside the new keyring-based index), a
+// transient error reading the legacy blob must not be treated as "the legacy
+// blob is empty." write() must refuse to reconcile-and-delete it in that case,
+// or an unread credential belonging to an older, still-installed zero binary
+// is destroyed irrecoverably.
+func TestStoreKeyringWriteRefusesToDeleteLegacyBlobOnTransientReadError(t *testing.T) {
+	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
+	kr := &legacyGetFailKR{fakeKR: newFakeKR()}
+	s, err := NewStore(StoreOptions{Storage: "keyring", Keyring: kr})
+	if err != nil {
+		t.Fatal(err)
+	}
+	// A first save publishes the index, so indexExisted is true for the next
+	// write and exercises the mixed-version reconciliation path in write()
+	// where the bug lived.
+	if err := s.Save(ProviderKey("alpha"), Token{AccessToken: "a"}); err != nil {
+		t.Fatal(err)
+	}
+
+	// A legacy blob still carries a credential written by an older,
+	// still-installed binary.
+	legacy := storeFile{SchemaVersion: storeSchemaVersion, Tokens: map[string]Token{
+		ProviderKey("stale-binary-login"): {AccessToken: "still-live"},
+	}}
+	data, err := json.Marshal(legacy)
+	if err != nil {
+		t.Fatal(err)
+	}
+	kr.data[keyringService+"/"+keyringLegacyAccount] = base64.StdEncoding.EncodeToString(data)
+
+	// A transient failure reading it: Get returns a real error, not
+	// ok=false/err=nil ("doesn't exist").
+	kr.fail = true
+	if err := s.Save(ProviderKey("beta"), Token{AccessToken: "b"}); err == nil {
+		t.Fatal("Save succeeded despite a transient legacy-blob read failure; it must refuse rather than silently treat the blob as empty")
+	}
+	// The legacy blob must still be present, not deleted out from under a
+	// read failure.
+	if _, ok := kr.data[keyringService+"/"+keyringLegacyAccount]; !ok {
+		t.Fatal("legacy blob was deleted despite a transient read failure (data loss)")
+	}
+	// Nothing else the aborted write touched should be visible either.
+	if _, ok, _ := s.Load(ProviderKey("beta")); ok {
+		t.Fatal("beta should not be visible: the write should have aborted entirely, not partially applied")
+	}
+	if _, ok, err := s.Load(ProviderKey("alpha")); err != nil || !ok {
+		t.Fatalf("alpha lost after an aborted write: ok=%v err=%v", ok, err)
+	}
+
+	// Once the transient failure clears, the legacy credential is recovered
+	// and the blob is cleaned up normally.
+	kr.fail = false
+	if err := s.Save(ProviderKey("beta"), Token{AccessToken: "b"}); err != nil {
+		t.Fatalf("retried Save: %v", err)
+	}
+	if _, ok, err := s.Load(ProviderKey("stale-binary-login")); err != nil || !ok {
+		t.Fatalf("Load(stale-binary-login): ok=%v err=%v (legacy credential lost)", ok, err)
+	}
+	if _, ok := kr.data[keyringService+"/"+keyringLegacyAccount]; ok {
+		t.Fatal("legacy blob should be removed once it was actually read and reconciled")
+	}
+}

From c07b9dc18ba7e45fb8d7d2ab9c8615564b7c5849 Mon Sep 17 00:00:00 2001
From: euxaristia <25621994+euxaristia@users.noreply.github.com>
Date: Wed, 22 Jul 2026 03:44:41 -0400
Subject: [PATCH 14/17] fix(oauth): dedupe and validate the keyring index
 before fanning out lookups

readKeyIndex's decoded key list was fanned out into one keyring Get per
key (Load, Status, Save, Delete all go through it) with no dedup or
format check, even though the cap on the index is documented at 25,600
entries. A corrupted or adversarially crafted index that repeats the
same key thousands of times still costs one blocking keyring lookup per
repeat (each up to the 10s command timeout) while the store lock is
held, reintroducing the fan-out DoS the index cap was meant to close.

readKeyIndex now runs its result through dedupeValidKeys, which drops
duplicate and malformed (non-ValidateKey-shaped) entries before any
caller fans them out, so repeats in the index collapse to one lookup
per distinct valid key.
---
 internal/oauth/store.go              |  31 ++++++++-
 internal/oauth/store_keyring_test.go | 100 +++++++++++++++++++++++++++
 2 files changed, 129 insertions(+), 2 deletions(-)

diff --git a/internal/oauth/store.go b/internal/oauth/store.go
index a3bb315ed..82272380b 100644
--- a/internal/oauth/store.go
+++ b/internal/oauth/store.go
@@ -861,7 +861,7 @@ func (b keyringBlob) readKeyIndex() ([]string, bool, int, error) {
 		if len(keys) > maxKeyringIndexKeys {
 			return nil, false, 0, errKeyringIndexTooManyKeys(len(keys))
 		}
-		return keys, true, 1, nil
+		return dedupeValidKeys(keys), true, 1, nil
 	}
 	var header keyIndexHeader
 	if err := json.Unmarshal(raw, &header); err != nil {
@@ -902,7 +902,34 @@ func (b keyringBlob) readKeyIndex() ([]string, bool, int, error) {
 		}
 		keys = append(keys, more...)
 	}
-	return keys, true, header.Chunks, nil
+	return dedupeValidKeys(keys), true, header.Chunks, nil
+}
+
+// dedupeValidKeys drops duplicates and malformed entries from a decoded
+// index's key list before it is fanned out into one keyring lookup per key by
+// read()/write() (via Load/Status/Save/Delete). maxKeyringIndexKeys already
+// bounds the raw decode, but that bound does nothing against a corrupted or
+// adversarially crafted index that packs its budget with repeats of the same
+// key (or garbage that was never a real ValidateKey-shaped entry): every
+// duplicate or malformed key would otherwise still cost its own blocking
+// keyring lookup (up to the 10s command timeout) while the store lock is
+// held, reintroducing the fan-out DoS the index cap was meant to close.
+// Order is preserved (first occurrence wins) so callers that sort or display
+// keys see stable results.
+func dedupeValidKeys(keys []string) []string {
+	seen := make(map[string]bool, len(keys))
+	out := make([]string, 0, len(keys))
+	for _, key := range keys {
+		if seen[key] {
+			continue
+		}
+		if ValidateKey(key) != nil {
+			continue
+		}
+		seen[key] = true
+		out = append(out, key)
+	}
+	return out
 }
 
 // writeKeyIndex persists keys as a chunked index and reports how many chunk
diff --git a/internal/oauth/store_keyring_test.go b/internal/oauth/store_keyring_test.go
index 6b09c99c1..4a43a33ad 100644
--- a/internal/oauth/store_keyring_test.go
+++ b/internal/oauth/store_keyring_test.go
@@ -1059,6 +1059,106 @@ func TestStoreKeyringWriteIndexRejectsOverCapKeys(t *testing.T) {
 	}
 }
 
+// TestStoreKeyringReadIndexDedupesDuplicateKeys is the regression test for the
+// index fan-out DoS: a corrupted or adversarially crafted index that repeats
+// the same key many times must collapse to its distinct, valid keys before
+// read()/write() fan them out into one blocking keyring lookup per key.
+func TestStoreKeyringReadIndexDedupesDuplicateKeys(t *testing.T) {
+	ckr := &countingKR{fakeKR: newFakeKR()}
+	blob := keyringBlob{kr: ckr, service: keyringService, legacyAccount: keyringLegacyAccount, indexAccount: keyringIndexAccount}
+
+	dup := ProviderKey("demo")
+	many := make([]string, 2000)
+	for i := range many {
+		many[i] = dup
+	}
+	header, err := json.Marshal(keyIndexHeader{Version: 1, Chunks: 1, Keys: many})
+	if err != nil {
+		t.Fatal(err)
+	}
+	ckr.data[keyringService+"/"+keyringIndexAccount] = base64.StdEncoding.EncodeToString(header)
+
+	keys, ok, _, err := blob.readKeyIndex()
+	if err != nil {
+		t.Fatalf("readKeyIndex: %v", err)
+	}
+	if !ok {
+		t.Fatal("readKeyIndex: expected an index to be found")
+	}
+	if len(keys) != 1 {
+		t.Fatalf("readKeyIndex returned %d keys for a 2000-duplicate index, want 1 (deduplicated)", len(keys))
+	}
+
+	// A malformed (non-ValidateKey-shaped) entry must also be dropped rather
+	// than fanned out into a lookup.
+	mixed, err := json.Marshal(keyIndexHeader{Version: 1, Chunks: 1, Keys: []string{dup, dup, "not a valid key", ProviderKey("other")}})
+	if err != nil {
+		t.Fatal(err)
+	}
+	ckr.data[keyringService+"/"+keyringIndexAccount] = base64.StdEncoding.EncodeToString(mixed)
+	keys, _, _, err = blob.readKeyIndex()
+	if err != nil {
+		t.Fatalf("readKeyIndex: %v", err)
+	}
+	want := map[string]bool{dup: true, ProviderKey("other"): true}
+	if len(keys) != len(want) {
+		t.Fatalf("readKeyIndex = %v, want exactly %v", keys, want)
+	}
+	for _, k := range keys {
+		if !want[k] {
+			t.Fatalf("readKeyIndex returned unexpected key %q", k)
+		}
+	}
+}
+
+// TestStoreKeyringDuplicateIndexDoesNotFanOutPerEntry is the end-to-end
+// regression test for the same DoS: a duplicate-heavy index must not drive
+// one keyring Get per listed entry. Before the index is deduplicated at its
+// source, a corrupted index holding thousands of copies of the same key would
+// hold the store lock for one blocking keyring lookup (each up to the 10s
+// command timeout) per copy, even though the cap on total distinct credentials
+// this bug was meant to bound was never actually exceeded.
+func TestStoreKeyringDuplicateIndexDoesNotFanOutPerEntry(t *testing.T) {
+	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
+	ckr := &countingKR{fakeKR: newFakeKR()}
+
+	dup := ProviderKey("demo")
+	raw, err := json.Marshal(Token{AccessToken: "a"})
+	if err != nil {
+		t.Fatal(err)
+	}
+	ckr.data[keyringService+"/"+dup] = base64.StdEncoding.EncodeToString(raw)
+
+	many := make([]string, 3000)
+	for i := range many {
+		many[i] = dup
+	}
+	header, err := json.Marshal(keyIndexHeader{Version: 1, Chunks: 1, Keys: many})
+	if err != nil {
+		t.Fatal(err)
+	}
+	ckr.data[keyringService+"/"+keyringIndexAccount] = base64.StdEncoding.EncodeToString(header)
+
+	s, err := NewStore(StoreOptions{Storage: "keyring", Keyring: ckr})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	ckr.gets = 0
+	statuses, err := s.Status("")
+	if err != nil {
+		t.Fatalf("Status: %v", err)
+	}
+	if len(statuses) != 1 {
+		t.Fatalf("Status returned %d entries for a 3000-duplicate index of one key, want 1", len(statuses))
+	}
+	// One Get for the index header, one Get for the single deduplicated key's
+	// own entry — not one per (duplicate) index entry.
+	if ckr.gets > 2 {
+		t.Fatalf("Status issued %d keyring gets for a 3000-entry duplicate index, want <= 2 (fan-out DoS regression)", ckr.gets)
+	}
+}
+
 // legacyGetFailKR fails Get for the legacy combined entry only, simulating a
 // transient keyring read error (as opposed to the entry genuinely not
 // existing, which fakeKR's normal Get reports as ok=false, err=nil).

From 9e3f52c379c2bceaef73a8c5de21e96487c882ab Mon Sep 17 00:00:00 2001
From: euxaristia <25621994+euxaristia@users.noreply.github.com>
Date: Wed, 22 Jul 2026 12:38:31 -0400
Subject: [PATCH 15/17] fix(oauth): anchor the keyring lock on home dir and
 honor the legacy lock

The keyring index lock was derived from os.UserCacheDir(), which reads
XDG_CACHE_HOME (and falls back through os.TempDir()/TMPDIR) per process.
Two same-user zero processes with different cache or temp roots picked
different lock files, both read the shared keyring index, and could
publish competing read-modify-write updates that hid one process's
token. The lock now anchors on the user's home directory instead, which
does not vary this way between processes of the same real user.

Anchoring on the home directory also lines it up with where a pre-PR
binary resolves its own lock (beside ResolveStorePath, which falls back
to the home directory too). That pre-PR lock is respected directly now:
write() additionally holds it for the whole reconcile-then-delete pass
over the legacy combined entry, so a live old binary can't sneak in a
fresh legacy login or refresh between this binary's reconciliation read
and its legacy-blob delete and have that write silently discarded.

Added TestKeyringLockPathIndependentOfCacheAndTempRoots, which varies
XDG_CACHE_HOME/TMPDIR between two simulated processes and confirms they
still resolve to the same lock, and
TestStoreKeyringWriteWaitsForLegacyLockDuringReconciliation, which holds
the legacy lock as a live old binary would and confirms Save blocks
until it is released, with the seeded legacy token intact afterward.
---
 internal/oauth/store.go              | 173 +++++++++++++++++++--------
 internal/oauth/store_keyring_test.go | 121 ++++++++++++++++++-
 2 files changed, 240 insertions(+), 54 deletions(-)

diff --git a/internal/oauth/store.go b/internal/oauth/store.go
index 82272380b..1534c6780 100644
--- a/internal/oauth/store.go
+++ b/internal/oauth/store.go
@@ -152,13 +152,9 @@ func ResolveStorePath(env map[string]string) (string, error) {
 	}
 	configHome := strings.TrimSpace(envValue(env, "XDG_CONFIG_HOME"))
 	if configHome == "" {
-		home := strings.TrimSpace(firstNonEmpty(envValue(env, "HOME"), envValue(env, "USERPROFILE")))
-		if home == "" {
-			var err error
-			home, err = os.UserHomeDir()
-			if err != nil {
-				return "", fmt.Errorf("oauth: resolve user home: %w", err)
-			}
+		home, err := resolveHomeDir(env)
+		if err != nil {
+			return "", err
 		}
 		configHome = filepath.Join(home, ".config")
 	} else if !filepath.IsAbs(configHome) {
@@ -171,6 +167,24 @@ func ResolveStorePath(env map[string]string) (string, error) {
 	return filepath.Join(configHome, "zero", "oauth-tokens.json"), nil
 }
 
+// resolveHomeDir returns the user's home directory, honoring HOME/USERPROFILE
+// hermetically (via env) before falling back to os.UserHomeDir(). Shared by
+// ResolveStorePath's config-root fallback and by keyringLockPath, which
+// anchors on this same identity so the keyring lock never varies with a
+// per-process override like XDG_CACHE_HOME/XDG_CONFIG_HOME/TMPDIR that two
+// processes of the same real user commonly set differently (sandboxes, CI,
+// per-shell env).
+func resolveHomeDir(env map[string]string) (string, error) {
+	if home := strings.TrimSpace(firstNonEmpty(envValue(env, "HOME"), envValue(env, "USERPROFILE"))); home != "" {
+		return home, nil
+	}
+	home, err := os.UserHomeDir()
+	if err != nil {
+		return "", fmt.Errorf("oauth: resolve user home: %w", err)
+	}
+	return home, nil
+}
+
 // NewStore builds a token store with the configured backend (file by default,
 // or the OS keyring when Storage/ZERO_OAUTH_STORAGE selects it).
 func NewStore(options StoreOptions) (*Store, error) {
@@ -209,16 +223,20 @@ func NewStore(options StoreOptions) (*Store, error) {
 			}
 			kr = osKeyring
 		}
-		// Serialize the keyring's read-modify-write across processes with a lock
-		// file keyed off the keyring identity itself (service + index account),
-		// never off the file-backend's path config: two processes with different
-		// ZERO_OAUTH_TOKENS_PATH / XDG_CONFIG_HOME roots but pointed at the SAME
-		// OS keyring entry (the service/account is fixed per binary, not per
-		// config root) must still serialize against each other, or they can race
-		// a read-modify-write on the shared keyring index and silently drop one
-		// process's token write.
-		lockPath := keyringLockPath(keyringService, keyringIndexAccount)
-		return &Store{blob: keyringBlob{kr: kr, service: keyringService, legacyAccount: keyringLegacyAccount, indexAccount: keyringIndexAccount, lockPath: lockPath}, now: now}, nil
+		// lockPath serializes this binary's own keyring read-modify-write across
+		// processes, keyed off the keyring identity itself (service + index
+		// account) and anchored on the user's home directory (see
+		// keyringLockPath), never off a per-process cache/temp/config override:
+		// two processes with different roots but pointed at the SAME OS keyring
+		// entry (the service/account is fixed per binary, not per config root)
+		// must still serialize against each other, or they can race a
+		// read-modify-write on the shared keyring index and silently drop one
+		// process's token write. legacyLockPath additionally coordinates with a
+		// still-running pre-PR binary during the supported mixed-version window
+		// (see legacyKeyringLockPath).
+		lockPath := keyringLockPath(options.Env, keyringService, keyringIndexAccount)
+		legacyLockPath := legacyKeyringLockPath(options.Env)
+		return &Store{blob: keyringBlob{kr: kr, service: keyringService, legacyAccount: keyringLegacyAccount, indexAccount: keyringIndexAccount, lockPath: lockPath, legacyLockPath: legacyLockPath}, now: now}, nil
 	default:
 		return nil, fmt.Errorf("oauth: unknown storage %q (want \"file\", \"encrypted-file\", or \"keyring\")", storage)
 	}
@@ -245,27 +263,46 @@ func resolveStoreFilePath(options StoreOptions) (string, error) {
 
 // keyringLockPath returns the cross-process lock file location for the
 // keyring backend's read-modify-write, derived from the keyring identity
-// itself (the service/account the index is stored under) rather than from
-// the unrelated file-backend path config (ZERO_OAUTH_TOKENS_PATH /
-// XDG_CONFIG_HOME): the file backend's location has nothing to do with which
-// OS keyring entry a process is about to read-modify-write, so a lock keyed
-// off it let two processes with different config roots but the SAME keyring
-// entry race the shared index and silently drop one process's token write.
-// A single shared ${TMPDIR}/zero-oauth-keyring.lockfile would also let any
-// other account on a multi-user host pre-create or keep refreshing the
-// victim's lock and time out their Load/Status/Save/Delete, even though each
-// user has a separate OS keychain, so this prefers the per-user OS cache
-// directory (created 0700 by acquireFileLock); only if that cannot be
-// resolved does it fall back to a temp file scoped by uid so two different
-// users never collide on one path.
-func keyringLockPath(service, account string) string {
+// itself (the service/account the index is stored under) and anchored on the
+// user's home directory (see resolveHomeDir) rather than os.UserCacheDir() or
+// os.TempDir(): those pick XDG_CACHE_HOME/TMPDIR per PROCESS, so two
+// processes of the SAME real user with different cache/temp roots (a common
+// case: sandboxes, CI, per-shell overrides) computed different lock files,
+// both read the same fixed keyring index, and could publish competing
+// updates that silently hid one process's token. HOME does not vary this way
+// for a given real user. A single shared ${TMPDIR}/zero-oauth-keyring.lockfile
+// would also let any other account on a multi-user host pre-create or keep
+// refreshing the victim's lock and time out their Load/Status/Save/Delete,
+// even though each user has a separate OS keychain, so only when even the
+// home directory can't be resolved does this fall back to a temp file scoped
+// by uid so two different users never collide on one path.
+func keyringLockPath(env map[string]string, service, account string) string {
 	name := keyringLockFileName(service, account)
-	if dir, err := os.UserCacheDir(); err == nil && strings.TrimSpace(dir) != "" {
-		return filepath.Join(dir, "zero", name)
+	if home, err := resolveHomeDir(env); err == nil && strings.TrimSpace(home) != "" {
+		return filepath.Join(home, ".cache", "zero", name)
 	}
 	return filepath.Join(os.TempDir(), keyringTempLockName(service, account))
 }
 
+// legacyKeyringLockPath returns the lock file a pre-PR binary acquires around
+// its own read-modify-write of the single combined keyring entry, beside
+// wherever ResolveStorePath resolves the file-backend location for that
+// process's env. A new binary must take this SAME lock (not just its own
+// keyringLockPath) around any write that reconciles or deletes the legacy
+// entry: an old binary observes no other lock, so only sharing its exact
+// lock file stops it from writing a fresh legacy login/refresh in the window
+// between this binary's reconciliation read and its legacy-blob delete,
+// which would otherwise discard that write permanently. Best-effort: ""
+// when the file-backend location can't be resolved at all, matching the
+// legacy code's own best-effort fallback (no cross-process lock at all).
+func legacyKeyringLockPath(env map[string]string) string {
+	storePath, err := ResolveStorePath(env)
+	if err != nil {
+		return ""
+	}
+	return filepath.Join(filepath.Dir(storePath), "oauth-keyring.lockfile")
+}
+
 // keyringLockFileName names the lock file after the keyring identity it
 // guards, so distinct (service, account) pairs never share a lock and the
 // same pair always resolves to the same lock regardless of caller config.
@@ -548,6 +585,12 @@ type keyringBlob struct {
 	// lockPath, when set, is a cross-process lock file serializing the keyring's
 	// read-modify-write so concurrent processes don't clobber each other's tokens.
 	lockPath string
+	// legacyLockPath, when set, is the lock file a pre-PR binary acquires around
+	// its own read-modify-write of the legacy combined entry (see
+	// legacyKeyringLockPath). write() holds it too, so a live old binary can't
+	// write a fresh legacy credential in the window between this write's legacy
+	// reconciliation and its legacy-blob delete.
+	legacyLockPath string
 }
 
 func (b keyringBlob) read() ([]byte, bool, error) {
@@ -1001,20 +1044,30 @@ func chunkIndexKeys(keys []string) [][]string {
 // exists to prevent. A var so tests can shorten it.
 var fileLockRefreshInterval = 10 * time.Second
 
-// withLock serializes the keyring's read-modify-write. Store.mu covers the
-// in-process case; lockPath (when set) adds cross-process exclusion so two
-// processes can't both read the blob, modify, and write — dropping a token.
-// While fn runs, the lock file's mtime is refreshed so the stale-reclaim
-// threshold only ever expires for a genuinely crashed holder.
-func (b keyringBlob) withLock(now func() time.Time, fn func() error) error {
-	if b.lockPath == "" {
-		return fn()
+// withLeasedLocks acquires every non-empty path in order, refreshes all of
+// their mtimes on a ticker while fn runs (so a legitimately slow multi-entry
+// keyring pass never looks like a crashed holder to another process), and
+// releases them in reverse order once fn returns.
+func withLeasedLocks(paths []string, now func() time.Time, fn func() error) error {
+	var unlocks []func()
+	var held []string
+	for _, p := range paths {
+		if p == "" {
+			continue
+		}
+		unlock, err := acquireFileLock(p, now)
+		if err != nil {
+			for i := len(unlocks) - 1; i >= 0; i-- {
+				unlocks[i]()
+			}
+			return err
+		}
+		unlocks = append(unlocks, unlock)
+		held = append(held, p)
 	}
-	unlock, err := acquireFileLock(b.lockPath, now)
-	if err != nil {
-		return err
+	if len(held) == 0 {
+		return fn()
 	}
-	defer unlock()
 	stop := make(chan struct{})
 	done := make(chan struct{})
 	go func() {
@@ -1028,22 +1081,42 @@ func (b keyringBlob) withLock(now func() time.Time, fn func() error) error {
 			case <-ticker.C:
 				// Lease with wall-clock time, never the injectable now: acquireFileLock
 				// judges staleness with real time.Since(mtime), so a fixed or stale
-				// StoreOptions.Now would stamp the live lock with an old mtime that
+				// StoreOptions.Now would stamp a live lock with an old mtime that
 				// another process would immediately reclaim, reviving the token-loss
-				// race this lease prevents.
+				// race these locks prevent.
 				at := time.Now()
-				_ = os.Chtimes(b.lockPath, at, at)
+				for _, p := range held {
+					_ = os.Chtimes(p, at, at)
+				}
 			}
 		}
 	}()
-	err = fn()
+	err := fn()
 	close(stop)
 	<-done
+	for i := len(unlocks) - 1; i >= 0; i-- {
+		unlocks[i]()
+	}
 	return err
 }
 
+// withLock serializes the keyring's read-modify-write. Store.mu covers the
+// in-process case; lockPath adds cross-process exclusion between this
+// binary's own instances so two of them can't both read the blob, modify,
+// and write — dropping a token. legacyLockPath is held for the same
+// duration so a live pre-PR binary, which only ever locks there (see
+// legacyKeyringLockPath), can't write a fresh legacy credential in the
+// window between this write's legacy reconciliation and its legacy-blob
+// delete.
+func (b keyringBlob) withLock(now func() time.Time, fn func() error) error {
+	return withLeasedLocks([]string{b.lockPath, b.legacyLockPath}, now, fn)
+}
+
+// withReadLock only takes lockPath: a pre-PR binary never locks for a read
+// (see legacyKeyringLockPath), so a read here has nothing to coordinate with
+// on the legacy side.
 func (b keyringBlob) withReadLock(now func() time.Time, fn func() error) error {
-	return b.withLock(now, fn)
+	return withLeasedLocks([]string{b.lockPath}, now, fn)
 }
 
 func (b keyringBlob) location() string { return "keyring:" + b.service + "/" + b.indexAccount }
diff --git a/internal/oauth/store_keyring_test.go b/internal/oauth/store_keyring_test.go
index 4a43a33ad..5cba1981d 100644
--- a/internal/oauth/store_keyring_test.go
+++ b/internal/oauth/store_keyring_test.go
@@ -901,14 +901,14 @@ func TestStoreKeyringReadIndexRejectsOversizedKeyList(t *testing.T) {
 // and the last-resort temp name must be scoped by uid so different users
 // never collide on one lock file.
 func TestKeyringLockPathIsPerUser(t *testing.T) {
-	got := keyringLockPath(keyringService, keyringIndexAccount)
+	got := keyringLockPath(nil, keyringService, keyringIndexAccount)
 	name := keyringLockFileName(keyringService, keyringIndexAccount)
 	if got == filepath.Join(os.TempDir(), "zero-"+name) {
 		t.Fatalf("lock path is the shared temp path %q; a co-tenant could grief it", got)
 	}
-	if cache, err := os.UserCacheDir(); err == nil && strings.TrimSpace(cache) != "" {
-		if want := filepath.Join(cache, "zero", name); got != want {
-			t.Fatalf("lock path = %q, want per-user cache path %q", got, want)
+	if home, err := os.UserHomeDir(); err == nil && strings.TrimSpace(home) != "" {
+		if want := filepath.Join(home, ".cache", "zero", name); got != want {
+			t.Fatalf("lock path = %q, want per-user home-anchored path %q", got, want)
 		}
 	}
 	tempName := keyringTempLockName(keyringService, keyringIndexAccount)
@@ -921,6 +921,119 @@ func TestKeyringLockPathIsPerUser(t *testing.T) {
 	}
 }
 
+// TestKeyringLockPathIndependentOfCacheAndTempRoots is the regression test
+// for [P1] Make the keyring lock path independent of XDG_CACHE_HOME
+// (2026-07-22): os.UserCacheDir() (and its os.TempDir() fallback) is chosen
+// per PROCESS from XDG_CACHE_HOME/TMPDIR, so two zero processes belonging to
+// the SAME real user but with different cache/temp roots (sandboxes, CI,
+// per-shell overrides) computed different lock files, both read the shared
+// keyring index, and could publish competing updates that hid one process's
+// token. The lock must resolve identically regardless of those roots, since
+// it is anchored on the user's home directory instead.
+func TestKeyringLockPathIndependentOfCacheAndTempRoots(t *testing.T) {
+	home := t.TempDir()
+	envA := map[string]string{
+		"HOME":           home,
+		"XDG_CACHE_HOME": filepath.Join(t.TempDir(), "cache-a"),
+		"TMPDIR":         filepath.Join(t.TempDir(), "tmp-a"),
+	}
+	envB := map[string]string{
+		"HOME":           home,
+		"XDG_CACHE_HOME": filepath.Join(t.TempDir(), "cache-b"),
+		"TMPDIR":         filepath.Join(t.TempDir(), "tmp-b"),
+	}
+
+	storeA, err := NewStore(StoreOptions{Storage: "keyring", Keyring: newFakeKR(), Env: envA})
+	if err != nil {
+		t.Fatal(err)
+	}
+	storeB, err := NewStore(StoreOptions{Storage: "keyring", Keyring: newFakeKR(), Env: envB})
+	if err != nil {
+		t.Fatal(err)
+	}
+	blobA, okA := storeA.blob.(keyringBlob)
+	blobB, okB := storeB.blob.(keyringBlob)
+	if !okA || !okB {
+		t.Fatal("Store.blob is not keyringBlob")
+	}
+	if blobA.lockPath == "" || blobB.lockPath == "" {
+		t.Fatal("lockPath should never be empty for the keyring backend")
+	}
+	if blobA.lockPath != blobB.lockPath {
+		t.Fatalf("two same-user processes with different cache/temp roots got different lock paths: %q vs %q (they can race the shared keyring index)", blobA.lockPath, blobB.lockPath)
+	}
+	if strings.Contains(blobA.lockPath, "cache-a") || strings.Contains(blobA.lockPath, "cache-b") ||
+		strings.Contains(blobA.lockPath, "tmp-a") || strings.Contains(blobA.lockPath, "tmp-b") {
+		t.Fatalf("lock path %q is still derived from XDG_CACHE_HOME/TMPDIR", blobA.lockPath)
+	}
+}
+
+// TestStoreKeyringWriteWaitsForLegacyLockDuringReconciliation is the
+// regression test for [P1] Coordinate migration with the legacy lock used by
+// old binaries (2026-07-22): a pre-PR binary locks beside ResolveStorePath
+// around its own read-modify-write of the legacy combined entry, but this
+// binary's write() used to lock only under the cache-derived keyring path, so
+// the two never excluded each other. A new save could reconcile the legacy
+// blob, an old process could then save a fresh legacy credential, and the new
+// save would unconditionally delete that blob without ever having observed
+// the old write, losing it permanently.
+//
+// This simulates a live old binary by holding the exact lock file
+// legacyKeyringLockPath computes (the same one a pre-PR binary's Save takes)
+// and asserting that Store.Save — which must reconcile and then drop the
+// legacy entry here, since one is seeded below — blocks until that lock is
+// released, and that the seeded legacy token survives the reconciliation.
+func TestStoreKeyringWriteWaitsForLegacyLockDuringReconciliation(t *testing.T) {
+	dir := t.TempDir()
+	t.Setenv("XDG_CONFIG_HOME", dir)
+	kr := newFakeKR()
+	legacy := storeFile{SchemaVersion: storeSchemaVersion, Tokens: map[string]Token{
+		ProviderKey("alpha"): {AccessToken: "legacy-alpha"},
+	}}
+	data, err := json.Marshal(legacy)
+	if err != nil {
+		t.Fatal(err)
+	}
+	kr.data[keyringService+"/"+keyringLegacyAccount] = base64.StdEncoding.EncodeToString(data)
+
+	s, err := NewStore(StoreOptions{Storage: "keyring", Keyring: kr})
+	if err != nil {
+		t.Fatal(err)
+	}
+	blob, ok := s.blob.(keyringBlob)
+	if !ok || blob.legacyLockPath == "" {
+		t.Fatal("keyring store has no legacy-compat lock path")
+	}
+
+	// Simulate an old binary holding the legacy lock for its own in-flight Save.
+	unlock, err := acquireFileLock(blob.legacyLockPath, s.now)
+	if err != nil {
+		t.Fatalf("acquire simulated legacy lock: %v", err)
+	}
+
+	saveDone := make(chan error, 1)
+	go func() {
+		saveDone <- s.Save(ProviderKey("beta"), Token{AccessToken: "beta"})
+	}()
+
+	select {
+	case err := <-saveDone:
+		t.Fatalf("Save proceeded (err=%v) while an old binary held the legacy lock; it can race the reconcile-then-delete window and lose a concurrent legacy write", err)
+	case <-time.After(200 * time.Millisecond):
+		// Still blocked, as expected.
+	}
+
+	unlock()
+	if err := <-saveDone; err != nil {
+		t.Fatalf("Save after the legacy lock was released: %v", err)
+	}
+
+	got, ok, err := s.Load(ProviderKey("alpha"))
+	if err != nil || !ok || got.AccessToken != "legacy-alpha" {
+		t.Fatalf("legacy alpha token lost across reconciliation: ok=%v err=%v got=%#v", ok, err, got)
+	}
+}
+
 // TestKeyringLockPathDerivedFromKeyringIdentityNotFileConfig is the
 // regression test for the cross-process lock racing bug: the lock guarding
 // the shared keyring index must be keyed off the keyring's own identity

From 226852b53eb9851770c610b5a431730d58f51e96 Mon Sep 17 00:00:00 2001
From: euxaristia <25621994+euxaristia@users.noreply.github.com>
Date: Thu, 23 Jul 2026 00:21:11 -0400
Subject: [PATCH 16/17] fix(oauth): preserve token scopes across refresh and
 encode keyring lock file components

---
 internal/oauth/flow.go      |  9 +++++----
 internal/oauth/flow_test.go | 15 +++++++++++++++
 internal/oauth/store.go     |  3 ++-
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/internal/oauth/flow.go b/internal/oauth/flow.go
index 9ff94cccd..42da93986 100644
--- a/internal/oauth/flow.go
+++ b/internal/oauth/flow.go
@@ -177,10 +177,11 @@ func Refresh(ctx context.Context, client *http.Client, cfg Config, current Token
 	if len(cfg.Scopes) > 0 {
 		form.Set("scope", strings.Join(cfg.Scopes, " "))
 	}
-	// Carry the existing token_type forward: a refresh response commonly omits it,
-	// and PostToken only overwrites TokenType when the response supplies one, so
-	// without seeding it here the type would be silently lost across refreshes (L15).
-	base := Token{Scopes: current.Scopes, RefreshToken: refresh, Account: current.Account, IDToken: current.IDToken, TokenType: current.TokenType}
+	scopes := current.Scopes
+	if len(scopes) == 0 {
+		scopes = cfg.Scopes
+	}
+	base := Token{Scopes: scopes, RefreshToken: refresh, Account: current.Account, IDToken: current.IDToken, TokenType: current.TokenType}
 	return PostToken(ctx, client, cfg.TokenEndpoint, form, base, now)
 }
 
diff --git a/internal/oauth/flow_test.go b/internal/oauth/flow_test.go
index 09ef01251..4b1da98d5 100644
--- a/internal/oauth/flow_test.go
+++ b/internal/oauth/flow_test.go
@@ -299,3 +299,18 @@ func TestRefreshPreservesTokenTypeWhenOmitted(t *testing.T) {
 		t.Fatalf("refresh should carry the existing token_type forward, got %q", tok.TokenType)
 	}
 }
+
+func TestRefreshPreservesScopesWhenOmitted(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		_, _ = w.Write([]byte(`{"access_token":"new-at","expires_in":3600}`)) // no scope in response
+	}))
+	defer server.Close()
+	cfg := Config{ClientID: "c", TokenEndpoint: server.URL, Scopes: []string{"fallback-scope"}}
+	tok, err := Refresh(context.Background(), server.Client(), cfg, Token{RefreshToken: "keep-me", Scopes: []string{"custom-scope"}}, nil)
+	if err != nil {
+		t.Fatalf("Refresh: %v", err)
+	}
+	if len(tok.Scopes) != 1 || tok.Scopes[0] != "custom-scope" {
+		t.Fatalf("refresh should carry existing scopes forward, got %v", tok.Scopes)
+	}
+}
diff --git a/internal/oauth/store.go b/internal/oauth/store.go
index 1534c6780..cbf471455 100644
--- a/internal/oauth/store.go
+++ b/internal/oauth/store.go
@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"net/url"
 	"os"
 	"path/filepath"
 	"regexp"
@@ -307,7 +308,7 @@ func legacyKeyringLockPath(env map[string]string) string {
 // guards, so distinct (service, account) pairs never share a lock and the
 // same pair always resolves to the same lock regardless of caller config.
 func keyringLockFileName(service, account string) string {
-	return fmt.Sprintf("oauth-keyring-%s-%s.lockfile", sanitizeLockComponent(service), sanitizeLockComponent(account))
+	return fmt.Sprintf("oauth-keyring-%s-%s.lockfile", sanitizeLockComponent(url.QueryEscape(service)), sanitizeLockComponent(url.QueryEscape(account)))
 }
 
 // lockComponentSafe keeps a service/account string safe as one path segment:

From ee4163d3177257ed93203c4a08ef22432b9ef26c Mon Sep 17 00:00:00 2001
From: euxaristia <25621994+euxaristia@users.noreply.github.com>
Date: Thu, 23 Jul 2026 20:50:58 -0400
Subject: [PATCH 17/17] fix(oauth): address review findings on legacy
 freshness, unindexed keys, lock path, and keyring key cap

---
 internal/oauth/store.go | 36 +++++++++++++++++++++++++++++-------
 1 file changed, 29 insertions(+), 7 deletions(-)

diff --git a/internal/oauth/store.go b/internal/oauth/store.go
index cbf471455..c062f56c7 100644
--- a/internal/oauth/store.go
+++ b/internal/oauth/store.go
@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"log"
 	"net/url"
 	"os"
 	"path/filepath"
@@ -297,11 +298,11 @@ func keyringLockPath(env map[string]string, service, account string) string {
 // when the file-backend location can't be resolved at all, matching the
 // legacy code's own best-effort fallback (no cross-process lock at all).
 func legacyKeyringLockPath(env map[string]string) string {
-	storePath, err := ResolveStorePath(env)
+	home, err := resolveHomeDir(nil)
 	if err != nil {
 		return ""
 	}
-	return filepath.Join(filepath.Dir(storePath), "oauth-keyring.lockfile")
+	return filepath.Join(home, ".config", "zero", "oauth-keyring.lockfile")
 }
 
 // keyringLockFileName names the lock file after the keyring identity it
@@ -649,6 +650,21 @@ func (b keyringBlob) read() ([]byte, bool, error) {
 		}
 		tokens[key] = token
 	}
+
+	if !legacyLoaded {
+		if lt, lerr := b.readLegacyTokens(); lerr == nil {
+			legacyTokens = lt
+		}
+	}
+	for key, legacyToken := range legacyTokens {
+		if ValidateKey(key) != nil {
+			continue
+		}
+		if _, exists := tokens[key]; !exists {
+			tokens[key] = legacyToken
+		}
+	}
+
 	data, err := json.Marshal(storeFile{SchemaVersion: storeSchemaVersion, Tokens: tokens})
 	if err != nil {
 		return nil, false, err
@@ -698,11 +714,16 @@ func (b keyringBlob) readLegacyTokens() (map[string]Token, error) {
 // legacyIsFresher reports whether the legacy copy of an already-indexed key
 // should win over the indexed copy. An old binary running alongside the new one
 // refreshes tokens only in the legacy combined entry, and a refresh pushes the
-// expiry later, so a strictly later, non-zero expiry on the legacy side is the
-// signal that it holds a newer credential. A zero (unknown) expiry on either
-// side is not evidence of freshness, so the indexed value is kept.
+// expiry later, so a strictly later expiry on the legacy side is the
+// signal that it holds a newer credential. Zero (unknown) expiries are valid.
 func legacyIsFresher(legacy, current Token) bool {
-	return !legacy.ExpiresAt.IsZero() && !current.ExpiresAt.IsZero() && legacy.ExpiresAt.After(current.ExpiresAt)
+	if legacy.ExpiresAt.After(current.ExpiresAt) {
+		return true
+	}
+	if legacy.ExpiresAt.Equal(current.ExpiresAt) {
+		return legacy.AccessToken != current.AccessToken || legacy.RefreshToken != current.RefreshToken
+	}
+	return false
 }
 
 // write replaces the keyring's token entries with state, ordered so that
@@ -859,11 +880,12 @@ const maxKeyringIndexChunks = 128
 // produces (short namespaced keys cost at least ~18 bytes each, so one
 // maxKeyringIndexChunkBytes chunk holds on the order of a hundred, times
 // maxKeyringIndexChunks) while still rejecting a damaged index promptly.
-const maxKeyringIndexKeys = maxKeyringIndexChunks * 200
+const maxKeyringIndexKeys = 512
 
 // errKeyringIndexTooManyKeys is returned when a decoded index (or one of its
 // chunks) claims more keys than maxKeyringIndexKeys.
 func errKeyringIndexTooManyKeys(count int) error {
+	log.Printf("warning: oauth: keyring token index lists %d keys, over the %d-key cap", count, maxKeyringIndexKeys)
 	return fmt.Errorf("oauth: keyring token index lists %d keys, over the %d-key cap", count, maxKeyringIndexKeys)
 }