From 704fa7699ca183d5d5e0518a6fcdbb953ec35a10 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Fri, 9 Oct 2026 05:10:31 +0800 Subject: [PATCH] fix(ghidra): preserve source failures and snapshot ownership --- docs/native-investigation.md | 10 + docs/testing.md | 12 ++ scripts/lib/real-ghidra-boundaries.mjs | 13 ++ scripts/lib/real-ghidra-target-admission.mjs | 191 ++++++++++++++++++ src/domain/analysisErrorPresentation.ts | 2 +- src/ghidra/GhidraProviderClient.ts | 16 ++ src/ghidra/GhidraTargetSnapshot.ts | 80 ++++++-- .../ghidra/ghidraTargetSnapshot.test.ts | 54 +---- 8 files changed, 316 insertions(+), 62 deletions(-) create mode 100644 scripts/lib/real-ghidra-target-admission.mjs diff --git a/docs/native-investigation.md b/docs/native-investigation.md index eea5a8d7d..6255d5e4e 100644 --- a/docs/native-investigation.md +++ b/docs/native-investigation.md @@ -110,6 +110,16 @@ After metadata edits, snapshot saves and imports remain unavailable in that session. Use `export_evidence_bundle` to retain the observations, close without `snapshot_path`, and reopen the target before importing or saving a snapshot. +Ghidra checks its private import copy against the artifact digest selected by +`open_binary`. If the source changes before that copy is acquired, the query +reports `artifact_changed` with the selected path and both digests. Reopen the +stable target to acquire its current identity. An already imported database +continues to describe its captured bytes even if the original path is changed +or removed; this identity failure does not mark the Ghidra installation unavailable. +On Linux and macOS, an unreadable source reports `access_denied`; a removed or +nonregular replacement reports `artifact_changed`. Source copying is cancellable +and never replaces an existing private snapshot. + ```bash rea inspect-native-instruction
--provider ghidra rea resolve-native-call-targets --provider ghidra diff --git a/docs/testing.md b/docs/testing.md index e0ffb7370..3dad3fd7a 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -308,6 +308,18 @@ snapshot is written by an independent real CLI session. They also start a real annotation and snapshot close concurrently: the edit must succeed, the snapshot must be rejected without creating a file, and the edited session must remain usable until explicitly closed. +Source-admission probes change a caller-owned fixture after `open_binary` but +before the first Ghidra query. They require an actionable `artifact_changed` +error preserving both digests and the selected path, unchanged provider +availability, failed-copy cleanup, and successful recovery after reopening. +After import, deleting that source must preserve the captured database identity. +Instruction inspection and containing-function lookup also agree across +hexadecimal case, leading zeros, and encoded default address-space spellings. +The same source-acquisition workflow exercises missing and directory-replaced +inputs, plus real read-permission denial on a non-root host. Root runs report +that permission-denial check as unverified. A focused filesystem integration +check retains selected-platform routing and exclusive creation; two copy/digest +checks duplicated by the real workflow have been removed. Malformed annotation readback, memory completeness, and inventory data remain separate SDK/provider integration cases; success from a real provider cannot establish rejection of a contradictory provider response. diff --git a/scripts/lib/real-ghidra-boundaries.mjs b/scripts/lib/real-ghidra-boundaries.mjs index 8ba6974a3..0de391002 100644 --- a/scripts/lib/real-ghidra-boundaries.mjs +++ b/scripts/lib/real-ghidra-boundaries.mjs @@ -10,6 +10,7 @@ import addFormats from "ajv-formats"; import { mcpTextValue, requireMcpResult } from "./mcp-verifier-results.mjs"; import { verifyLegacyGhidraReferenceSnapshot } from "./ghidra-reference-snapshot-e2e.mjs"; import { verifyGhidraSnapshotLifecycle } from "./real-ghidra-snapshot-lifecycle.mjs"; +import { verifyGhidraTargetAdmission } from "./real-ghidra-target-admission.mjs"; /** Probe real Ghidra location, annotation and error contracts through public adapters. */ export async function verifyGhidraBoundaries( @@ -648,6 +649,13 @@ export async function verifyGhidraBoundaries( ); successfulCalls += snapshotLifecycle.successfulCalls; rejectedCalls += snapshotLifecycle.rejectedCalls; + const targetAdmission = await verifyGhidraTargetAdmission({ + call, + reject: invalid, + target, + entry: address, + runtimeRoot: env.TMPDIR, + }); await assert.rejects(access(socketRoot), { code: "ENOENT" }); await assert.rejects(access(runtimeRoot), { code: "ENOENT" }); assert.equal(await call("procedure_address", { procedure: name }), address); @@ -683,6 +691,11 @@ export async function verifyGhidraBoundaries( legacy_reference_snapshot_rejected: true, mutation_snapshot_lifecycle: true, concurrent_annotation_snapshot_close: true, + changed_source_admission_and_recovery: true, + source_permission_denial_verified: targetAdmission.permissionDenied, + missing_and_nonregular_source_rejected: true, + imported_source_identity_retained: true, + equivalent_instruction_address_spellings: true, source_immutable: true, reopen_discards_edits: true, long_tmpdir_private_socket_cleanup: true, diff --git a/scripts/lib/real-ghidra-target-admission.mjs b/scripts/lib/real-ghidra-target-admission.mjs new file mode 100644 index 000000000..2c256c154 --- /dev/null +++ b/scripts/lib/real-ghidra-target-admission.mjs @@ -0,0 +1,191 @@ +import assert from "node:assert/strict"; +import { createHash } from "node:crypto"; +import { + chmod, + mkdir, + mkdtemp, + readFile, + realpath, + readdir, + rm, + writeFile, +} from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +/** Verify changed-source admission and retained private database identity through MCP. */ +export async function verifyGhidraTargetAdmission({ + call, + reject, + target, + entry, + runtimeRoot, +}) { + const workspace = await mkdtemp( + join(tmpdir(), "rea-ghidra-source-admission-"), + ); + const source = join(workspace, "caller-selected.%PATH%"); + try { + const original = await readFile(target.path); + await writeFile(source, original); + await call("close_binary"); + const opened = await call("open_binary", { + path: source, + provider_id: "ghidra", + }); + assert.equal(opened.sha256, target.sha256); + const selectedPath = await realpath(source); + assert.equal(opened.path, selectedPath); + const before = await call("binary_session"); + const changed = Buffer.concat([ + original, + Buffer.from("REA_SOURCE_CHANGED_AFTER_OPEN"), + ]); + const changedSha256 = createHash("sha256").update(changed).digest("hex"); + await writeFile(source, changed); + const failure = await reject("analyze_function", { procedure: entry }); + assert.equal(failure.code, "artifact_changed"); + assert.equal(failure.category, "integrity_mismatch"); + assert.equal(failure.details.operation, "analyze_function"); + assert.equal(failure.details.path, selectedPath); + assert.ok(failure.details.reason.includes(target.sha256)); + assert.ok(failure.details.reason.includes(changedSha256)); + assert.match(failure.remediation.action, /open_binary/u); + assert.doesNotMatch(failure.remediation.action, /doctor/u); + const after = await call("binary_session"); + assert.equal(after.analysis_run.run_id, before.analysis_run.run_id); + assert.equal(after.sha256, target.sha256); + assert.equal( + after.capabilities.find( + ({ operation }) => operation === "analyze_function", + )?.available, + true, + "An artifact identity mismatch must not mark the installed provider unavailable", + ); + assert.deepEqual( + (await readdir(runtimeRoot)).filter((name) => + name.startsWith("rea-ghidra-"), + ), + [], + "Failed admission must remove the private snapshot without launching Ghidra", + ); + assert.deepEqual(await readFile(source), changed); + const permissionDenied = await verifySourceReadFailures({ + call, + reject, + source, + bytes: changed, + entry, + runtimeRoot, + }); + + const reopened = await call("open_binary", { + path: source, + provider_id: "ghidra", + }); + assert.equal(reopened.sha256, changedSha256); + assert.notEqual( + (await call("binary_session")).analysis_run.run_id, + before.analysis_run.run_id, + ); + const dossier = await call("analyze_function", { procedure: entry }); + const instruction = await call("inspect_native_instruction", { + address: entry, + }); + assert.equal(dossier.procedure.address, entry); + assert.equal(instruction.status, "decoded"); + for (const address of [ + entry.toUpperCase(), + entry.slice(2), + `0x0000${entry.slice(2)}`, + `ram:${entry}`, + `%72%61%6D:${entry}`, + ]) { + assert.deepEqual( + await call("inspect_native_instruction", { address }), + instruction, + ); + assert.equal( + (await call("resolve_containing_procedure", { address })).procedure + .address, + entry, + ); + } + // Imported facts belong to the captured bytes even if the caller's path disappears. + await rm(source); + assert.deepEqual( + await call("analyze_function", { procedure: entry }), + dossier, + ); + assert.deepEqual( + await call("inspect_native_instruction", { address: entry }), + instruction, + ); + assert.equal((await call("binary_session")).sha256, changedSha256); + await call("close_binary"); + await call("open_binary", { path: target.path, provider_id: "ghidra" }); + return { permissionDenied }; + } finally { + await rm(workspace, { recursive: true, force: true }); + } +} + +async function verifySourceReadFailures({ + call, + reject, + source, + bytes, + entry, + runtimeRoot, +}) { + const permissionDenied = + typeof process.getuid === "function" && process.getuid() !== 0; + const scenarios = [ + "missing", + "directory", + ...(permissionDenied ? ["permission"] : []), + ]; + for (const scenario of scenarios) { + await call("open_binary", { path: source, provider_id: "ghidra" }); + const before = await call("binary_session"); + if (scenario === "permission") await chmod(source, 0); + else { + await rm(source); + if (scenario === "directory") await mkdir(source); + } + try { + const failure = await reject("analyze_function", { procedure: entry }); + assert.equal( + failure.code, + scenario === "permission" ? "access_denied" : "artifact_changed", + ); + assert.equal(failure.details.path, before.path); + assert.equal(failure.details.operation, "analyze_function"); + if (scenario === "permission") { + assert.ok(["EACCES", "EPERM"].includes(failure.details.system_code)); + assert.match(failure.remediation.action, /read access/u); + } else assert.match(failure.remediation.action, /open_binary/u); + assert.doesNotMatch(failure.remediation.action, /doctor/u); + const after = await call("binary_session"); + assert.equal(after.analysis_run.run_id, before.analysis_run.run_id); + assert.equal(after.sha256, before.sha256); + assert.equal( + after.capabilities.find( + ({ operation }) => operation === "analyze_function", + )?.available, + true, + ); + assert.deepEqual( + (await readdir(runtimeRoot)).filter((name) => + name.startsWith("rea-ghidra-"), + ), + [], + ); + } finally { + if (scenario === "permission") await chmod(source, 0o600); + await rm(source, { recursive: true, force: true }); + await writeFile(source, bytes); + } + } + return permissionDenied; +} diff --git a/src/domain/analysisErrorPresentation.ts b/src/domain/analysisErrorPresentation.ts index ff1823897..56b0ad273 100644 --- a/src/domain/analysisErrorPresentation.ts +++ b/src/domain/analysisErrorPresentation.ts @@ -75,7 +75,7 @@ export const analysisErrorRemediationAction = ( if (error instanceof AnalysisAccessDeniedError) return "Check the current process's read access to the selected path. Retry with a readable local file."; if (error instanceof AnalysisArtifactChangedError) - return "Wait until the selected file is stable, then retry this operation."; + return "Wait until the selected file is stable. For an active binary session, reopen the target with open_binary before retrying so REA acquires its current identity; for a CLI command or target-free tool, rerun the operation."; if (error instanceof AnalysisInputError) return "Correct the listed arguments and retry."; if (error instanceof UnknownRegistryError && error.reason === "not-found") diff --git a/src/ghidra/GhidraProviderClient.ts b/src/ghidra/GhidraProviderClient.ts index 00a1cfa87..aed08e04b 100644 --- a/src/ghidra/GhidraProviderClient.ts +++ b/src/ghidra/GhidraProviderClient.ts @@ -11,6 +11,8 @@ import type { AnalysisProfileCommitment } from "../domain/analysisProfile.js"; import type { BinaryTarget } from "../domain/binaryTarget.js"; import { AnalysisCancelledError, + AnalysisArtifactChangedError, + AnalysisAccessDeniedError, AnalysisCapabilityUnavailableError, AnalysisInputError, AnalysisResourceConstraintError, @@ -319,6 +321,20 @@ const projectSessionError = ( operation: AnalysisOperation, failure: GhidraSessionError, ): AnalysisError => { + if (failure.cause instanceof AnalysisAccessDeniedError) + return new AnalysisAccessDeniedError( + operation, + failure.cause.path, + failure.cause.systemCode, + { cause: failure }, + ); + if (failure.cause instanceof AnalysisArtifactChangedError) + return new AnalysisArtifactChangedError( + operation, + failure.cause.path, + failure.cause.reason, + { cause: failure }, + ); if ( operation === "annotate_native_function" && failure.kind === "remote" && diff --git a/src/ghidra/GhidraTargetSnapshot.ts b/src/ghidra/GhidraTargetSnapshot.ts index d30618d0f..b6447bc75 100644 --- a/src/ghidra/GhidraTargetSnapshot.ts +++ b/src/ghidra/GhidraTargetSnapshot.ts @@ -1,9 +1,15 @@ import { createHash } from "node:crypto"; -import { constants, createReadStream } from "node:fs"; -import { chmod, copyFile, rm } from "node:fs/promises"; +import { constants } from "node:fs"; +import { open, rm, type FileHandle } from "node:fs/promises"; import { extname, join } from "node:path"; +import { Transform } from "node:stream"; +import { pipeline } from "node:stream/promises"; import type { JsonValue } from "../domain/jsonValue.js"; +import { + AnalysisArtifactChangedError, + AnalysisAccessDeniedError, +} from "../domain/analysisErrorCore.js"; import { windowsPrivateRuntime } from "../windows/WindowsPrivateRuntime.js"; /** Immutable target copy admitted to one ephemeral Ghidra import. */ @@ -37,7 +43,9 @@ export const createGhidraTargetSnapshot = async ( options.signal, ); if (observed.sha256 !== expectedSha256) - throw new Error( + throw new AnalysisArtifactChangedError( + "open_binary", + sourcePath, `Ghidra target snapshot digest mismatch: expected ${expectedSha256}, observed ${observed.sha256}`, ); return { @@ -46,23 +54,71 @@ export const createGhidraTargetSnapshot = async ( admission: { source: observed.source, snapshot: observed.snapshot }, }; } + let source: FileHandle | undefined; + let destination: FileHandle | undefined; try { - await copyFile(sourcePath, snapshotPath, constants.COPYFILE_EXCL); - await chmod(snapshotPath, 0o600); + source = await openSnapshotSource(sourcePath); + if (!(await source.stat()).isFile()) + throw new AnalysisArtifactChangedError( + "open_binary", + sourcePath, + "The selected Ghidra source is no longer a regular file", + ); + destination = await open(snapshotPath, "wx", 0o600); + await destination.chmod(0o600); const hash = createHash("sha256"); - for await (const chunk of createReadStream(snapshotPath)) - hash.update(chunk); + const digest = new Transform({ + transform(chunk: unknown, _encoding, done) { + if (!Buffer.isBuffer(chunk)) { + done(new TypeError("Ghidra snapshot stream must contain bytes")); + return; + } + hash.update(chunk); + done(null, chunk); + }, + }); + await pipeline( + source.createReadStream(), + digest, + destination.createWriteStream(), + options.signal === undefined ? {} : { signal: options.signal }, + ); const observedSha256 = hash.digest("hex"); if (observedSha256 !== expectedSha256) - throw new Error( + throw new AnalysisArtifactChangedError( + "open_binary", + sourcePath, `Ghidra target snapshot digest mismatch: expected ${expectedSha256}, observed ${observedSha256}`, ); return { path: snapshotPath, sha256: observedSha256 }; } catch (cause: unknown) { - await rm(snapshotPath, { force: true }).catch((cause: unknown) => { - // best-effort cleanup: snapshot removal must not mask the copy failure. - void cause; - }); + if (destination !== undefined) + await rm(snapshotPath, { force: true }).catch((cause: unknown) => { + // best-effort cleanup: snapshot removal must not mask the copy failure. + void cause; + }); + throw cause; + } finally { + await Promise.all([source?.close(), destination?.close()]); + } +}; + +const openSnapshotSource = async (path: string): Promise => { + try { + // A path replaced by a FIFO must not block startup before its type is checked. + return await open(path, constants.O_RDONLY | constants.O_NONBLOCK); + } catch (cause: unknown) { + const code = + cause instanceof Error && "code" in cause ? cause.code : undefined; + if (code === "EACCES" || code === "EPERM") + throw new AnalysisAccessDeniedError("open_binary", path, code, { cause }); + if (code === "ENOENT" || code === "ENOTDIR" || code === "ELOOP") + throw new AnalysisArtifactChangedError( + "open_binary", + path, + `The selected Ghidra source could not be opened (${code})`, + { cause }, + ); throw cause; } }; diff --git a/tests/boundary/providers/ghidra/ghidraTargetSnapshot.test.ts b/tests/boundary/providers/ghidra/ghidraTargetSnapshot.test.ts index 30e796d15..13660b944 100644 --- a/tests/boundary/providers/ghidra/ghidraTargetSnapshot.test.ts +++ b/tests/boundary/providers/ghidra/ghidraTargetSnapshot.test.ts @@ -1,20 +1,16 @@ import { createHash } from "node:crypto"; -import { access, readFile, rm, writeFile } from "node:fs/promises"; +import { readFile, rm, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import { createTestTempDirectory } from "../../../fixtures/temporaryDirectory.js"; -import { createGhidraTestRuntime } from "../../../fixtures/ghidraRuntime.js"; -import type { PrivateRuntimeRoot } from "../../../../src/process/PrivateRuntimeRoot.js"; import { createGhidraTargetSnapshot } from "../../../../src/ghidra/GhidraTargetSnapshot.js"; const roots: string[] = []; -const runtimes: PrivateRuntimeRoot[] = []; afterEach(async () => { - await Promise.all(runtimes.splice(0).map((runtime) => runtime.close())); await Promise.all( roots.splice(0).map((root) => rm(root, { recursive: true, force: true })), ); @@ -33,52 +29,12 @@ describe("Ghidra target snapshot", () => { }); expect(snapshot.admission).toBeUndefined(); await expect(readFile(snapshot.path)).resolves.toEqual(bytes); + await expect( + createGhidraTargetSnapshot(source, root, sha256, { platform: "linux" }), + ).rejects.toMatchObject({ code: "EEXIST" }); + await expect(readFile(snapshot.path)).resolves.toEqual(bytes); await expect( createGhidraTargetSnapshot(source, root, sha256, { platform: "win32" }), ).rejects.toThrow(/No native private runtime owns/u); }); - it("copies an exact digest-bound target into the private runtime", async () => { - const root = await createTestTempDirectory("rea-ghidra-snapshot-"); - roots.push(root); - const source = join(root, "source.exe"); - const bytes = Buffer.from("native PE fixture"); - const sha256 = createHash("sha256").update(bytes).digest("hex"); - await writeFile(source, bytes); - const runtime = await createGhidraTestRuntime(root); - runtimes.push(runtime); - - const snapshot = await createGhidraTargetSnapshot( - source, - runtime.path, - sha256, - ); - - expect(snapshot).toMatchObject({ - path: join(runtime.path, `target-${sha256.slice(0, 12)}.exe`), - sha256, - }); - await expect(readFile(snapshot.path)).resolves.toEqual(bytes); - }); - - it("rejects a mismatched digest and removes the failed runtime snapshot", async () => { - const root = await createTestTempDirectory("rea-ghidra-snapshot-"); - roots.push(root); - const source = join(root, "source with unsafe extension.%PATH%"); - await writeFile(source, "changed target"); - const runtime = await createGhidraTestRuntime(root); - runtimes.push(runtime); - const expected = "a".repeat(64); - const snapshotPath = join( - runtime.path, - `target-${expected.slice(0, 12)}.bin`, - ); - - await expect( - createGhidraTargetSnapshot(source, runtime.path, expected), - ).rejects.toThrow(/digest mismatch/u); - await runtime.close(); - await expect(access(snapshotPath)).rejects.toMatchObject({ - code: "ENOENT", - }); - }); });