diff --git a/.agents/inbox/2026-10-23-zod-constrained-collections.md b/.agents/inbox/2026-10-23-zod-constrained-collections.md new file mode 100644 index 00000000..04122e79 --- /dev/null +++ b/.agents/inbox/2026-10-23-zod-constrained-collections.md @@ -0,0 +1,104 @@ +# Validate collection limits before walking elements and retain real SDK compatibility evidence + +Date: 2026-10-23. Searched the dependency/security lessons, MCP production +imports, SDK schemas and existing protocol tests. The final production audit +reported [GHSA-mrhx-c7xc-m3w5](https://github.com/advisories/GHSA-mrhx-c7xc-m3w5) +against the locked Zod 3.25.76 package. The advisory lists versions below 4.6.6 +as affected and 4.6.6 as patched. Its later audit appearance does not establish +when the vulnerability became reachable or why earlier audit responses differed. + +The public advisory was published October 1 and reviewed/updated October 23; +it was not newly published when the final check failed. On unchanged +`78a90cc7899e44c33aa27117bccb19ea7cbe85f6`, the fresh production audit exited 1 +with eight moderate findings and this one high finding, while the earlier CI +response contained eight moderate findings only. Repeating with CI's npm +10.9.9 also reported the high finding. The exact registry/cache/backend cause +of the differing responses is unproven. The CI audit step now uses its own +runner-temporary cache for each run attempt, retaining the existing install +cache and unchanged high/critical threshold. This removes restored audit-cache +reuse as one variable; it does not guarantee registry freshness. + +The direct MCP dependency is pinned to 4.6.7. A manifest-only install initially +left the vulnerable 3.25.76 copy hoisted for the SDK and nested the new version +under MCP. A targeted `npm update zod --package-lock-only --ignore-scripts +--legacy-peer-deps` resolved the single root copy instead. The final lock delta +changes only that root version, its registry integrity, and the MCP constraint; +SDK 1.32.2 stays unchanged. `npm ls zod --all --omit=dev` confirms both the SDK +and zod-to-json-schema deduplicate to 4.6.7. Do not equate the manifest pin alone +with remediation of the consumer's resolved dependency. + +The [upstream fix, PR #6652](https://github.com/colinhacks/zod/pull/6652), rejects +oversized collections before parsing their elements when a leading upper-size +check applies. Later refinements do not run after that rejection. Unbounded +containers are unchanged. The regression exercises that narrow contract with +three-element collections, eight invalid array elements and one 101-element +SDK completion response, not an exhaustion-sized input or timing threshold. + +Baseline evidence on clean dependency base +`78a90cc7899e44c33aa27117bccb19ea7cbe85f6`, Node 22.23.4, Zod 3.25.76 and MCP +SDK 1.32.2: + +```text +npm ci --legacy-peer-deps --no-audit --no-fund +added 1222 packages; exit 0 + +npm run test -w @graphdone/mcp-server -- tests/zod-sdk-compatibility.test.ts +13 cases: 5 failed, 8 passed; exit 1 +array/set/map upper bounds: 3 element/key refinement calls, expected 0 +array length 2 with 8 invalid elements: 9 issues, expected one too_big issue +SDK completion maximum 100 with 101 invalid values: 102 issues, expected one +``` + +The final-fixture failing-before log is retained in the owned regression worktree at +`test-artifacts/audit-20261023/zod-sdk-baseline-final.log`; the initial run remains +in `zod-sdk-baseline.log` in the same directory. Install output is retained +as `/tmp/graphdone-zod-baseline-install.log`. The test is +[`packages/mcp-server/tests/zod-sdk-compatibility.test.ts`](../../packages/mcp-server/tests/zod-sdk-compatibility.test.ts). +After cherry-picking the separate manifest/lock correction +`b843a1baef4b5de1efbca3cd73c1a0346806afff` onto the regression branch, the +combined local head was `8d206e2cd2698082ee71d709cd86a3d3d0d7fc58`. A fresh +`npm ci --legacy-peer-deps --no-audit --no-fund` installed Zod 4.6.7 with unchanged +SDK 1.32.2. The same command and unchanged test file then passed **13/13**, +zero failures/skips, exit 0 in 441ms. All five previously failing bounded cases +passed; the eight SDK compatibility/control cases stayed passing. Logs are +`zod-patched-install.log` and `zod-sdk-patched.log` beside the retained baseline. +No production audit or complete application result is inferred from this +focused run. + +No GraphDone application source imports Zod directly. The MCP SDK already uses +`zod/v4` from the 3.25.76 package for the low-level `Server` and stdio schemas; +it accepts either Zod 3.25 or 4. The regression's in-memory SDK client/server +checks initialization, tool listing, nested arguments and metadata, absent +arguments, default result content, malformed calls, invalid results and later +recovery. A small stdio buffer checks invalid-envelope rejection, next-message +parsing and the configured byte cap. These fixture handlers do not start the +GraphDone entry point or prove GraphService/Neo4j behavior. + +Malformed call parameters reject before fixture dispatch. The SDK's existing +low-level parse path maps these schema failures to an MCP error; the tests do +not require a specific message or claim every request error is `InvalidParams`. +Invalid handler results do use `InvalidParams`, and the next valid result still +succeeds. Schema/default compatibility here does not establish every SDK API. + +Core registers `tools/list` and `tools/call`, not completion. The constrained +completion schema is an SDK regression surface, not proof of a reachable Core +completion endpoint. Tool argument declarations remain JSON schemas; upgrading +Zod does not make the low-level server enforce every declared tool constraint +or apply its JSON-schema defaults. Current tool results use an unbounded +content array, and the SDK's stdio byte cap is not a general allocation bound. + +Corrective rule: pair dependency-audit remediation with a small failing-before +behavior test and real consumer compatibility checks. Preserve accepted input, +default and failure behavior without relying on complete error-message text. +Do not describe this bounded-collection correction as complete request-resource +protection, a deployed fix, or a new authentication/data-isolation guarantee. + +The combined candidate `df1fac165a5f1421602c3706db8b86090b54a4f5` subsequently +passed all 603 unit cases (26 gated skips), 161 harness cases and nine real +Neo4j/entry-point contract cases with the separate stderr-diagnostic correction. +Its compiled production-mode MCP probe recorded zero protocol errors and +verified owned child/data cleanup. The fresh-cache production audit exited 0 +with eight moderate findings and no high/critical finding. The consolidated +[validation ledger](../../docs/quality/CORE_VALIDATION_2026-10-23.md#follow-up-the-fresh-production-audit-held-final-approval) +keeps this combined evidence separate from the focused SDK fixture and from +pending final public CI/review. diff --git a/.agents/issues/security-audit-index.md b/.agents/issues/security-audit-index.md index 18563b13..e28461dd 100644 --- a/.agents/issues/security-audit-index.md +++ b/.agents/issues/security-audit-index.md @@ -4,4 +4,5 @@ | --- | --- | | Green production audit hides development-tool findings or package counts are treated as exploit counts | [Audit scope lesson](../inbox/2026-10-23-dependency-audit-scope.md); [dated security scope](../../docs/quality/SECURITY_SCOPE_2026-10-23.md) | | Nested lint glob stalls the process | `tests/lib/dependencies/minimatch.test.mjs`; scoped parser matcher override and bounded child-process regression | +| A fresh audit reports Zod high severity after an earlier green scan; bounded collections still walk rejected elements | [Zod consumer and bounded-validation lesson](../inbox/2026-10-23-zod-constrained-collections.md); recheck the resolved SDK dependency and use an isolated audit cache without inferring why older metadata differed | | Browser login lockout is mistaken for server throttling; one-use email token succeeds concurrently | [Remaining authentication work](../../docs/quality/SECURITY_SCOPE_2026-10-23.md#remaining-authentication-work) | diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 60825793..73eae2d3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -124,6 +124,8 @@ jobs: - name: Install dependencies run: npm ci --legacy-peer-deps - name: Reject high and critical production dependency advisories + env: + NPM_CONFIG_CACHE: ${{ runner.temp }}/graphdone-audit-${{ github.run_id }}-${{ github.run_attempt }} run: npm audit --omit=dev --audit-level=high # ── THE GATE: the app works from a user's point of view ──────────────────── diff --git a/docs/quality/CORE_VALIDATION_2026-10-23.md b/docs/quality/CORE_VALIDATION_2026-10-23.md index 0034c3dc..4fe4d8ae 100644 --- a/docs/quality/CORE_VALIDATION_2026-10-23.md +++ b/docs/quality/CORE_VALIDATION_2026-10-23.md @@ -1,5 +1,47 @@ # Core merge validation — 2026-10-23 +## Follow-up: the fresh production audit held final approval + +The snapshot below remains historical evidence. On the subsequent clean merged +head `78a90cc7899e44c33aa27117bccb19ea7cbe85f6`, a new pinned installation passed +build, 590 unit tests (25 explicitly DB-gated skips), 161 harness tests, types, +lint, bundle and eight real Neo4j contract checks. Its production high/critical +audit instead exited 1 with eight moderate findings plus the Zod high advisory +[GHSA-mrhx-c7xc-m3w5](https://github.com/advisories/GHSA-mrhx-c7xc-m3w5). That +fresh failure blocked final approval despite the earlier green CI response. +The exact metadata/cache cause of the discrepancy is not established. + +The [bounded-validation and consumer-compatibility lesson](../../.agents/inbox/2026-10-23-zod-constrained-collections.md) +records the targeted Zod 4.6.7 correction and unchanged MCP SDK 1.32.2. The same +13 small regression fixtures fail five cases on the old dependency and pass +all 13 on the patch. The patch's build and production audit pass; a complete +unit run at `ea46e39` passes 603 cases with 25 DB-gated skips. The SDK's actual +dependency path is verified, including avoiding a nested patched copy that +would leave its hoisted dependency vulnerable. CI gives only the audit step a +fresh per-attempt cache; the threshold is unchanged. These results do not prove +unbounded-collection protection, full MCP denial-of-service resistance or a +hosted deployment. Final combined public CI and protected review remain pending. + +The subsequent combined candidate +`df1fac165a5f1421602c3706db8b86090b54a4f5` includes the separate MCP stdout +correction in [PR #179](https://github.com/GraphDone/GraphDone-Core/pull/179) +and the dependency correction in [PR #180](https://github.com/GraphDone/GraphDone-Core/pull/180). +A fresh install on that clean, unchanged source passes build, **603 unit cases +with 26 DB-gated skips**, **161 harness cases**, types, lint, bundle, and the +fresh-cache production audit (zero high/critical, eight moderate). All **nine +real Neo4j contract cases** pass, including the actual MCP entry point's +create/read/error/recovery path, verified fixture cleanup and zero protocol +parse errors through shutdown. A separate compiled production-mode MCP probe +also initializes, lists 31 tools, reads an owned two-node/one-edge graph, +rejects malformed/unknown calls and recovers, with zero stdout protocol errors +and confirmed child/edge/node/graph cleanup. It is a one-off probe, distinct +from the committed nine-case contract. Evidence is retained in +`core-mcp-final-candidate-20261023/test-artifacts/audit-20261023/{final-mcp-combined,zod-stdio}/`. +These local combined results do not replace final public-head CI or a real +non-author approval of protected PR #157. + +## Earlier functional integration snapshot + **Snapshot: all planned local gates pass and the scoped stack is merged into the public audit branch. Final public integration CI and protected `dev` merge approval remain PENDING.** Public functional integration: diff --git a/package-lock.json b/package-lock.json index e4c4b195..26c9e90a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -14178,7 +14178,9 @@ } }, "node_modules/zod": { - "version": "3.25.76", + "version": "4.6.7", + "resolved": "https://registry.npmjs.org/zod/-/zod-4.6.7.tgz", + "integrity": "sha512-g8W2D2kU6a8untc053fX1uVBKzM7yqAxW/3zwk83+6RyF2+SKOuV+c0wBDhUwzV1PaGapmZH+rmcUzPf+3CGwg==", "license": "MIT", "funding": { "url": "https://github.com/sponsors/colinhacks" @@ -14782,7 +14784,7 @@ "@modelcontextprotocol/sdk": "^1.30.0", "dotenv": "^16.3.0", "neo4j-driver": "^5.15.0", - "zod": "^3.22.0" + "zod": "4.6.7" }, "devDependencies": { "@types/node": "^20.10.0", diff --git a/packages/mcp-server/package.json b/packages/mcp-server/package.json index dc2aae00..1ad00c4e 100644 --- a/packages/mcp-server/package.json +++ b/packages/mcp-server/package.json @@ -24,7 +24,7 @@ "@modelcontextprotocol/sdk": "^1.30.0", "dotenv": "^16.3.0", "neo4j-driver": "^5.15.0", - "zod": "^3.22.0" + "zod": "4.6.7" }, "devDependencies": { "@types/node": "^20.10.0", diff --git a/packages/mcp-server/tests/zod-sdk-compatibility.test.ts b/packages/mcp-server/tests/zod-sdk-compatibility.test.ts new file mode 100644 index 00000000..14ac61ae --- /dev/null +++ b/packages/mcp-server/tests/zod-sdk-compatibility.test.ts @@ -0,0 +1,179 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import * as z from 'zod/v4'; +import { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import { Server } from '@modelcontextprotocol/sdk/server/index.js'; +import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'; +import { ReadBuffer } from '@modelcontextprotocol/sdk/shared/stdio.js'; +import { + CallToolRequestSchema, + CallToolResultSchema, + CompleteResultSchema, + ErrorCode, + ListToolsRequestSchema, + McpError, + type CallToolRequest, + type CallToolResult, + type Tool, +} from '@modelcontextprotocol/sdk/types.js'; + +describe('Zod constrained collection parsing', () => { + it('rejects an oversized array before element and later container refinements', () => { + const element = vi.fn(() => true); + const collection = vi.fn(() => true); + const schema = z.array(z.string().refine(element)).max(2).refine(collection); + + expect(schema.safeParse(['a', 'b', 'c']).success).toBe(false); + expect(element).not.toHaveBeenCalled(); + expect(collection).not.toHaveBeenCalled(); + + expect(schema.parse(['a', 'b'])).toEqual(['a', 'b']); + expect(element).toHaveBeenCalledTimes(2); + expect(collection).toHaveBeenCalledTimes(1); + }); + + it('rejects an oversized set before element and later container refinements', () => { + const element = vi.fn(() => true); + const collection = vi.fn(() => true); + const schema = z.set(z.string().refine(element)).max(2).refine(collection); + + expect(schema.safeParse(new Set(['a', 'b', 'c'])).success).toBe(false); + expect(element).not.toHaveBeenCalled(); + expect(collection).not.toHaveBeenCalled(); + + expect(schema.parse(new Set(['a', 'b']))).toEqual(new Set(['a', 'b'])); + expect(element).toHaveBeenCalledTimes(2); + expect(collection).toHaveBeenCalledTimes(1); + }); + + it('rejects an oversized map before parsing keys or values', () => { + const key = vi.fn(() => true); + const value = vi.fn(() => true); + const schema = z.map(z.string().refine(key), z.string().refine(value)).check(z.maxSize(2)); + + expect(schema.safeParse(new Map([['a', 'A'], ['b', 'B'], ['c', 'C']])).success).toBe(false); + expect(key).not.toHaveBeenCalled(); + expect(value).not.toHaveBeenCalled(); + + const valid = new Map([['a', 'A'], ['b', 'B']]); + expect(schema.parse(valid)).toEqual(valid); + expect(key).toHaveBeenCalledTimes(2); + expect(value).toHaveBeenCalledTimes(2); + }); + + it('returns one size issue instead of collecting each oversized array element error', () => { + const result = z.array(z.string()).length(2).safeParse(Array(8).fill(0)); + + expect(result.success).toBe(false); + if (result.success) throw new Error('Expected the array length to be rejected'); + expect(result.error.issues).toHaveLength(1); + expect(result.error.issues[0]).toMatchObject({ code: 'too_big', maximum: 2, path: [] }); + }); + + it('still validates invalid elements inside the allowed size', () => { + const result = z.array(z.string()).max(2).safeParse([0]); + + expect(result.success).toBe(false); + if (result.success) throw new Error('Expected the invalid element to be rejected'); + expect(result.error.issues).toHaveLength(1); + expect(result.error.issues[0]).toMatchObject({ code: 'invalid_type', path: [0] }); + }); + + it('enforces the SDK completion result bound before validating 101 elements', () => { + const result = CompleteResultSchema.safeParse({ completion: { values: Array(101).fill(0) } }); + + expect(result.success).toBe(false); + if (result.success) throw new Error('Expected the completion size to be rejected'); + expect(result.error.issues).toHaveLength(1); + expect(result.error.issues[0]).toMatchObject({ + code: 'too_big', maximum: 100, path: ['completion', 'values'], + }); + expect(CompleteResultSchema.parse({ completion: { values: ['valid'], hasMore: false } })) + .toEqual({ completion: { values: ['valid'], hasMore: false } }); + }); +}); + +describe('MCP SDK boundaries used by GraphDone', () => { + let server: Server; + let client: Client; + let response: unknown; + const onCall = vi.fn(async (_request: CallToolRequest): Promise => response as CallToolResult); + const tool: Tool = { + name: 'fixture_echo', + description: 'Return a controlled result for the SDK compatibility fixture', + inputSchema: { + type: 'object', + properties: { + title: { type: 'string' }, + priority: { type: 'number', minimum: 0, maximum: 1, default: 0.5 }, + metadata: { type: 'object', additionalProperties: true }, + }, + additionalProperties: false, + }, + }; + + beforeEach(async () => { + onCall.mockClear(); + response = { content: [{ type: 'text', text: 'accepted' }] }; + server = new Server({ name: 'graphdone-sdk-fixture', version: '1.0.0' }, { capabilities: { tools: {} } }); + client = new Client({ name: 'graphdone-sdk-fixture-client', version: '1.0.0' }); + server.setRequestHandler(ListToolsRequestSchema, async () => ({ tools: [tool] })); + server.setRequestHandler(CallToolRequestSchema, onCall); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + await server.connect(serverTransport); + await client.connect(clientTransport); + }); + + afterEach(async () => { + await client?.close(); + await server?.close(); + }); + + it('initializes, lists the JSON schema unchanged and preserves nested arguments and metadata', async () => { + expect(client.getServerVersion()).toEqual({ name: 'graphdone-sdk-fixture', version: '1.0.0' }); + expect(await client.listTools()).toEqual({ tools: [tool] }); + const args = { title: 'Owned fixture', metadata: { tags: ['a'], cleared: null, enabled: false } }; + const metadata = { progressToken: 'fixture-progress', 'fixture/context': { graph: 'owned' } }; + + expect(await client.callTool({ name: tool.name, arguments: args, _meta: metadata })).toEqual(response); + expect(onCall).toHaveBeenCalledTimes(1); + expect(onCall.mock.calls[0][0].params).toEqual({ name: tool.name, arguments: args, _meta: metadata }); + expect(onCall.mock.calls[0][0].params.arguments).not.toHaveProperty('priority'); + }); + + it('keeps omitted arguments absent and normalizes an omitted result content list', async () => { + response = {}; + + expect(await client.callTool({ name: tool.name })).toEqual({ content: [] }); + expect(onCall.mock.calls[0][0].params).toEqual({ name: tool.name }); + }); + + it.each([ + { name: 42, arguments: {} }, + { name: 'fixture_echo', arguments: [] }, + { name: 'fixture_echo', arguments: null }, + ])('rejects malformed tool-call params before dispatch: %j', async params => { + await expect(client.request({ method: 'tools/call', params }, CallToolResultSchema)) + .rejects.toBeInstanceOf(McpError); + expect(onCall).not.toHaveBeenCalled(); + }); + + it('reports an invalid result as a protocol error and accepts the next valid result', async () => { + response = { content: [{ type: 'text', text: 42 }] }; + + await expect(client.callTool({ name: tool.name })).rejects.toMatchObject({ code: ErrorCode.InvalidParams }); + response = { isError: true, content: [{ type: 'text', text: 'Fixture operation failed' }] }; + expect(await client.callTool({ name: tool.name })).toEqual(response); + expect(onCall).toHaveBeenCalledTimes(2); + }); + + it('rejects a malformed stdio envelope and then parses the next bounded message', () => { + const buffer = new ReadBuffer({ maxBufferSize: 256 }); + buffer.append(Buffer.from('{"jsonrpc":"2.0","id":true,"method":"tools/list"}\n')); + expect(() => buffer.readMessage()).toThrow(); + buffer.append(Buffer.from('{"jsonrpc":"2.0","id":1,"method":"tools/list"}\r\n')); + expect(buffer.readMessage()).toEqual({ jsonrpc: '2.0', id: 1, method: 'tools/list' }); + expect(buffer.readMessage()).toBeNull(); + expect(() => buffer.append(Buffer.alloc(257))).toThrow('maximum size of 256 bytes'); + expect(buffer.readMessage()).toBeNull(); + }); +});