Skip to content

build(deps): fix critical vitest and shell-quote advisories, refresh stale overrides - #654

Open
MRmarioruci wants to merge 4 commits into
mainfrom
claude/dependabot-vitest-bugs-51363f
Open

build(deps): fix critical vitest and shell-quote advisories, refresh stale overrides#654
MRmarioruci wants to merge 4 commits into
mainfrom
claude/dependabot-vitest-bugs-51363f

Conversation

@MRmarioruci

Copy link
Copy Markdown
Contributor

Problem

Dependabot reports two critical advisories against this repo, plus a long tail of high/medium ones that are mostly transitive.

  1. vitest < 3.2.6 (GHSA-5xrq-8626-4rwp). The UI server's /__vitest_attachment__ handler misuses the deprecated isFileServingAllowed, and the API's saveTestFile + rerun pair means exposing it is equivalent to granting script execution. Worth stating plainly: this was not reachable here. The advisory requires the UI server to be listening (--ui, api.host, or browser mode), and Orbit runs vitest run in CI with no @vitest/ui dependency anywhere. It is still a direct devDependency two majors behind, which is its own maintenance problem.

  2. shell-quote <= 1.8.3 (GHSA-w7jw-789q-3m8p). Reaches the tree through concurrently.

Separately, several entries in the existing pnpm.overrides block had gone stale: immutable, dompurify, brace-expansion, form-data and axios were each pinned to a version that a later advisory has since superseded, so they were silently no longer doing their job.

Changes

The branch starts at the one place the vitest upgrade actually changed behaviour. SessionExpiredOverlay.spec.ts asserted that a loading spinner was visible after firing trigger('click') without awaiting it, then waiting a single $nextTick(); because signIn was mocked as an already-resolved promise, the .finally clearing isReauthenticating could land in the same microtask flush that rendered, so the spinner was gone before the assertion. That is a latent race in the test rather than anything vitest broke, and it is fixed first, on its own, so the tree is green at every commit.

From there vitest and @vitest/coverage-v8 move to 3.2.6 in the root and in cli/. The peer range of 3.2.6 already accepts the vite in the tree, so nothing else moves with it. The third commit then works outward to everything the runner does not own: the stale overrides are refreshed, entries are added for shell-quote, js-yaml, postcss, ws, svgo, mermaid and sharp, and vite and sharp also move as direct dependencies. vite has no patched 5.x line, so the root devDependency goes to 6.4.3. The last commit is the Rust side, where quinn-proto reaches Cargo.lock through reqwest.

Tests

Baseline on vitest 1.6.1 was recorded first, then compared after each step.

  • Before: 102 files / 473 tests (wallet) and 4 / 52 (cli), all passing.
  • Bumping vitest with no test changes: 1 failed | 472 passed. The single failure is the racy spinner assertion described above.
  • After the spec fix: 473/473 and 52/52 green. The fixed spec was re-run against vitest 1.6.1 as well and passes there too, so the first commit stands on its own and does not depend on the bump.
  • After the overrides sweep and the vite 5 to 6 bump: 473/473 and 52/52 still green, apps/wallet vite build succeeds, docs astro build completes.
  • vue-tsc --noEmit, prettier --check and eslint clean on the changed spec.
  • cargo check --locked --workspace --exclude integration-tests passes with the new Cargo.lock.

Deliberately out of scope

The spinner test fired trigger('click') without awaiting it and then relied
on a single $nextTick() to observe the loading state. Because signIn was
mocked as an already-resolved promise, the .finally that clears
isReauthenticating could land in the same microtask flush that rendered the
component, so the spinner was gone before the assertion ran.

Hand the mock a promise that only settles after the assertion, so the
in-flight state is genuinely observable rather than a race the runner
happens to win.
Vitest below 3.2.6 lets the UI server read and execute arbitrary files, so
exposing the API is equivalent to granting script execution. Orbit only ever
runs `vitest run` and depends on no @vitest/ui, so nothing here was reachable,
but the advisory is rated critical and the runner is a direct devDependency.

The peer range of 3.2.6 already accepts the vite version in the tree, so no
build tooling moves with it.
Most of the open alerts are transitive, and several existing overrides had
simply gone stale against newer advisories: immutable, dompurify,
brace-expansion, form-data and axios were all pinned to versions that have
since been superseded. Refresh those and add entries for shell-quote,
js-yaml, postcss, ws, svgo, mermaid and sharp.

vite and sharp also move as direct dependencies. vite has no patched 5.x
line, so the root devDependency goes to 6.4.3, which the wallet build and
the vitest run both already tolerate.

The critical shell-quote issue reaches the tree through concurrently.
Reaches the lockfile through reqwest. `cargo check --locked` across the
workspace is unaffected.
@MRmarioruci
MRmarioruci requested a review from a team as a code owner August 19, 2026 07:31
@zeropath-ai

zeropath-ai Bot commented Aug 19, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to b104036.

Security Overview
Detected Code Changes

The diff is too large to display a summary of code changes.

@MRmarioruci
MRmarioruci requested review from aterga and a lite review from Copilot August 19, 2026 08:03

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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