Skip to content

feat: [ENG-2657] reduce npm audit vulnerabilities via lockfile update#614

Merged
bao-byterover merged 1 commit into
mainfrom
feat/ENG-2657
May 6, 2026
Merged

feat: [ENG-2657] reduce npm audit vulnerabilities via lockfile update#614
bao-byterover merged 1 commit into
mainfrom
feat/ENG-2657

Conversation

@bao-byterover
Copy link
Copy Markdown
Collaborator

No description provided.

@bao-byterover bao-byterover requested a review from leehpham as a code owner May 6, 2026 09:24
@bao-byterover bao-byterover merged commit 6702b84 into main May 6, 2026
3 checks passed
@bao-byterover bao-byterover deleted the feat/ENG-2657 branch May 6, 2026 09:25
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

Claude finished @bao-byterover's task in 3m 16s —— View job


PR Review — feat: [ENG-2657] reduce npm audit vulnerabilities via lockfile update

  • Read PR diff (package-lock.json changes)
  • Verify package.json declared deps unchanged
  • Check version-bump SemVer / engine compatibility
  • Identify supply-chain & vulnerability deltas
  • Post inline comments on key concerns
  • Post summary

Summary

This is a lockfile-only change. package.json is untouched; all updates fall within existing semver ranges of transitive deps. The bumps are concentrated in two trees:

  1. vite-plugin-pwaworkbox-build 7.4.0 → 7.4.1 and its @rollup/* build chain (all dev-deps).
  2. @oclif/plugin-update (bundled) → express-rate-limit 8.3.0 → 8.5.0 (patch bump).

Net effect: deduplicates several @rollup/* plugins to top-level resolutions, drops the deprecated sourcemap-codec and unused transitive lodash, replaces ejs (workbox-build's template engine) with eta@4.6.0, and bumps serialize-javascript from 6.0.17.0.5 inside @rollup/plugin-terser (clears the known ReDoS family in v6).

Findings

praise

  • Engine alignment is clean: @rollup/plugin-terser@1.0.0 and eta@4.6.0 both require Node ≥20, which matches package.json#engines (">=20.0.0").
  • Removes deprecated sourcemap-codec and stale lodash from the workbox build tree.
  • Dedupe on @rollup/plugin-babel, @rollup/plugin-replace, @rollup/plugin-node-resolve, and rollup (workbox-build's nested rollup@2.80.0 → top-level rollup@4.x) is a real lock-graph improvement, not just a noise diff.

issue (non-blocking) — supply-chain trust delegation

  • workbox-build@7.4.1 swaps @surma/rollup-plugin-off-main-thread@2.2.3 for @trickfilm400/rollup-plugin-off-main-thread@3.0.0-pre1 — a community fork, on a -pre1 pre-release tag. Inherited from upstream, not introduced here, but it's the only new trust delegation in the diff and worth a one-time vetting. Inline comment on the relevant block.

issue (non-blocking) — missing PR description / audit evidence

  • The PR title claims "reduce npm audit vulnerabilities" but the PR has no body, no commit description, and no before/after npm audit output. Reviewers can't easily verify which advisories were actually closed. A short list (e.g., serialize-javascript<6.0.2, ejs ReDoS chain, etc.) in the description would make this easy to audit later.

nitpick — lockfile-only update, no package.json widening

  • Because no semver constraints changed, a future fresh npm install on a different mirror state could in principle re-resolve again. This is fine for a security-driven lockfile-only PR, just worth noting that the fix is "sticky" only as long as the lockfile stays. If the goal is to prevent regressing to the old vulnerable transitive versions, consider adding a top-level overrides block in package.json for the most critical pins (e.g. serialize-javascript: ^7.0.0).

question

  • Was the rollup engine bump (workbox-build's nested rollup@2.80.0 → top-level rollup@4.x) verified by an actual npm run build:ui? The lockfile change itself is mechanical, but Vite + workbox-build under rollup 4 is a non-trivial swap. CI should catch it, but worth confirming the PWA SW still generates.

Score

4/5 — Excellent

Mechanically clean, security-positive, no production runtime exposure (everything affected is dev-only or oclif-bundled). Loses one point for the missing PR description / audit evidence and the un-vetted @trickfilm400/* pre-release fork inherited from upstream.
· branch: claude/pr-614-20260506-0925

Comment thread package-lock.json
Comment on lines +8708 to +8723
"node_modules/@trickfilm400/rollup-plugin-off-main-thread": {
"version": "3.0.0-pre1",
"resolved": "https://registry.npmjs.org/@trickfilm400/rollup-plugin-off-main-thread/-/rollup-plugin-off-main-thread-3.0.0-pre1.tgz",
"integrity": "sha512-/67zpWDBLV+oYAEL682s1ktXL0HgqX76f6gaVGkGnVZlBbm1zd0v4Bz8MFF2GGhoX9rvfq3KSQHubFHwa6w6/Q==",
"dev": true,
"license": "Apache-2.0",
"dependencies": {
"ejs": "^3.1.10",
"json5": "^2.2.3",
"magic-string": "^0.30.21",
"string.prototype.matchall": "^4.0.12"
},
"engines": {
"node": ">=12"
}
},
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.

suggestion (non-blocking): Supply-chain — pre-release of a third-party fork.

This entry replaces @surma/rollup-plugin-off-main-thread with @trickfilm400/rollup-plugin-off-main-thread@3.0.0-pre1. Two flags:

  1. Fork: @trickfilm400/* is a community fork — not the original maintainer (@surma). The swap was made upstream by workbox-build@7.4.1, so we're inheriting it; but it's worth being aware of who we now transitively trust for build tooling.
  2. -pre1 pre-release: pinning a pre-release tag is fragile. Pre-releases can be unpublished or replaced more freely than stable versions. If workbox-build is the only thing pulling this in (it is), low practical risk — but worth a one-time human review of the fork's provenance.

No action required if you've already vetted this; flagging because the diff is otherwise mechanical and this line is the only meaningful trust delegation.

Comment thread package-lock.json
Comment on lines 7034 to +7043
}
}
},
"node_modules/@rollup/plugin-terser/node_modules/serialize-javascript": {
"version": "7.0.5",
"resolved": "https://registry.npmjs.org/serialize-javascript/-/serialize-javascript-7.0.5.tgz",
"integrity": "sha512-F4LcB0UqUl1zErq+1nYEEzSHJnIwb3AF2XWB94b+afhrekOUijwooAYqFyRbjYkm2PAKBabx6oYv/xDxNi8IBw==",
"dev": true,
"license": "BSD-3-Clause",
"engines": {
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.

praise: Resolves the serialize-javascript ReDoS family (CVE-2024-11831 / <6.0.2).

Bumping the nested serialize-javascript from ^6.0.17.0.5 inside @rollup/plugin-terser@1.0.0 removes the older vulnerable copy. The new @rollup/plugin-terser@1.0.0 requires Node ≥20, which matches package.json#engines (>=20.0.0), so no engine drift. Good.

Comment thread package-lock.json
"engines": {
"node": ">=20"
},
"funding": {
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.

note: Engine swap — eta replaces ejs inside workbox-build.

Both ejs and eta had ReDoS / template-injection advisories historically, but eta@4.x is the actively maintained successor and resolves the older ejs chain. Since this is a dev-only tool used by Vite-PWA's service-worker generation, runtime exposure is nil. Note that eta@4.6.0 requires Node ≥20 — matches our engines field, so no compatibility issue.

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