From d9dacd4bf0ad10b9cd61af5be8e80420f4295b1c 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 1/8] 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 26e69ba14de019c5596ed0053a85a62482568548 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 2/8] 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 697951b3fbf3f6c9f27fb3d0cb27477968472629 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 3/8] 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 6d8b05f59afab56a5eefae869deb2ca8220e6a88 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 4/8] 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 6e210bfbbc9ca66f015237610fccdd9939a1ab7b 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 5/8] 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 076c7d4d038b707f4c28cf08c0aaf89f64a7f641 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 6/8] 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 a2cbf2388e74a6aa4c5db429da5bea756be8ca73 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 7/8] 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 11d038df90fa6e1b542a56e36a61066b11b4ace2 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 8/8] 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")
+	}
+}