From d5017a211f49ef39bb3574d7c8039b52bedfd18f Mon Sep 17 00:00:00 2001 From: Stefan Hildebrandt <695494+hildebrandttk@users.noreply.github.com> Date: Sun, 28 Jun 2026 17:35:39 +0200 Subject: [PATCH] feat(email): open external links in a new tab (safely) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit External web links (http/https) in rendered email bodies open in a new browser tab with target="_blank" rel="noopener noreferrer". mailto:, tel:, and in-page #anchors keep their default behavior instead of spawning a blank tab. The plaintext render path is already handled on main by #594 (ADD_URI_SAFE_ATTR in PLAIN_TEXT_RENDERED_CONFIG), so this no longer adds its own hook there — the plaintext linkifier only ever emits http(s) anchors, so the config's declarative exemption is sufficient. This change covers the paths #594 did not: - iframe HTML render: both anchor passes (the DOMPurify hook and the post-render DOM walk in email-viewer.tsx) set target=_blank on EVERY , including mailto:/tel:. Now scoped to http(s) via the shared applyNewTabToAnchor() helper (http/https -> target+rel; mailto/tel/#/other -> strip target/rel). - sanitizeI18nHtml: the same DOMPurify strip dropped target/rel from translated links (e.g. the docs link in settings.security.not_available, target="_blank" in 19/22 locales). Keep the author's target and harden rel="noopener noreferrer". Tests: unit coverage for isHttpLinkHref / applyNewTabToAnchor / sanitizeI18nHtml plus an integration suite over the real plaintext and HTML/iframe render pipelines. The plaintext-hook-specific cases are dropped as redundant with #594. --- components/email/email-viewer.tsx | 16 ++-- .../email-link-targets.integration.test.ts | 96 +++++++++++++++++++ lib/__tests__/email-sanitization.test.ts | 82 ++++++++++++++++ lib/email-sanitization.ts | 51 +++++++++- 4 files changed, 234 insertions(+), 11 deletions(-) create mode 100644 lib/__tests__/email-link-targets.integration.test.ts diff --git a/components/email/email-viewer.tsx b/components/email/email-viewer.tsx index bc260cfd..6fee7328 100644 --- a/components/email/email-viewer.tsx +++ b/components/email/email-viewer.tsx @@ -5,7 +5,7 @@ import DOMPurify from "dompurify"; import { Email, ContactCard, Mailbox } from "@/lib/jmap/types"; import { emailExportFilename, attachmentDownloadFilename, attachmentsBundleFilename, DEFAULT_EMAIL_TEMPLATE, DEFAULT_ATTACHMENT_TEMPLATE } from "@/lib/download-filename"; import { EML_IMPORT_ACCEPT, expandImportableEmails } from "@/lib/eml-import"; -import { EMAIL_IFRAME_SANITIZE_CONFIG, blockExternalResourcesOnNode, collapseBlockedImageContainers, escapeHtml, plainTextToSafeHtml, sanitizeEmailHtml, sanitizePlainTextRenderedHtml } from "@/lib/email-sanitization"; +import { EMAIL_IFRAME_SANITIZE_CONFIG, applyNewTabToAnchor, blockExternalResourcesOnNode, collapseBlockedImageContainers, escapeHtml, plainTextToSafeHtml, sanitizeEmailHtml, sanitizePlainTextRenderedHtml } from "@/lib/email-sanitization"; import { hasMeaningfulHtmlBody } from "@/lib/signature-utils"; import { withBasePath } from "@/lib/browser-navigation"; import { Button } from "@/components/ui/button"; @@ -1662,10 +1662,8 @@ export function EmailViewer({ } } - if (node.tagName === 'A') { - node.setAttribute('target', '_blank'); - node.setAttribute('rel', 'noopener noreferrer'); - } + // http(s) links open in a new tab; other schemes keep their default. + applyNewTabToAnchor(node); // No dark mode color transforms - emails render true-to-life in iframe }); @@ -2280,11 +2278,9 @@ export function EmailViewer({ } }); - // Make links open in new tab - doc.querySelectorAll('a').forEach(a => { - a.setAttribute('target', '_blank'); - a.setAttribute('rel', 'noopener noreferrer'); - }); + // Second pass over the rendered iframe DOM (the hook above only sees + // DOMPurify's output); http(s) → new tab, other schemes left in place. + doc.querySelectorAll('a').forEach(applyNewTabToAnchor); // Plugin intercept: let plugins cancel or rewrite external links inside // the email body before navigation happens. Bound on the iframe doc so diff --git a/lib/__tests__/email-link-targets.integration.test.ts b/lib/__tests__/email-link-targets.integration.test.ts new file mode 100644 index 00000000..e94cdea4 --- /dev/null +++ b/lib/__tests__/email-link-targets.integration.test.ts @@ -0,0 +1,96 @@ +import { describe, it, expect } from 'vitest'; +import DOMPurify from 'dompurify'; +import { + EMAIL_IFRAME_SANITIZE_CONFIG, + applyNewTabToAnchor, + plainTextToSafeHtml, + sanitizePlainTextRenderedHtml, + parseHtmlSafely, +} from '../email-sanitization'; + +/** + * Regression guard for "email links open in a new tab". The at-risk links are + * generated by linkification (not in the source) and DOMPurify was silently + * stripping their target/rel, so they opened in the same tab. Drives both real + * EmailViewer pipelines end-to-end — plaintext and HTML/iframe — so it can't + * regress unnoticed. + */ + +/** Reproduce the EmailViewer iframe pipeline for an HTML body. */ +function renderIframeHtml(html: string): Document { + DOMPurify.addHook('afterSanitizeAttributes', applyNewTabToAnchor); + let clean: string; + try { + clean = DOMPurify.sanitize(html, EMAIL_IFRAME_SANITIZE_CONFIG); + } finally { + DOMPurify.removeAllHooks(); + } + const doc = parseHtmlSafely(clean); + // Post-render walk, exactly as handleIframeLoad does on the live iframe doc. + doc.querySelectorAll('a').forEach(applyNewTabToAnchor); + return doc; +} + +/** Reproduce the EmailViewer plaintext pipeline (rendered into the main DOM). */ +function renderPlaintext(text: string): Document { + return parseHtmlSafely(sanitizePlainTextRenderedHtml(plainTextToSafeHtml(text))); +} + +const findLink = (doc: Document, hrefIncludes: string): HTMLAnchorElement | undefined => + Array.from(doc.querySelectorAll('a')).find((a) => (a.getAttribute('href') || '').includes(hrefIncludes)); + +describe('email link new-tab behaviour (integration)', () => { + describe('plaintext body (links are generated, not in the source)', () => { + it('opens an http(s) URL in a new tab with noopener noreferrer', () => { + const doc = renderPlaintext('Please visit https://example.com/welcome today.'); + const link = findLink(doc, 'example.com'); + expect(link).toBeTruthy(); + expect(link!.getAttribute('href')).toBe('https://example.com/welcome'); + expect(link!.getAttribute('target')).toBe('_blank'); + expect(link!.getAttribute('rel')).toBe('noopener noreferrer'); + }); + + it('does not turn a bare email address into a new-tab link', () => { + const doc = renderPlaintext('Write to foo@bar.com for help.'); + // plaintext linkification only targets http(s) URLs, never mailto. + expect(doc.querySelectorAll('a').length).toBe(0); + }); + }); + + describe('HTML alternative that looks like plaintext (server-generated tags)', () => { + it('opens http(s) anchors in a new tab and adds noopener noreferrer', () => { + const doc = renderIframeHtml('Hi
http://example.org/page
Bye'); + const link = findLink(doc, 'example.org'); + expect(link!.getAttribute('target')).toBe('_blank'); + expect(link!.getAttribute('rel')).toBe('noopener noreferrer'); + }); + + it('does NOT add target=_blank to mailto links', () => { + const doc = renderIframeHtml('sales@example.com'); + const link = findLink(doc, 'mailto:'); + expect(link).toBeTruthy(); + expect(link!.getAttribute('target')).toBeNull(); + expect(link!.getAttribute('rel')).toBeNull(); + }); + + it('does NOT add target=_blank to in-page #anchors', () => { + const doc = renderIframeHtml('jump'); + const link = findLink(doc, '#section'); + expect(link!.getAttribute('target')).toBeNull(); + }); + + it('strips an author-supplied target=_blank from a mailto link', () => { + const doc = renderIframeHtml('x'); + const link = findLink(doc, 'mailto:'); + expect(link!.getAttribute('target')).toBeNull(); + }); + + it('handles a mixed body: http gets a new tab, mailto does not', () => { + const doc = renderIframeHtml( + 'See docs or mail us.', + ); + expect(findLink(doc, 'docs.example.com')!.getAttribute('target')).toBe('_blank'); + expect(findLink(doc, 'mailto:')!.getAttribute('target')).toBeNull(); + }); + }); +}); diff --git a/lib/__tests__/email-sanitization.test.ts b/lib/__tests__/email-sanitization.test.ts index 300591b1..ac7196b3 100644 --- a/lib/__tests__/email-sanitization.test.ts +++ b/lib/__tests__/email-sanitization.test.ts @@ -11,6 +11,9 @@ import { EMAIL_SANITIZE_CONFIG, EMAIL_IFRAME_SANITIZE_CONFIG, isExternalResourceUrl, + isHttpLinkHref, + applyNewTabToAnchor, + sanitizeI18nHtml, decodeCssEscapes, styleHasExternalUrl, stripExternalCssUrls, @@ -360,6 +363,85 @@ describe('email-sanitization', () => { }); }); + describe('isHttpLinkHref (open-in-new-tab eligibility)', () => { + it('treats http(s) and protocol-relative links as new-tab links', () => { + expect(isHttpLinkHref('https://example.com/page')).toBe(true); + expect(isHttpLinkHref('http://example.com/page')).toBe(true); + expect(isHttpLinkHref('//example.com/page')).toBe(true); + expect(isHttpLinkHref('HTTPS://EXAMPLE.COM')).toBe(true); + }); + + it('sees through obfuscated schemes (leading/embedded whitespace)', () => { + expect(isHttpLinkHref('\n\nhttps://example.com')).toBe(true); + expect(isHttpLinkHref(' \t https://example.com')).toBe(true); + expect(isHttpLinkHref('h\nttps://example.com')).toBe(true); + }); + + it('excludes mailto and other non-web schemes (must NOT open a new tab)', () => { + expect(isHttpLinkHref('mailto:someone@example.com')).toBe(false); + expect(isHttpLinkHref('mailto:someone@example.com?subject=Hi')).toBe(false); + expect(isHttpLinkHref('tel:+15551234567')).toBe(false); + expect(isHttpLinkHref('sms:+15551234567')).toBe(false); + expect(isHttpLinkHref('cid:image001@example.com')).toBe(false); + expect(isHttpLinkHref('#section')).toBe(false); + expect(isHttpLinkHref('/relative/path')).toBe(false); + expect(isHttpLinkHref('')).toBe(false); + expect(isHttpLinkHref(null)).toBe(false); + expect(isHttpLinkHref(undefined)).toBe(false); + }); + }); + + describe('applyNewTabToAnchor', () => { + const anchor = (html: string): HTMLAnchorElement => + parseHtmlSafely(html).querySelector('a')!; + + it('adds target/rel to http(s) links', () => { + const a = anchor('x'); + applyNewTabToAnchor(a); + expect(a.getAttribute('target')).toBe('_blank'); + expect(a.getAttribute('rel')).toBe('noopener noreferrer'); + }); + + it('strips target/rel from mailto links', () => { + const a = anchor('x'); + applyNewTabToAnchor(a); + expect(a.getAttribute('target')).toBeNull(); + expect(a.getAttribute('rel')).toBeNull(); + }); + + it('strips target from tel: and in-page #anchors', () => { + const tel = anchor('x'); + applyNewTabToAnchor(tel); + expect(tel.getAttribute('target')).toBeNull(); + const frag = anchor('x'); + applyNewTabToAnchor(frag); + expect(frag.getAttribute('target')).toBeNull(); + }); + + it('ignores non-anchor elements', () => { + const span = parseHtmlSafely('x').querySelector('span')!; + applyNewTabToAnchor(span); + expect(span.getAttribute('target')).toBe('_blank'); + }); + }); + + describe('sanitizeI18nHtml', () => { + it('preserves an authored target="_blank" and hardens rel (regression: DOMPurify strips target)', () => { + const out = sanitizeI18nHtml( + 'See the documentation.', + ); + expect(out).toContain('target="_blank"'); + expect(out).toContain('rel="noopener noreferrer"'); + expect(out).toContain('href="/docs/guides/account-security"'); + }); + + it('leaves links without a target untouched (no spurious new tab)', () => { + const out = sanitizeI18nHtml('Go here.'); + expect(out).toContain('href="/settings"'); + expect(out).not.toContain('target='); + }); + }); + describe('decodeCssEscapes', () => { it('decodes hex escapes (cssEscape bypass)', () => { expect(decodeCssEscapes('\\68ttp://x')).toBe('http://x'); diff --git a/lib/email-sanitization.ts b/lib/email-sanitization.ts index 81a8f5a9..3bcd75f2 100644 --- a/lib/email-sanitization.ts +++ b/lib/email-sanitization.ts @@ -154,7 +154,24 @@ const I18N_SANITIZE_CONFIG = { }; export function sanitizeI18nHtml(html: string): string { - return DOMPurify.sanitize(html, I18N_SANITIZE_CONFIG); + // A custom ALLOWED_URI_REGEXP makes DOMPurify strip target/rel from trusted + // translated links (e.g. settings.security.not_available's docs link); keep + // them, and force rel on _blank to prevent tab-nabbing when the catalog omits it. + DOMPurify.addHook('uponSanitizeAttribute', (_node, data) => { + if (data.attrName === 'target' || data.attrName === 'rel') { + data.forceKeepAttr = true; + } + }); + DOMPurify.addHook('afterSanitizeAttributes', (node) => { + if (node.tagName === 'A' && node.getAttribute('target') === '_blank') { + node.setAttribute('rel', 'noopener noreferrer'); + } + }); + try { + return DOMPurify.sanitize(html, I18N_SANITIZE_CONFIG); + } finally { + DOMPurify.removeAllHooks(); + } } /** @@ -178,6 +195,8 @@ const PLAIN_TEXT_RENDERED_CONFIG = { }; export function sanitizePlainTextRenderedHtml(html: string): string { + // target/rel survive the URI check via ADD_URI_SAFE_ATTR (#594); the plaintext + // linkifier only emits http(s), so no per-scheme handling is needed here. return DOMPurify.sanitize(html, PLAIN_TEXT_RENDERED_CONFIG); } @@ -208,6 +227,36 @@ export function isExternalResourceUrl(value: string | null | undefined): boolean } +/** + * True for external web links (http/https or protocol-relative `//host`) that + * should open in a new tab — unlike `mailto:`/`tel:`/`#fragments`, which navigate + * in place or hand off to the OS handler. Strips C0 controls first so obfuscated + * schemes (`"h\ttps://x"`) don't slip through. + */ +export function isHttpLinkHref(href: string | null | undefined): boolean { + if (!href) return false; + // eslint-disable-next-line no-control-regex + const normalized = href.replace(/[\u0000-\u0020]+/g, ''); + return /^(?:https?:\/\/|\/\/)/i.test(normalized); +} + +/** + * Give one `` the new-tab treatment uniformly across the iframe render paths + * (the DOMPurify hook and the post-render DOM walk in email-viewer): http(s) + * links get target=_blank + rel; other schemes have them stripped so they don't + * spawn a blank tab. (The plaintext path relies on ADD_URI_SAFE_ATTR instead.) + */ +export function applyNewTabToAnchor(node: Element): void { + if (node.tagName !== 'A') return; + if (isHttpLinkHref(node.getAttribute('href'))) { + node.setAttribute('target', '_blank'); + node.setAttribute('rel', 'noopener noreferrer'); + } else { + node.removeAttribute('target'); + node.removeAttribute('rel'); + } +} + /** * Decode CSS escape sequences so escaped tracking URLs can be recognised. * `\68ttp://x` and `\000068ttp://x` both decode to `http://x` (the `cssEscape`