fix: establish durable keyring publication boundary

Fsync the keyring directory after both complete temporary files are written and synced, before replacing the primary document. Pre-publication directory failures now preserve the prior primary state and remain rollback-safe. Keep the durable recovery temporary file available when later directory or backup publication work fails, and verify that it restores the committed keyring. Rank equal-time recovery candidates deterministically with recovery temps before primary temps and backups, and consider equal-time commit temps alongside a valid primary.
2026-08-22 15:36:11 +02:00
parent 8041aeead9
commit 82578b5f6e
2 changed files with 181 additions and 9 deletions
@@ -96,6 +96,13 @@ function createOnlineBackupKeyring({
      && /^\d+\.[0-9a-f-]+\.(?:primary|recovery)\.tmp$/u.test(value.slice(temporaryPrefix.length));
  }

+  function recoveryCandidatePriority(candidatePath) {
+    const name = path.basename(candidatePath);
+    if (name.endsWith('.recovery.tmp')) return 0;
+    if (name.endsWith('.primary.tmp')) return 1;
+    return 2;
+  }
+
  function encryptionError(error) {
    return issueError(error?.code === 'SECRET_STORE_UNAVAILABLE' ? KEYRING_ERROR_CODES.unavailable : KEYRING_ERROR_CODES.encrypt);
  }
@@ -141,12 +148,20 @@ function createOnlineBackupKeyring({
    for (const candidatePath of candidates) {
      try {
        const stats = await fsImpl.stat(candidatePath);
-        ranked.push({ candidatePath, modified: stats.mtimeMs });
+        ranked.push({
+          candidatePath,
+          modified: stats.mtimeMs,
+          priority: recoveryCandidatePriority(candidatePath)
+        });
      } catch (error) {
-        if (error?.code !== 'ENOENT') ranked.push({ candidatePath, modified: 0 });
+        if (error?.code !== 'ENOENT') ranked.push({ candidatePath, modified: 0, priority: recoveryCandidatePriority(candidatePath) });
      }
    }
-    ranked.sort((left, right) => right.modified - left.modified);
+    ranked.sort((left, right) =>
+      right.modified - left.modified
+      || left.priority - right.priority
+      || left.candidatePath.localeCompare(right.candidatePath)
+    );
    return ranked.map(candidate => candidate.candidatePath);
  }

@@ -229,7 +244,7 @@ function createOnlineBackupKeyring({
          try {
            candidateModified = (await fsImpl.stat(candidatePath)).mtimeMs;
          } catch {}
-          if (candidateModified <= primaryModified) continue;
+          if (candidateModified < primaryModified) continue;
          const candidate = await readCandidate(candidatePath);
          if (candidate.status !== 'valid') continue;
          const state = inspectSource({ ...candidate, recovered: true });
@@ -353,12 +368,11 @@ function createOnlineBackupKeyring({
    const primaryTemporaryPath = temporaryPath('primary');
    const recoveryTemporaryPath = temporaryPath('recovery');
    await fsImpl.mkdir(directory, { recursive: true });
-    let primaryPublished = false;
    try {
      await writeAndSync(primaryTemporaryPath, contents);
      await writeAndSync(recoveryTemporaryPath, contents);
+      await syncDirectory();
      await fsImpl.rename(primaryTemporaryPath, filePath);
-      primaryPublished = true;
    } catch (error) {
      await removeFile(primaryTemporaryPath);
      await removeFile(recoveryTemporaryPath);
@@ -370,9 +384,7 @@ function createOnlineBackupKeyring({
      await fsImpl.rename(recoveryTemporaryPath, backupPath);
      recoveryPath = null;
      await syncDirectory();
-    } catch {
-      if (!primaryPublished) throw issueError(KEYRING_ERROR_CODES.structure);
-    }
+    } catch {}
    try {
      await cleanupTemporaryFiles(recoveryPath);
    } catch {}
@@ -318,6 +318,107 @@ describe('encrypted online backup keyring', () => {
    assert.equal(fs.existsSync(filePath), true);
    assert.equal(fs.existsSync(backupPath), true);
    assert.ok(events.filter(event => event === `sync:${path.basename(path.dirname(filePath))}`).length >= 2);
+    assert.ok(
+      events.indexOf(`sync:${path.basename(path.dirname(filePath))}`)
+      < events.indexOf(renames.find(rename => rename.endsWith(`>${path.basename(filePath)}`)))
+    );
+  });
+
+  it('leaves the primary unchanged when the pre-publication directory sync fails', async () => {
+    const first = fixture();
+    const firstKey = validKey();
+    await first.keyring.commit(first.keyring.prepare(firstKey, timestamp));
+    const original = fs.readFileSync(first.filePath, 'utf8');
+    let directorySyncs = 0;
+    const fsImpl = {
+      ...fs.promises,
+      open: async (target, flags, mode) => {
+        const handle = await fs.promises.open(target, flags, mode);
+        if (path.resolve(String(target)) !== path.resolve(first.directory)) return handle;
+        return {
+          sync: async () => {
+            directorySyncs++;
+            if (directorySyncs === 1) throw Object.assign(new Error('directory sync failed'), { code: 'EIO' });
+            return handle.sync();
+          },
+          close: () => handle.close()
+        };
+      }
+    };
+    const { createOnlineBackupKeyring } = require('../lib/online-backup-keyring');
+    const keyring = createOnlineBackupKeyring({
+      filePath: first.filePath,
+      encryptField: encrypt,
+      decryptField: decrypt,
+      isEncrypted: isCanonicalEnvelope,
+      fsImpl
+    });
+
+    await assert.rejects(
+      keyring.commit(keyring.prepare(validKey(), '2026-08-22T11:00:00.000Z')),
+      /directory sync failed/u
+    );
+    assert.equal(fs.readFileSync(first.filePath, 'utf8'), original);
+    assert.equal(fs.readdirSync(first.directory).some(name => name.endsWith('.tmp')), false);
+    assert.equal(await first.keyring.getKey(parseOnlineBackupKey(firstKey).id), firstKey);
+  });
+
+  it('keeps a durable recovery temp after a post-publication sync failure', async () => {
+    const first = fixture();
+    const firstKey = validKey();
+    const secondKey = validKey();
+    await first.keyring.commit(first.keyring.prepare(firstKey, timestamp));
+    let primaryPublished = false;
+    let postPublicationSyncFailed = false;
+    const fsImpl = {
+      ...fs.promises,
+      open: async (target, flags, mode) => {
+        const handle = await fs.promises.open(target, flags, mode);
+        if (path.resolve(String(target)) !== path.resolve(first.directory)) return handle;
+        return {
+          sync: async () => {
+            if (primaryPublished && !postPublicationSyncFailed) {
+              postPublicationSyncFailed = true;
+              throw Object.assign(new Error('post-publication sync failed'), { code: 'EIO' });
+            }
+            return handle.sync();
+          },
+          close: () => handle.close()
+        };
+      },
+      rename: async (source, target) => {
+        await fs.promises.rename(source, target);
+        if (target === first.filePath) primaryPublished = true;
+      }
+    };
+    const { createOnlineBackupKeyring } = require('../lib/online-backup-keyring');
+    const keyring = createOnlineBackupKeyring({
+      filePath: first.filePath,
+      encryptField: encrypt,
+      decryptField: decrypt,
+      isEncrypted: isCanonicalEnvelope,
+      fsImpl
+    });
+
+    assert.equal(await keyring.commit(keyring.prepare(secondKey, '2026-08-22T11:00:00.000Z')), true);
+    const recoveryTemp = fs.readdirSync(first.directory).find(name => name.endsWith('.recovery.tmp'));
+    assert.ok(recoveryTemp);
+    fs.writeFileSync(first.filePath, '{damaged-primary');
+    fs.writeFileSync(first.backupPath, '{damaged-backup');
+    const recovered = createOnlineBackupKeyring({
+      filePath: first.filePath,
+      encryptField: encrypt,
+      decryptField: decrypt,
+      isEncrypted: isCanonicalEnvelope
+    });
+
+    const snapshot = await recovered.list();
+
+    assert.deepEqual(snapshot.entries.map(entry => entry.id), [
+      parseOnlineBackupKey(secondKey).id,
+      parseOnlineBackupKey(firstKey).id
+    ]);
+    assert.deepEqual(snapshot.issues, ['KEYRING_RECOVERED']);
  });

  it('recovers a validated backup without presenting corruption as an empty keyring', async () => {
@@ -418,6 +519,65 @@ describe('encrypted online backup keyring', () => {
    assert.deepEqual(snapshot.issues, ['KEYRING_RECOVERED']);
  });

+  it('prefers a recovery temp over the backup when both have the same modification time', async () => {
+    const first = fixture();
+    const older = validKey();
+    const newer = validKey();
+    const olderEntry = first.keyring.prepare(older, timestamp);
+    const newerEntry = first.keyring.prepare(newer, '2026-08-22T11:00:00.000Z');
+    await first.keyring.commit(olderEntry);
+    fs.writeFileSync(first.filePath, '{damaged-primary');
+    const recoveryTemp = path.join(first.directory, `.online-backup-keyring.json.${process.pid}.${crypto.randomUUID()}.recovery.tmp`);
+    writeKeyring(recoveryTemp, [olderEntry, newerEntry]);
+    const sameTime = new Date('2026-08-22T12:00:00.000Z');
+    fs.utimesSync(first.backupPath, sameTime, sameTime);
+    fs.utimesSync(recoveryTemp, sameTime, sameTime);
+    const { createOnlineBackupKeyring } = require('../lib/online-backup-keyring');
+    const recovered = createOnlineBackupKeyring({
+      filePath: first.filePath,
+      encryptField: encrypt,
+      decryptField: decrypt,
+      isEncrypted: isCanonicalEnvelope
+    });
+
+    const snapshot = await recovered.list();
+
+    assert.deepEqual(snapshot.entries.map(entry => entry.id), [
+      parseOnlineBackupKey(newer).id,
+      parseOnlineBackupKey(older).id
+    ]);
+    assert.deepEqual(snapshot.issues, ['KEYRING_RECOVERED']);
+  });
+
+  it('recovers an equal-time commit temp instead of skipping it behind the valid primary', async () => {
+    const first = fixture();
+    const older = validKey();
+    const newer = validKey();
+    const olderEntry = first.keyring.prepare(older, timestamp);
+    const newerEntry = first.keyring.prepare(newer, '2026-08-22T11:00:00.000Z');
+    await first.keyring.commit(olderEntry);
+    const recoveryTemp = path.join(first.directory, `.online-backup-keyring.json.${process.pid}.${crypto.randomUUID()}.recovery.tmp`);
+    writeKeyring(recoveryTemp, [olderEntry, newerEntry]);
+    const sameTime = new Date('2026-08-22T12:00:00.000Z');
+    fs.utimesSync(first.filePath, sameTime, sameTime);
+    fs.utimesSync(recoveryTemp, sameTime, sameTime);
+    const { createOnlineBackupKeyring } = require('../lib/online-backup-keyring');
+    const recovered = createOnlineBackupKeyring({
+      filePath: first.filePath,
+      encryptField: encrypt,
+      decryptField: decrypt,
+      isEncrypted: isCanonicalEnvelope
+    });
+
+    const snapshot = await recovered.list();
+
+    assert.deepEqual(snapshot.entries.map(entry => entry.id), [
+      parseOnlineBackupKey(newer).id,
+      parseOnlineBackupKey(older).id
+    ]);
+    assert.deepEqual(snapshot.issues, ['KEYRING_RECOVERED']);
+  });
+
  it('keeps the prior valid state and cleans temporary files when primary replacement fails', async () => {
    const first = fixture();
    const firstKey = validKey();