diff --git a/src/domain/localPath.ts b/src/domain/localPath.ts index 24f1f60e3b..2761e98719 100644 --- a/src/domain/localPath.ts +++ b/src/domain/localPath.ts @@ -1,6 +1,8 @@ import { isAbsolute, relative, sep } from "node:path"; import { z } from "zod"; +import { rejectUnpairedSurrogates } from "./stringPatterns.js"; + /** * Reject OS-invalid NUL bytes in a local filesystem name. This is not a path * shape check: an empty or relative value is admitted here and narrowed by the @@ -16,13 +18,15 @@ export const nulFreeStringSchema = z .string() .regex(/^[^\x00]*$/u, "Local filesystem paths cannot contain NUL"); -/** A selected local filesystem path: nonempty, NUL-free, and representable without replacement. */ -export const localPathStringSchema = nulFreeStringSchema - .min(1) - .regex( - /^[^\uD800-\uDFFF]*$/u, - "Local filesystem paths must contain well-formed Unicode", - ); +/** + * A selected local filesystem path: nonempty, NUL-free, and representable + * without replacement. The well-formed Unicode rule is enforced but not + * advertised; see `rejectUnpairedSurrogates`. + */ +export const localPathStringSchema = rejectUnpairedSurrogates( + nulFreeStringSchema.min(1), + "Local filesystem paths must contain well-formed Unicode", +); /** * A Windows absolute path: rooted, UNC, or drive-qualified. The classes spell diff --git a/src/domain/reverseEngineering.ts b/src/domain/reverseEngineering.ts index db0420d4a3..f21d0c8558 100644 --- a/src/domain/reverseEngineering.ts +++ b/src/domain/reverseEngineering.ts @@ -3,6 +3,7 @@ import { constants as bufferConstants } from "node:buffer"; import { commandEvidenceDetailSchema } from "./commandObservation.js"; import { localPathStringSchema } from "./localPath.js"; import { RAW_BINARY_ENTRY_POINT } from "./rawBinary.js"; +import { rejectUnpairedSurrogates } from "./stringPatterns.js"; /** Omitted one-shot objdump/rizin wall clock. Preflight shares it; the command gets it again. */ export const DEFAULT_REVERSE_COMMAND_TIMEOUT_MS = 120_000; @@ -62,15 +63,13 @@ const objdumpArtifactInputSchema = z const objdumpDisassemblyInputSchema = objdumpArtifactInputSchema.extend({ operation: z.literal("disassemble"), - symbol: z - .string() - .min(1) - .regex(/^[^\x00]*$/u) - // Keep UTF-16 checks out of patterns consumed by scalar Unicode engines. - .refine( - (symbol) => !/[\uD800-\uDFFF]/u.test(symbol), - "Symbol must be well-formed Unicode.", - ) + symbol: rejectUnpairedSurrogates( + z + .string() + .min(1) + .regex(/^[^\x00]*$/u), + "Symbol must be well-formed Unicode.", + ) .describe( "Native symbol to disassemble, expressed as well-formed Unicode. Matching and symbol boundaries follow the selected executable; omitted means all code sections.", ) diff --git a/src/domain/stringPatterns.test.ts b/src/domain/stringPatterns.test.ts index 4dd634d7a7..e96d87dd29 100644 --- a/src/domain/stringPatterns.test.ts +++ b/src/domain/stringPatterns.test.ts @@ -12,6 +12,7 @@ import { canonicalBase64Schema, isCanonicalBase64, isPaddedBase64, + rejectUnpairedSurrogates, } from "./stringPatterns.js"; it("preserves identifier grammars under Unicode and Unicode-set compilation", () => { @@ -154,3 +155,22 @@ it("validates complete projected canonical schemas at the encoded-input budget", expect(validate(value)).toBe(expected); } }); + +it("rejects unpaired surrogates at runtime without advertising a pattern", () => { + const schema = rejectUnpairedSurrogates(z.string(), "Must be well-formed"); + for (const value of ["\ud800", "a\udc00", "\udc00\ud800", "\ud83d"]) + expect( + schema.safeParse(value).error?.issues, + JSON.stringify(value), + ).toEqual([ + { + code: "invalid_format", + format: "regex", + message: "Must be well-formed", + path: [], + }, + ]); + for (const value of ["", "plain", "\ud83d\ude00", "\ufffd", "\uffff"]) + expect(schema.safeParse(value).success, JSON.stringify(value)).toBe(true); + expect(z.toJSONSchema(schema, { io: "input" })).not.toHaveProperty("pattern"); +}); diff --git a/src/domain/stringPatterns.ts b/src/domain/stringPatterns.ts index b97d08e45c..71424570f0 100644 --- a/src/domain/stringPatterns.ts +++ b/src/domain/stringPatterns.ts @@ -17,6 +17,34 @@ export const INVALID_BASE64_PADDING_PATTERN = /(?:=[^=]|===)/; export const INVALID_CANONICAL_BASE64_PAD_BITS_PATTERN = /(?:[^AQgw]==$|[BCDFGHJKLNOPRSTVWXZabdefhijlmnpqrtuvxyz1235679+\x2f]=$)/; +// In `u` mode a well-formed surrogate pair is one code point outside this +// range, so the class matches only unpaired surrogates. +const UNPAIRED_SURROGATE_PATTERN = /[\uD800-\uDFFF]/u; + +/** + * Reject unpaired UTF-16 surrogates without advertising a pattern. + * + * ECMA-262 spells the surrogate block only as `\uD800-\uDFFF`. RE2 has no `\u` + * escape and Rust's `regex` and `jsonschema` crates refuse non-scalar code + * points, and a client that cannot compile one advertised pattern refuses the + * complete tools request rather than the affected call. The issue keeps the + * `invalid_format` regex shape an advertised `.regex` raises, so input issue + * projection still reports `message`. + */ +export const rejectUnpairedSurrogates = ( + schema: Schema, + message: string, +): Schema => + schema.superRefine((value, context) => { + if (UNPAIRED_SURROGATE_PATTERN.test(value)) + context.addIssue({ + code: "invalid_format", + format: "regex", + message, + input: value, + }); + }); + export const isPaddedBase64 = (value: string): boolean => value.length % 4 === 0 && !INVALID_BASE64_CHARACTER_PATTERN.test(value) && diff --git a/tests/boundary/mcp/absoluteLocalPathSchema.test.ts b/tests/boundary/mcp/absoluteLocalPathSchema.test.ts index 329daf21c8..da46d49764 100644 --- a/tests/boundary/mcp/absoluteLocalPathSchema.test.ts +++ b/tests/boundary/mcp/absoluteLocalPathSchema.test.ts @@ -353,3 +353,82 @@ it("rejects relative paths through in-memory MCP handlers", async () => { await close(); } }); + +it("rejects unpaired surrogates through in-memory MCP handlers without advertising them", async () => { + const firmwareCalls: string[] = []; + const { client, close } = await createMcp(firmwareCalls, "full"); + try { + const absolute = resolve("rea-absolute-input"); + const cases = inputCases(absolute, `${absolute}\ud800`); + const listed = await client.listTools(); + const checked: string[] = []; + for (const { name, field, invalid, issuePath } of cases) { + const schema = listed.tools.find( + (tool) => tool.name === name, + )?.inputSchema; + if (schema === undefined) + throw new Error(`Missing advertised tool ${name}`); + // Strict pattern compilers refuse the complete tools request for a + // `\uD800-\uDFFF` range, so the rule is enforced only by the handler. + expect(JSON.stringify(schema), name).not.toMatch(/\\\\u[dD][89a-fA-F]/u); + const result = await client.callTool({ name, arguments: invalid }); + expect(result.isError, `${name}.${field}`).toBe(true); + expect(parseMcpToolError(result).error, `${name}.${field}`).toMatchObject( + { + code: "invalid_request", + details: { + operation: name, + issues: [ + { + path: issuePath, + reason: "invalid_format", + expected: "regex", + message: + "Local filesystem paths must contain well-formed Unicode", + }, + ], + }, + }, + ); + checked.push(`${name}.${field}`); + } + expect(checked).toEqual( + expect.arrayContaining([ + "open_binary.path", + "open_binary.snapshot_path", + "extract_firmware.path", + "extract_firmware.output_directory", + "inspect_analysis_view.source.path", + "capture_browser_scenario.browser.executable_path", + "capture_electron_scenario.application_root", + ]), + ); + + // Disassembly symbols share the rule and its correction shape. + const symbol = await client.callTool({ + name: "inspect_with_objdump", + arguments: { + path: absolute, + operation: "disassemble", + symbol: "main\ud800", + }, + }); + expect(symbol.isError).toBe(true); + expect(parseMcpToolError(symbol).error).toMatchObject({ + code: "invalid_request", + details: { + issues: [ + { + path: ["symbol"], + reason: "invalid_format", + expected: "regex", + message: "Symbol must be well-formed Unicode.", + }, + ], + }, + }); + expect(firmwareCalls).toEqual([]); + } finally { + await close(); + } +}); diff --git a/tests/boundary/mcp/androidDescriptorAdmissionMcp.test.ts b/tests/boundary/mcp/androidDescriptorAdmissionMcp.test.ts index d9ac95dfe4..dbbe215650 100644 --- a/tests/boundary/mcp/androidDescriptorAdmissionMcp.test.ts +++ b/tests/boundary/mcp/androidDescriptorAdmissionMcp.test.ts @@ -29,6 +29,13 @@ it.each(["compact", "full"])( const ajv = new Ajv2020({ strict: false, validateFormats: false }); expect(ajv.validateSchema(definition.inputSchema)).toBe(true); const validate = ajv.compile(definition.inputSchema); + // Compact delivery keeps validation keywords for a schema within its byte + // budget and omits nested detail only from a reduced schema, so the + // advertised dimension bound depends on the presentation, not the profile. + const advertisesDimensionBound = JSON.stringify( + definition.inputSchema, + ).includes("[{0,255}"); + expect(profile === "compact" || advertisesDimensionBound).toBe(true); for (const dimensions of [255, 256]) { const type = "[".repeat(dimensions) + "I"; const request = { @@ -36,8 +43,7 @@ it.each(["compact", "full"])( descriptors: [`(${type})${type}`], }; expect(validate(request), JSON.stringify(validate.errors)).toBe( - // Compact delivery intentionally omits scalar pattern constraints. - profile === "compact" || dimensions === 255, + dimensions === 255 || !advertisesDimensionBound, ); if (dimensions === 256) { const result = await client.callTool({