fix(ghidra): restore temporary verifier address aliases

Own temporary address-name edits in the shared DOS/COM boundary verifier. Restore and read back the original public name even after an uncertain write; preserve both verification and restoration failures. Keep strict address overflow, collision and CLI/MCP parity assertions.

Closes #2924

Co-authored-by: Kaoru0822 <208281322+kaoru0822-kitauji@users.noreply.github.com>

scripts/lib/real-ghidra-address-boundaries.d.mts

@@ -0,0 +1,8 @@
+import type { Client } from "@modelcontextprotocol/client";
+
+/** Verify segmented aliases and restore the entry's observed public name. */
+export function verifyGhidraAddressBoundaries(
+  client: Pick<Client, "callTool">,
+  entry: string,
+  cli: (command: string, address: string, flags: string[]) => Promise<unknown>,
+): Promise<void>;

scripts/lib/real-ghidra-address-boundaries.mjs

@@ -63,35 +63,67 @@ export async function verifyGhidraAddressBoundaries(client, entry, cli) {
     await query("read_bytes", { address: entry, length: 1 }),
   );
   const aliasKeys = [entry, padded];
-  assert.deepEqual(
-    await query("set_addresses_names", {
-      names: {
-        [entry]: "rea_same_address_alias",
-        [padded]: "rea_same_address_alias",
-      },
-    }),
-    Object.fromEntries(aliasKeys.map((address) => [address, true])),
-    "Equivalent address spellings with one requested name share one edit",
-  );
-  const nameBeforeAliasedBatch = await query("address_name", {
-    address: entry,
-  });
-  assert.equal(nameBeforeAliasedBatch, "rea_same_address_alias");
-  assert.deepEqual(
-    await query("set_addresses_names", {
-      names: {
-        [entry]: "rea_first_address_alias",
-        [padded]: "rea_second_address_alias",
-      },
-    }),
-    Object.fromEntries(aliasKeys.map((address) => [address, false])),
-    "Conflicting spellings of one Ghidra address must be rejected together",
-  );
-  assert.equal(
-    await query("address_name", { address: entry }),
-    nameBeforeAliasedBatch,
-    "A rejected alias collision must leave the preexisting target name unchanged",
-  );
+  const originalName = await query("address_name", { address: entry });
+  assert.equal(typeof originalName, "string", "Entry has no restorable name");
+  const failures = [];
+  let restorationFailed = false;
+  try {
+    assert.deepEqual(
+      await query("set_addresses_names", {
+        names: {
+          [entry]: "rea_same_address_alias",
+          [padded]: "rea_same_address_alias",
+        },
+      }),
+      Object.fromEntries(aliasKeys.map((address) => [address, true])),
+      "Equivalent address spellings with one requested name share one edit",
+    );
+    assert.equal(
+      await query("address_name", { address: entry }),
+      "rea_same_address_alias",
+    );
+    assert.deepEqual(
+      await query("set_addresses_names", {
+        names: {
+          [entry]: "rea_first_address_alias",
+          [padded]: "rea_second_address_alias",
+        },
+      }),
+      Object.fromEntries(aliasKeys.map((address) => [address, false])),
+      "Conflicting spellings of one Ghidra address must be rejected together",
+    );
+    assert.equal(
+      await query("address_name", { address: entry }),
+      "rea_same_address_alias",
+      "A rejected alias collision must leave the preexisting target name unchanged",
+    );
+  } catch (cause) {
+    failures.push(cause);
+  } finally {
+    // Restore even when a failed reply leaves the write outcome uncertain.
+    // The public API restores the observed name, not private symbol provenance.
+    try {
+      assert.equal(
+        await query("set_address_name", { address: entry, name: originalName }),
+        true,
+        "Temporary address alias restoration was rejected",
+      );
+      assert.equal(
+        await query("address_name", { address: entry }),
+        originalName,
+        "Temporary address alias restoration did not restore the observed name",
+      );
+    } catch (cause) {
+      restorationFailed = true;
+      failures.push(cause);
+    }
+  }
+  if (restorationFailed)
+    throw new AggregateError(
+      failures,
+      "Ghidra address boundary verification failed to restore its temporary name",
+    );
+  if (failures.length > 0) throw failures[0];
   for (const [command, flags, operation] of [
     ["read-bytes", ["--length", "1"], "read_bytes"],
     [

tests/boundary/providers/ghidra/addressBoundaryRestoration.test.ts

@@ -0,0 +1,136 @@
+import type { Client } from "@modelcontextprotocol/client";
+import { expect, it } from "vitest";
+
+import { verifyGhidraAddressBoundaries } from "../../../../scripts/lib/real-ghidra-address-boundaries.mjs";
+
+const entry = "0x10000";
+const originalName = "fixture::entry";
+
+const fixture = (
+  options: {
+    failAfterWrite?: Error;
+    restoration?: "reject" | "wrong-readback";
+    initialName?: null;
+  } = {},
+) => {
+  let name: string | null = options.initialName === null ? null : originalName;
+  const writes: Record<string, unknown>[] = [];
+  const success = (value: unknown) => ({
+    content: [],
+    structuredContent: { normalized_result: value },
+  });
+  const error = (operation: string, address: unknown) => ({
+    code: "invalid_request",
+    category: "invalid_input",
+    message: "Address cannot be represented without truncation",
+    retryable: false,
+    remediation: { action: "Select a representable address" },
+    details: {
+      operation,
+      issues: [
+        { address, message: "cannot be represented without truncation" },
+      ],
+    },
+  });
+  const client: Pick<Client, "callTool"> = {
+    callTool: async ({ name: operation, arguments: args = {} }) => {
+      const address = args.address ?? args.procedure;
+      if (typeof address === "string" && address.includes("100010000"))
+        return {
+          isError: true,
+          content: [
+            {
+              type: "text",
+              text: JSON.stringify({ error: error(operation, address) }),
+            },
+          ],
+        };
+      switch (operation) {
+        case "analyze_function":
+          return success({ procedure: { name } });
+        case "read_bytes":
+          return success({ bytes_hex: "90" });
+        case "address_name":
+          return success(name);
+        case "set_addresses_names": {
+          const names = args.names as Record<string, string>;
+          writes.push(names);
+          const requested = Object.values(names);
+          const accepted = new Set(requested).size === 1;
+          if (accepted) {
+            name = requested[0]!;
+            if (options.failAfterWrite !== undefined)
+              throw options.failAfterWrite;
+          }
+          return success(
+            Object.fromEntries(
+              Object.keys(names).map((key) => [key, accepted]),
+            ),
+          );
+        }
+        case "set_address_name":
+          writes.push(args);
+          if (options.restoration === "reject") return success(false);
+          name =
+            options.restoration === "wrong-readback"
+              ? "unexpected_name"
+              : String(args.name);
+          return success(true);
+        default:
+          throw new Error(`Unmodeled operation: ${operation}`);
+      }
+    },
+  };
+  const cli = async (command: string, address: string) => {
+    const operation =
+      command === "read-bytes" ? "read_bytes" : "annotate_native_function";
+    throw Object.assign(new Error("Rejected oversized address"), {
+      code: 1,
+      stdout: JSON.stringify(error(operation, address)),
+    });
+  };
+  return {
+    run: () => verifyGhidraAddressBoundaries(client, entry, cli),
+    observedName: () => name,
+    writes,
+  };
+};
+
+it("restores the exact observed name before the next verifier consumer", async () => {
+  const test = fixture();
+  await test.run();
+  expect(test.observedName()).toBe(originalName);
+});
+
+it("restores an uncertain write and preserves its original failure", async () => {
+  const failure = new Error("Reply lost after applying the alias");
+  const test = fixture({ failAfterWrite: failure });
+  await expect(test.run()).rejects.toBe(failure);
+  expect(test.observedName()).toBe(originalName);
+});
+
+it.each(["reject", "wrong-readback"] as const)(
+  "fails closed when restoration has %s",
+  async (restoration) => {
+    await expect(fixture({ restoration }).run()).rejects.toMatchObject({
+      name: "AggregateError",
+      errors: [expect.any(Error)],
+    });
+  },
+);
+
+it("retains both verification and restoration failures", async () => {
+  const failure = new Error("Reply lost after applying the alias");
+  await expect(
+    fixture({ failAfterWrite: failure, restoration: "reject" }).run(),
+  ).rejects.toMatchObject({
+    name: "AggregateError",
+    errors: [failure, expect.any(Error)],
+  });
+});
+
+it("does not mutate an entry without a restorable public name", async () => {
+  const test = fixture({ initialName: null });
+  await expect(test.run()).rejects.toThrow("Entry has no restorable name");
+  expect(test.writes).toEqual([]);
+});