Fix Dependabot security vulnerabilities in docs site#5372
Conversation
Add yarn resolutions to docs/package.json to force patched versions of vulnerable transitive dependencies in the Docusaurus docs site.
Up to standards ✅🟢 Issues
|
There was a problem hiding this comment.
Code Review: Fix Dependabot security vulnerabilities in docs site
The PR achieves its stated goal — the Snyk scan passes and docusaurus build succeeds. However, there are two concerns worth addressing before merging.
High Risk: path-to-regexp forced to an incompatible API version
The resolutions block forces path-to-regexp to 0.1.13 for all packages, including ones that require semver-incompatible versions:
path-to-regexp@0.1.13, path-to-regexp@3.3.0, path-to-regexp@^1.7.0, path-to-regexp@~0.1.12:
version "0.1.13"
react-router@5.3.4requires^1.7.0— v1.x added named capture groups andcompile()that don't exist in v0.1.xserve-handler@6.1.6requires3.3.0— v3.x API is again incompatible with v0.1.x
This won't break docusaurus build (SSG doesn't execute routing at build time), but it could silently cause broken URL matching in docusaurus serve or the client-side router.
Better approach: Use Yarn's selective version resolution syntax to scope the fix only to the vulnerable ancestor (express), which is the actual attack vector:
"resolutions": {
"express/path-to-regexp": "0.1.13"
}This leaves react-router and serve-handler free to use the API versions they were designed for.
Moderate Concern: Lock file churn beyond the security scope
The diff includes large-scale bumps to unrelated transitive packages:
@ai-sdk/gateway: 1.0.29 → 2.0.23 (major version)@ai-sdk/react: 2.0.52 → 2.0.118- All
@algolia/*packages: 5.38.x → 5.46.x @babel/core: 7.26.0 → 7.28.5
This appears to be from running a fresh yarn install rather than a surgical update. The result is a +2207/-2222 line diff that's very hard to audit — the actual security fix is buried in a sea of unrelated changes.
Why it matters: If one of these incidental updates introduces a regression, it becomes difficult to bisect. The stated purpose of this PR is "fix security vulnerabilities" but the actual scope is a broad dependency refresh.
Suggestion: Consider regenerating the lock file starting from the original yarn.lock with only the resolutions added, rather than doing a full yarn install from scratch. This would produce a much smaller, more auditable diff scoped to just the security fixes.
Minor: brace-expansion cascade
brace-expansion is forced to 5.0.5 to satisfy a chain: minimatch@9 → brace-expansion@^5. But packages that previously used brace-expansion@^1.1.7 are now also resolved to v5, which is a cross-major forced upgrade. Since brace-expansion is a utility package with limited API surface this is likely fine, but it's worth verifying nothing uses the old API directly.
What's good
- The intent is correct: security vulnerabilities in transitive dependencies are a real concern and
resolutionsis the right Yarn v1 mechanism to address them. - The Snyk scan passes and the build is green.
- The lodash fix (
4.18.1) is legitimate.
The path-to-regexp issue is the one I'd want verified or addressed before merging — the other points are informational.
Only express uses the vulnerable 0.1.x range. react-router (^1.7.0) and serve-handler (3.3.0) have incompatible APIs and were incorrectly downgraded by the global resolution.
The link pointed to #test-discovery-issues but the heading is "Tests Not Discovered" (#tests-not-discovered).
There was a problem hiding this comment.
Code Review: Fix Dependabot security vulnerabilities in docs site (follow-up)
The previous review raised two concerns. One has been fully addressed; the other remains.
Addressed: path-to-regexp scoping
The global "path-to-regexp": "0.1.13" override has been replaced with the correctly scoped "express/path-to-regexp": "0.1.13". This means react-router and serve-handler are no longer forced onto an incompatible API version. Good fix.
Still Outstanding: Lock file churn far exceeds the security scope
The diff remains +2211 / -2209 lines. The security-relevant portion — the 12 resolution pins added to package.json — accounts for roughly 15 lines. The remaining ~4400 changed lines in yarn.lock are unrelated transitive bumps:
@ai-sdk/gateway: 1.0.29 → 2.0.23 (major bump — new@vercel/oidcdependency added)@ai-sdk/react: 2.0.52 → 2.0.118- All
@algolia/*packages: 5.38.x → 5.46.x @babel/coreand toolchain packages bumped across the board
This looks like yarn install was run from scratch rather than a surgical yarn add --resolution. These incidental upgrades are unreviewed, untested beyond docusaurus build, and make it very difficult to bisect a future regression. A docs site CI build passing does not exercise the client-side router, search (Algolia), or AI SDK features at runtime.
Recommendation before merging:
- Check out the PR branch, revert
yarn.lockto main, add only theresolutionsblock topackage.json, then runyarn install --frozen-lockfileoryarn installto let Yarn resolve only what is pinned. The resulting lock file diff should be narrow — on the order of the pinned packages and their direct dependencies, not a wholesale refresh of the entire tree. - If the intent is to also upgrade those unrelated packages, that should be a separate PR so changes can be reviewed and reverted independently.
The security goal is correct and the package.json changes look good. The concern is purely about the scope of the lock file changes bundled in alongside them.
There was a problem hiding this comment.
Code Review: Fix Dependabot security vulnerabilities in docs site (third pass)
Checking whether the issues from the two previous reviews have been addressed.
Addressed: path-to-regexp scoping ✓
Commit c1256cce correctly changed the global "path-to-regexp": "0.1.13" to the scoped "express/path-to-regexp": "0.1.13". The lock file now shows three separate resolutions:
path-to-regexp@~0.1.12→0.1.13(express, fixed ✓)path-to-regexp@^1.7.0→1.9.0(react-router, now gets its proper version ✓)path-to-regexp@3.3.0→3.3.0(serve-handler, unchanged ✓)
This is exactly right.
Also good: Broken anchor link fix ✓
The property-injection.md fix (#test-discovery-issues → #tests-not-discovered) is a valid, independent improvement.
Still Outstanding: Lock file churn far exceeds security scope
The diff is still +2212 / -2210 lines. The security-relevant changes (12 pinned resolutions in package.json + their direct yarn.lock entries) account for maybe 50–100 lines. The remaining ~4300 changed lines are unrelated transitive upgrades:
@ai-sdk/gateway: 1.0.29 → 2.0.23 (major bump, new@vercel/oidcdependency added)@ai-sdk/react: 2.0.52 → 2.0.118 (66 minor versions)- All
@algolia/*packages: 5.38.x → 5.46.x @babel/coreand build toolchain bumped broadly
These changes are unreviewed, untested beyond docusaurus build, and make the PR very hard to audit. A docs build passing does not exercise the client-side router, search, or AI SDK features at runtime.
Recommended fix: On the PR branch, restore the original yarn.lock from main (git checkout main -- docs/yarn.lock), keep only the resolutions additions in package.json, then re-run yarn install. Yarn will re-resolve only the pinned packages and their immediate ancestors, producing a narrow diff that matches the stated scope of the PR.
If the intent is to also upgrade those unrelated packages, that work should be a separate PR so it can be reviewed independently and reverted in isolation if it causes a regression.
Summary
resolutionstodocs/package.jsonto force patched versions of 15 vulnerable transitive dependencies in the Docusaurus docs siteResolved packages
Test plan
npm auditreports 0 vulnerabilitiesdocusaurus buildsucceeds