fix(electron): harden simple store writes (#7297)

* fix: harden electron simple store writes

* fix(electron): harden simple store writes

---------

Co-authored-by: Aamer Akhter <aamer_akhter@users.noreply.bitbucket.org>

electron/simple-store.test.cjs

@@ -0,0 +1,128 @@
+const test = require('node:test');
+const assert = require('node:assert/strict');
+const os = require('node:os');
+const path = require('node:path');
+const { promises: fs } = require('node:fs');
+const fsModule = require('node:fs');
+const Module = require('node:module');
+
+require('ts-node/register/transpile-only');
+
+const originalModuleLoad = Module._load;
+const originalWriteFile = fsModule.promises.writeFile;
+
+let userDataDir;
+let logCalls = [];
+let errorCalls = [];
+
+const simpleStoreModulePath = path.resolve(__dirname, 'simple-store.ts');
+const getStorePath = () => path.join(userDataDir, 'simpleSettings');
+
+const installMocks = () => {
+  Module._load = function patchedLoad(request, parent, isMain) {
+    if (request === 'electron') {
+      return {
+        app: {
+          getPath: (key) => {
+            assert.equal(key, 'userData');
+            return userDataDir;
+          },
+        },
+      };
+    }
+    if (request === 'electron-log/main') {
+      return {
+        log: (...args) => logCalls.push(args),
+        error: (...args) => errorCalls.push(args),
+      };
+    }
+    return originalModuleLoad.call(this, request, parent, isMain);
+  };
+};
+
+const resetModule = () => {
+  delete require.cache[simpleStoreModulePath];
+};
+
+const loadSimpleStoreModule = () => {
+  resetModule();
+  return require(simpleStoreModulePath);
+};
+
+test.beforeEach(async () => {
+  userDataDir = await fs.mkdtemp(path.join(os.tmpdir(), 'sp-simple-store-'));
+  logCalls = [];
+  errorCalls = [];
+  fsModule.promises.writeFile = originalWriteFile;
+  installMocks();
+});
+
+test.afterEach(async () => {
+  Module._load = originalModuleLoad;
+  fsModule.promises.writeFile = originalWriteFile;
+  resetModule();
+  await fs.rm(userDataDir, { recursive: true, force: true });
+});
+
+test('loadSimpleStoreAll quarantines corrupt JSON files', async () => {
+  const storePath = getStorePath();
+  await fs.writeFile(storePath, '{"broken"', 'utf8');
+
+  const { loadSimpleStoreAll } = loadSimpleStoreModule();
+  const data = await loadSimpleStoreAll();
+
+  assert.deepEqual(data, {});
+
+  const dirEntries = await fs.readdir(userDataDir);
+  const quarantined = dirEntries.find((entry) => entry.startsWith('simpleSettings.corrupt-'));
+  assert.ok(quarantined, 'expected corrupt file to be quarantined');
+  await assert.rejects(fs.access(storePath));
+  assert.ok(
+    errorCalls.some(([message]) =>
+      String(message).includes('Failed to parse simple store JSON'),
+    ),
+  );
+});
+
+test('saveSimpleStore replaces legacy directories with a file', async () => {
+  const storePath = getStorePath();
+  await fs.mkdir(storePath, { recursive: true });
+
+  const { saveSimpleStore, loadSimpleStoreAll } = loadSimpleStoreModule();
+  await saveSimpleStore('main', { theme: 'dark' });
+
+  const stat = await fs.stat(storePath);
+  assert.equal(stat.isFile(), true);
+  assert.deepEqual(await loadSimpleStoreAll(), { main: { theme: 'dark' } });
+});
+
+test('saveSimpleStore serializes concurrent writes so keys are merged', async () => {
+  const { saveSimpleStore, loadSimpleStoreAll } = loadSimpleStoreModule();
+
+  let unblockFirstWrite;
+  const firstWriteBlocked = new Promise((resolve) => {
+    unblockFirstWrite = resolve;
+  });
+  let firstWriteSeen = false;
+
+  fsModule.promises.writeFile = async (...args) => {
+    const [target] = args;
+    if (!firstWriteSeen && String(target).includes('.tmp')) {
+      firstWriteSeen = true;
+      await firstWriteBlocked;
+    }
+    return originalWriteFile.apply(fsModule.promises, args);
+  };
+
+  const firstSave = saveSimpleStore('alpha', { ok: true });
+  await new Promise((resolve) => setImmediate(resolve));
+  const secondSave = saveSimpleStore('beta', { ok: true });
+
+  unblockFirstWrite();
+  await Promise.all([firstSave, secondSave]);
+
+  assert.deepEqual(await loadSimpleStoreAll(), {
+    alpha: { ok: true },
+    beta: { ok: true },
+  });
+});

electron/simple-store.ts

@@ -9,21 +9,43 @@ const getDataPath = (): string => path.join(app.getPath('userData'), 'simpleSett
 
 type SimpleStoreData = { [key: string]: unknown };
 
-export const saveSimpleStore = async (dataKey = 'main', data: unknown): Promise<void> => {
-  const prevData = await loadSimpleStoreAll();
+let _saveQueue: Promise<void> = Promise.resolve();
+
+const _getTmpDataPath = (): string => `${getDataPath()}.${process.pid}.${Date.now()}.tmp`;
+
+const _quarantineCorruptStore = async (): Promise<void> => {
   const dataPath = getDataPath();
-  const json = JSON.stringify({ ...prevData, [dataKey]: data });
+  const corruptPath = `${dataPath}.corrupt-${Date.now()}`;
+  try {
+    await fs.rename(dataPath, corruptPath);
+    log(`Quarantined corrupt simpleSettings file to ${corruptPath}`);
+  } catch (renameErr) {
+    error('Failed to quarantine corrupt simple store:', renameErr);
+  }
+};
+
+const _writeStoreAtomically = async (json: string): Promise<void> => {
+  const dataPath = getDataPath();
+  const tmpPath = _getTmpDataPath();
 
   try {
-    await fs.writeFile(dataPath, json, { encoding: 'utf8' });
+    await fs.writeFile(tmpPath, json, { encoding: 'utf8' });
+    await fs.rename(tmpPath, dataPath);
   } catch (e: unknown) {
     const nodeErr = e as NodeJS.ErrnoException;
+    try {
+      await fs.rm(tmpPath, { force: true });
+    } catch {
+      // Best effort cleanup only.
+    }
+
     if (nodeErr.code === 'EISDIR') {
       // In older app versions simpleSettings was stored as a directory.
       // Remove the legacy directory so we can write the file.
       log('simpleSettings is a directory, removing for file-based storage');
       await fs.rm(dataPath, { recursive: true });
-      await fs.writeFile(dataPath, json, { encoding: 'utf8' });
+      await fs.writeFile(tmpPath, json, { encoding: 'utf8' });
+      await fs.rename(tmpPath, dataPath);
     } else {
       error('Failed to save simple store:', e);
       throw e;
@@ -31,6 +53,17 @@ export const saveSimpleStore = async (dataKey = 'main', data: unknown): Promise<
   }
 };
 
+export const saveSimpleStore = async (dataKey = 'main', data: unknown): Promise<void> => {
+  const runSave = async (): Promise<void> => {
+    const prevData = await loadSimpleStoreAll();
+    const json = JSON.stringify({ ...prevData, [dataKey]: data });
+    await _writeStoreAtomically(json);
+  };
+
+  _saveQueue = _saveQueue.then(runSave, runSave);
+  return _saveQueue;
+};
+
 export const loadSimpleStoreAll = async (): Promise<SimpleStoreData> => {
   try {
     const data = await fs.readFile(getDataPath(), { encoding: 'utf8' });
@@ -48,8 +81,12 @@ export const loadSimpleStoreAll = async (): Promise<SimpleStoreData> => {
       } catch (renameErr) {
         error('Failed to rename legacy simpleSettings directory:', renameErr);
       }
-    } else if (nodeErr.code !== 'ENOENT') {
-      // ENOENT is expected on first run — only log unexpected errors
+    } else if (nodeErr.code === 'ENOENT') {
+      // ENOENT is expected on first run.
+    } else if (e instanceof SyntaxError) {
+      error('Failed to parse simple store JSON, quarantining corrupt file:', e);
+      await _quarantineCorruptStore();
+    } else {
       error('Failed to load simple store:', e);
     }
     return {};