fix(mail-ingest): read on to To when a delivery header names no address

Stalwart writes Delivered-To as the mailbox itself, its sub-address dropped,
so a forwarded mail to trek+<token>@ was refused as unknown_address: the
first delivery header won even though it carried no token. The headers are
now read in their order of trust, and the first that names a live address
decides.

server/src/nest/mail-ingest/mail-ingest-intake.service.ts

@@ -59,8 +59,8 @@ export class MailIngestIntakeService {
   /**
    * A mail a transport received, stored once for every live address it was
    * sent to. `envelopeRecipients` are the RCPT TO addresses where the
-   * transport knows them, and then the only ones read; otherwise the delivery
-   * header the receiving server put on top names the address.
+   * transport knows them, and then the only ones read; otherwise the headers
+   * name it (see headerAddresses).
    *
    * When one of its addresses refuses it for now, the whole mail answers that
    * refusal: the transport offers it again, and the copies stored already
@@ -77,8 +77,10 @@ export class MailIngestIntakeService {
     if (refused) return { status: 'refused', reason: refused };
     const mail = this.reader.walk(raw);
     if (!mail) return { status: 'refused', reason: 'not_a_mail' };
-    const recipients = envelopeRecipients.length > 0 ? envelopeRecipients : headerRecipients(mail.envelope);
-    const addresses = await this.addresses.resolveAll(recipients);
+    const addresses =
+      envelopeRecipients.length > 0
+        ? await this.addresses.resolveAll(envelopeRecipients)
+        : await this.headerAddresses(mail.envelope);
     if (addresses.length === 0) return { status: 'refused', reason: 'unknown_address' };
     const results = await addresses.reduce<Promise<MailIntakeResult[]>>(async (previous, address) => {
       const done = await previous;
@@ -101,6 +103,18 @@ export class MailIngestIntakeService {
     return this.store(raw, 'upload', address, mail.envelope.messageId);
   }
 
+  /**
+   * The live addresses a mail's headers name, read in the order they are
+   * trusted: Delivered-To and X-Original-To, which the receiving server
+   * writes, then To. The first that names one decides. A delivery header
+   * without a token names none: Stalwart writes the box itself there, its
+   * sub-address dropped, and the token then stands only in To.
+   */
+  private async headerAddresses(envelope: WalkedMail['envelope']): Promise<MailIngestAddressRow[]> {
+    const named = await Promise.all(headerRecipients(envelope).map((list) => this.addresses.resolveAll(list)));
+    return named.find((addresses) => addresses.length > 0) ?? [];
+  }
+
   private async precheck(raw: Buffer): Promise<MailIntakeRefusal | null> {
     if (!(await this.settings.isAvailable())) return 'unavailable';
     if (raw.length > this.settings.maxMailBytes()) return 'too_large';
@@ -155,10 +169,11 @@ export class MailIngestIntakeService {
  * the To line. A sender who names another token there gains nothing over
  * sending to that token directly.
  */
-function headerRecipients(envelope: WalkedMail['envelope']): string[] {
-  const delivered = [envelope.deliveredTo, envelope.xOriginalTo].find((value) => !!value?.trim());
-  if (delivered) return [delivered];
-  return (envelope.to ?? '').split(',').filter((value) => value.trim() !== '');
+/** The recipients each header names, most trusted first: Delivered-To, X-Original-To, To. */
+function headerRecipients(envelope: WalkedMail['envelope']): string[][] {
+  return [[envelope.deliveredTo ?? ''], [envelope.xOriginalTo ?? ''], (envelope.to ?? '').split(',')]
+    .map((list) => list.filter((value) => value.trim() !== ''))
+    .filter((list) => list.length > 0);
 }
 
 /** One answer for a mail stored per address: a passing refusal first, then accepted, then duplicate. */

server/tests/unit/nest/mail-ingest/mail-ingest.addresses.test.ts

@@ -138,6 +138,25 @@ describe('a mail to several addresses', () => {
     expect(await findRow(orm(), MailIngestMessages, { user: mia.id })).not.toBeNull();
     expect(await findRow(orm(), MailIngestMessages, { user: noah.id })).toBeNull();
   });
+
+  it('reads on to To when the delivery header names the box without its token, as Stalwart writes it', async () => {
+    const { user: mia } = await makeUser(orm());
+    const miaOwn = (await addresses().renewOwn(mia.id))?.address ?? '';
+    const box = miaOwn.replace(/\+[^@]+@/, '@');
+    const headers = [`Delivered-To: ${box}`, `To: <${miaOwn}>`, 'From: x@example.com'];
+    const raw = Buffer.from(
+      `${headers.join('\r\n')}\r\nSubject: WG: Booking\r\nMessage-ID: <fwd@x>\r\n\r\nBooking ABC123`,
+    );
+
+    expect(await t.get(MailIngestIntakeService).receive(raw, 'imap')).toEqual({ status: 'accepted' });
+    expect(await findRow(orm(), MailIngestMessages, { user: mia.id })).not.toBeNull();
+
+    const nobody = Buffer.from(`Delivered-To: ${box}\r\nTo: ${box}\r\nSubject: Hi\r\nMessage-ID: <none@x>\r\n\r\nHi`);
+    expect(await t.get(MailIngestIntakeService).receive(nobody, 'imap')).toEqual({
+      status: 'refused',
+      reason: 'unknown_address',
+    });
+  });
 });
 
 describe('a trip address', () => {

wiki/Booking-Mail-Import.md

@@ -67,7 +67,7 @@ Each person's token is 24 random characters.
 
 TREK reads one mailbox every two minutes. Point a catch-all, or a mailbox with sub-addressing (`trek+anything@`), at it and use the matching address template. Read mails move to the folder `TREK/processed` and are never deleted, so anyone with access to the mailbox can read them. A mail TREK cannot take for now, because its address took its 50 mails of the day or its person has 200 bookings waiting, moves to `TREK/deferred` and is offered again on every run for three days; after that it moves to `TREK/processed` as well.
 
-TREK files a mail under the address in the topmost `Delivered-To` header, else the topmost `X-Original-To`, which the receiving server adds; only without either does it read `To`. A mail addressed to several people's addresses reaches each of them.
+TREK files a mail under the address in the topmost `Delivered-To` header, else the topmost `X-Original-To`, which the receiving server adds, else `To`. A header that names the mailbox without its `+` part (Stalwart writes `Delivered-To` that way) names no address, so TREK reads on to the next. A mail addressed to several people's addresses reaches each of them.
 
 | Field | Default |
 |---|---|
@@ -88,7 +88,7 @@ Each request carries three headers:
 | Header | Value |
 |---|---|
 | `X-Trek-Timestamp` | The time of sending, in Unix seconds. Requests more than five minutes off are refused. |
-| `X-Trek-Recipient` | The address the mail was sent to (the envelope recipient). Optional; without it TREK reads the topmost `Delivered-To`, else `X-Original-To`, else `To`. |
+| `X-Trek-Recipient` | The address the mail was sent to (the envelope recipient). Optional; without it TREK reads the topmost `Delivered-To`, else `X-Original-To`, else `To`, skipping a header that names no TREK address. |
 | `X-Trek-Signature` | `sha256=` followed by the hex HMAC-SHA256 of `<timestamp>\n<recipient>\n<raw mail>`, keyed with the secret. |
 
 A Cloudflare Email Worker that forwards a mail: