fix(ida): retain private input after ambiguous open (#1845)

fix(ida): retain private input after unverified open
This commit is contained in:
jhuang-tw 2026-10-13 20:58:51 +08:00 committed by GitHub
parent 2a61deafc6
commit 4ea24f9767
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 103 additions and 18 deletions

View File

@ -88,6 +88,8 @@ REA copies the admitted binary into a private temporary directory, checks its di
One REA session owns one database. Stdio registrations default `IDA_MCP_MAX_WORKERS` to `1`; the explicit upstream `--max-workers 1` also bounds a supervisor. Database size and engine memory depend on the target and upstream; REA does not impose an engine memory limit. The original binary is never executed or overwritten. Optional `workspaceRoot` selects an existing absolute temporary-directory parent on the REA host. Ensure it has space for the input copy and IDA working files.
When a completed `idb_open` response reports success or is otherwise ambiguous, but no active owned private worker can be verified, REA retains the private workspace even if `idb_list` appears empty. A later close can retry verification if the original owned worker becomes discoverable. A completed explicit `idb_open` rejection retains the existing no-worker cleanup path. This is a local admission/ownership safeguard and **does not** provide a process-termination receipt or change the legacy reaped-worker protocol.
Headless cleanup uses the upstream database lifecycle. Abruptly terminating REA, its proxy, or the supervisor does not establish worker release. Prefer `close_binary` for MCP sessions; after an interrupted process, inspect upstream session inventory and retained workspace diagnostics before removing working files. This adapter does not claim Windows Job Object containment for the upstream detached workers.
## Connect an agent once

View File

@ -218,6 +218,14 @@ export const openDatabaseSchema = z
.passthrough(),
})
.passthrough();
/** Only an explicit completed rejection preserves legacy no-worker cleanup. */
export const rejectedOpenDatabaseSchema = z
.object({
error: z.string().min(1),
success: z.never().optional(),
})
.passthrough();
export const databaseListSchema = z
.object({
sessions: z.array(

View File

@ -515,7 +515,7 @@ describe("headless IDA transient busy health", () => {
});
it.each([
["free-form text", "token=abc123 /private/home/user secret"],
["free-form text", "token=abc123 /private<home> secret"],
["an over-long name", "x".repeat(200)],
])(
"reports %s from the worker as unknown instead of echoing it",
@ -899,6 +899,85 @@ describe("headless IDA reaped worker cleanup", () => {
});
});
/** A completed open response is not proof that its private Worker was ever released. */
describe("headless IDA unresolved successful-open ownership", () => {
it.each(["matching", "foreign-input", "malformed"] as const)(
"retains the private input after %s success with incomplete initial discovery",
async (kind) => {
const { producer, client, root } = await fixture("headless");
let hidden = true;
let ownedId: string | undefined;
let privateInput: string | undefined;
producer.beforeCall = async (name, args) => {
if (name !== "idb_open") return;
ownedId = z.string().parse(args.preferred_session_id);
privateInput = z.string().parse(args.input_path);
producer.database = ownedId;
producer.inputPath = privateInput;
};
producer.overrideCall = (name) => {
if (name === "idb_open") {
const originalId = z.string().parse(ownedId);
const originalInput = z.string().parse(privateInput);
if (kind === "foreign-input")
return {
success: true,
session: {
session_id: "foreign-session",
input_path: "/foreign/input",
},
};
if (kind === "malformed")
return { success: true, session: { session_id: originalId } };
return {
success: true,
session: { session_id: originalId, input_path: originalInput },
};
}
if (name === "idb_list" && hidden) return { sessions: [] };
return undefined;
};
expect((await client.execute("health", {})).ok).toBe(false);
const incomplete = await client.close();
expect(incomplete.ok).toBe(false);
if (!incomplete.ok) expect(incomplete.error.cleanupIncomplete).toBe(true);
expect(producer.closes).toBe(0);
expect(producer.database).toBe(ownedId);
expect(
(await readdir(root)).some((name) => name.startsWith("rea-ida-")),
).toBe(true);
expect(producer.calls.some(({ name }) => name === "idb_close")).toBe(
false,
);
// A later positive private ownership record permits ordinary save=false
// cleanup, without ever adopting the foreign open-response identity.
hidden = false;
expect((await client.close()).ok).toBe(true);
expect(producer.calls.filter(({ name }) => name === "idb_close")).toEqual(
[{ name: "idb_close", args: { database: ownedId, save: false } }],
);
expect(producer.closes).toBe(1);
expect(await readdir(root)).toEqual(["target.elf"]);
},
);
it("preserves cleanup after a completed, explicit idb_open rejection", async () => {
const { producer, client, root } = await fixture("headless");
producer.overrideCall = (name) =>
name === "idb_open"
? { error: "Input was rejected before creating a session" }
: undefined;
expect((await client.execute("health", {})).ok).toBe(false);
expect((await client.close()).ok).toBe(true);
expect(producer.calls.some(({ name }) => name === "idb_close")).toBe(false);
expect(producer.closes).toBe(1);
expect(await readdir(root)).toEqual(["target.elf"]);
});
});
describe("headless IDA cleanup verification", () => {
it("retries failed worker release through the retained connection before removing its workspace", async () => {
const { producer, client, root } = await fixture("headless");
@ -943,23 +1022,6 @@ describe("headless IDA cleanup verification", () => {
).toHaveLength(1);
expect(await readdir(root)).toEqual(["target.elf"]);
});
it("never closes an unrelated database when startup returns a different input identity", async () => {
const { producer, client, root } = await fixture("headless");
producer.overrideCall = (name) =>
name === "idb_open"
? {
success: true,
session: {
session_id: "user-database",
input_path: "/unrelated/input",
},
}
: undefined;
expect((await client.execute("health", {})).ok).toBe(false);
expect((await client.close()).ok).toBe(true);
expect(producer.calls.some(({ name }) => name === "idb_close")).toBe(false);
expect(await readdir(root)).toEqual(["target.elf"]);
});
it("releases a privately owned worker after malformed open output using verified session inventory", async () => {
const { producer, client, root } = await fixture("headless");
producer.beforeCall = async (name, args) => {

View File

@ -43,6 +43,7 @@ import {
legacyMetadataSchema,
modernHealthSchema,
openDatabaseSchema,
rejectedOpenDatabaseSchema,
} from "./IdaProtocolValues.js";
import {
idaFileDigest,
@ -122,6 +123,7 @@ export class IdaSessionClient implements AnalysisClient {
#databaseCloseCompleted = false;
#openAttempted = false;
#openCompleted = false;
#openRejected = false;
#callGraphAdvertised = false;
#surveyAdvertised = false;
#initialIdentity: string | undefined;
@ -469,6 +471,9 @@ export class IdaSessionClient implements AnalysisClient {
preferred_session_id: this.#workspace.sessionId,
});
this.#openCompleted = true;
// A completed but malformed/positive open may still own a live Worker.
// Only an explicit producer rejection keeps the old no-worker branch.
this.#openRejected = rejectedOpenDatabaseSchema.safeParse(raw).success;
this.#lifecycle.push(raw);
const opened = openDatabaseSchema.parse(raw);
if (
@ -633,6 +638,14 @@ export class IdaSessionClient implements AnalysisClient {
throw new AnalysisProtocolError(
"An unowned worker still uses the private workspace; it was retained.",
);
if (
this.#database === undefined &&
this.#openCompleted &&
!this.#openRejected
)
throw new AnalysisProtocolError(
"IDA open returned without a verified private worker binding. An empty session inventory does not prove that its Worker exited; the private workspace was retained.",
);
}
if (this.#database !== undefined) {
if (!this.#databaseCloseCompleted) {