Harden dependency baseline: upgrade axios/ws and enforce patched versions via pnpm overrides - #4
Conversation
|
|
@copilot resolve the merge conflicts in this pull request |
Resolved. I merged |
|
@copilot resolve the merge conflicts in this pull request |
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
Reviewer's GuideThis PR hardens the monorepo’s dependency and tooling baseline: it upgrades axios/ws to patched versions, enforces their resolutions via pnpm overrides, aligns the lockfile and root tooling with pnpm/Turbo workflows, and centralizes formatting/linting/configuration across the repo while adding setup docs. Flow diagram for dependency resolution with pnpm overridesflowchart TD
A[pnpm install] --> B[Read root package.json dependencies]
B --> C[Read pnpm.overrides]
C --> D{Dependency is axios or ws?}
D -->|yes| E[Force version axios 1.18.0 / ws 8.21.0]
D -->|no| F[Resolve version from dependency tree]
E --> G[Write resolutions to pnpm-lock.yaml]
F --> G[Write resolutions to pnpm-lock.yaml]
G --> H[Install dependencies across workspaces]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
Fixed security issues:
-
sharp (link)
-
undici (link)
-
ws (link)
-
The change from local .eslintrc.cjs files to a centralized .oxlintrc.json may drop app-specific lint rules or ignores; consider validating whether any app relied on custom overrides and, if so, recreating them via per-package config or root-level patterns.
-
The new
peerDependencyRules.allowedVersionsblock in package.json currently whitelists*for several packages; you may want to narrow these ranges to the versions actually in use to preserve the goal of a stricter, security-focused dependency baseline. -
In
testStatelessMcpAppthe double castas unknown as ExecutionContextsuggests a mismatch between the test fixture and the real type; consider adjusting the helper’s shape or using a dedicated test interface to avoid broadunknowncasting and keep type-safety tighter.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The change from local .eslintrc.cjs files to a centralized .oxlintrc.json may drop app-specific lint rules or ignores; consider validating whether any app relied on custom overrides and, if so, recreating them via per-package config or root-level patterns.
- The new `peerDependencyRules.allowedVersions` block in package.json currently whitelists `*` for several packages; you may want to narrow these ranges to the versions actually in use to preserve the goal of a stricter, security-focused dependency baseline.
- In `testStatelessMcpApp` the double cast `as unknown as ExecutionContext` suggests a mismatch between the test fixture and the real type; consider adjusting the helper’s shape or using a dedicated test interface to avoid broad `unknown` casting and keep type-safety tighter.
## Individual Comments
### Comment 1
<location path="packages/mcp-common/src/test/stateless-app.ts" line_range="50" />
<code_context>
waitUntil() {},
passThroughOnException() {},
- }) as ExecutionContext
+ }) as unknown as ExecutionContext
describe(`${name} stateless MCP transport`, () => {
</code_context>
<issue_to_address>
**suggestion:** Clarify/justify the double cast to `unknown as ExecutionContext` in the test helper
This change implies the mock object no longer satisfies `ExecutionContext`’s type. Since this helper is shared across stateless MCP transport tests, consider either updating the mock to be structurally compatible with `ExecutionContext` (avoiding the double cast), or add a short comment explaining why the double cast is required. That will help prevent future “cleanup” of the cast that accidentally changes the intended test context.
Suggested implementation:
```typescript
...(withAuth && { props: TEST_PROPS }),
waitUntil() {},
passThroughOnException() {},
// NOTE: This helper intentionally uses `unknown as ExecutionContext` because the
// minimal mock object used in tests does not (and should not) fully satisfy
// the `ExecutionContext` interface at compile time. The cast allows tests to
// exercise stateless MCP transport behavior without forcing unnecessary
// implementation details into the mock. Be careful when “cleaning up” this
// cast: changing it to a direct `as ExecutionContext` or altering the mock
// shape may accidentally change the intended test context.
}) as unknown as ExecutionContext
```
If the `ExecutionContext` type evolves or if you decide to align the mock structurally with `ExecutionContext`, you can:
1. Expand the mock object here to implement the required `ExecutionContext` properties/methods and then remove the `unknown` intermediate cast (i.e., use `as ExecutionContext` directly).
2. Update the comment accordingly to reflect that the mock is now structurally compatible and no longer needs the double cast.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| waitUntil() {}, | ||
| passThroughOnException() {}, | ||
| }) as ExecutionContext | ||
| }) as unknown as ExecutionContext |
There was a problem hiding this comment.
suggestion: Clarify/justify the double cast to unknown as ExecutionContext in the test helper
This change implies the mock object no longer satisfies ExecutionContext’s type. Since this helper is shared across stateless MCP transport tests, consider either updating the mock to be structurally compatible with ExecutionContext (avoiding the double cast), or add a short comment explaining why the double cast is required. That will help prevent future “cleanup” of the cast that accidentally changes the intended test context.
Suggested implementation:
...(withAuth && { props: TEST_PROPS }),
waitUntil() {},
passThroughOnException() {},
// NOTE: This helper intentionally uses `unknown as ExecutionContext` because the
// minimal mock object used in tests does not (and should not) fully satisfy
// the `ExecutionContext` interface at compile time. The cast allows tests to
// exercise stateless MCP transport behavior without forcing unnecessary
// implementation details into the mock. Be careful when “cleaning up” this
// cast: changing it to a direct `as ExecutionContext` or altering the mock
// shape may accidentally change the intended test context.
}) as unknown as ExecutionContextIf the ExecutionContext type evolves or if you decide to align the mock structurally with ExecutionContext, you can:
- Expand the mock object here to implement the required
ExecutionContextproperties/methods and then remove theunknownintermediate cast (i.e., useas ExecutionContextdirectly). - Update the comment accordingly to reflect that the mock is now structurally compatible and no longer needs the double cast.
|
@copilot reslolve merge conflicts |
There was a problem hiding this comment.
Pull request overview
This PR updates the monorepo’s dependency/tooling baseline, including pinning patched axios/ws versions via pnpm.overrides, and also introduces broader repo-wide build/lint/format workflow changes (Turbo task additions, Oxlint/Prettier centralization, and stricter pnpm settings).
Changes:
- Upgrades and enforces patched
axios@1.18.0andws@8.21.0via rootdependencies+pnpm.overrides. - Centralizes lint/format config around
oxlint+ Prettier (adds.oxlintrc.json/.prettierrc.json, removes per-package.eslintrc.cjsfiles, updates scripts). - Expands Turbo task definitions and adjusts root scripts to run through Turbo.
Reviewed changes
Copilot reviewed 32 out of 35 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| wrangler.jsonc | Reformat Wrangler config (no functional config changes apparent). |
| turbo.json | Adds dev/build/test Turbo tasks (currently introduces a duplicate build key). |
| SETUP.md | Adds repo setup and common pnpm/Turbo command documentation. |
| README.md | Fixes table alignment/formatting for the server list. |
| packages/tools/bin/run-eslint-workers | Switches workspace lint runner from ESLint to oxlint with centralized config. |
| packages/tools/.eslintrc.cjs | Removes package-local ESLint config. |
| packages/mcp-common/src/test/stateless-app.ts | Adjusts test execution context typing/casting. |
| packages/mcp-common/.eslintrc.cjs | Removes package-local ESLint config. |
| packages/eval-tools/package.json | Adds vitest devDependency (aligned to repo-level version). |
| package.json | Updates scripts, upgrades/pins axios/ws, adds pnpm overrides and peer rules, and changes toolchain deps. |
| apps/workers-observability/.eslintrc.cjs | Removes app-local ESLint config. |
| apps/workers-builds/.eslintrc.cjs | Removes app-local ESLint config. |
| apps/workers-bindings/.eslintrc.cjs | Removes app-local ESLint config. |
| apps/stack-mcp/.eslintrc.cjs | Removes app-local ESLint config. |
| apps/sandbox-container/package.json | Adds vitest devDependency (aligned to repo-level version). |
| apps/sandbox-container/.eslintrc.cjs | Removes app-local ESLint config. |
| apps/radar/.eslintrc.cjs | Removes app-local ESLint config. |
| apps/logpush/.eslintrc.cjs | Removes app-local ESLint config. |
| apps/graphql/.eslintrc.cjs | Removes app-local ESLint config. |
| apps/docs-ai-search/.eslintrc.cjs | Removes app-local ESLint config. |
| apps/dns-analytics/.eslintrc.cjs | Removes app-local ESLint config. |
| apps/dex-analysis/.eslintrc.cjs | Removes app-local ESLint config. |
| apps/demo-day/.eslintrc.cjs | Removes app-local ESLint config (including ignore patterns previously defined there). |
| apps/cloudflare-one-casb/.eslintrc.cjs | Removes app-local ESLint config. |
| apps/cloudflare-blog/.eslintrc.cjs | Removes app-local ESLint config. |
| apps/browser-rendering/.eslintrc.cjs | Removes app-local ESLint config. |
| apps/autorag/.eslintrc.cjs | Removes app-local ESLint config. |
| apps/auditlogs/.eslintrc.cjs | Removes app-local ESLint config. |
| apps/ai-gateway/.eslintrc.cjs | Removes app-local ESLint config. |
| .prettierrc.json | Adds JSON-based Prettier config with import sorting rules. |
| .prettierrc.cjs | Removes prior JS-based Prettier config. |
| .prettierignore | Adds pnpm-no.yaml to Prettier ignore list. |
| .oxlintrc.json | Adds centralized Oxlint config (primarily ignore patterns). |
| .npmrc | Tightens pnpm install behavior (strict peers, shared lockfile, prefer frozen lockfile). |
Suppressed comments (1)
package.json:18
- The PR title/description says this change is limited to upgrading
axios/wsand enforcing patched versions viapnpm.overrides, but thispackage.jsonalso changes repo scripts (format/lint/test/typecheck), lint/format toolchain dependencies (Prettier/Oxlint), and versions like TypeScript/Vite. Either update the PR description/title to reflect the broader scope or split these toolchain changes into a separate PR to make the security baseline change easier to review/revert.
"check": "pnpm run check:format && pnpm run lint && pnpm run typecheck && pnpm run test",
"check:deps": "syncpack lint",
"check:format": "prettier . --check --cache --ignore-unknown",
"check:turbo": "run-turbo check",
"dev": "node scripts/dev-manager.mjs",
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "build": { | ||
| "dependsOn": ["^build"], | ||
| "outputs": ["dist/**"], | ||
| "outputLogs": "new-only" | ||
| }, |
| ({ | ||
| ...(withAuth && { props: TEST_PROPS }), | ||
| waitUntil() {}, | ||
| passThroughOnException() {}, | ||
| }) as ExecutionContext | ||
| }) as unknown as ExecutionContext |
|
@copilot resolve the merge conflicts in this pull request |
Resolved in |
Handled in
Handled in |
Code Review
|
| Auto-apply | Compact |
|
|
Important
Your trial ends in 6 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more.
Was this helpful? React with 👍 / 👎 | Gitar
This updates the monorepo’s vulnerable HTTP/WebSocket client dependencies to patched releases and prevents transitive drift back to affected ranges. The goal is to keep installs/builds unchanged while closing known advisories in
axiosandws.Dependency security baseline
package.json:axios→1.18.0ws→8.21.0Transitive enforcement
pnpm.overridesentries for:axios: 1.18.0ws: 8.21.0Lockfile alignment
pnpm-lock.yamlto apply and persist the new resolution graph.{ "dependencies": { "axios": "1.18.0", "ws": "8.21.0" }, "pnpm": { "overrides": { "axios": "1.18.0", "ws": "8.21.0" } } }Summary by Sourcery
Upgrade and pin key dependencies, centralize tooling configuration, and refresh project setup and docs for a pnpm/Turbo-based monorepo workflow.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests:
Chores: