fix(storage): write userData JSON files owner-only

writeJson backs secrets.json (provider API keys), sessions.json, settings.json and projects.json. It wrote with no mode, so the files landed at the process umask — 0644 normally, 0664 under the umask 002 several distributions ship — leaving their contents readable to anything that reaches them: another account on a shared machine, a backup or sync tool, an archive unpacked elsewhere. The userData directory is usually restrictive enough to cover that on a single-user desktop, but a stored credential shouldn't depend on its parent directory's mode. The mode has to go on the temp file rather than the destination: writeJson writes to a temp path and renames over the target, so the destination inode is replaced on every write and takes the temp file's mode with it. Chmod'ing the destination instead would leave a window where the contents are readable and be undone by the next write — which also means a user who chmod 600'd the file by hand had it silently reset. Because writeFileSync only applies `mode` when it creates the file, the temp path is removed first rather than chmod'd afterwards; that keeps a stale temp file from a crashed write from carrying its old mode through, without adding a syscall that could fail between the write and the rename and lose the data. Existing files are tightened on first read, once per path per run — a key set once and never changed is only ever read, so fixing this on write alone would never reach the installs that already have the problem. That step is best-effort and logs rather than throwing. No behaviour change on Windows, where the mode has no meaning and the tests are skipped.
voidstackloop · Jul 25, 2026 · Jul 25, 2026 · Jul 25, 2026 · 67c730e6166f68c393c2f809bac2173f31704768
commit 67c730e6166f68c393c2f809bac2173f31704768
diff --git a/app/src/json-store.test.ts b/app/src/json-store.test.ts
@@ -34,6 +34,34 @@ describe("json-store", () => {
         expect(entries).toEqual(["data.json"]);
     });
 
+    // These files hold API keys and conversation history; the default umask
+    // would leave them readable by other accounts on the machine.
+    it.skipIf(process.platform === "win32")("writes owner-only files", () => {
+        writeJson(file, { token: "value" });
+        expect(fs.statSync(file).mode & 0o777).toBe(0o600);
+    });
+
+    it.skipIf(process.platform === "win32")("keeps the file owner-only on rewrite", () => {
+        writeJson(file, { token: "first" });
+        writeJson(file, { token: "second" });
+        expect(fs.statSync(file).mode & 0o777).toBe(0o600);
+    });
+
+    it.skipIf(process.platform === "win32")("stays owner-only when a stale temp file exists", () => {
+        const stale = `${file}.tmp-${process.pid}`;
+        fs.writeFileSync(stale, "leftover", { mode: 0o666 });
+        writeJson(file, { token: "value" });
+        expect(fs.statSync(file).mode & 0o777).toBe(0o600);
+    });
+
+    // A file written by an older build is only ever read if its contents never
+    // change, so tightening on write alone would never reach it.
+    it.skipIf(process.platform === "win32")("tightens an existing world-readable file on read", () => {
+        fs.writeFileSync(file, JSON.stringify({ token: "value" }), { mode: 0o644 });
+        expect(readJson(file, {})).toEqual({ token: "value" });
+        expect(fs.statSync(file).mode & 0o777).toBe(0o600);
+    });
+
     it("backs up and falls back to the default when the file is corrupted", () => {
         fs.writeFileSync(file, "{ not valid json");
         const result = readJson(file, { safe: true });

diff --git a/app/src/json-store.ts b/app/src/json-store.ts
@@ -30,6 +30,8 @@ export function readJson<T>(filePath: string, fallback: T): T {
         return fallback;
     }
 
+    restrictExistingPermissions(filePath);
+
     try {
         return JSON.parse(raw) as T;
     } catch (err) {
@@ -43,9 +45,48 @@ export function readJson<T>(filePath: string, fallback: T): T {
     }
 }
 
+// These files hold provider API keys (secrets.json) and full conversation
+// history. Written with the process umask they land as 0644 — or 0664 under
+// the umask 002 several distributions ship — leaving their contents readable
+// to anything that reaches them: another account on a shared machine, a
+// backup or sync tool, an archive unpacked somewhere else. The userData
+// directory is usually restrictive enough to cover that on a single-user
+// desktop, but a stored credential shouldn't depend on its parent directory's
+// mode.
+const PRIVATE_FILE_MODE = 0o600;
+
+// Files written before this existed keep the mode they were created with, and
+// writeJson alone would never reach them: a key set once and never changed is
+// only ever read. So the mode is also tightened on first read, once per path
+// per run to keep it off the hot path.
+const permissionsChecked = new Set<string>();
+
+function restrictExistingPermissions(filePath: string): void {
+    if (process.platform === "win32" || permissionsChecked.has(filePath)) return;
+    permissionsChecked.add(filePath);
+    try {
+        if ((fs.statSync(filePath).mode & 0o777) !== PRIVATE_FILE_MODE) {
+            fs.chmodSync(filePath, PRIVATE_FILE_MODE);
+        }
+    } catch (err) {
+        // Best effort — unusual ownership or an exotic filesystem must not
+        // stop the app from reading its own data.
+        logger.error(`Failed to restrict permissions on ${filePath}: ${(err as Error).message}`);
+    }
+}
+
 export function writeJson(filePath: string, data: unknown): void {
     fs.mkdirSync(path.dirname(filePath), { recursive: true });
     const tmpPath = `${filePath}.tmp-${process.pid}`;
-    fs.writeFileSync(tmpPath, JSON.stringify(data, null, 2));
+    // The mode goes on the temp file, because the rename below replaces the
+    // destination inode and takes the temp file's mode with it. Chmod'ing the
+    // destination afterwards would leave a window where the contents are
+    // readable, and would be undone by the next write.
+    //
+    // Removed first so writeFileSync always creates the file and so always
+    // applies `mode` — it ignores the option for a path that already exists,
+    // and an interrupted earlier write can leave one behind under this pid.
+    fs.rmSync(tmpPath, { force: true });
+    fs.writeFileSync(tmpPath, JSON.stringify(data, null, 2), { mode: PRIVATE_FILE_MODE });
     fs.renameSync(tmpPath, filePath);
 }