From a7cd27f3cf3be2d9fe3cb1bc4c817bb750782886 Mon Sep 17 00:00:00 2001
From: trevor-signal <131492920+trevor-signal@users.noreply.github.com>
Date: Thu, 31 Jul 2025 16:21:59 -0400
Subject: [PATCH] Skip queueing old attachments on non-media-enabled backup
 import

Co-authored-by: Scott Nonnenberg <scott@signal.org>
---
 ts/jobs/AttachmentDownloadManager.ts          | 36 +++++++-
 .../AttachmentDownloadManager_test.ts         | 87 +++++++++++++++++--
 2 files changed, 115 insertions(+), 8 deletions(-)

diff --git a/ts/jobs/AttachmentDownloadManager.ts b/ts/jobs/AttachmentDownloadManager.ts
index 9ec03dc1da..6da1862906 100644
--- a/ts/jobs/AttachmentDownloadManager.ts
+++ b/ts/jobs/AttachmentDownloadManager.ts
@@ -65,6 +65,8 @@ import { formatCountForLogging } from '../logging/formatCountForLogging';
 import { strictAssert } from '../util/assert';
 import { updateBackupMediaDownloadProgress } from '../util/updateBackupMediaDownloadProgress';
 import { HTTPError } from '../textsecure/Errors';
+import { isOlderThan } from '../util/timestamp';
+import { getMessageQueueTime as doGetMessageQueueTime } from '../util/getMessageQueueTime';
 
 const log = createLogger('AttachmentDownloadManager');
 
@@ -103,6 +105,7 @@ type RunDownloadAttachmentJobOptions = {
   isForCurrentlyVisibleMessage: boolean;
   maxAttachmentSizeInKib: number;
   maxTextAttachmentSizeInKib: number;
+  hasMediaBackups: boolean;
 };
 
 type AttachmentDownloadManagerParamsType = Omit<
@@ -122,6 +125,8 @@ type AttachmentDownloadManagerParamsType = Omit<
     dependencies?: DependenciesType;
   }) => Promise<JobManagerJobResultType<CoreAttachmentDownloadJobType>>;
   onLowDiskSpaceBackupImport: (bytesNeeded: number) => Promise<void>;
+  hasMediaBackups: () => boolean;
+  getMessageQueueTime: () => number;
   statfs: typeof statfs;
 };
 
@@ -149,6 +154,8 @@ export class AttachmentDownloadManager extends JobManager<CoreAttachmentDownload
     },
   });
   #onLowDiskSpaceBackupImport: (bytesNeeded: number) => Promise<void>;
+  #getMessageQueueTime: () => number;
+  #hasMediaBackups: () => boolean;
   #statfs: typeof statfs;
   #maxAttachmentSizeInKib = getMaximumIncomingAttachmentSizeInKb(getValue);
   #maxTextAttachmentSizeInKib =
@@ -204,6 +211,8 @@ export class AttachmentDownloadManager extends JobManager<CoreAttachmentDownload
         bytesNeeded
       );
     },
+    hasMediaBackups: () => window.Signal.Services.backups.hasMediaBackups(),
+    getMessageQueueTime: () => doGetMessageQueueTime(),
     statfs,
   };
 
@@ -244,6 +253,7 @@ export class AttachmentDownloadManager extends JobManager<CoreAttachmentDownload
           isLastAttempt,
           options: {
             abortSignal,
+            hasMediaBackups: this.#hasMediaBackups(),
             isForCurrentlyVisibleMessage,
             maxAttachmentSizeInKib: this.#maxAttachmentSizeInKib,
             maxTextAttachmentSizeInKib: this.#maxTextAttachmentSizeInKib,
@@ -252,6 +262,8 @@ export class AttachmentDownloadManager extends JobManager<CoreAttachmentDownload
       },
     });
     this.#onLowDiskSpaceBackupImport = params.onLowDiskSpaceBackupImport;
+    this.#getMessageQueueTime = params.getMessageQueueTime;
+    this.#hasMediaBackups = params.hasMediaBackups;
     this.#statfs = params.statfs;
   }
 
@@ -272,8 +284,25 @@ export class AttachmentDownloadManager extends JobManager<CoreAttachmentDownload
 
     const logId = `AttachmentDownloadManager/addJob(${sentAt}.${attachmentType})`;
 
-    if (attachment.error && source === AttachmentDownloadSource.BACKUP_IMPORT) {
-      return attachment;
+    if (source === AttachmentDownloadSource.BACKUP_IMPORT) {
+      if (attachment.error) {
+        return attachment;
+      }
+
+      // For non-media-enabled backups, we will skip queueing download for old attachments
+      // that cannot still be on the transit tier
+      if (!this.#hasMediaBackups() && !wasImportedFromLocalBackup(attachment)) {
+        const attachmentUploadedAt = attachment.uploadTimestamp || sentAt;
+
+        // Skip queueing download if attachment is older than twice the message queue time
+        // (a generous buffer that ensures we download anything that could still exist on
+        // the transit tier)
+        if (
+          isOlderThan(attachmentUploadedAt, this.#getMessageQueueTime() * 2)
+        ) {
+          return attachment;
+        }
+      }
     }
 
     const parseResult = safeParsePartial(coreAttachmentDownloadJobSchema, {
@@ -458,6 +487,7 @@ async function runDownloadAttachmentJob({
     const result = await runDownloadAttachmentJobInner({
       job,
       abortSignal: options.abortSignal,
+      hasMediaBackups: options.hasMediaBackups,
       isForCurrentlyVisibleMessage:
         options?.isForCurrentlyVisibleMessage ?? false,
       maxAttachmentSizeInKib: options.maxAttachmentSizeInKib,
@@ -582,6 +612,7 @@ export async function runDownloadAttachmentJobInner({
   isForCurrentlyVisibleMessage,
   maxAttachmentSizeInKib,
   maxTextAttachmentSizeInKib,
+  hasMediaBackups,
   dependencies,
 }: {
   job: AttachmentDownloadJobType;
@@ -616,7 +647,6 @@ export async function runDownloadAttachmentJobInner({
       `${logId}: Text attachment was ${sizeInKib}kib, max is ${maxTextAttachmentSizeInKib}kib`
     );
   }
-  const hasMediaBackups = window.Signal.Services.backups.hasMediaBackups();
   const mightBeInRemoteBackup = shouldAttachmentEndUpInRemoteBackup({
     attachment,
     hasMediaBackups,
diff --git a/ts/test-electron/services/AttachmentDownloadManager_test.ts b/ts/test-electron/services/AttachmentDownloadManager_test.ts
index 83950b7898..cf94aefc80 100644
--- a/ts/test-electron/services/AttachmentDownloadManager_test.ts
+++ b/ts/test-electron/services/AttachmentDownloadManager_test.ts
@@ -19,7 +19,7 @@ import {
   AttachmentDownloadUrgency,
 } from '../../types/AttachmentDownload';
 import { DataReader, DataWriter } from '../../sql/Client';
-import { MINUTE } from '../../util/durations';
+import { DAY, MINUTE, MONTH } from '../../util/durations';
 import { type AttachmentType, AttachmentVariant } from '../../types/Attachment';
 import { strictAssert } from '../../util/assert';
 import type { downloadAttachment as downloadAttachmentUtil } from '../../util/downloadAttachment';
@@ -78,6 +78,7 @@ describe('AttachmentDownloadManager/JobManager', () => {
   let runJob: sinon.SinonStub;
   let sandbox: sinon.SinonSandbox;
   let clock: sinon.SinonFakeTimers;
+  let hasMediaBackups: sinon.SinonStub;
   let isInCall: sinon.SinonStub;
   let onLowDiskSpaceBackupImport: sinon.SinonStub;
   let statfs: sinon.SinonStub;
@@ -89,6 +90,7 @@ describe('AttachmentDownloadManager/JobManager', () => {
     sandbox = sinon.createSandbox();
     clock = sandbox.useFakeTimers();
 
+    hasMediaBackups = sandbox.stub().returns(true);
     isInCall = sandbox.stub().returns(false);
     onLowDiskSpaceBackupImport = sandbox
       .stub()
@@ -123,6 +125,8 @@ describe('AttachmentDownloadManager/JobManager', () => {
         },
       }),
       onLowDiskSpaceBackupImport,
+      hasMediaBackups,
+      getMessageQueueTime: () => 45 * DAY,
       statfs,
     });
   });
@@ -162,7 +166,8 @@ describe('AttachmentDownloadManager/JobManager', () => {
     num: number,
     jobOverrides?:
       | Partial<AttachmentDownloadJobType>
-      | ((idx: number) => Partial<AttachmentDownloadJobType>)
+      | ((idx: number) => Partial<AttachmentDownloadJobType>),
+    attachmentOverrides?: Partial<AttachmentType>
   ): Promise<Array<AttachmentDownloadJobType>> {
     const jobs = new Array(num).fill(null).map((_, idx) =>
       composeJob({
@@ -170,6 +175,7 @@ describe('AttachmentDownloadManager/JobManager', () => {
         receivedAt: idx,
         jobOverrides:
           typeof jobOverrides === 'function' ? jobOverrides(idx) : jobOverrides,
+        attachmentOverrides,
       })
     );
     for (const job of jobs) {
@@ -496,6 +502,73 @@ describe('AttachmentDownloadManager/JobManager', () => {
       jobs[2],
     ]);
   });
+  describe('will drop jobs from non-media backup imports that are old', () => {
+    it('will not queue attachments older than 90 days (2 * message queue time)', async () => {
+      hasMediaBackups.returns(false);
+      await addJobs(
+        1,
+        {
+          source: AttachmentDownloadSource.BACKUP_IMPORT,
+        },
+        { uploadTimestamp: Date.now() - 4 * MONTH }
+      );
+
+      const savedJobs = await DataWriter.getNextAttachmentDownloadJobs({
+        limit: 100,
+      });
+      assert.strictEqual(savedJobs.length, 0);
+    });
+    it('will queue old attachments with media backups on', async () => {
+      hasMediaBackups.returns(true);
+      await addJobs(
+        1,
+        {
+          source: AttachmentDownloadSource.BACKUP_IMPORT,
+        },
+        { uploadTimestamp: Date.now() - 4 * MONTH }
+      );
+
+      const savedJobs = await DataWriter.getNextAttachmentDownloadJobs({
+        limit: 100,
+      });
+      assert.strictEqual(savedJobs.length, 1);
+    });
+    it('will queue old local backup attachments', async () => {
+      hasMediaBackups.returns(false);
+      await addJobs(
+        1,
+        {
+          source: AttachmentDownloadSource.BACKUP_IMPORT,
+        },
+        {
+          uploadTimestamp: Date.now() - 4 * MONTH,
+          localBackupPath: 'localBackupPath',
+          localKey: toBase64(generateAttachmentKeys()),
+        }
+      );
+
+      const savedJobs = await DataWriter.getNextAttachmentDownloadJobs({
+        limit: 100,
+      });
+      assert.strictEqual(savedJobs.length, 1);
+    });
+    it('will fallback to sentAt if uploadTimestamp is falsy', async () => {
+      hasMediaBackups.returns(false);
+      await addJobs(
+        1,
+        {
+          source: AttachmentDownloadSource.BACKUP_IMPORT,
+          sentAt: Date.now() - 4 * MONTH,
+        },
+        { uploadTimestamp: 0 }
+      );
+
+      const savedJobs = await DataWriter.getNextAttachmentDownloadJobs({
+        limit: 100,
+      });
+      assert.strictEqual(savedJobs.length, 0);
+    });
+  });
 });
 
 describe('AttachmentDownloadManager/runDownloadAttachmentJob', () => {
@@ -521,9 +594,6 @@ describe('AttachmentDownloadManager/runDownloadAttachmentJob', () => {
     downloadAttachment = sandbox
       .stub()
       .returns(Promise.resolve(downloadedAttachment));
-    sandbox
-      .stub(window.Signal.Services.backups, 'hasMediaBackups')
-      .returns(true);
 
     processNewAttachment = sandbox.stub().callsFake(attachment => attachment);
   });
@@ -544,6 +614,7 @@ describe('AttachmentDownloadManager/runDownloadAttachmentJob', () => {
       const result = await runDownloadAttachmentJobInner({
         job,
         isForCurrentlyVisibleMessage: true,
+        hasMediaBackups: true,
         abortSignal: abortController.signal,
         maxAttachmentSizeInKib: 100 * MEBIBYTE,
         maxTextAttachmentSizeInKib: 2 * MEBIBYTE,
@@ -573,6 +644,7 @@ describe('AttachmentDownloadManager/runDownloadAttachmentJob', () => {
       const result = await runDownloadAttachmentJobInner({
         job,
         isForCurrentlyVisibleMessage: true,
+        hasMediaBackups: true,
         abortSignal: abortController.signal,
         maxAttachmentSizeInKib: 100 * MEBIBYTE,
         maxTextAttachmentSizeInKib: 2 * MEBIBYTE,
@@ -620,6 +692,7 @@ describe('AttachmentDownloadManager/runDownloadAttachmentJob', () => {
       const result = await runDownloadAttachmentJobInner({
         job,
         isForCurrentlyVisibleMessage: true,
+        hasMediaBackups: true,
         abortSignal: abortController.signal,
         maxAttachmentSizeInKib: 100 * MEBIBYTE,
         maxTextAttachmentSizeInKib: 2 * MEBIBYTE,
@@ -654,6 +727,7 @@ describe('AttachmentDownloadManager/runDownloadAttachmentJob', () => {
         runDownloadAttachmentJobInner({
           job,
           isForCurrentlyVisibleMessage: true,
+          hasMediaBackups: true,
           abortSignal: abortController.signal,
           maxAttachmentSizeInKib: 100 * MEBIBYTE,
           maxTextAttachmentSizeInKib: 2 * MEBIBYTE,
@@ -692,6 +766,7 @@ describe('AttachmentDownloadManager/runDownloadAttachmentJob', () => {
       const result = await runDownloadAttachmentJobInner({
         job,
         isForCurrentlyVisibleMessage: false,
+        hasMediaBackups: true,
         abortSignal: abortController.signal,
         maxAttachmentSizeInKib: 100 * MEBIBYTE,
         maxTextAttachmentSizeInKib: 2 * MEBIBYTE,
@@ -727,6 +802,7 @@ describe('AttachmentDownloadManager/runDownloadAttachmentJob', () => {
       const result = await runDownloadAttachmentJobInner({
         job,
         isForCurrentlyVisibleMessage: false,
+        hasMediaBackups: true,
         abortSignal: abortController.signal,
         maxAttachmentSizeInKib: 100 * MEBIBYTE,
         maxTextAttachmentSizeInKib: 2 * MEBIBYTE,
@@ -781,6 +857,7 @@ describe('AttachmentDownloadManager/runDownloadAttachmentJob', () => {
         runDownloadAttachmentJobInner({
           job,
           isForCurrentlyVisibleMessage: false,
+          hasMediaBackups: true,
           abortSignal: abortController.signal,
           maxAttachmentSizeInKib: 100 * MEBIBYTE,
           maxTextAttachmentSizeInKib: 2 * MEBIBYTE,