From 69a8b3a954a89b9e083f89213c2835c9701e0af2 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sun, 11 Oct 2026 09:03:32 +0800 Subject: [PATCH] fix(ghidra): expose terminal-call boundary assumptions Preserve observed terminal-call sites and callee flags across function views, raw bridge responses, Evidence, and snapshot replay. Version analysis profiles so older unqualified results cannot be reused. Qualify Ghidra function extents without changing external no-return flags or claiming recovery of excluded code. Validation: 76 focused tests, check:fast, docs:check, formatting, and the full Ghidra 12.1.4 host lane pass. Real CLI and all five stdio MCP function views retain the terminal-call evidence; returning puts calls remain unflagged. The reported stripped Rust ELF and Windows host remain unverified. Fixes #1658 --- bridge/ghidra/ReaGhidraBridge.java | 99 +++++++++++++++++-- docs/ghidra-provider.md | 11 +++ scripts/lib/real-ghidra-large-results.mjs | 4 +- scripts/verify-real-ghidra-function.mjs | 57 +++++++++++ scripts/verify-real-ghidra-windows.mjs | 2 +- .../functionWorkflowToolContracts.ts | 2 +- src/contracts/officialToolContracts.ts | 8 +- src/ghidra/GhidraAnalysisProfile.ts | 1 + src/ghidra/GhidraFunctionValues.test.ts | 84 ++++++++++++---- src/ghidra/GhidraFunctionValues.ts | 46 ++++++++- src/ghidra/GhidraProvider.behavior.test.ts | 25 +++++ src/ghidra/GhidraProviderClient.ts | 26 +++-- src/ghidra/GhidraSnapshotCache.test.ts | 7 +- tests/boundary/mcp/ghidraEvidenceMcp.test.ts | 92 ++++++++++++++++- tests/boundary/mcp/ghidraMcpHarness.ts | 38 +++++-- tests/conformance/ghidra/inventory.c | 10 ++ 16 files changed, 457 insertions(+), 55 deletions(-) diff --git a/bridge/ghidra/ReaGhidraBridge.java b/bridge/ghidra/ReaGhidraBridge.java index 5da3e1374f..498a99e002 100644 --- a/bridge/ghidra/ReaGhidraBridge.java +++ b/bridge/ghidra/ReaGhidraBridge.java @@ -377,7 +377,10 @@ public final class ReaGhidraBridge extends HeadlessScript { case "inspect_native_instruction" -> inspectNativeInstruction(request.params); case "resolve_native_call_targets" -> resolveNativeCallTargets(request.params); case "xrefs" -> xrefs(request.params); - case "analyze_function" -> analyzeFunction(request.params); + case "analyze_function" -> { + FunctionAnalysisObservation observation = analyzeFunctionObservation(request.params); + yield functionObservation(observation.dossier(), observation.limitations()); + } case "set_address_name", "set_addresses_names" -> { if (!descriptor.transport.equals("unix-socket")) throw new RequestFailure("method_unavailable", "Windows P0 does not admit database mutation"); @@ -652,10 +655,14 @@ public final class ReaGhidraBridge extends HeadlessScript { private JsonElement procedurePseudocode(JsonObject params) throws Exception { requireKeys(params, Set.of("document", "procedure")); requireDocument(params); - String value = decompiledText( - decompile(resolveProcedure(requireString(params, "procedure"))) + Function function = resolveProcedure(requireString(params, "procedure")); + String value = decompiledText(decompile(function)); + return functionObservation( + value == null ? JsonNull.INSTANCE : GSON.toJsonTree(value), + noReturnCallLimitations( + currentProgram.getListing().getInstructions(function.getBody(), true) + ) ); - return value == null ? JsonNull.INSTANCE : GSON.toJsonTree(value); } private JsonElement procedureAssembly(JsonObject params) throws Exception { @@ -663,7 +670,10 @@ public final class ReaGhidraBridge extends HeadlessScript { requireDocument(params); Function function = resolveProcedure(requireString(params, "procedure")); InstructionScan scan = scanInstructions(function, Integer.MAX_VALUE); - return GSON.toJsonTree(renderAssembly(scan.instructions)); + return functionObservation( + GSON.toJsonTree(renderAssembly(scan.instructions)), + noReturnCallLimitations(scan.instructions) + ); } private JsonObject readFunctionInstructions(JsonObject params) throws Exception { @@ -681,11 +691,13 @@ public final class ReaGhidraBridge extends HeadlessScript { limitations.add( "The fast path does not invoke the decompiler or scan whole-program names and strings." ); + List boundaryLimitations = noReturnCallLimitations(scan.instructions); + for (String limitation : boundaryLimitations) limitations.add(limitation); JsonObject result = new JsonObject(); result.add("procedure", procedureIdentity(function)); result.add("instructions", items); result.add("limitations", limitations); - return result; + return functionObservation(result, boundaryLimitations); } private JsonArray procedureCalls(JsonObject params, boolean callers) throws Exception { @@ -718,7 +730,12 @@ public final class ReaGhidraBridge extends HeadlessScript { result.addProperty("signature", function.getPrototypeString(false, true)); result.add("locals", functionLocals(function)); result.add("classification", functionClassification(function)); - return result; + return functionObservation( + result, + noReturnCallLimitations( + currentProgram.getListing().getInstructions(function.getBody(), true) + ) + ); } private JsonObject procedureReferences(JsonObject params) throws Exception { @@ -1130,6 +1147,10 @@ public final class ReaGhidraBridge extends HeadlessScript { } private JsonObject analyzeFunction(JsonObject params) throws Exception { + return analyzeFunctionObservation(params).dossier(); + } + + private FunctionAnalysisObservation analyzeFunctionObservation(JsonObject params) throws Exception { requireKeys(params, Set.of("procedure")); Function function = resolveProcedure(requireString(params, "procedure")); InstructionScan scan = scanInstructions(function, Integer.MAX_VALUE); @@ -1187,12 +1208,16 @@ public final class ReaGhidraBridge extends HeadlessScript { limitations.add( "Synthetic Ghidra entry-point references without actionable memory sources are omitted." ); + List boundaryLimitations = noReturnCallLimitations(scan.instructions); + for (String limitation : boundaryLimitations) { + limitations.add(limitation); + } if (decompilation != null && decompilation.getErrorMessage() != null && !decompilation.getErrorMessage().isBlank()) { limitations.add("Ghidra decompiler diagnostic: " + decompilation.getErrorMessage()); } result.add("limitations", limitations); - return result; + return new FunctionAnalysisObservation(result, boundaryLimitations); } private JsonObject nativeValueFlow(DecompileResults decompilation) throws Exception { @@ -2654,6 +2679,63 @@ public final class ReaGhidraBridge extends HeadlessScript { return new InstructionScan(List.copyOf(instructions), iterator.hasNext()); } + private JsonObject functionObservation( + JsonElement value, + List limitations) { + JsonObject result = new JsonObject(); + result.add("value", value); + result.add("limitations", GSON.toJsonTree(limitations)); + return result; + } + + private List noReturnCallLimitations(Iterable instructions) + throws Exception { + List limitations = new ArrayList<>(); + for (Instruction instruction : instructions) { + monitor.checkCancelled(); + if (!instruction.getFlowType().isCall() || !instruction.getFlowType().isTerminal()) { + continue; + } + Address[] destinations = instruction.getFlows(); + StringBuilder limitation = new StringBuilder(); + limitation.append("Ghidra Listing reports a terminal call at ") + .append(canonicalAddress(instruction.getAddress())); + if (destinations.length == 0) { + limitation.append(" with no resolved callee; its no-return basis is unknown."); + } + else { + limitation.append(" to "); + for (int index = 0; index < destinations.length; index++) { + if (index > 0) limitation.append(", "); + Address destination = destinations[index]; + limitation.append(canonicalAddress(destination)); + Function target = currentProgram.getFunctionManager().getFunctionAt(destination); + if (target == null) { + limitation.append(" (callee identity unknown)"); + } + else { + limitation.append(" ").append(procedureName(target)); + limitation.append(" (external=").append(target.isExternal()); + limitation.append(", thunk=").append(target.isThunk()); + limitation.append(", hasNoReturn=").append(target.hasNoReturn()).append(")"); + } + } + limitation.append("; fallthrough is excluded by Ghidra's flow model."); + Function target = destinations.length == 1 + ? currentProgram.getFunctionManager().getFunctionAt(destinations[0]) + : null; + if (target != null && target.hasNoReturn()) { + limitation.append(" This records Ghidra's FunctionManager flag, not an independent verification that the callee cannot return."); + } + else { + limitation.append(" The reason for the terminal flow is not established by this observation."); + } + } + limitations.add(limitation.toString()); + } + return limitations; + } + private static String renderAssembly(List instructions) { return String.join("\n", renderAssemblyLines(instructions)); } @@ -3483,6 +3565,7 @@ public final class ReaGhidraBridge extends HeadlessScript { private record InventoryItem(Address address, String value, JsonObject facts) {} private record FunctionEntry(Function function, InventoryItem item) {} + private record FunctionAnalysisObservation(JsonObject dossier, List limitations) {} private record InstructionScan(List instructions, boolean truncated) {} private record SessionDescriptor( String transport, diff --git a/docs/ghidra-provider.md b/docs/ghidra-provider.md index af95d0ab4e..cdb9e13363 100644 --- a/docs/ghidra-provider.md +++ b/docs/ghidra-provider.md @@ -52,6 +52,17 @@ bytes and references. | P-code value flow | Function dossiers include up to 3,000 high-p-code operations, at most 64 inputs per operation, and 12,000 def-use edges. Results identify memory reads/writes, branches, and calls by p-code opcode, and mark decompiler-dead operations. Large results report truncation, an omitted-operation lower bound, and known omitted-input/edge counts. The dossier is an intra-function graph; `trace_native_values` composes derived parameter/argument and return/output dependencies across resolved calls. It does not resolve memory aliasing, call side effects, runtime behavior, persistent state, or business meaning. | | Result extent | Function instruction scans and native API boundary observations remain complete. P-code flow is the explicit bounded exception; its count fields distinguish exact retained counts from lower-bound or known-omitted counts. Caller cancellation and provider failures remain distinct. | +Function extents and instruction completeness refer to Ghidra's analyzed body. +An inferred no-return call can terminate that body before subsequent bytes. +Pseudocode, assembly, procedure info, instruction windows, and dossiers report +observed terminal call sites in Evidence limitations, including resolved callee +names, addresses, and FunctionManager flags when available. Instruction windows +and dossiers also retain these warnings in their result limitations. An +unresolved callee or unestablished terminal-flow basis remains explicit. These +observations qualify the analysis; they do not prove that a no-return flag is +incorrect or recover excluded code. Analysis profiles version this observation +contract so older snapshots cannot replay unqualified results. + `npm run verify:ghidra` compiles the versioned C oracle into debug and stripped host-native targets (x86-64/AArch64 ELF on Linux or Mach-O on macOS), plus a native DWARF 4 type-layout object. It proves the admitted operations, external functions, resolved thunks, exports, stripped-name diff --git a/scripts/lib/real-ghidra-large-results.mjs b/scripts/lib/real-ghidra-large-results.mjs index 5617a1b4a6..fa1c5115ff 100644 --- a/scripts/lib/real-ghidra-large-results.mjs +++ b/scripts/lib/real-ghidra-large-results.mjs @@ -183,7 +183,9 @@ export async function verifyGhidraLargeResults({ assert.equal(dossier.procedure.name, "rea_frame_probe"); assert.ok(dossier.comments.some(({ text }) => text === comment)); assert.ok( - dossierRecord.raw_result.comments.some(({ text }) => text === comment), + dossierRecord.raw_result.value.comments.some( + ({ text }) => text === comment, + ), ); const { stdout } = await exec( process.execPath, diff --git a/scripts/verify-real-ghidra-function.mjs b/scripts/verify-real-ghidra-function.mjs index b9d9c299e8..16bb172b6f 100644 --- a/scripts/verify-real-ghidra-function.mjs +++ b/scripts/verify-real-ghidra-function.mjs @@ -21,6 +21,10 @@ import { } from "./verify-real-ghidra-assertions.mjs"; async function functionCall(client, operation, parameters) { + return (await functionObservationCall(client, operation, parameters)).value; +} + +async function functionObservationCall(client, operation, parameters) { const input = parseGhidraFunctionInput(operation, parameters); if (!input.ok) throw input.error; const called = await client.callTool(operation, input.value); @@ -153,6 +157,57 @@ export async function verifyDebugFunctionOperations( const denseSwitchDossier = await functionCall(client, "analyze_function", { procedure: denseSwitchProcedure.address, }); + const terminalProcedure = requireProcedure( + procedures, + "rea_ghidra_inventory_terminal_call", + ); + const terminalObservations = await Promise.all( + [ + "procedure_info", + "procedure_assembly", + "read_function_instructions", + "procedure_pseudo_code", + "analyze_function", + ].map((operation) => + functionObservationCall(client, operation, { + ...(operation === "analyze_function" ? {} : { document: null }), + procedure: terminalProcedure.address, + }), + ), + ); + const terminalLimitations = terminalObservations.map((result) => + result.limitations.find( + (limitation) => + /terminal call at 0x[0-9a-f]+/i.test(limitation) && + / to 0x[0-9a-f]+/i.test(limitation) && + limitation.includes("abort") && + limitation.includes("hasNoReturn=true"), + ), + ); + if ( + terminalLimitations.some((limitation) => limitation === undefined) || + terminalLimitations.some( + (limitation) => + !limitation.includes("external=") || + !limitation.includes("thunk=") || + !limitation.includes("fallthrough is excluded") || + !limitation.includes("not an independent verification"), + ) + ) + throw new Error( + `Ghidra no-return boundary evidence was missing callee, address, or flag provenance: ${JSON.stringify(terminalObservations.map(({ limitations }) => limitations))}`, + ); + const returningObservation = await functionObservationCall( + client, + "procedure_info", + { document: null, procedure: leaf.address }, + ); + if ( + returningObservation.limitations.some((limitation) => + limitation.includes("puts"), + ) + ) + throw new Error("Ghidra marked an ordinary returning call as terminal"); // The debug fixture is compiled -O0, and GCC 16 and later lower dense // switches to compare chains at -O0, so this dossier asserts only its // complete shape; the optimized jump-table variant proves dense jump-table @@ -179,6 +234,8 @@ export async function verifyDebugFunctionOperations( instruction_window: { returned: instructionWindow.instructions.length, }, + terminal_call_limitations: terminalLimitations, + returning_call_limitations: returningObservation.limitations, cancellation, timeout, concurrency: concurrent, diff --git a/scripts/verify-real-ghidra-windows.mjs b/scripts/verify-real-ghidra-windows.mjs index 6eb5014375..d2bbb511b6 100644 --- a/scripts/verify-real-ghidra-windows.mjs +++ b/scripts/verify-real-ghidra-windows.mjs @@ -322,7 +322,7 @@ async function functionOperation(operation, parameters) { const result = parseGhidraFunctionResult(operation, called.value); if (!result.ok) throw result.error; observed.add(operation); - return result.value; + return result.value.value; } function assertSession(session, targetSha256, profileDigest) { diff --git a/src/contracts/functionWorkflowToolContracts.ts b/src/contracts/functionWorkflowToolContracts.ts index 5ef3f82f2b..c738b79275 100644 --- a/src/contracts/functionWorkflowToolContracts.ts +++ b/src/contracts/functionWorkflowToolContracts.ts @@ -36,7 +36,7 @@ const functionWorkflow = ( export const FUNCTION_WORKFLOW_TOOL_CONTRACTS = [ functionWorkflow( "analyze_function", - "Build a dossier for one native function identified by symbol or provider-returned address. Returns identity with complete inclusive body ranges or explicit unknown, provider-specific pseudocode and assembly, comments, calls, references, referenced strings/names, local CFG blocks, and available native API boundary observations.", + "Build a dossier for one native function identified by symbol or provider-returned address. Returns identity with complete inclusive analyzed body ranges or explicit unknown, provider-specific pseudocode and assembly, comments, calls, references, referenced strings/names, local CFG blocks, and available native API boundary observations. Ghidra terminal-call assumptions that may cut the analyzed body are reported in limitations with available callee identities.", ), functionWorkflow( "inspect_native_api", diff --git a/src/contracts/officialToolContracts.ts b/src/contracts/officialToolContracts.ts index 811b66c90d..0838e105eb 100644 --- a/src/contracts/officialToolContracts.ts +++ b/src/contracts/officialToolContracts.ts @@ -144,7 +144,7 @@ export const OFFICIAL_TOOL_CONTRACTS = [ ), official( "procedure_assembly", - "Return assembly for one analyzed procedure identified by symbol or hexadecimal address. Use when pseudocode loses calling-convention or instruction-level detail; output is currently returned as one unpaginated string.", + "Return assembly for one analyzed procedure identified by symbol or hexadecimal address. Use when pseudocode loses calling-convention or instruction-level detail; output is currently returned as one unpaginated string. Ghidra terminal-call assumptions that may cut the analyzed body are reported in Evidence limitations.", z.object({ procedure, document }), ), official( @@ -159,12 +159,12 @@ export const OFFICIAL_TOOL_CONTRACTS = [ ), official( "procedure_info", - "Return provider metadata for one procedure identified by symbol or address: entrypoint, signature, locals, size, block count, and complete inclusive body ranges when observed. Hopper locals preserve observed names and signed stack displacements; source types remain unknown. Body byte count and enclosing span remain distinct; unavailable extent is explicit.", + "Return provider metadata for one procedure identified by symbol or address: entrypoint, signature, locals, size, block count, and complete inclusive body ranges when observed. Extents reflect the provider's analyzed body; Ghidra terminal-call assumptions that may cut it are reported in Evidence limitations. Hopper locals preserve observed names and signed stack displacements; source types remain unknown. Body byte count and enclosing span remain distinct; unavailable extent is explicit.", z.object({ procedure, document }), ), official( "read_function_instructions", - "Return every raw instruction for one analyzed procedure. Instruction text is provider-specific.", + "Return every raw instruction for one analyzed procedure. Instruction text and body extent are provider-specific. Ghidra terminal-call assumptions that may cut the analyzed body are reported in limitations.", functionInstructionInputSchema, ), ...HOPPER_MEMORY_TOOL_DEFINITIONS.map(({ name, description, inputSchema }) => @@ -181,7 +181,7 @@ export const OFFICIAL_TOOL_CONTRACTS = [ ), official( "procedure_pseudo_code", - "Decompile one analyzed procedure by symbol name or provider-normalized address. Returns provider-specific pseudocode, never original source or cross-provider text equivalence, and may return null; request procedure_assembly when instruction precision matters.", + "Decompile one analyzed procedure by symbol name or provider-normalized address. Returns provider-specific pseudocode, never original source or cross-provider text equivalence, and may return null; request procedure_assembly when instruction precision matters. Ghidra no-return assumptions may cut the analyzed body; Evidence limitations report observed terminal calls and available callee identities.", z.object({ procedure, document }), ), official( diff --git a/src/ghidra/GhidraAnalysisProfile.ts b/src/ghidra/GhidraAnalysisProfile.ts index 59113b3b89..36b7d32ebd 100644 --- a/src/ghidra/GhidraAnalysisProfile.ts +++ b/src/ghidra/GhidraAnalysisProfile.ts @@ -70,6 +70,7 @@ export const resolveGhidraAnalysisProfile = ( string_inventory_evidence: "defined-data-coverage-v1", location_resolution: "explicit-address-exact-entry-symbol-first-v3", instruction_flow_evidence: "decoded-return-pcode-v1", + function_boundary_observations: "ghidra-terminal-call-limitations-v1", process_launch: installation.platform === "win32" ? "official-headless-script-v1" diff --git a/src/ghidra/GhidraFunctionValues.test.ts b/src/ghidra/GhidraFunctionValues.test.ts index a99d304a7e..f868d65a04 100644 --- a/src/ghidra/GhidraFunctionValues.test.ts +++ b/src/ghidra/GhidraFunctionValues.test.ts @@ -20,7 +20,55 @@ import { ghidraReferenceEdge, } from "../domain/ghidraValues.fixture.js"; +const observationOperations: ReadonlySet = new Set([ + "analyze_function", + "procedure_assembly", + "procedure_info", + "procedure_pseudo_code", + "read_function_instructions", +]); + +// Existing assertions focus on the value schema; this helper builds the current +// bridge envelope and returns its normalized value for those assertions. +const parseFunctionValueFixture = ( + operation: GhidraFunctionOperation, + value: JsonValue, +) => { + const wireValue = observationOperations.has(operation) + ? { value, limitations: [] } + : value; + const parsed = parseGhidraFunctionResult(operation, wireValue); + return parsed.ok ? { ok: true as const, value: parsed.value.value } : parsed; +}; + describe("Ghidra function-analysis result values", () => { + it.each([...observationOperations])( + "requires the function-observation limitations envelope for %s", + (operation) => { + expect(parseGhidraFunctionResult(operation, null)).toMatchObject({ + ok: false, + error: { _tag: "AnalysisOutputError" }, + }); + }, + ); + + it("preserves a named function-observation limitation", () => { + expect( + parseGhidraFunctionResult("procedure_pseudo_code", "void f() {}"), + ).toMatchObject({ ok: false, error: { _tag: "AnalysisOutputError" } }); + const limitation = + "Ghidra marks a call to external __tls_get_addr terminal; return behavior is not independently verified."; + expect( + parseGhidraFunctionResult("procedure_pseudo_code", { + value: "void f() {}", + limitations: [limitation], + }), + ).toEqual({ + ok: true, + value: { value: "void f() {}", limitations: [limitation] }, + }); + }); + it.each(["reference_kinds_available", "unresolved_calls"])( "rejects omitted %s instead of inventing complete reference observations", (field) => { @@ -35,10 +83,10 @@ describe("Ghidra function-analysis result values", () => { Object.entries(output).filter(([key]) => key !== field), ); expect( - parseGhidraFunctionResult("procedure_references", output), + parseFunctionValueFixture("procedure_references", output), ).toMatchObject({ ok: true }); expect( - parseGhidraFunctionResult("procedure_references", incomplete), + parseFunctionValueFixture("procedure_references", incomplete), ).toMatchObject({ ok: false, error: { _tag: "AnalysisOutputError" } }); }, ); @@ -80,7 +128,7 @@ describe("Ghidra function-analysis result values", () => { ); expect( - parseGhidraFunctionResult( + parseFunctionValueFixture( "analyze_function", jsonValueSchema.parse({ ...dossier, @@ -103,7 +151,7 @@ describe("Ghidra function-analysis result values", () => { if (typeof flow !== "object" || flow === null || Array.isArray(flow)) throw new TypeError("Ghidra p-code fixture is invalid"); expect( - parseGhidraFunctionResult("analyze_function", { + parseFunctionValueFixture("analyze_function", { ...dossier, native_value_flow: { ...flow, @@ -159,7 +207,7 @@ describe("Ghidra compact reference provenance", () => { }; for (const reference of [complete, legacy, unknown]) { const expected = output([reference]); - const parsed = parseGhidraFunctionResult(operation, expected); + const parsed = parseFunctionValueFixture(operation, expected); if (!parsed.ok) throw parsed.error; expect(parsed.value).toEqual(expected); } @@ -179,7 +227,7 @@ describe("Ghidra compact reference provenance", () => { ["source", false], ] as const) { expect( - parseGhidraFunctionResult( + parseFunctionValueFixture( operation, output([{ ...compactReference(), [field]: value }]), ), @@ -215,7 +263,7 @@ describe("Ghidra jump-table mapping contract", () => { ], }, ]; - const parsed = parseGhidraFunctionResult("analyze_function", { + const parsed = parseFunctionValueFixture("analyze_function", { ...dossier, native_api: { ...boundary, @@ -255,7 +303,7 @@ describe("Ghidra jump-table mapping contract", () => { throw new TypeError("Ghidra jump-table fixture is unavailable"); const { default_targets: omittedDefaults, ...withoutDefaults } = table; expect(omittedDefaults).toHaveLength(1); - const parsed = parseGhidraFunctionResult("analyze_function", { + const parsed = parseFunctionValueFixture("analyze_function", { ...dossier, native_api: { ...boundary, @@ -288,7 +336,7 @@ describe("Ghidra jump-table mapping contract", () => { { target_address: "0x401020", confidence: "high", evidence: [] }, ]) { expect( - parseGhidraFunctionResult("analyze_function", { + parseFunctionValueFixture("analyze_function", { ...dossier, native_api: { ...boundary, @@ -304,7 +352,7 @@ describe("Ghidra function-analysis malformed results", () => { it.each(malformedOutputs())( "rejects malformed %s output", (_name, operation, value) => { - expect(parseGhidraFunctionResult(operation, value)).toMatchObject({ + expect(parseFunctionValueFixture(operation, value)).toMatchObject({ ok: false, error: { _tag: "AnalysisOutputError" }, }); @@ -438,7 +486,7 @@ describe("complete Ghidra function body range evidence", () => { body: body(), }); it("preserves inclusive disjoint ranges and distinguishes owned bytes from enclosing span", () => { - expect(parseGhidraFunctionResult("procedure_info", info())).toMatchObject({ + expect(parseFunctionValueFixture("procedure_info", info())).toMatchObject({ ok: true, value: { body: body(), length: 5 }, }); @@ -446,7 +494,7 @@ describe("complete Ghidra function body range evidence", () => { it("rejects omitted body evidence instead of accepting a length as complete coverage", () => { const { body: omitted, ...withoutBody } = info(); expect(omitted.total_bytes).toBe(5); - expect(parseGhidraFunctionResult("procedure_info", withoutBody).ok).toBe( + expect(parseFunctionValueFixture("procedure_info", withoutBody).ok).toBe( false, ); }); @@ -460,14 +508,14 @@ describe("complete Ghidra function body range evidence", () => { contains_entry: false, }; expect( - parseGhidraFunctionResult("procedure_info", { + parseFunctionValueFixture("procedure_info", { ...info(), body: empty, length: 0, }).ok, ).toBe(false); expect( - parseGhidraFunctionResult("procedure_info", { + parseFunctionValueFixture("procedure_info", { ...info(), classification: { ...ghidraFunctionClassification(), external: true }, body: empty, @@ -480,7 +528,7 @@ describe("complete Ghidra function body range evidence", () => { const { body: omitted, ...identity } = ghidraFunctionIdentity(); expect(omitted.total_bytes).toBe(6); expect( - parseGhidraFunctionResult("analyze_function", { + parseFunctionValueFixture("analyze_function", { ...dossier, callers: [identity], }).ok, @@ -488,10 +536,10 @@ describe("complete Ghidra function body range evidence", () => { }); it("requires body length and Ghidra-specific body provenance", () => { expect( - parseGhidraFunctionResult("procedure_info", { ...info(), length: 18 }).ok, + parseFunctionValueFixture("procedure_info", { ...info(), length: 18 }).ok, ).toBe(false); expect( - parseGhidraFunctionResult("procedure_info", { + parseFunctionValueFixture("procedure_info", { ...info(), body: { ...body(), provenance: "unreviewed" }, }).ok, @@ -511,7 +559,7 @@ describe("complete Ghidra function body range evidence", () => { { ranges: [{ start: "0x401002", end: "0x401000" }] }, ])("rejects contradictory complete body facts %j", (change) => { expect( - parseGhidraFunctionResult("procedure_info", { + parseFunctionValueFixture("procedure_info", { ...info(), body: { ...body(), ...change }, }).ok, diff --git a/src/ghidra/GhidraFunctionValues.ts b/src/ghidra/GhidraFunctionValues.ts index e56f043f58..8876e31aed 100644 --- a/src/ghidra/GhidraFunctionValues.ts +++ b/src/ghidra/GhidraFunctionValues.ts @@ -387,14 +387,54 @@ const resultSchemas = { xrefs: z.array(ghidraCanonicalAddressSchema), } satisfies Readonly>; +const functionBoundaryObservationOperations: ReadonlySet = + new Set([ + "analyze_function", + "procedure_assembly", + "procedure_info", + "procedure_pseudo_code", + "read_function_instructions", + ]); + +const functionObservationEnvelope = z.strictObject({ + value: z.unknown(), + limitations: z.array(z.string()), +}); + +export interface ParsedGhidraFunctionResult { + readonly value: JsonValue; + readonly limitations: readonly string[]; +} + /** Require exact, bounded Java-bridge function output before creating Evidence. */ export const parseGhidraFunctionResult = ( operation: GhidraFunctionOperation, value: JsonValue, -): Result => { - const parsed = resultSchemas[operation].safeParse(value); +): Result => { + let observation: { + readonly value: unknown; + readonly limitations: readonly string[]; + }; + if (functionBoundaryObservationOperations.has(operation)) { + const envelope = functionObservationEnvelope.safeParse(value); + if (!envelope.success) + return err( + new AnalysisOutputError( + operation, + "Ghidra bridge omitted the function observation envelope", + { cause: envelope.error }, + ), + ); + observation = envelope.data; + } else { + observation = { value, limitations: [] }; + } + const parsed = resultSchemas[operation].safeParse(observation.value); return parsed.success - ? ok(jsonValueSchema.parse(parsed.data)) + ? ok({ + value: jsonValueSchema.parse(parsed.data), + limitations: observation.limitations, + }) : err( new AnalysisOutputError( operation, diff --git a/src/ghidra/GhidraProvider.behavior.test.ts b/src/ghidra/GhidraProvider.behavior.test.ts index cd623a2365..fd0a3f5f04 100644 --- a/src/ghidra/GhidraProvider.behavior.test.ts +++ b/src/ghidra/GhidraProvider.behavior.test.ts @@ -562,6 +562,31 @@ describe("Ghidra result projection", () => { }); }); + it("projects terminal-call facts from the function bridge into execution limitations", async () => { + const limitation = + "Ghidra Listing reports a terminal call at 0x401020 to 0x7f001000 __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."; + const ghidra = provider(installationHost(), () => ({ + start: () => Promise.resolve(ok(sessionInfo())), + callTool: () => + Promise.resolve( + ok({ value: "void main() {}", limitations: [limitation] }), + ), + close: () => Promise.resolve(ok(null)), + })); + const client = await createElfClient(ghidra); + const execution = await client.execute("procedure_pseudo_code", { + procedure: "main", + }); + if (!execution.ok) throw execution.error; + expect(execution.value.result).toBe("void main() {}"); + expect(execution.value.limitations).toContain(limitation); + expect(execution.value.rawResult).toEqual({ + value: "void main() {}", + limitations: [limitation], + }); + await client.close(); + }); + it.each([ ["invalid_request", "AnalysisInputError"], ["not_found", "AnalysisInputError"], diff --git a/src/ghidra/GhidraProviderClient.ts b/src/ghidra/GhidraProviderClient.ts index 0af34b3809..f659eb12cd 100644 --- a/src/ghidra/GhidraProviderClient.ts +++ b/src/ghidra/GhidraProviderClient.ts @@ -8,6 +8,7 @@ import { type AnalysisOperation, } from "../application/AnalysisProvider.js"; import type { AppConfig } from "../config/types.js"; +import type { JsonValue } from "../domain/jsonValue.js"; import type { AnalysisProfileCommitment } from "../domain/analysisProfile.js"; import type { BinaryTarget } from "../domain/binaryTargetTypes.js"; import { @@ -263,16 +264,23 @@ export const createGhidraProviderClient = (input: { config.ghidraStartupTimeoutMs, ), ); - const result = isGhidraFunctionOperation(operation) - ? parseGhidraFunctionResult(operation, called.value) - : parseGhidraInventoryResult(operation, called.value); - if (!result.ok) return result; - let normalized = result.value; + let normalized: JsonValue; + let observationLimitations: readonly string[] = []; + if (isGhidraFunctionOperation(operation)) { + const result = parseGhidraFunctionResult(operation, called.value); + if (!result.ok) return result; + normalized = result.value.value; + observationLimitations = result.value.limitations; + } else { + const result = parseGhidraInventoryResult(operation, called.value); + if (!result.ok) return result; + normalized = result.value; + } if (operation === "inspect_native_load_image") { const attested = await attestGhidraNativeLoadImage( target, operation, - result.value, + normalized, client, (failure) => projectSessionError( @@ -288,7 +296,11 @@ export const createGhidraProviderClient = (input: { createAnalysisExecution(normalized, committedProfile.provider, { rawResult: called.value, analysisProfile: committedProfile, - limitations: [...limitationsFor(operation), ...sessionLimitations], + limitations: [ + ...limitationsFor(operation), + ...observationLimitations, + ...sessionLimitations, + ], }), ); }, diff --git a/src/ghidra/GhidraSnapshotCache.test.ts b/src/ghidra/GhidraSnapshotCache.test.ts index 19f903ea4c..a7f30f196e 100644 --- a/src/ghidra/GhidraSnapshotCache.test.ts +++ b/src/ghidra/GhidraSnapshotCache.test.ts @@ -104,8 +104,13 @@ const fixture = (version = "12.1.4") => { persists_after_close: false, }, }); + const value = + operation === "procedure_pseudo_code" ? dossier.pseudocode : dossier; return ok( - operation === "procedure_pseudo_code" ? dossier.pseudocode : dossier, + operation === "procedure_pseudo_code" || + operation === "analyze_function" + ? { value, limitations: [] } + : value, ); }, close: async () => ok(null), diff --git a/tests/boundary/mcp/ghidraEvidenceMcp.test.ts b/tests/boundary/mcp/ghidraEvidenceMcp.test.ts index 9fa1f3b979..c28fcca390 100644 --- a/tests/boundary/mcp/ghidraEvidenceMcp.test.ts +++ b/tests/boundary/mcp/ghidraEvidenceMcp.test.ts @@ -4,8 +4,98 @@ import { EnhancedTools } from "../../../src/application/EnhancedTools.js"; import { connectGhidraMcp, sessionEvidence } from "./ghidraMcpHarness.js"; import { functionDossierSchema } from "../../../src/domain/hopperValues.js"; import { ghidraFunctionDossier } from "../../../src/domain/ghidraValues.fixture.js"; -import { jsonValueSchema } from "../../../src/domain/jsonValue.js"; +import { + jsonValueSchema, + type JsonValue, +} from "../../../src/domain/jsonValue.js"; import { ok } from "../../../src/domain/result.js"; +import { + isGhidraFunctionOperation, + type GhidraFunctionOperation, +} from "../../../src/ghidra/GhidraFunctionValues.js"; + +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."; + const dossier = functionDossierSchema.parse(ghidraFunctionDossier()); + dossier.limitations.push(limitation); + const { signature, locals, ...identity } = dossier.procedure; + if (!identity.body.available) + throw new Error("The function fixture must include its analyzed body"); + const values = new Map([ + ["procedure_pseudo_code", dossier.pseudocode], + ["procedure_assembly", dossier.assembly.join("\n")], + [ + "procedure_info", + { + name: identity.name, + entrypoint: identity.address, + signature, + locals, + basicblock_count: dossier.basic_blocks.length, + length: identity.body.total_bytes, + classification: identity.classification, + body: identity.body, + }, + ], + [ + "read_function_instructions", + { + procedure: identity, + instructions: dossier.assembly, + limitations: [limitation], + }, + ], + ["analyze_function", jsonValueSchema.parse(dossier)], + ]); + const harness = await connectGhidraMcp( + "ghidra-terminal-call-evidence", + (operation) => { + if (!isGhidraFunctionOperation(operation)) + throw new Error(`Unexpected operation: ${operation}`); + const value = values.get(operation); + if (value === undefined) + throw new Error(`Unexpected operation: ${operation}`); + return Promise.resolve(ok({ value, limitations: [limitation] })); + }, + ); + try { + for (const [name, value] of values) { + const request = { + name, + arguments: + name === "analyze_function" + ? { procedure: identity.address } + : { procedure: identity.address, document: "fixture" }, + }; + const reply = await harness.mcp.callTool(request); + expect(reply.isError, name).not.toBe(true); + const evidence = sessionEvidence( + harness.session, + reply.structuredContent, + ); + expect(evidence.normalized_result, name).toEqual(value); + expect(evidence.raw_result, name).toEqual({ + value, + limitations: [limitation], + }); + expect(evidence.limitations, name).toContain(limitation); + expect(evidence.analysis_profile?.parameters).toMatchObject({ + function_boundary_observations: "ghidra-terminal-call-limitations-v1", + }); + const replay = await harness.session.execute(name, evidence.parameters); + if (!replay.ok) throw replay.error; + expect(replay.value.result, name).toEqual(value); + expect(replay.value.limitations, name).toContain(limitation); + expect(replay.value.limitations, name).toContainEqual( + expect.stringContaining("Loaded from a local REA analysis snapshot"), + ); + } + expect(harness.calls).toEqual([...values.keys()]); + } finally { + await harness.close(); + } +}); it("rejects contradictory annotation readback across the provider and MCP boundaries", async () => { const dossier = functionDossierSchema.parse(ghidraFunctionDossier()); diff --git a/tests/boundary/mcp/ghidraMcpHarness.ts b/tests/boundary/mcp/ghidraMcpHarness.ts index cbfbb70194..7e2c3ae73d 100644 --- a/tests/boundary/mcp/ghidraMcpHarness.ts +++ b/tests/boundary/mcp/ghidraMcpHarness.ts @@ -206,15 +206,19 @@ const resultBuilders = new Map([ ["procedure_callers", () => []], [ "procedure_info", - () => ({ - name: "fixture_main", - entrypoint: "0x401000", - basicblock_count: 1, - length: 6, - signature: "int fixture_main(void)", - locals: [], - classification: ghidraFunctionClassification(), - }), + () => { + const identity = ghidraFunctionIdentity(); + return { + name: "fixture_main", + entrypoint: "0x401000", + basicblock_count: 1, + length: 6, + signature: "int fixture_main(void)", + locals: [], + classification: ghidraFunctionClassification(), + body: identity.body, + }; + }, ], ["procedure_pseudo_code", () => "int fixture_main(void) { return 42; }"], [ @@ -247,9 +251,23 @@ const resultFor = ( const builder = resultBuilders.get(operation); if (builder === undefined) throw new TypeError(`Unexpected Ghidra operation: ${operation}`); - return builder(input, typeof input.limit === "number" ? input.limit : 100); + const value = builder( + input, + typeof input.limit === "number" ? input.limit : 100, + ); + return functionObservationOperations.has(operation) + ? { value, limitations: [] } + : value; }; +const functionObservationOperations: ReadonlySet = new Set([ + "analyze_function", + "procedure_assembly", + "procedure_info", + "procedure_pseudo_code", + "read_function_instructions", +]); + const stringItem = (address: string, value: string): JsonValue => ({ address, value, diff --git a/tests/conformance/ghidra/inventory.c b/tests/conformance/ghidra/inventory.c index 8b57aa1ce2..e82c7d9ddb 100644 --- a/tests/conformance/ghidra/inventory.c +++ b/tests/conformance/ghidra/inventory.c @@ -1,4 +1,5 @@ #include +#include volatile int rea_ghidra_inventory_global = 7; @@ -43,6 +44,15 @@ __attribute__((noinline, used)) int rea_ghidra_inventory_leaf(int value) { return value + rea_ghidra_inventory_global; } +// Ghidra's no-return analysis recognizes abort; this function provides a real +// terminal-call boundary while retaining a returning path. +__attribute__((noinline, used)) int rea_ghidra_inventory_terminal_call( + int should_abort) { + if (should_abort) abort(); + puts("REA_GHIDRA_TERMINAL_CALL_RETURNED"); + return should_abort; +} + // A reference into the function body can land inside an instruction. __attribute__((used)) const unsigned char *volatile rea_ghidra_interior_pointer = (const unsigned char *)(void *)rea_ghidra_inventory_leaf + 1;