From 3ac2ab1d97857fbc81bd0046ef4308a167c32635 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sat, 26 Sep 2026 14:51:14 +0200 Subject: [PATCH] fix: keep remote images in a quoted reply from loading in the composer The quoted original in a reply or forward renders in the app document (a shadow root), where the viewer's iframe CSP does not apply. Opening a reply therefore loaded every remote image and background of the original, firing the trackers the viewer had just held back. The quote's node view now parks remote references in `data-bulwark-remote-*` attributes for display and restores them when edits are synced back, so the sent reply still quotes the original as written. Remote content shows when the policy is `allow` or the sender is trusted. --- .../quoted-html-remote-block.test.ts | 88 +++++++++++++++++++ components/email/email-composer.tsx | 11 ++- components/email/quoted-html.ts | 43 +++++++-- lib/email-sanitization.ts | 53 +++++++++++ 4 files changed, 187 insertions(+), 8 deletions(-) create mode 100644 components/email/__tests__/quoted-html-remote-block.test.ts diff --git a/components/email/__tests__/quoted-html-remote-block.test.ts b/components/email/__tests__/quoted-html-remote-block.test.ts new file mode 100644 index 000000000..5b3ec0eed --- /dev/null +++ b/components/email/__tests__/quoted-html-remote-block.test.ts @@ -0,0 +1,88 @@ +import { afterEach, describe, expect, it } from 'vitest'; +import { Editor } from '@tiptap/core'; +import Document from '@tiptap/extension-document'; +import Paragraph from '@tiptap/extension-paragraph'; +import Text from '@tiptap/extension-text'; +import type { Editor as ReactEditor } from '@tiptap/react'; + +import { QuotedHtml, buildQuotedHtmlBlock, serializeEditorContent } from '../quoted-html'; +import { parkRemoteResources, restoreRemoteResources } from '@/lib/email-sanitization'; +import { useSettingsStore } from '@/stores/settings-store'; + +// The quoted original in a reply renders in the app document (a shadow +// root), where the viewer's iframe CSP does not reach, so opening a reply +// loaded every tracker the viewer had held back. + +const QUOTE = '

Hi

logo
x
'; + +function mount(html: string): Editor { + return new Editor({ extensions: [Document, Paragraph, Text, QuotedHtml], content: html }); +} + +function shadowInner(editor: Editor): HTMLElement { + const host = editor.view.dom.querySelector('.quoted-html-island') as HTMLElement; + return host.shadowRoot!.firstElementChild as HTMLElement; +} + +let editor: Editor | null = null; +afterEach(() => { + editor?.destroy(); + editor = null; + useSettingsStore.setState({ externalContentPolicy: 'ask' }); +}); + +describe('parkRemoteResources', () => { + it('parks every external reference and restores it exactly', () => { + const { html, parked } = parkRemoteResources(QUOTE); + expect(parked).toBe(true); + expect(html).not.toMatch(/\s(src|background)="https:/); + expect(html).not.toMatch(/ style="[^"]*t\.example/); + const restored = restoreRemoteResources(html); + expect(restored).toContain('src="https://t.example/pixel.gif"'); + expect(restored).toContain('background="https://t.example/bg.gif"'); + expect(restored).toContain('url(https://t.example/cell.gif)'); + expect(restored).not.toContain('data-bulwark-remote'); + }); + + it('leaves HTML without remote references untouched', () => { + const html = '

plain

'; + expect(parkRemoteResources(html)).toEqual({ html, parked: false }); + }); +}); + +describe('QuotedHtml node view', () => { + it('shows the quote without remote resources but sends it as written', () => { + editor = mount(buildQuotedHtmlBlock(QUOTE)); + const inner = shadowInner(editor); + expect(inner.innerHTML).not.toMatch(/\s(src|background)="https:/); + expect(inner.innerHTML).not.toMatch(/ style="[^"]*t\.example/); + + const sent = serializeEditorContent(editor as unknown as ReactEditor); + expect(sent).toContain('src="https://t.example/pixel.gif"'); + expect(sent).not.toContain('data-bulwark-remote'); + expect(sent).not.toContain('data-quoted-remote'); + }); + + it('keeps the parked references when the user edits inside the quote', async () => { + editor = mount(buildQuotedHtmlBlock(QUOTE)); + const inner = shadowInner(editor); + inner.querySelector('p')!.textContent = 'Hi (redacted)'; + inner.dispatchEvent(new Event('input', { bubbles: true, composed: true })); + await new Promise((resolve) => requestAnimationFrame(() => resolve(undefined))); + + const sent = serializeEditorContent(editor as unknown as ReactEditor); + expect(sent).toContain('Hi (redacted)'); + expect(sent).toContain('src="https://t.example/pixel.gif"'); + expect(sent).not.toContain('data-bulwark-remote'); + }); + + it('shows remote resources for a trusted sender or under the allow policy', () => { + editor = mount(buildQuotedHtmlBlock(QUOTE, { remoteAllowed: true })); + expect(shadowInner(editor).innerHTML).toContain('src="https://t.example/pixel.gif"'); + editor.destroy(); + + useSettingsStore.setState({ externalContentPolicy: 'allow' }); + editor = mount(buildQuotedHtmlBlock(QUOTE)); + expect(shadowInner(editor).innerHTML).toContain('src="https://t.example/pixel.gif"'); + }); +}); diff --git a/components/email/email-composer.tsx b/components/email/email-composer.tsx index a2e120ab7..0652b672f 100644 --- a/components/email/email-composer.tsx +++ b/components/email/email-composer.tsx @@ -470,6 +470,11 @@ export function EmailComposer({ separator: signatureSeparatorEnabled, }); + // Whether the quoted original may show its remote images in the editor: + // the viewer's rule, minus the per-message "load" click it doesn't know. + const { externalContentPolicy, isSenderTrusted } = useSettingsStore.getState(); + const quoteRemoteAllowed = externalContentPolicy === 'allow' || (!!from?.email && isSenderTrusted(from.email)); + // Plugin override (resolved at composer open via onBuildQuoteHeader). if (replyTo.quoteHeaderHtml !== undefined && (mode === 'reply' || mode === 'replyAll' || mode === 'forward')) { if (replyTo.htmlBody) { @@ -477,7 +482,8 @@ export function EmailComposer({ // nested tables / MJML survive 1:1 (sanitize strips scripts/styles // first; cid rewrite runs after so its data-cid markers survive). const island = buildQuotedHtmlBlock( - rewriteCidImagesForEditor(sanitizeEmailHtml(replyTo.htmlBody)) + rewriteCidImagesForEditor(sanitizeEmailHtml(replyTo.htmlBody)), + { remoteAllowed: quoteRemoteAllowed }, ); return `${prefix}${signatureBlock}
${replyTo.quoteHeaderHtml}${island}`; } @@ -503,7 +509,8 @@ export function EmailComposer({ // scripts/styles/head; cid rewrite afterwards so data-cid markers // aren't dropped by the sanitizer's ALLOW_DATA_ATTR:false. const island = buildQuotedHtmlBlock( - rewriteCidImagesForEditor(sanitizeEmailHtml(replyTo.htmlBody)) + rewriteCidImagesForEditor(sanitizeEmailHtml(replyTo.htmlBody)), + { remoteAllowed: quoteRemoteAllowed }, ); return `${prefix}${signatureBlock}
${quoteHeader}
${island}`; } diff --git a/components/email/quoted-html.ts b/components/email/quoted-html.ts index 0350e6797..95aeaf81b 100644 --- a/components/email/quoted-html.ts +++ b/components/email/quoted-html.ts @@ -5,11 +5,18 @@ import { DOMSerializer } from "@tiptap/pm/model"; import type { Editor } from "@tiptap/react"; import { buildSignatureBlock } from "@/components/email/signature-block"; +import { parkRemoteResources, restoreRemoteResources } from "@/lib/email-sanitization"; +import { useSettingsStore } from "@/stores/settings-store"; // Marker attribute that identifies the quoted-original wrapper in serialized // HTML, so parseHTML can recognise it on the way back in. export const QUOTED_HTML_MARKER = "data-quoted-html"; +// Set on the wrapper the composer builds when the quoted message may load +// remote content (policy "allow", or a trusted sender). Display hint only: +// serializeEditorContent never emits it. +const QUOTED_REMOTE_ALLOWED_ATTR = "data-quoted-remote"; + // Reusable style for the quote bar when quoting email text (like in a reply). const QUOTE_BAR_STYLE = "border-left:2px solid #c5c5c5;padding-left:12px;margin-top:8px;"; @@ -52,6 +59,11 @@ export const QuotedHtml = TiptapNode.create({ // custom serializer. renderHTML below only needs the wrapper. renderHTML: () => ({}), }, + remoteAllowed: { + default: false, + parseHTML: (el) => el.getAttribute(QUOTED_REMOTE_ALLOWED_ATTR) === "allowed", + renderHTML: () => ({}), + }, }; }, @@ -85,7 +97,21 @@ export const QuotedHtml = TiptapNode.create({ const inner = document.createElement("div"); inner.contentEditable = "true"; inner.style.cssText = "outline:none;"; - inner.innerHTML = node.attrs.html || ""; + + // The quote renders in the app document, where the viewer's iframe + // CSP does not reach: unless the quoted message may load remote + // content, its images and backgrounds are parked so opening a reply + // doesn't fire the trackers the viewer held back. Parking is + // display-only - the html attribute (what gets sent) keeps them. + const blockRemote = + !node.attrs.remoteAllowed && useSettingsStore.getState().externalContentPolicy !== "allow"; + let renderedHtml = ""; + const render = (html: string) => { + renderedHtml = html; + inner.innerHTML = blockRemote ? parkRemoteResources(html).html : html; + }; + const readBack = () => (blockRemote ? restoreRemoteResources(inner.innerHTML) : inner.innerHTML); + render(node.attrs.html || ""); shadow.appendChild(inner); // Track focus via focusin/focusout: inside a shadow root, @@ -110,8 +136,9 @@ export const QuotedHtml = TiptapNode.create({ if (typeof getPos !== "function") return; const pos = getPos(); if (pos == null) return; - const current = inner.innerHTML; + const current = readBack(); if (current === node.attrs.html) return; + renderedHtml = current; editor.view.dispatch( editor.view.state.tr .setNodeAttribute(pos, "html", current) @@ -138,8 +165,8 @@ export const QuotedHtml = TiptapNode.create({ update: (updatedNode) => { if (updatedNode.type.name !== "quotedHtml") return false; // Don't clobber the caret while the user is redacting inside. - if (!focused && inner.innerHTML !== updatedNode.attrs.html) { - inner.innerHTML = updatedNode.attrs.html || ""; + if (!focused && (updatedNode.attrs.html || "") !== renderedHtml) { + render(updatedNode.attrs.html || ""); } return true; }, @@ -194,6 +221,10 @@ export function serializeEditorContent(editor: Editor): string { * The `data-quoted-html` marker is what parseHTML keys on, so this exact form * must be what serializeEditorContent emits too (round-trip consistency). */ -export function buildQuotedHtmlBlock(sanitizedInnerHtml: string): string { - return `
${sanitizedInnerHtml}
`; +export function buildQuotedHtmlBlock( + sanitizedInnerHtml: string, + opts: { remoteAllowed?: boolean } = {}, +): string { + const remote = opts.remoteAllowed ? ` ${QUOTED_REMOTE_ALLOWED_ATTR}="allowed"` : ""; + return `
${sanitizedInnerHtml}
`; } diff --git a/lib/email-sanitization.ts b/lib/email-sanitization.ts index 27c8600ea..a0afc1a1a 100644 --- a/lib/email-sanitization.ts +++ b/lib/email-sanitization.ts @@ -645,6 +645,59 @@ export function blockExternalResourcesOnNode(node: Element): boolean { return blocked; } +const PARKED_ATTR_PREFIX = 'data-bulwark-remote-'; +const PARKABLE_URL_ATTRS = ['src', 'poster', 'background'] as const; + +/** + * Display-only counterpart of {@link blockExternalResourcesOnNode} for HTML + * that is rendered in the app document itself (the quoted original in the + * composer) and still has to be sent as written. Every external resource + * reference is parked in a `data-bulwark-remote-*` attribute so nothing + * loads, and {@link restoreRemoteResources} puts it back exactly. Parsing + * happens in an inert DOMParser document, which fetches nothing. + */ +export function parkRemoteResources(html: string): { html: string; parked: boolean } { + const doc = parseHtmlSafely(`${html}`); + let parked = false; + for (const el of Array.from(doc.body.querySelectorAll('*'))) { + for (const attr of PARKABLE_URL_ATTRS) { + const value = el.getAttribute(attr); + if (isExternalResourceUrl(value)) { + el.setAttribute(PARKED_ATTR_PREFIX + attr, value!); + el.removeAttribute(attr); + parked = true; + } + } + const srcset = el.getAttribute('srcset'); + if (srcset && srcsetHasExternalUrl(srcset)) { + el.setAttribute(PARKED_ATTR_PREFIX + 'srcset', srcset); + el.removeAttribute('srcset'); + parked = true; + } + const style = el.getAttribute('style'); + if (style && styleHasExternalUrl(style)) { + el.setAttribute(PARKED_ATTR_PREFIX + 'style', style); + el.setAttribute('style', stripExternalCssUrls(style)); + parked = true; + } + } + return { html: parked ? doc.body.innerHTML : html, parked }; +} + +/** Undo {@link parkRemoteResources} so the HTML carries its original references again. */ +export function restoreRemoteResources(html: string): string { + if (!html.includes(PARKED_ATTR_PREFIX)) return html; + const doc = parseHtmlSafely(`${html}`); + for (const el of Array.from(doc.body.querySelectorAll('*'))) { + for (const attr of Array.from(el.attributes)) { + if (!attr.name.startsWith(PARKED_ATTR_PREFIX)) continue; + el.setAttribute(attr.name.slice(PARKED_ATTR_PREFIX.length), attr.value); + el.removeAttribute(attr.name); + } + } + return doc.body.innerHTML; +} + /** * Safe HTML parsing without execution * Use instead of innerHTML for detection/parsing