Skip to content

fix(sanitize): allow trusted-host iframe embeds in post/page body content - #369

Open
asachs01 wants to merge 2 commits into
CoreBunch:mainfrom
wyre-technology:fix/post-body-iframe-embeds-trusted-hosts
Open

fix(sanitize): allow trusted-host iframe embeds in post/page body content#369
asachs01 wants to merge 2 commits into
CoreBunch:mainfrom
wyre-technology:fix/post-body-iframe-embeds-trusted-hosts

Conversation

@asachs01

@asachs01 asachs01 commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Summary

base.outlet's html prop (the markdown-rendered post/page body) is typed richtext, so escapeProps() sanitizes it through RICHTEXT_CONFIG — an allowlist built for short marketing copy with no img/video/table/iframe. marked passes raw HTML blocks through untouched during markdown parsing, so a pasted YouTube <iframe> (or any other embed) survives parsing and then gets silently stripped at the later sanitize step — with no error or warning surfaced to the author.

Confirmed there's no existing zero-code path around this: the Tiptap post-body editor has no video/embed insertion feature ("Add Media" opens the media-library file picker, not a URL-embed), and base.video (the module that could otherwise host a YouTube embed) is page-builder-canvas-only — it has no bridge into markdown post-body fields.

Change

  • New richtextBody control type, distinct from richtext, used only by base.outlet.
  • New POST_BODY_CONFIG DOMPurify config: allows img/video/source/figure/figcaption/table/iframe, with iframe scoped to a trusted-host allowlist (youtube.com, youtube-nocookie.com, subdomain-aware) via an uponSanitizeElement hook.
  • Lookalike-host tests included (youtube.com.evil.com, evilyoutube.com both correctly rejected) to guard the allowlist logic itself.
  • Every other richtext-typed field (short-copy fields elsewhere in the module system) is unaffected — this only changes the config used by base.outlet's body prop.

Update (per review): the initial allowlist let video through but not source, so the common editor/importer output — <video controls poster="…"><source src="…" type="video/mp4"></video> — published as a bare, unplayable <video controls>. Added source to ALLOWED_TAGS and poster/type/playsinline/loop/muted/preload to ALLOWED_ATTR, plus figure/figcaption since that's what rich-text editors wrap images in.

That review also flagged a pre-existing DOMPurify/happy-dom quirk worth documenting here for anyone bisecting old behavior: under the narrower RICHTEXT_CONFIG (no img in ALLOWED_TAGS), DOMPurify's element traversal on happy-dom skips the node immediately after each removal — so exactly one <img> per document was silently dropped and the rest survived. That's why the old symptom read as an isolated rendering glitch (one missing picture per post) rather than "images are stripped from bodies entirely." This PR's wider POST_BODY_CONFIG allowlists img outright, so the traversal quirk no longer has anything to trigger it on base.outlet bodies — no separate fix needed, just calling it out so the history makes sense.

Test plan

  • Full suite green (6613+ tests passing at the time of the latest push, no regressions — one flaky, unrelated ExportDialog fetch-mock test observed and reproduced as flaky on rerun)
  • tsc clean
  • eslint clean
  • Manually verified against a real broken post in a live deployment: iframe embed survives the write path (confirmed via API), was stripped at render time before this fix, renders correctly after.
  • New unit tests cover <video poster> + <source type>, playsinline/loop/muted/preload, and <figure>/<figcaption> preservation.

Happy to adjust the trusted-host list or expose it as a config option if that's preferred upstream — went with a hardcoded YouTube-only allowlist since that's the immediate need, but a configurable allowlist would be a small follow-up if there's appetite for embeds from other providers.

…tent

base.outlet's `html` prop (the markdown-rendered entry body every post
uses) was declared type `richtext`, so escapeProps() ran it through
DOMPurify's RICHTEXT_CONFIG — an allowlist built for short-form fields
(p/strong/em/a/ul/li) with no img, video, table, or iframe. marked
passes raw HTML blocks through untouched, so a pasted YouTube <iframe>
survives markdown parsing fine and is then silently stripped at this
later sanitize step. Reported by WYRE (2 posts with embedded YouTube
videos not rendering).

Adds a new `richtextBody` control type, used only by base.outlet's
`html` prop, sanitized via a new POST_BODY_CONFIG: the same safe
formatting tags as RICHTEXT_CONFIG plus img/video/table (what a real
post body markdown-renders to) plus iframe — scoped to a trusted-host
allowlist (youtube.com, youtube-nocookie.com, subdomain-aware) via a
DOMPurify uponSanitizeElement hook that removes the whole element (not
just the src) on any non-match, including lookalike-host attempts
(youtube.com.evil.com, evilyoutube.com — tested).

Every other richtext-typed field in the CMS is unaffected — kept as a
distinct control type rather than widening RICHTEXT_CONFIG globally,
so short-form fields elsewhere don't gain a wider attack surface than
they need.

Full suite: 6611 tests passing (0 fail), tsc clean, lint clean.
@jozef-simo

Copy link
Copy Markdown

Tried this against a Webflow migration where the post bodies are HTML, and POST_BODY_CONFIG fixes the images — but video embeds still lose their source.

The config allows video but not source or poster, so the common form:

<video controls poster="/uploads/thumb.avif">
  <source src="/uploads/clip.mp4" type="video/mp4">
</video>

publishes as a bare <video controls> — no poster, no <source>, nothing to play. Only <video src=…> survives, and that isn't what most editors and importers emit.

Suggest adding source to ALLOWED_TAGS, and poster, type, playsinline, loop, muted, preload to ALLOWED_ATTR. figure/figcaption are worth considering too — they're what rich-text editors wrap images in.

Separately, and maybe worth a line in this PR's description: with the current narrow RICHTEXT_CONFIG, DOMPurify's element traversal on happy-dom skips the node after each removal, so exactly one <img> per document is dropped and the rest survive. That's why today's behaviour reads as an odd rendering glitch — one missing picture per post — rather than "images aren't allowed in bodies at all". Once this PR widens the allowlist the symptom disappears, but anyone bisecting the old behaviour will find it puzzling.

Context: found while doing an AI-assisted migration of a ~26-post site from Webflow into Instatic, so the findings come from that codebase rather than a hand-written repro.

…t body

POST_BODY_CONFIG allowed `video` but not `source`, `poster`, `type`,
`playsinline`, `loop`, `muted`, or `preload` — the form real editors and
importers (Webflow among them) actually emit is `<video controls
poster=…><source src=… type=…></video>`, not `<video src=…>`. Without
`source` in ALLOWED_TAGS the child was dropped entirely, publishing a
bare, unplayable `<video controls>`.

Also allow `figure`/`figcaption`, which rich-text editors commonly wrap
images in.

Found via PR review on CoreBunch#369 while testing against a real Webflow
migration.
@asachs01

Copy link
Copy Markdown
Contributor Author

Good catch, thanks for testing against a real migration — that's exactly the kind of gap a hand-written repro would've missed.

Pushed a fix (d55c33a): added source to ALLOWED_TAGS and poster/type/playsinline/loop/muted/preload to ALLOWED_ATTR, plus figure/figcaption per your suggestion since that's what rich-text editors wrap images in. New tests cover the <video poster><source type> combo, the boolean/preload attrs, and figure/figcaption preservation — all green, along with the full suite, tsc, and eslint.

Also added a line to the PR description documenting the happy-dom/DOMPurify traversal quirk you flagged (the "one dropped image per post" symptom under the old narrow config) — useful context for anyone bisecting the old behavior later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants