From b17487f482ce0769dd0c1415297f9c3e1bb7509f Mon Sep 17 00:00:00 2001 From: GuoJun Tang <76832285+Melon-tgj@users.noreply.github.com> Date: Tue, 13 Oct 2026 22:15:01 +0800 Subject: [PATCH] fix(comparison): keep unsupported function facets unknown (#2450) * fix(comparison): keep unsupported function facets unknown * docs(comparison): explain function facet availability * fix(analysis): preserve unavailable facets across Evidence consumers --------- Co-authored-by: morluto <76467478+morluto@users.noreply.github.com> --- docs/mcp-contracts.md | 14 ++++ src/contracts/functionComparisonExample.ts | 12 ++- src/domain/analysisView/analysisView.test.ts | 26 ++++++ src/domain/analysisView/nativeFunctionView.ts | 17 ++++ src/domain/callPath.test.ts | 18 ++++ src/domain/callPath.ts | 5 ++ src/domain/functionComparison.test.ts | 82 +++++++++++++++++++ src/domain/functionComparisonDimensions.ts | 62 +++++++++++++- src/domain/hopperValues.ts | 18 ++++ src/ida/IdaNativeProtocol.test.ts | 4 + src/ida/IdaOperationRunner.ts | 48 ++++++++++- .../providers/ida/calleeRelationships.test.ts | 16 ++++ 12 files changed, 318 insertions(+), 4 deletions(-) diff --git a/docs/mcp-contracts.md b/docs/mcp-contracts.md index f3f7892f6..7c219f6be 100644 --- a/docs/mcp-contracts.md +++ b/docs/mcp-contracts.md @@ -21,6 +21,20 @@ directly to `compare_functions`, and `inspect_artifact` Evidence to `compare_artifacts`. Use `get_evidence_bundle` when the task needs broader retained session history or an explicit bundle for transfer. +Function dossiers can declare per-facet coverage in the optional +`normalized_result.facet_availability` map. Each supplied facet is either +`{ "available": true }` or `{ "available": false, "reason": "..." }`. +Supported keys are `pseudocode`, `assembly`, `comments`, `callers`, `callees`, +`references`, `strings_names`, and `cfg`. Availability describes whether that +facet was observed, not whether its collection contains any entries. + +`compare_functions` keeps a dimension `unknown` with null counts when either +side declares a required facet unavailable. An empty legacy facet without a +coverage declaration is also unknown. A producer that actually observed an +empty facet can declare it available so observed absence remains comparable. +Call comparison additionally retains the Evidence relationship coverage +checks. IDA declares the facets its selected adapter cannot provide; an empty +unsupported array does not prove observed absence. Evidence and bundle imports reject unsupported fields in their envelopes, identities, manifests and other fixed metadata before authentication. Imports leave retained session state unchanged when validation fails. Keep application diff --git a/src/contracts/functionComparisonExample.ts b/src/contracts/functionComparisonExample.ts index 39dfe6f92..aefc75e1a 100644 --- a/src/contracts/functionComparisonExample.ts +++ b/src/contracts/functionComparisonExample.ts @@ -16,7 +16,7 @@ const dossier = (text: string) => locals: [], }, pseudocode: text, - assembly: [], + assembly: ["ret"], comments: [], callers: [], callees: [], @@ -27,6 +27,16 @@ const dossier = (text: string) => basic_blocks: [{ start: "0x1000", end: "0x1001", successors: [] }], native_api: null, native_value_flow: null, + facet_availability: { + pseudocode: { available: true }, + assembly: { available: true }, + comments: { available: true }, + callers: { available: true }, + callees: { available: true }, + references: { available: true }, + strings_names: { available: true }, + cfg: { available: true }, + }, limitations: ["The fixture omits native API and value-flow observations."], }); diff --git a/src/domain/analysisView/analysisView.test.ts b/src/domain/analysisView/analysisView.test.ts index 07e6909c3..31711ecd4 100644 --- a/src/domain/analysisView/analysisView.test.ts +++ b/src/domain/analysisView/analysisView.test.ts @@ -792,3 +792,29 @@ it("rejects unsupported parent operations and incompatible collections", () => { error: { _tag: "AnalysisInputError" }, }); }); + +it("preserves selected native facet unavailability instead of an exhausted empty page", () => { + const parent = nativeParent({ + ...ghidraFunctionDossier(), + facet_availability: { + comments: { available: false, reason: "Typed comments unavailable" }, + references: { available: false, reason: "Typed references unavailable" }, + }, + }); + for (const [facet, reason] of [ + ["comments", "Typed comments unavailable"], + ["incoming_references", "Typed references unavailable"], + ] as const) { + const result = projectAnalysisView(parent, { + kind: "native", + facet, + offset: 0, + limit: 8, + }); + if (!result.ok) throw result.error; + expect(result.value).toMatchObject({ + item: { available: false, reason }, + unknowns: expect.arrayContaining([reason]), + }); + } +}); diff --git a/src/domain/analysisView/nativeFunctionView.ts b/src/domain/analysisView/nativeFunctionView.ts index b56de24a1..cac7ef82f 100644 --- a/src/domain/analysisView/nativeFunctionView.ts +++ b/src/domain/analysisView/nativeFunctionView.ts @@ -110,6 +110,22 @@ export const projectNativeFunctionView = ( ]), ); const common = parentFields(parent, dossier, artifact); + const availabilityKeys: Readonly> = { + incoming_references: "references", + outgoing_references: "references", + referenced_strings: "strings_names", + referenced_names: "strings_names", + basic_blocks: "cfg", + }; + const availabilityKey = availabilityKeys[view.facet] ?? view.facet; + const availability = + dossier.facet_availability?.[ + availabilityKey as keyof NonNullable< + FunctionDossier["facet_availability"] + > + ]; + if (availability?.available === false) + common.unknowns = uniqueSorted([...common.unknowns, availability.reason]); const singleton = (item: unknown): UnsignedAnalysisView => ({ kind: "native", view, @@ -117,6 +133,7 @@ export const projectNativeFunctionView = ( coverage: completeWithinViewCoverage(1, 1), ...common, }); + if (availability?.available === false) return ok(singleton(availability)); const flow = dossier.native_value_flow; if ( view.facet === "procedure" || diff --git a/src/domain/callPath.test.ts b/src/domain/callPath.test.ts index e78a5f936..80a260ac3 100644 --- a/src/domain/callPath.test.ts +++ b/src/domain/callPath.test.ts @@ -15,6 +15,7 @@ const observe = ( readonly evidenceLimitations?: readonly string[]; readonly dossierLimitations?: readonly string[]; readonly relationshipCoverage?: RelationshipCoverage; + readonly calleeUnavailable?: string; } = {}, ): Evidence => { const value = { @@ -52,6 +53,13 @@ const observe = ( native_value_flow: null, basic_blocks: [], limitations: [...(options.dossierLimitations ?? [])], + ...(options.calleeUnavailable === undefined + ? {} + : { + facet_availability: { + callees: { available: false, reason: options.calleeUnavailable }, + }, + }), } satisfies JsonValue; return createEvidence( { path: "/tmp/a", sha256: "a".repeat(64), format: "mach-o" }, @@ -541,3 +549,13 @@ describe("call path evidence admission", () => { ); }); }); + +it("keeps an absent path unknown when only callee facet availability is declared", () => { + const result = run([ + observe("0x1000", [], "rea-workflow", { + calleeUnavailable: "Graph callees unavailable", + }), + ]); + expect(result.status).toBe("unknown"); + expect(result.limitations).toContain("Graph callees unavailable"); +}); diff --git a/src/domain/callPath.ts b/src/domain/callPath.ts index 0bd5e88e1..3ffbc2356 100644 --- a/src/domain/callPath.ts +++ b/src/domain/callPath.ts @@ -253,6 +253,11 @@ const inspectSearch = ({ // cannot add a path to the goal, so its dossier limits are contextual // but do not make the searched start-to-goal closure incomplete. if (node === goal) continue; + const callees = snapshot.dossier.facet_availability?.callees; + if (callees?.available === false) { + limitations.push(callees.reason); + incompleteExpandedCoverage = true; + } // Explicit, Evidence-bound relationship coverage owns completeness; // generic provider/dossier limitations remain contextual. Legacy Evidence // without that metadata retains its conservative limitation-based rule. diff --git a/src/domain/functionComparison.test.ts b/src/domain/functionComparison.test.ts index 954717bad..b71dcac1d 100644 --- a/src/domain/functionComparison.test.ts +++ b/src/domain/functionComparison.test.ts @@ -44,6 +44,16 @@ const dossier = ( referenced_names: [], native_api: null, native_value_flow: null, + facet_availability: { + pseudocode: { available: true }, + assembly: { available: true }, + comments: { available: true }, + callers: { available: true }, + callees: { available: true }, + references: { available: true }, + strings_names: { available: true }, + cfg: { available: true }, + }, basic_blocks: [ { start: base, @@ -119,6 +129,78 @@ const observe = ( ); describe("function comparison normalized identity", () => { + it("retains unresolved call details alongside missing facet coverage", () => { + const value: Record = { + ...dossier("return 0;", "0x1000"), + unresolved_calls: [{ address: "0x1010", reason: "Unknown indirect target" }], + }; + delete value.facet_availability; + const legacy = observe("a", functionDossierSchema.parse(value), "ida"); + const calls = compareFunctions(legacy, legacy).dimensions.find( + ({ dimension }) => dimension === "calls", + ); + expect(calls).toMatchObject({ + status: "unknown", + limitations: expect.arrayContaining([ + "Left: Unresolved call at 0x1010: Unknown indirect target", + "Left: callers: Provider did not declare coverage for this empty facet.", + ]), + }); + }); + it("keeps empty legacy facets without coverage declarations unknown", () => { + const value: Record = { + ...dossier("return 0;", "0x1000"), + basic_blocks: [], + }; + delete value.facet_availability; + const legacy = observe("a", functionDossierSchema.parse(value), "ida"); + const result = compareFunctions(legacy, legacy); + for (const dimension of [ + "comments", + "references", + "strings_names", + "cfg", + "calls", + ]) { + expect( + result.dimensions.find((item) => item.dimension === dimension), + ).toMatchObject({ status: "unknown" }); + } + }); + it("keeps unavailable facets unknown even when one side supplies values", () => { + const left = observe( + "a", + functionDossierSchema.parse({ + ...dossier("return 0;", "0x1000"), + facet_availability: { + comments: { available: false, reason: "Not collected" }, + }, + comments: [ + { address: "0x1000", kind: "comment", text: "partial record" }, + ], + }), + ); + const right = observe("b", dossier("return 0;", "0x2000")); + expect( + compareFunctions(left, right).dimensions.find( + ({ dimension }) => dimension === "comments", + ), + ).toMatchObject({ status: "unknown", left_count: null, right_count: null }); + }); + it("rejects malformed facet availability instead of treating it as absence", () => { + expect( + functionDossierSchema.safeParse({ + ...dossier("return 0;", "0x1000"), + facet_availability: { cfg: { available: false, reason: "" } }, + }).success, + ).toBe(false); + expect( + functionDossierSchema.safeParse({ + ...dossier("return 0;", "0x1000"), + facet_availability: { cfg: undefined }, + }).success, + ).toBe(false); + }); it("keeps differing generated names ambiguous and stable names mismatched", () => { const named = (base: "0x1000" | "0x2000", name: string) => functionDossierSchema.parse({ diff --git a/src/domain/functionComparisonDimensions.ts b/src/domain/functionComparisonDimensions.ts index c5a0f373f..f91057a6b 100644 --- a/src/domain/functionComparisonDimensions.ts +++ b/src/domain/functionComparisonDimensions.ts @@ -52,7 +52,67 @@ export const compareDimensions = ( compareReferences(left, right, context), compareStringsAndNames(left, right, context), compareCfg(left, right, context), - ]; + ].map((result) => { + const limitations = [ + ...facetLimitations(left, result.dimension).map( + (reason) => `Left: ${reason}`, + ), + ...facetLimitations(right, result.dimension).map( + (reason) => `Right: ${reason}`, + ), + ]; + return limitations.length === 0 + ? result + : unresolvedDimension({ + dimension: result.dimension, + links: context.links, + leftCount: null, + rightCount: null, + limitations: [...result.limitations, ...limitations], + }); + }); +}; + +/** An empty undeclared facet is not evidence of observed absence. */ +const facetLimitations = ( + snapshot: FunctionSnapshot, + dimension: DimensionName, +): string[] => { + const { dossier } = snapshot; + const fields = { + identity: [], + pseudocode: [["pseudocode", dossier.pseudocode.length]], + assembly: [["assembly", dossier.assembly.length]], + comments: [["comments", dossier.comments.length]], + calls: [ + ["callers", dossier.callers.length], + ["callees", dossier.callees.length], + ], + references: [ + [ + "references", + dossier.incoming_references.length + dossier.outgoing_references.length, + ], + ], + strings_names: [ + [ + "strings_names", + dossier.referenced_strings.length + dossier.referenced_names.length, + ], + ], + cfg: [["cfg", dossier.basic_blocks.length]], + } satisfies Record< + DimensionName, + [keyof NonNullable, number][] + >; + return fields[dimension].flatMap(([field, count]) => { + const availability = dossier.facet_availability?.[field]; + if (availability?.available === false) + return [`${field}: ${availability.reason}`]; + return availability === undefined && count === 0 + ? [`${field}: Provider did not declare coverage for this empty facet.`] + : []; + }); }; const compareText = ( diff --git a/src/domain/hopperValues.ts b/src/domain/hopperValues.ts index d6344ff3e..cb19ead3e 100644 --- a/src/domain/hopperValues.ts +++ b/src/domain/hopperValues.ts @@ -371,6 +371,11 @@ export const referenceEdgeSchema = z .exactOptional(), }) .strict(); +const facetAvailabilitySchema = z.discriminatedUnion("available", [ + z.object({ available: z.literal(true) }).strict(), + z.object({ available: z.literal(false), reason: z.string().min(1) }).strict(), +]); + export const functionDossierSchema = z .object({ procedure: procedureIdentitySchema.safeExtend({ @@ -428,6 +433,19 @@ export const functionDossierSchema = z ), native_api: nativeApiBoundarySchema.nullable(), native_value_flow: nativeValueFlowSchema.nullable(), + facet_availability: z + .object({ + pseudocode: facetAvailabilitySchema.exactOptional(), + assembly: facetAvailabilitySchema.exactOptional(), + comments: facetAvailabilitySchema.exactOptional(), + callers: facetAvailabilitySchema.exactOptional(), + callees: facetAvailabilitySchema.exactOptional(), + references: facetAvailabilitySchema.exactOptional(), + strings_names: facetAvailabilitySchema.exactOptional(), + cfg: facetAvailabilitySchema.exactOptional(), + }) + .strict() + .exactOptional(), limitations: z.array(z.string()), }) .strict(); diff --git a/src/ida/IdaNativeProtocol.test.ts b/src/ida/IdaNativeProtocol.test.ts index dc95727f6..384da0420 100644 --- a/src/ida/IdaNativeProtocol.test.ts +++ b/src/ida/IdaNativeProtocol.test.ts @@ -243,6 +243,10 @@ describe("native IDA provider observations", () => { expect(dossier.value.result).toMatchObject({ callers: [], callees: [{ address: "0x2000", name: "puts" }], + facet_availability: { + callers: { available: false }, + callees: { available: true }, + }, }); if (more) expect(dossier.value.limitations).toContain( diff --git a/src/ida/IdaOperationRunner.ts b/src/ida/IdaOperationRunner.ts index 25a49c268..647b06e52 100644 --- a/src/ida/IdaOperationRunner.ts +++ b/src/ida/IdaOperationRunner.ts @@ -70,6 +70,9 @@ const identity = (fn: IdaFunction) => ({ interface IdaRelationships { readonly items: ReturnType[]; readonly coverage: RelationshipCoverage; + readonly availability: + | { readonly available: true } + | { readonly available: false; readonly reason: string }; } /** The only lookup answers upstream reports for a name or address that is not a function. */ @@ -469,7 +472,14 @@ export class IdaOperationRunner { }; if (this.readsCallGraph(direction)) { coverage = await this.addCallGraphCallees(fn, result); - return { items: [...result.values()], coverage }; + return { + items: [...result.values()], + coverage, + availability: + coverage.status === "unknown" + ? { available: false, reason: coverage.reason } + : { available: true }, + }; } if (this.protocol === "legacy") { const value = await this.call( @@ -491,6 +501,10 @@ export class IdaOperationRunner { ); return { items: [], + availability: { + available: false, + reason: "Upstream xrefs do not establish direct caller relationships.", + }, coverage: { status: "unknown", reason: @@ -555,7 +569,11 @@ export class IdaOperationRunner { result.set(resolved.address, resolved); } } - return { items: [...result.values()], coverage }; + return { + items: [...result.values()], + coverage, + availability: { available: true }, + }; } async run( @@ -694,6 +712,32 @@ export class IdaOperationRunner { basic_blocks: [], native_api: null, native_value_flow: null, + facet_availability: { + pseudocode: + pseudocode === null + ? { available: false, reason: "IDA did not supply pseudocode." } + : { available: true }, + assembly: { available: true }, + comments: { + available: false, + reason: "IDA adapter does not expose typed comments.", + }, + callers: callers.availability, + callees: callees.availability, + references: { + available: false, + reason: "IDA adapter does not expose typed reference edges.", + }, + strings_names: { + available: false, + reason: + "IDA adapter does not expose function-referenced strings and names.", + }, + cfg: { + available: false, + reason: "IDA adapter does not expose basic blocks.", + }, + }, limitations: [ ...IDA_LIMITATIONS, ...this.limitations, diff --git a/tests/boundary/providers/ida/calleeRelationships.test.ts b/tests/boundary/providers/ida/calleeRelationships.test.ts index 1204b4d10..523304dac 100644 --- a/tests/boundary/providers/ida/calleeRelationships.test.ts +++ b/tests/boundary/providers/ida/calleeRelationships.test.ts @@ -208,6 +208,22 @@ it("preserves missing callgraph coverage through direct, graph, and trace workfl ]), ); + const dossier = await client.execute("analyze_function", { + procedure: "main", + }); + if (!dossier.ok) throw dossier.error; + expect(dossier.value.result).toMatchObject({ + callers: [], + callees: [], + facet_availability: { + callers: { available: false }, + callees: { + available: false, + reason: expect.stringContaining("callgraph was unavailable"), + }, + }, + }); + const application = new EnhancedTools(client); const graph = await application.execute("get_call_graph", { address: "main",