diff --git a/CHANGELOG.md b/CHANGELOG.md index 16e98eb..156448b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,15 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [Unreleased] +## [1.1.0] - unreleased + +### Fixed + +- Reject unknown tool parameters instead of silently dropping them. Every + tool's advertised `inputSchema` now sets `additionalProperties: false`, and + every argument schema is `zod.strict()`, so an invented filter raises an + error naming the bad key and the tool's accepted parameters rather than + returning unfiltered results for a different question ([#10]) ## [1.0.2] - 2026-07-06 @@ -65,3 +73,4 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#4]: https://github.com/BetaNYC/nyc-record-mcp/pull/4 [#6]: https://github.com/BetaNYC/nyc-record-mcp/pull/6 [#7]: https://github.com/BetaNYC/nyc-record-mcp/pull/7 +[#10]: https://github.com/BetaNYC/nyc-record-mcp/issues/10 diff --git a/package-lock.json b/package-lock.json index 8d8ed25..58600b2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@betanyc/nyc-record-mcp", - "version": "1.0.2", + "version": "1.1.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@betanyc/nyc-record-mcp", - "version": "1.0.2", + "version": "1.1.0", "license": "MIT", "dependencies": { "@modelcontextprotocol/sdk": "^1.0.0", diff --git a/package.json b/package.json index 3b2422f..80a2618 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@betanyc/nyc-record-mcp", - "version": "1.0.2", + "version": "1.1.0", "description": "MCP server for NYC City Record notices via NYC Open Data", "keywords": [ "mcp", diff --git a/src/index.ts b/src/index.ts index bd78b32..aacb438 100644 --- a/src/index.ts +++ b/src/index.ts @@ -6,7 +6,6 @@ import { ListToolsRequestSchema, } from "@modelcontextprotocol/sdk/types.js"; import { createRequire } from "node:module"; -import { z } from "zod"; import { searchNotices, getNoticesByAgency, @@ -15,12 +14,8 @@ import { getPublicHearings, getOpenSolicitations, getNoticesByDateRange, - NOTICE_TYPES, } from "./city-record.js"; - -const isoDate = z - .string() - .regex(/^\d{4}-\d{2}-\d{2}$/, "must be a date in YYYY-MM-DD format"); +import { TOOLS, parseToolArgs } from "./tools.js"; const require = createRequire(import.meta.url); const { version } = require("../package.json") as { version: string }; @@ -30,100 +25,7 @@ const server = new Server( { capabilities: { tools: {} } } ); -server.setRequestHandler(ListToolsRequestSchema, async () => ({ - tools: [ - { - name: "search_notices", - description: - "Full-text search across all NYC City Record notices. Returns recent matching notices sorted by date.", - inputSchema: { - type: "object", - properties: { - query: { type: "string", description: "Search term" }, - limit: { type: "number", description: "Max results (default 25, max 100)" }, - }, - required: ["query"], - }, - }, - { - name: "get_notices_by_agency", - description: - "Get City Record notices published by a specific city agency (partial name match).", - inputSchema: { - type: "object", - properties: { - agency_name: { type: "string", description: "Agency name or partial name, e.g. 'DCAS', 'Parks'" }, - limit: { type: "number", description: "Max results (default 25, max 100)" }, - }, - required: ["agency_name"], - }, - }, - { - name: "get_notices_by_type", - description: - "Get notices filtered by type. Valid types: Solicitation, Award, Intent to Award, Intent to Negotiate, Public Hearings, Public Comment, Meeting, Notice, Vendor List, Sale.", - inputSchema: { - type: "object", - properties: { - notice_type: { - type: "string", - enum: [...NOTICE_TYPES], - description: "Notice type", - }, - limit: { type: "number", description: "Max results (default 25, max 100)" }, - }, - required: ["notice_type"], - }, - }, - { - name: "get_procurement_notices", - description: - "Get recent procurement-related notices: solicitations, awards, intent to award, vendor lists. Useful for tracking open contracts and recent awards.", - inputSchema: { - type: "object", - properties: { - limit: { type: "number", description: "Max results (default 25, max 100)" }, - }, - }, - }, - { - name: "get_public_hearings", - description: - "Get recent public hearings, public comment periods, and agency meetings from the City Record.", - inputSchema: { - type: "object", - properties: { - limit: { type: "number", description: "Max results (default 25, max 100)" }, - }, - }, - }, - { - name: "get_open_solicitations", - description: - "Get active solicitations (RFPs, RFQs, IFBs) where the due date has not yet passed. Sorted by due date ascending — soonest deadlines first.", - inputSchema: { - type: "object", - properties: { - limit: { type: "number", description: "Max results (default 25, max 100)" }, - }, - }, - }, - { - name: "get_notices_by_date_range", - description: - "Get all City Record notices published within a date range.", - inputSchema: { - type: "object", - properties: { - start_date: { type: "string", description: "Start date, YYYY-MM-DD" }, - end_date: { type: "string", description: "End date, YYYY-MM-DD" }, - limit: { type: "number", description: "Max results (default 50, max 200)" }, - }, - required: ["start_date", "end_date"], - }, - }, - ], -})); +server.setRequestHandler(ListToolsRequestSchema, async () => ({ tools: TOOLS })); server.setRequestHandler(CallToolRequestSchema, async (request) => { const { name, arguments: args } = request.params; @@ -131,61 +33,46 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => { try { switch (name) { case "search_notices": { - const { query, limit } = z - .object({ query: z.string(), limit: z.number().max(100).optional() }) - .parse(args); + const { query, limit } = parseToolArgs("search_notices", args); const results = await searchNotices(query, limit ?? 25); return { content: [{ type: "text", text: JSON.stringify(results, null, 2) }] }; } case "get_notices_by_agency": { - const { agency_name, limit } = z - .object({ agency_name: z.string(), limit: z.number().max(100).optional() }) - .parse(args); + const { agency_name, limit } = parseToolArgs("get_notices_by_agency", args); const results = await getNoticesByAgency(agency_name, limit ?? 25); return { content: [{ type: "text", text: JSON.stringify(results, null, 2) }] }; } case "get_notices_by_type": { - const { notice_type, limit } = z - .object({ notice_type: z.enum(NOTICE_TYPES), limit: z.number().max(100).optional() }) - .parse(args); + const { notice_type, limit } = parseToolArgs("get_notices_by_type", args); const results = await getNoticesByType(notice_type, limit ?? 25); return { content: [{ type: "text", text: JSON.stringify(results, null, 2) }] }; } case "get_procurement_notices": { - const { limit } = z - .object({ limit: z.number().max(100).optional() }) - .parse(args ?? {}); + const { limit } = parseToolArgs("get_procurement_notices", args); const results = await getProcurementNotices(limit ?? 25); return { content: [{ type: "text", text: JSON.stringify(results, null, 2) }] }; } case "get_public_hearings": { - const { limit } = z - .object({ limit: z.number().max(100).optional() }) - .parse(args ?? {}); + const { limit } = parseToolArgs("get_public_hearings", args); const results = await getPublicHearings(limit ?? 25); return { content: [{ type: "text", text: JSON.stringify(results, null, 2) }] }; } case "get_open_solicitations": { - const { limit } = z - .object({ limit: z.number().max(100).optional() }) - .parse(args ?? {}); + const { limit } = parseToolArgs("get_open_solicitations", args); const results = await getOpenSolicitations(limit ?? 25); return { content: [{ type: "text", text: JSON.stringify(results, null, 2) }] }; } case "get_notices_by_date_range": { - const { start_date, end_date, limit } = z - .object({ - start_date: isoDate, - end_date: isoDate, - limit: z.number().max(200).optional(), - }) - .parse(args); + const { start_date, end_date, limit } = parseToolArgs( + "get_notices_by_date_range", + args + ); const results = await getNoticesByDateRange(start_date, end_date, limit ?? 50); return { content: [{ type: "text", text: JSON.stringify(results, null, 2) }] }; } diff --git a/src/tools.ts b/src/tools.ts new file mode 100644 index 0000000..5514658 --- /dev/null +++ b/src/tools.ts @@ -0,0 +1,171 @@ +import type { Tool } from "@modelcontextprotocol/sdk/types.js"; +import { z } from "zod"; +import { NOTICE_TYPES } from "./city-record.js"; + +const isoDate = z + .string() + .regex(/^\d{4}-\d{2}-\d{2}$/, "must be a date in YYYY-MM-DD format"); + +/** Tool definitions advertised to clients by the ListTools handler. */ +export const TOOLS: Tool[] = [ + { + name: "search_notices", + description: + "Full-text search across all NYC City Record notices. Returns recent matching notices sorted by date.", + inputSchema: { + type: "object", + properties: { + query: { type: "string", description: "Search term" }, + limit: { type: "number", description: "Max results (default 25, max 100)" }, + }, + required: ["query"], + additionalProperties: false, + }, + }, + { + name: "get_notices_by_agency", + description: + "Get City Record notices published by a specific city agency (partial name match).", + inputSchema: { + type: "object", + properties: { + agency_name: { type: "string", description: "Agency name or partial name, e.g. 'DCAS', 'Parks'" }, + limit: { type: "number", description: "Max results (default 25, max 100)" }, + }, + required: ["agency_name"], + additionalProperties: false, + }, + }, + { + name: "get_notices_by_type", + description: + "Get notices filtered by type. Valid types: Solicitation, Award, Intent to Award, Intent to Negotiate, Public Hearings, Public Comment, Meeting, Notice, Vendor List, Sale.", + inputSchema: { + type: "object", + properties: { + notice_type: { + type: "string", + enum: [...NOTICE_TYPES], + description: "Notice type", + }, + limit: { type: "number", description: "Max results (default 25, max 100)" }, + }, + required: ["notice_type"], + additionalProperties: false, + }, + }, + { + name: "get_procurement_notices", + description: + "Get recent procurement-related notices: solicitations, awards, intent to award, vendor lists. Useful for tracking open contracts and recent awards.", + inputSchema: { + type: "object", + properties: { + limit: { type: "number", description: "Max results (default 25, max 100)" }, + }, + additionalProperties: false, + }, + }, + { + name: "get_public_hearings", + description: + "Get recent public hearings, public comment periods, and agency meetings from the City Record.", + inputSchema: { + type: "object", + properties: { + limit: { type: "number", description: "Max results (default 25, max 100)" }, + }, + additionalProperties: false, + }, + }, + { + name: "get_open_solicitations", + description: + "Get active solicitations (RFPs, RFQs, IFBs) where the due date has not yet passed. Sorted by due date ascending — soonest deadlines first.", + inputSchema: { + type: "object", + properties: { + limit: { type: "number", description: "Max results (default 25, max 100)" }, + }, + additionalProperties: false, + }, + }, + { + name: "get_notices_by_date_range", + description: "Get all City Record notices published within a date range.", + inputSchema: { + type: "object", + properties: { + start_date: { type: "string", description: "Start date, YYYY-MM-DD" }, + end_date: { type: "string", description: "End date, YYYY-MM-DD" }, + limit: { type: "number", description: "Max results (default 50, max 200)" }, + }, + required: ["start_date", "end_date"], + additionalProperties: false, + }, + }, +]; + +/** + * Argument schemas, one per tool name in TOOLS. + * + * Every schema is `.strict()`. zod strips unknown keys by default, which meant + * an invented filter (`council_district`, `vendor`, …) was silently discarded + * and the tool answered a different question with real, plausible data — see + * issue #10. Strict turns that into a loud failure. + */ +export const SCHEMAS = { + search_notices: z + .object({ query: z.string(), limit: z.number().max(100).optional() }) + .strict(), + get_notices_by_agency: z + .object({ agency_name: z.string(), limit: z.number().max(100).optional() }) + .strict(), + get_notices_by_type: z + .object({ + notice_type: z.enum(NOTICE_TYPES), + limit: z.number().max(100).optional(), + }) + .strict(), + get_procurement_notices: z + .object({ limit: z.number().max(100).optional() }) + .strict(), + get_public_hearings: z.object({ limit: z.number().max(100).optional() }).strict(), + get_open_solicitations: z + .object({ limit: z.number().max(100).optional() }) + .strict(), + get_notices_by_date_range: z + .object({ + start_date: isoDate, + end_date: isoDate, + limit: z.number().max(200).optional(), + }) + .strict(), +} satisfies Record; + +export type ToolName = keyof typeof SCHEMAS; + +/** + * Parse a tool call's arguments, rejecting any parameter the tool does not + * declare. zod's own strict message names the offending key but offers no + * alternative, so the accepted names are read back off the advertised schema. + */ +export function parseToolArgs( + name: K, + args: unknown +): z.infer<(typeof SCHEMAS)[K]> { + const result = SCHEMAS[name].safeParse(args ?? {}); + if (result.success) return result.data; + + const unknown = result.error.issues.find((i) => i.code === "unrecognized_keys"); + if (unknown) { + const accepted = Object.keys(TOOLS.find((t) => t.name === name)?.inputSchema.properties ?? {}); + throw new Error( + `${name} received unrecognized parameter(s): ${unknown.keys.join(", ")}. ` + + `It accepts only: ${accepted.join(", ")}. The call was rejected rather ` + + `than run without that filter, because dropping it silently would ` + + `return results for a different question.` + ); + } + throw result.error; +} diff --git a/test/strict-schema.test.mjs b/test/strict-schema.test.mjs new file mode 100644 index 0000000..f49b89d --- /dev/null +++ b/test/strict-schema.test.mjs @@ -0,0 +1,96 @@ +// Unknown tool parameters must be rejected, not silently dropped (issue #10). +// +// Runs entirely offline: it exercises the argument-validation layer, which is +// where the drop happened, and never reaches the Socrata City Record API. +import test from "node:test"; +import assert from "node:assert/strict"; +import { TOOLS, SCHEMAS, parseToolArgs } from "../dist/tools.js"; + +// Durable: holds for tools added later, not just today's seven. +test("every advertised tool sets additionalProperties: false", () => { + assert.ok(TOOLS.length > 0, "TOOLS must not be empty"); + for (const tool of TOOLS) { + assert.equal( + tool.inputSchema.additionalProperties, + false, + `${tool.name} must set additionalProperties:false so unknown params are rejected` + ); + } +}); + +test("every advertised tool has an argument schema, and vice versa", () => { + assert.deepEqual( + TOOLS.map((t) => t.name).sort(), + Object.keys(SCHEMAS).sort() + ); +}); + +// The behavioral test from issue #10: today the call succeeds and returns +// normal results; after the fix it raises. +test("a call carrying an undeclared parameter is rejected", () => { + assert.throws( + () => + parseToolArgs("get_open_solicitations", { + limit: 1, + bogus_unknown_param: "SHOULD_REJECT", + }), + /unrecognized|unknown|not permitted/i, + "an undeclared parameter must raise, not be silently dropped" + ); +}); + +test("the rejection names the bad key and the accepted parameters", () => { + assert.throws( + () => parseToolArgs("search_notices", { query: "x", council_district: 10 }), + (err) => { + assert.match(err.message, /council_district/); + assert.match(err.message, /\bquery\b/); + assert.match(err.message, /\blimit\b/); + return true; + } + ); +}); + +test("every tool rejects an undeclared parameter", () => { + const valid = { + search_notices: { query: "x" }, + get_notices_by_agency: { agency_name: "DCAS" }, + get_notices_by_type: { notice_type: "Solicitation" }, + get_procurement_notices: {}, + get_public_hearings: {}, + get_open_solicitations: {}, + get_notices_by_date_range: { start_date: "2026-01-01", end_date: "2026-01-31" }, + }; + for (const [name, args] of Object.entries(valid)) { + assert.throws( + () => parseToolArgs(name, { ...args, bogus_unknown_param: "x" }), + /unrecognized|unknown|not permitted/i, + `${name} accepted an undeclared parameter` + ); + } +}); + +// Regression guard: do not over-correct into rejecting valid calls. +test("valid calls still parse", () => { + assert.deepEqual(parseToolArgs("get_open_solicitations", { limit: 1 }), { limit: 1 }); + assert.deepEqual(parseToolArgs("get_open_solicitations", undefined), {}); + assert.deepEqual(parseToolArgs("search_notices", { query: "rfp", limit: 5 }), { + query: "rfp", + limit: 5, + }); + assert.deepEqual( + parseToolArgs("get_notices_by_date_range", { + start_date: "2026-01-01", + end_date: "2026-01-31", + }), + { start_date: "2026-01-01", end_date: "2026-01-31" } + ); +}); + +// Ordinary validation errors must keep their own messages. +test("a bad value for a declared parameter still reports normally", () => { + assert.throws( + () => parseToolArgs("get_notices_by_date_range", { start_date: "Jan 1", end_date: "2026-01-31" }), + /YYYY-MM-DD/ + ); +});