fix: skip raw on open, validate folders, decode cached subjects, map move conflicts

README.md

@@ -92,7 +92,7 @@ Paths are relative to where the host mounts the sub-app. Every route returns 403
 | POST   | `/me/inbox/:uid/unread`      | `mailbox:*` `manage` | Clear `\Seen` on a message in `?folder=` (INBOX by default).                                                   |
 | POST   | `/me/inbox/:uid/archive`     | `mailbox:*` `manage` | Move from INBOX to Archive.                                                                                    |
 | POST   | `/me/inbox/:uid/trash`       | `mailbox:*` `manage` | Move from INBOX to Trash.                                                                                      |
-| POST   | `/me/inbox/:uid/restore`     | `mailbox:*` `manage` | Move back to INBOX from `?folder=` (Archive by default).                                                       |
+| POST   | `/me/inbox/:uid/restore`     | `mailbox:*` `manage` | Move back to INBOX from `?folder=`: Archive (default) or Trash.                                                |
 
 The grant is `mailbox:*` rather than a per-message resource because every query is already scoped to the caller's own tenant and principal.
 

e2e/mount-native.test.ts

@@ -3,8 +3,15 @@
 import { beforeEach, describe, expect, test } from "bun:test";
 import { createMailboxRoutes } from "../src/mount.js";
 import { createInMemoryMailboxEventBus } from "../src/bus.js";
+import { sql } from "drizzle-orm";
 import { writeMailboxMessage } from "../src/write.js";
-import { allowAllGrants, mountAs, withTestDb, seedScope } from "./helpers.js";
+import {
+  allowAllGrants,
+  handle,
+  mountAs,
+  withTestDb,
+  seedScope,
+} from "./helpers.js";
 import type { MailboxDb } from "../src/db.js";
 
 let db: MailboxDb;
@@ -224,9 +231,115 @@ describe("archive/trash/restore", () => {
     expect(inbox.messages).toHaveLength(1);
   });
 
+  test("restore out of anything but Archive or Trash is a 400", async () => {
+    const uid = await seedMessage("Stay");
+    const app = buildApp();
+    for (const folder of ["INBOX", "Sent", "junk"]) {
+      const res = await app.request(
+        `/me/inbox/${uid}/restore?folder=${folder}`,
+        {
+          method: "POST",
+        },
+      );
+      expect(res.status).toBe(400);
+    }
+    expect(
+      (await app.request(`/me/inbox/${uid}/read`, { method: "POST" })).status,
+    ).toBe(200);
+  });
+
+  test("an unknown ?folder= on read and unread is a 400 that creates no mailbox", async () => {
+    const app = buildApp();
+    for (const verb of ["read", "unread"]) {
+      const res = await app.request(`/me/inbox/1/${verb}?folder=junk`, {
+        method: "POST",
+      });
+      expect(res.status).toBe(400);
+    }
+    const rows = await db.execute(
+      sql`SELECT 1 FROM "mailbox"."mailbox_state" WHERE "folder" = 'junk'`,
+    );
+    expect(rows).toHaveLength(0);
+  });
+
   test("trash on an unknown uid is a 404", async () => {
     const app = buildApp();
     const res = await app.request("/me/inbox/999/trash", { method: "POST" });
     expect(res.status).toBe(404);
   });
+
+  test("a move onto a Message-ID the destination holds is a 409 that burns no uid", async () => {
+    const message = {
+      ...SCOPE,
+      address: "p1@t1.example",
+      fromAddress: "a@t1.example",
+      subject: "Twice",
+      body: "Body",
+      messageId: "<twice@t1.example>",
+    };
+    await writeMailboxMessage(db, { ...message, folder: "Archive" });
+    const written = await writeMailboxMessage(db, message);
+    const state = () =>
+      db.execute<{ folder: string; uid_next: string }>(
+        sql`SELECT "folder", "uid_next" FROM "mailbox"."mailbox_state" ORDER BY "folder"`,
+      );
+    const before = await state();
+    const app = buildApp();
+    const res = await app.request(`/me/inbox/${written!.uid}/archive`, {
+      method: "POST",
+    });
+    expect(res.status).toBe(409);
+    expect(await state()).toEqual(before);
+  });
+
+  test("interleaved archive and restore moves never deadlock or lose rows", async () => {
+    for (let i = 0; i < 16; i++) {
+      await writeMailboxMessage(db, {
+        ...SCOPE,
+        folder: i % 2 === 0 ? "INBOX" : "Archive",
+        address: "p1@t1.example",
+        fromAddress: "a@t1.example",
+        subject: `m${i}`,
+        body: "Body",
+      });
+    }
+    const app = buildApp();
+    const move = async (path: string) =>
+      (await app.request(path, { method: "POST" })).status;
+    const moves: Promise<number>[] = [];
+    for (let uid = 1; uid <= 8; uid++) {
+      moves.push(
+        move(`/me/inbox/${uid}/archive`),
+        move(`/me/inbox/${uid}/restore?folder=Archive`),
+      );
+    }
+    const statuses = await Promise.all(moves);
+    for (const status of statuses) expect([200, 404, 409]).toContain(status);
+
+    const rows = await db.execute<{
+      subject: string;
+      folder: string;
+      uid: number;
+    }>(sql`SELECT "subject", "folder", "uid" FROM "mailbox"."principal_mail"`);
+    expect(rows).toHaveLength(16);
+    expect(new Set(rows.map((r) => r.subject)).size).toBe(16);
+    expect(new Set(rows.map((r) => `${r.folder}/${r.uid}`)).size).toBe(16);
+  });
+
+  test("a move while the database is unreachable is a 500, not a 404", async () => {
+    const { client, db: down } = handle();
+    await client.end();
+    const app = mountAs(
+      SCOPE,
+      createMailboxRoutes({
+        db: down,
+        requireGrant: allowAllGrants,
+        bus: createInMemoryMailboxEventBus(),
+        senderAddressFor: () => "p1@t1.example",
+        deliver: () => {},
+      }),
+    );
+    const res = await app.request("/me/inbox/1/archive", { method: "POST" });
+    expect(res.status).toBe(500);
+  });
 });

e2e/persist.test.ts

@@ -600,6 +600,24 @@ describe("frame size and recipient hard caps", () => {
   });
 });
 
+describe("cached columns", () => {
+  test("an RFC 2047 subject is cached decoded", async () => {
+    const persist = createMailboxPersist(db, {
+      upstream: async () => undefined,
+      authorizeSender: () => ACTIVE,
+    });
+    await persist(
+      args({
+        raw: new TextEncoder().encode(
+          `From: ${SENDER}\r\nSubject: =?UTF-8?B?w6nDqcOp?=\r\nMessage-ID: <enc@acme.example>\r\n\r\nhi\r\n`,
+        ),
+      }),
+    );
+    const [row] = await rowsFor("acme", "user-1");
+    expect(row?.subject).toBe("\u00e9\u00e9\u00e9");
+  });
+});
+
 describe("Message-ID dedupe", () => {
   test("keys on the authorized envelope sender, not the From header", async () => {
     const persist = createMailboxPersist(db, {

src/frame.test.ts

@@ -2,6 +2,7 @@ import { describe, expect, test } from "bun:test";
 import { generateMessageId, parseHeaderSection } from "@intx/mime";
 import {
   buildMailFrame,
+  decodeEncodedWords,
   decodeMailFrame,
   generateMailboxMessageId,
   MESSAGE_ID_FALLBACK_DOMAIN,
@@ -191,3 +192,35 @@ describe("Message-ID domain fallback", () => {
     );
   });
 });
+
+describe("decodeEncodedWords", () => {
+  test("decodes B and Q words and joins adjacent ones", () => {
+    expect(decodeEncodedWords("=?UTF-8?B?w6nDqcOp?=")).toBe(
+      "\u00e9\u00e9\u00e9",
+    );
+    expect(decodeEncodedWords("Re: =?ISO-8859-1?Q?caf=E9_bar?=")).toBe(
+      "Re: caf\u00e9 bar",
+    );
+    expect(decodeEncodedWords("=?UTF-8?Q?a?= =?UTF-8?Q?b?= c")).toBe("ab c");
+  });
+
+  test("leaves plain text and unknown charsets as written", () => {
+    expect(decodeEncodedWords("plain ?= text")).toBe("plain ?= text");
+    expect(decodeEncodedWords("=?x-nope?B?YQ==?=")).toBe("=?x-nope?B?YQ==?=");
+  });
+
+  test("leaves a word that does not decode as written", () => {
+    expect(decodeEncodedWords("=?UTF-8?B?!!!!?=")).toBe("=?UTF-8?B?!!!!?=");
+    expect(decodeEncodedWords("=?UTF-8?Q?=C3?= x")).toBe("=?UTF-8?Q?=C3?= x");
+    expect(decodeEncodedWords("=?UTF-8?B?!!!!?= =?UTF-8?Q?b?=")).toBe(
+      "=?UTF-8?B?!!!!?= b",
+    );
+    expect(decodeEncodedWords("=?UTF-8?Q?a?= =?UTF-8?B?!!!!?=")).toBe(
+      "a =?UTF-8?B?!!!!?=",
+    );
+  });
+
+  test("keeps whitespace after a literal ?= that is not an encoded-word", () => {
+    expect(decodeEncodedWords("plain?= =?UTF-8?Q?b?=")).toBe("plain?= b");
+  });
+});

src/frame.ts

@@ -17,6 +17,76 @@ export function headerValue(value: string): string {
     .trim();
 }
 
+// RFC 2047 encoded-word: =?charset?B|Q?text?=, with the whitespace that
+// follows it when another encoded-word comes next.
+const WORD = String.raw`=\?[^?\s]+\?[BbQq]\?[^?\s]*\?=`;
+const ENCODED_WORD = new RegExp(
+  String.raw`=\?([^?\s]+)\?([BbQq])\?([^?\s]*)\?=(\s+(?=${WORD}))?`,
+  "g",
+);
+const BASE64 = /^(?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2,3}={0,2})?$/;
+
+function decodeEncodedWord(charset: string, encoding: string, text: string) {
+  let bytes: Buffer;
+  if (encoding.toUpperCase() === "B") {
+    if (!BASE64.test(text)) throw new Error("invalid base64");
+    bytes = Buffer.from(text, "base64");
+  } else {
+    bytes = Buffer.from(
+      text
+        .replace(/_/g, " ")
+        .replace(/=([0-9A-Fa-f]{2})/g, (_, hex: string) =>
+          String.fromCharCode(parseInt(hex, 16)),
+        ),
+      "latin1",
+    );
+  }
+  return new TextDecoder(charset.split("*")[0], { fatal: true }).decode(bytes);
+}
+
+/**
+ * A header value with its RFC 2047 encoded-words decoded, for the cached
+ * columns. Whitespace between adjacent encoded-words is dropped, as §6.2
+ * requires. A word that does not decode (unknown charset, bad base64, bytes
+ * invalid in its charset) stays as written, with the whitespace around it.
+ */
+export function decodeEncodedWords(value: string): string {
+  return value.replace(
+    ENCODED_WORD,
+    (
+      word,
+      charset: string,
+      encoding: string,
+      text: string,
+      gap = "",
+      at: number,
+    ) => {
+      const decoded = tryDecodeEncodedWord(charset, encoding, text);
+      if (decoded === undefined) return word;
+      const next = new RegExp(ENCODED_WORD.source, "y");
+      next.lastIndex = at + word.length;
+      const following = next.exec(value);
+      const joins =
+        following !== null &&
+        tryDecodeEncodedWord(following[1]!, following[2]!, following[3]!) !==
+          undefined;
+      return joins ? decoded : decoded + gap;
+    },
+  );
+}
+
+function tryDecodeEncodedWord(
+  charset: string,
+  encoding: string,
+  text: string,
+): string | undefined {
+  try {
+    return decodeEncodedWord(charset, encoding, text);
+  } catch {
+    return undefined;
+  }
+}
+
 /**
  * The domain a minted Message-ID falls back to when the sender address carries
  * none. Not a cosmetic choice:

src/mount.ts

@@ -16,6 +16,7 @@ import {
 } from "./bus.js";
 import {
   openNativeMailboxStore,
+  MailboxMessageNotFoundError,
   moveNativeMailboxMessage,
 } from "./native-store.js";
 import {
@@ -91,6 +92,18 @@ function isListFolder(value: string): value is ListFolder {
   return (LIST_FOLDERS as readonly string[]).includes(value);
 }
 
+/** Postgres unique_violation, raw or wrapped by drizzle as the cause. */
+function isUniqueViolation(err: unknown): boolean {
+  const code = (e: unknown) => (e as { code?: unknown } | null)?.code;
+  return (
+    code(err) === "23505" ||
+    (err instanceof Error && code(err.cause) === "23505")
+  );
+}
+
+/** Folders `restore` may move a message out of. */
+const RESTORE_FOLDERS: readonly string[] = ["Archive", "Trash"];
+
 function parseLimit(
   raw: string | undefined,
 ): { limit: number } | { error: string } {
@@ -649,7 +662,7 @@ export function createMailboxRoutes(
         parameters: [ID_PARAM],
         responses: {
           200: { description: "The flag was applied" },
-          400: { description: "uid is not a positive integer" },
+          400: { description: "Bad uid or folder" },
           403: { description: "No resolvable principalId" },
           404: { description: "No message with that uid in this mailbox" },
         },
@@ -662,6 +675,9 @@ export function createMailboxRoutes(
         if (!resolved)
           return c.json({ error: "No resolvable principalId" }, 403);
         const folder = c.req.query("folder") ?? DEFAULT_FOLDER;
+        if (!isListFolder(folder)) {
+          return c.json({ error: "invalid folder" }, 400);
+        }
         const store = await openNativeMailboxStore(
           db,
           inFolder(resolved, folder),
@@ -687,9 +703,16 @@ export function createMailboxRoutes(
         parameters: [ID_PARAM],
         responses: {
           200: { description: "The message was moved" },
-          400: { description: "uid is not a positive integer" },
+          400: {
+            description:
+              "Bad uid, or a restore folder other than Archive or Trash",
+          },
           403: { description: "No resolvable principalId" },
           404: { description: "No message with that uid in the source folder" },
+          409: {
+            description:
+              "The destination already holds this Message-ID from the same sender",
+          },
         },
       }),
       async (c) => {
@@ -702,6 +725,12 @@ export function createMailboxRoutes(
         // `restore` has no fixed source: a message can be restored out of
         // either Archive or Trash, named by `?folder=`.
         const fromFolder = from ?? c.req.query("folder") ?? "Archive";
+        if (from === undefined && !RESTORE_FOLDERS.includes(fromFolder)) {
+          return c.json(
+            { error: "restore folder must be Archive or Trash" },
+            400,
+          );
+        }
         let newUid: number;
         try {
           newUid = await moveNativeMailboxMessage(
@@ -711,8 +740,14 @@ export function createMailboxRoutes(
             uid,
             to,
           );
-        } catch {
-          return c.json({ error: "Message not found" }, 404);
+        } catch (err) {
+          if (err instanceof MailboxMessageNotFoundError) {
+            return c.json({ error: "Message not found" }, 404);
+          }
+          if (isUniqueViolation(err)) {
+            return c.json({ error: `${to} already holds this message` }, 409);
+          }
+          throw err;
         }
         publish(resolved, `${to}:${newUid}`, op);
         return c.json({ uid: newUid, ok: true as const });