Skip to content

fix(sanitization): scope allowlist to the rich content path#31245

Merged
thetaPC merged 3 commits into
nextfrom
FW-7599-next
Jul 14, 2026
Merged

fix(sanitization): scope allowlist to the rich content path#31245
thetaPC merged 3 commits into
nextfrom
FW-7599-next

Conversation

@thetaPC

@thetaPC thetaPC commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Issue number: N/A


What is the current behavior?

The attribute allowlist in core/src/utils/sanitization is shared between sanitizeDOMString and sanitizeDOMTree. The rich content work broadened it from 6 attributes to the full component and SVG set plus aria-* and data-*. Because sanitizeDOMString is also used by ion-toast, ion-loading, the ion-alert message, ion-refresher-content, and ion-infinite-scroll-content, those components keep attributes that were previously stripped when innerHTMLTemplatesEnabled is on.

renderClonedContent is the only place that sanitizes content for callers that pass an HTMLElement straight to renderOptionLabel, but the surrounding cloneToVNode comment implies sanitization always happens upstream, so the call looks redundant and could be removed.

The ion-select-option disabled JSDoc states the property does not apply to interface="action-sheet", which is no longer accurate.

What is the new behavior?

  • Scope the broadened allowlist to sanitizeDOMTree (the select rich content path). sanitizeDOMString reverts to its original narrow allowlist, so sanitization output is unchanged for toast, loading, the alert message, refresher content, and infinite scroll content. The shared dangerous vector scrubbing (style removal, script scheme checks, data: URI filtering) still applies to both paths.
  • Add a regression test asserting sanitizeDOMString strips attributes that are only allowed on the rich content path.
  • Add a note above the sanitizeDOMTree call in renderClonedContent explaining it is the only sanitization pass for direct HTMLElement callers and must not be removed.
  • Remove the stale action sheet note from the ion-select-option disabled prop JSDoc, since action sheet options now honor disabled.

Does this introduce a breaking change?

  • Yes
  • No

Other information

Follow-up to the select rich content work (FW-7599) addressing review feedback. The allowlist change restores the prior sanitization behavior for the non-select consumers rather than changing it, so there is no user facing behavior change for those components.

@vercel

vercel Bot commented Jun 25, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
ionic-framework Ready Ready Preview, Comment Jul 14, 2026 5:22pm

Request Review

@thetaPC thetaPC marked this pull request as ready for review June 25, 2026 22:30
@thetaPC thetaPC requested a review from a team as a code owner June 25, 2026 22:30
@thetaPC thetaPC requested review from BenOsodrac and ShaneK and removed request for BenOsodrac June 25, 2026 22:30

@ShaneK ShaneK left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Awesome work 🎉

@thetaPC thetaPC merged commit 398b525 into next Jul 14, 2026
49 checks passed
@thetaPC thetaPC deleted the FW-7599-next branch July 14, 2026 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: core @ionic/core package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants