Skip to content

chore(deps): refresh fast-uri, qs and browserslist to clear npm audit - #2249

Merged
cliffhall merged 2 commits into
v2/mainfrom
v2/chore/2244-npm-audit-advisories
Sep 5, 2026
Merged

chore(deps): refresh fast-uri, qs and browserslist to clear npm audit#2249
cliffhall merged 2 commits into
v2/mainfrom
v2/chore/2244-npm-audit-advisories

Conversation

@cliffhall

@cliffhall cliffhall commented Sep 5, 2026

Copy link
Copy Markdown
Member

Closes #2244
Closes #2225

All three outstanding advisories turned out to be stale lockfile resolutions, not upward-blocked pins. Every fixed version already sits inside the range its declaring parent asks for, so refreshing the lock entry is the whole fix:

Package Install Was Now Parent's declared range
fast-uri root 3.1.5 3.1.7 ajv@8.18.0^3.0.1
qs root 6.15.3 6.16.0 express@5.2.1^6.14.0
browserslist clients/tui 4.28.2 4.28.9 @babel/core → (unconstrained above 4.28.7)

Two lockfiles changed. No manifest changed, nothing moved between manifests, and no client re-declares a root-owned package.

Why no overrides entry

Both #2244 and #2225 proposed an overrides pin, and AGENTS.md does say to pin a transitive with overrides rather than with npm audit fix. That rule is about how to pin when a pin is needed — it does not call for one where the declared range already admits the fix.

A pin here would buy nothing and would cost something later: "fast-uri": "^3.1.6" forbids fast-uri@4.x for as long as it stands, including after ajv moves to it, and an override is applied by npm with no upward-bound check and no signal when it becomes obsolete. The three lock entries were refreshed with targeted npm update <pkg> calls rather than npm audit fix, so nothing was silently downgraded — the diff is the fixed versions plus their in-family transitives (caniuse-lite, electron-to-chromium, node-releases, update-browserslist-db, baseline-browser-mapping) and nothing else.

Regression cover is the two sweeps that just landed in this milestone (#2232, #2239, #2243) plus the release-time npm audit --audit-level=high report. npm never downgrades an existing lock entry, so the refreshed resolutions hold across an ordinary npm install.

Reachability — assessed, not assumed

The issue asked for this to be established rather than taken from the advisory headline.

fast-uri is the one that ships, and its impact is well below its headline. ajv is a root runtime dependency, and Vite pre-bundles it into the published clients/web/dist (getViteDevOptimizeDeps().include names both ajv and @modelcontextprotocol/client/validators/ajv), so the vulnerable code was inlined into the shipped SPA, not merely resolved at install time. The input is attacker-influenced: schemaUtils.ts compiles the outputSchema a server under test supplies, and ajv resolves that schema's $id/$ref through fast-uri (ajv/dist/runtime/uri.js). But both SSRF advisories (GHSA-f65p-4m7j-42xc, GHSA-fph4-wmhf-6fwf) require a consumer that fetches the parsed URI, and ajv never performs a network request. The realistic worst case is the host-confusion pair (GHSA-5jgf-p345-68v8, GHSA-jqff-g426-hqxp) mis-normalizing a crafted $id, yielding a wrong or failed validation of one tool's output. Real, worth fixing before the release build, not user-facing SSRF.

qs is installed in production, but nothing shipped runs it. (Corrected during review — my first pass called this dev-only, and that was wrong.) express is a root devDependency, but that is not its only path into the tree. npm ls express --omit=dev shows it reaching a production install two other ways:

@modelcontextprotocol/inspector@2.5.0
├─┬ @modelcontextprotocol/ext-apps@1.7.5
│ └─┬ @modelcontextprotocol/sdk@1.30.0
│   └── express@5.2.1 deduped
└─┬ @modelcontextprotocol/server-legacy@2.0.0
  ├─┬ express-rate-limit@8.6.2
  │ └── express@5.2.1 deduped
  └── express@5.2.1

@modelcontextprotocol/server-legacy is a root runtime dependency, so qs is present in every user install — a real supply-chain footprint, not nothing. What does hold is that no shipped code ever instantiates it: nothing in core/, clients/*/src or clients/web/server calls express(), the web backend is Hono, and server-legacy is imported only from test-servers/src, which the root files list does not publish. Both advisories (GHSA-x5fp-wj9c-mxmx, GHSA-4mjr-xmp4-gh2g) require express to parse an attacker-supplied query string, which needs a live express app. So: in the tree, not on any executed path.

browserslist is dev tooling, as #2225 established: it arrives in the clients/tui install through eslint-plugin-react-hooks@babel/core, is reached only by lint, and is in no published bundle. What identified it as a stale lock rather than a constrained one is that the root install already resolved a patched 4.28.8 from the same plugin at the same version — same range, newer resolution — so no pin was ever required, only a refresh.

Verification

npm audit is clean in all five installs, both with and without dev dependencies (10/10):

.                  dev+prod: found 0 vulnerabilities     prod-only: found 0 vulnerabilities
clients/web        dev+prod: found 0 vulnerabilities     prod-only: found 0 vulnerabilities
clients/cli        dev+prod: found 0 vulnerabilities     prod-only: found 0 vulnerabilities
clients/tui        dev+prod: found 0 vulnerabilities     prod-only: found 0 vulnerabilities
clients/launcher   dev+prod: found 0 vulnerabilities     prod-only: found 0 vulnerabilities

npm run format clean; npm run local:gate passes.

No UI change, so no screenshots.

On #2225

#2244 is a superset of #2225 — same browserslist finding, same tui install, same chain. This PR satisfies every box in #2225's "Done when" (dev audit clean in all five, --omit=dev clean in all five, gate passes); it just reaches the last one without the overrides entry that issue proposed, for the reason above. Closing both.

🤖 Generated with Claude Code

https://claude.ai/code/session_01YahVxMTGpigLbZBh1JGPDr

@cliffhall cliffhall added the v2 Issues and PRs for v2 label Sep 5, 2026
@cliffhall
cliffhall requested a balanced review from Copilot September 5, 2026 00:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Correct the security assessment that inaccurately describes qs as dev-only.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Refreshes transitive lockfile resolutions to patched versions without manifest changes.

Changes:

  • Updates root fast-uri and qs.
  • Updates TUI browserslist and related data packages.
  • Incorrectly characterizes production-reachable qs as dev-only.
File summaries
File Description
package-lock.json Refreshes patched dependencies; the qs reachability assessment needs correction.
clients/tui/package-lock.json Refreshes browserslist and its transitives.
Review details

Copilot wasn't able to review any files in this pull request.

Files not reviewed (1)

  • clients/tui/package-lock.json: Generated file
  • Files reviewed: 0/2 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Closes #2244
Closes #2225

All three outstanding advisories were stale lockfile resolutions, not
upward-blocked pins: every fixed version already sits inside the range its
declaring parent asks for, so refreshing the lock entry is the whole fix.

  fast-uri  3.1.5 -> 3.1.7   ajv@8.18.0 asks for ^3.0.1   (root)
  qs        6.15.3 -> 6.16.0 express@5.2.1 asks for ^6.14.0 (root)
  browserslist 4.28.2 -> 4.28.9  via @babel/core           (clients/tui)

No `overrides` entry is added, and no manifest changes. AGENTS.md's rule is
that a transitive is pinned with `overrides` rather than with `npm audit fix`
-- it does not call for a pin where none is needed. A permanent pin here would
buy nothing and would later hold a package back: `fast-uri: ^3.1.6` forbids
fast-uri 4.x for as long as it stands, including after ajv moves to it. The
monthly refresh sweep and the daily alert sweep are what catch a regression.

`npm audit` is clean in all five installs (root, web, cli, tui, launcher).

Reachability, assessed rather than assumed:

- fast-uri is the one that ships. ajv is a root runtime dependency, and Vite
  pre-bundles it into the published `clients/web/dist`, so the vulnerable code
  was inlined into the SPA rather than merely resolved at install time. Its
  input is attacker-influenced: `schemaUtils.ts` compiles the `outputSchema`
  a server under test supplies, and ajv resolves that schema's `$id`/`$ref`
  through fast-uri (`ajv/dist/runtime/uri.js`). Impact is bounded well below
  the advisory headlines, though: both SSRF advisories need a consumer that
  fetches the parsed URI, and ajv never performs a network request. The
  realistic worst case is the host-confusion pair mis-normalizing a crafted
  `$id`, giving a wrong or failed validation of one tool's output.
- qs is installed in production, but nothing shipped runs it. express is a root
  devDependency, but that is not its only path: `npm ls express --omit=dev`
  shows it reaching a production install through
  @modelcontextprotocol/server-legacy@2.0.0 (a root runtime dependency, also
  via express-rate-limit) and through @modelcontextprotocol/ext-apps ->
  @modelcontextprotocol/sdk@1.30.0. So qs is present in every user install.
  What holds is that no shipped module ever instantiates it: nothing in
  `core/`, `clients/*/src` or `clients/web/server` calls `express()`, the web
  backend is Hono, and server-legacy is imported only from `test-servers/src`,
  which the root `files` list does not publish. Both advisories need express to
  parse an attacker-supplied query string, which requires a live express app.
- browserslist is dev tooling. It arrives in the clients/tui install through
  eslint-plugin-react-hooks -> @babel/core, is reached only by lint, and is in
  no published bundle. The root install already resolved a patched 4.28.8 from
  the same plugin at the same version, which is what identified tui's copy as a
  stale lock rather than a constrained one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YahVxMTGpigLbZBh1JGPDr
Signed-off-by: cliffhall <cliff@futurescale.com>
@cliffhall
cliffhall force-pushed the v2/chore/2244-npm-audit-advisories branch from 9fec0f4 to 1018d81 Compare September 5, 2026 00:19
@cliffhall

Copy link
Copy Markdown
Member Author

Response to Copilot review round 1

One finding, no inline comments. Copilot is right and I was wrong — fixed in 1018d81 (commit message and PR body both).

✅ "Correct the security assessment that inaccurately describes qs as dev-only" — accepted

My first pass read "express": "^5.2.1" in the root devDependencies, confirmed the only first-party importers were test-servers/src, and stopped there. That was the error: npm ls qs --all prints the deduped top-level entry, which looks like the whole story. --omit=dev is what shows it isn't:

$ npm ls express --omit=dev
@modelcontextprotocol/inspector@2.5.0
├─┬ @modelcontextprotocol/ext-apps@1.7.5
│ └─┬ @modelcontextprotocol/sdk@1.30.0
│   └── express@5.2.1 deduped
└─┬ @modelcontextprotocol/server-legacy@2.0.0
  ├─┬ express-rate-limit@8.6.2
  │ └── express@5.2.1 deduped
  └── express@5.2.1

@modelcontextprotocol/server-legacy is a root runtime dependency, so express — and with it qs — is installed for every user, not just for development. "A user install never installs express and never resolves qs" was flatly false.

The corrected assessment, which is narrower than what I claimed and no longer leans on the dev/prod split at all:

qs is installed in production, but nothing shipped runs it. No first-party module calls express() — nothing in core/, clients/*/src or clients/web/server — the web backend is Hono, and server-legacy is imported only from test-servers/src, which the root files list does not publish. Both advisories (GHSA-x5fp-wj9c-mxmx, GHSA-4mjr-xmp4-gh2g) require express to parse an attacker-supplied query string, which needs a live express app.

So: present in the dependency tree with a real supply-chain footprint, not on any executed path. The fix itself is unchangedqs still goes 6.15.3 → 6.16.0 — and if anything this finding strengthens the case for making it, since the package ships rather than staying behind a dev boundary.

Note on the other two

This correction doesn't touch them, but for the record it does not generalize:

  • fast-uri was already assessed as shipping, and by a stronger route — Vite pre-bundles ajv into the published clients/web/dist, so the code is inlined into the SPA, not merely resolved at install time.
  • browserslist is the one claim of this shape that survives, and it rests on the install boundary rather than on a dev/prod range: it exists only in the separate clients/tui install, reached through eslint-plugin-react-hooks@babel/core. npm ls browserslist --omit=dev in clients/tui is empty, and the tarball ships only each client's build/.

Requesting a re-review.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

All reviewed lockfile updates are compatible, and no unresolved issues remain.

Review details

Copilot wasn't able to review any files in this pull request.

Files not reviewed (1)

  • clients/tui/package-lock.json: Generated file
  • Files reviewed: 0/2 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The lockfile-only updates address the advisories with no unresolved issues.

Review details

Copilot wasn't able to review any files in this pull request.

Files not reviewed (1)

  • clients/tui/package-lock.json: Generated file
  • Files reviewed: 0/2 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@cliffhall
cliffhall merged commit 0373bf9 into v2/main Sep 5, 2026
4 checks passed
@cliffhall
cliffhall deleted the v2/chore/2244-npm-audit-advisories branch September 5, 2026 02:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v2 Issues and PRs for v2

Projects

None yet

2 participants