Make ensureAttachmentIsReencryptable migration resilient to missing attachments

Co-authored-by: trevor-signal <131492920+trevor-signal@users.noreply.github.com>

ts/signal.ts

@@ -79,6 +79,9 @@ type MigrationsModuleType = {
   deleteSticker: (path: string) => Promise<void>;
   deleteTempFile: (path: string) => Promise<void>;
   doesAttachmentExist: (path: string) => Promise<boolean>;
+  ensureAttachmentIsReencryptable: (
+    attachment: TypesAttachment.LocallySavedAttachment
+  ) => Promise<TypesAttachment.ReencryptableAttachment>;
   getAbsoluteAttachmentPath: (path: string) => string;
   getAbsoluteAvatarPath: (src: string) => string;
   getAbsoluteBadgeImageFilePath: (path: string) => string;
@@ -161,6 +164,7 @@ export function initializeMigrations({
     createPlaintextReader,
     createWriterForNew,
     createDoesExist,
+    ensureAttachmentIsReencryptable,
     getAvatarsPath,
     getDraftPath,
     getDownloadsPath,
@@ -291,6 +295,7 @@ export function initializeMigrations({
     deleteSticker,
     deleteTempFile,
     doesAttachmentExist,
+    ensureAttachmentIsReencryptable,
     getAbsoluteAttachmentPath,
     getAbsoluteAvatarPath,
     getAbsoluteBadgeImageFilePath,
@@ -313,6 +318,7 @@ export function initializeMigrations({
     processNewAttachment: (attachment: AttachmentType) =>
       MessageType.processNewAttachment(attachment, {
         writeNewAttachmentData,
+        ensureAttachmentIsReencryptable,
         makeObjectUrl,
         revokeObjectUrl,
         getImageDimensions,
@@ -341,6 +347,8 @@ export function initializeMigrations({
 
       return MessageType.upgradeSchema(message, {
         deleteOnDisk,
+        doesAttachmentExist,
+        ensureAttachmentIsReencryptable,
         getImageDimensions,
         getRegionCode,
         makeImageThumbnail,
@@ -350,7 +358,6 @@ export function initializeMigrations({
         revokeObjectUrl,
         writeNewAttachmentData,
         writeNewStickerData,
-
         logger,
         maxVersion,
       });
@@ -404,6 +411,9 @@ type AttachmentsModuleType = {
     name: string;
   }) => Promise<null | { fullPath: string; name: string }>;
 
+  ensureAttachmentIsReencryptable: (
+    attachment: TypesAttachment.LocallySavedAttachment
+  ) => Promise<TypesAttachment.ReencryptableAttachment>;
   readAndDecryptDataFromDisk: (options: {
     absolutePath: string;
     keysBase64: string;

ts/test-node/types/Message2_test.ts

@@ -61,6 +61,10 @@ describe('Message', () => {
         width: 10,
         height: 20,
       }),
+      doesAttachmentExist: async () => true,
+      // @ts-expect-error ensureAttachmentIsReencryptable has type guards that we don't
+      // implement here
+      ensureAttachmentIsReencryptable: async attachment => attachment,
       getRegionCode: () => 'region-code',
       logger,
       makeImageThumbnail: async (_params: {
@@ -837,4 +841,56 @@ describe('Message', () => {
       assert.deepEqual(result, message);
     });
   });
+
+  describe('toVersion14: ensureAttachmentsAreReencryptable', () => {
+    it('migrates message if the file does not exist', async () => {
+      const message = getDefaultMessage({
+        schemaVersion: 13,
+        schemaMigrationAttempts: 0,
+        attachments: [
+          {
+            size: 128,
+            contentType: MIME.IMAGE_BMP,
+            path: 'no/file/here.png',
+            iv: 'iv',
+            digest: 'digest',
+            key: 'key',
+          },
+        ],
+        contact: [],
+      });
+      const result = await Message.upgradeSchema(message, {
+        ...getDefaultContext(),
+        doesAttachmentExist: async () => false,
+      });
+
+      assert.deepEqual({ ...message, schemaVersion: 14 }, result);
+    });
+    it('if file does exist, but migration errors, does not increment version', async () => {
+      const message = getDefaultMessage({
+        schemaVersion: 13,
+        schemaMigrationAttempts: 0,
+        attachments: [
+          {
+            size: 128,
+            contentType: MIME.IMAGE_BMP,
+            path: 'no/file/here.png',
+            iv: 'iv',
+            digest: 'digest',
+            key: 'key',
+          },
+        ],
+        contact: [],
+      });
+      const result = await Message.upgradeSchema(message, {
+        ...getDefaultContext(),
+        doesAttachmentExist: async () => true,
+        ensureAttachmentIsReencryptable: async () => {
+          throw new Error("Can't reencrypt!");
+        },
+      });
+
+      assert.deepEqual(message, result);
+    });
+  });
 });

ts/types/Message2.ts

@@ -10,6 +10,8 @@ import type {
   AttachmentType,
   AttachmentWithHydratedData,
   LocalAttachmentV2Type,
+  LocallySavedAttachment,
+  ReencryptableAttachment,
 } from './Attachment';
 import {
   captureDimensionsAndScreenshot,
@@ -49,12 +51,16 @@ import { encryptLegacyAttachment } from '../util/encryptLegacyAttachment';
 import { deepClone } from '../util/deepClone';
 import { LONG_ATTACHMENT_LIMIT } from './Message';
 import * as Bytes from '../Bytes';
-import { ensureAttachmentIsReencryptable } from '../util/ensureAttachmentIsReencryptable';
+import { redactGenericText } from '../util/privacy';
 
 export const GROUP = 'group';
 export const PRIVATE = 'private';
 
 export type ContextType = {
+  doesAttachmentExist: (relativePath: string) => Promise<boolean>;
+  ensureAttachmentIsReencryptable: (
+    attachment: LocallySavedAttachment
+  ) => Promise<ReencryptableAttachment>;
   getImageDimensions: (params: {
     objectUrl: string;
     logger: LoggerType;
@@ -635,17 +641,33 @@ const toVersion13 = _withSchemaVersion({
 
 const toVersion14 = _withSchemaVersion({
   schemaVersion: 14,
-  upgrade: _mapAllAttachments(async attachment => {
+  upgrade: _mapAllAttachments(
-    if (!isAttachmentLocallySaved(attachment)) {
+    async (
-      return attachment;
+      attachment,
+      { logger, ensureAttachmentIsReencryptable, doesAttachmentExist }
+    ) => {
+      const logId = `Message2.toVersion14(digest=${redactGenericText(attachment.digest ?? '')})`;
+
+      if (!isAttachmentLocallySaved(attachment)) {
+        return attachment;
+      }
+
+      if (!(await doesAttachmentExist(attachment.path))) {
+        // Attachments may be missing, e.g. for quote thumbnails that reference messages
+        // which have been deleted
+        logger.info(`${logId}: File does not exist`);
+        return attachment;
+      }
+
+      if (!attachment.digest) {
+        // Messages that are being upgraded prior to being sent may not have encrypted the
+        // attachment yet
+        return attachment;
+      }
+
+      return ensureAttachmentIsReencryptable(attachment);
     }
-    if (!attachment.digest) {
+  ),
-      // this attachment has not been encrypted yet; this would be expected for messages
-      // that are being upgraded prior to being sent
-      return attachment;
-    }
-    return ensureAttachmentIsReencryptable(attachment);
-  }),
 });
 
 const VERSIONS = [
@@ -677,6 +699,8 @@ export const upgradeSchema = async (
   {
     readAttachmentData,
     writeNewAttachmentData,
+    doesAttachmentExist,
+    ensureAttachmentIsReencryptable,
     getRegionCode,
     makeObjectUrl,
     revokeObjectUrl,
@@ -738,6 +762,8 @@ export const upgradeSchema = async (
       writeNewAttachmentData,
       makeObjectUrl,
       revokeObjectUrl,
+      doesAttachmentExist,
+      ensureAttachmentIsReencryptable,
       getImageDimensions,
       makeImageThumbnail,
       makeVideoScreenshot,
@@ -756,6 +782,7 @@ export const upgradeSchema = async (
 export const processNewAttachment = async (
   attachment: AttachmentType,
   {
+    ensureAttachmentIsReencryptable,
     writeNewAttachmentData,
     makeObjectUrl,
     revokeObjectUrl,
@@ -773,6 +800,7 @@ export const processNewAttachment = async (
     | 'makeVideoScreenshot'
     | 'logger'
     | 'deleteOnDisk'
+    | 'ensureAttachmentIsReencryptable'
   >
 ): Promise<AttachmentType> => {
   if (!isFunction(writeNewAttachmentData)) {

ts/util/ensureAttachmentIsReencryptable.ts

@@ -4,6 +4,7 @@
 import { PassThrough } from 'stream';
 import {
   type EncryptedAttachmentV2,
+  ReencryptedDigestMismatchError,
   type ReencryptionInfo,
   decryptAttachmentV2ToSink,
   encryptAttachmentV2,
@@ -20,6 +21,8 @@ import {
 import { strictAssert } from './assert';
 import * as logging from '../logging/log';
 import { fromBase64, toBase64 } from '../Bytes';
+import { redactGenericText } from './privacy';
+import { toLogFormat } from '../types/errors';
 
 /**
  * Some attachments on desktop are not reencryptable to the digest we received for them.
@@ -34,6 +37,7 @@ import { fromBase64, toBase64 } from '../Bytes';
 export async function ensureAttachmentIsReencryptable(
   attachment: LocallySavedAttachment
 ): Promise<ReencryptableAttachment> {
+  const logId = `ensureAttachmentIsReencryptable(digest=${redactGenericText(attachment.digest ?? '')})`;
   if (isReencryptableToSameDigest(attachment)) {
     return attachment;
   }
@@ -50,9 +54,12 @@ export async function ensureAttachmentIsReencryptable(
         isReencryptableToSameDigest: true,
       };
     } catch (e) {
-      logging.info(
+      if (e instanceof ReencryptedDigestMismatchError) {
-        'Unable to reencrypt attachment to original digest; must have had non-zero padding'
+        logging.info(
-      );
+          `${logId}: Unable to reencrypt attachment to original digest; must have had non-zero padding`
+        );
+      }
+      logging.error(`${logId}: error when reencrypting`, toLogFormat(e));
     }
   }
 

ts/windows/attachments.ts

@@ -12,6 +12,7 @@ import { writeWindowsZoneIdentifier } from '../util/windowsZoneIdentifier';
 import OS from '../util/os/osMain';
 import { getRelativePath, createName } from '../util/attachmentPath';
 
+export * from '../util/ensureAttachmentIsReencryptable';
 export * from '../../app/attachments';
 
 type FSAttrType = {