More SVG sanitization by cwillisf · Pull Request #513 · scratchfoundation/scratch-editor

cwillisf · 2026-04-06T21:01:43Z

This hotfix branch is intended for deployment as-is, and will be merged into develop (after resolving conflicts) as a followup

Proposed Changes

Catch more cases, including:

Hex-escaped url in attributes or <style> elements
Multiple url(...) items in one attribute, where the first one is OK and one of the later URLs is an external reference

Also, add tests for many cases that were already handled correctly.

Reason for Changes

We're seeing some cases in the wild where hex-escaped URLs are being used to add "creative" styling to the project page.

To clarify for those who are ...lurking... 👀
We know this won't block the CSS trickery itself; it just blocks the potential leakage of personal information. That's the super-urgent part of this. Turning the page blue or replacing the Scratch logo is cute and/or annoying, and we do plan to fix that, but the privacy implications of it are limited. I just have too many meetings today to safely do both in one go 😅

Test Coverage

Included.

github-actions · 2026-04-06T21:02:41Z

Test report for scratch-vm

1 files ±0 886 suites ±0 2m 3s ⏱️ +4s
1 916 tests ±0 1 916 ✅ ±0 0 💤 ±0 0 ❌ ±0
5 218 runs ±0 5 188 ✅ ±0 30 💤 ±0 0 ❌ ±0

Results for commit 629c2bd. ± Comparison against base commit 33eda2c.

♻️ This comment has been updated with latest results.

Copilot

Pull request overview

Hotfix to strengthen SVG sanitization against obfuscated/external CSS url(...) references (including CSS hex escapes), and to expand fixture-based test coverage for previously handled cases.

Changes:

Add CSS-escape decoding + css-tree AST scanning to detect external url(...) in attributes and <style> blocks.
Extend attribute sanitization to handle multiple url(...) occurrences and more SVG URI-carrying scenarios.
Add/adjust SVG fixture pairs to validate sanitization behavior across more cases.

Reviewed changes

Copilot reviewed 2 out of 12 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
packages/scratch-svg-renderer/src/sanitize-svg.js	Adds css-tree-based canonicalization and external `url()` detection for attributes and `<style>` elements.
packages/scratch-svg-renderer/package.json	Updates `css-tree` dependency to a newer major line.
package-lock.json	Locks updated dependency graph for `css-tree` upgrade/dedupe.
packages/scratch-svg-renderer/test/fixtures/css-escapes.svg	New fixture covering CSS hex-escaped `url` cases and multiple `url()` values.
packages/scratch-svg-renderer/test/fixtures/css-escapes.sanitized.svg	Expected sanitized output for CSS escape fixture.
packages/scratch-svg-renderer/test/fixtures/external-hrefs.svg	New fixture covering external `href`/`xlink:href` across more SVG elements.
packages/scratch-svg-renderer/test/fixtures/external-hrefs.sanitized.svg	Expected sanitized output for external href fixture.
packages/scratch-svg-renderer/test/fixtures/namespace-and-uri-attrs.svg	New fixture for namespace-prefixed href attributes and `xml:base` behavior.
packages/scratch-svg-renderer/test/fixtures/namespace-and-uri-attrs.sanitized.svg	Expected sanitized output for namespace/URI attribute fixture.
packages/scratch-svg-renderer/test/fixtures/blocked-elements.svg	New fixture for DOMPurify svgDisallowed elements with URI attributes.
packages/scratch-svg-renderer/test/fixtures/blocked-elements.sanitized.svg	Expected sanitized output for blocked-elements fixture.
packages/scratch-svg-renderer/test/fixtures/css-links.sanitized.svg	Updates expected `<style>` serialization after canonicalization/generation changes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Replace our hand-rolled decodeCssEscapes regex with css-tree's built-in ident.decode(). Upgrade css-tree from 1.1.3 to 3.2.1 and update Url node access for the new API (node.value instead of node.value.value).

The previous href check used `currentNode.href.baseVal` (the SVG DOM `SVGURIReference` API), which jsdom does not implement. The check was always falsy, so external href values on <image>, <feImage>, <mpath>, <pattern>, <linearGradient>, and <textPath> were never blocked. Replace with a direct getAttribute-based check in the attribute loop, alongside the existing CSS url() check.

Copilot

Pull request overview

Copilot reviewed 2 out of 12 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

github-actions · 2026-04-06T21:42:36Z

Test report for scratch-svg-renderer

1 files ±0 60 suites ±0 0s ⏱️ ±0s
129 tests +5 129 ✅ +5 0 💤 ±0 0 ❌ ±0
281 runs +5 280 ✅ +5 1 💤 ±0 0 ❌ ±0

Results for commit 629c2bd. ± Comparison against base commit 33eda2c.

♻️ This comment has been updated with latest results.

github-actions · 2026-04-06T21:42:54Z

Test report for task-herder

28 tests ±0 28 ✅ ±0 0s ⏱️ ±0s
7 suites ±0 0 💤 ±0
1 files ±0 0 ❌ ±0

Results for commit 629c2bd. ± Comparison against base commit 33eda2c.

♻️ This comment has been updated with latest results.

