fix: make credential file writes atomic and fail closed on corruption

[bukinoshita]

Resolves BU-652: Profile config race can silently erase saved auth profiles.

Problem

credentials.json was rewritten in place with no temp file, rename, or locking. Any JSON parse error was silently collapsed to null, causing mutators like storeApiKey() to rebuild an empty credentials object and overwrite the live file — erasing all previously saved profiles. Concurrent CLI mutations could also produce lost-update races.

Changes

Atomic writes (src/lib/write-file-atomic.ts)

• Writes to a PID-scoped .tmp file, fsyncs it, then renames over the original — prevents truncated/partially-written credentials files.

Advisory file locking (src/lib/file-lock.ts)

• withFileLock() wraps read/modify/write cycles with an O_EXCL-based lock file, stale-lock detection (10 s), and configurable retry — prevents concurrent CLI processes from clobbering each other.

Fail-closed on corrupted JSON (src/lib/corrupted-credentials-error.ts)

• readCredentials() now distinguishes "file does not exist" (returns null) from "file exists but contains invalid JSON" (throws CorruptedCredentialsError).
• The error message tells the user to repair or delete the file — the corrupted file is preserved on disk for manual inspection.
• removeApiKey() no longer deletes the file outright on parse failure.

Immutable mutation patterns (src/lib/config.ts)

• All credential mutators (storeApiKey, removeApiKey, setActiveProfile, renameProfile, storeApiKeyAsync) now use spread-based immutable updates instead of in-place mutation.
• All mutators are wrapped in withFileLock().

Logout resilience (src/commands/auth/logout.ts)

• Logout command catches credential-read errors gracefully so it can still proceed to the removal path and report remove_failed on downstream errors.

Testing

• All 745 existing tests pass
• New test cases for:
  • readCredentials returning null for missing/empty files, throwing CorruptedCredentialsError for malformed JSON
  • Corrupted file preserved on disk after error
  • Atomic write producing valid JSON with no leftover .tmp files
  • storeApiKey and removeApiKey throwing instead of silently replacing profiles when file is corrupted

Linear Issue: BU-652

  

---

Summary by cubic

Prevents silent loss of saved auth profiles (BU-652) by making credential writes atomic and locked, and by failing closed on corrupted JSON.

• Bug Fixes
  • Atomic writes: temp file + fsync + rename to avoid partial/truncated credentials.json.
  • Advisory lock around read/modify/write to prevent concurrent races.
  • readCredentials() throws CorruptedCredentialsError on invalid JSON; file is preserved; mutators stop instead of rebuilding empty state.
  • Mutators (storeApiKey, removeApiKey, setActiveProfile, renameProfile, storeApiKeyAsync) use immutable updates under the lock; removeApiKey no longer deletes on parse failure.
  • Logout gracefully handles corrupted credentials and still proceeds to the removal path.

^{Written for commit 219f90b. Summary will update on new commits.}

[bukinoshita]

@cubic-dev-ai can you review?

[cubic-dev-ai]

@cubic-dev-ai can you review?

@bukinoshita I have started the AI code review. It will take a few minutes to complete.

[cubic-dev-ai]

3 issues found across 7 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/lib/file-lock.ts">

<violation number="1" location="src/lib/file-lock.ts:61">
P2: `sleepSync` burns 100% CPU while spinning. Use `Atomics.wait` for a zero-CPU synchronous sleep — it's available in all supported Node versions (≥20).</violation>
</file>

<file name="src/lib/config.ts">

<violation number="1" location="src/lib/config.ts:394">
P1: The auto-migration write inside `resolveApiKeyAsync` still mutates `creds` in-place and calls `writeCredentials` without `withFileLock`, leaving the same lost-update race this PR eliminates everywhere else. Wrap the write in `withFileLock` and use immutable updates to match the other mutators.</violation>
</file>

<file name="src/commands/auth/logout.ts">

<violation number="1" location="src/commands/auth/logout.ts:52">
P2: This change still allows an uncaught credentials-read throw via `resolveProfileName()`, so logout can crash on corrupted credentials instead of reaching the `remove_failed` handling path.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.}

[cubic-dev-ai]

P1: The auto-migration write inside resolveApiKeyAsync still mutates creds in-place and calls writeCredentials without withFileLock, leaving the same lost-update race this PR eliminates everywhere else. Wrap the write in withFileLock and use immutable updates to match the other mutators.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/lib/config.ts, line 394:

<comment>The auto-migration write inside `resolveApiKeyAsync` still mutates `creds` in-place and calls `writeCredentials` without `withFileLock`, leaving the same lost-update race this PR eliminates everywhere else. Wrap the write in `withFileLock` and use immutable updates to match the other mutators.</comment>

<file context>
@@ -357,7 +391,7 @@ export async function resolveApiKeyAsync(
           );
         } catch {
-          // Non-fatal — plaintext key still works
+          /* non-fatal */
         }
       }
</file context>

[cubic-dev-ai]

P2: sleepSync burns 100% CPU while spinning. Use Atomics.wait for a zero-CPU synchronous sleep — it's available in all supported Node versions (≥20).

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/lib/file-lock.ts, line 61:

<comment>`sleepSync` burns 100% CPU while spinning. Use `Atomics.wait` for a zero-CPU synchronous sleep — it's available in all supported Node versions (≥20).</comment>

<file context>
@@ -0,0 +1,66 @@
+  }
+};
+
+const sleepSync = (ms: number): void => {
+  const end = Date.now() + ms;
+  while (Date.now() < end) {
</file context>

[cubic-dev-ai]

P2: This change still allows an uncaught credentials-read throw via resolveProfileName(), so logout can crash on corrupted credentials instead of reaching the remove_failed handling path.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/commands/auth/logout.ts, line 52:

<comment>This change still allows an uncaught credentials-read throw via `resolveProfileName()`, so logout can crash on corrupted credentials instead of reaching the `remove_failed` handling path.</comment>

<file context>
@@ -39,7 +48,8 @@ If no credentials file exists, exits cleanly with no error.`,
     const configPath = getCredentialsPath();
-    const creds = readCredentials();
+
+    const creds = tryReadCredentials();
 
     if (!creds && !existsSync(configPath)) {
</file context>