From 9528d18d5b53c7fc24aec2b3af800cbd77379979 Mon Sep 17 00:00:00 2001 From: Levalicious <24482560+Levalicious@users.noreply.github.com> Date: Tue, 3 Nov 2026 16:35:13 -0500 Subject: [PATCH 1/2] fix(api): reject undeclared and off-enum tool arguments (policy R4) The server never validated call arguments against the advertised inputSchemas, so anything undeclared was silently dropped. In practice: open_nodes called with `cursor` (get_neighbors' page key) instead of `relationCursor` re-served page 1 with an identical nextCursor, making every relation past page 1 unreachable; an off-enum `direction` ("both") ran without complaint. dispatch now validates every call against the same tool definitions that ListTools advertises. Undeclared keys and off-enum values fail with VALIDATION_FAILED, listing every offender, and the tool does not run. A misnamed cursor key names the cursor parameter(s) the tool does take. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01B7UQfn7Vue83jAPFaWfmAZ --- server.ts | 85 +++++++++++++++++++++-- tests/arg-validation.test.ts | 128 +++++++++++++++++++++++++++++++++++ 2 files changed, 207 insertions(+), 6 deletions(-) create mode 100644 tests/arg-validation.test.ts diff --git a/server.ts b/server.ts index 11ea88c..defa767 100644 --- a/server.ts +++ b/server.ts @@ -403,6 +403,77 @@ function paginateGraph(graph: KnowledgeGraph, entityCursor: number | string = 0, }; } +/** The slice of a tool's JSON-Schema `inputSchema` that argument validation reads. */ +interface ToolArgSchema { + // `| undefined`: the inferred union of all tool schemas pads each one with + // the other tools' property names as optional-undefined keys. + properties?: Record; +} + +/** One rejected argument, reported in the VALIDATION_FAILED details list. */ +interface ArgViolation { + argument: string; + code: 'VALIDATION_FAILED'; + message: string; + allowed: unknown[]; +} + +/** + * Policy R4 (docs/api-error-policy.md) applied to arguments: an argument the + * tool does not declare, or a value outside a declared enum, is an error — not + * something to drop. Before this check the server ignored both silently, so a + * misnamed cursor (open_nodes `cursor` instead of `relationCursor`) re-served + * page 1 with the same nextCursor, and an off-enum `direction` ran as some + * other direction. + * + * Validate-first (R2): every offending argument is reported in one error and + * the tool does not run. + */ +export function validateToolArgs(tool: string, schema: ToolArgSchema, args: Record): void { + const properties = schema.properties ?? {}; + const declared = Object.keys(properties); + const violations: ArgViolation[] = []; + + for (const [key, value] of Object.entries(args)) { + const spec = Object.prototype.hasOwnProperty.call(properties, key) ? properties[key] : undefined; + if (spec === undefined) { + violations.push({ argument: key, code: 'VALIDATION_FAILED', message: unknownArgumentMessage(tool, key, declared), allowed: declared }); + continue; + } + const allowedValues = (spec as { enum?: unknown }).enum; + if (Array.isArray(allowedValues) && !allowedValues.includes(value)) { + violations.push({ + argument: key, + code: 'VALIDATION_FAILED', + message: `${JSON.stringify(value)} is not a valid ${key} for ${tool}; expected one of ${allowedValues.map(v => JSON.stringify(v)).join(', ')}.`, + allowed: [...allowedValues], + }); + } + } + + if (violations.length > 0) { + throw new ToolError( + 'VALIDATION_FAILED', + `${violations.length} argument(s) rejected for ${tool}; the tool did not run (details below).`, + violations, + ); + } +} + +/** + * Cursor parameter names differ between tools (`cursor` on single-list tools, + * `entityCursor` / `relationCursor` on graph-shaped ones), which is exactly + * the slip that hid open_nodes pages. Name the right key when that happens. + */ +function unknownArgumentMessage(tool: string, key: string, declared: string[]): string { + const base = `${JSON.stringify(key)} is not a parameter of ${tool}.`; + const cursorParams = declared.filter(k => /cursor$/i.test(k)); + if (/cursor$/i.test(key) && cursorParams.length > 0) { + return `${base} Its cursor parameter(s): ${cursorParams.join(', ')} — pass each nextCursor back under the key of the section it came from.`; + } + return `${base} Declared parameters: ${declared.join(', ')}.`; +} + // The KnowledgeGraphManager class contains all operations to interact with the knowledge graph export class KnowledgeGraphManager { private db: Store; @@ -1521,9 +1592,9 @@ export function createServer(memoryFilePath?: string): Server { knowledgeGraphManager.close(); }; -server.setRequestHandler(ListToolsRequestSchema, async () => { - return { - tools: [ + // The advertised tool list is also the argument contract: dispatch validates + // every call against these same inputSchemas (validateToolArgs, policy R4). + const toolDefinitions = [ { name: "create_entities", description: "Create multiple new entities in the knowledge graph", @@ -1874,9 +1945,9 @@ The file MUST be plaintext (.txt, .tex, .md, source code, etc.). For PDFs, use p required: ["filePath"], }, }, - ], - }; -}); + ]; + + server.setRequestHandler(ListToolsRequestSchema, async () => ({ tools: toolDefinitions })); /** * Dispatch a single tool call. Extracted from the request handler so the @@ -1889,6 +1960,8 @@ The file MUST be plaintext (.txt, .tex, .md, source code, etc.). For PDFs, use p * remains a JSON-RPC protocol error and is reserved for server faults. */ async function dispatch(name: string, args: Record): Promise { + const definition = toolDefinitions.find(t => t.name === name); + if (definition) validateToolArgs(name, definition.inputSchema, args); switch (name) { case "create_entities": { const result = await knowledgeGraphManager.createEntities(args.entities as Entity[]); diff --git a/tests/arg-validation.test.ts b/tests/arg-validation.test.ts new file mode 100644 index 0000000..d4ee4e7 --- /dev/null +++ b/tests/arg-validation.test.ts @@ -0,0 +1,128 @@ +import { describe, it, expect, beforeEach, afterEach } from '@jest/globals'; +import { promises as fs } from 'fs'; +import path from 'path'; +import os from 'os'; +import { createServer, type Relation } from '../server.js'; +import { createTestClient, callTool, callToolRaw, parseToolError, type PaginatedGraph } from './test-utils.js'; + +interface ArgViolation { + argument: string; + code: string; + message: string; + allowed: unknown[]; +} + +// Policy R4 applied to arguments: undeclared keys and off-enum values are +// rejected, never dropped. Regression: open_nodes called with `cursor` (the +// get_neighbors key) instead of `relationCursor` silently re-served page 1 +// with an identical nextCursor, making every edge past page 1 unreachable. +describe('Tool argument validation (R4: no silently dropped arguments)', () => { + let testDir: string; + let client: Awaited>['client']; + let cleanup: () => Promise; + + beforeEach(async () => { + testDir = await fs.mkdtemp(path.join(os.tmpdir(), 'mcp-memory-argval-')); + const server = createServer(path.join(testDir, 'test-memory.json')); + const result = await createTestClient(server); + client = result.client; + cleanup = result.cleanup; + }); + + afterEach(async () => { + await cleanup(); + try { + await fs.rm(testDir, { recursive: true }); + } catch { + // Ignore cleanup errors + } + }); + + async function seedHub(): Promise { + await callTool(client, 'create_entities', { + entities: [{ name: 'Hub', entityType: 'Test', observations: [] }], + }); + } + + it('rejects open_nodes `cursor` and names the cursor keys it does take', async () => { + await seedHub(); + const raw = await callToolRaw(client, 'open_nodes', { + names: ['Hub'], direction: 'backward', cursor: '31:950827ed', + }); + expect(raw.isError).toBe(true); + const err = parseToolError(raw); + expect(err.code).toBe('VALIDATION_FAILED'); + const details = err.details as ArgViolation[]; + expect(details).toHaveLength(1); + expect(details[0].argument).toBe('cursor'); + expect(details[0].message).toContain('entityCursor'); + expect(details[0].message).toContain('relationCursor'); + }); + + it('rejects get_neighbors `relationCursor` and names `cursor`', async () => { + await seedHub(); + const raw = await callToolRaw(client, 'get_neighbors', { entityName: 'Hub', relationCursor: '0:00000000' }); + const err = parseToolError(raw); + expect(err.code).toBe('VALIDATION_FAILED'); + const details = err.details as ArgViolation[]; + expect(details[0].argument).toBe('relationCursor'); + expect(details[0].message).toMatch(/cursor parameter\(s\): cursor\b/); + }); + + it('rejects an off-enum direction instead of running it as something else', async () => { + await seedHub(); + const raw = await callToolRaw(client, 'open_nodes', { names: ['Hub'], direction: 'both' }); + const err = parseToolError(raw); + expect(err.code).toBe('VALIDATION_FAILED'); + const details = err.details as ArgViolation[]; + expect(details).toHaveLength(1); + expect(details[0].argument).toBe('direction'); + expect(details[0].allowed).toEqual(['forward', 'backward', 'any']); + }); + + it('reports every offending argument in one error (validate-first)', async () => { + await seedHub(); + const raw = await callToolRaw(client, 'open_nodes', { + names: ['Hub'], direction: 'both', cursor: '31:950827ed', + }); + const details = parseToolError(raw).details as ArgViolation[]; + expect(details.map(d => d.argument).sort()).toEqual(['cursor', 'direction']); + }); + + it('a write with an undeclared argument changes nothing', async () => { + const raw = await callToolRaw(client, 'create_entities', { + entities: [{ name: 'NeverWritten', entityType: 'Test', observations: [] }], + dryRun: true, + }); + expect(parseToolError(raw).code).toBe('VALIDATION_FAILED'); + const open = await callToolRaw(client, 'open_nodes', { names: ['NeverWritten'] }); + expect(parseToolError(open).code).toBe('ENTITY_NOT_FOUND'); + }); + + it('drains a multi-page backward relation list via relationCursor', async () => { + await seedHub(); + const sources = Array.from({ length: 120 }, (_, i) => `Src${String(i).padStart(3, '0')}`); + await callTool(client, 'create_entities', { + entities: sources.map(name => ({ name, entityType: 'Test', observations: [] })), + }); + await callTool(client, 'create_relations', { + relations: sources.map(from => ({ from, to: 'Hub', relationType: 'POINTS_AT' })), + }); + + const seen: Relation[] = []; + let relationCursor: string | number | null = 0; + let pages = 0; + while (relationCursor !== null && pages < 50) { + const page = await callTool(client, 'open_nodes', { + names: ['Hub'], direction: 'backward', relationCursor, + }) as PaginatedGraph; + seen.push(...page.relations.items); + relationCursor = page.relations.nextCursor; + pages++; + } + + expect(pages).toBeGreaterThan(1); + expect(new Set(seen.map(r => r.from)).size).toBe(120); + expect(seen).toHaveLength(120); + }); +}); From c9235410b2e72bee1995567464ce68ff6b9417a1 Mon Sep 17 00:00:00 2001 From: Levalicious <24482560+Levalicious@users.noreply.github.com> Date: Tue, 3 Nov 2026 16:36:39 -0500 Subject: [PATCH 2/2] docs(api): R4 covers tool arguments (undeclared keys, off-enum values) Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01B7UQfn7Vue83jAPFaWfmAZ --- docs/api-error-policy.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/api-error-policy.md b/docs/api-error-policy.md index 6250e75..2524392 100644 --- a/docs/api-error-policy.md +++ b/docs/api-error-policy.md @@ -49,6 +49,10 @@ Every mutating success enumerates per-item outcomes An item that cannot be satisfied errors (R1) unless the caller's intent is provably satisfied — duplicates are "already there", deleting an absent thing is "already absent". Those cases succeed **and are reported** (R3). +This covers arguments too: a key the tool's `inputSchema` does not declare, +or a value outside a declared `enum`, is `VALIDATION_FAILED` (every offender +listed, the tool does not run) — never ignored. A misnamed cursor key names +the cursor parameter(s) the tool actually takes. **R5 — Pagination integrity.** Cursors identify the result set they came from (a fingerprint now; a store