From d329b33912921a851c548bffef5085e5fbf72bed Mon Sep 17 00:00:00 2001 From: jcoffey <51408202+jcoffey-dev@users.noreply.github.com> Date: Sat, 19 Sep 2026 15:48:13 -0700 Subject: [PATCH] Quoting follows the message's own image decision (#410) (#411) Replying sanitized the quoted body with allowRemote: true, so quoting fetched every remote image in the message whatever the reader had decided about it. A tracking pixel in the quote then reported the message read, and the address live, to whoever was counting -- the thing leaving the images blocked was meant to prevent. Edit as new and opening a draft that quotes a message did the same. The decision now lives in one place, remoteImagesAllowed(), asked with the same inputs the reader's answer used: the image policy, the trusted senders, whether the sender is a contact, and whether Show images was pressed on that message. The last of those was component state, so it moves to the mail store, where the composer can see it. Blocked images already keep their address in data-ihm-remote, so nothing is lost by not fetching: it goes back on the way out, and the sent quote is what its sender wrote. The recipient's client decides for itself, as it would with any other client's reply. Before pr408 this needed a rich-text default to reach; the format offer made it reachable from plain text, which is how it was found. No new strings. --- FEATURES.md | 6 ++ web/src/lib/mail/remoteImages.ts | 45 +++++++++ .../__tests__/quote-image-policy.test.ts | 91 +++++++++++++++++++ web/src/store/compose.ts | 40 +++++++- web/src/store/mail/index.ts | 5 + web/src/store/mail/types.ts | 8 ++ web/src/views/mail/MessageView.tsx | 10 +- 7 files changed, 199 insertions(+), 6 deletions(-) create mode 100644 web/src/lib/mail/remoteImages.ts create mode 100644 web/src/store/__tests__/quote-image-policy.test.ts diff --git a/FEATURES.md b/FEATURES.md index 77a5d0c..8e64471 100644 --- a/FEATURES.md +++ b/FEATURES.md @@ -441,6 +441,12 @@ minimizable and maximizable; full-screen on mobile. code block, links (`Ctrl+K`), inline images, an emoji picker, and remove formatting. Tab and Shift+Tab indent inside the body. - **Plain text** as a per-message or default format. +- **Quoting follows the message's own image decision.** A quote renders the + message again, so the reply blocks its remote images unless that message was + allowed them — by policy, by a trusted sender, by the sender being a + contact, or by *Show images* having been pressed on it. Blocked images keep + their address and get it back when the reply is sent, so the recipient's + copy is the quote as its sender wrote it. - **Answering in the format the message was written in.** Replying in plain text to a rich text message, or the reverse, loses either the formatting or the plain text somebody chose to write in. The composer opens in the default diff --git a/web/src/lib/mail/remoteImages.ts b/web/src/lib/mail/remoteImages.ts new file mode 100644 index 0000000..b31b773 --- /dev/null +++ b/web/src/lib/mail/remoteImages.ts @@ -0,0 +1,45 @@ +import type { ImagePolicy } from "@/store/settings"; + +/** + * Whether a message's remote images may be fetched. + * + * The reader's decision, in one place, because the composer has to make the + * same one. Quoting a message into a reply renders it again — and a quote that + * fetched what the reader had declined would report the message read, and the + * address live, to whoever was counting. The tracking pixel does not care + * which window it loaded in. + */ +export function remoteImagesAllowed(opts: { + from: string | null | undefined; + policy: ImagePolicy; + trusted: string[]; + inContacts: boolean; + /** The reader pressed "Show images" on this message. */ + shown: boolean; +}): boolean { + if (opts.shown || opts.policy === "always") return true; + if (opts.trusted.includes((opts.from ?? "").toLowerCase())) return true; + return opts.policy === "contacts" && opts.inContacts; +} + +/** + * Put back the addresses of images that were blocked when the message was + * quoted, on the way out. + * + * Blocking keeps the original URL on the element (`data-ihm-remote`), so + * nothing was lost by not fetching it. The copy that leaves here should be the + * quote as its sender wrote it: the recipient's client decides for itself + * whether to load those images, the same as it would have with any other + * client's reply. + */ +export function restoreBlockedImages(html: string): string { + if (!html.includes("data-ihm-blocked")) return html; + const doc = new DOMParser().parseFromString(html, "text/html"); + for (const img of Array.from(doc.querySelectorAll("img[data-ihm-blocked]"))) { + const url = img.getAttribute("data-ihm-remote"); + if (url) img.setAttribute("src", url); + img.removeAttribute("data-ihm-blocked"); + img.removeAttribute("data-ihm-remote"); + } + return doc.body.innerHTML; +} diff --git a/web/src/store/__tests__/quote-image-policy.test.ts b/web/src/store/__tests__/quote-image-policy.test.ts new file mode 100644 index 0000000..38b00f9 --- /dev/null +++ b/web/src/store/__tests__/quote-image-policy.test.ts @@ -0,0 +1,91 @@ +import { beforeEach, describe, expect, it } from "vitest"; +import { buildEmailObject, useCompose } from "@/store/compose"; +import { useMail } from "@/store/mail"; +import { useContacts } from "@/store/contacts"; +import { DEFAULT_SETTINGS, useSettings } from "@/store/settings"; +import type { Email, EmailAddress, Identity } from "@/jmap/types"; + +/* + * Remote images in a quoted message (#410). + * + * Quoting renders the message a second time. The reply was fetching every + * remote image in it, whatever the reader had decided — so replying to a + * message whose images had been left blocked told the tracker the mail was + * read and the address live. The composer is a window like any other. + */ + +const PIXEL = "https://tracker.example/open.gif?id=42"; + +const MESSAGE = { + id: "m1", messageId: [""], subject: "Sale", references: [], inReplyTo: [], keywords: {}, + attachments: [], receivedAt: "2026-09-04T10:00:00Z", mailboxIds: {}, + from: [{ name: "Shop", email: "shop@example.com" }], to: [{ name: "John", email: "john@example.org" }], cc: [], + htmlBody: [{ partId: "2", type: "text/html" }], + textBody: [{ partId: "1", type: "text/plain" }], + bodyValues: { + "1": { value: "Sale on now", isEncodingProblem: false, isTruncated: false }, + "2": { value: `

Sale on now

`, isEncodingProblem: false, isTruncated: false }, + }, +} as unknown as Email; + +const IDENTITIES = [{ id: "i1", name: "John", email: "john@example.org", replyTo: null }] as unknown as Identity[]; + +function replyDraft() { + useMail.setState({ + accountId: "a1", + identities: IDENTITIES as never, + getEmails: (async () => [MESSAGE]) as never, + defaultIdentity: (() => IDENTITIES[0]) as never, + loadIdentities: (async () => IDENTITIES) as never, + roleId: (() => null) as never, + }); + return useCompose.getState().reply(MESSAGE, "reply").then((key) => useCompose.getState().drafts.find((d) => d.key === key)!); +} + +beforeEach(() => { + useCompose.setState({ drafts: [], activeKey: null, pendingSends: {} }); + useMail.setState({ imagesShown: {} }); + useContacts.setState({ loaded: false } as never); + useSettings.setState({ settings: { ...DEFAULT_SETTINGS, imagePolicy: "ask", composeFormat: "html" } }); +}); + +describe("quoting a message whose images were not allowed", () => { + it("does not put a fetchable address in the draft", async () => { + const d = await replyDraft(); + expect(d.html).not.toContain(PIXEL.split("?")[0]! + '"'); + expect(d.html).toContain("data-ihm-blocked"); + // The src is what the browser would fetch; nothing else in the draft is. + expect(/]+src="https:/.test(d.html)).toBe(false); + }); + + it("keeps the address, so the sent copy is the quote as it was written", async () => { + const d = await replyDraft(); + expect(d.html).toContain(PIXEL); + const email = await buildEmailObject({ ...d, to: [{ name: null, email: "shop@example.com" }] as EmailAddress[] }, { forSend: true }); + const sent = JSON.stringify(email); + expect(sent).toContain(PIXEL); + expect(sent).not.toContain("data-ihm-blocked"); + }); + + it("fetches them once the reader has shown images on that message", async () => { + useMail.setState({ imagesShown: { m1: true } }); + const d = await replyDraft(); + expect(d.html).toContain(`src="${PIXEL}"`); + expect(d.html).not.toContain("data-ihm-blocked"); + }); + + it("fetches them when the policy is to show images always", async () => { + useSettings.setState((s) => ({ settings: { ...s.settings, imagePolicy: "always" } })); + expect((await replyDraft()).html).toContain(`src="${PIXEL}"`); + }); + + it("fetches them from a sender the reader trusts", async () => { + useSettings.setState((s) => ({ settings: { ...s.settings, trustedImageSenders: ["shop@example.com"] } })); + expect((await replyDraft()).html).toContain(`src="${PIXEL}"`); + }); + + it("leaves them blocked for a stranger when the policy is contacts only", async () => { + useSettings.setState((s) => ({ settings: { ...s.settings, imagePolicy: "contacts" } })); + expect((await replyDraft()).html).toContain("data-ihm-blocked"); + }); +}); diff --git a/web/src/store/compose.ts b/web/src/store/compose.ts index 7166125..7f432dc 100644 --- a/web/src/store/compose.ts +++ b/web/src/store/compose.ts @@ -5,6 +5,7 @@ import { formatFullDate, uid } from "@/lib/format"; import { formatAddress, parseMailto, sameAddress, uniqueAddresses } from "@/lib/address"; import { escapeHtml, htmlToText, quoteText, replySubject, textToHtml } from "@/lib/text/text"; import { hasHtmlAlternative, sanitizeEmailHtml, sanitizeEditorHtml } from "@/lib/text/html"; +import { remoteImagesAllowed, restoreBlockedImages } from "@/lib/mail/remoteImages"; import { toast } from "@/ui/toast"; import { useMail, FULL_PROPS, BODY_PROPS } from "./mail"; import { useSession } from "./session"; @@ -13,6 +14,7 @@ import { formatScheduleTime, holdUntil } from "@/lib/schedule"; import { t as translate } from "@/lib/i18n"; import { BASE_PATH } from "@/lib/basePath"; import { settings } from "./settings"; +import { useContacts } from "./contacts"; import { emlFilename } from "@/lib/text/emlName"; import { fillPlaceholders, type PlaceholderContext } from "@/lib/templatePlaceholders"; import { shareBody, type SharedContent } from "@/lib/shareTarget"; @@ -167,6 +169,26 @@ function blankDraft(init: Partial = {}): Draft { }; } +/** + * Whether this message's remote images may be fetched into a composer. + * + * The same question the reader answered, asked with the same inputs: the + * policy, the trusted senders, whether the sender is a contact, and whether + * the reader pressed "Show images" on this message. + */ +function remoteImagesForMessage(email: Email): boolean { + const s = settings(); + const from = email.from?.[0]?.email; + const contacts = useContacts.getState(); + return remoteImagesAllowed({ + from, + policy: s.imagePolicy, + trusted: s.trustedImageSenders, + inContacts: Boolean(from && contacts.loaded && contacts.lookupByEmail(from)), + shown: Boolean(useMail.getState().imagesShown[email.id]), + }); +} + export function signatureBlock(identity: Identity | undefined, format: "html" | "text"): string { if (!identity) return ""; if (format === "text") return identity.textSignature ? `\n\n-- \n${identity.textSignature}` : ""; @@ -260,7 +282,7 @@ export const useCompose = create((set, get) => ({ showCc: Boolean(full.cc?.length), showBcc: Boolean(full.bcc?.length), subject: full.subject ?? "", - html: html ? sanitizeEmailHtml(html, { cidMap, allowRemote: true, dropStyleBlocks: true }).html : textToHtml(text).replace(/\n/g, "
"), + html: html ? sanitizeEmailHtml(html, { cidMap, allowRemote: remoteImagesForMessage(full), dropStyleBlocks: true }).html : textToHtml(text).replace(/\n/g, "
"), text: text || (html ? htmlToText(html) : ""), format: html ? "html" : settings().composeFormat, attachments, @@ -326,7 +348,7 @@ export const useCompose = create((set, get) => ({ showCc: Boolean(full.cc?.length), showBcc: Boolean(full.bcc?.length), subject: full.subject ?? "", - html: html ? sanitizeEmailHtml(html, { cidMap, allowRemote: true, dropStyleBlocks: true }).html : textToHtml(text).replace(/\n/g, "
"), + html: html ? sanitizeEmailHtml(html, { cidMap, allowRemote: remoteImagesForMessage(full), dropStyleBlocks: true }).html : textToHtml(text).replace(/\n/g, "
"), text: text || (html ? htmlToText(html) : ""), format: html ? "html" : settings().composeFormat, attachments, @@ -413,9 +435,17 @@ export const useCompose = create((set, get) => ({ attachments.push({ id: uid("a"), name: a.name ?? "attachment", type: a.type, size: a.size, blobId: a.blobId, progress: 100, error: null, cid: a.cid ?? undefined, inline }); } } + /* + * Quoting renders the message a second time, so the reader's decision + * about its remote images applies here too: a quote that fetched what + * they declined would report the message read to whoever was counting + * (#410). Blocked images keep their address and get it back on the way + * out, so the recipient's copy is the quote as its sender wrote it. + */ + const allowRemote = remoteImagesForMessage(full); // Inline images are shown via their blob URLs in the editor and converted back to cid: at send time. const quotedHtmlBody = origHtml - ? sanitizeEmailHtml(origHtml, { cidMap, allowRemote: true, proxyRemote: false, dropStyleBlocks: true }).html + ? sanitizeEmailHtml(origHtml, { cidMap, allowRemote, proxyRemote: false, dropStyleBlocks: true }).html : textToHtml(origText).replace(/\n/g, "
"); const fromStr = escapeHtml((full.from ?? []).map(formatAddress).join(", ")); const date = formatFullDate(full.receivedAt); @@ -781,7 +811,9 @@ export async function buildEmailObject(d: Draft, opts: { forSend: boolean; mailb if (!ident) throw new Error(translate("No sending identity available")); const from: EmailAddress = { name: ident.name || null, email: ident.email }; - let html = d.format === "html" ? d.html : ""; + // Images blocked when the message was quoted keep their address; the copy + // that leaves carries it, and the recipient's client decides for itself. + let html = d.format === "html" ? restoreBlockedImages(d.html) : ""; const text = d.format === "html" ? htmlToText(d.html) : d.text; // Inline attachments shown via blob URLs in the editor → back to cid: references. diff --git a/web/src/store/mail/index.ts b/web/src/store/mail/index.ts index e51048d..59ed65c 100644 --- a/web/src/store/mail/index.ts +++ b/web/src/store/mail/index.ts @@ -78,6 +78,7 @@ function offerArchiveFolder(retry: () => Promise): void { export const useMail = create((set, get) => ({ accountId: null, + imagesShown: {}, mailboxes: {}, mailboxState: null, mailboxesLoaded: false, @@ -866,6 +867,10 @@ export const useMail = create((set, get) => ({ } }, + showImages(id) { + set((s) => ({ imagesShown: { ...s.imagesShown, [id]: true } })); + }, + select(ids, on) { set((s) => { const next = { ...s.selected }; diff --git a/web/src/store/mail/types.ts b/web/src/store/mail/types.ts index 751a677..c8a65aa 100644 --- a/web/src/store/mail/types.ts +++ b/web/src/store/mail/types.ts @@ -33,6 +33,12 @@ export interface ListState extends ListQuery { export interface MailState { accountId: Id | null; + /** + * Messages the reader pressed "Show images" on, this session. Kept here + * rather than in the message view because replying quotes the message into + * a second window, which has to honour the same decision. + */ + imagesShown: Record; mailboxes: Record; mailboxState: string | null; mailboxesLoaded: boolean; @@ -113,6 +119,8 @@ export interface MailState { saveVacation(patch: Partial): Promise; loadQuota(): Promise; + /** Remember that this message's remote images were allowed by hand. */ + showImages(id: Id): void; select(ids: Id[], on: boolean): void; clearSelection(): void; /** Refresh the per-label unread counts, in one request. */ diff --git a/web/src/views/mail/MessageView.tsx b/web/src/views/mail/MessageView.tsx index 5aa22a4..dec9298 100644 --- a/web/src/views/mail/MessageView.tsx +++ b/web/src/views/mail/MessageView.tsx @@ -17,6 +17,7 @@ import { internalDomains, isExternalSender, linkVerdict } from "@/lib/warnings"; import { spamReport, type SpamReport } from "@/lib/spamScore"; import { formatFullDate, formatListDate, formatSize } from "@/lib/format"; import { displayName, domainOf, formatAddress } from "@/lib/address"; +import { remoteImagesAllowed } from "@/lib/mail/remoteImages"; import { EMAIL_BASE_CSS, TEXT_EMAIL_CSS, hasHtmlAlternative, htmlDeclaresColors, markKeptSurfaces, sanitizeEmailHtml } from "@/lib/text/html"; import { openableInTab, previewKind } from "@/lib/preview"; // Loaded when first opened: it is not needed to show mail, and it is not small. @@ -118,7 +119,12 @@ export const MessageView = memo(function MessageView({ email: e, expanded, wasUn /* Stable, so the body's click handler keeps its identity between renders. Passing an inline arrow here is what made the handler change on every render in the first place. */ - const showImages = useCallback(() => setAllowRemote(true), []); + const showImages = useCallback(() => { + setAllowRemote(true); + // Recorded for the composer: a reply quotes this message and must not + // fetch what the reader has not agreed to (#410). + useMail.getState().showImages(e.id); + }, [e.id]); const [filterOpen, setFilterOpen] = useState(false); const moreMenu = useMenu(); const [, navigate] = useLocation(); @@ -128,7 +134,7 @@ export const MessageView = memo(function MessageView({ email: e, expanded, wasUn const from = e.from?.[0]; const senderTrusted = settings.trustedImageSenders.includes((from?.email ?? "").toLowerCase()); const inContacts = useContacts((s) => Boolean(from && s.loaded && s.lookupByEmail(from.email))); - const remoteAllowed = allowRemote || settings.imagePolicy === "always" || senderTrusted || (settings.imagePolicy === "contacts" && inContacts); + const remoteAllowed = remoteImagesAllowed({ from: from?.email, policy: settings.imagePolicy, trusted: settings.trustedImageSenders, inContacts, shown: allowRemote }); const imageProxy = useSession((s) => s.session?.ihasmail?.imageProxy ?? true); const scheduled = useScheduled((s) => s.pending[e.id]); const receipt = useMemo(() => mdnDecision(e), [e]);