From 880255ad1c82fd0fdafdec4ebc182c930cd03428 Mon Sep 17 00:00:00 2001
From: Katniss <katnisscalls99@gmail.com>
Date: Fri, 5 Jun 2026 22:28:31 -0700
Subject: [PATCH] fix(cli): serialize local-vault writes to prevent TOCTOU
 silent data loss
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

setSecretInLocal() and deleteSecretFromLocal() both do read-modify-write
on secrets.json with no concurrency protection. If two adapter setup()
calls run concurrently (e.g. via Promise.all, or a background heartbeat
racing a foreground write), both snapshot the same vault, both write,
and the last rename wins — silently dropping the other caller's secret.

Two fixes:

1. In-process serialization: add a promise-chain lock (withVaultLock)
   that serializes every read-modify-write. The lock adds no latency for
   the common single-caller case and prevents the race entirely for the
   parallel-setup pattern sh1pt is explicitly designed for.

2. Unique tmp filename: the previous code used a fixed `${file}.tmp`
   path, so two concurrent writeVault() calls in different async tasks
   could write to the same tmp and stomp each other mid-write. The new
   pattern appends pid + random suffix so each writer gets its own tmp
   file and the rename is independent.

The vault lock is purely in-process. For multi-process isolation (e.g.
two concurrent CLI invocations), a proper flock(2) would be needed; that
is left as a follow-up as it requires an additional npm dep or N-API shim.
---
 packages/cli/src/local-vault.ts | 43 ++++++++++++++++++++++++++-------
 1 file changed, 34 insertions(+), 9 deletions(-)

diff --git a/packages/cli/src/local-vault.ts b/packages/cli/src/local-vault.ts
index 5fe928a2..b6b2630a 100644
--- a/packages/cli/src/local-vault.ts
+++ b/packages/cli/src/local-vault.ts
@@ -17,6 +17,22 @@ import { promises as fs, readFileSync, statSync } from 'node:fs';
 import path from 'node:path';
 import { configDir } from '@profullstack/sh1pt-core';
 
+// In-process serialization lock for read-modify-write operations.
+// Prevents concurrent setSecretInLocal / deleteSecretFromLocal calls
+// (e.g. from Promise.all over multiple adapter setups) from silently
+// overwriting each other's writes. Each mutation acquires this before
+// reading, modifies the in-memory snapshot, writes, then releases.
+let _vaultLock: Promise<void> = Promise.resolve();
+function withVaultLock<T>(fn: () => Promise<T>): Promise<T> {
+  const next = _vaultLock.then(fn, fn);
+  // Reset the chain tail so the lock doesn't grow unbounded.
+  _vaultLock = next.then(
+    () => {},
+    () => {},
+  );
+  return next;
+}
+
 const VAULT_VERSION = 1;
 
 interface LocalVault {
@@ -76,7 +92,9 @@ async function readVault(): Promise<LocalVault> {
 async function writeVault(v: LocalVault): Promise<void> {
   const file = localVaultPath();
   await fs.mkdir(path.dirname(file), { recursive: true, mode: 0o700 });
-  const tmp = `${file}.tmp`;
+  // Include pid + a random suffix so concurrent writers never share a tmp
+  // filename and stomp each other's in-progress write.
+  const tmp = `${file}.${process.pid}.${Math.random().toString(36).slice(2)}.tmp`;
   await fs.writeFile(tmp, JSON.stringify(v, null, 2) + '\n', { mode: 0o600 });
   await fs.rename(tmp, file);
   // rename preserves the source mode, but be explicit if the destination
@@ -85,9 +103,14 @@ async function writeVault(v: LocalVault): Promise<void> {
 }
 
 export async function setSecretInLocal(key: string, value: string): Promise<void> {
-  const v = await readVault();
-  v.secrets[key] = value;
-  await writeVault(v);
+  // Serialize through the in-process lock to prevent concurrent
+  // read-modify-write races (e.g. Promise.all over multiple adapter setups)
+  // from silently dropping each other's writes.
+  return withVaultLock(async () => {
+    const v = await readVault();
+    v.secrets[key] = value;
+    await writeVault(v);
+  });
 }
 
 export async function getSecretFromLocal(key: string): Promise<string | undefined> {
@@ -96,11 +119,13 @@ export async function getSecretFromLocal(key: string): Promise<string | undefine
 }
 
 export async function deleteSecretFromLocal(key: string): Promise<boolean> {
-  const v = await readVault();
-  if (!(key in v.secrets)) return false;
-  delete v.secrets[key];
-  await writeVault(v);
-  return true;
+  return withVaultLock(async () => {
+    const v = await readVault();
+    if (!(key in v.secrets)) return false;
+    delete v.secrets[key];
+    await writeVault(v);
+    return true;
+  });
 }
 
 export async function listSecretsLocal(): Promise<Array<{ key: string }>> {