Sanitize HTML icon options in Chips and NavOverflow - #42805
Open
aljojoby9 wants to merge 2 commits into
Open
Conversation
dismissIcon, moreIcon, and [data-bs-overflow-icon] markup were inserted via innerHTML without going through the sanitizer. moreText and menuPlacement were also interpolated into HTML string templates. Route icon HTML through a dedicated DefaultIconAllowlist, insert moreText with textContent, and build the NavOverflow toggle with DOM APIs so those values cannot break out of their slots.
The icon allowlist and sanitizer calls in Chips and NavOverflow push the gzipped min builds a few dozen bytes over the previous caps.
There was a problem hiding this comment.
Pull request overview
This PR reduces XSS risk in Bootstrap v6 by sanitizing icon HTML options in Chips and NavOverflow.
Changes:
- Add a new
DefaultIconAllowlistfor icon-related HTML sanitization. - Replace NavOverflow toggle string templates with DOM API construction, and insert
moreTextas plain text. - Add docs and unit tests for XSS-style payloads and for default SVG icon rendering.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| site/src/content/docs/getting-started/javascript.mdx | Document the icon allowlist and clarify sanitizer scope. |
| site/src/content/docs/forms/chips.mdx | Note that dismissIcon is sanitized before DOM insertion. |
| site/src/content/docs/components/nav-overflow.mdx | Note that moreText is plain text and moreIcon / markup icons are sanitized. |
| js/tests/unit/util/sanitizer.spec.js | Add tests for DefaultIconAllowlist SVG retention and XSS stripping. |
| js/tests/unit/nav-overflow.spec.js | Add tests for moreText, moreIcon, menuPlacement, and markup icon sanitization. |
| js/tests/unit/chips.spec.js | Add tests for default dismiss SVG retention and dismissIcon sanitization (JS and data API). |
| js/src/util/sanitizer.ts | Introduce DefaultIconAllowlist and expose it for docs and components. |
| js/src/nav-overflow.ts | Build the overflow toggle with DOM APIs and sanitize icon HTML before insertion. |
| js/src/chips.ts | Sanitize dismissIcon HTML before inserting it into the dismiss button. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Description
Chips and NavOverflow accept HTML for their icon options, and those values can also come from the data API. They were written straight into the DOM with
innerHTML/ string templates, so a crafteddismissIcon,moreIcon,moreText,menuPlacement, or[data-bs-overflow-icon]value could inject markup.dismissIcon,moreIcon, and[data-bs-overflow-icon]HTML through a newDefaultIconAllowlistin the sanitizer (covers the default SVGs and common icon markup)moreTextwithtextContentand setmenuPlacementwithsetAttributeChecklist