Skip to content

Patch the MCP Zod dependency and verify real SDK validation boundaries - #180

Merged
mvalancy merged 5 commits into
audit/core-validation-20261023from
audit/core-zod-fix-20261023
Oct 23, 2026
Merged

mvalancy merged 5 commits into
audit/core-validation-20261023from
audit/core-zod-fix-20261023

Conversation

@mvalancy

Copy link
Copy Markdown
Member

The fresh production audit on unchanged integration 78a90cc now rejects Zod 3.25.76 under GHSA-mrhx-c7xc-m3w5, although an earlier CI response omitted it. This pins MCP's dependency to patched Zod 4.6.7 and resolves the actual hoisted SDK consumer to that version. SDK 1.32.2 stays unchanged; no application source directly imports Zod.

The audit step receives a separate cache under the runner temporary directory for each run attempt. The install cache and high/critical threshold remain unchanged. This removes restored audit-cache reuse as one variable; the earlier discrepancy's cause and registry freshness are not claimed.

Validation:

  • Same 13 bounded and real SDK compatibility fixtures: old dependency 5 failed / 8 passed; patched dependency 13 passed, no skips. Small inputs demonstrate early size rejection without exhaustion-sized payloads.
  • Full unit run: 603 passed, with 25 explicitly DB-gated skips. Build, types, lint and 161 harness tests pass.
  • Fresh lockfile install resolves the MCP SDK and zod-to-json-schema to the single patched copy. Targeted lock diff changes only Zod and its direct workspace constraint.
  • An audit with a newly created owned cache exits 0 with zero high/critical and eight moderate findings. Independent npm 10.9.9 confirmation also passes.
  • The canonical lesson retains earlier audit responses, the nested-dependency pitfall, baseline failures and acceptance limits.

The actual SDK fixtures cover initialization, tool listing/calling, metadata/defaults, malformed params/results, recovery and bounded stdio parsing. They do not launch GraphDone's real entry point. The separate actual-process check found stdout diagnostic contamination, tracked in PR #179; that correction and its real Neo4j/stdio regression remain separate.

This closes the observed dependency finding, not all MCP resource-exhaustion risk: unbounded collections are unchanged, Core does not register the tested completion endpoint, and tool JSON-schema declarations are not made universally enforced by this update. Final combined integration CI and non-author review of #157 remain required. No deployment occurred.

Tracks #156.

@github-actions

Copy link
Copy Markdown

🧪 Comprehensive Test Suite

  • Unit suites (Node 22.18.0 & 24.x) — core, web, server, mcp-server: ✅ passed
  • Installer & deploy config: ✅ passed

Full-stack smoke gate runs in the CI workflow.

@mvalancy

Copy link
Copy Markdown
Member Author

Independent review at 234fcc8: no blocking finding in this seven-file scope. This comment does not satisfy protected dev approval or replace final combined CI.

  • Verified the dependency delta contains one hoisted Zod 4.6.7 and the MCP workspace's exact pin; no nested vulnerable copy remains. Installed SDK 1.32.2 and zod-to-json-schema resolve to that package within their declared compatible ranges. The SDK's zod/v4 resolution exports core 4.6.7; the package integrity matches the official registry. GraphDone source has no direct Zod schema imports.
  • Independently reproduced the old bounded-array behavior: a maximum-two schema visited all 100 elements and could produce 101 issues. The patched package visits zero oversized elements and produces one too_big issue. Independently ran the identical committed compatibility fixture: 13 passed, zero skipped, exit 0. Its test bytes match the retained baseline that reports 5 failures / 8 passes. Real SDK initialization/list/call/default/error/recovery and bounded stdio parsing complement the primitive checks.
  • Independently ran npm audit --omit=dev --audit-level=high --json at this exact head with a newly created, isolated NPM_CONFIG_CACHE: exit 0, eight moderate findings, zero high/critical, observed 2026-10-23 21:12:47 UTC. A separate npm 10.9.9 audit on the patched dependency also passed. The same npm 10.9.9 against the old unchanged lock had reproduced the Zod high finding.
  • The CI environment change is scoped only to the existing audit step, uses an owned runner-temporary directory unique to the run/attempt, and retains the exact audit threshold. It does not alter install-cache reuse, delete host caches, or waive failures. It removes one local-cache variable; it neither proves why prior audit responses differed nor guarantees registry freshness.
  • The ledger now preserves the old snapshot as historical and explicitly records the fresh audit failure and pending final review. The official advisory's dates predate the earlier green CI response; the discrepancy is not presented as proof of a newly published advisory.

Limits are accurate: upstream's constrained-collection fix leaves unbounded collections unchanged. The completion-schema regression is an SDK surface, not a registered Core completion endpoint. This does not establish general MCP denial-of-service protection, all JSON-schema enforcement, hosted deployment, or broad authentication security. The stdout correction and combined nine-case real-entrypoint/DB validation are tracked separately in #179. No source changes were made during this review.

@github-actions

Copy link
Copy Markdown

🧪 Comprehensive Test Suite

  • Unit suites (Node 22.18.0 & 24.x) — core, web, server, mcp-server: ✅ passed
  • Installer & deploy config: ✅ passed

Full-stack smoke gate runs in the CI workflow.

@mvalancy
mvalancy marked this pull request as ready for review October 23, 2026 21:18
@mvalancy

Copy link
Copy Markdown
Member Author

Follow-up review completed at c041814. No blocking finding. Since reviewed 234fcc83, this adds exactly 28 lines in two Markdown files; no dependency, test, workflow, or application behavior changed. Worktree and diff checks are clean.

Independently inspected retained combined-candidate evidence for df1fac165a5f1421602c3706db8b86090b54a4f5: execution records the same clean source at completion and exit 0 for every listed check. Logs corroborate 603 unit passes plus 26 DB-gated skips, 161 harness passes, 9 real Neo4j/stdio contract passes, zero lint errors with existing warnings, and a passing bundle budget. The fresh-cache production audit reports eight moderate findings, zero high/critical. Its file has trailing npm update notices after the JSON object; those are not additional audit findings.

The separate compiled-entrypoint probe reports success, initialization, 31 listed tools, owned graph reads, malformed/unknown-call rejection and recovery, independent DB readback, and zero protocol errors. Recorded cleanup confirms child termination, one edge, two nodes and one graph removed, then all owned identifiers absent. The documentation correctly separates this one-off probe from the committed contract tests and preserves the pending final public CI/non-author approval boundary.

This follow-up inspected existing artifacts and the documentation delta; no tests, browsers, database probes or builds were rerun. Earlier exact-head source/dependency/fixture review remains applicable. These local combined results are not deployment evidence or a substitute for the new final integration-head checks.

@mvalancy
mvalancy merged commit 385e767 into audit/core-validation-20261023 Oct 23, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant