From e7edaf34da886c27e9d0827057e90beef751b349 Mon Sep 17 00:00:00 2001 From: automated-signal <37887102+automated-signal@users.noreply.github.com> Date: Mon, 9 Mar 2026 14:03:51 -0500 Subject: [PATCH] Local backup validation improvements Co-authored-by: trevor-signal <131492920+trevor-signal@users.noreply.github.com> --- ts/components/Preferences.dom.stories.tsx | 1 + ts/services/backups/export.preload.ts | 15 ++++- ts/services/backups/index.preload.ts | 58 ++++++++++++------- ts/services/backups/types.std.ts | 10 +--- ts/services/backups/util/localBackup.node.ts | 6 +- ts/services/backups/validator.preload.ts | 18 +++++- .../backup/filePointer_test.preload.ts | 2 - 7 files changed, 71 insertions(+), 39 deletions(-) diff --git a/ts/components/Preferences.dom.stories.tsx b/ts/components/Preferences.dom.stories.tsx index 9aeb5bb0cb..48f3d7a26f 100644 --- a/ts/components/Preferences.dom.stories.tsx +++ b/ts/components/Preferences.dom.stories.tsx @@ -115,6 +115,7 @@ const availableSpeakers = [ const validateBackupResult: ExportResultType = { attachmentBackupJobs: [], + mediaNames: [], totalBytes: 100, duration: 10000, stats: { diff --git a/ts/services/backups/export.preload.ts b/ts/services/backups/export.preload.ts index 82b1f4b4c4..2a908fafa1 100644 --- a/ts/services/backups/export.preload.ts +++ b/ts/services/backups/export.preload.ts @@ -291,7 +291,11 @@ export class BackupExportStream extends Readable { // array. #customColorIdByUuid = new Map(); - constructor(private readonly options: Readonly) { + constructor( + private readonly options: Readonly & { + validationRun?: boolean; + } + ) { super(); } @@ -365,8 +369,8 @@ export class BackupExportStream extends Readable { ); } - public getMediaNamesIterator(): MapIterator { - return this.#mediaNamesToFilePointers.keys(); + public getMediaNames(): Array { + return [...this.#mediaNamesToFilePointers.keys()]; } public getStats(): Readonly { @@ -826,6 +830,11 @@ export class BackupExportStream extends Readable { chatItem, }); + if (this.options.validationRun) { + // flush every chatItem to expose all validation errors + await this.#flush(); + } + this.#stats.messages += 1; }, { concurrency: MAX_CONCURRENCY } diff --git a/ts/services/backups/index.preload.ts b/ts/services/backups/index.preload.ts index 0902c646b3..713f13b68a 100644 --- a/ts/services/backups/index.preload.ts +++ b/ts/services/backups/index.preload.ts @@ -354,10 +354,8 @@ export class BackupsService { fnLog.info('offline; skipping wait for empty queues'); } - const snapshotDir = join( - options.backupsBaseDir, - `signal-backup-${getTimestampForFolder()}` - ); + // Just in case it's been deleted, ensure the backup dir exists + await mkdir(options.backupsBaseDir, { recursive: true }); const freeSpaceBytes = await getFreeDiskSpace(options.backupsBaseDir); const bytesNeeded = MIMINUM_DISK_SPACE_FOR_LOCAL_EXPORT - freeSpaceBytes; @@ -378,15 +376,31 @@ export class BackupsService { backupsBaseDir: options.backupsBaseDir, }); + const snapshotDir = join( + options.backupsBaseDir, + `signal-backup-${getTimestampForFolder()}` + ); + try { await mkdir(snapshotDir, { recursive: true }); const exportResult = await this.exportToDisk(join(snapshotDir, 'main'), { type: 'local-encrypted', - snapshotDir, abortSignal: options.abortSignal, }); + log.info('exportLocalBackup: writing local backup files list'); + const filesWritten = await writeLocalBackupFilesList({ + snapshotDir, + mediaNames: exportResult.mediaNames, + }); + const filesRead = await readLocalBackupFilesList(snapshotDir); + strictAssert( + isEqual(filesWritten, filesRead), + 'exportBackup: Local backup files proto must match files written' + ); + + log.info('exportLocalBackup: writing metadata'); const metadataArgs = { snapshotDir, backupId: getBackupId(), @@ -394,6 +408,7 @@ export class BackupsService { }; await writeLocalBackupMetadata(metadataArgs); await verifyLocalBackupMetadata(metadataArgs); + await this.#runLocalAttachmentBackupJobs({ attachmentBackupJobs: exportResult.attachmentBackupJobs, baseDir: options.backupsBaseDir, @@ -702,14 +717,19 @@ export class BackupsService { } // Test harness - public async _internalValidate(): Promise { + public async _internalValidate( + exportOptions: BackupExportOptions = { + type: 'local-encrypted', + abortSignal: new AbortController().signal, + } + ): Promise { try { + log.info('internal validation: starting'); const start = Date.now(); const recordStream = new BackupExportStream({ - type: 'remote', - level: BackupLevel.Free, - abortSignal: new AbortController().signal, + ...exportOptions, + validationRun: true, }); recordStream.run(); @@ -718,15 +738,21 @@ export class BackupsService { const duration = Date.now() - start; + log.info('internal validation: succeeded'); return { result: { attachmentBackupJobs: recordStream.getAttachmentBackupJobs(), + mediaNames: recordStream.getMediaNames(), duration, stats: recordStream.getStats(), totalBytes, }, }; } catch (error) { + log.warn( + 'internal validation: failed with errors', + Errors.toLogFormat(error) + ); return { error: Errors.toLogFormat(error) }; } } @@ -1212,22 +1238,10 @@ export class BackupsService { throw missingCaseError(type); } - if (type === 'local-encrypted') { - log.info('exportBackup: writing local backup files list'); - const filesWritten = await writeLocalBackupFilesList({ - snapshotDir: options.snapshotDir, - mediaNamesIterator: recordStream.getMediaNamesIterator(), - }); - const filesRead = await readLocalBackupFilesList(options.snapshotDir); - strictAssert( - isEqual(filesWritten, filesRead), - 'exportBackup: Local backup files proto must match files written' - ); - } - const duration = Date.now() - start; return { attachmentBackupJobs: recordStream.getAttachmentBackupJobs(), + mediaNames: recordStream.getMediaNames(), totalBytes, stats: recordStream.getStats(), duration, diff --git a/ts/services/backups/types.std.ts b/ts/services/backups/types.std.ts index a3460bfe5e..843edd91f2 100644 --- a/ts/services/backups/types.std.ts +++ b/ts/services/backups/types.std.ts @@ -39,13 +39,8 @@ export type BackupExportOptions = { abortSignal: AbortSignal } & ( type: 'remote' | 'cross-client-integration-test'; level: BackupLevel; } - | { - type: 'plaintext-export'; - } - | { - type: 'local-encrypted'; - snapshotDir: string; - } + | { type: 'plaintext-export' } + | { type: 'local-encrypted' } ); export type BackupImportOptions = ( @@ -86,6 +81,7 @@ export type ExportResultType = Readonly<{ attachmentBackupJobs: ReadonlyArray< CoreAttachmentBackupJobType | CoreAttachmentLocalBackupJobType >; + mediaNames: Array; totalBytes: number; duration: number; stats: Readonly; diff --git a/ts/services/backups/util/localBackup.node.ts b/ts/services/backups/util/localBackup.node.ts index 0573b250bc..a12082ff7c 100644 --- a/ts/services/backups/util/localBackup.node.ts +++ b/ts/services/backups/util/localBackup.node.ts @@ -153,10 +153,10 @@ export async function verifyLocalBackupMetadata({ export async function writeLocalBackupFilesList({ snapshotDir, - mediaNamesIterator, + mediaNames, }: { snapshotDir: string; - mediaNamesIterator: MapIterator; + mediaNames: Array; }): Promise> { const { promise, resolve, reject } = explodePromise>(); @@ -167,7 +167,7 @@ export async function writeLocalBackupFilesList({ }); const files: Array = []; - for (const mediaName of mediaNamesIterator) { + for (const mediaName of mediaNames) { const data = Signal.backup.local.FilesFrame.encodeDelimited({ mediaName, }).finish(); diff --git a/ts/services/backups/validator.preload.ts b/ts/services/backups/validator.preload.ts index b45dc3d8ce..1a773fc70b 100644 --- a/ts/services/backups/validator.preload.ts +++ b/ts/services/backups/validator.preload.ts @@ -62,6 +62,8 @@ export async function validateBackupStream( let totalBytes = 0; let frameCount = 0; + + const allErrorMessages: Array = []; readable.on('data', delimitedFrame => { totalBytes += delimitedFrame.byteLength; frameCount += 1; @@ -79,12 +81,24 @@ export async function validateBackupStream( } strictAssert(validator != null, 'validator must be already created'); - validator.addFrame(frame); + try { + validator.addFrame(frame); + } catch (error) { + allErrorMessages.push(error.message); + } }); await once(readable, 'end'); strictAssert(validator != null, 'no frames'); - validator.finalize(); + try { + validator.finalize(); + } catch (error) { + allErrorMessages.push(error.message); + } + + if (allErrorMessages.length) { + throw new Error(allErrorMessages.join('\n')); + } return totalBytes; } diff --git a/ts/test-electron/backup/filePointer_test.preload.ts b/ts/test-electron/backup/filePointer_test.preload.ts index 14fe421e24..16b5095ffb 100644 --- a/ts/test-electron/backup/filePointer_test.preload.ts +++ b/ts/test-electron/backup/filePointer_test.preload.ts @@ -467,7 +467,6 @@ describe('getFilePointerForAttachment', () => { attachment: defaultAttachment, backupOptions: { type: 'local-encrypted', - snapshotDir: '/root/backups/signal-backup-12-12-12', abortSignal: new AbortController().signal, }, getBackupCdnInfo: notInBackupCdn, @@ -507,7 +506,6 @@ describe('getFilePointerForAttachment', () => { attachment: { ...defaultAttachment, path: 'no/file/here' }, backupOptions: { type: 'local-encrypted', - snapshotDir: '/root/backups/signal-backup-12-12-12', abortSignal: new AbortController().signal, }, getBackupCdnInfo: notInBackupCdn,