Sanitize HTML icon options in Chips and NavOverflow - #42805
Merged
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.
Clarified the explanation of components that can render or accept HTML, emphasizing the use of the built-in content sanitizer to prevent XSS attacks.
The icon allowlist uses the lowercased SVG attribute name that the sanitizer matches on, so cspell flagged it as unknown.
This comment was marked as low quality.
This comment was marked as low quality.
Sorry, something went wrong.
Document that viewbox is lowercased on purpose for attribute matching, and drop <use> so icon HTML cannot load external SVG fragments via href.
Author
|
Thanks for the review.
|
mdo
added a commit
that referenced
this pull request
Aug 11, 2026
The #42805 XSS specs still put data-bs-toggle on the nav itself. Point them at the wrapper markup. Raise the minified JS caps by a quarter kilobyte for the wrapper path growth.
mdo
added a commit
that referenced
this pull request
Aug 11, 2026
* Make nav overflow a wrapper around the nav The component measured and observed the same .nav that it collapsed, so each pass changed the width that produced it. Where the nav width tracked its content, the two states straddled the collapseBelow threshold and the ResizeObserver ran hundreds of times a second. That read as items that never collapse, plus flicker on load. The nav now lives inside a .nav-overflow wrapper that carries the data attribute. The component measures and observes the wrapper, which its own output cannot change. Inline-size containment on the wrapper closes the last path back in: the min-content width of the items otherwise leaks through a flex ancestor and widens the wrapper whenever the items are visible. Items no longer shrink, so the measured widths are the widths the nav really needs, and the nav gap is counted instead of a 10px fudge. Fixes #42639 * Let moreText be false for an icon-only overflow toggle An empty string already hid the label, but it left an empty element behind and took the accessible name of the button with it. false says the intent, writes no text element, and falls back to aria-label="More" so the button keeps a name. An empty string now behaves the same way. Closes #42639 * Use an English label in the custom toggle example * Fix sanitizer specs for the wrapper API and bump JS budgets The #42805 XSS specs still put data-bs-toggle on the nav itself. Point them at the wrapper markup. Raise the minified JS caps by a quarter kilobyte for the wrapper path growth. * Document the nav overflow wrapper in the migration guide
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