Skip to content

chore: resolve non-breaking client security advisories#34

Open
DibranMulder wants to merge 1 commit into
masterfrom
chore/client-advisories-non-breaking
Open

chore: resolve non-breaking client security advisories#34
DibranMulder wants to merge 1 commit into
masterfrom
chore/client-advisories-non-breaking

Conversation

@DibranMulder

Copy link
Copy Markdown

Summary

Clears the client security advisories that have fixes not requiring a major build-chain migration, by pinning two vulnerable transitive packages via overrides:

  • serialize-javascript ^7.0.5 — HIGH: RCE/XSS in terser-webpack-plugin / copy-webpack-plugin output serialization
  • tar ^7.5.16 — HIGH: path traversal / arbitrary file write in build caching (via cacache)

Impact

  • Client advisories: 65 → 60 (HIGH: 17 → 12) — all 5 cleared are HIGH severity
  • npm run build passes
  • Dependency tree gains no new invalid peers (stays at the legacy baseline)
  • No production runtime code changed — both packages are build-time only

npm audit fix is a no-op here: every remaining fixable leaf is pinned by a parent (jest 26 / css-loader 4 / webpack 4), so npm can't upgrade in place without --force (which pulls webpack 5 / wds 5). overrides is the only lever that works without the major migration.

Out of scope

The remaining 60 advisories are all dev-only build tooling (webpack 4, webpack-dev-server 3, jest 26, stylelint 13) that don't ship in the production bundle. Clearing them requires a major build-chain migration (webpack 5 / jest 30 / stylelint 17), kept out of this PR to keep the app building and tests green.

One remaining advisory does ship (@amsterdam/arm-coreuuid, moderate) but is non-exploitable here (the advisory needs a buf argument; arm-core calls uuid() with none) and can't be overridden without breaking arm-core's use of the removed uuid v3 default-export API — it needs an arm-core upgrade.

🤖 Generated with Claude Code

Clear the client advisories that have fixes not requiring a major
build-chain migration by pinning vulnerable transitive packages:

- serialize-javascript ^7.0.5 (HIGH: RCE/XSS in terser/copy-webpack-plugin
  output serialization)
- tar ^7.5.16 (HIGH: path traversal / arbitrary file write in build caching
  via cacache)

Reduces client advisories 65 -> 60 (HIGH 17 -> 12). Build passes and the
dependency tree gains no new invalid peers. The remaining advisories are all
dev-only build tooling (webpack 4, webpack-dev-server 3, jest 26, stylelint
13) that don't ship in the production bundle; clearing them requires a major
build-chain migration (webpack 5 / jest 30 / stylelint 17), out of scope here
to keep the app building and tests green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@dobby-coder dobby-coder Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two stylistic concerns on the overrides pinning — both are non-blocking but worth a look before merge:

  • serialize-javascript is pinned higher than the advisory requires; ^6.0.0 would clear it with a looser engine constraint.
  • tar jumps a major version across cacache; please confirm runtime caching paths were exercised (not just npm run build), or prefer a 6.x release if one covers the advisory.

Approach (overrides over --force) is sound given the legacy webpack 4 / jest 26 baseline; production runtime impact looks nil.

Comment thread client/package.json
"overrides": {
"trim": "^1.0.1"
"trim": "^1.0.1",
"serialize-javascript": "^7.0.5",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

serialize-javascript@^7.0.5 is more aggressive than the advisory requires. GHSA-76p7-773f-r4q5 (the relevant RCE/XSS advisory) is fixed in serialize-javascript@>=6.0.0. Pinning to ^7.0.5 adds an engines: node>=20 constraint with no extra security benefit; ^6.0.0 would clear the advisory with a smaller blast radius and looser engine requirements. The production Dockerfile uses node:lts-alpine so the bump works today, but the override has no security upside over v6.

Comment thread client/package.json
"trim": "^1.0.1"
"trim": "^1.0.1",
"serialize-javascript": "^7.0.5",
"tar": "^7.5.16"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tar@^7.5.16 is a major version bump across the only consumer here — cacache@15 declares tar@^6.0.2. tar v7 ships meaningful API/behavior changes from v6 (engines bumped to node>=18, dependency overhaul, new minipass major). Build passing does not guarantee that cacache's runtime caching paths still behave correctly under tar v7. If a 6.x release covers the advisory (e.g. ^6.2.1), prefer that; if v7 is required, please confirm cacache's caching paths were smoke-tested, not just npm run build.

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.

1 participant