Stop database snapshots losing their permissions (#289)

copyFileBytes replaced fs.copyFileSync because copyFileSync does not merely
copy bytes - it fchmods the destination to match the source, and exFAT has no
permission bits, so the pre-migration snapshot failed with EPERM on a player.

The replacement dropped the chmod entirely, which fixed that and introduced a
worse problem everywhere else: the copy landed at the default 0666 & ~umask.
Measured on ext4, a 0600 database file copied to a 0664 snapshot - the whole
database readable by group and other, on every install, not just on a player.

Removing a permission operation to fix a permission error is not a fix.

The mode is now applied as a separate, failure-tolerant step after the bytes
are written. That is the actual difference from copyFileSync: there the chmod
is inseparable from the copy, so a filesystem without modes fails the whole
operation; here the data is already safe and a refusal simply means there were
never permissions to carry across.

Two tests: the source mode survives the copy (fails on the current main), and
a filesystem that refuses fchmod still gets its bytes.


Claude-Session: https://claude.ai/code/session_014kfhrUPit5MCqxeTQyqr56

Co-authored-by: Dan Walters <dan.walters@bytetinker.net>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

server/lib/fsutil.js

@@ -40,6 +40,27 @@ function copyFileBytes(src, dest) {
       while (written < read) written += fs.writeSync(outFd, buf, written, read - written);
       pos += read;
     }
+    /*
+     * Carry the source's permissions across where the filesystem has any.
+     *
+     * ⚠️ NOT optional, and the reason this function exists does not excuse skipping it. Dropping
+     * the chmod entirely - which is what the first version did - creates the copy at the default
+     * 0666 & ~umask. A database snapshot that was 0600 came out 0664, so the whole database became
+     * group- and world-readable on every install. That is a worse bug than the one this function
+     * was written to fix.
+     *
+     * Doing it as a SEPARATE, failure-tolerant step is the difference from fs.copyFileSync: there
+     * the chmod is inseparable from the copy, so a filesystem that refuses modes - exFAT, which is
+     * what a BrightSign player's storage is - fails the whole operation with EPERM. Here the bytes
+     * are already written and safe; the mode is applied if it can be, and its refusal is not an
+     * error because on such a filesystem there were never permissions to preserve.
+     */
+    try {
+      fs.fchmodSync(outFd, fs.fstatSync(inFd).mode & 0o777);
+    } catch (e) {
+      /* no permission bits on this filesystem - nothing to carry across */
+    }
+
     // The caller is usually taking a backup it is about to rely on, so make sure the bytes are
     // actually on the device before it proceeds to modify the original.
     fs.fsyncSync(outFd);

server/test/fsutil.test.js

@@ -73,3 +73,43 @@ test('a missing source throws rather than leaving an empty destination behind',
   assert.strictEqual(fs.existsSync(dest), false);
   fs.rmSync(dir, { recursive: true, force: true });
 });
+
+test('the copy carries the source permissions across', () => {
+  // ⚠️ REGRESSION GUARD. The first version of copyFileBytes dropped the chmod entirely, because
+  // chmod is exactly what made fs.copyFileSync fail on exFAT. The copy then landed at the default
+  // 0666 & ~umask: a 0600 database snapshot came out 0664, making the whole database group- and
+  // world-readable on every install. Removing a permission check to fix a permission error is not
+  // a fix.
+  const dir = tmp();
+  const src = path.join(dir, 'db.sqlite');
+  const dest = path.join(dir, 'snapshot.db');
+  fs.writeFileSync(src, 'SQLite format 3\0payload');
+  fs.chmodSync(src, 0o600);
+
+  copyFileBytes(src, dest);
+
+  const mode = (p) => fs.statSync(p).mode & 0o777;
+  assert.strictEqual(mode(dest), 0o600,
+    `snapshot should be 0600 like its source, was 0${mode(dest).toString(8)}`);
+  fs.rmSync(dir, { recursive: true, force: true });
+});
+
+test('a filesystem that refuses chmod still gets its bytes', () => {
+  // The exFAT case, which is the whole reason this function exists rather than fs.copyFileSync.
+  // The bytes are written before the mode is attempted, so a refusal must not fail the copy.
+  const dir = tmp();
+  const src = path.join(dir, 'a.bin');
+  const dest = path.join(dir, 'b.bin');
+  const data = Buffer.from('bytes that must survive a chmod refusal');
+  fs.writeFileSync(src, data);
+
+  const realFchmod = fs.fchmodSync;
+  fs.fchmodSync = () => { const e = new Error('EPERM: operation not permitted, fchmod'); e.code = 'EPERM'; throw e; };
+  try {
+    assert.doesNotThrow(() => copyFileBytes(src, dest), 'a refused chmod must not fail the copy');
+    assert.deepStrictEqual(fs.readFileSync(dest), data);
+  } finally {
+    fs.fchmodSync = realFchmod;
+  }
+  fs.rmSync(dir, { recursive: true, force: true });
+});