fix(credentials): serialize profile writes with cross-process file locking

writeProfile and deleteProfile read-modify-write the credentials file; without a lock, concurrent CLI processes can each read the same snapshot and the last atomic rename wins, silently dropping the other update.

Acquire an exclusive lock file ({path}.lock) before read-modify-write, with stale-lock reclamation and a bounded retry loop. Add subprocess regression tests proving concurrent writes to different profiles both survive.

Author: SahilRakhaiya05

src/lib/credentials.test.ts

@@ -1,7 +1,11 @@
+import { execSync } from 'node:child_process';
+import type { ChildProcess } from 'node:child_process';
+import { spawn } from 'node:child_process';
 import { mkdtempSync, statSync, readFileSync, writeFileSync, existsSync, mkdirSync } from 'node:fs';
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';
-import { afterEach, beforeEach, describe, expect, it } from 'vitest';
+import { fileURLToPath } from 'node:url';
+import { afterEach, beforeAll, beforeEach, describe, expect, it } from 'vitest';
 import {
   DEFAULT_PROFILE,
   defaultCredentialsPath,
@@ -168,3 +172,62 @@ describe('defaultCredentialsPath', () => {
     expect(defaultCredentialsPath().endsWith('/.testsprite/credentials')).toBe(true);
   });
 });
+
+const projectRoot = fileURLToPath(new URL('../..', import.meta.url));
+const credentialsWriteChild = join(projectRoot, 'test/helpers/credentials-write-child.mjs');
+
+function waitForChild(child: ChildProcess): Promise<void> {
+  return new Promise((resolve, reject) => {
+    let stderr = '';
+    child.stderr?.on('data', chunk => {
+      stderr += String(chunk);
+    });
+    child.on('error', reject);
+    child.on('close', code => {
+      if (code === 0) resolve();
+      else reject(new Error(`child exited with code ${code}: ${stderr}`));
+    });
+  });
+}
+
+function spawnCredentialsWriter(profile: string, apiKey: string, path: string): ChildProcess {
+  return spawn(process.execPath, [credentialsWriteChild], {
+    env: {
+      ...process.env,
+      CRED_PROFILE: profile,
+      CRED_PATH: path,
+      CRED_API_KEY: apiKey,
+    },
+    stdio: ['ignore', 'ignore', 'pipe'],
+  });
+}
+
+describe('credentials write lock', () => {
+  beforeAll(() => {
+    execSync('npm run build', { cwd: projectRoot, stdio: 'pipe' });
+  });
+
+  it('serializes cross-process writes so concurrent profile updates are not lost', async () => {
+    const children = [
+      spawnCredentialsWriter('dev', 'sk-dev', credentialsPath),
+      spawnCredentialsWriter('staging', 'sk-staging', credentialsPath),
+    ];
+    await Promise.all(children.map(waitForChild));
+
+    const file = readCredentialsFile({ path: credentialsPath });
+    expect(file.dev).toEqual({ apiKey: 'sk-dev' });
+    expect(file.staging).toEqual({ apiKey: 'sk-staging' });
+  });
+
+  it('removes the lock file after writeProfile completes', () => {
+    writeProfile('default', { apiKey: 'sk-lock-cleanup' }, { path: credentialsPath });
+    expect(existsSync(`${credentialsPath}.lock`)).toBe(false);
+  });
+
+  it('removes the lock file after deleteProfile completes', () => {
+    writeProfile('default', { apiKey: 'sk-d' }, { path: credentialsPath });
+    writeProfile('dev', { apiKey: 'sk-dev' }, { path: credentialsPath });
+    expect(deleteProfile('dev', { path: credentialsPath })).toBe(true);
+    expect(existsSync(`${credentialsPath}.lock`)).toBe(false);
+  });
+});

src/lib/credentials.ts

@@ -1,10 +1,13 @@
 import {
   chmodSync,
+  closeSync,
   existsSync,
   mkdirSync,
+  openSync,
   readFileSync,
   renameSync,
   statSync,
+  unlinkSync,
   writeFileSync,
 } from 'node:fs';
 import { homedir } from 'node:os';
