From 65b0e9e1035ad404df25bfb083773add393af092 Mon Sep 17 00:00:00 2001 From: Mark Stuart Date: Wed, 28 Oct 2026 04:20:36 +0000 Subject: [PATCH] fix: honor OpenAPI matching and validation boundaries --- src/matcher.ts | 43 +++++++- src/types.ts | 5 +- src/validator.ts | 153 +++++++++++++++++++++-------- tests/maintenance.test.ts | 200 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 354 insertions(+), 47 deletions(-) create mode 100644 tests/maintenance.test.ts diff --git a/src/matcher.ts b/src/matcher.ts index 05e617f..d64341d 100644 --- a/src/matcher.ts +++ b/src/matcher.ts @@ -2,9 +2,38 @@ import type { HttpMethod, MatchedOperation, OpenApiSpec, + Operation, + Parameter, PathItem, } from "./types.js"; +const HTTP_METHODS = new Set([ + "get", + "post", + "put", + "patch", + "delete", + "options", + "head", + "trace", +]); + +function inheritParameters( + operation: Operation, + inherited?: Parameter[] +): Operation { + if (!inherited?.length) { + return operation; + } + const parameters = new Map(); + for (const parameter of [...inherited, ...(operation.parameters ?? [])]) { + const name = + parameter.in === "header" ? parameter.name.toLowerCase() : parameter.name; + parameters.set(`${parameter.in}:${name}`, parameter); + } + return { ...operation, parameters: [...parameters.values()] }; +} + interface CompiledPath { paramNames: string[]; pathItem: PathItem; @@ -72,6 +101,9 @@ export function createMatcher(spec: OpenApiSpec) { pathname: string ): MatchedOperation | null { const normalizedMethod = method.toLowerCase() as HttpMethod; + if (!HTTP_METHODS.has(normalizedMethod)) { + return null; + } for (const entry of compiled) { const match = entry.regex.exec(pathname); @@ -82,16 +114,21 @@ export function createMatcher(spec: OpenApiSpec) { const operation = entry.pathItem[normalizedMethod]; if (!operation) { - continue; + return null; } const pathParams: Record = {}; for (let i = 0; i < entry.paramNames.length; i += 1) { - pathParams[entry.paramNames[i]] = decodePathParameter(match[i + 1]); + Object.defineProperty(pathParams, entry.paramNames[i], { + configurable: true, + enumerable: true, + value: decodePathParameter(match[i + 1]), + writable: true, + }); } return { - operation, + operation: inheritParameters(operation, entry.pathItem.parameters), pathParams, pathTemplate: entry.template, }; diff --git a/src/types.ts b/src/types.ts index 278b883..7599aea 100644 --- a/src/types.ts +++ b/src/types.ts @@ -11,9 +11,11 @@ export interface PathItem { get?: Operation; head?: Operation; options?: Operation; + parameters?: Parameter[]; patch?: Operation; post?: Operation; put?: Operation; + trace?: Operation; } export type HttpMethod = @@ -23,7 +25,8 @@ export type HttpMethod = | "patch" | "delete" | "options" - | "head"; + | "head" + | "trace"; export interface Operation { parameters?: Parameter[]; diff --git a/src/validator.ts b/src/validator.ts index 93cb8e2..096c7b7 100644 --- a/src/validator.ts +++ b/src/validator.ts @@ -1,4 +1,9 @@ -import type { MatchedOperation, SchemaObject, Violation } from "./types.js"; +import type { + MatchedOperation, + MediaType, + SchemaObject, + Violation, +} from "./types.js"; function now(): string { return new Date().toISOString(); @@ -30,6 +35,57 @@ function checkType(value: unknown, expectedType: string): boolean { } } +function selectMediaType( + content: Record, + contentType: string +): MediaType | undefined { + if (!contentType) { + return; + } + const entries = Object.entries(content); + for (const candidate of [ + contentType, + `${contentType.split("/")[0]}/*`, + "*/*", + ]) { + const selected = entries.find(([name]) => name.toLowerCase() === candidate); + if (selected) { + return selected[1]; + } + } +} + +function validateArrayItems( + body: unknown[], + schema: SchemaObject, + method: string, + path: string, + location: string +): Violation[] { + const { items } = schema; + if (!items) { + return []; + } + const violations: Violation[] = []; + for (const [index, value] of body.entries()) { + const itemLocation = `${location}[${index}]`; + if (items.type && !checkType(value, items.type)) { + violations.push({ + issue: `${itemLocation} expected type '${items.type}'`, + method, + path, + timestamp: now(), + type: "request", + }); + } else if (Array.isArray(value)) { + violations.push( + ...validateArrayItems(value, items, method, path, itemLocation) + ); + } + } + return violations; +} + /** * Validate a request body against the matched operation's schema. * Performs structural checks: required fields, unknown fields, scalar types. @@ -45,7 +101,7 @@ function validateBody( // Check required fields if (schema.required) { for (const field of schema.required) { - if (!(field in body)) { + if (!Object.hasOwn(body, field)) { violations.push({ issue: `Missing required field: ${field}`, method, @@ -57,37 +113,41 @@ function validateBody( } } + if (!schema.properties) { + return violations; + } + // Check unknown fields at top level - if (schema.properties) { - for (const key of Object.keys(body)) { - if (!(key in schema.properties)) { - violations.push({ - issue: `Unknown field: ${key}`, - method, - path, - timestamp: now(), - type: "request", - }); - } + for (const key of Object.keys(body)) { + if (!Object.hasOwn(schema.properties, key)) { + violations.push({ + issue: `Unknown field: ${key}`, + method, + path, + timestamp: now(), + type: "request", + }); } } // Check types for known properties - if (schema.properties) { - for (const [key, propSchema] of Object.entries(schema.properties)) { - if ( - key in body && - propSchema.type && - !checkType(body[key], propSchema.type) - ) { - violations.push({ - issue: `Field '${key}' expected type '${propSchema.type}', got '${typeof body[key]}'`, - method, - path, - timestamp: now(), - type: "request", - }); - } + for (const [key, propSchema] of Object.entries(schema.properties)) { + if ( + Object.hasOwn(body, key) && + propSchema.type && + !checkType(body[key], propSchema.type) + ) { + violations.push({ + issue: `Field '${key}' expected type '${propSchema.type}', got '${typeof body[key]}'`, + method, + path, + timestamp: now(), + type: "request", + }); + } else if (Object.hasOwn(body, key) && Array.isArray(body[key])) { + violations.push( + ...validateArrayItems(body[key], propSchema, method, path, key) + ); } } @@ -142,15 +202,16 @@ export async function validateRequest( return violations; } + if (!req.body) { + return violations; + } + // Check Content-Type matches one of the expected media types const expectedTypes = Object.keys(rb.content); - const baseContentType = contentType.split(";")[0].trim(); + const baseContentType = contentType.split(";")[0].trim().toLowerCase(); - if ( - baseContentType && - expectedTypes.length > 0 && - !expectedTypes.includes(baseContentType) - ) { + const selectedMediaType = selectMediaType(rb.content, baseContentType); + if (baseContentType && expectedTypes.length > 0 && !selectedMediaType) { violations.push({ issue: `Unexpected Content-Type '${baseContentType}', expected one of: ${expectedTypes.join(", ")}`, method, @@ -161,15 +222,11 @@ export async function validateRequest( } // Validate body structure for JSON - if ( - baseContentType === "application/json" && - // biome-ignore lint/suspicious/noUnnecessaryConditions: OpenAPI media types are dynamic despite the index signature. - rb.content["application/json"]?.schema - ) { + if (baseContentType === "application/json" && selectedMediaType?.schema) { try { const cloned = req.clone(); const body: unknown = await cloned.json(); - const { schema } = rb.content["application/json"]; + const { schema } = selectedMediaType; if (!schema) { return violations; } @@ -183,7 +240,17 @@ export async function validateRequest( }); return violations; } - if (typeof body === "object" && body !== null && !Array.isArray(body)) { + if (Array.isArray(body)) { + violations.push( + ...validateArrayItems( + body, + schema, + method, + pathname, + "Request body" + ) + ); + } else if (typeof body === "object" && body !== null) { violations.push( ...validateBody( body as Record, @@ -247,13 +314,13 @@ export function validateResponse( matched.operation.responses.default; if (responseSpec?.content) { const contentType = res.headers.get("content-type") || ""; - const baseContentType = contentType.split(";")[0].trim(); + const baseContentType = contentType.split(";")[0].trim().toLowerCase(); const expectedTypes = Object.keys(responseSpec.content); if ( baseContentType && expectedTypes.length > 0 && - !expectedTypes.includes(baseContentType) + !selectMediaType(responseSpec.content, baseContentType) ) { violations.push({ issue: `Unexpected response Content-Type '${baseContentType}', expected one of: ${expectedTypes.join(", ")}`, diff --git a/tests/maintenance.test.ts b/tests/maintenance.test.ts new file mode 100644 index 0000000..981645b --- /dev/null +++ b/tests/maintenance.test.ts @@ -0,0 +1,200 @@ +import assert from "node:assert/strict"; +import { it } from "node:test"; +import { createMatcher } from "../src/matcher.js"; +import type { + MatchedOperation, + OpenApiSpec, + Operation, + SchemaObject, +} from "../src/types.js"; +import { validateRequest, validateResponse } from "../src/validator.js"; + +function operation(): Operation { + return { responses: { "200": { description: "OK" } } }; +} +function spec(paths: OpenApiSpec["paths"]): OpenApiSpec { + return { info: { title: "test", version: "1" }, openapi: "3.1.1", paths }; +} +function matched(schema: SchemaObject, required = true): MatchedOperation { + return { + operation: { + ...operation(), + requestBody: { content: { "application/json": { schema } }, required }, + }, + pathParams: {}, + pathTemplate: "/items", + }; +} +function request(body?: string, contentType = "application/json"): Request { + return new Request("https://example.com/items", { + body, + headers: { "content-type": contentType }, + method: "POST", + }); +} + +it("checks required and unknown JSON fields as own properties", async () => { + const schema = { + properties: { name: { type: "string" } }, + required: ["toString"], + type: "object", + }; + const missing = await validateRequest( + request('{"name":"A"}'), + matched(schema), + "/items" + ); + assert.ok( + missing.some((v) => v.issue === "Missing required field: toString") + ); + const unknown = await validateRequest( + request('{"toString":"value"}'), + matched({ properties: { name: { type: "string" } }, type: "object" }), + "/items" + ); + assert.ok(unknown.some((v) => v.issue === "Unknown field: toString")); +}); +it("does not match inherited object methods as HTTP operations", () => { + const match = createMatcher( + spec({ "/items": { get: operation(), trace: operation() } }) + ); + for (const method of ["constructor", "toString", "__proto__", "parameters"]) { + assert.equal(match(method, "/items"), null); + } + assert.ok(match("GET", "/items")); + assert.ok(match("TRACE", "/items")); +}); +it("preserves prototype-named path parameters as own data", () => { + const match = createMatcher( + spec({ "/items/{__proto__}": { get: operation() } }) + ); + const result = match("GET", "/items/a%20b"); + assert.ok(result); + assert.equal(Object.hasOwn(result.pathParams, "__proto__"), true); + assert.equal( + Object.getOwnPropertyDescriptor(result.pathParams, "__proto__")?.value, + "a b" + ); + assert.equal(Object.getPrototypeOf(result.pathParams), Object.prototype); +}); +it("does not fall through concrete paths to another operation", () => { + const match = createMatcher( + spec({ + "/items/{id}": { get: operation() }, + "/items/me": { post: operation() }, + }) + ); + assert.equal(match("GET", "/items/me"), null); + assert.equal(match("POST", "/items/me")?.pathTemplate, "/items/me"); + assert.equal(match("GET", "/items/other")?.pathTemplate, "/items/{id}"); +}); +it("matches content media ranges and selects the most specific JSON schema", async () => { + const target = matched({ type: "object" }); + target.operation.requestBody = { + content: { "application/*": { schema: { type: "object" } } }, + }; + assert.deepEqual( + await validateRequest( + request("{}", "Application/JSON; charset=utf-8"), + target, + "/items" + ), + [] + ); + assert.equal( + (await validateRequest(request('"text"'), target, "/items")).length, + 1 + ); + target.operation.requestBody.content["application/json"] = { + schema: { type: "string" }, + }; + assert.deepEqual( + await validateRequest(request('"text"'), target, "/items"), + [] + ); + target.operation.responses["200"].content = { "text/*": {} }; + assert.deepEqual( + await validateResponse( + request("{}"), + new Response("ok", { headers: { "content-type": "Text/Plain" } }), + target, + "/items" + ), + [] + ); +}); +it("accepts absent optional JSON bodies without parsing them", async () => { + assert.deepEqual( + await validateRequest( + request(), + matched({ type: "object" }, false), + "/items" + ), + [] + ); + assert.equal( + ( + await validateRequest( + request("not json"), + matched({ type: "object" }, false), + "/items" + ) + ).length, + 1 + ); +}); +it("inherits path-level query parameters with operation overrides", async () => { + const op = operation(); + op.parameters = [{ in: "query", name: "search", required: false }]; + const match = createMatcher( + spec({ + "/items": { + get: op, + parameters: [ + { in: "query", name: "tenant", required: true }, + { in: "query", name: "search", required: true }, + ], + }, + }) + ); + const target = match("GET", "/items"); + assert.ok(target); + const missing = await validateRequest( + new Request("https://example.com/items"), + target, + "/items" + ); + assert.deepEqual( + missing.map((v) => v.issue), + ["Missing required query parameter: tenant"] + ); + assert.deepEqual( + await validateRequest( + new Request("https://example.com/items?tenant=a"), + target, + "/items" + ), + [] + ); + assert.equal(op.parameters.length, 1); +}); +it("checks array item types in top-level and property arrays", async () => { + const array = matched({ items: { type: "integer" }, type: "array" }); + assert.equal( + (await validateRequest(request("[1,2.5]"), array, "/items")).length, + 1 + ); + assert.deepEqual( + await validateRequest(request("[1,2]"), array, "/items"), + [] + ); + const object = matched({ + properties: { tags: { items: { type: "string" }, type: "array" } }, + type: "object", + }); + assert.equal( + (await validateRequest(request('{"tags":["a",3]}'), object, "/items")) + .length, + 1 + ); +});