fix(many): sanitize unsafe hrefs, SVG and TruncateText output, and isolate DOMPurify (GHSA-qqj4-cgcr-pcjj)#2655
Merged
Conversation
- sanitize href props and Avatar's src prop from URLs that contain
javascript: or data: with DOMPurify. InstUI will add an error to the
console in this case ([InstUI] Blocked unsafe ${tag} ${attr}:)
- sanitize InlineSVG's src prop (and <svg>) with DOMPurify
- mergeDeep util does not copy constructor or __proto__
Add webcal:, sms:, callto:, cid:, xmpp:, ftps:, feed:, and geo: to SAFE_SCHEMES. The browser path now also consults this list before delegating to DOMPurify, so schemes DOMPurify rejects by default (e.g. webcal:) are no longer dropped. This also fixes silent SSR/browser drift for sms:, callto:, cid:, xmpp:, and ftps:, which DOMPurify already allows but the SSR fallback was stripping. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…onstruction INSTUI-5032
… singleton INSTUI-5057
git-nandor
changed the title
git-nandor
marked this pull request as ready for review
July 23, 2026 15:33
|
Visual regression report✅ No changes.
Baselines come from the |
There was a problem hiding this comment.
Pull request overview
Hardens Instructure UI against XSS vectors and prototype-pollution per advisory GHSA-qqj4-cgcr-pcjj, introducing shared URL/SVG sanitization utilities, removing risky DOM patterns, and tightening deep-merge behavior across affected components/packages.
Changes:
- Adds
safeHref/safeLinkPropsto@instructure/ui-utilsand applies safer link handling across multiple components. - Sanitizes Inline SVG source via a dedicated DOMPurify instance, and removes
innerHTMLstring building from TruncateText v1/v2. - Blocks prototype-pollution keys in
mergeDeep, and hardens docs search to avoid ReDoS.
Reviewed changes
Copilot reviewed 38 out of 40 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Adds/removes dependencies to support DOMPurify usage and drop escape-html. |
| packages/ui-utils/src/safeLinkProps.ts | New helper to normalize link-like props (href + default rel). |
| packages/ui-utils/src/safeHref.ts | New URL attribute validator with DOMPurify + SSR fallback. |
| packages/ui-utils/src/mergeDeep.ts | Blocks __proto__/constructor/prototype keys to prevent prototype pollution. |
| packages/ui-utils/src/index.ts | Exports safeHref and safeLinkProps publicly. |
| packages/ui-utils/src/tests/safeLinkProps.test.tsx | Unit tests for safeLinkProps. |
| packages/ui-utils/src/tests/safeHref.test.tsx | Unit tests for safeHref (incl. singleton isolation). |
| packages/ui-utils/src/tests/mergeDeep.test.tsx | Adds prototype-pollution regression tests. |
| packages/ui-utils/package.json | Adds dompurify dependency. |
| packages/ui-truncate-text/src/TruncateText/v2/utils/truncate.ts | Replaces innerHTML string building with manual DOM node construction. |
| packages/ui-truncate-text/src/TruncateText/v1/utils/truncate.ts | Same innerHTML removal for v1 truncation logic. |
| packages/ui-truncate-text/package.json | Removes escape-html (+ types) dependency. |
| packages/ui-svg-images/src/InlineSVG/sanitizeSvg.ts | Adds SVG sanitizer using a dedicated DOMPurify instance. |
| packages/ui-svg-images/src/InlineSVG/index.tsx | Sanitizes src before parsing attributes / injecting SVG markup. |
| packages/ui-svg-images/src/InlineSVG/tests/sanitizeSvg.test.tsx | Unit tests for SVG sanitization and singleton isolation. |
| packages/ui-svg-images/src/InlineSVG/tests/InlineSVG.test.tsx | Component-level tests verifying sanitization is applied. |
| packages/ui-svg-images/package.json | Adds dompurify dependency for svg-images package. |
| packages/ui-side-nav-bar/tsconfig.build.json | Adds ui-utils TS project reference. |
| packages/ui-side-nav-bar/src/SideNavBar/v2/SideNavBarItem/index.tsx | Uses safeLinkProps for href + rel on v2 items. |
| packages/ui-side-nav-bar/src/SideNavBar/v1/SideNavBarItem/index.tsx | Uses safeLinkProps for href + rel on v1 items. |
| packages/ui-side-nav-bar/package.json | Adds @instructure/ui-utils dependency. |
| packages/ui-navigation/src/AppNav/v2/Item/index.tsx | Uses safeLinkProps for href + rel on v2 items. |
| packages/ui-navigation/src/AppNav/v1/Item/index.tsx | Uses safeLinkProps for href + rel on v1 items. |
| packages/ui-menu/src/Menu/v2/MenuItem/index.tsx | Uses safeLinkProps for href + rel on v2 items. |
| packages/ui-menu/src/Menu/v1/MenuItem/index.tsx | Uses safeLinkProps for href + rel on v1 items. |
| packages/ui-link/src/Link/v2/index.tsx | Uses safeLinkProps for href + rel in v2 Link. |
| packages/ui-link/src/Link/v1/index.tsx | Uses safeLinkProps for href + rel in v1 Link. |
| packages/ui-link/src/Link/tests/Link.test.tsx | Adds regression tests for href sanitization + rel defaulting. |
| packages/ui-file-drop/src/FileDrop/v2/index.tsx | Tracks preview blob URLs and revokes them on unmount. |
| packages/ui-file-drop/src/FileDrop/v1/index.tsx | Tracks preview blob URLs and revokes them on unmount. |
| packages/ui-buttons/src/BaseButton/v2/index.tsx | Uses safeLinkProps for href + rel in v2 BaseButton. |
| packages/ui-buttons/src/BaseButton/v1/index.tsx | Uses safeLinkProps for href + rel in v1 BaseButton. |
| packages/ui-buttons/src/BaseButton/tests/BaseButton.test.tsx | Adds tests for href sanitization + rel defaulting. |
| packages/ui-avatar/tsconfig.build.json | Adds ui-utils TS project reference. |
| packages/ui-avatar/src/Avatar/v1/styles.ts | Sanitizes avatar src and safely escapes CSS url(...) interpolation. |
| packages/ui-avatar/package.json | Adds @instructure/ui-utils dependency. |
| packages/docs/src/Nav/props.ts | Simplifies query state type for safer search. |
| packages/docs/src/Nav/index.tsx | Replaces RegExp search with lowercased substring match (ReDoS hardening). |
| packages/docs/src/CodeSandboxButton/index.tsx | Removes xlink-based SVG usage in generated icon SVG. |
| packages/docs/buildScripts/samplemedia/placeholder.svg | Removes xlink-based SVG usage in placeholder asset. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (1)
packages/ui-utils/src/safeHref.ts:80
- ssrSafeHref() uses the same limited control-character stripping as extractScheme (only tab/newline/CR). If an obfuscated scheme makes it into SSR markup, users could potentially interact with the unsafe href before hydration. Normalizing the broader ASCII control-character range here reduces the risk of scheme parsing discrepancies under SSR.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+116
to
+120
| // Pre-check our scheme allowlist before DOMPurify, since DOMPurify's default | ||
| // ALLOWED_URI_REGEXP rejects schemes like `webcal:` that we treat as safe. | ||
| const scheme = extractScheme(value) | ||
| if (scheme && SAFE_SCHEMES.includes(scheme)) return href | ||
|
|
Comment on lines
+57
to
+61
| function extractScheme(value: string): string | null { | ||
| const normalized = value.replace(/[\t\n\r]/g, '').trim() | ||
| const schemeMatch = normalized.match(/^([a-zA-Z][a-zA-Z0-9+.-]*):/) | ||
| return schemeMatch ? schemeMatch[1].toLowerCase() + ':' : null | ||
| } |
Collaborator
There was a problem hiding this comment.
Added a commit that fixes this
Comment on lines
+51
to
+57
| function safeLinkProps(input: SafeLinkInput): SafeLinkOutput { | ||
| const { target, rel, tag, attr } = input | ||
| const href = safeHref(input.href, tag, attr) | ||
| const finalRel = | ||
| target === '_blank' && rel == null ? 'noopener noreferrer' : rel | ||
| return { href, target, rel: finalRel } | ||
| } |
Comment on lines
+87
to
+95
| componentWillUnmount() { | ||
| // Release any preview blob URLs we minted; otherwise the browser pins | ||
| // each File in memory until the document goes away. | ||
| this._objectUrls.forEach((url) => window.URL.revokeObjectURL(url)) | ||
| this._objectUrls = [] | ||
| } | ||
|
|
||
| _objectUrls: string[] = [] | ||
|
|
Comment on lines
+88
to
+96
| componentWillUnmount() { | ||
| // Release any preview blob URLs we minted; otherwise the browser pins | ||
| // each File in memory until the document goes away. | ||
| this._objectUrls.forEach((url) => window.URL.revokeObjectURL(url)) | ||
| this._objectUrls = [] | ||
| } | ||
|
|
||
| _objectUrls: string[] = [] | ||
|
|
balzss
approved these changes
Jul 24, 2026
matyasf
approved these changes
Jul 24, 2026
matyasf
left a comment
matyasf
left a comment
Collaborator
There was a problem hiding this comment.
Looks good! FYI I've added a commit that also filters out some weird control characters:
const normalized = value.replace(/[\u0000-\u001F\u007F]/g, '')
| Character | Unicode | Escape |
|---|---|---|
| NUL | U+0000 | \0 |
| SOH–US | U+0001–U+001F | various control chars |
| Tab | U+0009 | \t |
| Line Feed | U+000A | \n |
| Vertical Tab | U+000B | \v |
| Form Feed | U+000C | \f |
| Carriage Return | U+000D | \r |
| Escape | U+001B | \x1B |
| DEL | U+007F | \x7F |
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.
Summary
Addresses the vulnerabilities tracked in security advisory
GHSA-qqj4-cgcr-pcjj. This hardens Instructure UI against XSS throughunsafe URLs, unsanitized SVG/HTML output, and against prototype
pollution in
mergeDeep.The work is split into four focused commits so each mitigation can be
reviewed independently.
Changes
1. Block
javascript:anddata:URLs —fix(many)@instructure/ui-utils:safeHref(value, tag, attr)— strips URLs whose scheme is notallow-listed (blocks
javascript:,data:, etc.) using DOMPurify,and logs
[InstUI] Blocked unsafe <tag> <attr>:to the console whenit rejects a value.
safeLinkProps(...)— convenience wrapper for link-like props.Link,Menu/MenuItem,AppNavitems,SideNavBaritems,BaseButtonand
FileDrop.Avatar'ssrc(and escapes it before interpolating intothe
background-imageurl(...)).sanitizeSvgto@instructure/ui-svg-imagesand sanitizesInlineSVG'ssrc/<svg>content with DOMPurify.mergeDeeputil so it no longer copiesconstructoror__proto__keys (prototype-pollution guard).2. Expand the
safeHrefscheme allow-list —fix(ui-utils)safeHrefaccepts so legitimate links(e.g.
tel:,mailto:, and other expected protocols) keep workingwhile the dangerous schemes stay blocked.
3. Replace
innerHTMLwith manual DOM construction in TruncateText —fix(ui-truncate-text,ui-scripts)— INSTUI-5032TruncateText(v1 and v2) no longer builds an HTML string and assignsit via
innerHTML. It now constructs nodes directly withdocument.createElement/setAttribute/textContent, which isinherently XSS-safe.
escape-htmldependency.4. Isolate DOMPurify config from the shared singleton —
fix(ui-utils,ui-svg-images)— INSTUI-5057so InstUI's sanitization config can't leak into (or be affected by)
other consumers of DOMPurify on the page.
New public API
@instructure/ui-utilsnow exportssafeHrefandsafeLinkProps.@instructure/ui-svg-imagesgains an internalsanitizeSvghelper.These are additive; no existing prop, export, theme variable, or default
behavior is removed or changed for valid input, so this is not a
breaking change.
Affected packages
ui-utils,ui-svg-images,ui-truncate-text,ui-avatar,ui-buttons,ui-file-drop,ui-link,ui-menu,ui-navigation,ui-side-nav-bar(plus an internal ReDoS hardening in the__docs__app search).
Testing
safeHref,safeLinkProps,sanitizeSvg, andmergeDeep.pnpm run test:vitestJira