From b1b0211c284f06e67d697dd8e016925f833eca6c Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 14 Oct 2026 07:11:55 +0800 Subject: [PATCH] fix(native): preserve observed function dossier coverage Declare observed Ghidra and Hopper facets, preserve unavailable pseudocode, and attach dossier call-set coverage. Version the analysis profiles to invalidate older snapshots and remove the retired include_assembly fixture branch. Add MCP comparison and snapshot-replay regressions plus real-provider checks for empty and unavailable facets. Fixes #2980. --- bridge/ghidra/ReaGhidraBridge.java | 19 +++- bridge/hopper_bridge.py | 10 +- docs/mcp-contracts.md | 8 +- scripts/lib/native-value-e2e.mjs | 6 +- scripts/lib/real-ghidra-boundaries.mjs | 19 ++++ scripts/lib/real-hopper-semantic.mjs | 53 ++++++++++- src/domain/ghidraValues.fixture.ts | 16 +++- src/ghidra/GhidraAnalysisProfile.ts | 3 + src/ghidra/GhidraProvider.behavior.test.ts | 1 + src/ghidra/GhidraProviderCapabilities.ts | 4 +- src/hopper/HopperAnalysisProfile.ts | 1 + src/hopper/HopperProvider.ts | 3 +- tests/boundary/mcp/ghidraEvidenceMcp.test.ts | 92 +++++++++++++++++++ tests/boundary/mcp/ghidraMcpHarness.ts | 5 +- .../hopper/hopperAnalysisProfile.test.ts | 2 + .../hopper/hopperBridgeFacade.test.ts | 13 +++ 16 files changed, 232 insertions(+), 23 deletions(-) diff --git a/bridge/ghidra/ReaGhidraBridge.java b/bridge/ghidra/ReaGhidraBridge.java index 84e809ccb..f91f003e4 100644 --- a/bridge/ghidra/ReaGhidraBridge.java +++ b/bridge/ghidra/ReaGhidraBridge.java @@ -1490,9 +1490,6 @@ public final class ReaGhidraBridge extends HeadlessScript { InstructionScan scan = scanInstructions(function, Integer.MAX_VALUE); DecompileResults decompilation = decompile(function); String pseudocode = decompiledText(decompilation); - if (pseudocode == null) { - pseudocode = ""; - } List incomingReferences = collectReferences(function, "incoming") .stream() .filter(reference -> !function.getBody().contains(reference.getFromAddress())) @@ -1514,7 +1511,7 @@ public final class ReaGhidraBridge extends HeadlessScript { procedure.add("locals", functionLocals(function)); JsonObject result = new JsonObject(); result.add("procedure", procedure); - result.addProperty("pseudocode", pseudocode); + result.addProperty("pseudocode", pseudocode == null ? "" : pseudocode); result.add("assembly", assembly); result.add("comments", comments); result.add("callers", callers); @@ -1523,6 +1520,20 @@ public final class ReaGhidraBridge extends HeadlessScript { result.add("outgoing_references", outgoing); result.add("referenced_strings", referencedStrings); result.add("referenced_names", referencedNames); + JsonObject facets = new JsonObject(); + JsonObject pseudocodeAvailability = new JsonObject(); + pseudocodeAvailability.addProperty("available", pseudocode != null); + if (pseudocode == null) { + pseudocodeAvailability.addProperty("reason", "Ghidra did not supply pseudocode."); + } + facets.add("pseudocode", pseudocodeAvailability); + for (String facet : List.of("assembly", "comments", "callers", "callees", + "references", "strings_names", "cfg")) { + JsonObject available = new JsonObject(); + available.addProperty("available", true); + facets.add(facet, available); + } + result.add("facet_availability", facets); boolean swift = swiftProgram(); boolean aarch64 = currentProgram.getLanguageID().getIdAsString().startsWith("AARCH64:"); SwiftStringLiteralDecode swiftLiterals = swift && aarch64 ? swiftStringLiterals(function) : null; diff --git a/bridge/hopper_bridge.py b/bridge/hopper_bridge.py index 92a473c06..4188fe0da 100644 --- a/bridge/hopper_bridge.py +++ b/bridge/hopper_bridge.py @@ -644,7 +644,7 @@ def _analyze_function(document, params): "end": _hex(_basic_block_end(procedure, block)), "successors": sorted(set(successors), key=lambda value: int(value, 16)), }) - pseudo = _pseudocode(document, procedure) or "" + pseudo = _pseudocode(document, procedure) assembly_lines = _assembly(procedure).splitlines() callers = sorted((_procedure_identity(item) for item in procedure.getAllCallerProcedures()), key=lambda item: int(item["address"], 16)) callees = sorted((_procedure_identity(item) for item in procedure.getAllCalleeProcedures()), key=lambda item: int(item["address"], 16)) @@ -706,7 +706,7 @@ def _analyze_function(document, params): "signature": procedure.signatureString(), "locals": _procedure_locals(procedure), }, - "pseudocode": pseudo, + "pseudocode": pseudo or "", "assembly": assembly_lines, "comments": comments, "callers": callers, "callees": callees, @@ -718,6 +718,12 @@ def _analyze_function(document, params): "basic_blocks": blocks, "native_api": None, "native_value_flow": None, + "facet_availability": { + "pseudocode": {"available": True} if pseudo is not None else _unavailable("Hopper did not supply pseudocode."), + **{facet: {"available": True} for facet in ( + "assembly", "comments", "callers", "callees", "references", "strings_names", "cfg" + )}, + }, "limitations": [ "Native CallReference classifications are observed where available; detailed reference flags remain unknown.", "Hopper's public Python API does not expose equivalent external or thunk classification in this dossier.", diff --git a/docs/mcp-contracts.md b/docs/mcp-contracts.md index 307d323df..557c98ab4 100644 --- a/docs/mcp-contracts.md +++ b/docs/mcp-contracts.md @@ -58,7 +58,13 @@ 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 +checks. Ghidra and Hopper declare their observed dossier facets and complete +provider-observed caller/callee sets, including empty sets. Missing decompiler +output remains unavailable; complete call coverage does not claim every runtime +target. Hopper's unavailable reference-kind metadata and reported unresolved +calls still prevent equality conclusions for those dimensions. Analysis profiles +version these declarations so older snapshots are not reused as current results. +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 diff --git a/scripts/lib/native-value-e2e.mjs b/scripts/lib/native-value-e2e.mjs index 513259b30..d329bc822 100644 --- a/scripts/lib/native-value-e2e.mjs +++ b/scripts/lib/native-value-e2e.mjs @@ -235,7 +235,11 @@ async function verifyNativeValueAdapters( ); assert.notEqual(comparison.isError, true, JSON.stringify(comparison)); const comparisonEvidence = parseEvidence(comparison.structuredContent); - assert.equal(comparisonEvidence.normalized_result.status, "unchanged"); + assert.equal( + comparisonEvidence.normalized_result.status, + "unchanged", + JSON.stringify(comparisonEvidence.normalized_result.dimensions), + ); assert.deepEqual( comparisonEvidence.normalized_result, comparison.structuredContent?.normalized_result, diff --git a/scripts/lib/real-ghidra-boundaries.mjs b/scripts/lib/real-ghidra-boundaries.mjs index f6ee1527a..c2347dde1 100644 --- a/scripts/lib/real-ghidra-boundaries.mjs +++ b/scripts/lib/real-ghidra-boundaries.mjs @@ -131,6 +131,25 @@ export async function verifyGhidraBoundaries( const externalDossier = await call("analyze_function", { procedure: external.address, }); + assert.equal(externalDossier.pseudocode, ""); + assert.deepEqual(externalDossier.assembly, []); + assert.deepEqual(externalDossier.callees, []); + assert.deepEqual(externalDossier.facet_availability.pseudocode, { + available: false, + reason: "Ghidra did not supply pseudocode.", + }); + for (const facet of [ + "assembly", + "comments", + "callers", + "callees", + "references", + "strings_names", + "cfg", + ]) + assert.deepEqual(externalDossier.facet_availability[facet], { + available: true, + }); const externalEntry = await call("resolve_containing_procedure", { address: external.address, }); diff --git a/scripts/lib/real-hopper-semantic.mjs b/scripts/lib/real-hopper-semantic.mjs index cb11b433c..7739db6be 100644 --- a/scripts/lib/real-hopper-semantic.mjs +++ b/scripts/lib/real-hopper-semantic.mjs @@ -327,14 +327,57 @@ export const analyzeFixtureProcedure = async ( options, procedure, normalizedResult, -) => - normalizedResult( +) => { + const reply = await client.callTool( + { + name: "analyze_function", + arguments: { procedure: procedure.address }, + }, + options, + ); + const dossier = normalizedResult(reply, "analyze_function"); + assert.deepEqual(reply.structuredContent.relationship_coverage, { + status: "complete", + }); + for (const facet of [ + "pseudocode", + "assembly", + "comments", + "callers", + "callees", + "references", + "strings_names", + "cfg", + ]) + assert.deepEqual(dossier.facet_availability[facet], { available: true }); + const comparison = normalizedResult( await client.callTool( { - name: "analyze_function", - arguments: { procedure: procedure.address }, + name: "compare_functions", + arguments: { + left: reply.structuredContent, + right: reply.structuredContent, + }, }, options, ), - "analyze_function", + "compare_functions", ); + assert.deepEqual( + Object.fromEntries( + comparison.dimensions.map(({ dimension, status }) => [dimension, status]), + ), + { + identity: "unchanged", + pseudocode: "unchanged", + assembly: "unchanged", + comments: "unchanged", + calls: "unchanged", + references: "unknown", + strings_names: "unchanged", + cfg: "unchanged", + }, + JSON.stringify(comparison.dimensions), + ); + return dossier; +}; diff --git a/src/domain/ghidraValues.fixture.ts b/src/domain/ghidraValues.fixture.ts index d0c6d183c..4927cf210 100644 --- a/src/domain/ghidraValues.fixture.ts +++ b/src/domain/ghidraValues.fixture.ts @@ -147,7 +147,7 @@ export const ghidraNativeApiBoundary = () => ({ ], }); -export const ghidraFunctionDossier = (includeAssembly = true): JsonValue => { +export const ghidraFunctionDossier = (): JsonValue => { const pseudocode = "int fixture_main(void) { return 42; }"; return { procedure: { @@ -156,10 +156,18 @@ export const ghidraFunctionDossier = (includeAssembly = true): JsonValue => { locals: [], }, pseudocode, - assembly: includeAssembly - ? ["0x401000: CALL 0x401020", "0x401005: RET"] - : [], + assembly: ["0x401000: CALL 0x401020", "0x401005: RET"], comments: [], + 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 }, + }, callers: [], callees: [], incoming_references: [], diff --git a/src/ghidra/GhidraAnalysisProfile.ts b/src/ghidra/GhidraAnalysisProfile.ts index 8175b776b..da39ce53c 100644 --- a/src/ghidra/GhidraAnalysisProfile.ts +++ b/src/ghidra/GhidraAnalysisProfile.ts @@ -126,6 +126,8 @@ export const resolveGhidraAnalysisProfile = async ( function_body_evidence: "complete-inclusive-ranges-v1", function_references: "complete-body-and-entry-reference-manager-v2", call_relationship_coverage: "resolved-call-reference-diagnostics-v1", + function_dossier_coverage: + "observed-facets-and-call-relationships-v1", string_inventory_evidence: "defined-data-coverage-v1", location_resolution: "explicit-address-exact-entry-symbol-first-v3", procedure_file_offset_resolution: @@ -221,6 +223,7 @@ export const resolveGhidraAnalysisProfile = async ( function_body_evidence: "complete-inclusive-ranges-v1", function_references: "complete-body-and-entry-reference-manager-v2", call_relationship_coverage: "resolved-call-reference-diagnostics-v1", + function_dossier_coverage: "observed-facets-and-call-relationships-v1", string_inventory_evidence: "defined-data-coverage-v1", location_resolution: "explicit-address-exact-entry-symbol-first-v3", procedure_file_offset_resolution: diff --git a/src/ghidra/GhidraProvider.behavior.test.ts b/src/ghidra/GhidraProvider.behavior.test.ts index b485f6ab3..4a052aaef 100644 --- a/src/ghidra/GhidraProvider.behavior.test.ts +++ b/src/ghidra/GhidraProvider.behavior.test.ts @@ -628,6 +628,7 @@ describe("Ghidra client projection", () => { language_id: "auto-from-header", compiler_spec_id: "auto-default", call_relationship_coverage: "resolved-call-reference-diagnostics-v1", + function_dossier_coverage: "observed-facets-and-call-relationships-v1", }); const result = await ghidra diff --git a/src/ghidra/GhidraProviderCapabilities.ts b/src/ghidra/GhidraProviderCapabilities.ts index 0714d1ce0..f4bff8d9c 100644 --- a/src/ghidra/GhidraProviderCapabilities.ts +++ b/src/ghidra/GhidraProviderCapabilities.ts @@ -51,7 +51,9 @@ const resolvedCallReferenceLimitation = export const relationshipCoverageFor = ( operation: string, ): RelationshipCoverage | undefined => - operation === "procedure_callers" || operation === "procedure_callees" + operation === "procedure_callers" || + operation === "procedure_callees" || + operation === "analyze_function" ? { status: "complete", diagnostics: [resolvedCallReferenceLimitation], diff --git a/src/hopper/HopperAnalysisProfile.ts b/src/hopper/HopperAnalysisProfile.ts index 79ba3ab50..74618a60c 100644 --- a/src/hopper/HopperAnalysisProfile.ts +++ b/src/hopper/HopperAnalysisProfile.ts @@ -82,6 +82,7 @@ export const resolveHopperAnalysisProfile = async ( profile: createAnalysisProfile(provider, { target_kind: target.kind, target_format: target.format, + function_dossier_coverage: "observed-facets-and-call-relationships-v1", architecture: target.architecture ?? null, available_architectures: [ ...(target.availableArchitectures ?? []), diff --git a/src/hopper/HopperProvider.ts b/src/hopper/HopperProvider.ts index f9826575b..e4d7c8381 100644 --- a/src/hopper/HopperProvider.ts +++ b/src/hopper/HopperProvider.ts @@ -272,7 +272,8 @@ export class HopperProvider implements AnalysisProviderCandidate { rawResult, ...(profile === undefined ? {} : { analysisProfile: profile }), ...(operation === "procedure_callers" || - operation === "procedure_callees" + operation === "procedure_callees" || + operation === "analyze_function" ? { relationshipCoverage: { status: "complete" as const } } : {}), limitations: [ diff --git a/tests/boundary/mcp/ghidraEvidenceMcp.test.ts b/tests/boundary/mcp/ghidraEvidenceMcp.test.ts index ed82400aa..e786fc3cc 100644 --- a/tests/boundary/mcp/ghidraEvidenceMcp.test.ts +++ b/tests/boundary/mcp/ghidraEvidenceMcp.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it } from "vitest"; import { EnhancedTools } from "../../../src/application/EnhancedTools.js"; import { connectGhidraMcp, sessionEvidence } from "./ghidraMcpHarness.js"; import { functionDossierSchema } from "../../../src/domain/hopperValues.js"; +import { functionComparisonResultSchema } from "../../../src/domain/functionComparisonSchemas.js"; import { ghidraFunctionDossier } from "../../../src/domain/ghidraValues.fixture.js"; import { jsonValueSchema, @@ -14,6 +15,97 @@ import { type GhidraFunctionOperation, } from "../../../src/ghidra/GhidraFunctionValues.js"; +it.each([true, false])( + "compares observed empty Ghidra facets while preserving pseudocode availability (%s)", + async (pseudocodeAvailable) => { + const dossier = functionDossierSchema.parse(ghidraFunctionDossier()); + Object.assign(dossier, { + comments: [], + callers: [], + callees: [], + incoming_references: [], + outgoing_references: [], + referenced_strings: [], + referenced_names: [], + pseudocode: pseudocodeAvailable ? dossier.pseudocode : "", + facet_availability: { + pseudocode: pseudocodeAvailable + ? { available: true } + : { available: false, reason: "Ghidra did not supply pseudocode." }, + assembly: { available: true }, + comments: { available: true }, + callers: { available: true }, + callees: { available: true }, + references: { available: true }, + strings_names: { available: true }, + cfg: { available: true }, + }, + }); + const harness = await connectGhidraMcp( + "ghidra-empty-dossier-coverage", + (operation) => { + if (operation !== "analyze_function") + throw new Error(`Unexpected operation: ${operation}`); + return Promise.resolve( + ok({ value: jsonValueSchema.parse(dossier), limitations: [] }), + ); + }, + ); + try { + const analyzed = await harness.mcp.callTool({ + name: "analyze_function", + arguments: { procedure: dossier.procedure.address }, + }); + expect(analyzed.isError).not.toBe(true); + const evidence = sessionEvidence( + harness.session, + analyzed.structuredContent, + ); + expect(evidence.relationship_coverage).toMatchObject({ + status: "complete", + }); + expect(evidence.normalized_result).toEqual(dossier); + const reply = await harness.mcp.callTool({ + name: "compare_functions", + arguments: { left: evidence, right: evidence }, + }); + expect(reply.isError).not.toBe(true); + const comparison = functionComparisonResultSchema.parse( + sessionEvidence(harness.session, reply.structuredContent) + .normalized_result, + ); + expect( + Object.fromEntries( + comparison.dimensions.map(({ dimension, status }) => [ + dimension, + status, + ]), + ), + ).toEqual({ + identity: "unchanged", + pseudocode: pseudocodeAvailable ? "unchanged" : "unknown", + assembly: "unchanged", + comments: "unchanged", + calls: "unchanged", + references: "unchanged", + strings_names: "unchanged", + cfg: "unchanged", + }); + const replay = await harness.session.execute( + "analyze_function", + evidence.parameters, + ); + if (!replay.ok) throw replay.error; + expect(replay.value.relationshipCoverage).toEqual( + evidence.relationship_coverage, + ); + expect(harness.calls).toEqual(["analyze_function"]); + } finally { + await harness.close(); + } + }, +); + it("retains terminal-call qualifications across function views and snapshot replay", async () => { const limitation = "Ghidra Listing reports a terminal call at 0x401000 to EXTERNAL:0x20 __tls_get_addr (external=true, thunk=false, hasNoReturn=true); fallthrough is excluded by Ghidra's flow model. This records Ghidra's FunctionManager flag, not an independent verification that the callee cannot return."; diff --git a/tests/boundary/mcp/ghidraMcpHarness.ts b/tests/boundary/mcp/ghidraMcpHarness.ts index fea27b037..43e49b551 100644 --- a/tests/boundary/mcp/ghidraMcpHarness.ts +++ b/tests/boundary/mcp/ghidraMcpHarness.ts @@ -246,10 +246,7 @@ const resultBuilders = new Map([ }), ], ["xrefs", () => ["0x401001"]], - [ - "analyze_function", - (input) => ghidraFunctionDossier(input.include_assembly === true), - ], + ["analyze_function", () => ghidraFunctionDossier()], ]); const resultFor = ( diff --git a/tests/boundary/providers/hopper/hopperAnalysisProfile.test.ts b/tests/boundary/providers/hopper/hopperAnalysisProfile.test.ts index a8f121e61..46b71deb7 100644 --- a/tests/boundary/providers/hopper/hopperAnalysisProfile.test.ts +++ b/tests/boundary/providers/hopper/hopperAnalysisProfile.test.ts @@ -106,6 +106,8 @@ describe("Hopper analysis profiles", () => { value: { profile: { parameters: { + function_dossier_coverage: + "observed-facets-and-call-relationships-v1", loader: { source: scenario.source, arguments: scenario.arguments, diff --git a/tests/boundary/providers/hopper/hopperBridgeFacade.test.ts b/tests/boundary/providers/hopper/hopperBridgeFacade.test.ts index 515172eab..a4be45673 100644 --- a/tests/boundary/providers/hopper/hopperBridgeFacade.test.ts +++ b/tests/boundary/providers/hopper/hopperBridgeFacade.test.ts @@ -167,6 +167,19 @@ describe("Hopper API facade", () => { }, native_api: null, native_value_flow: null, + facet_availability: { + pseudocode: { + available: false, + reason: "Hopper did not supply pseudocode.", + }, + assembly: { available: true }, + comments: { available: true }, + callers: { available: true }, + callees: { available: true }, + references: { available: true }, + strings_names: { available: true }, + cfg: { available: true }, + }, }); expect(result.provider_faults.map((reply) => reply.error.message)).toEqual([ "TypeError: Hopper bridge operation failed",