fix: open signature links in a new tab instead of navigating the app away
Signatures render into the main document - the identity form's live preview and the composer's signature block - rather than the sandboxed iframe used for message bodies. SIGNATURE_SANITIZE_CONFIG allows no target attribute, so those anchors were live and target-less: one click navigated the whole app away, discarding the unsent draft or the unsaved signature with it. Add sanitizeSignatureHtmlForDisplay, which keeps the storage sanitizer's image restrictions but forces target="_blank" rel="noopener noreferrer" on every anchor, and use it at the two render sites. The composer's SignatureBlock NodeView stamps the target on its rendered DOM instead, because attrs.html is what serializeEditorContent emits into the sent message - storage and the recipient's copy stay exactly as the user wrote them.
This commit is contained in:
@@ -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
|
||||
? `<div>${sanitizeSignatureHtml(signatureIdentity.htmlSignature)}</div>`
|
||||
? `<div>${sanitizeSignatureHtmlForDisplay(signatureIdentity.htmlSignature)}</div>`
|
||||
: signatureIdentity?.textSignature
|
||||
? `<div>${getPlainTextSignature(signatureIdentity).replace(/&/g, '&').replace(/</g, '<').replace(/>/g, '>').replace(/\n/g, '<br>')}</div>`
|
||||
: '';
|
||||
|
||||
@@ -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;
|
||||
},
|
||||
|
||||
@@ -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)
|
||||
<div className="text-xs text-muted-foreground mb-1">{tDisplay('preview')}</div>
|
||||
<div
|
||||
dangerouslySetInnerHTML={{
|
||||
__html: sanitizeSignatureHtml(formData.htmlSignature)
|
||||
__html: sanitizeSignatureHtmlForDisplay(formData.htmlSignature)
|
||||
}}
|
||||
/>
|
||||
</div>
|
||||
|
||||
@@ -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('<p><a href="https://example.com">Site</a></p>');
|
||||
expect(clean).toContain('target="_blank"');
|
||||
expect(clean).toContain('rel="noopener noreferrer"');
|
||||
});
|
||||
|
||||
it('overrides a target the user supplied themselves', () => {
|
||||
const clean = sanitizeSignatureHtmlForDisplay('<a href="https://example.com" target="_top">x</a>');
|
||||
expect(clean).toContain('target="_blank"');
|
||||
expect(clean).not.toContain('_top');
|
||||
});
|
||||
|
||||
it('keeps the image restrictions of the storage sanitizer', () => {
|
||||
const clean = sanitizeSignatureHtmlForDisplay(
|
||||
'<img src="http://insecure.example.com/l.png"><img src="https://cdn.example.com/l.png">',
|
||||
);
|
||||
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('<p><a href="https://example.com">Site</a></p>');
|
||||
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.
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user