diff --git a/internal/oauth/store.go b/internal/oauth/store.go index 951e9616..f1a669a1 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, @@ -197,13 +210,16 @@ 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 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") } - 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) } @@ -228,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() } @@ -256,7 +296,19 @@ func (s *Store) Load(key string) (Token, bool, error) { } s.mu.Lock() defer s.mu.Unlock() - state, err := s.readState() + // 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.withReadLock(s.now, func() error { + var readErr error + state, readErr = s.readState() + return readErr + }) if err != nil { return Token{}, false, err } @@ -292,7 +344,15 @@ 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.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.withReadLock(s.now, func() error { + var readErr error + state, readErr = s.readState() + return readErr + }) if err != nil { return nil, err } @@ -386,6 +446,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 } @@ -429,21 +496,93 @@ 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 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) + keys, ok, _, err := b.readKeyIndex() + if err != nil { + return nil, false, err + } + 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) + if err != nil { + return nil, false, err + } + if !ok { + // 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)) + 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 } @@ -454,13 +593,293 @@ func (b keyringBlob) read() ([]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 +// 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() 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 { - 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, indexExisted, priorChunks, err := b.readKeyIndex() + if 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, 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 { + 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 + } + } + + keys := make([]string, 0, len(state.Tokens)) + for key := range state.Tokens { + keys = append(keys, key) + } + sort.Strings(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 + } + // 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 { + return err + } + } + } + // 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 (its fresh writes were merged above). + _, _ = b.kr.Delete(b.service, b.legacyAccount) + return nil +} + +// 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 + +// 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 +// "-" 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 { + return nil, false, 0, err + } + if !ok { + return nil, false, 0, nil + } + raw, err := base64.StdEncoding.DecodeString(strings.TrimSpace(enc)) + if err != nil { + 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) + } + // 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)) + 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...) + } + return keys, true, header.Chunks, nil +} + +// 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 0, 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 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() @@ -470,10 +889,38 @@ 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: + // 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) + } + } + }() + 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.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 8931dc6d..e615a18e 100644 --- a/internal/oauth/store_keyring_test.go +++ b/internal/oauth/store_keyring_test.go @@ -1,8 +1,14 @@ package oauth import ( + "encoding/base64" + "encoding/json" + "fmt" + "os" + "path/filepath" "strings" "testing" + "time" ) // fakeKR is an in-memory KeyringClient for exercising the keyring backend @@ -49,13 +55,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 +75,406 @@ 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) + } + } +} + +// 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) + } +} + +// 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) { @@ -93,6 +503,81 @@ 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) + } + 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) { t.Setenv("XDG_CONFIG_HOME", t.TempDir()) kr := newFakeKR() @@ -111,3 +596,211 @@ 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") + } +} + +// 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) + } +} + +// 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") + } +} + +// 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") + } +}