@@ -109,22 +112,26 @@ export function writeProfile(
   options: CredentialsOptions = {},
 ): void {
   const path = resolvePath(options);
-  const file = readCredentialsFile(options);
-  file[profile] = { ...file[profile], ...entry };
-  writeCredentialsAtomic(path, file);
+  withCredentialsLock(path, () => {
+    const file = readCredentialsFile(options);
+    file[profile] = { ...file[profile], ...entry };
+    writeCredentialsAtomic(path, file);
+  });
 }
 
 export function deleteProfile(profile: string, options: CredentialsOptions = {}): boolean {
   const path = resolvePath(options);
-  const file = readCredentialsFile(options);
-  if (!(profile in file)) return false;
-  delete file[profile];
-  if (Object.keys(file).length === 0) {
-    writeCredentialsAtomic(path, {});
-  } else {
-    writeCredentialsAtomic(path, file);
-  }
-  return true;
+  return withCredentialsLock(path, () => {
+    const file = readCredentialsFile(options);
+    if (!(profile in file)) return false;
+    delete file[profile];
+    if (Object.keys(file).length === 0) {
+      writeCredentialsAtomic(path, {});
+    } else {
+      writeCredentialsAtomic(path, file);
+    }
+    return true;
+  });
 }
 
 export function ensureRestrictiveMode(path: string): void {
@@ -144,3 +151,91 @@ function writeCredentialsAtomic(path: string, file: CredentialsFile): void {
   renameSync(tmp, path);
   ensureRestrictiveMode(path);
 }
+
+/** Max wall-clock wait when another process holds the credentials lock. */
+const CREDENTIALS_LOCK_MAX_WAIT_MS = 10_000;
+/** Back-off between lock attempts. */
+const CREDENTIALS_LOCK_RETRY_MS = 25;
+/** Reclaim a lock file when the holder pid is gone or the file is older than this. */
+const CREDENTIALS_LOCK_STALE_MS = 30_000;
+
+function credentialsLockPath(credentialsPath: string): string {
+  return `${credentialsPath}.lock`;
+}
+
+/**
+ * Serialize read-modify-write on the credentials file across processes.
+ * `writeCredentialsAtomic` only makes the final rename atomic; without this
+ * lock, concurrent `writeProfile` / `deleteProfile` calls can each read the
+ * same snapshot and the last rename wins — silently dropping the other update.
+ */
+function withCredentialsLock<T>(credentialsPath: string, fn: () => T): T {
+  acquireCredentialsLock(credentialsPath);
+  try {
+    return fn();
+  } finally {
+    releaseCredentialsLock(credentialsPath);
+  }
+}
+
+function acquireCredentialsLock(credentialsPath: string): void {
+  const lockPath = credentialsLockPath(credentialsPath);
+  const deadline = Date.now() + CREDENTIALS_LOCK_MAX_WAIT_MS;
+  while (Date.now() < deadline) {
+    try {
+      const fd = openSync(lockPath, 'wx');
+      try {
+        writeFileSync(fd, `${process.pid}\n${Date.now()}\n`, 'utf8');
+      } finally {
+        closeSync(fd);
+      }
+      return;
+    } catch (err) {
+      const code = (err as NodeJS.ErrnoException).code;
+      if (code !== 'EEXIST') throw err;
+      if (isStaleCredentialsLock(lockPath)) {
+        try {
+          unlinkSync(lockPath);
+        } catch {
+          // Another waiter may have claimed or released the lock.
+        }
+        continue;
+      }
+      syncSleep(CREDENTIALS_LOCK_RETRY_MS);
+    }
+  }
+  throw new Error(`Timed out acquiring credentials lock: ${lockPath}`);
+}
+
+function releaseCredentialsLock(credentialsPath: string): void {
+  try {
+    unlinkSync(credentialsLockPath(credentialsPath));
+  } catch {
+    // Lock already released or never acquired — teardown must not mask errors.
+  }
+}
+
+function isStaleCredentialsLock(lockPath: string): boolean {
+  try {
+    const stat = statSync(lockPath);
+    if (Date.now() - stat.mtimeMs > CREDENTIALS_LOCK_STALE_MS) return true;
+    const firstLine = readFileSync(lockPath, 'utf8').split('\n')[0] ?? '';
+    const pid = Number.parseInt(firstLine, 10);
+    if (!Number.isFinite(pid) || pid <= 0) return true;
+    try {
+      process.kill(pid, 0);
+      return false;
+    } catch {
+      return true;
+    }
+  } catch {
+    return false;
+  }
+}
+
+function syncSleep(ms: number): void {
+  const until = Date.now() + ms;
+  while (Date.now() < until) {
+    // Busy-wait: credentials I/O is sync-only; sub-ms precision is unnecessary.
+  }
+}

test/helpers/credentials-write-child.mjs

@@ -0,0 +1,12 @@
+import { writeProfile } from '../../dist/lib/credentials.js';
+
+const profile = process.env.CRED_PROFILE;
+const credentialsPath = process.env.CRED_PATH;
+const apiKey = process.env.CRED_API_KEY;
+
+if (!profile || !credentialsPath || !apiKey) {
+  console.error('CRED_PROFILE, CRED_PATH, and CRED_API_KEY are required');
+  process.exit(1);
+}
+
+writeProfile(profile, { apiKey }, { path: credentialsPath });