From d3e9ee5505251f760666bb8e1e6c295b6d6aedb2 Mon Sep 17 00:00:00 2001 From: Mykola Dementiev Date: Sun, 11 Oct 2026 16:52:16 +0200 Subject: [PATCH] fix(ida): report a procedure IDA cannot resolve as invalid input (#1785) * test(ida): expect a missing procedure to be reported as invalid input * test(ida): name the invalid_request code and its category for a missing procedure * fix(ida): report a procedure IDA cannot resolve as invalid input * test(ida): pin the missing-procedure issue shape and the lookups that stay unreadable * fix(ida): treat a lookup answer for another query as a producer fault * test(ida): expect an argument-independent issue path and pin legacy error replies and escaping * fix(ida): report a missing procedure without naming an argument path * test(ida): expect provider-derived selectors to stay producer faults in composed workflows * fix(ida): keep provider-derived procedure selectors out of the caller's input errors * test(ida): cover session forwarding, parameter hygiene and structured derived-node errors * docs(ida): describe when procedure resolution blames the caller --- src/application/AnalysisProvider.ts | 7 + src/application/CallPathTracing.ts | 8 +- .../EnhancedToolSelectorOrigin.test.ts | 142 +++++++++ src/application/EnhancedTools.ts | 17 +- src/application/binary/BinarySession.ts | 4 +- .../EnhancedNativeInvestigation.test.ts | 34 ++ src/application/native/NativeCallRoutes.ts | 11 +- src/application/native/NativeUiActionTrace.ts | 1 + .../native/NativeValueTrace.test.ts | 29 ++ src/application/native/NativeValueTrace.ts | 5 +- src/ida/IdaOperationFailures.test.ts | 290 ++++++++++++++++++ src/ida/IdaOperationRunner.ts | 40 ++- src/ida/IdaSessionClient.ts | 17 +- .../providers/ida/selectorProvenance.test.ts | 131 ++++++++ .../analysis-sessions/binarySession.test.ts | 38 +++ 15 files changed, 753 insertions(+), 21 deletions(-) create mode 100644 src/application/EnhancedToolSelectorOrigin.test.ts create mode 100644 tests/boundary/providers/ida/selectorProvenance.test.ts diff --git a/src/application/AnalysisProvider.ts b/src/application/AnalysisProvider.ts index a922cdb40..d88da6df7 100644 --- a/src/application/AnalysisProvider.ts +++ b/src/application/AnalysisProvider.ts @@ -19,6 +19,13 @@ import type { ProviderOperationHealth } from "../domain/providerOperationHealth. export interface ExecutionOptions { readonly signal?: AbortSignal; readonly progress?: ProgressReporter; + /** + * Set when a procedure selector came from provider output, such as a + * revisited callee, rather than from the caller. A provider-derived + * selector is never the caller's input mistake. Absent means the caller + * supplied it. + */ + readonly selectorOrigin?: "provider"; } /** Closed operations accepted by provider-backed analysis sessions. */ diff --git a/src/application/CallPathTracing.ts b/src/application/CallPathTracing.ts index b6e9eb339..c743e91a9 100644 --- a/src/application/CallPathTracing.ts +++ b/src/application/CallPathTracing.ts @@ -1,4 +1,7 @@ -import type { AnalysisOperation } from "./AnalysisProvider.js"; +import type { + AnalysisOperation, + ExecutionOptions, +} from "./AnalysisProvider.js"; import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import type { AnalysisError } from "../domain/analysisErrorBase.js"; import { parseRelatedAddresses } from "../domain/hopperValues.js"; @@ -11,6 +14,7 @@ type AnalysisCall = ( name: AnalysisOperation, arguments_: Readonly>, signal?: AbortSignal, + selectorOrigin?: ExecutionOptions["selectorOrigin"], ) => Promise>; export interface CallPathTraceInput { @@ -114,7 +118,7 @@ const expandAddress = async ( const relation = input.direction === "forward" ? "callees" : "callers"; const tool = input.direction === "forward" ? "procedure_callees" : "procedure_callers"; - const result = await call(tool, { procedure: address }, signal); + const result = await call(tool, { procedure: address }, signal, "provider"); if (!result.ok) { state.failures.push({ address, diff --git a/src/application/EnhancedToolSelectorOrigin.test.ts b/src/application/EnhancedToolSelectorOrigin.test.ts new file mode 100644 index 000000000..387d4ebfb --- /dev/null +++ b/src/application/EnhancedToolSelectorOrigin.test.ts @@ -0,0 +1,142 @@ +import { describe, expect, it } from "vitest"; +import { + createAnalysisExecution, + type AnalysisOperation, + type AnalysisOperationPort, +} from "./AnalysisProvider.js"; +import { AnalysisInputError } from "../domain/analysisErrorCore.js"; +import type { JsonValue } from "../domain/jsonValue.js"; +import { err, ok } from "../domain/result.js"; +import { EnhancedTools } from "./EnhancedTools.js"; + +const provider = { id: "fixture", name: "Fixture", version: "1" }; + +interface RecordedCall { + readonly operation: AnalysisOperation; + readonly procedure: unknown; + readonly origin: unknown; +} + +/** Port that knows `main` (0x1000, calling 0x2000) and rejects `nosuch`. */ +const recordingPort = () => { + const calls: RecordedCall[] = []; + const sentParameters: Readonly>[] = []; + const port: AnalysisOperationPort = { + execute: async (operation, parameters, options) => { + sentParameters.push(parameters); + calls.push({ + operation, + procedure: parameters.procedure, + origin: options?.selectorOrigin, + }); + const known = (value: JsonValue) => + ok(createAnalysisExecution(value, provider)); + if (parameters.procedure === "nosuch") + return err(new AnalysisInputError(operation)); + switch (operation) { + case "procedure_address": + return known(parameters.procedure === "main" ? "0x1000" : "0x2000"); + case "procedure_callees": + return known(parameters.procedure === "0x1000" ? ["0x2000"] : []); + case "procedure_pseudo_code": + return known("int main(void) { return 0; }"); + case "address_name": + return known("main"); + default: + throw new Error(`Unexpected operation: ${operation}`); + } + }, + }; + return { calls, sentParameters, tools: new EnhancedTools(port) }; +}; + +const of = ( + calls: readonly RecordedCall[], + operation: AnalysisOperation, +): RecordedCall[] => calls.filter((call) => call.operation === operation); + +describe("selector origin of composed workflows", () => { + it("marks every call-graph expansion as provider-derived and the root selector as caller-supplied", async () => { + const { calls, tools } = recordingPort(); + + const result = await tools.execute("get_call_graph", { address: "main" }); + + expect(result.ok, "the graph is built").toBe(true); + expect( + of(calls, "procedure_address"), + "the caller's selector carries no provider origin", + ).toEqual([ + { operation: "procedure_address", procedure: "main", origin: undefined }, + ]); + expect( + of(calls, "procedure_callees"), + "the canonical start and every discovered callee are provider-derived", + ).toEqual([ + { + operation: "procedure_callees", + procedure: "0x1000", + origin: "provider", + }, + { + operation: "procedure_callees", + procedure: "0x2000", + origin: "provider", + }, + ]); + }); + + it("marks trace expansions as provider-derived and the start and goal as caller-supplied", async () => { + const { calls, tools } = recordingPort(); + + const result = await tools.execute("trace_call_path", { + start: "main", + goal: "0x2000", + }); + + expect(result.ok, "the trace completes").toBe(true); + expect( + of(calls, "procedure_address").map(({ origin }) => origin), + "start and goal resolutions are caller-supplied", + ).toEqual([undefined, undefined]); + expect( + of(calls, "procedure_callees").map(({ origin }) => origin), + "expansion of the resolved start is provider-derived", + ).toEqual(["provider"]); + }); + + it("marks the canonical retry of batch_decompile as derived and keeps the unresolved fallback caller-supplied", async () => { + const { calls, tools } = recordingPort(); + + const result = await tools.execute("batch_decompile", { + addresses: ["main", "nosuch"], + }); + + expect(result.ok, "per-item failures do not fail the batch").toBe(true); + expect( + of(calls, "procedure_pseudo_code") + .map(({ procedure, origin }) => [procedure, origin]) + .sort(([left], [right]) => String(left).localeCompare(String(right))), + "the resolved canonical address is derived; the failed item keeps the caller's text", + ).toEqual([ + ["0x1000", "provider"], + ["nosuch", undefined], + ]); + }); + + it("never copies the origin into the parameters sent to the provider", async () => { + const { sentParameters, tools } = recordingPort(); + + await tools.execute("get_call_graph", { address: "main" }); + await tools.execute("trace_call_path", { start: "main" }); + await tools.execute("batch_decompile", { addresses: ["main"] }); + + expect( + sentParameters.length, + "workflows issued provider calls", + ).toBeGreaterThan(0); + expect( + sentParameters.some((parameters) => "selectorOrigin" in parameters), + "the origin travels in the options only, never in Evidence-bound parameters", + ).toBe(false); + }); +}); diff --git a/src/application/EnhancedTools.ts b/src/application/EnhancedTools.ts index c4ec83c6c..06862be0e 100644 --- a/src/application/EnhancedTools.ts +++ b/src/application/EnhancedTools.ts @@ -3,6 +3,7 @@ import { z } from "zod"; import type { AnalysisOperation, AnalysisOperationPort, + ExecutionOptions, } from "./AnalysisProvider.js"; import type { EnhancedToolName } from "../contracts/enhancedInputs.js"; import { enhancedInputSchemas } from "../contracts/enhancedInputs.js"; @@ -163,8 +164,8 @@ export class EnhancedTools { ); case "trace_call_path": return traceCallPath( - (name, arguments_, operationSignal) => - this.#call(name, arguments_, operationSignal), + (name, arguments_, operationSignal, selectorOrigin) => + this.#call(name, arguments_, operationSignal, selectorOrigin), call.input, signal, ); @@ -267,6 +268,7 @@ export class EnhancedTools { "procedure_pseudo_code", { procedure: resolved.ok ? resolved.value : address }, signal, + resolved.ok ? "provider" : undefined, ); if (!result.ok) return { @@ -361,6 +363,7 @@ export class EnhancedTools { tool, { procedure: current.address }, signal, + "provider", ); if (!result.ok) { graph[level].push({ @@ -518,13 +521,13 @@ export class EnhancedTools { name: AnalysisOperation, arguments_: Readonly>, signal?: AbortSignal, + selectorOrigin?: ExecutionOptions["selectorOrigin"], ): Promise> { if (isAborted(signal)) return err(new AnalysisCancelledError(name)); - const execution = await this.analysis.execute( - name, - arguments_, - signal === undefined ? {} : { signal }, - ); + const execution = await this.analysis.execute(name, arguments_, { + ...(signal === undefined ? {} : { signal }), + ...(selectorOrigin === undefined ? {} : { selectorOrigin }), + }); if (isAborted(signal)) return err(new AnalysisCancelledError(name)); return execution.ok ? ok(execution.value.result) : execution; } diff --git a/src/application/binary/BinarySession.ts b/src/application/binary/BinarySession.ts index d64e04f89..b12b310ff 100644 --- a/src/application/binary/BinarySession.ts +++ b/src/application/binary/BinarySession.ts @@ -497,7 +497,7 @@ export class BinarySession execute( name: Parameters[0], arguments_: Readonly>, - options?: { readonly signal?: AbortSignal }, + options?: Pick, ): Promise> { const generation = this.#transitionGeneration; const call = this.#execute(name, arguments_, options, this.#transition); @@ -535,7 +535,7 @@ export class BinarySession async #execute( name: Parameters[0], arguments_: Readonly>, - options: { readonly signal?: AbortSignal } | undefined, + options: Pick | undefined, transition: Promise, ): Promise> { const transitioned = await this.#waitForTransition( diff --git a/src/application/native/EnhancedNativeInvestigation.test.ts b/src/application/native/EnhancedNativeInvestigation.test.ts index 89756fcd2..087b5981f 100644 --- a/src/application/native/EnhancedNativeInvestigation.test.ts +++ b/src/application/native/EnhancedNativeInvestigation.test.ts @@ -246,6 +246,40 @@ describe("native UI action trace", () => { }); }); +describe("native UI action selector origin", () => { + it("marks provider-reported function addresses as derived and the action seed as caller-supplied", async () => { + const inner = testAnalysis({ + procedure_address: () => execution("procedure_address", "0x1000"), + }); + const recorded: { operation: string; origin: unknown }[] = []; + + const result = await new EnhancedTools({ + execute: async (operation, parameters, options) => { + recorded.push({ operation, origin: options?.selectorOrigin }); + return inner.execute(operation, parameters); + }, + }).execute("trace_native_ui_action", { action: "0x1000" }); + + expect(result.ok, "the trace completes").toBe(true); + expect( + recorded + .filter(({ operation }) => operation === "procedure_address") + .map(({ origin }) => origin), + "the caller's action seed carries no provider origin", + ).toEqual([undefined]); + const probes = recorded.filter( + ({ operation }) => + operation === "procedure_references" || + operation === "procedure_callees", + ); + expect(probes.length, "relationships were probed").toBeGreaterThan(0); + expect( + probes.every(({ origin }) => origin === "provider"), + "every relationship probe uses a provider-reported address", + ).toBe(true); + }); +}); + describe("bounded native UI tracing", () => { it("continues handler traversal from its own depth when another UI branch is deeper", async () => { const deepGraph = { diff --git a/src/application/native/NativeCallRoutes.ts b/src/application/native/NativeCallRoutes.ts index bb819d742..fe77f61e9 100644 --- a/src/application/native/NativeCallRoutes.ts +++ b/src/application/native/NativeCallRoutes.ts @@ -1,5 +1,8 @@ import { z } from "zod"; -import type { AnalysisOperationPort } from "../AnalysisProvider.js"; +import type { + AnalysisOperationPort, + ExecutionOptions, +} from "../AnalysisProvider.js"; import { parseRelatedAddresses, referenceKindSchema, @@ -26,8 +29,12 @@ export const readNativeCallRoutes = async ( analysis: AnalysisOperationPort, procedure: string, signal?: AbortSignal, + selectorOrigin?: ExecutionOptions["selectorOrigin"], ) => { - const options = signal === undefined ? {} : { signal }; + const options = { + ...(signal === undefined ? {} : { signal }), + ...(selectorOrigin === undefined ? {} : { selectorOrigin }), + }; const execution = await analysis.execute( "procedure_references", { procedure, direction: "outgoing" }, diff --git a/src/application/native/NativeUiActionTrace.ts b/src/application/native/NativeUiActionTrace.ts index 9b975b32c..141b31bc0 100644 --- a/src/application/native/NativeUiActionTrace.ts +++ b/src/application/native/NativeUiActionTrace.ts @@ -411,6 +411,7 @@ const addDirectCallees = async (input: { input.analysis, current.address, input.signal, + "provider", ); examined += 1; if (!execution.ok) { diff --git a/src/application/native/NativeValueTrace.test.ts b/src/application/native/NativeValueTrace.test.ts index bea7675ba..cdc663d54 100644 --- a/src/application/native/NativeValueTrace.test.ts +++ b/src/application/native/NativeValueTrace.test.ts @@ -529,3 +529,32 @@ describe("bounded native value dependency composition", () => { }); }); }); + +describe("native value trace selector origin", () => { + it("marks only the initial procedure as caller-supplied", async () => { + const analyzed: { procedure: unknown; origin: unknown }[] = []; + + const result = await traceNativeValues( + { + execute: async (operation, parameters, options) => { + if (operation === "analyze_function") + analyzed.push({ + procedure: parameters.procedure, + origin: options?.selectorOrigin, + }); + return analysis.execute(operation, parameters, options); + }, + }, + { procedure: "0x1000" }, + ); + + expect(result.ok, "the trace completes").toBe(true); + expect( + analyzed, + "the caller's procedure is unmarked and the discovered callee is provider-derived", + ).toEqual([ + { procedure: "0x1000", origin: undefined }, + { procedure: "0x2000", origin: "provider" }, + ]); + }); +}); diff --git a/src/application/native/NativeValueTrace.ts b/src/application/native/NativeValueTrace.ts index 3bf7b56db..71c602396 100644 --- a/src/application/native/NativeValueTrace.ts +++ b/src/application/native/NativeValueTrace.ts @@ -57,7 +57,10 @@ export const traceNativeValues = async ( const execution = await analysis.execute( "analyze_function", { procedure: next.procedure }, - signal === undefined ? {} : { signal }, + { + ...(signal === undefined ? {} : { signal }), + ...(next.depth > 0 ? { selectorOrigin: "provider" as const } : {}), + }, ); decompilations++; if (!execution.ok) { diff --git a/src/ida/IdaOperationFailures.test.ts b/src/ida/IdaOperationFailures.test.ts index 25b2bab8a..e863266d0 100644 --- a/src/ida/IdaOperationFailures.test.ts +++ b/src/ida/IdaOperationFailures.test.ts @@ -5,6 +5,9 @@ import { RecordingIdaMcp, } from "../../tests/fixtures/idaMcp.js"; import type { JsonValue } from "../domain/jsonValue.js"; +import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; +import { AnalysisInputError } from "../domain/analysisErrorCore.js"; +import { decodeIdaToolResult } from "./IdaMcpConnection.js"; import type { AnalysisOperation } from "../application/AnalysisProvider.js"; import { IdaSessionClient } from "./IdaSessionClient.js"; @@ -96,3 +99,290 @@ describe("headless IDA operation failures", () => { }, ); }); + +const lookup = (query: string, fn: JsonValue, error: string | null) => [ + { query, fn, error }, +]; +const knownFunction = { addr: "0x1000", name: "main", size: "0x20" }; + +const missingProcedureFixture = async ( + answer: (query: string) => JsonValue | undefined, +) => { + const { root, target } = await createIdaTarget(); + roots.push(root); + const producer = new RecordingIdaMcp(target, "headless"); + producer.overrideCall = (name, args) => { + if (name !== "lookup_funcs") return undefined; + const queries = args.queries; + const query = Array.isArray(queries) ? String(queries[0]) : ""; + return answer(query); + }; + const client = new IdaSessionClient( + { + command: "fixture", + args: [], + env: {}, + mode: "headless", + timeoutMs: 1000, + workspaceRoot: root, + }, + target, + producer, + ); + return client; +}; + +describe("headless IDA procedure lookup failures", () => { + it.each([ + ["Not found", "procedure_pseudo_code", "main"], + ["Not found", "procedure_address", "main"], + ["Not found", "analyze_function", "main"], + ["Not a function", "procedure_pseudo_code", "main"], + ["Not a function", "read_function_instructions", "main"], + ["Not a function", "procedure_pseudo_code", "0x1000"], + ["Not found", "procedure_assembly", "_missing"], + ["Not found", "procedure_pseudo_code", "main\nsecond line"], + ] as const)( + "reports the upstream %s answer for %s of %s as an invalid request naming the procedure", + async (upstream, operation, procedure) => { + const client = await missingProcedureFixture((query) => + lookup(query, null, upstream), + ); + try { + const result = await client.execute(operation, { procedure }); + + if (result.ok) throw new Error("expected invalid input"); + expect(result.error._tag, "the caller's procedure is the fault").toBe( + "AnalysisInputError", + ); + const projected = projectAnalysisError(result.error); + expect(projected.code, "the public code is invalid_request").toBe( + "invalid_request", + ); + expect(projected.category, "the category is invalid_input").toBe( + "invalid_input", + ); + expect(projected.retryable, "the caller can correct and retry").toBe( + true, + ); + expect( + projected.details, + "the operation and exactly one issue that names the procedure in its message", + ).toEqual({ + operation, + issues: [ + { + path: [], + reason: "invalid_value", + message: `IDA could not resolve procedure ${JSON.stringify(procedure)}: ${upstream}.`, + }, + ], + }); + if (!(result.error instanceof AnalysisInputError)) + throw new Error("expected an AnalysisInputError"); + expect( + result.error.issues.map(({ message }) => message ?? "").join(""), + "a control character in the query is carried JSON-escaped, never raw", + ).not.toContain("\n"); + expect( + (result.error.cause as Error | undefined)?.message, + "the upstream text stays on the retained cause", + ).toContain(upstream); + expect( + JSON.stringify(projected).toLowerCase(), + "no retry or doctor advice for a caller mistake", + ).not.toMatch(/doctor|retry once/u); + } finally { + expect((await client.close()).ok).toBe(true); + } + }, + ); + + it.each([ + ["an upstream exception text", lookup("main", null, "Database is closed")], + ["a function with an error", lookup("main", knownFunction, "Not found")], + ["neither a function nor an error", lookup("main", null, null)], + [ + "a near-miss spelling of the not-found text", + lookup("main", null, "not found"), + ], + [ + "an exception text that merely contains the not-found text", + lookup("main", null, "Database lookup failed: Not found"), + ], + ["a padded not-found text", lookup("main", null, " Not found ")], + ["an empty error", lookup("main", null, "")], + ["a whitespace-only error", lookup("main", null, " ")], + ["an answer for a different query", lookup("helper", null, "Not found")], + ])("keeps %s as unreadable output", async (_label, answer) => { + const client = await missingProcedureFixture(() => answer); + try { + const result = await client.execute("procedure_pseudo_code", { + procedure: "main", + }); + + if (result.ok) throw new Error("expected unreadable output"); + expect(result.error._tag, "a producer fault, not caller input").toBe( + "AnalysisProtocolError", + ); + expect( + projectAnalysisError(result.error).code, + "unknown beats a wrong fact about the caller's input", + ).toBe("unreadable_output"); + } finally { + expect((await client.close()).ok).toBe(true); + } + }); + + it("keeps an unresolvable callee reported by IDA itself as unreadable output", async () => { + const { root, target } = await createIdaTarget(); + roots.push(root); + const producer = new RecordingIdaMcp(target, "headless"); + producer.overrideCall = (name, args) => { + if (name === "callees") + return [ + { + callees: [{ addr: "0x2000", name: "sub_2000", type: "internal" }], + more: false, + }, + ]; + if (name !== "lookup_funcs") return undefined; + const queries = args.queries; + const query = Array.isArray(queries) ? String(queries[0]) : ""; + return query === "0x2000" + ? lookup(query, null, "Not a function") + : lookup(query, knownFunction, null); + }; + const client = new IdaSessionClient( + { + command: "fixture", + args: [], + env: {}, + mode: "headless", + timeoutMs: 1000, + workspaceRoot: root, + }, + target, + producer, + ); + try { + const result = await client.execute("procedure_callees", { + procedure: "main", + }); + + if (result.ok) throw new Error("expected unreadable output"); + expect( + projectAnalysisError(result.error).code, + "an address IDA itself reported is not the caller's mistake", + ).toBe("unreadable_output"); + } finally { + expect((await client.close()).ok).toBe(true); + } + }); +}); + +describe("provider-derived procedure selectors", () => { + it.each(["Not found", "Not a function"])( + "reports a derived selector answered %s as a producer fault", + async (upstream) => { + const client = await missingProcedureFixture((query) => + lookup(query, null, upstream), + ); + try { + const result = await client.execute( + "procedure_callees", + { procedure: "0x2000" }, + { selectorOrigin: "provider" }, + ); + + if (result.ok) throw new Error("expected a producer fault"); + expect(result.error._tag, "not the caller's mistake").toBe( + "AnalysisProtocolError", + ); + expect( + projectAnalysisError(result.error).code, + "projected as on main", + ).toBe("unreadable_output"); + } finally { + expect((await client.close()).ok).toBe(true); + } + }, + ); +}); + +describe("attached IDA procedure lookup failures", () => { + it("leaves a failed legacy lookup as unreadable output", async () => { + const { root, target } = await createIdaTarget(); + roots.push(root); + const producer = new RecordingIdaMcp(target, "attached"); + producer.overrideCall = (name) => + name === "get_function_by_name" ? null : undefined; + const client = new IdaSessionClient( + { + command: "fixture", + args: [], + env: {}, + mode: "attached", + timeoutMs: 1000, + workspaceRoot: root, + }, + target, + producer, + ); + try { + const result = await client.execute("procedure_pseudo_code", { + procedure: "main", + }); + + if (result.ok) throw new Error("expected unreadable output"); + expect( + projectAnalysisError(result.error).code, + "the legacy profile keeps its pre-existing classification", + ).toBe("unreadable_output"); + } finally { + expect((await client.close()).ok).toBe(true); + } + }); + + it("leaves an MCP error reply for an unknown legacy name as unreadable output", async () => { + const { root, target } = await createIdaTarget(); + roots.push(root); + const producer = new RecordingIdaMcp(target, "attached"); + producer.beforeCall = async (name) => { + if (name === "get_function_by_name") + decodeIdaToolResult({ + content: [{ type: "text", text: "Function not found: main" }], + isError: true, + }); + }; + const client = new IdaSessionClient( + { + command: "fixture", + args: [], + env: {}, + mode: "attached", + timeoutMs: 1000, + workspaceRoot: root, + }, + target, + producer, + ); + try { + const result = await client.execute("procedure_pseudo_code", { + procedure: "main", + }); + + if (result.ok) throw new Error("expected unreadable output"); + expect( + result.error._tag, + "a producer error reply, not caller input", + ).toBe("AnalysisProtocolError"); + expect( + projectAnalysisError(result.error).code, + "the legacy profile keeps its existing classification", + ).toBe("unreadable_output"); + } finally { + expect((await client.close()).ok).toBe(true); + } + }); +}); diff --git a/src/ida/IdaOperationRunner.ts b/src/ida/IdaOperationRunner.ts index 553053a31..51de0352f 100644 --- a/src/ida/IdaOperationRunner.ts +++ b/src/ida/IdaOperationRunner.ts @@ -51,6 +51,11 @@ const identity = (fn: IdaFunction) => ({ classification: null, body: unknownBody, }); +/** The only lookup answers upstream reports for a name or address that is not a function. */ +const MISSING_PROCEDURE_ANSWERS: ReadonlySet = new Set([ + "Not found", + "Not a function", +]); const lines = (asm: IdaDisassembly) => asm.lines.map(({ address, instruction }) => `${address}: ${instruction}`); @@ -64,6 +69,7 @@ export class IdaOperationRunner { private readonly protocol: "legacy" | "modern", private readonly database?: string, private readonly signal?: AbortSignal, + private readonly selectorOrigin?: "provider", ) {} async call( @@ -106,11 +112,42 @@ export class IdaOperationRunner { return result; } - async resolve(query: string): Promise { + /** + * Resolves a procedure. When `requestedBy` names the caller's operation, an + * upstream "not found" or "not a function" answer is reported as invalid + * input; every other failed lookup stays a producer fault. `run()` passes + * `requestedBy` only for caller-supplied selectors. Provider-derived + * selectors, such as the addresses composed workflows revisit, and the + * callee resolution inside `relationships()` pass none, because those + * addresses were reported by IDA itself. + */ + async resolve(query: string, requestedBy?: string): Promise { if (this.protocol === "modern") { const item = modernLookupSchema.parse( await this.call("lookup_funcs", { queries: [query] }), )[0]; + if ( + requestedBy !== undefined && + item?.query === query && + item.fn === null && + item.error !== null && + MISSING_PROCEDURE_ANSWERS.has(item.error) + ) + throw new AnalysisInputError( + requestedBy, + { + cause: new AnalysisProtocolError( + `IDA function lookup failed for ${query}: ${item.error}`, + ), + }, + [ + { + path: [], + reason: "invalid_value", + message: `IDA could not resolve procedure ${JSON.stringify(query)}: ${item.error}.`, + }, + ], + ); if (item?.fn == null || item.error !== null) throw new AnalysisProtocolError( `IDA function lookup failed for ${query}: ${item?.error ?? "missing function"}`, @@ -351,6 +388,7 @@ export class IdaOperationRunner { } const fn = await this.resolve( z.string().min(1).parse(parameters.procedure), + this.selectorOrigin === "provider" ? undefined : operation, ); this.locations.push({ kind: "address", address: fn.address }); switch (operation) { diff --git a/src/ida/IdaSessionClient.ts b/src/ida/IdaSessionClient.ts index 48920a725..8de42a67d 100644 --- a/src/ida/IdaSessionClient.ts +++ b/src/ida/IdaSessionClient.ts @@ -192,12 +192,7 @@ export class IdaSessionClient implements AnalysisClient { throw new AnalysisProtocolError( "IDA operation changed after validation.", ); - const runner = new IdaOperationRunner( - this.connection, - this.config.mode === "attached" ? "legacy" : "modern", - this.#database, - options?.signal, - ); + const runner = this.#runner(options); const result = await runner.run(operation, parameters); if (wasCancelled(options)) return err(new AnalysisCancelledError(operation)); @@ -227,6 +222,16 @@ export class IdaSessionClient implements AnalysisClient { } } + #runner(options: ExecutionOptions | undefined): IdaOperationRunner { + return new IdaOperationRunner( + this.connection, + this.config.mode === "attached" ? "legacy" : "modern", + this.#database, + options?.signal, + options?.selectorOrigin, + ); + } + #health(metadata: JsonValue) { return ok( createAnalysisExecution( diff --git a/tests/boundary/providers/ida/selectorProvenance.test.ts b/tests/boundary/providers/ida/selectorProvenance.test.ts new file mode 100644 index 000000000..e18a1d3af --- /dev/null +++ b/tests/boundary/providers/ida/selectorProvenance.test.ts @@ -0,0 +1,131 @@ +import { rm } from "node:fs/promises"; +import { afterEach, describe, expect, it } from "vitest"; +import { EnhancedTools } from "../../../../src/application/EnhancedTools.js"; +import { IdaSessionClient } from "../../../../src/ida/IdaSessionClient.js"; +import type { JsonValue } from "../../../../src/domain/jsonValue.js"; +import { createIdaTarget, RecordingIdaMcp } from "../../../fixtures/idaMcp.js"; + +const roots: string[] = []; +afterEach(async () => { + for (const root of roots.splice(0)) + await rm(root, { recursive: true, force: true }); +}); + +const entry = { addr: "0x1000", name: "main", size: "0x20" }; + +/** Headless session where `main` resolves, its external callee 0x2000 is not a function, and `nosuch` is unknown. */ +const session = async () => { + const { root, target } = await createIdaTarget(); + roots.push(root); + const producer = new RecordingIdaMcp(target, "headless"); + producer.overrideCall = (name, args): JsonValue | undefined => { + if (name !== "lookup_funcs") return undefined; + const queries = args.queries; + const query = Array.isArray(queries) ? String(queries[0]) : ""; + if (query === "main" || query === "0x1000") + return [{ query, fn: entry, error: null }]; + return [ + { + query, + fn: null, + error: query === "nosuch" ? "Not found" : "Not a function", + }, + ]; + }; + const client = new IdaSessionClient( + { + command: "fixture", + args: [], + env: {}, + mode: "headless", + timeoutMs: 1000, + workspaceRoot: root, + }, + target, + producer, + ); + return { client, tools: new EnhancedTools(client) }; +}; + +describe("provider-derived selectors in IDA composed workflows", () => { + it("records an external callee IDA cannot resolve as a producer fault in the call graph", async () => { + const { client, tools } = await session(); + try { + const result = await tools.execute("get_call_graph", { address: "main" }); + + if (!result.ok) throw result.error; + expect( + result.value, + "the resolved root stays ok and only the derived node carries the producer fault", + ).toMatchObject({ + "0": [{ address: "0x1000", status: "ok" }], + "1": [ + { + address: "0x2000", + status: "error", + error: { code: "unreadable_output" }, + }, + ], + }); + } finally { + expect((await client.close()).ok).toBe(true); + } + }); + + it("records the same node as a producer fault in the call path", async () => { + const { client, tools } = await session(); + try { + const result = await tools.execute("trace_call_path", { start: "main" }); + + if (!result.ok) throw result.error; + expect( + result.value, + "the failure belongs to the derived address and is not blamed on the caller", + ).toMatchObject({ + failures: [{ address: "0x2000", error: { code: "unreadable_output" } }], + }); + } finally { + expect((await client.close()).ok).toBe(true); + } + }); + + it("keeps the caller's own unknown selector an invalid request in each workflow", async () => { + const { client, tools } = await session(); + try { + const graph = await tools.execute("get_call_graph", { + address: "nosuch", + }); + const trace = await tools.execute("trace_call_path", { start: "nosuch" }); + const batch = await tools.execute("batch_decompile", { + addresses: ["nosuch"], + }); + + if (graph.ok || trace.ok) throw new Error("expected invalid requests"); + expect(graph.error._tag, "graph start is caller input").toBe( + "AnalysisInputError", + ); + expect(trace.error._tag, "trace start is caller input").toBe( + "AnalysisInputError", + ); + if (!batch.ok) throw batch.error; + expect( + batch.value, + "the unresolved batch item and its fallback retry are both caller input", + ).toMatchObject({ + items: [ + { + address: "nosuch", + procedure: { + status: "unknown", + error: { code: "invalid_request" }, + }, + status: "error", + error: { code: "invalid_request" }, + }, + ], + }); + } finally { + expect((await client.close()).ok).toBe(true); + } + }); +}); diff --git a/tests/composition/analysis-sessions/binarySession.test.ts b/tests/composition/analysis-sessions/binarySession.test.ts index c0563e06b..aa809b6fb 100644 --- a/tests/composition/analysis-sessions/binarySession.test.ts +++ b/tests/composition/analysis-sessions/binarySession.test.ts @@ -1114,3 +1114,41 @@ describe("no replacement start", () => { expect(session.status()).toMatchObject({ open: false }); }); }); + +describe("selector origin forwarding", () => { + it("forwards the per-call selector origin to the provider client on both execution paths", async () => { + const [first] = await createBinarySessionTargets(); + const seen: unknown[] = []; + const session = createTestBinarySession(() => ({ + execute: (_operation, _parameters, options) => { + seen.push(options?.selectorOrigin); + return Promise.resolve(ok(null)); + }, + close: () => Promise.resolve(resultOk(null)), + })); + expect((await session.open(first)).ok, "the target opens").toBe(true); + seen.length = 0; + + await session.execute( + "address_name", + { address: "0x1000" }, + { selectorOrigin: "provider" }, + ); + await session.withAdmittedAnalysis( + "binary_overview", + undefined, + (analysis) => + analysis.execute( + "address_name", + { address: "0x2000" }, + { selectorOrigin: "provider" }, + ), + ); + await session.execute("address_name", { address: "0x3000" }); + + expect( + seen, + "direct and admitted calls forward the origin; an unmarked call carries none", + ).toEqual(["provider", "provider", undefined]); + }); +});