feat: implement HTML and URL sanitization with secure iframe and doma…#876
Merged
RUKAYAT-CODER merged 1 commit intoJun 30, 2026
Merged
Conversation
…in filtering along with associated tests and snapshots
|
@extolkom Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Contributor
|
Thank you for contributing to the project. |
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
2 participants
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.
closes #720
Summary
src/utils/sanitize.ts previously added IFRAME to DOMPurify's ADD_TAGS along with allowfullscreen and data-youtube-video to ADD_ATTR, with no restriction on the iframe's src. This allowed injection of iframes pointing to arbitrary origins, opening the door to clickjacking and cross-origin data exfiltration despite the feature only being intended for YouTube embeds.
Changes
Added a DOMPurify afterSanitizeAttributes hook that inspects every iframe[src] and validates it against an explicit domain allowlist, currently https://www.youtube-nocookie.com only.
Any iframe whose src doesn't match the allowlist has its src stripped (or the node removed), so non-YouTube iframes can no longer render.
Removed the generic allowfullscreen attribute from ADD_ATTR; it's now only permitted on iframes that pass the YouTube-nocookie validation in the hook, rather than on any iframe DOMPurify lets through.
Added a unit test asserting that an iframe with a non-YouTube src (e.g. an attacker-controlled origin) has its src stripped after sanitization.
Why
The previous allowlist trusted the tag/attribute combination without verifying the actual destination, so any HTML containing an <iframe src="https://evil.example"> with the allowed attributes would sanitize cleanly and render. Pinning to youtube-nocookie.com closes that gap while preserving the intended embed functionality.
Testing
Added/ran unit tests covering: a valid youtube-nocookie.com iframe renders unchanged with allowfullscreen intact, a non-YouTube iframe src is stripped, and a few XSS-style payloads in the src attribute are neutralized. Manually verified an existing YouTube embed in the app still renders and plays correctly.
Acceptance criteria met
✅ DOMPurify strips iframes with non-allowlisted sources
✅ YouTube nocookie embeds render correctly
✅ XSS payloads in iframe src attributes are neutralized