Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pnpm-workspace.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ catalogs:
raw-body: ^3.0.0
runtimeShared:
'@cfworker/json-schema': ^4.1.1
ajv: ^8.17.1
ajv: ^6.14.0
Copy link

Choose a reason for hiding this comment

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

🔴 This PR downgrades the direct catalog dependency from ajv: ^8.17.1 to ajv: ^6.14.0 — a major version regression, not the transitive patch bump the title describes. The build will break because packages/core/src/validators/ajvProvider.ts (and two other files) use the v8-only named import import { Ajv } from 'ajv' and v8-only constructor options (strict, validateFormats), and ajv-formats@3.0.1 declares ajv: ^8.0.0 as a peer dependency. Additionally, the vulnerable transitive ajv@6.12.6 (from @eslint/eslintrc) remains in the lockfile unchanged, so this PR doesn't fix the CVE either — it should be closed and Dependabot reconfigured.

Extended reasoning...

What the bug is

Dependabot's PR title claims to bump ajv from 6.12.6 → 6.14.0 — i.e., a minor security-patch bump of the transitive dependency pulled in by @eslint/eslintrc. However, the actual diff does something entirely different: it changes the direct catalog entry in pnpm-workspace.yaml from ajv: ^8.17.1 to ajv: ^6.14.0. This is a two-major-version downgrade of the version the application code actually imports. Dependabot appears to have confused the transitive v6 dependency (used only by eslint tooling) with the direct v8 dependency (used by the SDK's runtime validator).

Step-by-step proof it breaks the build

  1. Named import fails. packages/core/src/validators/ajvProvider.ts:5 reads:

    import { Ajv } from 'ajv';

    In ajv v8, Ajv is exposed as a named export. In ajv v6, the class is the default export only (import Ajv from 'ajv'). With the catalog now resolving to 6.14.0, TypeScript will fail with "Module '"ajv"' has no exported member 'Ajv'". The same import pattern appears in packages/core/src/validators/ajvProvider.examples.ts:10 and examples/client/src/simpleStreamableHttp.ts:21.

  2. Constructor options are v8-only. ajvProvider.ts:11-16 constructs the instance:

    new Ajv({ strict: false, validateFormats: true, validateSchema: false, allErrors: true })

    The strict and validateFormats options were introduced in ajv v7/v8. The v6 Options interface does not declare them, so under strict: true TS config they produce an excess-property error; even if they compiled, v6 would silently ignore them, altering runtime validation semantics.

  3. Peer-dependency violation. ajv-formats@3.0.1 declares "ajv": "^8.0.0" as its peer dependency. The lockfile now pairs it with ajv@6.14.0 (line ajv-formats@3.0.1(ajv@6.14.0)), which is outside the supported range. addFormats(ajv) at ajvProvider.ts:19 expects a v8 instance and its internal plugin registration hooks do not exist on v6 — this would throw or no-op at runtime.

The security fix isn't applied anyway

The actual vulnerable package is the transitive ajv@6.12.6 pulled in by @eslint/eslintrc (a dev-time dependency). The lockfile diff shows this entry is still present after the change:

ajv@6.12.6:
  resolution: {integrity: sha512-j3fVLgvTo527anyYyJOGTYJbG+...}

The PR only adds a new ajv@6.14.0 entry that feeds the catalog consumers. So even setting aside the build breakage, the PR fails at its stated goal.

Why existing guards don't catch it

The ajv-formats peer dependency check would surface as a pnpm install warning, but pnpm warnings don't fail installs by default, and Dependabot doesn't run the TypeScript build. CI typechecking/tests would catch the named-import failure, but the PR should not be merged regardless.

How to fix

Close this PR. To patch the transitive CVE, use a pnpm.overrides entry in the root package.json:

"pnpm": { "overrides": { "ajv@<6.14.0": "^6.14.0" } }

or bump @eslint/eslintrc (via eslint) to a version that depends on a patched ajv. The catalog entry for the direct dependency should remain at ^8.17.1.

ajv-formats: ^3.0.1
json-schema-typed: ^8.0.2
pkce-challenge: ^5.0.0
Expand Down
Loading