bug(storage): settings.json, atomic replace has no durability fence and a corrupted file is unrecoverable; the atomic-write pattern drifts across stores

[chinawch007]

What happened

Two coupled defects in packages/storage/src/settings-store.ts, one on the write path and one on the read path:

1. write() (settings-store.ts:209-214) performs temp + rename with no durability fence at all.

private async write(settings: AppSettings): Promise<void> {
  await mkdir(dirname(this.settingsPath), { recursive: true });
  const tempPath = `${this.settingsPath}.${process.pid}.${Date.now()}.tmp`;
  await writeFile(tempPath, JSON.stringify(settings, null, 2) + '\n', 'utf8');
  await rename(tempPath, this.settingsPath);
}

There is no handle.sync() on the temp file and no directory sync after the rename, so nothing orders the rename behind the file's data blocks. On a power loss or hard crash inside the write window, POSIX does not guarantee the data reached disk even though the rename did (the classic rename-without-fsync hazard — ext4's delayed-allocation zero-length files being the well-known instance), leaving settings.json present but zero-length or truncated. Additionally, the temp name is predictable (pid + Date.now()), the file is created without wx/O_EXCL, and a failed rename leaves the temp file behind.

2. readOrCreate() (settings-store.ts:92-102) only treats ENOENT as recoverable.

try {
  const text = await readFile(this.settingsPath, 'utf8');
  return normalizeSettings(JSON.parse(text));
} catch (error) {
  if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error;
  /* first run: write defaults */
}

A zero-length or truncated file makes JSON.parse throw a SyntaxError, which has no .code, so it falls into the rethrow branch: no fallback to defaults and no recovery guidance. The error propagates out of get(), and the desktop main call sites I checked invoke it without a catch (e.g. apps/desktop/src/main/runtime-host-settings-ipc-main.ts:146 and :179, apps/desktop/src/main/client-settings-ipc-main.ts:41-42), so settings loading fails outright.

Net effect: the write path can produce a corrupt file, and the read path treats corrupt as fatal. A missing settings.json gracefully gets defaults; a corrupted one bricks settings load until the user figures out they must delete <workspaceRoot>/settings.json by hand. mcp-config-store.ts:181-190 has the same ENOENT-only readOrCreate, so its read path shares the recovery gap.

How to reproduce

Real-world trigger (intermittent, low probability, deterministic consequence): change any setting, then lose power / hard reset inside the write window.

Deterministic simulation of the post-crash state:

1. Locate the workspace settings file <workspaceRoot>/settings.json (constructed at settings-store.ts:80)
2. Truncate it — either empty it completely or keep the first ~20 bytes so the JSON is cut mid-token
3. Start the desktop app (or call settingsStore.get() directly)

Expected: settings load falls back to defaults, the same grace already extended to a missing file. Actual: a SyntaxError propagates out of get() and settings loading fails with no recovery path.

Environment

• Maka version or commit: 8c491e6 (main)
• OS and version: macOS 14 (the code paths are cross-platform; the missing-fsync hazard is POSIX-relevant)
• Surface: Desktop / @maka/storage from source
• Node.js version, if running from source: 24.x

Logs, screenshots, or additional context

The deeper issue is divergence: the same "atomic JSON file replace" pattern is implemented three times inside one package, at three different strictness levels — and the strictest one is the package's own documented standard.

hardening	settings-store write() (:209)	mcp-config-store write() (:197)	credential-store writeSecretFileAtomic() (:261)
unpredictable temp name	✗ pid+Date.now()	✓ randomUUID()	✓ randomUUID()
exclusive create (wx / O_EXCL)	✗	✓	✓
0600 temp + chmod	✗	✓	✓
fsync temp before rename	✗	✗	✓ handle.sync()
fsync parent dir after rename	✗	✗	✓ syncDirectory()
temp cleanup on failure	✗	✓ (swallows the error)	✓ (rethrows)

credential-store's JSDoc states the standard explicitly:

Owner-only atomic write for a credentials file: a 0700 dir, an exclusive 0600 temp ('wx'/O_EXCL so we never follow a pre-planted symlink at a predictable path), a durability fence before and after the atomic rename, and temp cleanup on failure.

settings-store meets none of these properties; mcp-config-store meets all but the durability fence. Meanwhile the fence primitives already exist and are exported — stable-storage.ts:90-133 provides syncFile / syncDirectory / syncDirectoryChain, already used by marker-file, memory-bundle-io and the session-bundle paths — and credential-store.ts:281 even carries a private duplicate of syncDirectory.

Suggested direction (happy to take this once triaged):

1. Extract one shared atomic-write helper implementing the credential-store standard (randomUUID temp, wx, 0600, fsync file → rename → fsync dir, cleanup that rethrows) and point settings-store and mcp-config-store at it — removing the drift rather than patching each site.
2. Harden both readOrCreate() implementations to distinguish "missing" from "corrupt": on corrupt, move the file aside (e.g. settings.json.corrupt-<timestamp>), rewrite defaults, and surface a warning — or at minimum throw a typed error naming the file to remove, instead of letting a bare SyntaxError propagate through startup.

[chinawch007]

take