From 75dfd1c74722e68bc4e80007b4f9afd8a1d833cb Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sat, 24 Oct 2026 04:20:54 +0800 Subject: [PATCH] fix(ida): retain registration constraints and identity refusal guidance --- docs/ida-provider.md | 8 +++ src/ida/IdaConfiguration.ts | 44 ++++++++++--- src/ida/IdaSessionClient.test.ts | 17 ++++- src/ida/IdaSessionClient.ts | 50 +++++++++++--- .../cli/idaDoctorRegistration.test.ts | 66 +++++++++++++++++++ .../cli/idaStartupAttributionCli.test.ts | 43 +++++++++++- 6 files changed, 206 insertions(+), 22 deletions(-) diff --git a/docs/ida-provider.md b/docs/ida-provider.md index 3f8c4e1eca..25b0191606 100644 --- a/docs/ida-provider.md +++ b/docs/ida-provider.md @@ -195,3 +195,11 @@ an established provider failure. Cancellation retains the caller's original reas and collected diagnostics; metadata timeouts name the requested analysis operation while retaining the resource-read failure as their cause. This request cancellation does not authorize saving or closing an attached GUI database. + +Registration refusals retain schema-authored constraints, including the +native/attached lifecycle requirement, local credential-free endpoint requirement +and absolute workspace root. Configuration diagnostics omit registration values. +Target and private-worker identity refusals retain the failed check, its reason +and recovery guidance in the public error before any analysis replies exist. +These diagnostics do not relax identity admission or authorize closing an attached +GUI database. diff --git a/src/ida/IdaConfiguration.ts b/src/ida/IdaConfiguration.ts index 97684fdfdd..400720998c 100644 --- a/src/ida/IdaConfiguration.ts +++ b/src/ida/IdaConfiguration.ts @@ -4,6 +4,7 @@ import { AnalysisCleanupLease } from "../artifacts/AnalysisCleanupLease.js"; import type { RetainedResourceScope } from "../application/AnalysisProvider.js"; import { ConfigurationError } from "../domain/configurationErrors.js"; import { AnalysisCancelledError } from "../domain/analysisErrorCore.js"; +import { projectInputIssues } from "../domain/inputIssueProjection.js"; import { err, ok, type Result } from "../domain/result.js"; import { withOwnedRegularFile, @@ -132,17 +133,40 @@ export const readIdaConfiguration = ( const envelope = z .object({ mcpServers: z.object({ "ida-pro-mcp": z.unknown() }) }) .safeParse(input); - const parsed = registrationSchema.safeParse( - envelope.success ? envelope.data.mcpServers["ida-pro-mcp"] : input, + const registration = envelope.success + ? envelope.data.mcpServers["ida-pro-mcp"] + : input; + const parsed = registrationSchema.safeParse(registration); + if (parsed.success) return ok(parsed.data); + // Only schema-authored correction text is admitted; Zod's aggregate + // message can contain registration values, including credentials. + const constraints = projectInputIssues( + parsed.error.issues, + registration, + ).flatMap((issue) => + issue.message === undefined + ? [] + : [`${issue.path.join(".") || "registration"}: ${issue.message}`], + ); + if (constraints.length === 0) + return err( + idaConfigurationError( + "REA_IDA_MCP_CONFIG must contain an IDA MCP command/args or local url registration; see docs/ida-provider.md.", + parsed.error, + ), + ); + return err( + new ConfigurationError( + `REA_IDA_MCP_CONFIG: ${constraints.join("; ")}. See docs/ida-provider.md.`, + { + cause: parsed.error, + settings: constraints.map((constraint) => ({ + setting: "REA_IDA_MCP_CONFIG", + constraint, + })), + }, + ), ); - return parsed.success - ? ok(parsed.data) - : err( - idaConfigurationError( - "REA_IDA_MCP_CONFIG must contain an IDA MCP command/args or local url registration; see docs/ida-provider.md.", - parsed.error, - ), - ); } catch (cause: unknown) { if (cause instanceof ConfigurationError) return err(cause); if (cause instanceof NonRegularFileReadError) diff --git a/src/ida/IdaSessionClient.test.ts b/src/ida/IdaSessionClient.test.ts index fb06fb6f28..15afaa203a 100644 --- a/src/ida/IdaSessionClient.test.ts +++ b/src/ida/IdaSessionClient.test.ts @@ -620,7 +620,22 @@ describe("attached IDA function failure handling", () => { producer.inputSha256 = "a".repeat(64); const mismatch = await client.execute("health", {}); expect(mismatch.ok).toBe(false); - if (!mismatch.ok) expect(mismatch.error.message).toContain("SHA-256"); + if (!mismatch.ok) { + expect(projectAnalysisError(mismatch.error)).toMatchObject({ + code: "unreadable_output", + retryable: false, + message: expect.stringContaining("SHA-256"), + remediation: { action: expect.stringContaining("headless mode") }, + details: { + operation: "health", + diagnostics: { + identity_check: "attached_sha256", + reason: expect.stringContaining(target.path), + }, + }, + }); + } + expect(producer.calls.map(({ name }) => name)).toEqual(["get_metadata"]); producer.inputSha256 = target.sha256; producer.beforeCall = async (name) => { if (name === "decompile_function") producer.imageBase = "0x2000"; diff --git a/src/ida/IdaSessionClient.ts b/src/ida/IdaSessionClient.ts index 518fce5d0e..aba4d2fcb2 100644 --- a/src/ida/IdaSessionClient.ts +++ b/src/ida/IdaSessionClient.ts @@ -175,6 +175,17 @@ const isReleasedWorkerRow = (entry: { entry.owned === false && entry.is_active === false; +/** Format-owner facts for a refused database/target binding, before any operation replies exist. */ +class IdaIdentityRefusal extends AnalysisProtocolError { + constructor( + readonly reason: string, + readonly recovery: string, + check: string, + ) { + super(reason, { diagnostics: { identity_check: check, reason, recovery } }); + } +} + /** Serial read-only session whose lifecycle authority excludes an attached GUI. */ export class IdaSessionClient implements AnalysisClient { #tail: Promise = Promise.resolve(); @@ -432,7 +443,14 @@ export class IdaSessionClient implements AnalysisClient { context: IdaOperationFailureContext, ) { let failure: AnalysisError; - if (cause instanceof AnalysisCancelledError) { + if (cause instanceof IdaIdentityRefusal) { + failure = new AnalysisOutputError(operation, cause.reason, { + cause, + userMessage: cause.reason, + remediationAction: cause.recovery, + diagnostics: cause.diagnostics, + }); + } else if (cause instanceof AnalysisCancelledError) { failure = new AnalysisCancelledError(operation, { cause: cause.cause === undefined ? options?.signal?.reason : cause.cause, @@ -680,12 +698,16 @@ export class IdaSessionClient implements AnalysisClient { if ( !sameIdaHostPath(opened.session.input_path, this.#workspace.inputPath) ) - throw new AnalysisProtocolError( + throw new IdaIdentityRefusal( "IDA opened a different input than the private admitted target copy.", + "Check the selected input and upstream worker binding. Retry only after retained private-worker cleanup is confirmed.", + "opened_input", ); if (opened.session.session_id !== this.#workspace.sessionId) - throw new AnalysisProtocolError( + throw new IdaIdentityRefusal( "IDA returned an adopted or unexpected session instead of the requested private worker.", + "Use an IDA supervisor that honors the requested private worker session ID. Confirm retained cleanup before reopening the target.", + "worker_session", ); const inventory = databaseListSchema.parse( await this.connection.call("idb_list", {}), @@ -708,8 +730,10 @@ export class IdaSessionClient implements AnalysisClient { entry.is_active !== true || !sameIdaHostPath(entry.input_path, this.#workspace.inputPath) ) - throw new AnalysisProtocolError( + throw new IdaIdentityRefusal( "IDA did not return an active owned headless worker for the private input copy.", + "Configure the supervisor to return an active owned worker for the private input. Confirm retained cleanup before reopening the target.", + "owned_worker_binding", ); this.#database = opened.session.session_id; } @@ -742,8 +766,10 @@ export class IdaSessionClient implements AnalysisClient { this.#metadata = raw; onObserved?.(raw); if (metadata.sha256.toLowerCase() !== this.target.sha256.toLowerCase()) - throw new AnalysisProtocolError( + throw new IdaIdentityRefusal( `The attached IDA input SHA-256 does not match the selected target ${this.target.path}; open that target in IDA or use headless mode.`, + "Open the selected original input in IDA, or configure headless mode and reopen the target through REA.", + "attached_sha256", ); identity = JSON.stringify([ metadata.path, @@ -752,8 +778,10 @@ export class IdaSessionClient implements AnalysisClient { ]); } else { if (this.#database === undefined || this.#workspace === undefined) - throw new AnalysisProtocolError( + throw new IdaIdentityRefusal( "IDA headless session has no database binding.", + "Close the REA analysis session and reopen the selected target using the configured headless supervisor.", + "database_binding", ); const raw = await readSettledHealth(this.connection, { database: this.#database, @@ -768,8 +796,10 @@ export class IdaSessionClient implements AnalysisClient { (await idaFileDigest(this.#workspace.inputPath)) !== this.#workspace.inputSha256 ) - throw new AnalysisProtocolError( + throw new IdaIdentityRefusal( "IDA headless input identity changed after target admission.", + "Close the REA analysis session, confirm private-worker cleanup, then reopen the selected original target.", + "headless_input", ); if (metadata.auto_analysis_ready !== true) throw new AnalysisCapabilityUnavailableError( @@ -798,8 +828,10 @@ export class IdaSessionClient implements AnalysisClient { this.#initialIdentity !== undefined && identity !== this.#initialIdentity ) - throw new AnalysisProtocolError( - "The attached IDA target or image base changed during the REA session; reopen the target.", + throw new IdaIdentityRefusal( + "The IDA target or image base changed during the REA session; reopen the target.", + "Close the REA analysis session and reopen the selected target after confirming the intended input and image base in IDA.", + "session_identity", ); this.#initialIdentity = identity; return this.#metadata; diff --git a/tests/boundary/cli/idaDoctorRegistration.test.ts b/tests/boundary/cli/idaDoctorRegistration.test.ts index 83c61e7aba..dc3389a337 100644 --- a/tests/boundary/cli/idaDoctorRegistration.test.ts +++ b/tests/boundary/cli/idaDoctorRegistration.test.ts @@ -79,3 +79,69 @@ cliTest.skipIf(process.platform !== "darwin").for([true, false])( expect(await readdir(parent)).toEqual([]); }, ); + +cliTest.for([ + { + name: "native headless lifecycle conflict", + registration: { + mode: "headless", + protocol: "native", + url: "http://127.0.0.1:13337/mcp", + }, + constraint: "requires attached mode", + }, + { + name: "nonlocal endpoint", + registration: { + url: "http://192.0.2.10:13337/mcp", + headers: { Authorization: "Bearer registration-secret" }, + }, + constraint: "local HTTP endpoint without URL credentials", + }, + { + name: "relative workspace root", + registration: { + command: "provider-must-not-launch", + workspaceRoot: "relative", + }, + constraint: "workspaceRoot must be absolute", + }, + { + name: "valid registration", + registration: { command: "provider-must-not-launch", args: [] }, + constraint: undefined, + }, +] as const)( + "doctor retains the IDA registration constraint: $name", + async ({ registration, constraint }, { cli }) => { + const root = await createTestTempDirectory("rea-ida-doctor-constraint-"); + const path = join(root, "registration.json"); + await writeFile(path, JSON.stringify(registration)); + const result = await cli.run({ + arguments: ["doctor", "--provider", "ida", "--json"], + environment: { HOME: root, REA_IDA_MCP_CONFIG: path }, + timeoutMs: 10_000, + }); + expect(result.exitCode, result.stderr).toBe( + constraint === undefined ? 0 : 1, + ); + expect(result.json).toMatchObject({ + providerInspections: expect.arrayContaining([ + expect.objectContaining({ + id: "ida", + available: constraint === undefined, + checks: expect.arrayContaining([ + expect.objectContaining({ + name: "registration", + ok: constraint === undefined, + detail: expect.stringContaining( + constraint ?? "no IDA process was launched", + ), + }), + ]), + }), + ]), + }); + expect(result.stdout).not.toContain("registration-secret"); + }, +); diff --git a/tests/boundary/cli/idaStartupAttributionCli.test.ts b/tests/boundary/cli/idaStartupAttributionCli.test.ts index d5e82e4345..2eac95b715 100644 --- a/tests/boundary/cli/idaStartupAttributionCli.test.ts +++ b/tests/boundary/cli/idaStartupAttributionCli.test.ts @@ -34,6 +34,8 @@ type StartupMode = | "connect-timeout" | "catalog-timeout" | "catalog-refused" + | "headless-input-mismatch" + | "unexpected-worker" | "success"; const createStartupSupervisor = async (mode: StartupMode) => { @@ -98,7 +100,11 @@ const createStartupSupervisor = async (mode: StartupMode) => { input = z.string().parse(args.input_path); result = { success: true, - session: { session_id: database, input_path: input }, + session: { + session_id: + mode === "unexpected-worker" ? "unexpected-worker" : database, + input_path: input, + }, }; } else if (name === "idb_list") result = { @@ -127,7 +133,8 @@ const createStartupSupervisor = async (mode: StartupMode) => { } else if (name === "server_health") result = { status: "ok", - input_path: input, + input_path: + mode === "headless-input-mismatch" ? input + ".other" : input, idb_path: input + ".i64", module: "target.elf", imagebase: "0x1000", @@ -165,6 +172,8 @@ workspaceCliTest.for([ "connect-timeout", "catalog-timeout", "catalog-refused", + "headless-input-mismatch", + "unexpected-worker", "success", ] as const)( "attributes selected IDA startup to the public analysis: %s", @@ -208,6 +217,36 @@ workspaceCliTest.for([ return; } expect(result.exitCode).toBe(1); + if (mode === "headless-input-mismatch" || mode === "unexpected-worker") { + expect(calls).not.toContain("decompile"); + expect(calls).toContain("idb_close"); + expect(result.json).toMatchObject({ + code: "unreadable_output", + retryable: false, + message: expect.stringContaining( + mode === "headless-input-mismatch" + ? "input identity changed" + : "unexpected session", + ), + remediation: { + action: expect.stringContaining( + mode === "headless-input-mismatch" ? "reopen" : "session ID", + ), + }, + details: { + operation: "procedure_pseudo_code", + diagnostics: { + identity_check: + mode === "headless-input-mismatch" + ? "headless_input" + : "worker_session", + reason: expect.any(String), + recovery: expect.any(String), + }, + }, + }); + return; + } expect(calls).toEqual([]); expect(result.json).toMatchObject({ code: mode.endsWith("timeout")