chore(deps): untangle the vitest/Storybook exact-pin knot to clear GHSA-p63j-vcc4-9vmv - #1899
Conversation
…SA-p63j-vcc4-9vmv (#1839) GHSA-p63j-vcc4-9vmv (**critical**) — `@vitest/browser` Browser Mode provider commands accept a file path from the browser and act on it without checking the `allowWrite` permission gate. Patched in 4.1.10; `clients/web` was pinned at 4.1.0. Dev-scope only — `@vitest/*` is absent from the published `files` allowlist, and Browser Mode here runs this project's own Storybook play functions, never a third-party page. That is why #1837 deferred it rather than attempting a toolchain upgrade immediately before an irreversible publish. ## The knot, and what actually unties it `^4.1.0` already admits 4.1.10, so the declared ranges were never the problem. The family is welded by *exact* peer pins — `vitest` pins `@vitest/browser-playwright`, which pins `@vitest/browser` — so nothing moves alone. The issue predicted that an explicit coordinated `npm i -D` would fix it. It does not. With `clients/web/package-lock.json` in place, npm ERESOLVEs even when every member of the family plus the Storybook packages are named in one command and `node_modules` is deleted first: Found: @storybook/addon-a11y@10.2.19 Could not resolve dependency: dev @storybook/addon-a11y@"^10.5.5" Conflicting peer dependency: storybook@10.5.5 npm resolves against the existing lock tree and will not move the whole constellation at once. **Regenerating `clients/web/package-lock.json` is the only thing that unties it** — a clean resolve places all four at 4.1.10 with no peer complaints at all. ## Versions vitest 4.1.0 -> 4.1.10 @vitest/browser 4.1.0 -> 4.1.10 (transitive) @vitest/browser-playwright 4.1.0 -> 4.1.10 @vitest/coverage-v8 4.1.0 -> 4.1.10 storybook + @storybook/* 10.2.19 -> 10.5.5 eslint-plugin-storybook 10.2.19 -> 10.5.5 @chromatic-com/storybook 5.0.1 -> 5.2.1 `clients/{cli,tui,launcher}` were already resolving 4.1.9 (no browser pins, so they floated freely) and were never vulnerable. Their declared `^4.1.0` still *permitted* 4.1.0 on a fresh resolve, so the floor is raised to `^4.1.10` in all four manifests — same reasoning as the vite floor in #1841. Pinning only the lockfile would have left a regression path open. ## The vite 8 peer override Resolved, as a consequence rather than a workaround. `@joshwooding/vite-plugin-react-docgen-typescript@0.6.4` declared `peer vite@"^3 || ^4 || ^5 || ^6 || ^7"` against our vite 8. It is not a direct dependency — it arrives through `@storybook/react-vite`, which at 10.5.5 depends on `^0.7.0`, and 0.7.0 adds `^8.0.0` to that range. So taking Storybook 10.5.5 for the peer set drops the override too; nothing is forced. ## Two holds the regeneration forced, each with a tracked issue Regenerating the lock floats every other in-range dependency to current. Most of that is welcome — it is what takes `clients/web` from 19 advisories to 1 — but two floats broke the gate and are unrelated to this security fix: - **zod 4.3.6 -> 4.4.3** makes `clients/web`'s `tsc -b` die with `FATAL ERROR: Ineffective mark-compacts near heap limit` at the ~4GB default. Reverting zod alone makes it exit 0, so the attribution is unambiguous. Held at `~4.3.6`. See #1896. - **eslint-plugin-react-hooks 7.0.1 -> 7.1.1** enables `set-state-in-effect`, which fails `lint` on 8 pre-existing violations across 7 components. Fixing them is a real refactor with interaction-behavior consequences, not a suppression. Held at `~7.0.1`. See #1897. Both `~` pins are deliberate and temporary, and both issues say so — they are constraints to remove, not preferences. Storybook 10.5.5 also reports that `setProjectAnnotations` is applied automatically since 10.3, making `.storybook/vitest.setup.ts` redundant. Left in place: `./preview` carries the Mantine decorator and the a11y annotations drive the play-function assertions, and if automatic provisioning missed either, the 462 stories would render unthemed with a11y checks inert and very likely still pass. That silent-failure risk does not belong in a security bump. See #1898. ## Audit delta `clients/web`, where the advisory lived: before 19 (4 critical, 6 high, 6 moderate, 3 low) after 1 (1 low — esbuild-in-tsup, Windows dev server only) All four critical entries — `vitest`, `@vitest/browser`, `@vitest/browser-playwright`, `@vitest/coverage-v8` — are gone. The other 14 are transitive dev-tree advisories (hono, ws, js-yaml, flatted, brace-expansion, ...) cleared as a side effect of the clean resolve. Elsewhere: cli 6 -> 5, launcher 3 -> 2, tui 4 -> 4, root unchanged at 9 (its lock is untouched). Those remainders are pre-existing and unrelated. ## Verification `npm run ci` green end to end on the upgraded stack — the point of caring here, since this *is* the test infrastructure: - coverage gate (>=90 per file, all four dimensions) passes on all four clients; no file regressed, so no gate was lowered and no `v8 ignore` added - 4802 web tests, 304 cli, 282 tui, 5 launcher - 462 Storybook play functions across 109 files, on vitest 4.1.10 Browser Mode - verify:build-gate, and all five smokes including `smoke:tui` (real TTY here) Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Upgrades the Vitest Browser Mode + Storybook toolchain (dev-only) to clear GHSA-p63j-vcc4-9vmv by moving the vitest/@vitest/* family to 4.1.10 in clients/web, while also raising the declared Vitest floors in the other clients to prevent fresh-resolve regressions back to vulnerable patch versions.
Changes:
- Bump
vitest,@vitest/browser(-playwright), and@vitest/coverage-v8to 4.1.10 (and raise declared ranges in all clients). - Upgrade Storybook packages in
clients/webto 10.5.5 to satisfy the peer-locked Vitest family. - Add temporary holds in
clients/web(zod~4.3.6,eslint-plugin-react-hooks~7.0.1) to keep unrelated gate-breaking floats out of this security PR.
Reviewed changes
Copilot reviewed 4 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| clients/web/package.json | Raises Vitest/Storybook floors; adds temporary holds for zod and eslint-plugin-react-hooks. |
| clients/web/package-lock.json | Regenerated lock to untie peer pins; confirms @vitest/browser/vitest resolve to 4.1.10 and Storybook to 10.5.5. |
| clients/cli/package.json | Raises declared vitest / @vitest/coverage-v8 floors to ^4.1.10. |
| clients/cli/package-lock.json | Updates resolved Vitest family to 4.1.10 (plus incidental transitive bumps). |
| clients/tui/package.json | Raises declared vitest / @vitest/coverage-v8 floors to ^4.1.10. |
| clients/tui/package-lock.json | Updates resolved Vitest family to 4.1.10 (plus incidental transitive bumps). |
| clients/launcher/package.json | Raises declared vitest / @vitest/coverage-v8 floors to ^4.1.10. |
| clients/launcher/package-lock.json | Updates resolved Vitest family to 4.1.10 (plus incidental transitive bumps). |
Files not reviewed (3)
- clients/cli/package-lock.json: Generated file
- clients/launcher/package-lock.json: Generated file
- clients/tui/package-lock.json: Generated file
|
Cross-PR note: the #1900 is forced to 7.1.1 — 7.0.1's peer range stops at So if this PR merges after #1900, the |
Resolves the dependency conflicts introduced by #1899 (the vitest/Storybook security untangle), which regenerated all four client lockfiles. package.json resolution is a union of both PRs' intents: - Kept from #1899: vitest / @vitest/* 4.1.10, Storybook 10.5.5 (+ @chromatic-com/storybook 5.2.1), and the deliberate `zod: "~4.3.6"` hold on clients/web (4.4.3 OOMs web's `tsc -b`, tracked as #1896). cli/tui/root keep their pre-existing `^4.3.6`, which resolves to 4.4.3 exactly as on v2/main. - Kept from this branch: eslint ^10.8.0 and @eslint/js ^10.0.1 in every client and at the root. - eslint-plugin-react-hooks resolved to ^7.1.1, intentionally DROPPING #1899's `~7.0.1` hold. That last point is the one semantic decision here. #1899 pinned eslint-plugin-react-hooks to ~7.0.1 (issue #1897) because 7.1.1 enables `react-hooks/set-state-in-effect`, which failed on 8 pre-existing violations. This PR supersedes that hold on both counts: 7.0.1's peer range stops at eslint ^9, so it ERESOLVEs against eslint 10, and this PR already fixed all 8 violations by introducing the `useValueChange` hook. Restoring the hold would break the install, and keeping it would preserve a constraint we no longer need — so #1897 is closed out by this branch rather than carried forward. Lockfiles were not hand-merged — they were reset to v2/main's regenerated versions and re-resolved with a root `npm install` (postinstall cascade). Verified with `npm run ci` from the root: validate, the per-file >=90 coverage gate, verify:build-gate, all smokes (smoke:tui included, run locally with a real TTY), and the Storybook play functions all pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU
Closes #1839
Clears GHSA-p63j-vcc4-9vmv (critical) —
@vitest/browserBrowser Mode provider commands accept a file path from the browser and act on it without checking theallowWritepermission gate. Patched in 4.1.10;clients/webwas pinned at 4.1.0.Dev-scope only, as #1837 recorded:
@vitest/*is absent from the publishedfilesallowlist, and Browser Mode here runs this project's own Storybook play functions, never a third-party page.Version matrix
vitest@vitest/browser@vitest/browser-playwright@vitest/coverage-v8storybook,@storybook/{addon-a11y,addon-docs,addon-onboarding,addon-vitest,react-vite}eslint-plugin-storybook@chromatic-com/storybook@joshwooding/vite-plugin-react-docgen-typescriptclients/{cli,tui,launcher}were already resolving 4.1.9 — they carry no browser pins, so they floated freely and were never vulnerable. Their declared^4.1.0still permitted the vulnerable 4.1.0 on a fresh resolve, so the floor is raised to^4.1.10in all four manifests. Same reasoning as the vite floor in #1841: pinning the lockfile alone leaves a regression path open.How the peer knot was untangled
^4.1.0already admits 4.1.10 — the declared ranges were never the problem. The family is welded by exact peer pins (vitest→@vitest/browser-playwright→@vitest/browser), so nothing moves alone.The issue predicted an explicit coordinated
npm i -Dnaming the whole family would do it. It does not. Withclients/web/package-lock.jsonin place, npm ERESOLVEs even when every vitest and Storybook package is named in one command andnode_modulesis deleted first:npm resolves against the existing lock tree and will not move the whole constellation at once. Regenerating
clients/web/package-lock.jsonis the only thing that unties it — a clean resolve places all four at 4.1.10 with no peer complaints at all. That is why the web lockfile diff is large: it is the mechanism, not incidental churn.The vite 8 peer override
Resolved, as a consequence rather than a workaround.
@joshwooding/vite-plugin-react-docgen-typescript@0.6.4declaredpeer vite@"^3 || ^4 || ^5 || ^6 || ^7"against our vite 8. It is not a direct dependency — it arrives through@storybook/react-vite, which at 10.5.5 depends on^0.7.0, and 0.7.0 adds^8.0.0to that range.So taking Storybook 10.5.5 for the peer set drops the override too. Nothing is forced and no
overridesentry was needed.Two holds the regeneration forced
Regenerating the lock floats every other in-range dependency to current. Most of that is welcome — it is what takes
clients/webfrom 19 advisories to 1 — but two floats broke the gate and are unrelated to this security fix. Both are held with~, and both have a tracked issue saying the hold is temporary:4.3.6→4.4.3makesclients/web'stsc -bdie withFATAL ERROR: Ineffective mark-compacts near heap limitat the ~4GB default heap. Reverting zod alone and changing nothing else makes it exit 0, so the attribution is unambiguous. Held at~4.3.6— chore(deps): zod 4.4 blows the tsc heap in clients/web — held at ~4.3.6 #1896.7.0.1→7.1.1enables the newset-state-in-effectrule, which failslinton 8 pre-existing violations across 7 components (six are the sameuseEffect(() => setIsExpanded(isListExpanded), [isListExpanded])shape). The rule is pointing at something real, so the fix is a refactor with interaction-behavior consequences — not a suppression, and not work to smuggle into a security bump. Held at~7.0.1— chore(lint): migrate to eslint-plugin-react-hooks 7.1 (set-state-in-effect) — held at ~7.0.1 #1897.Neither is a preference. Both issues describe the constraint as one to remove.
Separately, Storybook 10.5.5 now reports that
setProjectAnnotationsis applied automatically since 10.3, making.storybook/vitest.setup.tsredundant. Left in place deliberately — #1898 explains why:./previewcarries the Mantine decorator anda11yAddonAnnotationsdrives the a11y assertions inside the play functions. If automatic provisioning missed either, the 462 stories would render unthemed with a11y checks inert and would very likely still pass. A green run would not prove the removal was safe, and that silent-failure mode does not belong in a dependency-security PR.Audit delta
clients/web, where the advisory lived:All four critical entries cleared:
The other 14 were transitive dev-tree advisories (hono, ws, js-yaml, flatted, brace-expansion, fast-uri, qs, …) cleared as a side effect of the clean resolve. Elsewhere: cli 6 → 5, launcher 3 → 2, tui 4 → 4, root unchanged at 9 (its lockfile is untouched). Those remainders are pre-existing and out of scope for this issue.
Dependabot alerts un-dismissed
The issue asks that the dismissed alerts be re-opened rather than left dismissed-but-fixed. Done — all three were
@vitest/browser, alltolerable_risk, and all three are fixed by 4.1.10:otelCarrierquery parameter served as inline scriptEach is back to
state: openand should close on its own once this merges and Dependabot re-scansclients/web/package-lock.json. Worth confirming after merge — if any lingers, it means the alert is keyed to a path this change did not move.Test plan
npm run cigreen end to end (CI_EXIT=0) on the upgraded stack — which matters more than usual here, because this is the test infrastructure:v8 ignorewas added. Totals: web 98.66/96.13/98.69/99.01, cli 99/94.06/99.09/99.39, tui 96.2/93.07/96.53/96.6.verify:build-gatefired correctly (the Build gate: fail vite build on a browser-externalized Node built-in (SEP of #1615) #1769 gate).smoke:tui— this machine has a real TTY, so it ran rather than self-skipping.No source files changed; the diff is four
package.jsons and four lockfiles. No screenshots: there is no UI change to show.🤖 Generated with Claude Code
https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU