Skip to content

Security: pin transitive deps to clear all critical/high Dependabot alerts - #24

Open
jignesh-travelopia wants to merge 1 commit into
mainfrom
chore/security-fix-dependabot-alerts
Open

Security: pin transitive deps to clear all critical/high Dependabot alerts#24
jignesh-travelopia wants to merge 1 commit into
mainfrom
chore/security-fix-dependabot-alerts

Conversation

@jignesh-travelopia

Copy link
Copy Markdown
Contributor

Summary

Adds npm overrides for the transitive packages that carried known CVEs through @wordpress/scripts and @wordpress/env. Clears every critical and high alert without bumping the parent majors (the @wordpress/scripts 31 → 32 path was tested and rejected — it breaks ESLint config compatibility with the @Travelopia coding-standards plugin).

Before: 43 alerts (2 critical, 20 high, 18 moderate, 3 low)
After: 12 alerts (0 critical, 0 high, 11 moderate, 1 low) — all moderate-severity dev-tool chains, deferred to Dependabot per-package PRs

Why overrides instead of direct upgrades

The two parent packages we control (@wordpress/scripts, @wordpress/env) are already at their last working versions for this project's ESLint chain. The vulnerable packages are deeper transitive dependencies the parents pin internally and won't refresh on our timeline. npm overrides is the standard mechanism for this case.

Coverage

Severity Package Why
Critical simple-git RCE / option-parsing bypass
Critical basic-ftp Path traversal in downloadToDir()
High axios Multiple — prototype pollution, header injection, SSRF
High minimatch ReDoS — collapses 5 @typescript-eslint/* chains
High serialize-javascript RCE via RegExp.flags
High lodash, lodash-es Code injection via _.template
High fast-xml-parser DoS via entity expansion
High flatted Prototype pollution via parse()
High immutable Prototype pollution
High node-forge Signature forgery (Ed25519 / RSA-PKCS)
High svgo Billion-laughs DoS
High @babel/plugin-transform-modules-systemjs Arbitrary code generation
typescript Pinned to 5.9.x to prevent dedupe-drift to 6.x

What's still flagged (intentionally)

12 moderate alerts in @babel/runtime, webpack-dev-server, @wp-playground/*, ajv, @wordpress/components/dataviews/i18n/icons, @wordpress/env. These are dev-time chains with practical low risk — Dependabot will surface them as individual PRs, which is easier to review than a comprehensive sweep.

Maintenance plan

When the next monthly dependency refresh runs (per travelopia-wp-update), drop the overrides block, run npm install && npm audit. Anything that comes back is still needed; the rest can be removed because the parent has caught up.

Test plan

  • npm audit — 0 critical, 0 high, 12 moderate
  • npm run lint:js — clean
  • npm run lint:css — clean
  • npm run type-check — clean
  • npm run build — webpack compiles successfully
  • PHPUnit — 62 tests, 123 assertions, all passing
  • Validate on a Travelopia brand site (build pipeline, lint, dev server)

Adds npm overrides for transitive packages that bundled known CVEs
through the @wordpress/scripts and @wordpress/env tooling chain. All
overrides target the smallest semver-compatible bump that includes the
upstream fix. Direct parents are kept on their existing majors —
bumping @wordpress/scripts past 31 introduces an ESLint flat-config
regression with the @travelopia/eslint-plugin-wordpress-coding-standards
chain, so we hold there.

Coverage:
- Critical: simple-git (RCE), basic-ftp (path traversal)
- High: axios, minimatch, serialize-javascript, lodash, lodash-es,
  fast-xml-parser, flatted, immutable, node-forge, svgo,
  @babel/plugin-transform-modules-systemjs
- TypeScript pinned to 5.9.x to keep tsc warnings stable across the
  override-driven dedupe (otherwise drifts to 6.x).

12 moderate-severity alerts remain (mostly @babel/runtime regex
complexity and webpack-dev-server source-leak chains) — left to
Dependabot per-package PRs since the practical risk on dev tooling
is low.

Verification: 0 critical / 0 high after install. lint:js, lint:css,
type-check, npm run build, PHPUnit (62 tests) all clean.
@jignesh-travelopia

Copy link
Copy Markdown
Contributor Author

Field test report — Quark Expeditions

Validated on a Quark (quarkexpeditions) dx worktree, Node 24, real build pipeline.

Pipeline

Step Result
npm ci (pinned lockfile) ✅ installs clean
npm run type-check ✅ clean
npm run build ✅ webpack compiled, dist/ produced
npm run lint:js ✅ clean
npm run lint:css ✅ clean

Audit — same-time main vs this branch

critical high moderate low total
main 5 24 57 2 88
this PR 2 5 39 2 48

Real reduction. The 7 residual critical/high are all dev-tooling transitive chainsshell-quote / websocket-driver (wp-env), ws (wp-playground / @php-wasm), markdown-it / linkify-it (markdownlint), form-data, tmp. None ship in dist/ or PHP.

Caveat on the headline

The PR description's "0 critical / 0 high" no longer holds under a current npm audit (measured 2 critical + 5 high on the branch). This is new CVEs published since the PR was authored plus npm audit ≠ GitHub Dependabot (different DBs / severity mapping) — not a regression. Shipped code is unaffected; the residuals are all build-time.

Verdict

✅ LGTM as a dev-toolchain security improvement. Suggest tweaking the description to "zero critical/high in shipped code" rather than "zero alerts."

Comment thread package.json
"prettier": "npm:wp-prettier@3.0.3"
"prettier": "npm:wp-prettier@3.0.3",
"@babel/plugin-transform-modules-systemjs": "^7.27.0",
"axios": "^1.12.0",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @jignesh-travelopia does it make sense to update the main packages instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I did look at going that route first. Ran into it during dev actually — it's the reason I ended up on overrides (touched on it in the description too).

The blocker is @wordpress/scripts. Anything past 31 moves to ESLint 9/10 with the new flat config, but our @travelopia coding-standards plugin is still on the ESLint 8 / .eslintrc setup, so the build's lint step breaks the moment I bump it. Not something I can fix in this repo — it really needs a flat-config release of the coding-standards plugin first. So for now overrides felt like the safer interim call to clear the CVEs without dragging the whole ESLint setup into a rewrite.

@wordpress/env is the one exception — it's independent of the lint chain, so that one can be bumped to 11 properly. Happy to pull that out into its own PR.

That said, keen on your take — do you think it's worth tackling the ESLint flat-config migration (and pushing for the plugin update) as a separate piece of work so we can drop the overrides properly? Or hold with these until the next dependency refresh? Happy to go whichever way you'd prefer.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @jignesh-travelopia let's update our coding standards first and then update this

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.

2 participants