feat(email): open external links in a new tab (safely) by hildebrandttk · Pull Request #598 · bulwarkmail/webmail

hildebrandttk · 2026-07-11T16:19:27Z

Summary

External web links (http/https) in rendered email bodies now open in a new browser tab with target="_blank" rel="noopener noreferrer". mailto:, tel:, and in-page #anchors keep their default behavior (hand off to the mail client / navigate in place) instead of spawning a blank tab.

Two root causes were fixed:

The iframe HTML render path set target=_blank on EVERY , including mailto:/tel:. It is now scoped to http(s) links.
Plaintext emails (and HTML alternatives that are really plaintext) have no tags in the source - they are generated by plainTextToSafeHtml and rendered in the main DOM via sanitizePlainTextRenderedHtml. Although the generator emits target/rel, DOMPurify silently stripped them: when a custom ALLOWED_URI_REGEXP is set it validates target/rel against the regexp (which "_blank" never matches) and drops them, so those links opened in the same tab.

Changes

email-sanitization: add isHttpLinkHref() and a shared applyNewTabToAnchor() helper (http/https -> target+rel; mailto/tel/#/other -> strip target/rel).
sanitizePlainTextRenderedHtml: re-apply target/rel after sanitization via an afterSanitizeAttributes hook (the established codebase pattern).
sanitizeI18nHtml: same DOMPurify strip affected the in-app docs link in settings.security.not_available (target="_blank") - keep the translator's target and harden rel="noopener noreferrer".
email-viewer: both anchor passes (DOMPurify hook + post-render walk) now use the shared applyNewTabToAnchor() helper.
tests: unit coverage for isHttpLinkHref / applyNewTabToAnchor / sanitizeI18nHtml plus an end-to-end integration suite exercising the real plaintext and HTML/iframe render pipelines so this can't regress unnoticed.

Related Issues

Closes #

Type of Change

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation update
Refactor / code quality improvement
Chore / dependency update / CI change

Checklist

I have read the Contributing Guide
My code follows the project's code style and conventions
I have run npm run typecheck && npm run lint and there are no errors
The build passes (npm run build)
I have tested my changes locally
I have added or updated documentation if needed
I have updated translations (locales/) if my changes affect user-facing text
I have included screenshots or a screen recording for UI changes

Screenshots / Demo

Notes for Reviewers

External web links (http/https) in rendered email bodies now open in a new browser tab with target="_blank" rel="noopener noreferrer". mailto:, tel:, and in-page #anchors keep their default behavior (hand off to the mail client / navigate in place) instead of spawning a blank tab. Two root causes were fixed: 1. The iframe HTML render path set target=_blank on EVERY <a>, including mailto:/tel:. It is now scoped to http(s) links. 2. Plaintext emails (and HTML alternatives that are really plaintext) have no <a> tags in the source - they are generated by plainTextToSafeHtml and rendered in the main DOM via sanitizePlainTextRenderedHtml. Although the generator emits target/rel, DOMPurify silently stripped them: when a custom ALLOWED_URI_REGEXP is set it validates target/rel against the regexp (which "_blank" never matches) and drops them, so those links opened in the same tab. Changes: - email-sanitization: add isHttpLinkHref() and a shared applyNewTabToAnchor() helper (http/https -> target+rel; mailto/tel/#/other -> strip target/rel). - sanitizePlainTextRenderedHtml: re-apply target/rel after sanitization via an afterSanitizeAttributes hook (the established codebase pattern). - sanitizeI18nHtml: same DOMPurify strip affected the in-app docs link in settings.security.not_available (target="_blank") - keep the translator's target and harden rel="noopener noreferrer". - email-viewer: both anchor passes (DOMPurify hook + post-render walk) now use the shared applyNewTabToAnchor() helper. - tests: unit coverage for isHttpLinkHref / applyNewTabToAnchor / sanitizeI18nHtml plus an end-to-end integration suite exercising the real plaintext and HTML/iframe render pipelines so this can't regress unnoticed.

honzup · 2026-07-18T10:41:57Z

Sorry – I think we've collided. I wrote #594, which fixes the same thing and landed on main on 11 Jul at 08:45 UTC. Your branch was cut the day before (merge base 38a396d1, 10 Jul at 12:13 UTC), about twenty hours ahead of that merge, so there was no way for you to have seen it coming. It hasn't been rebased since and is now 43 commits behind. Nothing wrong with the work itself – we just hit the same wall a day apart.

The overlap. We diagnosed the identical root cause: DOMPurify URI-tests every attribute not on its URI-safe list, so a custom ALLOWED_URI_REGEXP makes it drop target/rel because _blank is not a URI. #594 fixed it declaratively in the config rather than with a hook – lib/email-sanitization.ts on current main:

const PLAIN_TEXT_RENDERED_CONFIG = {
  ALLOWED_TAGS: ['a', 'br', 'p', 'div', 'span'],
  ALLOWED_ATTR: ['href', 'target', 'rel', 'class', 'style'],
  // ...the strict ALLOWED_URI_REGEXP below would strip target="_blank" (and rel)
  // — "_blank" is not a URI. ... Exempt the two from the URI check.
  ADD_URI_SAFE_ATTR: ['target', 'rel'],
  ALLOW_DATA_ATTR: false,
  ALLOWED_URI_REGEXP: /^(?:https?:|mailto:|tel:|cid:|#)/i,
};

So the sanitizePlainTextRenderedHtml half of this PR is already fixed upstream.

GitHub now flags the branch as conflicting. Simulating the merge locally (git merge-tree main <head>) suggests the conflict is the easy part:

lib/__tests__/email-sanitization.test.ts – a real conflict. We both added cases to the same region. This is the one Git will make you resolve.
lib/email-sanitization.ts – merges cleanly, which is the part to watch. Your hook lands in the function body while ADD_URI_SAFE_ATTR sits up in the config object, so they land in different hunks and Git merges both without complaint. The result runs both mechanisms against the same problem. It won't break at runtime – the afterSanitizeAttributes hook runs last and sets or removes target/rel explicitly, so its per-scheme result is what survives and ADD_URI_SAFE_ATTR just stops mattering on that path – but you end up with two things doing one job and nothing to flag it.
components/email/email-viewer.tsx also merges cleanly, despite 43 commits of drift.

So the rebase needs a decision about how the two mechanisms divide up, not just conflict resolution. Suggestion below.

What this PR fixes that #594 didn't:

The iframe path. email-viewer.tsx sets target="_blank" on every <a> in two places – the DOMPurify hook at line 1666 and the post-render DOM walk at line 2285, both line numbers as of current main rather than this branch – with no scheme check. fix: links in plain-text bodies and signatures navigate the whole app away #594 didn't touch this at all. In real HTML mail mailto: and tel: links are common, so today they spawn a blank tab and hand off to the mail client. Your isHttpLinkHref() scoping is the right fix and I don't think anyone else is working on it.
sanitizeI18nHtml. Same DOMPurify quirk, also untouched by fix: links in plain-text bodies and signatures navigate the whole app away #594 – and I checked, it's live: I18N_SANITIZE_CONFIG sets a custom ALLOWED_URI_REGEXP but no ADD_URI_SAFE_ATTR, so target/rel are dropped there too. settings.security.not_available carries a target="_blank" documentation link and is rendered through that path at account-security-settings.tsx:832 and :835, so that link currently opens in the same tab. 19 of the 22 locale files have target= in their strings, so it isn't English-only. Good catch; I'd missed that path entirely.
Forcing rel="noopener noreferrer" even when the source omits it.

One detail that might save you some work: plainTextToSafeHtml linkifies only /(https?:\/\/[^\s<>"']+)/g, so it never emits mailto: or tel: anchors at all. The per-scheme logic isn't needed on the plaintext path – the blank-tab-on-mailto: problem lives entirely in the iframe branch. That means the duplication splits rather than needing a winner picked: ADD_URI_SAFE_ATTR for the plaintext path, applyNewTabToAnchor for the iframe walk, where the scheme actually varies.

If it helps, the smallest rebase I can see is: drop the sanitizePlainTextRenderedHtml hook and keep ADD_URI_SAFE_ATTR there, then keep isHttpLinkHref + applyNewTabToAnchor and point them at the two email-viewer.tsx sites plus sanitizeI18nHtml. That clears the duplication and shrinks the test diff, which should take most of the conflict with it.

Entirely the maintainers' call on which idiom they want, and I'm not precious about #594's approach – if the hook is preferred as the single mechanism, that's a reasonable consolidation too. Happy to help rebase, or to review, if that's welcome.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

feat(email): open external links in a new tab (safely)#598

feat(email): open external links in a new tab (safely)#598
hildebrandttk wants to merge 1 commit into
bulwarkmail:mainfrom
hildebrandttk:feat/email-links-new-tab

hildebrandttk commented Jul 11, 2026

Uh oh!

honzup commented Jul 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

hildebrandttk commented Jul 11, 2026

Summary

Changes

Related Issues

Type of Change

Checklist

Screenshots / Demo

Notes for Reviewers

Uh oh!

honzup commented Jul 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants