fix(oauth): serialize keyring operations under user-level lock

Derive the keyring backend lock file path from the user's home directory rather than file store configuration, ensuring processes with distinct store paths share the same lock domain for the OS keychain.

Refs #938

Author: euxaristia

internal/oauth/store.go

@@ -100,6 +100,10 @@ type StoreOptions struct {
 	// Keyring is the client used when Storage=="keyring"; nil => keyring.New().
 	// Injected by tests to avoid touching a real keychain.
 	Keyring KeyringClient
+	// KeyringLockPath overrides the cross-process lock file used by the keyring
+	// backend. When empty, it is derived from the keyring storage identity
+	// via ResolveKeyringLockPath(Env).
+	KeyringLockPath string
 }
 
 // KeyringClient is the minimal OS-keyring surface the store needs. *keyring.Keyring
@@ -191,6 +195,35 @@ func ResolveStorePath(env map[string]string) (string, error) {
 	return filepath.Join(configHome, "zero", "oauth-tokens.json"), nil
 }
 
+// ResolveKeyringLockPath determines the on-disk cross-process lock file for the
+// keyring backend. Unlike file-backed storage, keyring entries are anchored to
+// the user's OS keychain (zero/oauth-tokens) and are shared across all processes
+// for that OS user, regardless of file-store overrides such as
+// ZERO_OAUTH_TOKENS_PATH, FilePath, or XDG_CONFIG_HOME.
+func ResolveKeyringLockPath(env map[string]string) (string, error) {
+	if override := strings.TrimSpace(envValue(env, "ZERO_OAUTH_KEYRING_LOCK_PATH")); override != "" {
+		if filepath.IsAbs(override) {
+			return filepath.Clean(override), nil
+		}
+		return filepath.Abs(override)
+	}
+	home := strings.TrimSpace(firstNonEmpty(envValue(env, "HOME"), envValue(env, "USERPROFILE")))
+	if home == "" {
+		var err error
+		home, err = os.UserHomeDir()
+		if err != nil {
+			return "", fmt.Errorf("oauth: resolve user home for keyring lock: %w", err)
+		}
+	} else if !filepath.IsAbs(home) {
+		resolved, err := filepath.Abs(home)
+		if err != nil {
+			return "", err
+		}
+		home = resolved
+	}
+	return filepath.Join(home, ".zero", "oauth-keyring.lockfile"), nil
+}
+
 // NewStore builds a token store with the configured backend (file by default,
 // or the OS keyring when Storage/ZERO_OAUTH_STORAGE selects it).
 func NewStore(options StoreOptions) (*Store, error) {
@@ -230,11 +263,15 @@ 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 := ""
-		if storePath, perr := ResolveStorePath(options.Env); perr == nil {
-			lockPath = filepath.Join(filepath.Dir(storePath), "oauth-keyring.lockfile")
+		// file derived from the keyring storage identity ("zero"/"oauth-tokens").
+		// Unlike the file backend, the keyring does not vary with file configuration
+		// (ZERO_OAUTH_TOKENS_PATH, FilePath, or XDG_CONFIG_HOME). Best-effort: if no
+		// home location resolves, fall back to in-process serialization only.
+		lockPath := strings.TrimSpace(options.KeyringLockPath)
+		if lockPath == "" {
+			if lp, perr := ResolveKeyringLockPath(options.Env); perr == nil {
+				lockPath = lp
+			}
 		}
 		return &Store{blob: keyringBlob{kr: kr, service: keyringService, account: keyringAccount, lockPath: lockPath}, now: now}, nil
 	default:

internal/oauth/store_keyring_chunked_test.go

@@ -2,6 +2,7 @@ package oauth
 
 import (
 	"errors"
+	"path/filepath"
 	"strconv"
 	"strings"
 	"testing"
@@ -30,7 +31,10 @@ func bigToken(seed string) Token {
 
 func newCappedKeyringStore(t *testing.T, kr KeyringClient) *Store {
 	t.Helper()
-	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
+	dir := t.TempDir()
+	t.Setenv("HOME", dir)
+	t.Setenv("USERPROFILE", dir)
+	t.Setenv("XDG_CONFIG_HOME", dir)
 	s, err := NewStore(StoreOptions{Storage: "keyring", Keyring: kr})
 	if err != nil {
 		t.Fatalf("NewStore(keyring): %v", err)
@@ -486,6 +490,8 @@ func TestStoreKeyringSweepsStrayChunksOnFirstGrowth(t *testing.T) {
 // has deleted old-generation chunks).
 func TestStoreKeyringReadSerializedWithLockDuringChunkedWrite(t *testing.T) {
 	dir := t.TempDir()
+	t.Setenv("HOME", dir)
+	t.Setenv("USERPROFILE", dir)
 	t.Setenv("XDG_CONFIG_HOME", dir)
 
 	kr := newCappedFakeKR(macOSLikeBudget)
@@ -565,6 +571,112 @@ func TestStoreKeyringReadSerializedWithLockDuringChunkedWrite(t *testing.T) {
 	}
 }
 
+// TestStoreKeyringSharedLockAcrossDifferentEnvironmentRoots is the regression for
+// split lock domains when processes run with different ZERO_OAUTH_TOKENS_PATH,
+// FilePath, or XDG_CONFIG_HOME. Because the OS keychain is single-domain for the
+// user ("zero"/"oauth-tokens"), all stores must share the same lock file so
+// concurrent reads, writes, and manifest rotations remain strictly serialized.
+func TestStoreKeyringSharedLockAcrossDifferentEnvironmentRoots(t *testing.T) {
+	homeDir := t.TempDir()
+	t.Setenv("HOME", homeDir)
+	t.Setenv("USERPROFILE", homeDir)
+
+	dirA := t.TempDir()
+	dirB := t.TempDir()
+
+	kr := newCappedFakeKR(macOSLikeBudget)
+	storeA, err := NewStore(StoreOptions{
+		Storage:  "keyring",
+		Keyring:  kr,
+		FilePath: filepath.Join(dirA, "custom-tokens.json"),
+		Env: map[string]string{
+			"ZERO_OAUTH_TOKENS_PATH": filepath.Join(dirA, "custom-tokens.json"),
+			"XDG_CONFIG_HOME":        dirA,
+		},
+	})
+	if err != nil {
+		t.Fatalf("NewStore(storeA): %v", err)
+	}
+
+	storeB, err := NewStore(StoreOptions{
+		Storage:  "keyring",
+		Keyring:  kr,
+		FilePath: filepath.Join(dirB, "custom-tokens.json"),
+		Env: map[string]string{
+			"ZERO_OAUTH_TOKENS_PATH": filepath.Join(dirB, "custom-tokens.json"),
+			"XDG_CONFIG_HOME":        dirB,
+		},
+	})
+	if err != nil {
+		t.Fatalf("NewStore(storeB): %v", err)
+	}
+
+	lockA := storeA.blob.(keyringBlob).lockPath
+	lockB := storeB.blob.(keyringBlob).lockPath
+	if lockA == "" || lockB == "" {
+		t.Fatalf("lockPath should not be empty: lockA=%q lockB=%q", lockA, lockB)
+	}
+	if lockA != lockB {
+		t.Fatalf("stores with distinct env roots must share the keyring lock: lockA=%q != lockB=%q", lockA, lockB)
+	}
+
+	mustSave(t, storeA, "keyA", bigToken("a"))
+
+	// 1. Force overlapping save/read: hold the lock manually to simulate an
+	// in-flight multi-step write by storeA, and verify storeB's Load / Status blocks.
+	unlock, err := acquireFileLock(lockA, time.Now)
+	if err != nil {
+		t.Fatalf("acquireFileLock: %v", err)
+	}
+
+	loaded := make(chan Token, 1)
+	loadErr := make(chan error, 1)
+	go func() {
+		tok, _, err := storeB.Load(ProviderKey("keyA"))
+		loadErr <- err
+		loaded <- tok
+	}()
+
+	select {
+	case <-loaded:
+		t.Fatal("storeB.Load completed while storeA's lock was held")
+	case <-time.After(50 * time.Millisecond):
+	}
+
+	unlock()
+
+	select {
+	case err := <-loadErr:
+		if err != nil {
+			t.Fatalf("storeB.Load failed after unlock: %v", err)
+		}
+		tok := <-loaded
+		if tok.AccessToken != bigToken("a").AccessToken {
+			t.Errorf("storeB.Load got unexpected token: %v", tok)
+		}
+	case <-time.After(2 * time.Second):
+		t.Fatal("storeB.Load timed out waiting for lock release")
+	}
+
+	// 2. Force overlapping save/save: write from storeB and storeA consecutively,
+	// verifying both updates are preserved and readable by either store.
+	if err := storeB.Save(ProviderKey("keyB"), bigToken("b")); err != nil {
+		t.Fatalf("storeB.Save: %v", err)
+	}
+	if err := storeA.Save(ProviderKey("keyC"), bigToken("c")); err != nil {
+		t.Fatalf("storeA.Save: %v", err)
+	}
+
+	gotA := mustLoad(t, storeB, "keyA")
+	gotB := mustLoad(t, storeA, "keyB")
+	gotC := mustLoad(t, storeB, "keyC")
+	if gotA.AccessToken != bigToken("a").AccessToken ||
+		gotB.AccessToken != bigToken("b").AccessToken ||
+		gotC.AccessToken != bigToken("c").AccessToken {
+		t.Errorf("concurrent state was not preserved across stores")
+	}
+}
+
 // TestStoreKeyringShrinkResidueIsReclaimedOnRegrowth is the regression for a
 // retired generation orphaned permanently. Cleanup is hygiene rather than
 // correctness only while a manifest exists to state the counts; writeWhole

internal/oauth/store_keyring_test.go

@@ -91,8 +91,10 @@ func (f *fakeKR) chunkAccounts(family string) []string {
 }
 
 func TestStoreKeyringBackendRoundTrip(t *testing.T) {
-	// Keep the cross-process keyring lock file inside a temp config dir.
-	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
+	// Keep the cross-process keyring lock file inside a temp dir.
+	dir := t.TempDir()
+	t.Setenv("HOME", dir)
+	t.Setenv("USERPROFILE", dir)
 	kr := newFakeKR()
 	s, err := NewStore(StoreOptions{Storage: "keyring", Keyring: kr})
 	if err != nil {
@@ -158,7 +160,9 @@ func TestNewStoreStorageSelection(t *testing.T) {
 }
 
 func TestStoreKeyringStatus(t *testing.T) {
-	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
+	dir := t.TempDir()
+	t.Setenv("HOME", dir)
+	t.Setenv("USERPROFILE", dir)
 	kr := newFakeKR()
 	s, err := NewStore(StoreOptions{Storage: "keyring", Keyring: kr})
 	if err != nil {