onSentMessage: Create destination conversation before further processing

Co-authored-by: Scott Nonnenberg <scott@signal.org>
Co-authored-by: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com>

ts/background.ts

@@ -138,7 +138,11 @@ import {
 import { themeChanged } from './shims/themeChanged';
 import { createIPCEvents } from './util/createIPCEvents';
 import type { ServiceIdString } from './types/ServiceId';
-import { ServiceIdKind, isServiceIdString } from './types/ServiceId';
+import {
+  ServiceIdKind,
+  isPniString,
+  isServiceIdString,
+} from './types/ServiceId';
 import { isAciString } from './util/isAciString';
 import { normalizeAci } from './util/normalizeAci';
 import * as log from './logging/log';
@@ -2727,6 +2731,25 @@ export async function startApp(): Promise<void> {
     const sourceServiceId = window.textsecure.storage.user.getAci();
     strictAssert(source && sourceServiceId, 'Missing user number and uuid');
 
+    // Make sure destination conversation is created before we hit getMessageDescriptor
+    if (data.destinationServiceId !== sourceServiceId) {
+      const { mergePromises } =
+        window.ConversationController.maybeMergeContacts({
+          e164: data.destination,
+          aci: isAciString(data.destinationServiceId)
+            ? data.destinationServiceId
+            : undefined,
+          pni: isPniString(data.destinationServiceId)
+            ? data.destinationServiceId
+            : undefined,
+          reason: `onSentMessage(${data.timestamp})`,
+        });
+
+      if (mergePromises.length > 0) {
+        await Promise.all(mergePromises);
+      }
+    }
+
     const messageDescriptor = getMessageDescriptor({
       ...data,
     });

ts/services/storage.ts

@@ -67,6 +67,7 @@ import type {
 import { MY_STORY_ID } from '../types/Stories';
 import { isNotNil } from '../util/isNotNil';
 import { isSignalConversation } from '../util/isSignalConversation';
+import { redactExtendedStorageID, redactStorageID } from '../util/privacy';
 
 type IManifestRecordIdentifier = Proto.ManifestRecord.IIdentifier;
 
@@ -104,22 +105,6 @@ const conflictBackOff = new BackOff([
   30 * durations.SECOND,
 ]);
 
-function redactStorageID(
-  storageID: string,
-  version?: number,
-  conversation?: ConversationModel
-): string {
-  const convoId = conversation ? ` ${conversation?.idForLogging()}` : '';
-  return `${version ?? '?'}:${storageID.substring(0, 3)}${convoId}`;
-}
-
-function redactExtendedStorageID({
-  storageID,
-  storageVersion,
-}: ExtendedStorageID): string {
-  return redactStorageID(storageID, storageVersion);
-}
-
 function encryptRecord(
   storageID: string | undefined,
   storageRecord: Proto.IStorageRecord
@@ -942,6 +927,10 @@ async function mergeRecord(
   itemToMerge: MergeableItemType
 ): Promise<MergedRecordType> {
   const { itemType, storageID, storageRecord } = itemToMerge;
+  const redactedStorageID = redactExtendedStorageID({
+    storageID,
+    storageVersion,
+  });
 
   const ITEM_TYPE = Proto.ManifestRecord.Identifier.Type;
 
@@ -953,7 +942,10 @@ async function mergeRecord(
 
   try {
     if (itemType === ITEM_TYPE.UNKNOWN) {
-      log.warn('storageService.mergeRecord: Unknown item type', storageID);
+      log.warn(
+        'storageService.mergeRecord: Unknown item type',
+        redactedStorageID
+      );
     } else if (itemType === ITEM_TYPE.CONTACT && storageRecord.contact) {
       mergeResult = await mergeContactRecord(
         storageID,
@@ -999,10 +991,7 @@ async function mergeRecord(
     } else {
       isUnsupported = true;
       log.warn(
-        `storageService.merge(${redactStorageID(
-          storageID,
-          storageVersion
-        )}): unknown item type=${itemType}`
+        `storageService.merge(${redactedStorageID}): unknown item type=${itemType}`
       );
     }
 
@@ -1430,9 +1419,13 @@ async function fetchRemoteRecords(
 
       const remoteRecord = remoteOnlyRecords.get(base64ItemID);
       if (!remoteRecord) {
+        const redactedStorageID = redactExtendedStorageID({
+          storageID: base64ItemID,
+          storageVersion,
+        });
         throw new Error(
           "Got a remote record that wasn't requested with " +
-            `storageID: ${base64ItemID}`
+            `storageID: ${redactedStorageID}`
         );
       }
 

ts/services/storageRecordOps.ts

@@ -64,6 +64,7 @@ import { MY_STORY_ID, StorySendMode } from '../types/Stories';
 import { findAndDeleteOnboardingStoryIfExists } from '../util/findAndDeleteOnboardingStoryIfExists';
 import { downloadOnboardingStory } from '../util/downloadOnboardingStory';
 import { drop } from '../util/drop';
+import { redactExtendedStorageID } from '../util/privacy';
 
 const MY_STORY_BYTES = uuidToBytes(MY_STORY_ID);
 
@@ -694,8 +695,12 @@ export async function mergeGroupV1Record(
   storageVersion: number,
   groupV1Record: Proto.IGroupV1Record
 ): Promise<MergeResultType> {
+  const redactedStorageID = redactExtendedStorageID({
+    storageID,
+    storageVersion,
+  });
   if (!groupV1Record.id) {
-    throw new Error(`No ID for ${storageID}`);
+    throw new Error(`No ID for ${redactedStorageID}`);
   }
 
   const groupId = Bytes.toBinary(groupV1Record.id);
@@ -861,8 +866,12 @@ export async function mergeGroupV2Record(
   storageVersion: number,
   groupV2Record: Proto.IGroupV2Record
 ): Promise<MergeResultType> {
+  const redactedStorageID = redactExtendedStorageID({
+    storageID,
+    storageVersion,
+  });
   if (!groupV2Record.masterKey) {
-    throw new Error(`No master key for ${storageID}`);
+    throw new Error(`No master key for ${redactedStorageID}`);
   }
 
   const masterKeyBuffer = groupV2Record.masterKey;
@@ -1013,9 +1022,16 @@ export async function mergeContactRecord(
 
   // We're going to ignore this; it's likely a PNI-only contact we've already merged
   if (conversation.getServiceId() !== serviceId) {
+    const previousStorageID = conversation.get('storageID');
+    const redactedpreviousStorageID = previousStorageID
+      ? redactExtendedStorageID({
+          storageID: previousStorageID,
+          storageVersion: conversation.get('storageVersion'),
+        })
+      : '<none>';
     log.warn(
       `mergeContactRecord: ${conversation.idForLogging()} ` +
-        `with storageId ${conversation.get('storageID')} ` +
+        `with storageId ${redactedpreviousStorageID} ` +
         `had serviceId that didn't match provided serviceId ${serviceId}`
     );
     return {
@@ -1548,8 +1564,14 @@ export async function mergeStoryDistributionListRecord(
   storageVersion: number,
   storyDistributionListRecord: Proto.IStoryDistributionListRecord
 ): Promise<MergeResultType> {
+  const redactedStorageID = redactExtendedStorageID({
+    storageID,
+    storageVersion,
+  });
   if (!storyDistributionListRecord.identifier) {
-    throw new Error(`No storyDistributionList identifier for ${storageID}`);
+    throw new Error(
+      `No storyDistributionList identifier for ${redactedStorageID}`
+    );
   }
 
   const details: Array<string> = [];
@@ -1683,8 +1705,12 @@ export async function mergeStickerPackRecord(
   storageVersion: number,
   stickerPackRecord: Proto.IStickerPackRecord
 ): Promise<MergeResultType> {
+  const redactedStorageID = redactExtendedStorageID({
+    storageID,
+    storageVersion,
+  });
   if (!stickerPackRecord.packId || Bytes.isEmpty(stickerPackRecord.packId)) {
-    throw new Error(`No stickerPackRecord identifier for ${storageID}`);
+    throw new Error(`No stickerPackRecord identifier for ${redactedStorageID}`);
   }
 
   const details: Array<string> = [];
@@ -1714,7 +1740,7 @@ export async function mergeStickerPackRecord(
       !stickerPackRecord.packKey ||
       Bytes.isEmpty(stickerPackRecord.packKey)
     ) {
-      throw new Error(`No stickerPackRecord key for ${storageID}`);
+      throw new Error(`No stickerPackRecord key for ${redactedStorageID}`);
     }
 
     stickerPack = {

ts/test-mock/messaging/sendSync_test.ts

@@ -0,0 +1,94 @@
+// Copyright 2023 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import { assert } from 'chai';
+import createDebug from 'debug';
+import Long from 'long';
+
+import type { App } from '../playwright';
+import * as durations from '../../util/durations';
+import { Bootstrap } from '../bootstrap';
+
+export const debug = createDebug('mock:test:sendSync');
+
+describe('sendSync', function (this: Mocha.Suite) {
+  this.timeout(durations.MINUTE);
+
+  let bootstrap: Bootstrap;
+  let app: App;
+
+  beforeEach(async () => {
+    bootstrap = new Bootstrap();
+    await bootstrap.init();
+    app = await bootstrap.link();
+  });
+
+  afterEach(async function (this: Mocha.Context) {
+    if (!bootstrap) {
+      return;
+    }
+
+    await bootstrap.maybeSaveLogs(this.currentTest, app);
+    await app.close();
+    await bootstrap.teardown();
+  });
+
+  it('creates conversation for sendSync to PNI', async () => {
+    const { desktop, phone, server } = bootstrap;
+
+    debug('Creating stranger');
+    const STRANGER_NAME = 'Mysterious Stranger';
+    const stranger = await server.createPrimaryDevice({
+      profileName: STRANGER_NAME,
+    });
+
+    const timestamp = Date.now();
+    const messageText = 'hey there, just reaching out';
+    const destinationServiceId = stranger.device.pni;
+    const destination = stranger.device.number;
+    const originalDataMessage = {
+      body: messageText,
+      timestamp: Long.fromNumber(timestamp),
+    };
+    const content = {
+      syncMessage: {
+        sent: {
+          destinationServiceId,
+          destination,
+          timestamp: Long.fromNumber(timestamp),
+          message: originalDataMessage,
+        },
+      },
+    };
+    const sendOptions = {
+      timestamp,
+    };
+    await phone.sendRaw(desktop, content, sendOptions);
+
+    const page = await app.getWindow();
+    const leftPane = page.locator('#LeftPane');
+
+    debug('checking left pane for conversation');
+    const strangerName = await leftPane
+      .locator(
+        '.module-conversation-list__item--contact-or-conversation .module-contact-name'
+      )
+      .first()
+      .innerText();
+
+    assert.equal(
+      strangerName.slice(-4),
+      destination?.slice(-4),
+      'no profile, just phone number'
+    );
+
+    debug('opening conversation');
+    await leftPane
+      .locator('.module-conversation-list__item--contact-or-conversation')
+      .first()
+      .click();
+
+    debug('checking for latest message');
+    await page.locator(`.module-message__text >> "${messageText}"`).waitFor();
+  });
+});

ts/util/privacy.ts

@@ -8,6 +8,9 @@ import path from 'path';
 import { compose } from 'lodash/fp';
 import { escapeRegExp, isString, isRegExp } from 'lodash';
 
+import type { ExtendedStorageID } from '../types/StorageService.d';
+import type { ConversationModel } from '../models/conversations';
+
 export const APP_ROOT_PATH = path.join(__dirname, '..', '..');
 
 const PHONE_NUMBER_PATTERN = /\+\d{7,12}(\d{3})/g;
@@ -23,6 +26,22 @@ const REDACTION_PLACEHOLDER = '[REDACTED]';
 
 export type RedactFunction = (value: string) => string;
 
+export function redactStorageID(
+  storageID: string,
+  version?: number,
+  conversation?: ConversationModel
+): string {
+  const convoId = conversation ? ` ${conversation?.idForLogging()}` : '';
+  return `${version ?? '?'}:${storageID.substring(0, 3)}${convoId}`;
+}
+
+export function redactExtendedStorageID({
+  storageID,
+  storageVersion,
+}: ExtendedStorageID): string {
+  return redactStorageID(storageID, storageVersion);
+}
+
 export const _redactPath = (filePath: string): RedactFunction => {
   if (!isString(filePath)) {
     throw new TypeError("'filePath' must be a string");