diff --git a/docs/providers/ghidra-provider.md b/docs/providers/ghidra-provider.md index 177f187b3e..73244e6015 100644 --- a/docs/providers/ghidra-provider.md +++ b/docs/providers/ghidra-provider.md @@ -140,6 +140,11 @@ also accept source-file byte offsets. Existing symbols and analysis addresses take precedence; a `0x`-prefixed number that resolves to no procedure falls back to its observed source mapping. Use `--file-offset=0x...` on the CLI or `procedure: "file-offset:0x..."` through MCP to select file coordinates explicitly. +To select an analysis address without the bare-hex fallback, qualify it with its +Ghidra address space, for example `ram:0x...`. The `inspect-native-load-image` +CLI command or MCP `inspect_native_load_image` reports Ghidra's observed default +address space; use the space appropriate to the address, and retain any +load-image verification limitations. Multiple mapping candidates fail as ambiguous rather than selecting a bank or address space. A successful conversion retains the file offset, mapped address and procedure entry in raw Evidence and `locations`. diff --git a/src/cli/coreBinaryCommands.ts b/src/cli/coreBinaryCommands.ts index a4f36e1c36..c34a477b08 100644 --- a/src/cli/coreBinaryCommands.ts +++ b/src/cli/coreBinaryCommands.ts @@ -198,7 +198,7 @@ const registerDecompileCommand = ( runDirectAnalysis: DirectAnalysis["runDirectAnalysis"], ): void => { cli.command(CLI_COMMANDS.decompile, { - description: `Read one part of an app as code. ${decompileSelectorGuidance} Equal positional and --procedure values are accepted; conflicts fail.`, + description: `Read one part of an app as code. ${decompileSelectorGuidance} Equal positional and --procedure values are accepted; conflicts fail. With Ghidra, a space-qualified address such as ram:0x... avoids file-offset fallback; inspect-native-load-image reports the observed default space.`, args: z.object({ path: localPathStringSchema.describe("App or program path"), address: z diff --git a/src/contracts/officialToolContracts.ts b/src/contracts/officialToolContracts.ts index 1313223cd8..de2fd06c5a 100644 --- a/src/contracts/officialToolContracts.ts +++ b/src/contracts/officialToolContracts.ts @@ -214,7 +214,7 @@ export const OFFICIAL_TOOL_CONTRACTS = [ ), official( "procedure_pseudo_code", - "Decompile one analyzed procedure by symbol name or provider-normalized address. To request a fresh independent attempt, use analysis: independent_fresh when binary_session reports independent_decompile for this provider operation; otherwise it is refused. The independent result may have different analysis context. Ghidra also accepts file-offset:0x... selectors and falls back to a unique source-file mapping when a 0x-prefixed number resolves to no procedure; existing analysis addresses take precedence and ambiguous mappings fail explicitly. Returns provider-specific pseudocode, never original source or cross-provider text equivalence, and may return null. When supplied, raw_result.pseudocode_absence distinguishes provider-failure from not-in-export; absence of that fact means unknown. 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. Select detail summary to retain complete Evidence and receive text availability and counts; inspect_analysis_view summary or native/pseudocode pages read the original observation without decompiling again. Complete remains the default.", + "Decompile one analyzed procedure by symbol name or provider-normalized address. To request a fresh independent attempt, use analysis: independent_fresh when binary_session reports independent_decompile for this provider operation; otherwise it is refused. The independent result may have different analysis context. Ghidra also accepts file-offset:0x... selectors and falls back to a unique source-file mapping when a 0x-prefixed number resolves to no procedure; existing analysis addresses take precedence and ambiguous mappings fail explicitly. A space-qualified address such as ram:0x... avoids that fallback; inspect_native_load_image reports Ghidra's observed default address space. Returns provider-specific pseudocode, never original source or cross-provider text equivalence, and may return null. When supplied, raw_result.pseudocode_absence distinguishes provider-failure from not-in-export; absence of that fact means unknown. 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. Select detail summary to retain complete Evidence and receive text availability and counts; inspect_analysis_view summary or native/pseudocode pages read the original observation without decompiling again. Complete remains the default.", z.object({ procedure, document, diff --git a/src/ghidra/GhidraProvider.behavior.test.ts b/src/ghidra/GhidraProvider.behavior.test.ts index 9d0d646895..734c7f7c56 100644 --- a/src/ghidra/GhidraProvider.behavior.test.ts +++ b/src/ghidra/GhidraProvider.behavior.test.ts @@ -1315,6 +1315,8 @@ describe("Ghidra result projection", () => { ["invalid_request", "AnalysisInputError"], ["not_found", "AnalysisInputError"], ["ambiguous", "AnalysisInputError"], + ["file_offset_ambiguous", "AnalysisInputError"], + ["procedure_ambiguous", "AnalysisInputError"], ["method_unavailable", "AnalysisCapabilityUnavailableError"], ] as const)( "projects remote %s without losing its typed meaning", diff --git a/src/ghidra/GhidraProviderClient.ts b/src/ghidra/GhidraProviderClient.ts index 22f26b391b..a99294735f 100644 --- a/src/ghidra/GhidraProviderClient.ts +++ b/src/ghidra/GhidraProviderClient.ts @@ -1376,7 +1376,7 @@ const projectPrimarySessionError = ( break; case "file_offset_ambiguous": recovery = - "Choose an explicit analysis address from the reported mappings. Use inspect_native_load_image to inspect the loaded mappings; REA does not select one automatically."; + "Choose a space-qualified analysis address from the reported mappings, such as ram:0x1000. A bare 0x... selector falls back to file-offset lookup and can report this ambiguity again. Use inspect_native_load_image to inspect the loaded mappings and Ghidra's observed default address space; REA does not select one automatically."; break; case "procedure_ambiguous": recovery = diff --git a/tests/boundary/mcp/ghidraAmbiguousProcedureMcp.test.ts b/tests/boundary/mcp/ghidraAmbiguousProcedureMcp.test.ts new file mode 100644 index 0000000000..d302ab8f2d --- /dev/null +++ b/tests/boundary/mcp/ghidraAmbiguousProcedureMcp.test.ts @@ -0,0 +1,103 @@ +import { expect, it } from "vitest"; +import { err } from "../../../src/domain/result.js"; +import { GhidraSessionError } from "../../../src/ghidra/GhidraSessionError.js"; +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; +import { connectGhidraMcp } from "./ghidraMcpHarness.js"; + +it.each(["compact", "full"] as const)( + "preserves actionable Ghidra procedure ambiguity over MCP (%s)", + async (profile) => { + const message = + "Ghidra procedure name is ambiguous: shared; select an exact entry address from [0x1000, 0x2000]"; + const harness = await connectGhidraMcp( + "ambiguous-ghidra-procedure", + () => + Promise.resolve( + err( + new GhidraSessionError( + "remote", + message, + {}, + { remoteCode: "procedure_ambiguous" }, + ), + ), + ), + { environment: { REA_MCP_OUTPUT_SCHEMA_PROFILE: profile } }, + ); + try { + const response = await harness.mcp.callTool({ + name: "analyze_function", + arguments: { procedure: "shared" }, + }); + const { error } = parseMcpToolError(response); + expect(error).toMatchObject({ + code: "invalid_request", + message, + remediation: { + action: expect.stringContaining("explicit procedure entry address"), + }, + }); + expect(error.recovery).toBeUndefined(); + const malformed = await harness.mcp.callTool({ + name: "analyze_function", + arguments: { procedure: 42 }, + }); + expect(parseMcpToolError(malformed).error).toMatchObject({ + code: "invalid_request", + recovery: { kind: "inspect_input_contract" }, + details: { + issues: [ + expect.objectContaining({ + path: ["procedure"], + reason: "invalid_type", + }), + ], + }, + }); + } finally { + await harness.close(); + } + }, +); + +it.each(["compact", "full"] as const)( + "keeps file-offset mapping ambiguity distinct from function identity (%s)", + async (profile) => { + const message = + "File offset 0x400 has multiple loaded mappings: [0x1000, 0x2000]; select an explicit analysis address"; + const harness = await connectGhidraMcp( + "ambiguous-ghidra-offset", + () => + Promise.resolve( + err( + new GhidraSessionError( + "remote", + message, + {}, + { remoteCode: "file_offset_ambiguous" }, + ), + ), + ), + { environment: { REA_MCP_OUTPUT_SCHEMA_PROFILE: profile } }, + ); + try { + const response = await harness.mcp.callTool({ + name: "procedure_pseudo_code", + arguments: { procedure: "file-offset:0x400" }, + }); + const { error } = parseMcpToolError(response); + expect(error).toMatchObject({ + code: "invalid_request", + message, + remediation: { + action: expect.stringContaining("ram:0x1000"), + }, + }); + expect(error.remediation.action).toContain("inspect_native_load_image"); + expect(error.remediation.action).toContain("bare 0x"); + expect(error.recovery).toBeUndefined(); + } finally { + await harness.close(); + } + }, +);