From b67c1e8ce60d5279a16c403ac203ec7c6cdc0663 Mon Sep 17 00:00:00 2001 From: gensokyo-dev Date: Fri, 13 Nov 2026 16:52:20 +0000 Subject: [PATCH] fix(contracts): enforce well-formed Unicode paths without advertising surrogate patterns rea-agents 6.9.0 advertised `^[^\uD800-\uDFFF]*$` on every local path input. RE2 has no `\u` escape and Rust's `regex`/`jsonschema` crates refuse non-scalar code points, so strict MCP clients and gateways (DeepSeek, RE2 gateways, Open Design) rejected the complete tools request and no REA tool could run. Move the rule into one runtime check, `rejectUnpairedSurrogates`, which advertises nothing and still raises the `invalid_format` regex issue, so CLI and MCP corrections keep reason, expected value, and message. The objdump disassembly symbol already enforced the same rule with its own refinement; it now shares the owner and the correction shape. Dropping the pattern also brings inspect_android_jni_bindings within the compact profile's per-schema budget, so compact delivery now keeps its DEX dimension pattern. The descriptor admission test follows the actual presentation instead of assuming compact always omits it. Fixes #10238 Fixes #10348 Fixes #10198 Co-Authored-By: gensokyo-dev Co-Authored-By: Claude Opus 5.5 --- src/domain/localPath.ts | 18 +++-- src/domain/reverseEngineering.ts | 17 ++-- src/domain/stringPatterns.test.ts | 20 +++++ src/domain/stringPatterns.ts | 28 +++++++ .../mcp/absoluteLocalPathSchema.test.ts | 79 +++++++++++++++++++ .../mcp/androidDescriptorAdmissionMcp.test.ts | 10 ++- 6 files changed, 154 insertions(+), 18 deletions(-) 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({