fix: keep target/rel on links in plain-text message bodies
Plain-text bodies render into the main document rather than the sandboxed iframe, so an anchor without target="_blank" navigates the whole app away instead of opening a new tab. plainTextToSafeHtml emits target and rel correctly, but sanitizePlainTextRenderedHtml stripped both back off: DOMPurify URI-tests every attribute value not on its URI-safe list, and "_blank" does not match PLAIN_TEXT_RENDERED_CONFIG's ALLOWED_URI_REGEXP. EMAIL_SANITIZE_CONFIG avoids this only because its regex carries a catch-all alternation for non-URI values. Mark target and rel as URI-safe so they survive the URI test, rather than loosening href validation.
This commit is contained in:
@@ -6,6 +6,7 @@ import {
|
|||||||
parseHtmlSafely,
|
parseHtmlSafely,
|
||||||
hasRichFormatting,
|
hasRichFormatting,
|
||||||
plainTextToSafeHtml,
|
plainTextToSafeHtml,
|
||||||
|
sanitizePlainTextRenderedHtml,
|
||||||
EMAIL_SANITIZE_CONFIG,
|
EMAIL_SANITIZE_CONFIG,
|
||||||
EMAIL_IFRAME_SANITIZE_CONFIG,
|
EMAIL_IFRAME_SANITIZE_CONFIG,
|
||||||
isExternalResourceUrl,
|
isExternalResourceUrl,
|
||||||
@@ -580,4 +581,24 @@ describe('email-sanitization', () => {
|
|||||||
expect(result).toContain('javascript:alert(1)');
|
expect(result).toContain('javascript:alert(1)');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
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.
|
||||||
|
it('preserves target and rel on links emitted by plainTextToSafeHtml', () => {
|
||||||
|
const rendered = sanitizePlainTextRenderedHtml(
|
||||||
|
plainTextToSafeHtml('see https://github.com/honzup/webmail/pull/560'),
|
||||||
|
);
|
||||||
|
expect(rendered).toContain('target="_blank"');
|
||||||
|
expect(rendered).toContain('rel="noopener noreferrer"');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('still strips dangerous schemes and tags', () => {
|
||||||
|
const rendered = sanitizePlainTextRenderedHtml(
|
||||||
|
'<a href="javascript:alert(1)" target="_blank">x</a><script>alert(1)</script>',
|
||||||
|
);
|
||||||
|
expect(rendered).not.toContain('javascript:');
|
||||||
|
expect(rendered).not.toContain('<script');
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -132,6 +132,12 @@ export function sanitizeI18nHtml(html: string): string {
|
|||||||
const PLAIN_TEXT_RENDERED_CONFIG = {
|
const PLAIN_TEXT_RENDERED_CONFIG = {
|
||||||
ALLOWED_TAGS: ['a', 'br', 'p', 'div', 'span'],
|
ALLOWED_TAGS: ['a', 'br', 'p', 'div', 'span'],
|
||||||
ALLOWED_ATTR: ['href', 'target', 'rel', 'class', 'style'],
|
ALLOWED_ATTR: ['href', 'target', 'rel', 'class', 'style'],
|
||||||
|
// DOMPurify URI-tests every attribute value not on its URI-safe list, so the
|
||||||
|
// strict ALLOWED_URI_REGEXP below would strip target="_blank" (and rel) —
|
||||||
|
// "_blank" is not a URI. This branch renders into the main document rather
|
||||||
|
// than the sandboxed iframe, so losing target turns every link into a
|
||||||
|
// whole-app navigation. Exempt the two from the URI check.
|
||||||
|
ADD_URI_SAFE_ATTR: ['target', 'rel'],
|
||||||
ALLOW_DATA_ATTR: false,
|
ALLOW_DATA_ATTR: false,
|
||||||
ALLOWED_URI_REGEXP: /^(?:https?:|mailto:|tel:|cid:|#)/i,
|
ALLOWED_URI_REGEXP: /^(?:https?:|mailto:|tel:|cid:|#)/i,
|
||||||
};
|
};
|
||||||
|
|||||||
Reference in New Issue
Block a user