diff --git a/src/application/BinaryOverview.test.ts b/src/application/BinaryOverview.test.ts index 6c38350b36..310f9e8373 100644 --- a/src/application/BinaryOverview.test.ts +++ b/src/application/BinaryOverview.test.ts @@ -6,6 +6,7 @@ import { AnalysisCapabilityUnavailableError, AnalysisOutputError, } from "../domain/analysisErrorCore.js"; +import type { JsonValue } from "../domain/jsonValue.js"; import { err, ok } from "../domain/result.js"; it("uses the provider-owned overview without requesting complete inventories", async () => { @@ -59,3 +60,93 @@ it.each([ expect(calls).toEqual(["read_binary_inventory_summary"]); }, ); + +const fallbackOverview = (procedures: JsonValue, strings: JsonValue) => + new EnhancedTools({ + execute: async (operation) => { + if ( + operation === "read_binary_inventory_summary" || + operation === "list_segments" + ) + return err( + new AnalysisCapabilityUnavailableError( + "fixture", + operation, + "Not supported", + ), + ); + const result = + operation === "current_document" + ? "selected" + : operation === "list_procedures" + ? procedures + : strings; + return ok( + createAnalysisExecution(result, { + id: "fixture", + name: "Fixture", + version: "1", + }), + ); + }, + }); + +it.each(["list_procedures", "list_strings"] as const)( + "rejects malformed successful %s inventories in the public overview workflow", + async (operation) => { + for (const malformed of [ + { error: "inventory unavailable" }, + { "0x1000": "obsolete map" }, + { [operation === "list_procedures" ? "procedures" : "strings"]: [] }, + JSON.parse('{"__proto__":"observed"}'), + [null], + [{ address: "0x1000" }], + [{ address: "0x1000", value: 1 }], + ]) { + const tools = fallbackOverview( + operation === "list_procedures" ? malformed : [], + operation === "list_strings" ? malformed : [], + ); + const result = await tools.execute("binary_overview", {}); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toBeInstanceOf(AnalysisOutputError); + if (result.error instanceof AnalysisOutputError) + expect(result.error.operation).toBe(operation); + } + } + }, +); + +it("counts canonical fallback inventories including typed and display-only strings", async () => { + const tools = fallbackOverview( + [{ address: "0x1000", value: "entry" }], + [ + { address: "0x2000", value: "display-only" }, + { + address: "0x2000", + value: "typed", + provider_value: "source display", + string: { + encoding: "utf-8", + termination: "present_or_not_required", + byte_length: 5, + }, + }, + ], + ); + expect(await tools.execute("binary_overview", {})).toEqual( + ok({ + document: "selected", + procedure_count: 1, + string_count: 2, + }), + ); + expect(await fallbackOverview([], []).execute("binary_overview", {})).toEqual( + ok({ + document: "selected", + procedure_count: 0, + string_count: 0, + }), + ); +}); diff --git a/src/application/EnhancedTools.ts b/src/application/EnhancedTools.ts index 89179d6c08..a23d66aa20 100644 --- a/src/application/EnhancedTools.ts +++ b/src/application/EnhancedTools.ts @@ -23,7 +23,7 @@ import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import type { AnalysisError } from "../domain/analysisErrorBase.js"; import { parseFunctionDossier } from "../domain/native/functionDossier.js"; import { - parseListCount, + parseNativeInventoryCount, addressedValueSchema, } from "../domain/native/nativeInventory.js"; import { parseRelatedAddresses } from "../domain/native/nativeObservation.js"; @@ -763,9 +763,15 @@ export class EnhancedTools { ? parseSegments(segmentsResult.value) : undefined; if (segments !== undefined && !segments.ok) return segments; - const procedureCount = parseListCount(proceduresResult.value, "procedures"); + const procedureCount = parseNativeInventoryCount( + proceduresResult.value, + "list_procedures", + ); if (!procedureCount.ok) return procedureCount; - const stringCount = parseListCount(stringsResult.value, "strings"); + const stringCount = parseNativeInventoryCount( + stringsResult.value, + "list_strings", + ); if (!stringCount.ok) return stringCount; return ok( diff --git a/src/application/firmware/FirmwareAnalysisService.test.ts b/src/application/firmware/FirmwareAnalysisService.test.ts index 90be958f77..b39c69d152 100644 --- a/src/application/firmware/FirmwareAnalysisService.test.ts +++ b/src/application/firmware/FirmwareAnalysisService.test.ts @@ -1,4 +1,5 @@ -import { expect, it } from "vitest"; +import { expect, it, vi } from "vitest"; +import { ZodError } from "zod"; import { FirmwareAnalysisService } from "./FirmwareAnalysisService.js"; import { createAnalysisExecution } from "../AnalysisProvider.js"; @@ -65,6 +66,44 @@ const fixture = ( return { input, execution }; }; +it.each([ + { + input: {}, + issue: { path: ["path"], reason: "missing_argument" }, + }, + { + input: { path: 17 }, + issue: { path: ["path"], reason: "invalid_type" }, + }, + { + input: { path: "/firmware.bin", range: { offset: -1, length: 1 } }, + issue: { path: ["range", "offset"], reason: "out_of_range" }, + }, + { + input: { path: "/firmware.bin", unexpected: "caller value" }, + issue: { path: ["unexpected"], reason: "unknown_argument" }, + }, +])( + "rejects malformed firmware inputs with caller-relative issues: $issue", + async ({ input, issue }) => { + const execute = vi.fn(); + const result = await new FirmwareAnalysisService({ execute }).execute( + "extract_firmware", + { output_directory: "/output", ...input }, + ); + expect(execute).not.toHaveBeenCalled(); + expect(result).toMatchObject({ + ok: false, + error: { + _tag: "AnalysisInputError", + operation: "extract_firmware", + cause: expect.any(ZodError), + issues: expect.arrayContaining([expect.objectContaining(issue)]), + }, + }); + }, +); + it.each(["inspect_firmware_regions", "extract_firmware"] as const)( "retains %s Evidence after late cancellation and preserves provider failure precedence", async (operation) => { diff --git a/src/application/firmware/FirmwareAnalysisService.ts b/src/application/firmware/FirmwareAnalysisService.ts index cc05d63997..af74216d8d 100644 --- a/src/application/firmware/FirmwareAnalysisService.ts +++ b/src/application/firmware/FirmwareAnalysisService.ts @@ -9,12 +9,11 @@ import { } from "../../domain/firmware/firmwareAnalysis.js"; import { AnalysisCancelledError, - AnalysisInputError, AnalysisOutputError, } from "../../domain/analysisErrorCore.js"; import type { AnalysisError } from "../../domain/analysisErrorBase.js"; import type { Evidence } from "../../domain/evidence.js"; -import { projectInputIssues } from "../../domain/inputIssueProjection.js"; +import { analysisOperationInputErrorFromIssues } from "../../domain/inputIssueProjection.js"; import { err, type Result } from "../../domain/result.js"; import { completeProviderEvidence } from "../ProviderOperationExecution.js"; @@ -39,17 +38,11 @@ export class FirmwareAnalysisService { const parsed = firmwareRequestSchema.safeParse({ operation, input }); if (!parsed.success) return err( - new AnalysisInputError( + analysisOperationInputErrorFromIssues( operation, - { cause: parsed.error }, - projectInputIssues(parsed.error.issues, { operation, input }).map( - (issue) => ({ - ...issue, - // `input` is the internal discriminated request envelope. - path: - issue.path[0] === "input" ? issue.path.slice(1) : issue.path, - }), - ), + parsed.error.issues, + input, + parsed.error, ), ); const executed = await this.provider.execute(parsed.data, options); diff --git a/src/contracts/toolOutputSchemaPrimitives.ts b/src/contracts/toolOutputSchemaPrimitives.ts index 1b1e42c73b..3aaa48332a 100644 --- a/src/contracts/toolOutputSchemaPrimitives.ts +++ b/src/contracts/toolOutputSchemaPrimitives.ts @@ -19,7 +19,10 @@ import { PROVIDER_RETRY_ACTIONS, } from "../domain/providerOperationHealth.js"; import { analysisErrorProjectionSchema } from "./errorSchemas.js"; -import { objcClassImportIdentitySchema } from "../domain/native/objcSwiftMetadata.js"; +import { + objcClassDiscoveryCoverageSchema, + objcClassImportIdentitySchema, +} from "../domain/native/objcSwiftMetadata.js"; import { BINARY_SESSION_SUMMARY_OMITTED_FIELDS } from "./sessionStatusContract.js"; /** Specialize the single normalized result inside a complete Evidence record. */ @@ -683,17 +686,7 @@ export const objcClassDiscoveryOutput = evidenceResultOf( .optional(), }), ), - identity_coverage: z.strictObject({ - status: z.enum([ - "complete", - "partial", - "unsupported", - "not_requested", - "unknown", - "not_applicable", - ]), - reason: z.string().nullable(), - }), + identity_coverage: objcClassDiscoveryCoverageSchema, }), ); diff --git a/src/domain/native/nativeInventory.ts b/src/domain/native/nativeInventory.ts index ef7d932730..b8ca5d7eb1 100644 --- a/src/domain/native/nativeInventory.ts +++ b/src/domain/native/nativeInventory.ts @@ -4,11 +4,7 @@ import { analysisStringSchema } from "./nativeString.js"; import { ok, type Result } from "../result.js"; import { AnalysisOutputError } from "../analysisErrorCore.js"; import type { JsonValue } from "../jsonValue.js"; -import { jsonObjectSchema } from "../jsonValue.js"; -import { - unwrapNativeProperty, - invalidNativeOutput, -} from "./nativeProviderOutput.js"; +import { invalidNativeOutput } from "./nativeProviderOutput.js"; /** Canonical named native observations; same-address alternatives stay separate. */ export const addressedValueSchema = z.object({ @@ -69,17 +65,17 @@ export const analysisBookmarkSchema = z.strictObject({ address: z.string(), name: z.string().nullable(), }); -/** Parse a direct or wrapped list when only its cardinality is required. */ -export const parseListCount = ( +/** Count only complete canonical inventories, using the advertised row contracts. */ +export const parseNativeInventoryCount = ( value: JsonValue, - property: string, - operation = property === "procedures" ? "list_procedures" : "list_strings", + operation: "list_procedures" | "list_strings", ): Result => { - const unwrapped = unwrapNativeProperty(value, property); - const list = z.array(z.unknown()).safeParse(unwrapped); - if (list.success) return ok(list.data.length); - const map = jsonObjectSchema.safeParse(unwrapped); - return map.success - ? ok(Object.keys(map.data).length) - : invalidNativeOutput(operation, `${property} list`, map.error); + const schema = + operation === "list_procedures" + ? nativeInventoryRowsSchemas.names + : nativeInventoryRowsSchemas.strings; + const parsed = schema.safeParse(value); + return parsed.success + ? ok(parsed.data.length) + : invalidNativeOutput(operation, "inventory rows", parsed.error); }; diff --git a/src/domain/native/nativeProviderOutput.test.ts b/src/domain/native/nativeProviderOutput.test.ts index f315587800..69ac6b4a6c 100644 --- a/src/domain/native/nativeProviderOutput.test.ts +++ b/src/domain/native/nativeProviderOutput.test.ts @@ -2,7 +2,6 @@ import { describe, expect, it } from "vitest"; import { addressDistance } from "./addressCoordinate.js"; import { parseDocuments } from "./nativeDocument.js"; -import { parseListCount } from "./nativeInventory.js"; import { parseRelatedAddresses } from "./nativeObservation.js"; import { parseSegments } from "./nativeSegments.js"; @@ -32,12 +31,6 @@ describe("native provider boundary values", () => { parseSegments([{ name: "__TEXT", start: "0x1", end: "0x2" }]).ok, ).toBe(false); }); - it("counts Hopper's address-keyed string maps", () => { - expect( - parseListCount({ "0x1000": "hello", "0x2000": "world" }, "strings"), - ).toEqual({ ok: true, value: 2 }); - }); - it("parses direct and wrapped protocol shapes", () => { expect(parseRelatedAddresses({ callers: ["0x3"] }, "callers")).toEqual({ ok: true, @@ -98,32 +91,3 @@ describe("native provider boundary values", () => { } }); }); - -it.each(["procedures", "strings"])( - "counts every own entry of direct and wrapped %s maps", - (property) => { - for (const text of [ - '{"__proto__":"observed"}', - '{"__proto__":"observed","0x1000":"one"}', - ]) { - const source = JSON.parse(text); - const count = Object.keys(source).length; - expect(parseListCount(source, property)).toEqual({ - ok: true, - value: count, - }); - expect(parseListCount({ [property]: source }, property)).toEqual({ - ok: true, - value: count, - }); - expect(JSON.stringify(source)).toBe(text); - } - expect(parseListCount({ [property]: [] }, property)).toEqual({ - ok: true, - value: 0, - }); - expect(parseListCount({}, property)).toEqual({ ok: true, value: 0 }); - for (const value of [null, "not a list", 3, false]) - expect(parseListCount(value, property).ok).toBe(false); - }, -); diff --git a/src/domain/native/objcSwiftMetadata.ts b/src/domain/native/objcSwiftMetadata.ts index baa3368ec1..c5cfa26041 100644 --- a/src/domain/native/objcSwiftMetadata.ts +++ b/src/domain/native/objcSwiftMetadata.ts @@ -316,17 +316,30 @@ export const nativeMetadataCoverageSchema = z.strictObject({ export const objcClassImportIdentitySchema = machoImportBindingSchema.omit({ requested_address: true, }); -export const objcClassImportBindingCoverageSchema = z.strictObject({ - facet: z.literal("objc_class_import_bindings"), - status: z.enum(["complete", "partial", "unsupported", "not_requested"]), - reason: z.string().nullable(), - examined: z.number().int().nonnegative(), - decoded: z.number().int().nonnegative(), -}); +export const objcClassImportBindingCoverageSchema = + nativeMetadataCoverageSchema.extend({ + facet: z.literal("objc_class_import_bindings"), + examined: z.number().int().nonnegative(), + decoded: z.number().int().nonnegative(), + }); export type ObjcClassImportBindingCoverage = z.infer< typeof objcClassImportBindingCoverageSchema >; +/** Discovery retains unknown or inapplicable import identity separately from scan coverage. */ +export const objcClassDiscoveryCoverageSchema = nativeMetadataCoverageSchema + .pick({ status: true, reason: true }) + .extend({ + status: z.enum([ + ...nativeMetadataCoverageSchema.shape.status.options, + "unknown", + "not_applicable", + ]), + }); +export type ObjcClassDiscoveryCoverage = z.infer< + typeof objcClassDiscoveryCoverageSchema +>; + /** Nominal Swift type kinds recorded by `__swift5_types` descriptors. */ export const swiftDeclKindSchema = z.enum(["class", "struct", "enum", "actor"]); diff --git a/src/domain/symbolAnalysis.ts b/src/domain/symbolAnalysis.ts index e178437d1f..66b3264501 100644 --- a/src/domain/symbolAnalysis.ts +++ b/src/domain/symbolAnalysis.ts @@ -3,7 +3,7 @@ import type { JsonValue } from "./jsonValue.js"; import type { AddressedName } from "./native/nativeInventory.js"; import type { MachoImportBinding, - ObjcClassImportBindingCoverage, + ObjcClassDiscoveryCoverage, } from "./native/objcSwiftMetadata.js"; import { classifySwiftSymbol, @@ -20,14 +20,6 @@ const LEGACY_OBJC_CLASS_PREFIX = "l_OBJC_CLASS_" as const; */ const OBJC_PROTOCOL_MARKER = "OBJC_PROTOCOL_$_" as const; -export interface ObjcClassDiscoveryCoverage { - readonly status: - | ObjcClassImportBindingCoverage["status"] - | "unknown" - | "not_applicable"; - readonly reason: string | null; -} - export interface ObjcClassImportBindingScan { readonly bindings: readonly MachoImportBinding[]; readonly coverage: ObjcClassDiscoveryCoverage; diff --git a/tests/boundary/mcp/enhancedTools.04-uses-complete-inventories-inline.test.ts b/tests/boundary/mcp/enhancedTools.04-uses-complete-inventories-inline.test.ts index 0a45132738..ee0e53b924 100644 --- a/tests/boundary/mcp/enhancedTools.04-uses-complete-inventories-inline.test.ts +++ b/tests/boundary/mcp/enhancedTools.04-uses-complete-inventories-inline.test.ts @@ -6,12 +6,12 @@ import { connect as connectTool, jsonResult, } from "./enhancedToolsHarness.js"; -import type { - AnalysisExecution, - AnalysisOperationPort, +import { + createAnalysisExecution, + type AnalysisExecution, + type AnalysisOperation, + type AnalysisOperationPort, } from "../../../src/application/AnalysisProvider.js"; -import { createAnalysisExecution } from "../../../src/application/AnalysisProvider.js"; -import type { AnalysisOperation } from "../../../src/application/AnalysisProvider.js"; import type { AnalysisError } from "../../../src/domain/analysisErrorBase.js"; import { AnalysisCapabilityUnavailableError, @@ -22,6 +22,7 @@ import { ok as resultOk, type Result, } from "../../../src/domain/result.js"; +import type { JsonValue } from "../../../src/domain/jsonValue.js"; import { createAnalysisProfile } from "../../../src/domain/analysisProfile.js"; import { observed as ok } from "../../fixtures/analysisExecution.js"; @@ -411,6 +412,47 @@ describe("enhanced MCP binary overview", () => { }); describe("enhanced MCP inventory validation", () => { + // Injected provider successes verify public MCP composition, not real-provider behavior. + it.each(["list_procedures", "list_strings"] as const)( + "rejects malformed successful %s inventory without manufacturing overview counts", + async (operation) => { + let inventory: JsonValue = []; + const client = await connect({ + execute: (name, parameters, options) => + name === operation + ? Promise.resolve(ok(inventory)) + : completeOverviewPort.execute(name, parameters, options), + }); + for (const malformed of [ + { error: "inventory unavailable" }, + { "0x1000": "obsolete map" }, + [{ address: "0x1000", value: 7 }], + ]) { + inventory = malformed; + const response = await client.callTool({ + name: "binary_overview", + arguments: {}, + }); + expect(response.isError).toBe(true); + expect(parseMcpToolError(response).error).toMatchObject({ + code: "unreadable_output", + details: { operation }, + }); + } + inventory = [{ address: "0x1000", value: "canonical control" }]; + const response = await client.callTool({ + name: "binary_overview", + arguments: {}, + }); + expect(response.isError).not.toBe(true); + expect(jsonResult(response)).toMatchObject({ + document: "fixture", + procedure_count: operation === "list_procedures" ? 1 : 2, + string_count: operation === "list_strings" ? 1 : 700, + }); + }, + ); + it("rejects paginated provider output", async () => { const client = await connect({ execute: () =>