Skip to content

FOLIO-4474: R1-2025: dompurify ^3.3.3#3585

Closed
julianladisch wants to merge 1 commit intoR1-2025from
FOLIO-4474-R1-2025
Closed

FOLIO-4474: R1-2025: dompurify ^3.3.3#3585
julianladisch wants to merge 1 commit intoR1-2025from
FOLIO-4474-R1-2025

Conversation

@julianladisch
Copy link
Copy Markdown
Contributor

https://folio-org.atlassian.net/browse/FOLIO-4474

Bump dompurify from 3.2.3/3.2.6 to 3.3.3.

This fixes several security vulnerabilities:

https://folio-org.atlassian.net/browse/FOLIO-4474

Bump dompurify from 3.2.3/3.2.6 to 3.3.3.

This fixes several security vulnerabilities:

* GHSA-cj63-jhhr-wcxv USE_PROFILES prototype pollution allows event handlers
* GHSA-cjmm-f4jc-qw8r ADD_ATTR predicate skips URI validation
* GHSA-9p3w-6h5p-cv75 XSS via ADD_ATTR/ADD_TAGS Function Predicate State Leakage Across sanitize() Calls
* GHSA-4w3q-35jp-p934 IN_PLACE mode fails to sanitize cross-window DOM elements, allowing XSS bypass
@zburke
Copy link
Copy Markdown
Member

zburke commented Apr 10, 2026

I think this build is failing due to folio-org/jenkins-pipeline-libs#162, which is preventing sharp from being installed by favicons-webpack-plugin.

We have choices:

  1. revert PR 162. This will make builds less safe (the axios hack leveraged a post-install hook) but more flexible (favicons-webpack-plugin leverages a post-install hook to install an architecture-specific version of sharp, a highly-efficient image processor).
  2. back-port STRWEB-151 / STRWEB-151 use the given favicon without favicons-webpack-plugin stripes-webpack#179 to Sunflower-compatible releases of stripes-webpack and stripes-build.
  3. spend some Real Time looking for an alternative to favicons-webpack-plugin that provides similar functionality without the problematic post-install script.

Those are in order both from riskiest -> safest and also from least-effort -> most-effort.

I like option 2 for splitting the difference, but! A side-effect of STRWEB-151 is that the favicon files provided during a build are no longer be resized. Prior to this change, favicons-webpack-plugin would convert the image if necessary, so if you handed it a 12 megapixel photo of your cat, it would be resized down to something appropriate for a favicon. After this change, the file you provide is passed straight through, so the onus is now on you to provide something reasonable. With minimal effort, we could add a max-filesize check to prevent large files.

It may be worth having this discussion in a more public location, e.g. Slack#folio-development, if you think it would benefit from a wider audience.

@wafschneider
Copy link
Copy Markdown
Contributor

Thanks for the comments here, @zburke. I think you are right, to continue to support platform-complete for building Sunflower on Okapi, we need to resolve the favicon issue and if that requires work from teams other than the Sunflower on Okapi folks that would need to be coordinated. Probably not in the Trillium release week, however.

In any case, it is a little orthogonal to this particular PR, which appears to have a sibling in platform-lsp's PR #95. In that PR, the resolution was expressed as ^3.2.7. That is the model I followed when building the release for Sunflower CSP 6 on Okapi (#3588). PR 3588 effectively bumps the dompurify version in yarn.lock to v3.3.3.

I'll go ahead and close this PR for now since the change is duplicated in PR 3588 but we can continue the discussion of safer yarn install elsewhere. @julianladisch please feel free to reopen if I've missed something else.

@zburke
Copy link
Copy Markdown
Member

zburke commented Apr 16, 2026

@wafschneider, you're right that the --ignore-scripts conversation is orthogonal, but it has to be resolved in order for us to be able to merge this PR, or any future UI PR that resembles it whether for a CSP or security, including PR #3588. Where do you think we should have that conversation and who needs to be involved?

@wafschneider
Copy link
Copy Markdown
Contributor

@zburke we're discussing internally about doing the backport of STRWEB-151 for Sunflower using the Sunflower on Okapi team resources, raising a PR on the appropriate branch, and asking for review and then CSP approval. Does that seem like a good way forward, with minimal disruption for your team?

As usual, the actual technical work pales in comparison to the process. So it goes.

FWIW, we can merge #3588 for our purposes without passing the test, of course. The risk there is only to the Sunflower on Okapi community.

@zburke
Copy link
Copy Markdown
Member

zburke commented Apr 16, 2026

@wafschneider, I think backporting STRWEB-151 (folio-org/stripes-webpack#179 and folio-org/stripes-webpack#184) is a good plan, but it's important to note that there is the side-effect to consider: favicons won't be resized. Have we made enough noise such that relevant parties are aware and either unaffected or willing to resize any large images they have? Obviously the ideal bugfix is "fix the bug but don't otherwise change behavior", but that's the high-effort Option-3 up above, where as this is the low-effort but-whoops-a-side-effect Option-2.

If it would be helpful, I'm happy to write up the CSP and do the backport if somebody else will shepherd the CSP through. The change is small and focused so I'm not worried about the disruption.

@julianladisch julianladisch deleted the FOLIO-4474-R1-2025 branch April 17, 2026 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants