Repository navigation
fix(blocks): keep feeds same-origin and sanitize inserted HTML - #197
Merged
Merged
Conversation
The feed's REST URL and entry HTML come from block markup, which an author without unfiltered_html can still set. initBlock() now honours data-rest-url only when it is same-origin with the page, and sanitizeHtml() also drops srcdoc, object/embed and javascript: URLs on top of scripts and on* handlers. Load more runs its entries through the same sanitizer as polls. Ad markup is left as served, since it is same-origin and needs the markup an ad ships with.
Same-origin isn't the same as trusted: the planted root also chooses the full data-rest-url, so the reply need not be the plugin's own KSES'd output. Route adHtml and the jump-to-latest feed (its control and entries) through sanitizeHtml() too, so every client-inserted fragment is cleaned uniformly. A provider's ad placeholder survives sanitizing; the ad's own script loads the creative. Updates the DEVELOPMENT.md note and the sanitizeHtml docblock to match.
Member
Author
|
Self-review summary — 5 rounds on this branch before handoff. Accepted:
Declined:
|
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved security gaps remain around redirects, executable URL attributes, and embedded iframes.
3 open findings
🧠 Review effort: Lite
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
Addresses PR review. sanitizeHtml() now also drops data: and vbscript: URLs in href/src, so a data:text/html iframe (which survived as an opaque-origin frame) no longer renders; KSES omits data: from its protocols too. fetchEntries() drops a reply whose final URL is not same-origin, so a same-origin data-rest-url that redirects to another origin can't feed the page — matching the check fetchLiveBlock() already makes.
Addresses PR review. The scheme check was gated to href and src, so action, formaction and SVG xlink:href could still carry a javascript: URL that runs on submit or click. Test the scheme on every attribute's value instead, so any URL-bearing attribute is covered; a normal https URL is untouched.
Self-review round 3. sanitizeHtml() now reads a URL's scheme the way the browser's parser does and removes data: only from attributes that load or navigate, so an alt text, title or Lite placeholder caption that opens with "Data:" survives; it also removes base, http-equiv meta and SVG animate/set elements. fetchEntries() drops a successful reply that isn't JSON and the jump to the live feed takes only an HTML page, so a same-origin uploaded file can't stand in for the feed. initBlock() logs a warning when it refuses an off-origin REST URL.
Self-review round 4. The sanitizeHtml docblock and the DEVELOPMENT.md note described data: removal as covering every attribute the browser loads, but it covers href, src, xlink:href, action and formaction only.
Contributor
|
Hey @miguelpeixe, good job getting this PR merged! 🎉 Now, the Please check if this PR needs to be included in the "Upcoming Changes" and "Release Notes" doc. If it doesn't, simply remove the label. If it does, please add an entry to our shared document, with screenshots and testing instructions if applicable, then remove the label. Thank you! ❤️ |
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.


The Rolling Coverage feed refreshes itself in the browser, fetching new entries from the site and inserting them into the page while readers watch. This change tightens where a feed may fetch from and what it may insert, so a feed only loads entries from the site's own API and nothing active can ride in with an entry.
What changes
The view script now ignores a feed whose data source points anywhere other than the current site, accepts only the kind of reply the site's API sends, and cleans every piece of markup it inserts before it reaches the page: entries, ad placeholders, and the swap to the latest posts alike.
With this change:
On a page served from a domain other than the site address, such as an alias domain that doesn't redirect, the feed shows its first page of entries without updating, and the browser console says why.
How to test
Technical details
A feed's REST URL comes from the block's markup, which an author without
unfiltered_htmlcan still set, and with it the server whose HTML the feed inserts. So:initBlock()honoursdata-rest-urlonly when it is same-origin with the page (isSameOrigin()), and logs a console warning when it refuses one.fetchEntries()drops a reply that a redirect carried to another origin, or a successful one that isn't JSON, so a same-origin uploaded file can't stand in for the entries route. The jump to the live feed takes only a same-origin HTML page.sanitizeHtml()on top of the server's KSES. It removes scripts,object/embed,base,meta[http-equiv], SVGanimate/set,on*handlers, andsrcdoc;javascript:andvbscript:URLs in any attribute; anddata:URLs inhref,src,xlink:href,action, andformaction. It reads a URL's scheme the way the browser's URL parser does, so an alt text that opens with "Data:" survives. A provider's ad placeholder survives too; the ad's own script loads the creative.Because of the same-origin gate, a page served from a host other than
rest_url()'s shows a static first page, even though WordPress's REST CORS headers would let it poll cross-origin. Core redirects between www and non-www, so this is limited to alias domains and proxies that nothing redirects.Verified in a headless Chromium harness against the built
view.js, before and after each change: an off-origin feed URL, a redirect to another origin, a non-JSON reply, and active content in entries and ad markup are all refused or stripped, while a legitimate same-origin entry inserts with its text, links, alt text, and ad placeholder intact.tsc --noEmitandlint-jsare clean; CI runs the PHP and JS suites on push.Self-review: five rounds (Newspack WP expert, deep (Opus 5.5), deep (opus)), no blockers fixed.
🤖 Generated with Claude Code