Bump sanitize-html from 2.17.4 to 2.17.6 - #1467
Conversation
Bumps [sanitize-html](https://github.com/apostrophecms/apostrophe/tree/HEAD/packages/sanitize-html) from 2.17.4 to 2.17.6. - [Changelog](https://github.com/apostrophecms/apostrophe/blob/main/packages/sanitize-html/CHANGELOG.md) - [Commits](https://github.com/apostrophecms/apostrophe/commits/HEAD/packages/sanitize-html) --- updated-dependencies: - dependency-name: sanitize-html dependency-version: 2.17.6 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
✅ Deploy Preview for bettervoting ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
This is causing all our tests to fail It's already been reported at apostrophecms/apostrophe#5526 |
|
I considered a lot of approaches with my AI. Ultimately we decided to mock out sanitize-html since it's new build configuration wasn't compatible with jest. That's okay since we weren't testing sanitize-html anyway, and we still have the E2E tests to catch build failures. More details below: Backend tests failing after
|
Mock sanitize-html in Jest (chosen) |
Babel-transpile the ESM chain for Jest | |
|---|---|---|
| How | packages/backend/src/__mocks__/sanitize-html.js — Jest's manual-mock convention for node_modules packages applies it automatically, no config changes |
@babel/preset-env + transformIgnorePatterns transpile htmlparser2/domhandler/domutils/dom-serializer/domelementtype/entities to CJS before Jest wraps them |
| Exercises real sanitize-html logic under test | No | Yes |
| New toolchain / deps | None | @babel/preset-env, @babel/plugin-transform-modules-commonjs, new babel.config.js |
| Maintenance burden | Only if sanitize-html's usage in our code changes | Must keep the transformIgnorePatterns package allowlist in sync if htmlparser2's dependency chain changes again |
| Test suite speed | ~9–15s | ~21–27s (measured, cold and warm cache) |
| Fidelity to prod | N/A (stubbed) | High, but not identical — prod uses Node's native require(esm) with zero transformation; this runs Babel-transpiled CJS of the same source |
Both were implemented and verified (all 22 suites / 149 tests passing under each). We went with the mock:
- No unit test in the repo asserts on
sanitize-html's actual output (it's used only for stripping/allow-listing HTML in election metadata, emails, and markdown rendering — verified via grep), so the extra fidelity of the Babel route isn't buying unit-test coverage, just cost (new toolchain, ~2x slower suite, an extra config surface to maintain). - Prod fidelity is already covered by the Playwright E2E suite, which exercises the real built app (real Node runtime, real
sanitize-html, realhtmlparser2@12). The Babel route would have been redundant fidelity at the unit-test layer, not new coverage.
⚠️ Do not "fix" this by downgrading htmlparser2
An earlier iteration of this fix pinned htmlparser2 back to the CJS-compatible v10 line via package.json overrides. This is wrong — it silently reverts part of a real XSS fix repo-wide. It happened to be harmless today only because our 3 current sanitize-html call sites (Util.ts, EmailTemplates.ts — both allowedTags: []; formatMarkdown.ts — uses sanitizeHtml.defaults.allowedTags, which doesn't include svg/math/textarea/xmp) don't hit the vulnerable precondition. That's a fragile guarantee that breaks silently the moment someone's allowedTags config changes. Flagged explicitly in CLAUDE.md so it doesn't get reintroduced.
Files changed
packages/backend/src/__mocks__/sanitize-html.js— new passthrough mock, placed alongside the repo's existing manual-mock convention (__mocks__/ServiceLocator.ts) and picked up automatically by Jest — nojest.config.jschanges neededCLAUDE.md— documented the root cause and the "don't downgrade htmlparser2" warning
(That stray jest.config.js diagnostic in your IDE is just a pre-existing lint hint unrelated to this change — jest.config.js is back to its original untouched state, not something I introduced.)
|
@ArendPeter another solution would be to update jest (we are running jest from september 2023 right now, could be reasonable to go up to 30.4). I think the problem is that the latest htmlparser is packaged differently, and the old jest gets confused by that. |
Bumps sanitize-html from 2.17.4 to 2.17.6.
Changelog
Sourced from sanitize-html's changelog.
Commits
Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot show <dependency name> ignore conditionswill show all of the ignore conditions of the specified dependency@dependabot ignore this major versionwill close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor versionwill close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependencywill close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)You can disable automated security fix PRs for this repo from the Security Alerts page.