Merge pull request #379 from Coffey-Labs/fix/security-followups

Close the smaller gaps from the security review

server/src/account.test.ts

@@ -69,7 +69,7 @@ test("the registry reports an account with nothing set up yet", async () => {
 });
 
 test("app passwords are created, listed once with their secret, and revoked", async () => {
-  const created = await post("/api/account/app-passwords", { description: "Thunderbird" });
+  const created = await post("/api/account/app-passwords", { description: "Thunderbird", current: "demo-password" });
   assert.equal(created.status, 200);
   assert.match(created.body.secret, /^\$app\$/, "the server's generated secret is returned");
   assert.ok(created.body.id);
@@ -84,8 +84,40 @@ test("app passwords are created, listed once with their secret, and revoked", as
   assert.deepEqual((await call("/api/account/security")).body.appPasswords, []);
 });
 
+test("an app password needs the account password", async () => {
+  const missing = await post("/api/account/app-passwords", { description: "Stolen" });
+  assert.equal(missing.status, 400);
+  assert.equal(missing.body.error, "missing_fields");
+  const wrong = await post("/api/account/app-passwords", { description: "Stolen", current: "not-my-password" });
+  assert.equal(wrong.status, 403);
+  assert.equal(wrong.body.error, "invalid_credentials");
+  assert.deepEqual((await call("/api/account/security")).body.appPasswords, [], "nothing was created");
+});
+
+test("a checked session cannot mint one through the JMAP proxy instead", async () => {
+  // Signed in without "my own device", so the proxy reads every request.
+  const res = await call("/api/jmap", {
+    method: "POST",
+    body: JSON.stringify({ using: ["urn:ietf:params:jmap:core"], methodCalls: [["x:AppPassword/set", { create: { n: { description: "Stolen" } } }, "0"]] }),
+  });
+  assert.equal(res.status, 403);
+  assert.deepEqual((await call("/api/account/security")).body.appPasswords, []);
+});
+
+test("attachments are kept out of the disk cache of a device that is not the person's own", async () => {
+  const up = await app.request("/api/upload/a1", { method: "POST", headers: { "x-requested-with": "ihasmail", "content-type": "text/plain", cookie }, body: "hello" });
+  assert.equal(up.status, 200);
+  const { blobId } = (await up.json()) as { blobId: string };
+  const name = encodeURIComponent("Invoice_\u202Efdp.exe");
+  const res = await app.request(`/api/blob/a1/${blobId}/${name}?accept=text/plain`, { headers: { cookie } });
+  assert.equal(res.status, 200);
+  assert.equal(res.headers.get("cache-control"), "no-store");
+  assert.equal(res.headers.get("content-disposition"), "attachment; filename*=UTF-8''Invoice_fdp.exe", "no direction override in the saved name");
+  await res.arrayBuffer();
+});
+
 test("an app password needs a name", async () => {
-  const res = await post("/api/account/app-passwords", { description: "   " });
+  const res = await post("/api/account/app-passwords", { description: "   ", current: "demo-password" });
   assert.equal(res.status, 400);
   assert.equal(res.body.error, "missing_fields");
 });

server/src/adminGate.test.ts

@@ -14,8 +14,18 @@ test("mail, calendars and the rest pass untouched", () => {
   assert.equal(r.ok, true);
 });
 
-test("the account's own registry objects pass", () => {
-  assert.equal(gateAdministration(req("x:AccountSettings/get", "x:AppPassword/set", "x:PublicKey/get", "x:MaskedEmail/set")).ok, true);
+test("the account's own registry objects can be read", () => {
+  assert.equal(gateAdministration(req("x:AccountSettings/get", "x:AppPassword/get", "x:PublicKey/get", "x:MaskedEmail/query")).ok, true);
+});
+
+test("but not written: a credential minted here would outlive a borrowed session", () => {
+  for (const m of ["x:AppPassword/set", "x:AccountPassword/set", "x:MaskedEmail/set"]) {
+    assert.deepEqual(gateAdministration(req("x:AccountSettings/get", m)), { ok: false, method: m });
+  }
+});
+
+test("API keys are not the account's to reach from here at all", () => {
+  assert.deepEqual(gateAdministration(req("x:ApiKey/get")), { ok: false, method: "x:ApiKey/get" });
 });
 
 test("directory and server objects are refused, and named", () => {

server/src/adminGate.ts

@@ -18,7 +18,7 @@
  * The standard JMAP methods (mail, calendars, contacts, files, sharing) are not
  * touched: they act on what the account can already reach.
  */
-const SELF_SERVICE = new Set(["AccountSettings", "AccountPassword", "AppPassword", "ApiKey", "PublicKey", "MaskedEmail"]);
+const SELF_SERVICE = new Set(["AccountSettings", "AccountPassword", "AppPassword", "PublicKey", "MaskedEmail"]);
 
 export type GateResult = { ok: true; body: string } | { ok: false; method: string | null };
 
@@ -83,8 +83,15 @@ export function gateAdministration(raw: string): GateResult {
     const name = Array.isArray(call) ? call[0] : undefined;
     if (typeof name !== "string") return { ok: false, method: null };
     if (!name.startsWith("x:")) continue;
-    const object = name.slice(2).split("/")[0] ?? "";
-    if (!SELF_SERVICE.has(object)) return { ok: false, method: name };
+    const [object = "", op = ""] = name.slice(2).split("/");
+    /*
+     * Read, never write. The browser sends none of these itself -- password,
+     * app-password and 2FA changes go through /api/account, which checks the
+     * account password first -- so a write here could only come from
+     * somebody working the console of a session on a borrowed machine, and
+     * `x:AppPassword/set` would hand them a credential that outlives it.
+     */
+    if (!SELF_SERVICE.has(object) || op === "set") return { ok: false, method: name };
   }
   return { ok: true, body: JSON.stringify(parsed) };
 }

server/src/app.ts

@@ -11,9 +11,10 @@ import { getConnInfo } from "@hono/node-server/conninfo";
 import { config } from "./config.js";
 import { fetchPermissions } from "./permissionSchema.js";
 import { administrationAllowed, gateAdministration, grantsAdministration } from "./adminGate.js";
-import { SessionStore, type SessionBackend, type LiveSession } from "./sessions.js";
+import { SessionStore, accountKey, type SessionBackend, type LiveSession } from "./sessions.js";
 import { RateLimiter } from "./ratelimit.js";
 import { rateLimitKey, resolveClientIp } from "./clientip.js";
+import { safeEqual } from "./crypto.js";
 import {
   type AccountInfo,
   UpstreamError,
@@ -385,6 +386,7 @@ export function createApp(basePath = config.basePath): Hono<Env> {
       loginLimiter.reset(limitKey);
       const { cookie, session } = sessions.create({
         username,
+        account: accountKey(upstreamFor(username), upstream.username || username),
         password: effectivePassword,
         remember: Boolean(body.remember),
         userAgent: c.req.header("user-agent") ?? "",
@@ -466,12 +468,12 @@ export function createApp(basePath = config.basePath): Hono<Env> {
 
   api.get("/auth/sessions", requireSession, (c) => {
     const session = c.get("session");
-    return c.json({ current: session.id, sessions: sessions.listForUser(session.username) });
+    return c.json({ current: session.id, sessions: sessions.listForUser(session.account) });
   });
 
   api.post("/auth/sessions/revoke-others", requireSession, (c) => {
     const session = c.get("session");
-    const n = sessions.destroyAllForUser(session.username, session.id);
+    const n = sessions.destroyAllForUser(session.account, session.id);
     return c.json({ revoked: n });
   });
 
@@ -499,8 +501,8 @@ export function createApp(basePath = config.basePath): Hono<Env> {
   };
 
   /** Guard the endpoints that check a password against brute-forcing. */
-  const guarded = (c: Context<Env>): Response | null => {
-    const key = `account|${c.get("session").username.toLowerCase()}`;
+  const guarded = (c: Context<Env>, scope = "account"): Response | null => {
+    const key = `${scope}|${c.get("session").username.toLowerCase()}`;
     if (accountLimiter.check(key)) return null;
     c.header("Retry-After", String(accountLimiter.retryAfterSeconds(key)));
     return c.json({ error: "rate_limited", message: "Too many attempts. Please wait and try again." }, 429);
@@ -538,7 +540,7 @@ export function createApp(basePath = config.basePath): Hono<Env> {
     const otpCode = body.otpCode?.trim();
     sessions.reseal(getCookie(c, config.cookieName), otpCode ? `${next}$${otpCode}` : next);
     forgetUpstreamSession(session.id);
-    const revoked = sessions.destroyAllForUser(session.username, session.id);
+    const revoked = sessions.destroyAllForUser(session.account, session.id);
     return c.json({ ok: true, revokedSessions: revoked });
   });
 
@@ -552,12 +554,26 @@ export function createApp(basePath = config.basePath): Hono<Env> {
     }
   });
 
+  /*
+   * An app password is a credential that outlives this session, a password
+   * change and a sign-out -- so minting one asks for the account password, as
+   * changing the password does. Otherwise a session left open on somebody
+   * else's machine is enough to take a permanent key away from it.
+   */
   api.post("/account/app-passwords", requireSession, async (c) => {
+    // A budget of its own: guessing here never reaches Stalwart (see confirmsPassword).
+    const limited = guarded(c, "app-password");
+    if (limited) return limited;
     const session = c.get("session");
-    const body = await readJson<{ description?: string }>(c);
+    const body = await readJson<{ description?: string; current?: string }>(c);
     if (!body) return c.json({ error: "bad_request" }, 400);
     const description = (body.description ?? "").trim().slice(0, 120);
     if (!description) return c.json({ error: "missing_fields", message: "Give the app password a name." }, 400);
+    const current = body.current ?? "";
+    if (!current || current.length > 1024) return c.json({ error: "missing_fields", message: "Enter your current password." }, 400);
+    if (!(await confirmsPassword(session, current))) {
+      return c.json({ error: "invalid_credentials", message: "That password is not correct." }, 403);
+    }
     try {
       return c.json(await createAppPassword(await accountCtx(c), { description }));
     } catch (err) {
@@ -632,7 +648,7 @@ export function createApp(basePath = config.basePath): Hono<Env> {
       if (sessionKept) forgetUpstreamSession(session.id);
     }
     // Other sessions still hold the bare password and will be refused.
-    const revoked = sessions.destroyAllForUser(session.username, session.id);
+    const revoked = sessions.destroyAllForUser(session.account, session.id);
     return c.json({ ok: true, sessionKept, revokedSessions: revoked });
   });
 
@@ -803,7 +819,7 @@ export function createApp(basePath = config.basePath): Hono<Env> {
       const safeInline = inline && isInlineSafe(type);
       headers.set(
         "Content-Disposition",
-        `${safeInline ? "inline" : "attachment"}; filename*=UTF-8''${encodeURIComponent(name)}`,
+        `${safeInline ? "inline" : "attachment"}; filename*=UTF-8''${encodeURIComponent(withoutBidiControls(name))}`,
       );
       headers.set("X-Content-Type-Options", "nosniff");
       // Sandbox everything except the browser's built-in PDF viewer (which needs scripts to render).
@@ -822,7 +838,9 @@ export function createApp(basePath = config.basePath): Hono<Env> {
       } else {
         headers.set("Content-Security-Policy", "sandbox; default-src 'none'; style-src 'unsafe-inline'; img-src data:");
       }
-      headers.set("Cache-Control", "private, max-age=3600");
+      // Kept out of the browser's disk cache on a device that is not the
+      // person's own: signing out wipes what the app stores, not that.
+      headers.set("Cache-Control", session.remember ? "private, max-age=3600" : "no-store");
       return new Response(res.body, { status: 200, headers });
     } catch (err) {
       return upstreamFailure(c, err);
@@ -908,6 +926,43 @@ async function readJson<T>(c: Context): Promise<T | null> {
   }
 }
 
+/**
+ * Is `candidate` the password of the account this session is signed in to?
+ *
+ * Compared with the credential the session holds first, which costs nothing
+ * and tells Stalwart nothing -- its auto-ban counts failures against the
+ * proxy's address, which every user shares. That credential is the password,
+ * with a TOTP code after a `$` when one was given at sign-in. A session that
+ * turning on 2FA moved onto an app password (Stalwart's secrets start
+ * `$app$`) holds something else, and only then is the candidate put to the
+ * server.
+ */
+async function confirmsPassword(session: LiveSession, candidate: string): Promise<boolean> {
+  const decoded = Buffer.from(session.authorization.replace(/^Basic /, ""), "base64").toString("utf8");
+  const held = decoded.slice(decoded.indexOf(":") + 1);
+  if (safeEqual(held, candidate)) return true;
+  const withoutCode = held.replace(/\$\d{6,8}$/, "");
+  if (withoutCode !== held && safeEqual(withoutCode, candidate)) return true;
+  // Holding the password, the comparison above is the answer, and a wrong
+  // guess never reaches the server's auto-ban.
+  if (!held.startsWith("$app$")) return false;
+  try {
+    const authorization = `Basic ${Buffer.from(`${session.username}:${candidate}`, "utf8").toString("base64")}`;
+    await fetchUpstreamSession(authorization, upstreamFor(session.username));
+    return true;
+  } catch {
+    return false;
+  }
+}
+
+/**
+ * Direction overrides and isolates, which can make `Invoice_\u202Efdp.exe`
+ * read as a PDF in the downloads list. A filename has no use for them.
+ */
+function withoutBidiControls(name: string): string {
+  return name.replace(/[\u061C\u200E\u200F\u202A-\u202E\u2066-\u2069]/g, "");
+}
+
 /** Name the app password after the browser it will live in. */
 function appPasswordName(c: Context): string {
   const ua = c.req.header("user-agent") ?? "";

server/src/imageproxy.ts

@@ -217,7 +217,8 @@ export async function imageProxyHandler(c: Context) {
   res.on("close", done);
   const headers = new Headers({
     "Content-Type": type,
-    "Cache-Control": "private, max-age=86400",
+    // As for attachments: nothing left in the disk cache of a device that is not the person's own.
+    "Cache-Control": (c.get("session") as { remember?: boolean } | undefined)?.remember ? "private, max-age=86400" : "no-store",
     "X-Content-Type-Options": "nosniff",
     "Content-Security-Policy": "sandbox; default-src 'none'",
     "Cross-Origin-Resource-Policy": "same-origin",

server/src/sessions.test.ts

@@ -1,6 +1,6 @@
 import { test } from "node:test";
 import assert from "node:assert/strict";
-import { SessionStore } from "./sessions.js";
+import { SessionStore, accountKey } from "./sessions.js";
 import { normalizeLocale } from "./upstream.js";
 import { deriveKey, open, seal, sha256 } from "./crypto.js";
 import { RateLimiter } from "./ratelimit.js";
@@ -30,6 +30,20 @@ test("session store creates, resolves, and refuses tampered cookies", () => {
   assert.equal(store.resolve(cookie), null);
 });
 
+test("sessions group by the account, however its name was typed", () => {
+  const store = new SessionStore("");
+  const key = accountKey("https://mail.example.com", "[email protected]");
+  const a = store.create({ username: "alice", account: key, password: "pw", remember: false, userAgent: "", ip: "" });
+  const b = store.create({ username: "[email protected]", account: accountKey("https://mail.example.com", "[email protected]"), password: "pw", remember: false, userAgent: "", ip: "" });
+  // The same name on another configured server is another account.
+  store.create({ username: "[email protected]", account: accountKey("https://other.example.net", "[email protected]"), password: "pw", remember: false, userAgent: "", ip: "" });
+  assert.equal(a.session.account, b.session.account);
+  assert.equal(store.listForUser(a.session.account).length, 2);
+  assert.equal(store.destroyAllForUser(a.session.account, a.session.id), 1);
+  assert.equal(store.resolve(b.cookie), null, "the other spelling was signed out");
+  assert.ok(store.resolve(a.cookie), "this session was kept");
+});
+
 test("persisted session data does not contain the password", () => {
   const store = new SessionStore("");
   store.create({ username: "u", password: "super-secret-pw", remember: true, userAgent: "", ip: "" });

server/src/sessions.ts

@@ -13,6 +13,8 @@ export interface StoredSession {
   /** sealed JSON {username, password} */
   sealedCredentials: string;
   username: string;
+  /** Which account this is; see `accountKey`. Absent on sessions saved before it existed. */
+  account?: string;
   createdAt: number;
   lastSeenAt: number;
   expiresAt: number;
@@ -24,6 +26,8 @@ export interface StoredSession {
 export interface LiveSession {
   id: string;
   username: string;
+  /** See `accountKey`. */
+  account: string;
   /** Basic Authorization header value for upstream calls. */
   authorization: string;
   remember: boolean;
@@ -46,8 +50,27 @@ export interface SessionSummary {
   ip: string;
 }
 
+/**
+ * The key sessions are grouped by for "sign out everywhere else".
+ *
+ * Not the username as typed: Stalwart takes `[email protected]` and a bare
+ * `alice` as the same account, and a session opened either way was missing
+ * from the list and survived the sign-out. The server's own name for the
+ * account, lower-cased, and the server it lives on -- the same name on two
+ * configured servers is two accounts.
+ */
+export function accountKey(upstream: string, canonicalUsername: string): string {
+  return `${upstream}|${canonicalUsername.trim().toLowerCase()}`;
+}
+
+function accountOf(s: StoredSession): string {
+  return s.account ?? s.username.trim().toLowerCase();
+}
+
 export interface CreateSessionParams {
   username: string;
+  /** From `accountKey`; defaults to the lower-cased username. */
+  account?: string;
   password: string;
   remember: boolean;
   userAgent: string;
@@ -85,8 +108,9 @@ export interface SessionBackend {
   resolve(cookie: string | undefined): LiveSession | null;
   reseal(cookie: string | undefined, password: string): boolean;
   destroy(id: string): void;
-  destroyAllForUser(username: string, exceptId?: string): number;
-  listForUser(username: string): SessionSummary[];
+  /** `account` is an `accountKey`, as carried on `LiveSession.account`. */
+  destroyAllForUser(account: string, exceptId?: string): number;
+  listForUser(account: string): SessionSummary[];
 }
 
 const COOKIE_SEP = ".";
@@ -172,6 +196,7 @@ export class SessionStore implements SessionBackend {
       salt: salt.toString("base64"),
       sealedCredentials: seal(JSON.stringify({ u: params.username, p: params.password }), key),
       username: params.username,
+      account: params.account ?? params.username.trim().toLowerCase(),
       createdAt: now,
       lastSeenAt: now,
       expiresAt: now + ttl,
@@ -248,10 +273,10 @@ export class SessionStore implements SessionBackend {
     if (this.sessions.delete(id)) this.scheduleSave();
   }
 
-  destroyAllForUser(username: string, exceptId?: string): number {
+  destroyAllForUser(account: string, exceptId?: string): number {
     let n = 0;
     for (const [id, s] of this.sessions) {
-      if (s.username === username && id !== exceptId) {
+      if (accountOf(s) === account && id !== exceptId) {
         this.sessions.delete(id);
         n++;
       }
@@ -260,11 +285,11 @@ export class SessionStore implements SessionBackend {
     return n;
   }
 
-  listForUser(username: string): SessionSummary[] {
+  listForUser(account: string): SessionSummary[] {
     const out = [];
     for (const s of this.sessions.values()) {
-      if (s.username !== username) continue;
-      const { secretHash: _h, salt: _s, sealedCredentials: _c, ...rest } = s;
+      if (accountOf(s) !== account) continue;
+      const { secretHash: _h, salt: _s, sealedCredentials: _c, account: _a, ...rest } = s;
       out.push(rest);
     }
     return out;
@@ -274,6 +299,7 @@ export class SessionStore implements SessionBackend {
     return {
       id: s.id,
       username,
+      account: accountOf(s),
       authorization: `Basic ${Buffer.from(`${username}:${password}`, "utf8").toString("base64")}`,
       remember: s.remember,
       createdAt: s.createdAt,

web/src/lib/__tests__/address.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, it } from "vitest";
-import { formatAddress, initials, isValidEmail, parseAddressList, parseMailto } from "../address";
+import { displayName, formatAddress, initials, isValidEmail, parseAddressList, parseMailto } from "../address";
 
 describe("address parsing", () => {
   it("parses mixed lists", () => {
@@ -55,3 +55,14 @@ describe("mailto URLs", () => {
     expect(m.to).toHaveLength(1);
   });
 });
+
+describe("names that reorder themselves", () => {
+  const spoof = { name: "[email protected]\u202E", email: "[email protected]" };
+  it("lose their direction controls when displayed", () => {
+    expect(displayName({ name: "\u202Egnp.exe\u202C Ann", email: "[email protected]" })).toBe("gnp.exe Ann");
+    expect(formatAddress(spoof)).toBe("[email protected] <[email protected]>");
+  });
+  it("fall back to the address when nothing else is left", () => {
+    expect(displayName({ name: "\u200F\u202E", email: "[email protected]" })).toBe("[email protected]");
+  });
+});

web/src/lib/__tests__/tnef.test.ts

@@ -79,6 +79,12 @@ describe("isTnef", () => {
 });
 
 describe("parseTnef", () => {
+  it("takes the direction overrides out of a name", () => {
+    const out = parseTnef(tnef(file("x.bin", "MZ", [
+      { id: ATT.attachment, data: mapi([{ id: 0x3707, type: 0x001f, value: "Invoice_\u202Efdp.exe" }]) },
+    ])));
+    expect(out[0]!.name).toBe("Invoice_fdp.exe");
+  });
   it("pulls one attachment out, with its name and bytes", () => {
     const out = parseTnef(tnef(file("report.pdf", "hello")));
     expect(out).toHaveLength(1);

web/src/lib/address.ts

@@ -1,4 +1,5 @@
 import type { EmailAddress } from "@/jmap/types";
+import { withoutBidiControls } from "@/lib/text/text";
 
 const EMAIL_RE = /^[^\s@<>"',;]+@[^\s@<>"',;]+\.[^\s@<>"',;]+$/;
 
@@ -48,9 +49,10 @@ export function parseOne(raw: string): EmailAddress | null {
 
 export function formatAddress(a: EmailAddress | null | undefined): string {
   if (!a) return "";
-  if (!a.name) return a.email;
-  const needsQuote = /[,;<>"()\\]/.test(a.name);
-  const name = needsQuote ? `"${a.name.replace(/(["\\])/g, "\\$1")}"` : a.name;
+  const clean = a.name ? withoutBidiControls(a.name) : "";
+  if (!clean) return a.email;
+  const needsQuote = /[,;<>"()\\]/.test(clean);
+  const name = needsQuote ? `"${clean.replace(/(["\\])/g, "\\$1")}"` : clean;
   return `${name} <${a.email}>`;
 }
 
@@ -60,7 +62,8 @@ export function formatAddressList(list: EmailAddress[] | null | undefined): stri
 
 export function displayName(a: EmailAddress | null | undefined, fallback = "(unknown)"): string {
   if (!a) return fallback;
-  if (a.name?.trim()) return a.name.trim();
+  const name = a.name ? withoutBidiControls(a.name).trim() : "";
+  if (name) return name;
   return a.email || fallback;
 }
 

web/src/lib/text/text.ts

@@ -1,3 +1,12 @@
+/**
+ * Remove the characters that reorder text around them: direction overrides,
+ * embeddings, isolates and marks. A sender-supplied name has no honest use for
+ * them, and `Invoice_\u202Efdp.exe` displays as "Invoice_exe.pdf".
+ */
+export function withoutBidiControls(s: string): string {
+  return s.replace(/[\u061C\u200E\u200F\u202A-\u202E\u2066-\u2069]/g, "");
+}
+
 export function escapeHtml(s: string): string {
   return s.replace(/&/g, "&amp;").replace(/</g, "&lt;").replace(/>/g, "&gt;").replace(/"/g, "&quot;").replace(/'/g, "&#39;");
 }

web/src/lib/tnef.ts

@@ -1,3 +1,4 @@
+import { withoutBidiControls } from "@/lib/text/text";
 /**
  * `winmail.dat`, opened.
  *
@@ -210,7 +211,7 @@ export function parseTnef(input: ArrayBuffer | Uint8Array): TnefAttachment[] {
       current = null;
       return;
     }
-    const name = (current.mapiName || current.title || "attachment").trim() || "attachment";
+    const name = withoutBidiControls(current.mapiName || current.title || "attachment").trim() || "attachment";
     out.push({
       name,
       type: current.mapiType || guessType(name),

web/src/store/__tests__/compose-sign-out.test.ts

@@ -0,0 +1,53 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import { useCompose } from "@/store/compose";
+import { useSession } from "@/store/session";
+
+/**
+ * A message being written belongs to the session it was written in. On a
+ * shared machine the next person to sign in -- after an idle sign-out, with no
+ * reload in between -- used to find the last one's composer still open.
+ */
+
+beforeEach(() => {
+  vi.useFakeTimers();
+  useSession.setState({ status: "authenticated" });
+});
+
+afterEach(() => {
+  useCompose.setState({ drafts: [], activeKey: null, pendingSends: {} });
+  vi.useRealTimers();
+});
+
+describe("signing out", () => {
+  it("closes every composer and stops sends that are still waiting", () => {
+    const run = vi.fn(async () => {});
+    const timer = window.setTimeout(() => void run(), 5000);
+    useCompose.setState({
+      drafts: [{ key: "d1", subject: "Half written" } as never],
+      activeKey: "d1",
+      pendingSends: { d2: { timer, toastId: 1, draft: { key: "d2" } as never, run } },
+    });
+    useSession.setState({ status: "anonymous" });
+    expect(useCompose.getState().drafts).toEqual([]);
+    expect(useCompose.getState().activeKey).toBeNull();
+    expect(useCompose.getState().pendingSends).toEqual({});
+    vi.advanceTimersByTime(10_000);
+    expect(run).not.toHaveBeenCalled();
+  });
+
+  it("leaves the composer alone while still signed in", () => {
+    useCompose.setState({ drafts: [{ key: "d1" } as never], activeKey: "d1" });
+    useSession.setState({ pushConnected: true });
+    expect(useCompose.getState().drafts).toHaveLength(1);
+  });
+
+  it("sends what is inside its undo window before the session goes", async () => {
+    const run = vi.fn(async () => {});
+    const timer = window.setTimeout(() => void run(), 5000);
+    useCompose.setState({ pendingSends: { d2: { timer, toastId: 1, draft: { key: "d2" } as never, run } } });
+    await useCompose.getState().flushPendingSends();
+    expect(run).toHaveBeenCalledTimes(1);
+    vi.advanceTimersByTime(10_000);
+    expect(run).toHaveBeenCalledTimes(1);
+  });
+});

web/src/store/compose.ts

@@ -7,6 +7,7 @@ import { escapeHtml, htmlToText, quoteText, replySubject, textToHtml } from "@/l
 import { sanitizeEmailHtml, sanitizeEditorHtml } from "@/lib/text/html";
 import { toast } from "@/ui/toast";
 import { useMail, FULL_PROPS, BODY_PROPS } from "./mail";
+import { useSession } from "./session";
 import { ensureScheduledMailbox, useScheduled } from "./scheduled";
 import { formatScheduleTime, holdUntil } from "@/lib/schedule";
 import { t as translate } from "@/lib/i18n";
@@ -82,7 +83,9 @@ export interface Draft {
 interface ComposeState {
   drafts: Draft[];
   activeKey: string | null;
-  pendingSends: Record<string, { timer: number; toastId: number; draft: Draft }>;
+  pendingSends: Record<string, { timer: number; toastId: number; draft: Draft; run: () => Promise<void> }>;
+  /** Send everything still inside its undo window now. For signing out, while the session can still send. */
+  flushPendingSends(): Promise<void>;
   open(init?: Partial<Draft>): string;
   /** Open a draft holding what the operating system's share sheet sent us. */
   openFromShare(share: SharedContent): string;
@@ -632,7 +635,16 @@ export const useCompose = create<ComposeState>((set, get) => ({
     }
     const toastId = toast.show(translate("Sending…"), { duration: delay * 1000, progress: true, action: { label: translate("Undo"), onClick: () => get().undoSend(key) } });
     const timer = window.setTimeout(() => void doSend(), delay * 1000);
-    set((s) => ({ pendingSends: { ...s.pendingSends, [key]: { timer, toastId, draft: d } } }));
+    set((s) => ({ pendingSends: { ...s.pendingSends, [key]: { timer, toastId, draft: d, run: doSend } } }));
+  },
+
+  async flushPendingSends() {
+    const pending = Object.values(get().pendingSends);
+    for (const p of pending) {
+      window.clearTimeout(p.timer);
+      toast.dismiss(p.toastId);
+    }
+    await Promise.all(pending.map((p) => p.run()));
   },
 
   undoSend(key) {
@@ -999,3 +1011,28 @@ export function draftFromMailto(url: string): Partial<Draft> {
     ...(body ? { html: body, text: m.body } : {}),
   };
 }
+
+/*
+ * Nothing written in one session is left for the next.
+ *
+ * The other stores let go of their data when the session ends; this one used
+ * to keep its open composers, so on a shared machine the next person to sign
+ * in -- without a reload, after an idle sign-out, say -- found the last one's
+ * draft open and could send it. A draft that was saved is still in Drafts on
+ * the server. A send still in its undo window was sent on the way out if the
+ * sign-out was a deliberate one (see `logout`); if the session had already
+ * ended there is nothing left to send it with, so its timer is stopped rather
+ * than let it fire under whoever signs in next.
+ */
+useSession.subscribe((s) => {
+  if (s.status !== "anonymous") return;
+  const { drafts, pendingSends } = useCompose.getState();
+  if (!drafts.length && !Object.keys(pendingSends).length) return;
+  for (const t of autosaveTimers.values()) window.clearTimeout(t);
+  autosaveTimers.clear();
+  for (const p of Object.values(pendingSends)) {
+    window.clearTimeout(p.timer);
+    toast.dismiss(p.toastId);
+  }
+  useCompose.setState({ drafts: [], activeKey: null, pendingSends: {} });
+});

web/src/store/session.ts

@@ -78,6 +78,14 @@ export const useSession = create<SessionState>((set, get) => ({
       /* never block signing out over this */
     }
     stopSettingsSync();
+    // A message still inside its undo window goes now, while there is a
+    // session to send it with; signing out is not an undo.
+    try {
+      const { useCompose } = await import("./compose");
+      await useCompose.getState().flushPendingSends();
+    } catch {
+      /* never block signing out over this */
+    }
     try {
       await apiFetch("/api/auth/logout", { method: "POST" });
     } catch {

web/src/ui/filepreview.tsx

@@ -2,6 +2,7 @@ import { useCallback, useEffect, useMemo, useRef, useState, type ReactNode } fro
 import { Code2, Download, Eye, Pencil, Printer, Save, Share2, X } from "lucide-react";
 import { confirmDialog, Dialog } from "./dialog";
 import { formatSize } from "@/lib/format";
+import { withoutBidiControls } from "@/lib/text/text";
 import { previewKind, TEXT_PREVIEW_CHARS, TEXT_PREVIEW_MAX } from "@/lib/preview";
 import { isMarkdown, renderMarkdown } from "@/lib/text/markdown";
 import { canShareFiles, shareFile } from "@/lib/share";
@@ -168,7 +169,7 @@ export function FilePreviewDialog({
     const download = () => {
       const l = document.createElement("a");
       l.href = file.url;
-      l.download = file.name;
+      l.download = withoutBidiControls(file.name);
       l.click();
     };
     try {
@@ -226,7 +227,7 @@ export function FilePreviewDialog({
     <Dialog
       open={Boolean(file)}
       onClose={requestClose}
-      title={file?.name ?? t("Preview")}
+      title={file ? withoutBidiControls(file.name) : t("Preview")}
       size="xl"
       closeOnBackdrop={!editing}
       footer={

web/src/views/mail/MessageView.tsx

@@ -21,7 +21,7 @@ import { displayName, domainOf, formatAddress } from "@/lib/address";
 import { EMAIL_BASE_CSS, TEXT_EMAIL_CSS, hasHtmlAlternative, htmlDeclaresColors, markKeptSurfaces, sanitizeEmailHtml } from "@/lib/text/html";
 import { openableInTab, previewKind } from "@/lib/preview";
 import { FilePreviewDialog } from "@/ui/filepreview";
-import { findQuoteStart, htmlToText, textToHtml } from "@/lib/text/text";
+import { findQuoteStart, htmlToText, textToHtml, withoutBidiControls } from "@/lib/text/text";
 import { canShare, canShareFiles, shareFile, shareText } from "@/lib/share";
 import { Avatar } from "@/ui/misc";
 import { MenuItem, MenuSep, Popover, useMenu } from "@/ui/popover";
@@ -796,7 +796,13 @@ function TnefContents({ part, accountId }: { part: EmailBodyPart; accountId: Id
       const blob = await client.fetchBlob(accountId, part.blobId, part.type);
       const found = parseTnef(await blob.arrayBuffer());
       setFiles(found);
-      setUrls(found.map((f) => URL.createObjectURL(new Blob([f.data as unknown as BlobPart], { type: f.type }))));
+      /*
+       * The type inside a winmail.dat is whatever the sender wrote, and never
+       * passed the server's check on what may be shown inline. Opened from its
+       * blob: URL, text/html would render as a page on this origin -- so only
+       * the types the server itself would show are kept.
+       */
+      setUrls(found.map((f) => URL.createObjectURL(new Blob([f.data as unknown as BlobPart], { type: openableInTab(f.type) ? f.type : "application/octet-stream" }))));
       setState("done");
     } catch {
       setState("error");
@@ -859,7 +865,7 @@ function AttachmentList({ attachments, accountId, email }: { attachments: EmailB
    */
   const shareAttachment = async (a: EmailBodyPart) => {
     if (!a.blobId) return;
-    const name = a.name ?? "attachment";
+    const name = withoutBidiControls(a.name ?? "") || "attachment";
     const download = () => {
       const l = document.createElement("a");
       l.href = client.downloadUrl(accountId, a.blobId!, name, a.type);
@@ -885,16 +891,17 @@ function AttachmentList({ attachments, accountId, email }: { attachments: EmailB
       ))}
       <div className="attachments">
         {attachments.map((a, i) => {
-          const url = a.blobId ? client.downloadUrl(accountId, a.blobId, a.name ?? "attachment", a.type) : "#";
-          const inlineUrl = a.blobId ? client.downloadUrl(accountId, a.blobId, a.name ?? "attachment", a.type, true) : "#";
+          const name = a.name ? withoutBidiControls(a.name) : null;
+          const url = a.blobId ? client.downloadUrl(accountId, a.blobId, name ?? "attachment", a.type) : "#";
+          const inlineUrl = a.blobId ? client.downloadUrl(accountId, a.blobId, name ?? "attachment", a.type, true) : "#";
           return (
-            <a key={a.blobId ?? i} className="attachment" href={url} download={a.name ?? undefined} title={`${a.name ?? translate("Attachment")} (${formatSize(a.size)})`} onClick={(ev) => { if (viewable(a)) { ev.preventDefault(); setPreview(a); } }}>
+            <a key={a.blobId ?? i} className="attachment" href={url} download={name ?? undefined} title={`${name ?? translate("Attachment")} (${formatSize(a.size)})`} onClick={(ev) => { if (viewable(a)) { ev.preventDefault(); setPreview(a); } }}>
               <span className="att-icon">{a.type.startsWith("image/") && a.type !== "image/svg+xml" && a.blobId ? <img src={inlineUrl} alt="" loading="lazy" /> : attachmentIcon(a.type, a.name)}</span>
               <span className="att-text">
-                <span className="att-name">{a.name ?? "(unnamed)"}</span>
+                <span className="att-name">{name ?? "(unnamed)"}</span>
                 <span className="att-size">{formatSize(a.size)}</span>
                 <span className="att-actions">
-                  <button className="icon-btn xs" title={translate("Download")} onClick={(ev) => { ev.preventDefault(); ev.stopPropagation(); const l = document.createElement("a"); l.href = url; l.download = a.name ?? ""; l.click(); }}><Download size={14} /></button>
+                  <button className="icon-btn xs" title={translate("Download")} onClick={(ev) => { ev.preventDefault(); ev.stopPropagation(); const l = document.createElement("a"); l.href = url; l.download = name ?? ""; l.click(); }}><Download size={14} /></button>
                   {canShareFiles() && a.blobId && <button className="icon-btn xs" title={tc("share sheet", "Share")} onClick={(ev) => { ev.preventDefault(); ev.stopPropagation(); void shareAttachment(a); }}><Share2 size={14} /></button>}
                   {openableInTab(a.type) && a.blobId && <button className="icon-btn xs" title={translate("Open in new tab")} onClick={(ev) => { ev.preventDefault(); ev.stopPropagation(); window.open(inlineUrl, "_blank", "noopener"); }}><ExternalLink size={14} /></button>}
                 </span>

web/src/views/settings/SecuritySettings.tsx

@@ -231,6 +231,7 @@ function TwoFactorOff({ reload }: { reload: () => Promise<void> }) {
 
 function AppPasswords({ state, reload }: { state: SecurityState | null; reload: () => Promise<void> }) {
   const [name, setName] = useState("");
+  const [current, setCurrent] = useState("");
   const [busy, setBusy] = useState(false);
   const [issued, setIssued] = useState<{ description: string; secret: string } | null>(null);
 
@@ -242,10 +243,11 @@ function AppPasswords({ state, reload }: { state: SecurityState | null; reload:
     try {
       const res = await apiFetch<{ id: string; secret: string }>("/api/account/app-passwords", {
         method: "POST",
-        body: JSON.stringify({ description: name }),
+        body: JSON.stringify({ description: name, current }),
       });
       setIssued({ description: name, secret: res.secret });
       setName("");
+      setCurrent("");
       await reload();
     } catch (err) {
       toast.error((err as Error).message);
@@ -296,7 +298,12 @@ function AppPasswords({ state, reload }: { state: SecurityState | null; reload:
           <label htmlFor="ap-name">{t("New app password for")}</label>
           <input id="ap-name" value={name} onChange={(e) => setName(e.target.value)} placeholder={t("Thunderbird on my laptop")} required />
         </div>
-        <button className="btn" disabled={busy || !name.trim()}>{busy ? "Creating…" : "Create"}</button>
+        {/* A credential that outlives this session: the server asks for the password first. */}
+        <div className="field" style={{ marginBottom: 0, minWidth: 200 }}>
+          <label htmlFor="ap-current">{t("Current password")}</label>
+          <input id="ap-current" type="password" autoComplete="current-password" value={current} onChange={(e) => setCurrent(e.target.value)} required />
+        </div>
+        <button className="btn" disabled={busy || !name.trim() || !current}>{busy ? "Creating…" : "Create"}</button>
       </form>
 
       <Dialog open={Boolean(issued)} onClose={() => setIssued(null)} title={t("Your new app password")} size="sm"