diff --git a/docs/mcp-contracts.md b/docs/mcp-contracts.md index e37aab52e2..011dc9826c 100644 --- a/docs/mcp-contracts.md +++ b/docs/mcp-contracts.md @@ -867,6 +867,12 @@ after SDK conversion and check advertised validation and actual calls. Individual model APIs can impose additional nesting limits; complete producer captures can exceed ten structural levels. +`search_bytes` advertises its pattern and mask as descriptive strings. Its +shared canonical validator scans byte pairs, whitespace, wildcards and aligned +masks without a repeating regular expression or temporary byte arrays. The +server rejects malformed or unconstrained patterns before provider dispatch; +the advertised strings alone do not express those rules. + Boolean literal constraints in input schemas are relaxed only in this advertisement. For example, a boolean `const` is presented as `type: boolean`; an affected `not` is omitted, conditional predicates and their branches are diff --git a/src/domain/native/bytePattern.test.ts b/src/domain/native/bytePattern.test.ts index 9d2cc92c11..296b425149 100644 --- a/src/domain/native/bytePattern.test.ts +++ b/src/domain/native/bytePattern.test.ts @@ -1,5 +1,14 @@ import { describe, expect, it } from "vitest"; -import { parseBytePattern, byteSearchResultSchema } from "./bytePattern.js"; +import { z } from "zod"; +import { + bytePatternFields, + validateBytePattern, + byteSearchResultSchema, +} from "./bytePattern.js"; + +const patternSchema = z + .strictObject(bytePatternFields) + .superRefine(validateBytePattern); it.each(["0x20", "overlay:0x10", "invalid"])( "rejects a match outside reported searched coverage: %s", @@ -53,15 +62,13 @@ it.each(["", "overlay:"])( describe("masked byte patterns", () => { it("preserves whole-byte wildcards and combines explicit bit masks", () => { - const parsed = parseBytePattern("48 8b\t??89", "ff f0 ff 0f"); - if (!parsed.ok) throw parsed.error; - expect(parsed.value).toEqual({ - bytes: [0x48, 0x8b, 0, 0x89], - masks: [255, 240, 0, 15], - }); - const exact = parseBytePattern("Aa\u00a000"); - if (!exact.ok) throw exact.error; - expect(exact.value).toEqual({ bytes: [170, 0], masks: [255, 255] }); + for (const input of [ + { pattern: "48 8b\t??89", mask: "ff f0 ff 0f" }, + { pattern: "Aa\u00a000" }, + { pattern: "00" }, + { pattern: "????aa", mask: "000001" }, + ]) + expect(patternSchema.parse(input)).toEqual(input); }); it.each([ @@ -73,11 +80,54 @@ describe("masked byte patterns", () => { ["ff", "00", "pattern"], ["ffff", "ff", "mask"], ["ff", "??", "mask"], + ["aa ??", "00 ff", "pattern"], + ["??", "0000", "mask"], + ["aa", "00gg", "mask"], + ["??gg", "00gg", "pattern"], ])("rejects invalid pattern %s with typed issues", (pattern, mask, field) => { - const parsed = parseBytePattern(pattern, mask); - if (parsed.ok) throw new Error("Expected a rejected pattern"); + const parsed = patternSchema.safeParse({ pattern, mask }); + expect(parsed.success).toBe(false); + if (parsed.success) throw new Error("Expected a rejected pattern"); expect(parsed.error.issues).toMatchObject([ - { path: [field], reason: "invalid_value" }, + { path: [field], code: "custom" }, ]); }); + + it("accepts ECMAScript whitespace only between complete byte pairs", () => { + const whitespace = [ + "\t", + "\n", + "\v", + "\f", + "\r", + " ", + "\u00a0", + "\u1680", + ...Array.from({ length: 11 }, (_, i) => String.fromCharCode(0x2000 + i)), + "\u2028", + "\u2029", + "\u202f", + "\u205f", + "\u3000", + "\ufeff", + ]; + for (const separator of whitespace) { + expect( + patternSchema.safeParse({ + pattern: `aa${separator}??`, + mask: `ff${separator}ff`, + }).success, + ).toBe(true); + for (const pattern of [ + `${separator}aa`, + `aa${separator}`, + `a${separator}a`, + ]) + expect(patternSchema.safeParse({ pattern }).success).toBe(false); + } + for (const separator of ["\u0085", "\u180e", "\u200b", "\u200c", "\u200d"]) + expect( + patternSchema.safeParse({ pattern: `aa${separator}bb` }).success, + ).toBe(false); + }); }); diff --git a/src/domain/native/bytePattern.ts b/src/domain/native/bytePattern.ts index 118102dcf5..d0ae144585 100644 --- a/src/domain/native/bytePattern.ts +++ b/src/domain/native/bytePattern.ts @@ -3,45 +3,96 @@ import { addressCoordinate } from "./addressCoordinate.js"; import { AnalysisInputError } from "../analysisErrorCore.js"; import { err, ok, type Result } from "../result.js"; -/** Byte pairs, with whole-byte wildcards; explicit masks can select individual bits. */ +/** Exact grammar belongs to the object refinement, not a repeating advertised regex. */ export const bytePatternFields = { pattern: z .string() - .regex(/^([0-9a-fA-F]{2}|\?\?)(\s*([0-9a-fA-F]{2}|\?\?))*$/u), + .describe( + "Hexadecimal byte pairs or ?? whole-byte wildcards, with optional whitespace between complete pairs; no leading or trailing whitespace. At least one bit must be constrained after applying the mask.", + ), mask: z .string() - .regex(/^[0-9a-fA-F]{2}(\s*[0-9a-fA-F]{2})*$/u) - .optional(), + .optional() + .describe( + "Hexadecimal mask byte pairs, with optional whitespace between complete pairs; no leading or trailing whitespace. Must contain the same number of bytes as pattern.", + ), }; -/** Parse once at the input boundary, retaining typed correction issues. */ -export const parseBytePattern = ( +const hexDigit = (code: number): number | undefined => { + if (code >= 48 && code <= 57) return code - 48; + if (code >= 65 && code <= 70) return code - 55; + if (code >= 97 && code <= 102) return code - 87; + return undefined; +}; + +const readBytePair = ( + value: string, + offset: number, +): number | "wildcard" | undefined => { + if (value[offset] === "?" && value[offset + 1] === "?") return "wildcard"; + const high = hexDigit(value.charCodeAt(offset)); + const low = hexDigit(value.charCodeAt(offset + 1)); + return high === undefined || low === undefined ? undefined : high * 16 + low; +}; + +const nextPairOffset = (value: string, offset: number): number => { + const end = offset + 2; + let next = end; + while (next < value.length && /\s/u.test(value[next]!)) next += 1; + return next === value.length && next !== end ? -1 : next; +}; + +const countBytePairs = ( + value: string, + field: "pattern" | "mask", +): number | undefined => { + let count = 0; + for (let offset = 0; offset < value.length;) { + const byte = readBytePair(value, offset); + if (byte === undefined || (field === "mask" && byte === "wildcard")) + return undefined; + count += 1; + offset = nextPairOffset(value, offset); + if (offset < 0) return undefined; + } + return count === 0 ? undefined : count; +}; + +/** Validate without materializing byte arrays or using size-dependent regexes. */ +const validateBytePatternValue = ( pattern: string, mask?: string, -): Result<{ bytes: number[]; masks: number[] }, AnalysisInputError> => { +): Result => { const invalid = (field: string, message: string) => err( new AnalysisInputError("search_bytes", undefined, [ { path: [field], reason: "invalid_value", message }, ]), ); - if (!bytePatternFields.pattern.safeParse(pattern).success) + const patternCount = countBytePairs(pattern, "pattern"); + if (patternCount === undefined) return invalid("pattern", "Use hexadecimal byte pairs or ?? wildcards"); - if (mask !== undefined && !bytePatternFields.mask.safeParse(mask).success) + const maskCount = + mask === undefined ? undefined : countBytePairs(mask, "mask"); + if (mask !== undefined && maskCount === undefined) return invalid("mask", "Use hexadecimal mask byte pairs"); - const pairs = pattern.replace(/\s/gu, "").match(/../gu) ?? []; - const maskPairs = mask?.replace(/\s/gu, "").match(/../gu); - if (maskPairs && maskPairs.length !== pairs.length) + if (maskCount !== undefined && maskCount !== patternCount) return invalid("mask", "Mask length must equal pattern length"); - const bytes = pairs.map((pair) => - pair === "??" ? 0 : Number.parseInt(pair, 16), - ); - const masks = pairs.map((pair, index) => - pair === "??" ? 0 : Number.parseInt(maskPairs?.[index] ?? "ff", 16), - ); - if (!masks.some((value) => value !== 0)) - return invalid("pattern", "Pattern must constrain at least one bit"); - return ok({ bytes, masks }); + let patternOffset = 0; + let maskOffset = 0; + for (let index = 0; index < patternCount; index += 1) { + const byte = readBytePair(pattern, patternOffset); + const selected = mask === undefined ? 255 : readBytePair(mask, maskOffset); + if ( + typeof byte === "number" && + typeof selected === "number" && + selected !== 0 + ) + return ok(null); + patternOffset = nextPairOffset(pattern, patternOffset); + if (mask !== undefined) maskOffset = nextPairOffset(mask, maskOffset); + } + return invalid("pattern", "Pattern must constrain at least one bit"); }; /** Shared semantic validation for public and provider inputs. */ @@ -49,7 +100,7 @@ export const validateBytePattern = ( value: { pattern: string; mask?: string | undefined }, context: z.RefinementCtx, ): void => { - const parsed = parseBytePattern(value.pattern, value.mask); + const parsed = validateBytePatternValue(value.pattern, value.mask); if (!parsed.ok) for (const issue of parsed.error.issues) context.addIssue({ diff --git a/src/ghidra/GhidraInventoryValues.test.ts b/src/ghidra/GhidraInventoryValues.test.ts index 42d0c262de..b66fed3ca5 100644 --- a/src/ghidra/GhidraInventoryValues.test.ts +++ b/src/ghidra/GhidraInventoryValues.test.ts @@ -5,6 +5,30 @@ import { parseGhidraInventoryResult, } from "./GhidraInventoryValues.js"; +it("validates large masked byte queries at the provider boundary without changing caller spelling", () => { + const input = { + pattern: "aA??".repeat(1024 * 1024), + mask: "ff00".repeat(1024 * 1024), + }; + const parsed = parseGhidraInventoryInput("search_bytes", input); + expect(parsed.ok).toBe(true); + if (!parsed.ok) throw parsed.error; + expect(parsed.value.pattern === input.pattern).toBe(true); + expect(parsed.value.mask === input.mask).toBe(true); + expect( + parseGhidraInventoryInput("search_bytes", { + ...input, + mask: `${input.mask.slice(0, -2)}g0`, + }).ok, + ).toBe(false); + expect( + parseGhidraInventoryInput("search_bytes", { + ...input, + mask: "00ff".repeat(1024 * 1024), + }).ok, + ).toBe(false); +}); + describe("provider-owned overview boundary", () => { const overview = { document: "fixture", diff --git a/tests/boundary/mcp/largeBytePattern.test.ts b/tests/boundary/mcp/largeBytePattern.test.ts new file mode 100644 index 0000000000..21d18778a2 --- /dev/null +++ b/tests/boundary/mcp/largeBytePattern.test.ts @@ -0,0 +1,63 @@ +import { expect, it } from "vitest"; +import type { CallToolResult } from "@modelcontextprotocol/server"; + +import { connectLocalToolsMcp } from "../../fixtures/localToolsMcp.js"; +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; + +const LARGE_PATTERN_BYTES = 4 * 1024 * 1024; + +const inputValidationText = (result: CallToolResult): string => { + expect(result.isError).toBe(true); + const text = result.content.find((item) => item.type === "text"); + if (text?.type !== "text") throw new Error("Input error omitted text"); + expect(text.text).toMatch( + /^Input validation error: Invalid arguments for tool search_bytes:/u, + ); + expect(text.text).not.toMatch(/RangeError|Maximum call stack/i); + return text.text; +}; + +it("validates large search_bytes inputs without breaking stdio MCP", async () => { + const { client, call } = await connectLocalToolsMcp(); + + const definition = (await client.listTools()).tools.find( + ({ name }) => name === "search_bytes", + ); + expect(definition?.inputSchema.properties?.pattern).toMatchObject({ + type: "string", + }); + expect(definition?.inputSchema.properties?.pattern).not.toHaveProperty( + "pattern", + ); + expect(definition?.inputSchema.properties?.mask).toMatchObject({ + type: "string", + }); + expect(definition?.inputSchema.properties?.mask).not.toHaveProperty( + "pattern", + ); + + const initial = await call("search_bytes", { pattern: "aa" }); + expect(parseMcpToolError(initial).error.code).toBe("target_unavailable"); + + const largePattern = "aa".repeat(LARGE_PATTERN_BYTES); + const largeSearch = await call("search_bytes", { pattern: largePattern }); + expect(parseMcpToolError(largeSearch).error.code).toBe("target_unavailable"); + + const malformedPattern = `${largePattern.slice(0, -2)}a?`; + const malformed = inputValidationText( + await call("search_bytes", { pattern: malformedPattern }), + ); + expect(malformed).toMatch(/pattern/i); + + const oversizedMask = inputValidationText( + await call("search_bytes", { + pattern: "aa", + mask: "ff".repeat(LARGE_PATTERN_BYTES), + }), + ); + expect(oversizedMask).toContain("Mask length must equal pattern length"); + + const final = await call("search_bytes", { pattern: "aa" }); + expect(parseMcpToolError(final).error.code).toBe("target_unavailable"); + await client.ping(); +});