Repository navigation
Conversation
🦋 Changeset detectedLatest commit: a2d39c8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@modelcontextprotocol/client
@modelcontextprotocol/codemod
@modelcontextprotocol/core
@modelcontextprotocol/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
|
Verified this end-to-end on the PR checkout. |
The ajv copy inlined into the client and server dist resolved fast-uri 3.1.0, which carries nine published advisories (all fixed in 3.1.8). Because the copy is inlined rather than declared, consumer overrides/resolutions and lockfile-based scanners cannot reach it. Pin fast-uri through resolutions — the security-vulnerability trigger in DEPENDENCY_POLICY.md — so the bundled copy is the patched one. Patch release: the published dependency manifests are unchanged; only dist content changes. The dts shim's provenance comment now names 3.1.8; the URIComponent interface is field-for-field identical, so no type change. Which copy ships is observable only in the built output, so each package now pins it there: a dist-scanning test asserts every fast-uri version marker (rolldown's #region comments and the dts shim's provenance comment) names the pinned version, and that at least one exists — the inlining itself is the fix, so a dropped noExternal entry or a stale pin fails CI instead of shipping. Fixes modelcontextprotocol#2966
47aa1f0 to
a2d39c8
Compare
ascswe
left a comment
There was a problem hiding this comment.
Thanks for the focused fix and the bundle-level regression tests. I reviewed the two new fastUriBundlePin.test.ts files at head a2d39c8 and noticed two small opportunities to strengthen the future regression guard:
-
Keep a security floor independent of the configured pin.
pinnedFastUri()currently checks only that the root resolution is a validx.y.zversion. If that resolution and the built bundles were accidentally reverted to3.1.0, the test would still pass. Could the test also enforcefast-uri >= 3.1.8(or the project's approved minimum), rather than only agreement with the pin? -
Require runtime evidence in each format. The scan combines
.mjs/.cjsruntime files with.d.mts/.d.ctsdeclarations. Afast-uri@3.1.8string present only in the d.ts shim's provenance comment could satisfytoContain(pinned)without checking that the ESM and CJS runtime bundles contain a matching version marker. Could the test require at least one matching marker in each runtime format, while still rejecting any conflicting markers across artifacts?
These are defense-in-depth suggestions for the tests, not a claim that the current patch is ineffective. This is a static source review; I did not run the builds or test suites.
Closes #2966.
What
Pin
fast-urito3.1.8through the rootresolutions(the mechanism already used forstrip-ansi), so theajvcopy inlined into theclientandserverdist/bundles the patchedfast-uriinstead of 3.1.0. The published dependency manifests are unchanged, so this is a patch, not a major.Why
packages/client/tsdown.config.tsandpackages/server/tsdown.config.tslistajv/ajv-formatsinnoExternal, so the transitivefast-uriis inlined intodist/while neither package declares it independencies. Consumers therefore cannot raise it withoverrides/resolutions, and lockfile-based scanners cannot see it.fast-uri3.1.0 carries nine published advisories, all fixed in 3.1.8:GHSA-4c8g-83qw-93j6, GHSA-7p8r-x3mc-p8w7, GHSA-f65p-4m7j-42xc, GHSA-hrr3-gc8f-f4qj, GHSA-jqff-g426-hqxp, GHSA-q3j6-qgpj-74h6, GHSA-qw65-cvwx-89v3, GHSA-v2hh-gcrm-f6hx, GHSA-v39h-62p7-jpjc.
This is the security-vulnerability update trigger in
DEPENDENCY_POLICY.md, and the minimal shape that trigger allows underCLAUDE.md's "small changes" principle.Reachability on the default path (analysis from #2036 by a downstream consumer): on Node,
ClientdefaultsjsonSchemaValidatorto the bundled-ajv-backed validator, andClient.callTool()compiles theoutputSchemareturned by the remote server'stools/list, so URI strings the server controls reach the bundledfast-uriparser.Verification (all on this branch)
pnpm audit: fast-uri advisories 9 → 0. (The remaining findings are pre-existing dev-tree advisories in other packages, untouched by this change.)pnpm run build:all, then the region markers in the built output:#region ../../node_modules/.pnpm/fast-uri@3.1.8/inclient/serverdist/validators/ajv*.{mjs,cjs}; nofast-uri@3.1.0code anywhere indist/.fastUriBundlePintests inclientandserveragainst the built output: pass on this branch (every marker names the pinned 3.1.8) and fail when the pin and the builtdist/disagree.node scripts/smoke-dist-types.mjs: clean for both the ESM and the CommonJS consumer (skipLibCheck: false).pnpm -r --filter '!@modelcontextprotocol/test-e2e' test: all green. Per package:server577 passed,client998 passed,core-internal1525 passed.pnpm run typecheck:allandpnpm run lint:all: clean (also re-run by the pre-push hook on push).Packaging pin:
clientandservereach gained a smallfastUriBundlePintest that scans the builtdist/and asserts everyfast-uri@<version>marker — rolldown's#regioncomments over the inlined modules and the dts shim's provenance comment — names the rootresolutionspin, and that at least one marker exists (i.e.ajv/ajv-formatsare stillnoExternal). There is no behavior change to test — same ajv, same options, patched transitive parser — but the shipped copy reaches neither the published manifest nor the lockfile, so the version that actually lands indist/is now pinned directly.Deliberately not in this PR
DEPENDENCY_POLICY.mdcalls adding a runtime dependency a significant change that needs discussion first, and it would change every consumer's install tree. Happy to do it as a follow-up if you prefer that direction.ajvitself. All nine advisories are infast-uri; an ajv bump without a concrete motivation is exactly what the dependency policy rules out.One note on the shim:
packages/core-internal/src/validators/fastUriShim.d.tscopiesfast-uri'sURIComponent; I verified 3.1.8's interface is field-for-field identical to 3.1.0's, so only the provenance comment changed.