Skip to content

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

Open
thetaPC wants to merge 2 commits into
nextfrom
FW-7599-next
Open

fix(sanitization): scope allowlist to the rich content path#31245
thetaPC wants to merge 2 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 Jun 25, 2026 10:16pm

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
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.

1 participant