github-actions · 2026-04-06T21:43:03Z

Test report for scratch-render

1 files ±0 55 suites ±0 2s ⏱️ ±0s
209 tests ±0 209 ✅ ±0 0 💤 ±0 0 ❌ ±0
279 runs ±0 279 ✅ ±0 0 💤 ±0 0 ❌ ±0

Results for commit 629c2bd. ± Comparison against base commit 33eda2c.

♻️ This comment has been updated with latest results.

github-actions · 2026-04-06T21:45:13Z

Test report for scratch-gui

2 files ±0 63 suites ±0 10m 38s ⏱️ +41s
399 tests ±0 391 ✅ ±0 8 💤 ±0 0 ❌ ±0
417 runs ±0 409 ✅ ±0 8 💤 ±0 0 ❌ ±0

Results for commit 629c2bd. ± Comparison against base commit 33eda2c.

♻️ This comment has been updated with latest results.

KManolov3

Looks good! Thanks for turning this around so quickly!

Copilot

Pull request overview

Copilot reviewed 2 out of 14 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 2 out of 14 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

github-actions · 2026-04-07T18:32:49Z

Test report for scratch-media-lib-scripts

20 tests ±0 20 ✅ ±0 0s ⏱️ ±0s
1 suites ±0 0 💤 ±0
1 files ±0 0 ❌ ±0

Results for commit 629c2bd. ± Comparison against base commit 33eda2c.

♻️ This comment has been updated with latest results.

fix: catch more external url(...) references

fbea3d1

cwillisf requested a review from Copilot April 6, 2026 21:01

cwillisf requested a review from a team as a code owner April 6, 2026 21:01

Copilot started reviewing on behalf of cwillisf April 6, 2026 21:02 View session

Copilot AI reviewed Apr 6, 2026

View reviewed changes

Comment thread packages/scratch-svg-renderer/src/sanitize-svg.js Outdated

Comment thread packages/scratch-svg-renderer/src/sanitize-svg.js

Comment thread packages/scratch-svg-renderer/package.json Outdated

cwillisf added 4 commits April 6, 2026 14:13

fix: canonicalize CSS first, then sanitize

6d20ec3

fix: use css-tree 3.2.1 to decode CSS escapes

2570c39

Replace our hand-rolled decodeCssEscapes regex with css-tree's built-in ident.decode(). Upgrade css-tree from 1.1.3 to 3.2.1 and update Url node access for the new API (node.value instead of node.value.value).

test: add test cases to protect existing sanitizations

7821a42

cwillisf force-pushed the hotfix/more-svg-sanitization branch from 38d96df to 7821a42 Compare April 6, 2026 21:34

cwillisf requested a review from Copilot April 6, 2026 21:34

Copilot started reviewing on behalf of cwillisf April 6, 2026 21:35 View session

Copilot AI reviewed Apr 6, 2026

View reviewed changes

Comment thread packages/scratch-svg-renderer/src/sanitize-svg.js

KManolov3 approved these changes Apr 7, 2026

View reviewed changes

cwillisf requested a review from Copilot April 7, 2026 17:33

Copilot AI reviewed Apr 7, 2026

View reviewed changes

Comment thread packages/scratch-svg-renderer/src/sanitize-svg.js Outdated

Comment thread packages/scratch-svg-renderer/src/sanitize-svg.js Outdated

fix: sanitize external url() in CSS custom property values

9442435

cwillisf force-pushed the hotfix/more-svg-sanitization branch from 89c851d to 9442435 Compare April 7, 2026 17:41

cwillisf requested a review from Copilot April 7, 2026 17:41

Copilot started reviewing on behalf of cwillisf April 7, 2026 17:42 View session

Copilot AI reviewed Apr 7, 2026

View reviewed changes

Comment thread packages/scratch-svg-renderer/src/sanitize-svg.js

Merge branch 'develop' into hotfix/more-svg-sanitization

629c2bd

cwillisf merged commit 5dfbe9a into develop Apr 7, 2026
34 of 35 checks passed

cwillisf deleted the hotfix/more-svg-sanitization branch April 7, 2026 18:40

github-actions bot locked and limited conversation to collaborators Apr 7, 2026

Conversation

cwillisf commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

Reason for Changes

Test Coverage

Uh oh!

github-actions bot commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test report for scratch-vm

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

github-actions bot commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test report for scratch-svg-renderer

Uh oh!

github-actions bot commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test report for task-herder

Uh oh!

github-actions bot commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test report for scratch-render

Uh oh!

github-actions bot commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test report for scratch-gui

Uh oh!

KManolov3 left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

github-actions bot commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test report for scratch-media-lib-scripts

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cwillisf commented Apr 6, 2026 •

edited

Loading

github-actions bot commented Apr 6, 2026 •

edited

Loading

github-actions bot commented Apr 6, 2026 •

edited

Loading

github-actions bot commented Apr 6, 2026 •

edited

Loading

github-actions bot commented Apr 6, 2026 •

edited

Loading

github-actions bot commented Apr 6, 2026 •

edited

Loading

github-actions bot commented Apr 7, 2026 •

edited

Loading