From 8041aeead96ffa24080721500abac4b1476f78c6 Mon Sep 17 00:00:00 2001 From: Sucukdeluxe <259325684+Sucukdeluxe@users.noreply.github.com> Date: Sat, 22 Aug 2026 15:24:33 +0200 Subject: [PATCH] fix: complete managed backup key remediation Recover from structurally valid but undecryptable primary keyrings by selecting a fully cryptographically validated backup or recovery temp and reporting KEYRING_RECOVERED. Keep committed primary writes successful when best-effort post-publication cleanup fails, preventing ambiguous retries after durable local publication. Intercept clipboard writes inside the hidden Electron Main-process integration probe, remove the duplicate key-created toast, and make authoritative refresh coverage differ from optimistic state. --- lib/online-backup-keyring.js | 28 ++++++++++----- renderer/app.js | 1 - renderer/i18n.js | 1 - tests/online-backup-keyring.test.js | 55 +++++++++++++++++++++++++++++ tests/startup-renderer.test.js | 37 +++++++++++-------- 5 files changed, 98 insertions(+), 24 deletions(-) diff --git a/lib/online-backup-keyring.js b/lib/online-backup-keyring.js index 3c4f853..1f825f1 100644 --- a/lib/online-backup-keyring.js +++ b/lib/online-backup-keyring.js @@ -216,24 +216,34 @@ function createOnlineBackupKeyring({ async function readState() { const primary = await readCandidate(filePath); if (primary.status === 'valid') { + const primaryState = inspectSource({ ...primary, recovered: false }); let primaryModified = 0; try { primaryModified = (await fsImpl.stat(filePath)).mtimeMs; } catch {} const candidates = await recoveryCandidates(); + if (!firstBlockingIssue(primaryState)) { + for (const candidatePath of candidates) { + if (!isTemporaryFileName(path.basename(candidatePath))) continue; + let candidateModified = 0; + try { + candidateModified = (await fsImpl.stat(candidatePath)).mtimeMs; + } catch {} + if (candidateModified <= primaryModified) continue; + const candidate = await readCandidate(candidatePath); + if (candidate.status !== 'valid') continue; + const state = inspectSource({ ...candidate, recovered: true }); + if (!firstBlockingIssue(state)) return state; + } + return primaryState; + } for (const candidatePath of candidates) { - if (!isTemporaryFileName(path.basename(candidatePath))) continue; - let candidateModified = 0; - try { - candidateModified = (await fsImpl.stat(candidatePath)).mtimeMs; - } catch {} - if (candidateModified <= primaryModified) continue; const candidate = await readCandidate(candidatePath); if (candidate.status !== 'valid') continue; const state = inspectSource({ ...candidate, recovered: true }); if (!firstBlockingIssue(state)) return state; } - return inspectSource({ ...primary, recovered: false }); + return primaryState; } const candidates = await recoveryCandidates(); let fallback = null; @@ -363,7 +373,9 @@ function createOnlineBackupKeyring({ } catch { if (!primaryPublished) throw issueError(KEYRING_ERROR_CODES.structure); } - await cleanupTemporaryFiles(recoveryPath); + try { + await cleanupTemporaryFiles(recoveryPath); + } catch {} } async function list() { diff --git a/renderer/app.js b/renderer/app.js index 1703840..fc24126 100644 --- a/renderer/app.js +++ b/renderer/app.js @@ -2977,7 +2977,6 @@ async function doOnlineBackupCreate() { if (!upsertManagedOnlineBackup(result.entry)) setManagedOnlineBackupRefreshIssue('Online-Sicherungen konnten nicht geladen werden', 'error'); await loadManagedOnlineBackups({ mutationGeneration: authority.mutationGeneration }); setOnlineBackupStatus('Neuer Schlüssel erstellt.', 'success', authority.statusContext); - showCopyToast('Online-Schlüssel erstellt'); } catch { setOnlineBackupStatus('Online-Sicherung konnte nicht erstellt werden', 'error', authority.statusContext); } finally { diff --git a/renderer/i18n.js b/renderer/i18n.js index 9fda6b9..cef0ea8 100644 --- a/renderer/i18n.js +++ b/renderer/i18n.js @@ -462,7 +462,6 @@ ['Log-Pfad automatisch auf funktionierenden Ordner gesetzt', 'Log path automatically changed to a writable folder'], ['Neuer Schlüssel erstellt.', 'New key created.'], ['Nicht alle Einstellungen konnten gespeichert werden', 'Not all settings could be saved'], - ['Online-Schlüssel erstellt', 'Online key created'], ['Online-Schlüssel kopiert', 'Online key copied'], ['Online-Sicherung konnte nicht erstellt werden', 'Online backup could not be created'], ['Online-Sicherungen konnten nicht geladen werden', 'Online backups could not be loaded'], diff --git a/tests/online-backup-keyring.test.js b/tests/online-backup-keyring.test.js index 579e35e..85f8816 100644 --- a/tests/online-backup-keyring.test.js +++ b/tests/online-backup-keyring.test.js @@ -339,6 +339,30 @@ describe('encrypted online backup keyring', () => { assert.deepEqual(snapshot.issues, ['KEYRING_RECOVERED']); }); + it('recovers a cryptographically valid backup when the primary only passes structural validation', async () => { + const first = fixture(); + const key = validKey(); + await first.keyring.commit(first.keyring.prepare(key, timestamp)); + writeKeyring(first.filePath, [{ + id: parseOnlineBackupKey(key).id, + encryptedKey: 'enc:v1:YWJjZA', + createdAt: timestamp + }]); + 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(key).id]); + assert.deepEqual(snapshot.issues, ['KEYRING_RECOVERED']); + assert.equal(await recovered.getKey(parseOnlineBackupKey(key).id), key); + }); + it('skips a newer cryptographically invalid recovery temp in favor of the validated backup', async () => { const first = fixture(); const key = validKey(); @@ -421,6 +445,37 @@ describe('encrypted online backup keyring', () => { assert.equal(await first.keyring.getKey(parseOnlineBackupKey(firstKey).id), firstKey); }); + it('commits successfully once the primary is published even when later cleanup fails', async () => { + const first = fixture(); + const firstKey = validKey(); + const secondKey = validKey(); + await first.keyring.commit(first.keyring.prepare(firstKey, timestamp)); + let readdirCalls = 0; + const fsImpl = { + ...fs.promises, + readdir: async (...args) => { + readdirCalls++; + if (readdirCalls === 2) throw Object.assign(new Error('cleanup failed'), { code: 'EIO' }); + return fs.promises.readdir(...args); + } + }; + 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); + assert.equal(await keyring.getKey(parseOnlineBackupKey(secondKey).id), secondKey); + assert.deepEqual((await keyring.list()).entries.map(entry => entry.id), [ + parseOnlineBackupKey(secondKey).id, + parseOnlineBackupKey(firstKey).id + ]); + }); + it('types invalid documents and never includes plaintext or ciphertext in errors', async () => { const { filePath, backupPath, keyring } = fixture(); const key = validKey(); diff --git a/tests/startup-renderer.test.js b/tests/startup-renderer.test.js index 163a4d9..ce5298c 100644 --- a/tests/startup-renderer.test.js +++ b/tests/startup-renderer.test.js @@ -67,7 +67,8 @@ const managedOnlineBackupIds = { a: 'AAAAAAAAAAAAAAAAAAAAAA', b: 'AQEBAQEBAQEBAQEBAQEBAQ', c: 'AgICAgICAgICAgICAgICAg', - d: 'AwMDAwMDAwMDAwMDAwMDAw' + d: 'AwMDAwMDAwMDAwMDAwMDAw', + e: 'BAQEBAQEBAQEBAQEBAQEBA' }; const managedOnlineBackupListResponses = [ { ok: true, warningCode: 'KEYRING_DECRYPT_FAILED', warning: 'Gespeicherter Online-Sicherungsschlüssel konnte nicht entschlüsselt werden', entries: [ @@ -107,8 +108,8 @@ contextBridge.exposeInMainWorld('api', { const resolve = pendingManagedOnlineBackupLists.get(index); pendingManagedOnlineBackupLists.delete(index); resolve({ ok: true, entries: [ + { id: managedOnlineBackupIds.e, displayKey: 'MHU2-AUTH…9999', createdAt: '2026-08-26T10:00:00.000Z' }, { id: managedOnlineBackupIds.d, displayKey: 'MHU2-DFGH…2468', createdAt: '2026-08-25T10:00:00.000Z' }, - { id: managedOnlineBackupIds.c, displayKey: 'MHU2-QWER…4321', createdAt: '2026-08-23T12:00:00.000Z' }, { id: managedOnlineBackupIds.a, displayKey: 'MHU2-ABCD…1234', createdAt: '2026-08-20T08:00:00.000Z' } ] }); }, @@ -146,7 +147,8 @@ contextBridge.exposeInMainWorld('api', { a: 'AAAAAAAAAAAAAAAAAAAAAA', b: 'AQEBAQEBAQEBAQEBAQEBAQ', c: 'AgICAgICAgICAgICAgICAg', - d: 'AwMDAwMDAwMDAwMDAwMDAw' + d: 'AwMDAwMDAwMDAwMDAwMDAw', + e: 'BAQEBAQEBAQEBAQEBAQEBA' }; const fixture = document.createElement('section'); fixture.innerHTML = '
'; @@ -168,6 +170,8 @@ contextBridge.exposeInMainWorld('api', { }; flushPendingSettingsSaves = async () => {}; openOnlineBackupView = () => {}; + const toastMessages = []; + showCopyToast = message => toastMessages.push(localizeUiText(message)); await loadManagedOnlineBackups(); await new Promise(resolve => setTimeout(resolve, 0)); const initialKeys = [...document.querySelectorAll('.online-backup-managed-key')].map(element => element.textContent); @@ -198,6 +202,7 @@ contextBridge.exposeInMainWorld('api', { focusAction: document.activeElement?.dataset.managedOnlineBackupAction, focusId: document.activeElement?.dataset.managedOnlineBackupId }; + const beforeCreateToasts = toastMessages.length; await doOnlineBackupCreate(); await new Promise(resolve => setTimeout(resolve, 0)); const refreshFailure = { @@ -207,6 +212,7 @@ contextBridge.exposeInMainWorld('api', { warning: document.getElementById('managedOnlineBackupRefreshMessage')?.textContent, retryVisible: document.getElementById('reloadManagedOnlineBackupsBtn')?.offsetParent !== null }; + const createSuccessToasts = toastMessages.slice(beforeCreateToasts); const beforeRetryCalls = (await calls('list')).length; document.getElementById('reloadManagedOnlineBackupsBtn')?.click(); const retryTriggeredLoad = await waitFor(async () => (await calls('list')).length === beforeRetryCalls + 1); @@ -251,6 +257,7 @@ contextBridge.exposeInMainWorld('api', { ariaDescriptions, afterDelete, refreshFailure, + createSuccessToasts, retryTriggeredLoad, afterRetry, copyFocusRestored, @@ -418,6 +425,7 @@ app.whenReady().then(async () => { warning: 'Stored online backup key could not be decrypted', retryVisible: true }); + assert.deepEqual(result.onlineBackupBehavior.createSuccessToasts, []); assert.equal(result.onlineBackupBehavior.retryTriggeredLoad, true); assert.deepEqual(result.onlineBackupBehavior.afterRetry, { keys: ['MHU2-QWER…4321', 'MHU2-ABCD…1234'], @@ -425,7 +433,7 @@ app.whenReady().then(async () => { }); assert.equal(result.onlineBackupBehavior.copyFocusRestored, true); assert.deepEqual(result.onlineBackupBehavior.raceResult, { - keys: ['MHU2-DFGH…2468', 'MHU2-QWER…4321', 'MHU2-ABCD…1234'], + keys: ['MHU2-AUTH…9999', 'MHU2-DFGH…2468', 'MHU2-ABCD…1234'], status: 'New key created.', statusState: 'success', warningHidden: true @@ -523,7 +531,13 @@ const serverModulePath = process.env.MHU_DPAPI_SERVER_MODULE; app.setPath('userData', userDataPath); let server = null; let window = null; -let previousClipboard = ''; +let interceptedClipboard = ''; +let interceptedClipboardWrites = 0; +const originalClipboardWriteText = clipboard.writeText; +clipboard.writeText = value => { + interceptedClipboard = value; + interceptedClipboardWrites++; +}; const logs = []; const originalConsole = { log: console.log, warn: console.warn, error: console.error }; for (const method of Object.keys(originalConsole)) console[method] = (...values) => { logs.push(values.map(String).join(' ')); }; @@ -545,7 +559,6 @@ async function closeServer() { server = null; } app.whenReady().then(async () => { - previousClipboard = clipboard.readText(); if (!safeStorage.isEncryptionAvailable()) throw new Error('safeStorage unavailable'); const { createBackupServer } = await import(pathToFileURL(serverModulePath).href); fs.mkdirSync(serverDataPath, { recursive: true }); @@ -597,7 +610,7 @@ app.whenReady().then(async () => { ipcMain.handle('online-backup:copy-managed', async (event, id) => { if (!trusted(event)) return { ok: false, error: 'rejected' }; const result = await manager.copyManaged(requireId(id)); - clipboardMatched = clipboard.readText() === fullKey; + clipboardMatched = interceptedClipboard === fullKey && interceptedClipboardWrites === 1; return result; }); ipcMain.handle('online-backup:delete-managed', (event, id) => trusted(event) ? manager.deleteManaged(requireId(id)) : { ok: false, error: 'rejected' }); @@ -635,6 +648,7 @@ app.whenReady().then(async () => { rendererSecretAbsent: !ipcJson.includes(fullKey) && !/MHU2-[A-Za-z0-9_-]{70}/.test(ipcJson), logSecretAbsent: logs.every(line => !line.includes(fullKey)) && !logs.join(' ').match(/MHU2-[A-Za-z0-9_-]{70}/), clipboardMatched, + interceptedClipboardWrites, serverRecordBeforeDelete, serverEmptyAfterDelete: remainingServerRecords.length === 0, keyringEmptyAfterDelete: afterDocument.keys.length === 0, @@ -645,12 +659,7 @@ app.whenReady().then(async () => { fs.writeFileSync(outputPath, JSON.stringify({ error: error.stack || String(error) }), 'utf8'); process.exitCode = 1; }).finally(async () => { - clipboard.writeText(previousClipboard); - if (fs.existsSync(outputPath)) { - const result = JSON.parse(fs.readFileSync(outputPath, 'utf8')); - result.clipboardRestored = clipboard.readText() === previousClipboard; - fs.writeFileSync(outputPath, JSON.stringify(result), 'utf8'); - } + clipboard.writeText = originalClipboardWriteText; for (const channel of ['online-backup:list-managed', 'online-backup:create-managed', 'online-backup:copy-managed', 'online-backup:delete-managed']) ipcMain.removeHandler(channel); if (window && !window.isDestroyed()) window.destroy(); await closeServer(); @@ -696,7 +705,7 @@ app.whenReady().then(async () => { rendererSecretAbsent: true, logSecretAbsent: true, clipboardMatched: true, - clipboardRestored: true, + interceptedClipboardWrites: 1, serverRecordBeforeDelete: true, serverEmptyAfterDelete: true, keyringEmptyAfterDelete: true,