Skip to content

chore(deps): override esbuild to ^0.28.1 to clear dev-only audit advisories#127

Closed
Goosterhof wants to merge 1 commit into
mainfrom
chore/esbuild-override-dev-audit
Closed

chore(deps): override esbuild to ^0.28.1 to clear dev-only audit advisories#127
Goosterhof wants to merge 1 commit into
mainfrom
chore/esbuild-override-dev-audit

Conversation

@Goosterhof

Copy link
Copy Markdown
Contributor

Splits the esbuild fix out of #120 so it can land on its own merit and un-red main independently of that PR's design discussion.

What

Adds "esbuild": "^0.28.1" to the root overrides block and regenerates package-lock.json.

Why

main currently fails CI gate 1 (audit) — three dev-only high advisories reach in transitively:

The override forces the patched version across the tree; the strict CI audit gate stays intact (no --omit=dev relaxation, no advisory allow-listing).

Verification

Check Result
npm audit 0 vulnerabilities (was 3 high)
npm ls esbuild 0.28.1 everywhere, deduped under both vite@7 and vite@8
lockfile workspace links every node_modules/@script-development/* resolves to the workspace; 0 registry-sourced copies
format:check (oxfmt) clean (override entry oxfmt-ordered)

No package versions bumped, no source touched — dependency/lockfile only.

🤖 Generated with Claude Code

…sories

GHSA-gv7w-rqvm-qjhr + GHSA-g7r4-m6w7-qqqr (high) are transitive via
vitepress -> vite -> esbuild (dev-only; `npm audit --omit=dev` was already
clean). Both are first-patched in esbuild 0.28.1. Forces the patched version
via the existing root `overrides` block; the strict CI audit gate stays intact.

`main` currently fails the audit gate since these advisories were published, so
this un-reds it independently of any feature work. Split out of #120 per review
so the design discussion there isn't coupled to a dependency fix.

Lockfile regenerated; every node_modules/@script-development/* still resolves to
the workspace (link), no nested registry copies.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Goosterhof Goosterhof requested a review from a team as a code owner June 15, 2026 09:13
@Goosterhof Goosterhof added the Agent Review Requested Requesting review of specialized AI review agents. label Jun 15, 2026
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 15, 2026

Copy link
Copy Markdown

Deploying fs-packages with  Cloudflare Pages  Cloudflare Pages

Latest commit: 7224e41
Status: ✅  Deploy successful!
Preview URL: https://1185ada7.fs-packages.pages.dev
Branch Preview URL: https://chore-esbuild-override-dev-a.fs-packages.pages.dev

View logs

@jasperboerhof

Copy link
Copy Markdown
Contributor

Review Loop · 9/10 · PASS — 🟡 1

fs-packages #127 · AC anchor: none — no issue/plan/PR-AC; dependency-override chore · head 7224e4156a

Tip

1 finding(s) posted inline — see the review comments on the changed lines.

Action

merge-ready

Comment thread package.json
"vitest": "^4"
},
"overrides": {
"esbuild": "^0.28.1",

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.

🟡 MINOR — Global esbuild override forces vitepress's pinned vite@7.3.3 onto an out-of-range esbuild (^0.27.0)

The top-level overrides.esbuild: "^0.28.1" forces every esbuild instance to 0.28.1, including vitepress/node_modules/vite@7.3.3, whose regular dependencies declares esbuild: "^0.27.0" (no ||^0.28.0).

Fix: Optional: if the docs build later breaks under esbuild 0.28, either bump vitepress (to pull a vite whose esbuild range includes 0.28) or scope the override to spare vitepress's vite. No action needed for the stated audit-clearing goal.

Evidence and reasoning

The main vite@8.0.15 peer accepts 0.28, but vite@7's docs path now runs an esbuild minor outside its declared range.

package-lock.json:9792 — node_modules/vitepress/node_modules/vite (7.3.3) declares dependencies.esbuild: "^0.27.0", which does not satisfy 0.28.1. By contrast node_modules/vite (8.0.15) has peerDependencies.esbuild: "^0.27.0 || ^0.28.0" (package-lock.json:9627), which is satisfied. esbuild ships breaking changes at minor bumps, so vite@7 is running against an esbuild it wasn't pinned to. Scope is bounded: this only affects the vitepress docs build (docs:build/docs:dev), a dev-only, local-only path that CI does NOT run (ci.yml runs audit/build/test/mutation, never docs) — and no published package depends on esbuild at runtime or peer (grep of packages/*/package.json: exit:1). npm's overrides is explicitly designed to force exactly this kind of transitive version, so this is informational, not a defect.

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.

Confirmed against the lockfile at the head SHA: node_modules/vite (8.0.15) peer is ^0.27.0 || ^0.28.0 (satisfies 0.28.1), node_modules/vitepress/node_modules/vite (7.3.3) is dependencies.esbuild: ^0.27.0 (does not). Bounded exactly as you describe — CI never runs the docs path (ci.yml runs npm audit / npm run build / test:coverage / test:mutation, no docs:build).

One forward-looking extension: the override is a caret (^0.28.1), and since esbuild ships breaking changes at minor bumps, a future npm install is free to float this to 0.29.x — which would both widen this same vite@7 mismatch and land an unvetted esbuild minor on the audit-clearing path itself. If the intent is "the minimum version that clears the advisory," a tilde (~0.28.1) or exact pin holds the audit fix stable and defers the next esbuild minor to a deliberate bump. Not blocking the stated goal.

@jasperboerhof jasperboerhof left a comment

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.

Auto-approved — review-loop verdict PASS, CI green, no human blocker, no open MAJOR+ threads. See the verdict comment + inline notes.

@Goosterhof Goosterhof left a comment

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.

⚠️ Concerns

0 blockers · 1 concern · 0 nits · 1 praise · 0 inline

Splits the esbuild fix out of #120: adds esbuild: "^0.28.1" to the root overrides block and regenerates the lockfile to force the patched esbuild across the tree, clearing the three dev-only high advisories (GHSA-gv7w-rqvm-qjhr, GHSA-g7r4-m6w7-qqqr) that reach in via vitepress → vite → esbuild. CI is green (audit + build/test:coverage/test:mutation all pass), the strict audit gate stays intact (no --omit=dev, no allow-listing), and the description's scope claims check out against the diff — package.json adds exactly one override line, no package versions bumped, no source touched, lockfile is the regeneration. The hydration/$fillable trace in the review brief is N/A: this is a dependency-only change with no model or guard surface.

Concern — override caret floats past the audit fix

The override is ^0.28.1 (caret). esbuild ships breaking changes at minor bumps, so a future npm install is free to float to 0.29.x — landing an unvetted esbuild minor on the audit-clearing path and widening the vite@7.3.3 mismatch the ally flagged. A tilde (~0.28.1) or exact pin holds the audit fix stable and defers the next esbuild minor to a deliberate bump. Detail + lockfile evidence in the thread on package.json:42. Not blocking the stated goal.

Praise

Discipline of the fix: override + lockfile regeneration with no gate relaxation, verified npm audit 0-vuln and npm ls esbuild dedup across both vite@7 and vite@8 — the audit gate stays honest rather than being silenced. Splitting it out of #120 to un-red main independently is the right call.

Automated war-room agent review — posted because this PR carries the Agent Review Requested label.

@Goosterhof

Copy link
Copy Markdown
Contributor Author

Closing as a duplicate of #126 — Jasper opened that ~10 min before this one with the identical esbuild ^0.28.1 override, and I didn't spot it when I split the fix out of #120 per the review there. #126 is green and mergeable; it's the canonical one. Apologies for the redundant branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Agent Review Requested Requesting review of specialized AI review agents.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants