refactor(persistence): canonical atomic file writer + notifications.json crash-safety (DC-099) [glm-grade=B]

- src/utils/atomic-write.js: single shared tmp+fsync+rename writer
  (exclusive-create 0600, unique tmp names, cleanup-on-failure,
  best-effort parent-dir fsync after rename for swap durability)
- notification-manager: both write paths (load-time canonicalization
  write-back + saveConfig) converted from plain writeFileSync — a
  crash mid-write can no longer truncate notifications.json
- DC-097/098 test seams migrated to the atomic path; new suite pins
  syscall discipline (order, wx flags, tmp naming, error cleanup,
  dir-fsync swallow)
- 121 suites / 2768 tests green
- Judge: GLM-5.3 cold read grade B/ship (deleg_23f7abad); polish items
  folded: dir-fsync added, header copy-count corrected. Remaining:
  migrate invite/user/share-store private _atomicWriteJSON copies as
  they are touched (queued).

URN: pending (recorded post-commit)

dashcaddy-api/__tests__/atomic-write-dc099.test.js

@@ -0,0 +1,198 @@
+/**
+ * DC-099: canonical atomic file writer (src/utils/atomic-write.js).
+ *
+ * The notification config's two write paths (load-time canonicalization
+ * write-back and the UI saveConfig) used plain fs.writeFileSync — a crash or
+ * power loss mid-write could leave a truncated/empty notifications.json. The
+ * same risk exists in every store that grew its own private
+ * _atomicWriteJSON copy (invite-store, user-store, share-store, …).
+ *
+ * These tests pin the shared writer's contract:
+ *   - durability: fsync before rename, exclusive create, 0600 default
+ *   - atomicity: destination only ever replaced via rename
+ *   - failure: destination untouched, temp cleaned up, error propagated
+ *   - JSON helper: single serialization shape (2-space, no trailing newline —
+ *     notification-manager._persistCanonicalForm depends on byte-for-byte
+ *     idempotence)
+ */
+
+'use strict';
+
+const fs = require('fs');
+const os = require('os');
+const path = require('path');
+const { atomicWriteFile, atomicWriteJSON, tmpPathFor } = require('../src/utils/atomic-write');
+
+// Real-FS tests: the actual syscalls, in a private temp dir.
+describe('DC-099 atomic-write (real fs)', () => {
+  let dir;
+
+  beforeEach(() => {
+    dir = fs.mkdtempSync(path.join(os.tmpdir(), 'dc099-atomic-'));
+  });
+
+  afterEach(() => {
+    fs.rmSync(dir, { recursive: true, force: true });
+  });
+
+  test('writes contents and returns the final path', () => {
+    const target = path.join(dir, 'state.json');
+    const ret = atomicWriteFile(target, '{"a":1}');
+    expect(ret).toBe(target);
+    expect(fs.readFileSync(target, 'utf8')).toBe('{"a":1}');
+  });
+
+  test('replaces an existing file completely (no torn writes possible)', () => {
+    const target = path.join(dir, 'state.json');
+    atomicWriteFile(target, 'x'.repeat(1000));
+    atomicWriteFile(target, 'y'.repeat(10));
+    expect(fs.readFileSync(target, 'utf8')).toBe('y'.repeat(10));
+  });
+
+  test('creates the file 0600 by default', () => {
+    const target = path.join(dir, 'secret.json');
+    atomicWriteJSON(target, { ok: true });
+    expect(fs.statSync(target).mode & 0o777).toBe(0o600);
+  });
+
+  test('honors an explicit mode override', () => {
+    const target = path.join(dir, 'public.json');
+    atomicWriteFile(target, '{}', { mode: 0o644 });
+    expect(fs.statSync(target).mode & 0o777).toBe(0o644);
+  });
+
+  test('leaves no temp files behind after success', () => {
+    const target = path.join(dir, 'state.json');
+    atomicWriteFile(target, 'abc');
+    expect(fs.readdirSync(dir).filter((f) => f.includes('.tmp-'))).toEqual([]);
+  });
+
+  test('two rapid writes both land (unique tmp names per write)', () => {
+    const target = path.join(dir, 'state.json');
+    atomicWriteFile(target, 'first');
+    atomicWriteFile(target, 'second');
+    expect(fs.readFileSync(target, 'utf8')).toBe('second');
+  });
+
+  test('atomicWriteJSON serializes 2-space, no trailing newline', () => {
+    const target = path.join(dir, 'conf.json');
+    atomicWriteJSON(target, { a: { b: 1 } });
+    const raw = fs.readFileSync(target, 'utf8');
+    expect(raw).toBe('{\n  "a": {\n    "b": 1\n  }\n}');
+  });
+
+  test('write failure leaves the destination untouched and cleans the temp file', () => {
+    const target = path.join(dir, 'state.json');
+    fs.writeFileSync(target, 'ORIGINAL');
+    const origWrite = fs.writeSync;
+    fs.writeSync = () => {
+      throw Object.assign(new Error('ENOSPC: no space left on device'), { code: 'ENOSPC' });
+    };
+    try {
+      expect(() => atomicWriteFile(target, 'NEW-CONTENT')).toThrow(/ENOSPC/);
+    } finally {
+      fs.writeSync = origWrite;
+    }
+    expect(fs.readFileSync(target, 'utf8')).toBe('ORIGINAL');
+    expect(fs.readdirSync(dir).filter((f) => f.includes('.tmp-'))).toEqual([]);
+  });
+
+  test('tmpPathFor: unique per call, hidden dotfile in the same directory', () => {
+    const a = tmpPathFor('/data/x.json');
+    const b = tmpPathFor('/data/x.json');
+    expect(a).not.toBe(b);
+    expect(path.dirname(a)).toBe('/data');
+    expect(path.basename(a)).toMatch(/^\.x\.json\.tmp-/);
+  });
+});
+
+// Mocked-FS tests: pin the syscall DISCIPLINE itself (order + flags), which
+// the real-fs tests can't observe directly.
+describe('DC-099 atomic-write syscall discipline (mocked fs)', () => {
+  const calls = [];
+
+  beforeEach(() => {
+    calls.length = 0;
+    const rec = (name, impl) =>
+      jest.spyOn(fs, name).mockImplementation((...args) => {
+        calls.push(name);
+        return impl(...args);
+      });
+    rec('openSync', () => 3);
+    rec('writeSync', () => 8);
+    rec('fsyncSync', () => {});
+    rec('closeSync', () => {});
+    rec('renameSync', () => {});
+    rec('unlinkSync', () => {});
+  });
+
+  afterEach(() => {
+    jest.restoreAllMocks();
+  });
+
+  test('order: open → write → fsync → close → rename, then dir fsync (open → fsync → close)', () => {
+    atomicWriteFile('/data/x.json', '{"a":1}');
+    expect(calls).toEqual([
+      'openSync', 'writeSync', 'fsyncSync', 'closeSync', 'renameSync',
+      'openSync', 'fsyncSync', 'closeSync',
+    ]);
+  });
+
+  test('dir fsync opens the PARENT directory (second openSync), not another tmp file', () => {
+    atomicWriteFile('/data/x.json', '{}');
+    const dirOpen = fs.openSync.mock.calls[1];
+    expect(dirOpen[0]).toBe('/data');
+    expect(dirOpen[1]).toBe('r');
+  });
+
+  test('dir fsync failure is swallowed (write still succeeds)', () => {
+    let n = 0;
+    fs.fsyncSync.mockImplementation(() => {
+      n += 1;
+      if (n === 2) throw new Error('EINVAL: invalid argument'); // 2nd fsync = dir
+    });
+    expect(() => atomicWriteFile('/data/x.json', '{}')).not.toThrow();
+    expect(fs.renameSync).toHaveBeenCalled();
+  });
+
+  test('open uses exclusive-create with the 0600 default on the tmp path', () => {
+    atomicWriteFile('/data/x.json', '{}');
+    const [tmpPath, flags, modeArg] = fs.openSync.mock.calls[0];
+    expect(tmpPath).toMatch(/^\/data\/\.x\.json\.tmp-/);
+    expect(flags).toBe('wx');
+    expect(modeArg).toBe(0o600);
+  });
+
+  test('write passes the payload with utf8 encoding', () => {
+    atomicWriteFile('/data/x.json', '{"a":1}');
+    expect(fs.writeSync.mock.calls[0]).toEqual([3, '{"a":1}', null, 'utf8']);
+  });
+
+  test('rename swaps a same-dir temp onto the target', () => {
+    atomicWriteFile('/data/x.json', '{}');
+    const [tmp, dest] = fs.renameSync.mock.calls[0];
+    expect(tmp).toMatch(/\/data\/\.x\.json\.tmp-/);
+    expect(dest).toBe('/data/x.json');
+  });
+
+  test('rename failure unlinks the temp and propagates the error', () => {
+    fs.renameSync.mockImplementation(() => {
+      calls.push('renameSync');
+      throw new Error('EXDEV: cross-device link not permitted');
+    });
+    expect(() => atomicWriteFile('/data/x.json', '{}')).toThrow(/EXDEV/);
+    expect(calls).toEqual([
+      'openSync', 'writeSync', 'fsyncSync', 'closeSync', 'renameSync', 'unlinkSync',
+    ]);
+  });
+
+  test('open failure propagates without write/rename (nothing was created)', () => {
+    fs.openSync.mockImplementation(() => {
+      calls.push('openSync');
+      throw new Error('EACCES: permission denied');
+    });
+    expect(() => atomicWriteFile('/data/x.json', '{}')).toThrow(/EACCES/);
+    // best-effort unlink of the never-created temp, then stop
+    expect(calls).toEqual(['openSync', 'unlinkSync']);
+  });
+});

dashcaddy-api/__tests__/notification-config-writeback-dc097.test.js

@@ -12,6 +12,13 @@ jest.mock('fs', () => ({
   readFileSync: jest.fn().mockReturnValue('{}'),
   writeFileSync: jest.fn(),
   mkdirSync: jest.fn(),
+  // DC-099 atomic write path (open tmp → write → fsync → close → rename).
+  openSync: jest.fn().mockReturnValue(3),
+  writeSync: jest.fn(),
+  fsyncSync: jest.fn(),
+  closeSync: jest.fn(),
+  renameSync: jest.fn(),
+  unlinkSync: jest.fn(),
 }));
 
 jest.mock('nodemailer', () => ({
@@ -86,10 +93,10 @@ describe('DC-097 notification config canonicalization write-back', () => {
     expect(nm.config.events['container-down']).toBe(false);
     expect(nm.config.events['deploy-success']).toBe(false);
 
-    // On-disk write-back: exactly one write of the full canonical config.
-    expect(fs.writeFileSync).toHaveBeenCalledTimes(1);
-    const [pathArg, contentsArg] = fs.writeFileSync.mock.calls[0];
-    expect(pathArg).toBe(NOTIF_FILE);
+    // On-disk write-back: exactly one atomic write (DC-099: write tmp → fsync → rename).
+    expect(fs.renameSync).toHaveBeenCalledTimes(1);
+    expect(fs.renameSync.mock.calls[0][1]).toBe(NOTIF_FILE);
+    const contentsArg = fs.writeSync.mock.calls[0][1];
     const written = JSON.parse(contentsArg);
     expect(written.providers.email.username).toBe('legacy-user');
     expect(written.providers.email.password).toBe('legacy-pass');
@@ -108,14 +115,15 @@ describe('DC-097 notification config canonicalization write-back', () => {
       events: { containerDown: true },
     });
     const first = loadWithFile(legacy, log);
-    expect(fs.writeFileSync).toHaveBeenCalledTimes(1);
-    const canonicalContents = fs.writeFileSync.mock.calls[0][1];
+    expect(fs.renameSync).toHaveBeenCalledTimes(1);
+    const canonicalContents = fs.writeSync.mock.calls[0][1];
     first.stopHealthDaemon && first.stopHealthDaemon();
-    fs.writeFileSync.mockClear();
+    fs.renameSync.mockClear();
+    fs.writeSync.mockClear();
 
     // Second load against the canonical bytes: no write.
     const second = loadWithFile(canonicalContents, log);
-    expect(fs.writeFileSync).not.toHaveBeenCalled();
+    expect(fs.renameSync).not.toHaveBeenCalled();
     expect(second.config.providers.email.username).toBe('u');
     second.stopHealthDaemon && second.stopHealthDaemon();
   });
@@ -125,11 +133,12 @@ describe('DC-097 notification config canonicalization write-back', () => {
     // Build it by round-tripping: write-back from a minimal legacy file
     // produces the canonical full shape; feed those exact bytes back.
     const nm = loadWithFile(ser({ enabled: true }), log); // 1 write (defaults fill-in)
-    const canonicalContents = fs.writeFileSync.mock.calls[0][1];
+    const canonicalContents = fs.writeSync.mock.calls[0][1];
     nm.stopHealthDaemon && nm.stopHealthDaemon();
-    fs.writeFileSync.mockClear();
+    fs.renameSync.mockClear();
+    fs.writeSync.mockClear();
     const again = loadWithFile(canonicalContents, log);
-    expect(fs.writeFileSync).not.toHaveBeenCalled();
+    expect(fs.renameSync).not.toHaveBeenCalled();
     again.stopHealthDaemon && again.stopHealthDaemon();
   });
 
@@ -138,7 +147,7 @@ describe('DC-097 notification config canonicalization write-back', () => {
       providers: { email: { user: 'u2', pass: 'p2' } },
       events: { workflowDone: true },
     });
-    fs.writeFileSync.mockImplementation(() => { throw new Error('EACCES: permission denied'); });
+    fs.openSync.mockImplementation(() => { throw new Error('EACCES: permission denied'); });
     let nm;
     expect(() => { nm = loadWithFile(legacy, log); }).not.toThrow();
     expect(nm.config.providers.email.username).toBe('u2');
@@ -153,7 +162,7 @@ describe('DC-097 notification config canonicalization write-back', () => {
     fs.existsSync.mockReturnValue(false);
     const nm = new NotificationManager(makeCtx(log));
     expect(fs.readFileSync).not.toHaveBeenCalled();
-    expect(fs.writeFileSync).not.toHaveBeenCalled();
+    expect(fs.renameSync).not.toHaveBeenCalled();
     nm.stopHealthDaemon && nm.stopHealthDaemon();
   });
 });

dashcaddy-api/__tests__/notification-manager.test.js

@@ -10,6 +10,13 @@ jest.mock('fs', () => ({
   readFileSync: jest.fn().mockReturnValue('{}'),
   writeFileSync: jest.fn(),
   mkdirSync: jest.fn(),
+  // DC-099 atomic write path (open tmp → write → fsync → close → rename).
+  openSync: jest.fn().mockReturnValue(3),
+  writeSync: jest.fn(),
+  fsyncSync: jest.fn(),
+  closeSync: jest.fn(),
+  renameSync: jest.fn(),
+  unlinkSync: jest.fn(),
 }));
 
 jest.mock('nodemailer', () => ({
@@ -63,10 +70,12 @@ describe('NotificationManager', () => {
     fs.existsSync.mockReturnValue(false);
     await nm.saveConfig();
     expect(fs.mkdirSync).toHaveBeenCalled();
-    expect(fs.writeFileSync).toHaveBeenCalled();
-    const callArgs = fs.writeFileSync.mock.calls[0];
-    expect(callArgs[0]).toBe(NOTIF_FILE);
-    expect(callArgs[1]).toContain('enabled');
+    // DC-099: atomic write path — payload lands via writeSync, then tmp is renamed onto the target.
+    expect(fs.writeSync).toHaveBeenCalled();
+    expect(fs.renameSync).toHaveBeenCalled();
+    const writeArgs = fs.writeSync.mock.calls[0];
+    expect(fs.renameSync.mock.calls[0][1]).toBe(NOTIF_FILE);
+    expect(writeArgs[1]).toContain('enabled');
   });
 
   test('loadConfig merges file content with defaults', () => {

dashcaddy-api/src/managers/notification-manager.js

@@ -6,6 +6,7 @@ const EventEmitter = require('events');
 const fs = require('fs');
 const path = require('path');
 const nodemailer = require('nodemailer');
+const { atomicWriteJSON } = require('../utils/atomic-write');
 
 // Canonical event names are kebab-case ('container-down'). Emitters and the
 // settings UI historically send camelCase ('containerDown', 'deploymentSuccess')
@@ -125,7 +126,9 @@ class NotificationManager extends EventEmitter {
     try {
       const canonical = JSON.stringify(this.config, null, 2);
       if (canonical !== rawFileContents) {
-        fs.writeFileSync(this.NOTIFICATIONS_FILE, canonical);
+        // DC-099: tmp+fsync+rename — a crash mid-write can no longer leave a
+        // truncated/empty notifications.json (plain writeFileSync could).
+        atomicWriteJSON(this.NOTIFICATIONS_FILE, this.config);
         this.log.info?.('notification', 'Notification config canonicalized on disk (legacy keys normalized)', {});
       }
     } catch (writeError) {
@@ -198,7 +201,9 @@ class NotificationManager extends EventEmitter {
       if (!fs.existsSync(dir)) {
         fs.mkdirSync(dir, { recursive: true });
       }
-      fs.writeFileSync(this.NOTIFICATIONS_FILE, JSON.stringify(this.config, null, 2));
+      // DC-099: atomic tmp+fsync+rename — the UI save path gets the same
+      // crash-safety as the load-path write-back (no torn notifications.json).
+      atomicWriteJSON(this.NOTIFICATIONS_FILE, this.config);
       return true;
     } catch (error) {
       this.log.error('notification', error, null, { note: 'Failed to save config' });

dashcaddy-api/src/utils/atomic-write.js

@@ -0,0 +1,101 @@
+/**
+ * Canonical atomic file writer (DC-099).
+ *
+ * Write discipline: same-directory temp file → write → fsync → close → rename.
+ * rename() is atomic on POSIX, so a reader (or a crash) can only ever see the
+ * complete old file or the complete new file — never a truncated mix. fsync
+ * before rename pins the bytes so a post-rename power loss doesn't leave an
+ * empty/short file behind (the failure mode plain writeFileSync has).
+ *
+ * This is the ONE shared implementation. It replaces the three private
+ * `_atomicWriteJSON` copies (invite-store, user-store, share-store) as they
+ * are touched, and mirrors the DC-098 redact tool's write path. Do not add a
+ * fourth copy — require this module instead.
+ */
+
+'use strict';
+
+const fs = require('fs');
+const path = require('path');
+
+// Monotonic counter guarantees unique tmp names even for back-to-back writes
+// of the same target within one process tick.
+let writeCounter = 0;
+
+function tmpPathFor(filePath) {
+  writeCounter += 1;
+  const base = path.basename(filePath);
+  const dir = path.dirname(filePath);
+  return path.join(dir, `.${base}.tmp-${process.pid}-${Date.now().toString(36)}-${writeCounter}`);
+}
+
+/**
+ * Best-effort durability for the rename itself: fsync the parent directory so
+ * the swap survives a post-rename power loss. POSIX guarantees the rename is
+ * atomic *visibly*, but without a dir fsync a crash can leave the old entry —
+ * a stale-but-complete file, never a torn one, so failure here is not fatal.
+ */
+function fsyncDir(dirPath) {
+  let dfd = null;
+  try {
+    dfd = fs.openSync(dirPath, 'r');
+    fs.fsyncSync(dfd);
+  } catch (_) {
+    // Some platforms/filesystems reject fsync on directory fds; the payload
+    // is already durable via the file-level fsync above.
+  } finally {
+    if (dfd !== null) {
+      try { fs.closeSync(dfd); } catch (_) { /* fd already closed */ }
+    }
+  }
+}
+
+/**
+ * Atomically replace `filePath` with `contents`.
+ *
+ * @param {string} filePath - destination (parent dir must exist)
+ * @param {string} contents - full file contents
+ * @param {object} [opts]
+ * @param {number} [opts.mode=0o600] - mode for a newly created file
+ * @returns {string} the final path (filePath)
+ * @throws whatever fs throws (ENOSPC, EACCES, …); on failure the destination
+ *   is untouched and the temp file is removed best-effort.
+ */
+function atomicWriteFile(filePath, contents, opts = {}) {
+  const mode = typeof opts.mode === 'number' ? opts.mode : 0o600;
+  const tmp = tmpPathFor(filePath);
+  let fd = null;
+  try {
+    // 'wx' — fail loudly if the tmp name somehow exists rather than clobber.
+    fd = fs.openSync(tmp, 'wx', mode);
+    fs.writeSync(fd, contents, null, 'utf8');
+    fs.fsyncSync(fd);
+    fs.closeSync(fd);
+    fd = null;
+    fs.renameSync(tmp, filePath);
+    fsyncDir(path.dirname(filePath));
+    return filePath;
+  } catch (err) {
+    if (fd !== null) {
+      try { fs.closeSync(fd); } catch (_) { /* fd already closed or broken */ }
+    }
+    try { fs.unlinkSync(tmp); } catch (_) { /* nothing to clean up */ }
+    throw err;
+  }
+}
+
+/**
+ * Atomically write `data` as JSON. Single serializer for the whole codebase:
+ * 2-space indent, no trailing newline (matches the notification config's
+ * byte-for-byte idempotence check in _persistCanonicalForm).
+ *
+ * @param {string} filePath
+ * @param {*} data - JSON.stringify-able value
+ * @param {object} [opts] - passed through to atomicWriteFile
+ * @returns {string} the final path (filePath)
+ */
+function atomicWriteJSON(filePath, data, opts = {}) {
+  return atomicWriteFile(filePath, JSON.stringify(data, null, 2), opts);
+}
+
+module.exports = { atomicWriteFile, atomicWriteJSON, tmpPathFor };