fix(sanitize): allow trusted-host iframe embeds in post/page body content - #369
Open
asachs01 wants to merge 1 commit into
Open
fix(sanitize): allow trusted-host iframe embeds in post/page body content#369asachs01 wants to merge 1 commit into
asachs01 wants to merge 1 commit into
Conversation
…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.
4 tasks
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
1 participant
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
base.outlet'shtmlprop (the markdown-rendered post/page body) is typedrichtext, soescapeProps()sanitizes it throughRICHTEXT_CONFIG— an allowlist built for short marketing copy with noimg/video/table/iframe.markedpasses 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
richtextBodycontrol type, distinct fromrichtext, used only bybase.outlet.POST_BODY_CONFIGDOMPurify config: allowsimg/video/table/iframe, withiframescoped to a trusted-host allowlist (youtube.com,youtube-nocookie.com, subdomain-aware) via anuponSanitizeElementhook.youtube.com.evil.com,evilyoutube.comboth correctly rejected) to guard the allowlist logic itself.richtext-typed field (short-copy fields elsewhere in the module system) is unaffected — this only changes the config used bybase.outlet's body prop.Test plan
tsccleaneslintcleanHappy 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.