diff --git a/components/email/email-composer.tsx b/components/email/email-composer.tsx index 4e022d29..df7543bf 100644 --- a/components/email/email-composer.tsx +++ b/components/email/email-composer.tsx @@ -11,7 +11,7 @@ import { debug } from "@/lib/debug"; import { toast } from "@/stores/toast-store"; import { useContextMenu } from "@/hooks/use-context-menu"; import { ContextMenu, ContextMenuItem, ContextMenuSeparator } from "@/components/ui/context-menu"; -import { sanitizeSignatureHtml, sanitizeEmailHtml, escapeHtml } from "@/lib/email-sanitization"; +import { sanitizeSignatureHtml, sanitizeSignatureHtmlForDisplay, sanitizeEmailHtml, escapeHtml } from "@/lib/email-sanitization"; import { buildReplySubject, buildForwardSubject } from "@/lib/subject-prefix"; import { isFilePreviewable } from "@/lib/file-preview"; import { buildQuotedHtmlBlock, serializeEditorContent } from "@/components/email/quoted-html"; @@ -823,7 +823,7 @@ export function EmailComposer({ }, [composerClient, plainTextMode, mode]); const composerSignatureHtml = signatureIdentity?.htmlSignature - ? `
${sanitizeSignatureHtml(signatureIdentity.htmlSignature)}
` + ? `
${sanitizeSignatureHtmlForDisplay(signatureIdentity.htmlSignature)}
` : signatureIdentity?.textSignature ? `
${getPlainTextSignature(signatureIdentity).replace(/&/g, '&').replace(//g, '>').replace(/\n/g, '
')}
` : ''; diff --git a/components/email/signature-block.ts b/components/email/signature-block.ts index 231b9f6a..9b0ee3ef 100644 --- a/components/email/signature-block.ts +++ b/components/email/signature-block.ts @@ -6,6 +6,23 @@ import { Node as TiptapNode, mergeAttributes } from "@tiptap/core"; // so parseHTML can recognise it on the way back in (initial content, drafts). export const SIGNATURE_BLOCK_MARKER = "data-signature-block-node"; +/** + * Force every link in the rendered signature to open in a new tab. + * + * Applied to the NodeView's DOM only, never to `attrs.html` — that attribute is + * what serializeEditorContent emits into the sent message, and the recipient's + * copy should stay exactly as the user wrote it. Without this the composer's + * signature is a set of live, target-less anchors in the main document (the + * message body gets a sandboxed iframe; this does not), so one stray click + * navigates the whole app away and takes the unsent draft with it. + */ +function forceLinksToNewTab(root: HTMLElement): void { + root.querySelectorAll("a[href]").forEach((a) => { + a.setAttribute("target", "_blank"); + a.setAttribute("rel", "noopener noreferrer"); + }); +} + /** * SignatureBlock — an atomic, NON-editable block node that carries the * *verbatim* HTML of the user's identity signature in its `html` attribute. @@ -61,6 +78,7 @@ export const SignatureBlock = TiptapNode.create({ dom.setAttribute(SIGNATURE_BLOCK_MARKER, ""); dom.className = "signature-block-island"; + // CRITICAL: render the signature inside a Shadow Root. The app's global // CSS (Tailwind preflight, .tiptap table/td rules, box-sizing resets) // would otherwise cascade INTO the signature and destroy its layout - @@ -71,7 +89,12 @@ export const SignatureBlock = TiptapNode.create({ const inner = document.createElement("div"); // Read-only: a signature is inserted/removed as a unit, not edited inline. inner.contentEditable = "false"; - inner.innerHTML = node.attrs.html || ""; + // Track what we were given, not what's in the DOM: forceLinksToNewTab + // rewrites the markup, so inner.innerHTML no longer round-trips against + // attrs.html and comparing the two would rewrite on every transaction. + let appliedHtml = node.attrs.html || ""; + inner.innerHTML = appliedHtml; + forceLinksToNewTab(inner); shadow.appendChild(inner); return { @@ -83,8 +106,11 @@ export const SignatureBlock = TiptapNode.create({ stopEvent: () => false, update: (updatedNode) => { if (updatedNode.type.name !== "signatureBlock") return false; - if (inner.innerHTML !== (updatedNode.attrs.html || "")) { - inner.innerHTML = updatedNode.attrs.html || ""; + const nextHtml = updatedNode.attrs.html || ""; + if (nextHtml !== appliedHtml) { + appliedHtml = nextHtml; + inner.innerHTML = nextHtml; + forceLinksToNewTab(inner); } return true; }, diff --git a/components/identity/identity-form.tsx b/components/identity/identity-form.tsx index 2e083225..0155b7b7 100644 --- a/components/identity/identity-form.tsx +++ b/components/identity/identity-form.tsx @@ -5,7 +5,7 @@ import { useTranslations } from 'next-intl'; import { Button } from '@/components/ui/button'; import { Input } from '@/components/ui/input'; import type { Identity, EmailAddress } from '@/lib/jmap/types'; -import { sanitizeSignatureHtml } from '@/lib/email-sanitization'; +import { sanitizeSignatureHtml, sanitizeSignatureHtmlForDisplay } from '@/lib/email-sanitization'; import { getEmailValidationError, validateEmailList } from '@/lib/validation'; // Stalwarts JMAP Identity/set caps signature fields at 2047 UTF-8 bytes @@ -305,7 +305,7 @@ export function IdentityForm({ identity, onSave, onCancel }: IdentityFormProps)
{tDisplay('preview')}
diff --git a/lib/__tests__/email-sanitization.test.ts b/lib/__tests__/email-sanitization.test.ts index 103d2dcf..910d99d1 100644 --- a/lib/__tests__/email-sanitization.test.ts +++ b/lib/__tests__/email-sanitization.test.ts @@ -3,6 +3,7 @@ import DOMPurify from 'dompurify'; import { sanitizeEmailHtml, sanitizeSignatureHtml, + sanitizeSignatureHtmlForDisplay, parseHtmlSafely, hasRichFormatting, plainTextToSafeHtml, @@ -582,6 +583,43 @@ describe('email-sanitization', () => { }); }); + describe('sanitizeSignatureHtmlForDisplay', () => { + // Signatures render into the main document (identity-form preview, composer + // block), not the sandboxed iframe, so a target-less anchor navigates the + // whole app away and takes the unsaved draft/signature with it. + it('forces target=_blank and rel on signature links', () => { + const clean = sanitizeSignatureHtmlForDisplay('

Site

'); + expect(clean).toContain('target="_blank"'); + expect(clean).toContain('rel="noopener noreferrer"'); + }); + + it('overrides a target the user supplied themselves', () => { + const clean = sanitizeSignatureHtmlForDisplay('x'); + expect(clean).toContain('target="_blank"'); + expect(clean).not.toContain('_top'); + }); + + it('keeps the image restrictions of the storage sanitizer', () => { + const clean = sanitizeSignatureHtmlForDisplay( + '', + ); + expect(clean).not.toContain('insecure.example.com'); + expect(clean).toContain('https://cdn.example.com/l.png'); + }); + + it('does not leak target into the stored or sent signature', () => { + // sanitizeSignatureHtml feeds both storage and the outgoing message body. + const stored = sanitizeSignatureHtml('

Site

'); + expect(stored).toContain('href="https://example.com"'); + expect(stored).not.toContain('target='); + }); + + it('handles empty input', () => { + expect(sanitizeSignatureHtmlForDisplay('')).toBe(''); + expect(sanitizeSignatureHtmlForDisplay(' ')).toBe(''); + }); + }); + describe('sanitizePlainTextRenderedHtml', () => { // This branch renders into the main document, not the sandboxed iframe, so // an anchor that loses target="_blank" navigates the whole app away. diff --git a/lib/email-sanitization.ts b/lib/email-sanitization.ts index fe1bbff7..69afdc3d 100644 --- a/lib/email-sanitization.ts +++ b/lib/email-sanitization.ts @@ -79,26 +79,61 @@ export const SIGNATURE_SANITIZE_CONFIG = { FORBID_ATTR: ['onerror', 'onload', 'onclick', 'onmouseover'], }; +/** Drop images whose src isn't https: or a base64 raster data: URI. */ +function restrictSignatureImages(node: Element): void { + if (node.tagName !== 'IMG') return; + const src = node.getAttribute('src'); + if (!src || !/^(?:https:\/\/|data:image\/(?:png|jpe?g|gif|webp);base64,)/i.test(src)) { + node.remove(); + } +} + /** - * Sanitize HTML signature for storage and display. + * Sanitize an HTML signature for storage and for the outgoing message. * img src is restricted to https: or base64-embedded raster data: URIs * (png/jpeg/gif/webp). SVG is excluded because DOMPurify cannot inspect * bytes inside a data: URI. Images with a disallowed src are removed * entirely so they don't render as broken-image icons. + * + * Deliberately does NOT force target="_blank": what we store, and what the + * recipient receives, should stay as the user wrote it. Use + * `sanitizeSignatureHtmlForDisplay` for anything rendered in our own DOM. * @param html - User-provided HTML signature * @returns Sanitized signature (no scripts, no external resources) */ export function sanitizeSignatureHtml(html: string): string { + if (!html?.trim()) return ''; + DOMPurify.addHook('afterSanitizeAttributes', restrictSignatureImages); + try { + return DOMPurify.sanitize(html, SIGNATURE_SANITIZE_CONFIG); + } finally { + DOMPurify.removeAllHooks(); + } +} + +const SIGNATURE_DISPLAY_CONFIG = { + ...SIGNATURE_SANITIZE_CONFIG, + ALLOWED_ATTR: [...SIGNATURE_SANITIZE_CONFIG.ALLOWED_ATTR, 'target', 'rel'], +}; + +/** + * Sanitize an HTML signature for rendering inside our own DOM — the identity + * form's live preview and the composer's signature block. Both inject into the + * main document rather than the sandboxed iframe used for message bodies, so a + * link without target="_blank" navigates the whole app away, taking any unsent + * draft or unsaved signature with it. Force every anchor to open a new tab. + */ +export function sanitizeSignatureHtmlForDisplay(html: string): string { if (!html?.trim()) return ''; DOMPurify.addHook('afterSanitizeAttributes', (node) => { - if (node.tagName !== 'IMG') return; - const src = node.getAttribute('src'); - if (!src || !/^(?:https:\/\/|data:image\/(?:png|jpe?g|gif|webp);base64,)/i.test(src)) { - node.remove(); + restrictSignatureImages(node); + if (node.tagName === 'A') { + node.setAttribute('target', '_blank'); + node.setAttribute('rel', 'noopener noreferrer'); } }); try { - return DOMPurify.sanitize(html, SIGNATURE_SANITIZE_CONFIG); + return DOMPurify.sanitize(html, SIGNATURE_DISPLAY_CONFIG); } finally { DOMPurify.removeAllHooks(); }