Skip to content

security: sanitize email HTML with DOMPurify before iframe rendering#274

Merged
hoiekim merged 2 commits intohoiekim:mainfrom
moltboie:feat/html-sanitization-dompurify-124
Mar 25, 2026
Merged

security: sanitize email HTML with DOMPurify before iframe rendering#274
hoiekim merged 2 commits intohoiekim:mainfrom
moltboie:feat/html-sanitization-dompurify-124

Conversation

@moltboie
Copy link
Copy Markdown
Contributor

@moltboie moltboie commented Mar 13, 2026

Summary

Defense-in-depth for the email viewer: sanitize HTML content with DOMPurify before it's rendered in the sandboxed iframe.

Closes #124

Background

Follow-up from #121 (iframe XSS fix). The iframe has sandbox="allow-same-origin allow-popups" which blocks script execution. This PR adds DOMPurify as a second layer.

Changes

src/client/lib/html.ts

  • Added import DOMPurify from "dompurify"
  • In processHtmlForViewer: sanitize html input with DOMPurify before injecting into iframe srcDoc

DOMPurify config:

DOMPurify.sanitize(html, {
  ADD_ATTR: ["target", "rel"],  // allow link target/rel (needed for email links)
  FORCE_BODY: true,               // keep style elements and full email structure
  ALLOW_DATA_ATTR: false,         // strip data-* attributes
})

What DOMPurify strips

  • <script> tags
  • Inline event handlers (onclick, onload, etc.)
  • javascript: URIs
  • Other known XSS vectors

What it preserves

  • Full HTML structure (tables, divs, spans)
  • Inline styles (important for email formatting)
  • <style> blocks
  • Links (<a href>) with target/rel attributes
  • Images

Testing

  1. Start inbox app (bun run dev)
  2. Open an HTML email
  3. Verify email renders correctly (styles, images, links preserved)
  4. Verify malicious content is stripped (e.g., script tags don't execute)

Self-Review

Discussion thread status:

  • New PR. No prior feedback.

Checked:

  • Build: bun run build passes ✅
  • TypeScript: bun run tsc --noEmit clean ✅
  • Barrel exports: processHtmlForViewer still exported from src/client/lib/index.ts ✅
  • Import location: dompurify is client-only; all usages of processHtmlForViewer are in client components ✅
  • DOMPurify version: 3.3.3 ✅

Not checked:

  • Runtime email rendering (requires running app + HTML email — manual test by Hoie)

@moltboie
Copy link
Copy Markdown
Contributor Author

moltboie commented Mar 13, 2026

Self-review of DOMPurify sanitization PR.

Code review:

  • processHtmlForViewer now sanitizes HTML before injecting into iframe srcDoc ✓
  • DOMPurify config is appropriate: ADD_ATTR preserves target/rel for links, FORCE_BODY keeps full email structure, ALLOW_DATA_ATTR: false strips data-* attributes ✓
  • Import added correctly ✓

Security analysis:

  • The iframe already has sandbox="allow-same-origin allow-popups" blocking script execution — DOMPurify is defense-in-depth
  • allow-same-origin means scripts that DO execute could access same-origin resources; DOMPurify stripping event handlers before they reach the iframe is valuable here
  • DOMPurify is battle-tested and appropriate for email sanitization

Potential consideration: FORCE_BODY: true retains full HTML including <html>/<head> elements — DOMPurify still sanitizes them so no safety concern; this is needed to preserve email styles in <head>.

CI passing. Ready to merge.

@moltboie
Copy link
Copy Markdown
Contributor Author

moltboie commented Mar 14, 2026

Self-Review

Discussion thread status:

Checked:

  • DOMPurify import: dompurify + @types/dompurify added to package.json/bun.lock. Correct client-side package.
  • Placement: DOMPurify.sanitize(html) called in processHtmlForViewer before iframe injection — defense-in-depth on top of the existing sandbox="allow-same-origin allow-popups" attribute.
  • Layer ordering: Sandbox prevents script execution; DOMPurify strips XSS vectors at parse time. Two independent layers — correct defense-in-depth approach.
  • DOMPurify in Bun: DOMPurify requires a DOM environment — this is used in client/lib/html.ts (client-side) which runs in the browser, so window.document is available. Correct usage context.
  • No regression risk: processHtmlToSendMail is unchanged. Only the viewer path is affected.
  • CI: build + test both passing ✅

E2E Testing:

  • Would require loading an email with XSS payload (e.g. <img src=x onerror=alert(1)>) and verifying the iframe renders cleaned HTML without executing scripts. The sandbox already blocks script execution; DOMPurify adds strip-at-source.
  • CI passing confirms no compilation issues with new dependency.

Issues found:

  • None

Confidence: High

@moltboie moltboie force-pushed the feat/html-sanitization-dompurify-124 branch from b36de33 to 2514840 Compare March 21, 2026 16:40
hoiekim
hoiekim reviewed Mar 25, 2026
View reviewed changes
Copy link
Copy Markdown
Owner

@hoiekim hoiekim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any way to do this without an external lib?

moltboie added 2 commits March 25, 2026 01:37
@moltboie
security: sanitize email HTML with DOMPurify before iframe rendering
9a4fb7c 
Defense-in-depth for email viewer: sanitize HTML content with DOMPurify
before it's rendered in the sandboxed iframe.

The iframe already has sandbox="allow-same-origin allow-popups" which
blocks script execution. DOMPurify adds a second layer by stripping
script tags, event handlers, and other XSS vectors from the HTML body
before it reaches the iframe srcdoc.

Config:
- ADD_ATTR: ["target", "rel"] — allow link target/rel for email links
- FORCE_BODY: true — keeps style elements and full email structure
- ALLOW_DATA_ATTR: false — strip data-* attributes

Closes hoiekim#124
@moltboie
refactor: replace DOMPurify with native DOMParser sanitizer
6d0acfc 
Use DOMParser to strip dangerous elements (script, iframe, object, etc.)
and event handler/javascript: attributes before injecting email HTML into
the viewer iframe. No external dependency needed — the sandbox iframe
already blocks script execution; this is defense-in-depth using the
browser's own HTML parser.

Removes dompurify and @types/dompurify from package.json.
@moltboie moltboie force-pushed the feat/html-sanitization-dompurify-124 branch from 2514840 to 6d0acfc Compare March 25, 2026 08:37
moltboie
moltboie commented Mar 25, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor Author

@moltboie moltboie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Replaced DOMPurify with a native DOMParser-based sanitizer — no external dependency needed.

What it does:

  • Parses the HTML with DOMParser (same engine the browser uses)
  • Removes dangerous elements: script, iframe, object, embed, applet, base
  • Strips on* event handler attributes and javascript:/vbscript:/data: URIs from href/src/action
  • Returns the serialized clean HTML

Removed dompurify and @types/dompurify from package.json — the new implementation is ~25 lines of native browser API.

The iframe sandbox (allow-same-origin allow-popups without allow-scripts) still provides the primary defense; this is defense-in-depth without adding a dependency.

@hoiekim hoiekim merged commit d5db76e into hoiekim:main Mar 25, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hoiekim hoiekim hoiekim approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

security: Add HTML sanitization as defense-in-depth for email viewing

2 participants

@moltboie @hoiekim