From 6381aa0b1dcb7092673db5ac739af57ce220ed37 Mon Sep 17 00:00:00 2001 From: gensokyo-dev <342580529+gensokyo-dev@users.noreply.github.com> Date: Sun, 25 Oct 2026 22:37:32 +0800 Subject: [PATCH] fix(ghidra): scope rejected-input diagnostics to their request (#7140) Scope expected Ghidra input failures to the active request after the authenticated startup handshake. Preserve operation output, identity and log coordinates while retaining cumulative output with its session owner; lifecycle and unexpected failures keep full diagnostics. Share remote input-code classification with domain error projection. Verify startup rejection, healthy continuation, cleanup, unknown errors, OOM diagnostics and real CLI/MCP workflows. The measured unknown-symbol reply falls from 13,170 to 2,209 bytes with unchanged successful facts and Evidence IDs. --- docs/ghidra-provider.md | 11 +++ src/ghidra/GhidraClient.ts | 20 ++++- src/ghidra/GhidraDiagnostics.test.ts | 58 +++++++++++++ src/ghidra/GhidraDiagnostics.ts | 34 ++++++-- src/ghidra/GhidraProviderClient.ts | 8 +- src/ghidra/GhidraSessionError.test.ts | 33 +++++++ src/ghidra/GhidraSessionError.ts | 19 +++- .../providers/ghidra/ghidraClient.fixture.ts | 1 + .../providers/ghidra/ghidraClient.test.ts | 87 ++++++++++++++----- tests/fixtures/fakeGhidra.mjs | 41 +++++++-- 10 files changed, 260 insertions(+), 52 deletions(-) diff --git a/docs/ghidra-provider.md b/docs/ghidra-provider.md index 7632ca5d78..4553147977 100644 --- a/docs/ghidra-provider.md +++ b/docs/ghidra-provider.md @@ -30,6 +30,17 @@ signal. Confirm resource causes from process output or host/container records; operation-scoped JVM `OutOfMemoryError` observations retain their separate resource classification. Automatic kernel or cgroup OOM attribution is not implemented. +After the authenticated startup handshake, rejected inputs (`invalid_request`, +`not_found`, `ambiguous`, or an invalid function name) use +`diagnostics.diagnostic_scope: "request"` when their active output checkpoint is +available. They preserve the active request's +`failure_output`, target/profile identity and runtime/log coordinates, while +omitting cumulative launcher stdout/stderr and launch-support output. Startup, +process, protocol and other remote failures retain full session diagnostics. +The Ghidra client still owns its bounded cumulative output; this projection does +not clear it or alter session readiness. Log paths refer to the owned ephemeral +runtime and are removed by normal cleanup. + Provider selection and profile identity follow [MCP contracts](mcp-contracts.md#identity-and-discovery). One deep provider stays bound to the target; runtime failure never selects another diff --git a/src/ghidra/GhidraClient.ts b/src/ghidra/GhidraClient.ts index ebf9e9a09e..5a5a740e21 100644 --- a/src/ghidra/GhidraClient.ts +++ b/src/ghidra/GhidraClient.ts @@ -54,6 +54,7 @@ import { ghidraFailureOutput, GHIDRA_JVM_OOME_MARKER, GHIDRA_PROCESS_DIAGNOSTIC_BYTES, + type GhidraDiagnosticsScope, } from "./GhidraDiagnostics.js"; import { GhidraLaunchError, type GhidraLaunch } from "./GhidraLauncher.js"; import { GhidraResponseRouter } from "./GhidraResponseRouter.js"; @@ -158,7 +159,7 @@ export class GhidraClient { #startupAttempt: GhidraStartupAttempt | undefined; #closePromise: Promise | undefined; readonly #failure = bindGhidraSessionFailure( - () => this.#diagnostics(), + (scope) => this.#diagnostics(scope), (value, incomplete) => this.#redactAuthentication(value, incomplete), ); readonly #wire: GhidraWire; @@ -1339,7 +1340,17 @@ export class GhidraClient { }); } - #diagnostics(): Readonly> { + #diagnostics( + requestedScope: GhidraDiagnosticsScope = "session", + ): Readonly> { + // Startup errors still need launcher evidence. A request projection must + // have both a completed handshake and its own active output checkpoint. + const scope = + requestedScope === "request" && + this.#startupSucceeded() && + this.#activeOutputCheckpoint !== undefined + ? "request" + : "session"; const currentSnapshot = this.#process?.snapshot() ?? this.#processSnapshot; const snapshot: ProviderProcessSnapshot | undefined = this.#outputDrainIncomplete && currentSnapshot !== undefined @@ -1350,7 +1361,9 @@ export class GhidraClient { } : currentSnapshot; const checkpoint = - this.#activeOutputCheckpoint ?? this.#startupOutputCheckpoint; + scope === "request" + ? this.#activeOutputCheckpoint + : (this.#activeOutputCheckpoint ?? this.#startupOutputCheckpoint); const scoped = checkpoint === undefined ? undefined @@ -1360,6 +1373,7 @@ export class GhidraClient { ? undefined : ghidraFailureOutput(scoped, this.#token, this.#outputDrainIncomplete); return createGhidraDiagnostics({ + scope, targetPath: this.#options.targetPath, targetSha256: this.#options.targetSha256, ...(this.#targetAdmission === undefined diff --git a/src/ghidra/GhidraDiagnostics.test.ts b/src/ghidra/GhidraDiagnostics.test.ts index b6ce54e03e..0d5dd5e82e 100644 --- a/src/ghidra/GhidraDiagnostics.test.ts +++ b/src/ghidra/GhidraDiagnostics.test.ts @@ -5,6 +5,64 @@ import { } from "./GhidraDiagnostics.js"; import { expect, it } from "vitest"; +it("keeps request output and provenance without projecting launcher history", () => { + const output = { text: "launcher output", bytes: 15, observedBytes: 15 }; + const options = { + targetPath: "./caller-selected/../target", + targetSha256: "a".repeat(64), + transport: "unix-socket" as const, + providerVersion: "12.1.4", + profileDigest: "b".repeat(64), + runtimeRoot: "/owned/runtime", + snapshot: { stdout: output, stderr: output, exitCode: null, signal: null }, + previous: {}, + failure_output: { stdout: { text: "query output" }, complete: false }, + }; + const request = createGhidraDiagnostics({ ...options, scope: "request" }); + expect(request).toMatchObject({ + diagnostic_scope: "request", + target_path: options.targetPath, + target_sha256: options.targetSha256, + profile_digest: options.profileDigest, + runtime_root: options.runtimeRoot, + failure_output: options.failure_output, + }); + expect(request).not.toHaveProperty("stdout"); + expect(request).not.toHaveProperty("stderr"); + expect(createGhidraDiagnostics(options)).toMatchObject({ + stdout: { text: output.text }, + stderr: { text: output.text }, + failure_output: options.failure_output, + }); +}); + +it("keeps cached session output owned while projecting historical request coordinates", () => { + const previous = { + target_path: "/selected/target", + ghidra_log_path: "/owned/project/ghidra.log", + stdout: { text: "old launcher output" }, + stderr: { text: "old launcher stderr" }, + launch_support: [{ stdout: "JDK version" }], + failure_output: { stdout: { text: "old request" }, complete: true }, + }; + const request = createGhidraDiagnostics({ + scope: "request", + targetPath: "/selected/target", + targetSha256: "a".repeat(64), + transport: "unix-socket", + providerVersion: "12.1.4", + profileDigest: "b".repeat(64), + previous, + }); + expect(request).toEqual({ + diagnostic_scope: "request", + target_path: previous.target_path, + ghidra_log_path: previous.ghidra_log_path, + }); + expect(previous.stdout.text).toBe("old launcher output"); + expect(previous.failure_output.stdout.text).toBe("old request"); +}); + it("does not reuse historical operation-scoped failure output without a current owner", () => { const diagnostics = createGhidraDiagnostics({ targetPath: "/tmp/target", diff --git a/src/ghidra/GhidraDiagnostics.ts b/src/ghidra/GhidraDiagnostics.ts index 6aa465df0e..fde16ab308 100644 --- a/src/ghidra/GhidraDiagnostics.ts +++ b/src/ghidra/GhidraDiagnostics.ts @@ -13,8 +13,12 @@ export const GHIDRA_PROCESS_DIAGNOSTIC_BYTES = 8 * 1024 * 1024; /** JVM error class located in all launcher output, beyond the retention budget. */ export const GHIDRA_JVM_OOME_MARKER = "java.lang.OutOfMemoryError"; +/** A rejected caller input owns request output, not the launcher's history. */ +export type GhidraDiagnosticsScope = "session" | "request"; + /** Inputs needed to project bounded diagnostics for one Ghidra runtime. */ export interface GhidraDiagnosticsOptions { + readonly scope?: GhidraDiagnosticsScope; readonly targetPath: string; readonly targetSha256: string; readonly targetAdmission?: JsonValue; @@ -43,9 +47,16 @@ export const createGhidraDiagnostics = ( ) { const history: Record = { ...options.previous }; delete history.failure_output; + if (options.scope === "request") { + delete history.stdout; + delete history.stderr; + delete history.launch_support; + history.diagnostic_scope = "request"; + } return history; } return { + ...(options.scope === "request" ? { diagnostic_scope: "request" } : {}), target_path: options.targetPath, target_sha256: options.targetSha256, ...(options.targetAdmission === undefined @@ -71,7 +82,8 @@ export const createGhidraDiagnostics = ( ghidra_log_path: options.launch.ghidraLogPath, script_log_path: options.launch.scriptLogPath, process_id: options.launch.process.pid ?? null, - ...(options.launch.launchSupport === undefined + ...(options.scope === "request" || + options.launch.launchSupport === undefined ? {} : { launch_support: options.launch.launchSupport.map((output) => ({ @@ -90,14 +102,18 @@ export const createGhidraDiagnostics = ( stop_requested: options.snapshot.stopRequested === true, diagnostic_truncated: options.snapshot.diagnosticTruncated === true, output_incomplete: options.snapshot.outputIncomplete === true, - stdout: ghidraDiagnosticStream( - options.snapshot.stdout, - options.token, - ), - stderr: ghidraDiagnosticStream( - options.snapshot.stderr, - options.token, - ), + ...(options.scope === "request" + ? {} + : { + stdout: ghidraDiagnosticStream( + options.snapshot.stdout, + options.token, + ), + stderr: ghidraDiagnosticStream( + options.snapshot.stderr, + options.token, + ), + }), }), ...(options.failure_output === undefined ? {} diff --git a/src/ghidra/GhidraProviderClient.ts b/src/ghidra/GhidraProviderClient.ts index 86a874a2e0..5718436937 100644 --- a/src/ghidra/GhidraProviderClient.ts +++ b/src/ghidra/GhidraProviderClient.ts @@ -99,6 +99,7 @@ import { } from "./GhidraProviderCapabilities.js"; import { GHIDRA_STARTUP_DEADLINE_ELAPSED, + isGhidraInputFailureCode, type GhidraSessionError, } from "./GhidraSessionError.js"; import type { GhidraSessionInfo } from "./GhidraSessionValues.js"; @@ -1046,12 +1047,7 @@ const projectPrimarySessionError = ( "Retry this search in literal mode or simplify the regex.", }, ); - if ( - failure.kind === "remote" && - ["invalid_request", "not_found", "ambiguous"].includes( - failure.remoteCode ?? "", - ) - ) + if (failure.kind === "remote" && isGhidraInputFailureCode(failure.remoteCode)) return new AnalysisInputError(operation, context, [ { path: [], reason: "invalid_value", message: failure.message }, ]); diff --git a/src/ghidra/GhidraSessionError.test.ts b/src/ghidra/GhidraSessionError.test.ts index 1044ac5749..fd461f2898 100644 --- a/src/ghidra/GhidraSessionError.test.ts +++ b/src/ghidra/GhidraSessionError.test.ts @@ -7,6 +7,39 @@ import { bindGhidraSessionFailure, } from "./GhidraSessionError.js"; import { GhidraLaunchError } from "./GhidraLauncher.js"; +import { createGhidraDiagnostics } from "./GhidraDiagnostics.js"; + +it.each(["decompile_failed", "future_remote_failure"])( + "retains complete session diagnostics for non-input remote code %s", + (remoteCode) => { + const factory = bindGhidraSessionFailure((scope) => + createGhidraDiagnostics({ + scope, + targetPath: "/selected/target", + targetSha256: "a".repeat(64), + transport: "unix-socket", + providerVersion: "12.1.4", + profileDigest: "b".repeat(64), + runtimeRoot: "/owned/runtime", + snapshot: { + stdout: { text: "engine diagnostic", bytes: 17, observedBytes: 17 }, + stderr: { text: "", bytes: 0, observedBytes: 0 }, + exitCode: null, + signal: null, + }, + previous: {}, + }), + ); + const failure = factory("remote", "Engine failed", undefined, { + remoteCode, + }); + expect(failure.diagnostics).toMatchObject({ + stdout: { text: "engine diagnostic" }, + remote_code: remoteCode, + }); + expect(failure.diagnostics).not.toHaveProperty("diagnostic_scope"); + }, +); it("reports truncated cyclic causes in serializable diagnostics", () => { const cause = new Error("cyclic launcher failure"); diff --git a/src/ghidra/GhidraSessionError.ts b/src/ghidra/GhidraSessionError.ts index 7e5b2b3dcb..a53ecf205c 100644 --- a/src/ghidra/GhidraSessionError.ts +++ b/src/ghidra/GhidraSessionError.ts @@ -6,6 +6,7 @@ import { import type { JsonValue } from "../domain/jsonValue.js"; import { DarwinProcessOwnershipInspectionError } from "../process/DarwinProcessRunTokenReader.js"; import { boundedUtf8Text } from "../process/boundedUtf8Text.js"; +import type { GhidraDiagnosticsScope } from "./GhidraDiagnostics.js"; import { GhidraLaunchSupportError } from "./GhidraJavaLaunch.js"; import { GhidraLaunchError, @@ -184,10 +185,16 @@ const createGhidraSessionError = (failure: { }, ); -/** Bind session-failure construction to a live, token-redacted diagnostic view. */ +/** Bridge input failures share classification with their domain error projection. */ +export const isGhidraInputFailureCode = (code: string | undefined): boolean => + code === "invalid_request" || code === "not_found" || code === "ambiguous"; + +/** Bind session-failure construction to its causal diagnostic scope. */ export const bindGhidraSessionFailure = ( - diagnostics: () => Readonly>, + diagnostics: ( + scope: GhidraDiagnosticsScope, + ) => Readonly>, redact: (value: string, incomplete?: boolean) => string = (value) => value, ) => ( @@ -203,7 +210,13 @@ export const bindGhidraSessionFailure = kind, message: redact(message), diagnostics: { - ...diagnostics(), + ...diagnostics( + kind === "remote" && + (isGhidraInputFailureCode(options.remoteCode) || + options.remoteCode === "invalid_function_name") + ? "request" + : "session", + ), ...(cause === undefined ? {} : { failure_cause: describeCause(cause, redact) }), diff --git a/tests/boundary/providers/ghidra/ghidraClient.fixture.ts b/tests/boundary/providers/ghidra/ghidraClient.fixture.ts index 823a58f882..3167be3aa6 100644 --- a/tests/boundary/providers/ghidra/ghidraClient.fixture.ts +++ b/tests/boundary/providers/ghidra/ghidraClient.fixture.ts @@ -26,6 +26,7 @@ type FixtureMode = | "future_id" | "analysis_timeout" | "remote_error" + | "remote_startup_error" | "shutdown_error" | "invalid_shutdown_ack" | "hang_after_start" diff --git a/tests/boundary/providers/ghidra/ghidraClient.test.ts b/tests/boundary/providers/ghidra/ghidraClient.test.ts index 46d002d96d..2242043fb8 100644 --- a/tests/boundary/providers/ghidra/ghidraClient.test.ts +++ b/tests/boundary/providers/ghidra/ghidraClient.test.ts @@ -950,6 +950,71 @@ describe("GhidraClient captured source snapshot", () => { }); }); +it("scopes authenticated input-failure diagnostics to the request while retaining session output", async () => { + const client = clientFor(new FixtureLauncher("remote_error")); + + const result = await client.callTool("list_procedures", { + document: null, + offset: 0, + limit: 500, + }); + + expect(result).toMatchObject({ + ok: false, + error: { + kind: "remote", + remoteCode: "not_found", + diagnostics: { + diagnostic_scope: "request", + remote_code: "not_found", + remote_message: "Unknown Ghidra procedure name", + failure_output: { + stdout: { text: "Fixture request rejected: missing procedure\n" }, + complete: true, + }, + }, + }, + }); + if (result.ok) throw new Error("Expected remote lookup failure"); + expect(result.error.diagnostics).not.toHaveProperty("stdout"); + expect(result.error.diagnostics).not.toHaveProperty("stderr"); + expect(result.error.diagnostics).not.toHaveProperty("launch_support"); + expect(client.diagnostics()).toMatchObject({ + stdout: { + text: "Fixture launcher startup output\nFixture request rejected: missing procedure\n", + }, + }); + expect(client.isReady()).toBe(true); + await expect( + client.callTool("list_strings", { document: null }), + ).resolves.toMatchObject({ ok: true }); + await client.close(); + expect(client.diagnostics()).toMatchObject({ + stdout: { + text: expect.stringContaining("Fixture launcher startup output"), + }, + }); +}); + +it("retains launcher diagnostics when an input error rejects the startup handshake", async () => { + const client = clientFor(new FixtureLauncher("remote_startup_error")); + const result = await client.start(); + expect(result).toMatchObject({ + ok: false, + error: { + kind: "remote", + remoteCode: "invalid_request", + diagnostics: { + stdout: { + text: expect.stringContaining("Fixture launcher startup output"), + }, + }, + }, + }); + if (result.ok) throw new Error("Expected startup failure"); + expect(result.error.diagnostics).not.toHaveProperty("diagnostic_scope"); +}); + describe("GhidraClient", () => { it("completes an exact, fragmented post-analysis handshake", async () => { const launcher = new FixtureLauncher("fragmented"); @@ -1051,28 +1116,6 @@ describe("GhidraClient", () => { }); }); - it("preserves an authenticated operation failure code and diagnostics", async () => { - const client = clientFor(new FixtureLauncher("remote_error")); - - const result = await client.callTool("list_procedures", { - document: null, - offset: 0, - limit: 500, - }); - - expect(result).toMatchObject({ - ok: false, - error: { - kind: "remote", - remoteCode: "not_found", - diagnostics: { - remote_code: "not_found", - remote_message: "Unknown Ghidra procedure name", - }, - }, - }); - }); - it.each([ ["wrong_identity", "protocol"], ["malformed", "protocol"], diff --git a/tests/fixtures/fakeGhidra.mjs b/tests/fixtures/fakeGhidra.mjs index 5af2455a1e..3af185a9fa 100644 --- a/tests/fixtures/fakeGhidra.mjs +++ b/tests/fixtures/fakeGhidra.mjs @@ -62,6 +62,11 @@ const cutIdentifierAtBudget = async () => { const writeThen = (stream, text, then) => stream.write(text, () => setTimeout(then, 10)); +if (mode === "remote_error" || mode === "remote_startup_error") + await new Promise((resolve) => + process.stdout.write("Fixture launcher startup output\n", resolve), + ); + let endpointPublication; if (mode === "exit") process.exit(73); if (mode === "flood_silent") { @@ -199,15 +204,20 @@ const handleRequest = (socket, server, request, state) => { return; } if (state.mode === "remote_error" && request.method === "list_procedures") { - socket.write( - `${JSON.stringify({ - id: request.id, - ok: false, - error: { - code: "not_found", - message: "Unknown Ghidra procedure name", - }, - })}\n`, + writeThen( + process.stdout, + "Fixture request rejected: missing procedure\n", + () => + socket.write( + `${JSON.stringify({ + id: request.id, + ok: false, + error: { + code: "not_found", + message: "Unknown Ghidra procedure name", + }, + })}\n`, + ), ); return; } @@ -266,6 +276,19 @@ const handleRequest = (socket, server, request, state) => { }; const handlePing = (socket, request, state) => { + if (state.mode === "remote_startup_error") { + socket.write( + `${JSON.stringify({ + id: request.id, + ok: false, + error: { + code: "invalid_request", + message: "Fixture rejected startup handshake", + }, + })}\n`, + ); + return; + } state.pings.value += 1; if (state.mode === "diagnostic_output" && state.pings.value === 1) { const retainedTokenPrefixBytes = Math.floor(state.token.length / 2);