From 15c363912fbeef0d05a6fc1558a825a7c2814298 Mon Sep 17 00:00:00 2001 From: Sucukdeluxe <259325684+Sucukdeluxe@users.noreply.github.com> Date: Sat, 22 Aug 2026 16:13:43 +0200 Subject: [PATCH] fix: bind removal plans to full key identity Require both the current record ID and decrypted full key to match a prepared removal plan before deleting local state. Invalidate mismatched plans before returning KEYRING_REMOVE_PLAN_INVALID so a replacement with the same canonical ID cannot be deleted and the stale plan cannot be reused. Cover normal, idempotent, rebased, duplicate-ID, and same-ID replacement removal behavior while preserving the replacement generation. --- lib/online-backup-keyring.js | 7 ++++++- tests/online-backup-keyring.test.js | 26 ++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/lib/online-backup-keyring.js b/lib/online-backup-keyring.js index bf61127..ff2b541 100644 --- a/lib/online-backup-keyring.js +++ b/lib/online-backup-keyring.js @@ -507,10 +507,15 @@ function createOnlineBackupKeyring({ const state = await readState(); const blockingIssue = firstBlockingIssue(state); if (blockingIssue) throw issueError(blockingIssue); - if (!state.entries.some(current => current.id === plan.id)) { + const currentEntry = state.entries.find(current => current.id === plan.id); + if (!currentEntry) { removalPlans.delete(plan); return false; } + if (currentEntry.key !== plan.key) { + removalPlans.delete(plan); + throw issueError(KEYRING_ERROR_CODES.plan); + } await writeEntries( state.entries.filter(current => current.id !== plan.id), nextGeneration(state.generation) diff --git a/tests/online-backup-keyring.test.js b/tests/online-backup-keyring.test.js index fb687fe..fbcf8c5 100644 --- a/tests/online-backup-keyring.test.js +++ b/tests/online-backup-keyring.test.js @@ -284,6 +284,32 @@ describe('encrypted online backup keyring', () => { assert.deepEqual((await keyring.list()).entries, []); }); + it('invalidates a stale removal plan when the same ID now belongs to a different key', async () => { + const { filePath, keyring } = fixture(); + const original = validKey(); + const replacement = keyWithRecordId(original, 0x5a); + const id = parseOnlineBackupKey(original).id; + await keyring.commit(keyring.prepare(original, timestamp)); + const stalePlan = await keyring.prepareRemove(id); + const currentPlan = await keyring.prepareRemove(id); + assert.equal(await keyring.commitRemove(currentPlan), true); + assert.equal(await keyring.commit(keyring.prepare(replacement, '2026-08-22T11:00:00.000Z')), true); + const generationBefore = JSON.parse(fs.readFileSync(filePath, 'utf8')).generation; + + await assert.rejects( + keyring.commitRemove(stalePlan), + error => error.code === 'KEYRING_REMOVE_PLAN_INVALID' + ); + await assert.rejects( + keyring.commitRemove(stalePlan), + error => error.code === 'KEYRING_REMOVE_PLAN_INVALID' + ); + + assert.equal(await keyring.getKey(id), replacement); + assert.equal(JSON.parse(fs.readFileSync(filePath, 'utf8')).generation, generationBefore); + assert.equal(generationBefore, 3); + }); + it('blocks removal before remote work when any neighboring entry is corrupt', async () => { const { filePath, keyring } = fixture(); const valid = validKey();