From cd68181c90792775ef4b3fd4910982084f79c285 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Mon, 26 Oct 2026 22:01:51 +0800 Subject: [PATCH] fix(providers): preserve report bytes and dictionary facts --- src/contracts/toolOutputSchemaGroups.ts | 9 +-- src/contracts/toolOutputSchemaPrimitives.ts | 16 +++-- src/domain/analysisView/machoView.test.ts | 14 +++++ src/domain/native/nativeInspection.ts | 3 +- src/javascript/recovery/WakaruReport.test.ts | 61 ++++++++++++++++++- src/javascript/recovery/WakaruReport.ts | 15 ++--- src/native/parsers/otool.ts | 9 ++- tests/boundary/mcp/officialTools.test.ts | 31 ++++++++++ .../boundary/mcp/providerSelectionMcp.test.ts | 52 ++++++++++++++++ .../native/nativeMachoHeader.golden.test.ts | 41 +++++++++++++ 10 files changed, 225 insertions(+), 26 deletions(-) diff --git a/src/contracts/toolOutputSchemaGroups.ts b/src/contracts/toolOutputSchemaGroups.ts index 30149c2174..dd4ee73d91 100644 --- a/src/contracts/toolOutputSchemaGroups.ts +++ b/src/contracts/toolOutputSchemaGroups.ts @@ -2,10 +2,8 @@ import { nativeAddressListSchema, nativeObservationSourceSchemas, } from "../domain/native/nativeObservation.js"; -import { nativeObservationSummaryViewResultSchemas } from "../domain/analysisView/analysisView.js"; -import { literalFeatureMatchSchema } from "../domain/native/literalFeatureMatch.js"; -import { byteEntropySchema } from "../domain/native/byteEntropy.js"; import { + nativeObservationSummaryViewResultSchemas, byteEntropySummaryViewResultSchema, nativeFunctionMatchingSummaryViewResultSchema, nativeFunctionSummaryViewResultSchema, @@ -17,6 +15,8 @@ import { managedArtifactSummaryViewResultSchema, managedMemberSummaryViewResultSchema, } from "../domain/analysisView/analysisView.js"; +import { literalFeatureMatchSchema } from "../domain/native/literalFeatureMatch.js"; +import { byteEntropySchema } from "../domain/native/byteEntropy.js"; import { nativeFunctionMatchingSchema } from "../domain/native/functionMatching.js"; import { nativeByteReadSchema } from "../domain/native/nativeByteRead.js"; import { analysisDetailOutputSchemaOf } from "./analysisDetail.js"; @@ -31,6 +31,7 @@ import { } from "../domain/hopperValues.js"; import { nativeInventoryRowsSchemas } from "../domain/native/nativeInventory.js"; import { z } from "zod"; +import { jsonRecordSchema } from "../domain/jsonRecord.js"; import { byteSearchResultSchema } from "../domain/native/bytePattern.js"; import { evidenceBundleSummarySchema } from "../domain/evidenceRecordSummary.js"; import { @@ -243,7 +244,7 @@ export const officialOutputSchemas: Readonly> = { }), ), set_address_name: evidenceResultOf(z.boolean()), - set_addresses_names: evidenceResultOf(z.record(z.string(), z.boolean())), + set_addresses_names: evidenceResultOf(jsonRecordSchema(z.boolean())), set_bookmark: evidenceResultOf(z.boolean()), set_comment: evidenceResultOf(z.boolean()), set_inline_comment: evidenceResultOf(z.boolean()), diff --git a/src/contracts/toolOutputSchemaPrimitives.ts b/src/contracts/toolOutputSchemaPrimitives.ts index fc412344f6..3d2c7ca651 100644 --- a/src/contracts/toolOutputSchemaPrimitives.ts +++ b/src/contracts/toolOutputSchemaPrimitives.ts @@ -8,7 +8,7 @@ import { evidenceSchema, validateAnalysisProfileProvider, } from "../domain/evidence.js"; -import { jsonValueSchema } from "../domain/jsonValue.js"; +import { jsonObjectSchema } from "../domain/jsonValue.js"; import { PROVIDER_REJECTION_CODES, type ProviderRejectionCode, @@ -255,8 +255,6 @@ const providerRejectionCode: z.ZodType = z.enum( PROVIDER_REJECTION_CODES, ); -const providerDiagnostics = z.record(z.string(), jsonValueSchema); - const providerAvailabilityStates = { unknown: z.object({ status: z.literal("unknown"), @@ -277,13 +275,13 @@ const providerAvailabilityStates = { const providerAvailability = z.discriminatedUnion("status", [ providerAvailabilityStates.unknown.extend({ - diagnostics: providerDiagnostics, + diagnostics: jsonObjectSchema, }), providerAvailabilityStates.available.extend({ - diagnostics: providerDiagnostics, + diagnostics: jsonObjectSchema, }), providerAvailabilityStates.unavailable.extend({ - diagnostics: providerDiagnostics, + diagnostics: jsonObjectSchema, }), ]); @@ -313,13 +311,13 @@ const providerTargetSupportStates = { const providerTargetSupport = z.discriminatedUnion("status", [ providerTargetSupportStates.unknown.extend({ - diagnostics: providerDiagnostics, + diagnostics: jsonObjectSchema, }), providerTargetSupportStates.supported.extend({ - diagnostics: providerDiagnostics, + diagnostics: jsonObjectSchema, }), providerTargetSupportStates.unsupported.extend({ - diagnostics: providerDiagnostics, + diagnostics: jsonObjectSchema, }), ]); diff --git a/src/domain/analysisView/machoView.test.ts b/src/domain/analysisView/machoView.test.ts index c4a6220dfe..98361c0042 100644 --- a/src/domain/analysisView/machoView.test.ts +++ b/src/domain/analysisView/machoView.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from "vitest"; +import { inspectMachoSchema } from "../native/nativeInspection.js"; import { projectAnalysisView, @@ -269,3 +270,16 @@ describe("Mach-O analysis views", () => { if (!malformed.ok) expect(malformed.error._tag).toBe("AnalysisInputError"); }); }); + +it("validates a prototype-named command field rather than dropping an invalid value", () => { + const fields = + inspectMachoSchema.shape.load_commands.shape.items.element.shape.fields; + const parsed = fields.safeParse( + Object.fromEntries([["__proto__", { invalid: true }]]), + ); + expect(parsed.success).toBe(false); + if (parsed.success) throw new Error("Expected an invalid command field"); + expect(parsed.error.issues).toContainEqual( + expect.objectContaining({ path: ["__proto__"] }), + ); +}); diff --git a/src/domain/native/nativeInspection.ts b/src/domain/native/nativeInspection.ts index fab17ee30d..086028c17c 100644 --- a/src/domain/native/nativeInspection.ts +++ b/src/domain/native/nativeInspection.ts @@ -1,5 +1,6 @@ import { z } from "zod"; import { bundleResourceRootSchema } from "../apple/bundleResourceRoot.js"; +import { jsonRecordSchema } from "../jsonRecord.js"; import { jsonValueSchema } from "../jsonValue.js"; const hexAddress = z.string().regex(/^0x[a-fA-F0-9]+$/u); @@ -111,7 +112,7 @@ const dependencySchema = z.object({ const loadCommandSchema = z.object({ index: z.int().min(0), kind: z.string(), - fields: z.record(z.string(), z.union([z.string(), z.number(), z.null()])), + fields: jsonRecordSchema(z.union([z.string(), z.number(), z.null()])), }); const buildMetadataSchema = z.object({ platform: z.string().nullable(), diff --git a/src/javascript/recovery/WakaruReport.test.ts b/src/javascript/recovery/WakaruReport.test.ts index b6682d7052..5e4002d5f2 100644 --- a/src/javascript/recovery/WakaruReport.test.ts +++ b/src/javascript/recovery/WakaruReport.test.ts @@ -1,10 +1,14 @@ -import { truncate, writeFile } from "node:fs/promises"; +import { readFile, truncate, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { expect, it } from "vitest"; import { projectAnalysisError } from "../../domain/analysisErrorProjection.js"; import { ProviderAdapterError } from "../../domain/providerAdapterError.js"; import { createTestTempDirectory } from "../../../tests/fixtures/temporaryDirectory.js"; -import { parseWakaruReports } from "./WakaruReport.js"; +import { AnalysisOutputError } from "../../domain/analysisErrorCore.js"; +import { + parseWakaruReports, + validateRecoverySourceMap, +} from "./WakaruReport.js"; import { RECOVERY_LIMITS } from "./WakaruRelease.js"; const reportStdout = JSON.stringify({ @@ -73,3 +77,56 @@ it("keeps a malformed provenance report distinct from the fixed report allowance expect(JSON.stringify(projected)).not.toContain("report allowance"); expect(JSON.stringify(projected)).not.toContain("no runtime override"); }); + +it("rejects malformed UTF-8 provenance bytes and retains literal replacement characters", async () => { + const root = await createTestTempDirectory("rea-wakaru-provenance-utf8-"); + const provenancePath = join(root, "provenance.json"); + const prefix = Buffer.from('{"format":"'); + const suffix = Buffer.from('","strategy":"safe","modules":{}}'); + await writeFile( + provenancePath, + Buffer.concat([prefix, Buffer.from([0xff]), suffix]), + ); + const context = { + execution, + provenancePath, + snapshots: new Map(), + files: [{ relativePath: "provenance.json" }], + }; + const refusal: unknown = await parseWakaruReports(context).catch( + (cause: unknown) => cause, + ); + expect(refusal).toBeInstanceOf(ProviderAdapterError); + if (!(refusal instanceof ProviderAdapterError)) + throw new Error("Expected provider wrapper"); + expect(refusal.cause).toBeInstanceOf(TypeError); + await writeFile( + provenancePath, + Buffer.concat([prefix, Buffer.from("�"), suffix]), + ); + const accepted = await parseWakaruReports(context); + expect(accepted.provenance.format).toBe("�"); +}); + +it("rejects malformed UTF-8 emitted source maps without rejecting literal replacement characters", async () => { + const root = await createTestTempDirectory("rea-wakaru-sourcemap-utf8-"); + const mapPath = join(root, "module.js.map"); + const prefix = Buffer.from('{"version":3,"sources":["'); + const suffix = Buffer.from('"],"names":[],"mappings":""}'); + await writeFile( + mapPath, + Buffer.concat([prefix, Buffer.from([0xff]), suffix]), + ); + let refusal: unknown; + try { + validateRecoverySourceMap(await readFile(mapPath), "module.js"); + } catch (cause: unknown) { + refusal = cause; + } + expect(refusal).toBeInstanceOf(AnalysisOutputError); + if (!(refusal instanceof AnalysisOutputError)) + throw new Error("Expected source-map wrapper"); + expect(refusal.cause).toBeInstanceOf(TypeError); + await writeFile(mapPath, Buffer.concat([prefix, Buffer.from("�"), suffix])); + validateRecoverySourceMap(await readFile(mapPath), "module.js"); +}); diff --git a/src/javascript/recovery/WakaruReport.ts b/src/javascript/recovery/WakaruReport.ts index 99fc44ff2b..76af1c934e 100644 --- a/src/javascript/recovery/WakaruReport.ts +++ b/src/javascript/recovery/WakaruReport.ts @@ -1,4 +1,5 @@ import { z } from "zod"; +import { safeParseUtf8Json } from "../../domain/safeJson.js"; import { ArtifactReaderFailure } from "../../artifacts/ArtifactReader.js"; import type { ArtifactResourceScope } from "../../artifacts/ArtifactResourceScope.js"; import { @@ -78,13 +79,11 @@ export const parseWakaruReports = async (context: ReportContext) => { "Wakaru did not complete recovery (expected exit 0 or a reported partial exit 1)", ); const report = reportSchema.parse(JSON.parse(execution.stdout)); - const provenance = provenanceSchema.parse( - JSON.parse( - ( - await readWakaruProvenanceReport(provenancePath, signal, resources) - ).toString("utf8"), - ), + const decoded = safeParseUtf8Json( + await readWakaruProvenanceReport(provenancePath, signal, resources), ); + if (!decoded.ok) throw decoded.cause; + const provenance = provenanceSchema.parse(decoded.value); if (report.total !== report.modules.length || report.failed > report.total) throw new TypeError("Reported module totals disagree"); if ( @@ -249,7 +248,9 @@ export const validateRecoverySourceMap = ( mappings: z.string(), }); try { - schema.parse(JSON.parse(bytes.toString("utf8"))); + const decoded = safeParseUtf8Json(bytes); + if (!decoded.ok) throw decoded.cause; + schema.parse(decoded.value); } catch (cause: unknown) { throw new AnalysisOutputError( OPERATION, diff --git a/src/native/parsers/otool.ts b/src/native/parsers/otool.ts index d3dc96781b..8fd62a06b3 100644 --- a/src/native/parsers/otool.ts +++ b/src/native/parsers/otool.ts @@ -473,7 +473,7 @@ const parseFields = (block: string): ParsedFields => { for (const field of rows) if (field !== undefined) occurrences.set(field[0], (occurrences.get(field[0]) ?? 0) + 1); - const fields: Record = {}; + const fields = new Map(); const ordinals = new Map(); for (const field of rows) { if (field === undefined) continue; @@ -482,9 +482,12 @@ const parseFields = (block: string): ParsedFields => { ordinals.set(key, ordinal); const label = (occurrences.get(key) ?? 0) > 1 ? `${key} #${String(ordinal)}` : key; - fields[label] = TEXT_FIELDS.test(key) ? value : (numeric(value) ?? value); + fields.set( + label, + TEXT_FIELDS.test(key) ? value : (numeric(value) ?? value), + ); } - return { values: fields, rows }; + return { values: Object.fromEntries(fields), rows }; }; /** Names and linker options are text even when they look numeric. */ diff --git a/tests/boundary/mcp/officialTools.test.ts b/tests/boundary/mcp/officialTools.test.ts index 04187b0c9a..fa0e997b97 100644 --- a/tests/boundary/mcp/officialTools.test.ts +++ b/tests/boundary/mcp/officialTools.test.ts @@ -5,6 +5,8 @@ import { afterEach, describe, expect, it } from "vitest"; import type { AnalysisOperationPort } from "../../../src/application/AnalysisProvider.js"; import { OFFICIAL_TOOL_CONTRACTS } from "../../../src/contracts/officialToolContracts.js"; +import { toolContract } from "../../../src/contracts/toolContracts.js"; +import { jsonObjectSchema } from "../../../src/domain/jsonValue.js"; import { HopperRemoteError } from "../../../src/domain/hopperErrors.js"; import { err } from "../../../src/domain/result.js"; import { observed as ok } from "../../fixtures/analysisExecution.js"; @@ -231,6 +233,35 @@ describe("official tool input contracts", () => { }); describe("official Hopper proxy tools", () => { + it("delivers every batch naming readback key in structured and text Evidence", async () => { + const readback = JSON.parse( + '{"0x1000":true,"__proto__":false,"constructor":true}', + ); + const client = await connect({ + execute: () => Promise.resolve(ok(readback)), + }); + const response = await client.callTool({ + name: "set_addresses_names", + arguments: { + names: JSON.parse( + '{"0x1000":"entry","__proto__":"other","constructor":"named"}', + ), + }, + }); + expect(response.isError).toBeFalsy(); + const evidence = toolContract("set_addresses_names").outputSchema.parse( + response.structuredContent, + ); + const result = jsonObjectSchema.parse(evidence.normalized_result); + expect(result).toEqual(readback); + expect(Object.hasOwn(result, "__proto__")).toBe(true); + const text = response.content.find((item) => item.type === "text"); + if (text?.type !== "text") throw new Error("Missing serialized Evidence"); + expect( + jsonObjectSchema.parse(JSON.parse(text.text)).normalized_result, + ).toEqual(readback); + }); + it("returns stable safe MCP error content", async () => { const client = await connect({ execute: () => Promise.resolve(err(new HopperRemoteError(-1, "denied"))), diff --git a/tests/boundary/mcp/providerSelectionMcp.test.ts b/tests/boundary/mcp/providerSelectionMcp.test.ts index 5c617fac5a..a93013cdfb 100644 --- a/tests/boundary/mcp/providerSelectionMcp.test.ts +++ b/tests/boundary/mcp/providerSelectionMcp.test.ts @@ -7,6 +7,7 @@ import { AjvJsonSchemaValidator } from "@modelcontextprotocol/client/validators/ import type { CallToolResult } from "@modelcontextprotocol/server"; import { afterEach, describe, expect, it } from "vitest"; import { z } from "zod"; +import { jsonObjectSchema } from "../../../src/domain/jsonValue.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; @@ -48,6 +49,57 @@ afterEach(async () => { directory = undefined; }); +it("delivers provider-owned diagnostic keys without narrowing their JSON facts", async () => { + const diagnostics = jsonObjectSchema.parse( + JSON.parse( + '{"__proto__":{"observed":true},"constructor":null,"empty":"","count":0}', + ), + ); + const provider = candidate("alpha", []); + const session = composeBinarySession( + new AnalysisProviderRegistry([ + { + ...provider, + inspectAvailability: () => ({ + status: "available", + code: null, + reason: null, + diagnostics, + }), + }, + ]), + ); + const server = createServer( + { kind: "session", session }, + { logger: silentLogger }, + ); + const mcp = new Client({ name: "provider-diagnostics", version: "1" }); + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + resources.push(mcp, server, { close: () => session.close() }); + await server.connect(serverTransport); + await mcp.connect(clientTransport); + const response = await mcp.callTool({ + name: "binary_session", + arguments: { detail: "complete" }, + }); + expect(response.isError).toBeFalsy(); + const payload = toolContract("binary_session").outputSchema.parse( + response.structuredContent, + ); + const found = analysisProviderCandidate + .array() + .parse(payload.result.analysis_provider_candidates) + .find((entry) => entry.provider.id === "alpha"); + if (found === undefined) throw new Error("Missing provider candidate"); + expect(found.availability.diagnostics).toEqual(diagnostics); + expect(Object.hasOwn(found.availability.diagnostics, "__proto__")).toBe(true); + const text = response.content.find((item) => item.type === "text"); + if (text?.type !== "text") + throw new Error("Missing serialized session status"); + expect(JSON.parse(text.text)).toEqual(response.structuredContent); +}); + describe("provider selection over MCP", () => { it("keeps open_binary and binary_session binding semantics exact", async () => { directory = await createTestTempDirectory("rea-provider-mcp-"); diff --git a/tests/boundary/providers/native/nativeMachoHeader.golden.test.ts b/tests/boundary/providers/native/nativeMachoHeader.golden.test.ts index aeae2b523d..ed8bbdf872 100644 --- a/tests/boundary/providers/native/nativeMachoHeader.golden.test.ts +++ b/tests/boundary/providers/native/nativeMachoHeader.golden.test.ts @@ -1,4 +1,6 @@ import { expect, it } from "vitest"; +import { inspectMachoSchema } from "../../../../src/domain/native/nativeInspection.js"; +import { projectAnalysisView } from "../../../../src/domain/analysisView/analysisView.js"; import { NativeMacOSProvider } from "../../../../src/native/NativeMacOSProvider.js"; import { parseOtoolLoadCommands } from "../../../../src/native/parsers/otool.js"; @@ -332,3 +334,42 @@ it("preserves an unknown legacy platform spelling on Intel", () => { expect(result.builds[0]?.platform).toBe("FUTURE"); expect(result.commands[0]?.kind).toBe("LC_VERSION_MIN_FUTURE"); }); + +it("retains unknown otool field labels through the inspection contract and Mach-O page", async () => { + const output = (await nativeFixture("otool-dylib-use.txt")).replace( + /^(\s*cmdsize \d+)$/mu, + "$1\n __proto__ selected-source\n constructor 17\n repeated first\n repeated second", + ); + const parsed = parseOtoolLoadCommands(output); + const fields = parsed.commands[0]!.fields; + expect(Object.hasOwn(fields, "__proto__")).toBe(true); + expect(fields["__proto__"]).toBe("selected-source"); + expect(fields.constructor).toBe(17); + expect(fields["repeated #1"]).toBe("first"); + expect(fields["repeated #2"]).toBe("second"); + const runner = new NativeFixtureRunner({ "otool:-h": output }); + const client = new NativeMacOSProvider({}, runner, "darwin").createClient( + nativeMachoTarget("/owned/fixture"), + ); + const result = await client.execute("inspect_macho", {}); + if (!result.ok) throw result.error; + const observed = inspectMachoSchema.parse(result.value.result); + expect( + Object.hasOwn(observed.load_commands.items[0]!.fields, "__proto__"), + ).toBe(true); + const projected = projectAnalysisView( + { + evidenceId: `ev_${"c".repeat(64)}`, + operation: "inspect_macho", + normalizedResult: observed, + subject: null, + limitations: [], + }, + { kind: "macho", facet: "load_commands", offset: 0, limit: 64 }, + ); + if (!projected.ok) throw projected.error; + if (projected.value.kind !== "macho") throw new Error("Expected Mach-O view"); + expect(JSON.stringify(projected.value.item)).toBe( + JSON.stringify(parsed.commands), + ); +});