diff --git a/docs/ghidra-provider.md b/docs/ghidra-provider.md index d162054d91..f1322f4b33 100644 --- a/docs/ghidra-provider.md +++ b/docs/ghidra-provider.md @@ -36,6 +36,18 @@ provider stays bound to the target; runtime failure never selects another provider automatically. The contracts below describe Ghidra observations, not equivalence with another engine's pseudocode or analysis database. +## Request payload ceiling + +Ghidra requests use `REA_BRIDGE_MAX_REQUEST_BYTES`: default 16 MiB, with a +configurable range of 7,464–536,870,887 bytes. REA measures UTF-8 JSON, including +escaping, before allocating the serialized request or sending it. The framing +newline is excluded. An oversized request returns `resource_constraint` with +the effective setting, its source, and the exact payload byte count. + +Set the value in the environment that starts REA, then restart REA to apply it. +This setting is enforced by the REA client; the Java bridge has no corresponding +request ceiling. + ## Admitted inventory semantics | Concern | Ghidra contract | diff --git a/src/config/bridgeMaxRequestBytes.ts b/src/config/bridgeMaxRequestBytes.ts index 12857df6a1..6c8ffc2fd8 100644 --- a/src/config/bridgeMaxRequestBytes.ts +++ b/src/config/bridgeMaxRequestBytes.ts @@ -55,6 +55,13 @@ export const DEFAULT_BRIDGE_MAX_REQUEST: BridgeMaxRequestBytes = { source: "default", }; +/** + * Peer a ceiling refusal is reported for. Ghidra differs from the other two: + * its Java bridge reads no request ceiling, so only the REA client enforces + * this setting for Ghidra and recovery must not ask for a bridge restart. + */ +export type BridgePeer = "Hopper" | "Cutter" | "Ghidra"; + export const bridgeMaxRequestBytesConstraint = `${BRIDGE_MAX_REQUEST_BYTES_SETTING} must be a positive decimal integer from ${String(MINIMUM_BRIDGE_MAX_REQUEST_BYTES)} to ${String(MAXIMUM_BRIDGE_MAX_REQUEST_BYTES)} bytes; the default is ${String(DEFAULT_BRIDGE_MAX_REQUEST_BYTES)} bytes (16 MiB). A value below ${String(MINIMUM_BRIDGE_MAX_REQUEST_BYTES)} bytes cannot carry the largest admitted symbol-name command, and a larger value is refused so the encoded payload plus its framing LF stays one string under Node's single-string ceiling.`; const decimalInteger = /^[1-9]\d*$/u; @@ -114,11 +121,14 @@ export const resolveBridgeMaxRequestBytes = ( /** How to read and change the effective bridge request ceiling. */ export const bridgeMaxRequestBytesRecovery = ( limit: BridgeMaxRequestBytes, + peer: BridgePeer = "Hopper", ): string => [ `The effective ${BRIDGE_MAX_REQUEST_BYTES_SETTING} value is ${String(limit.maximumBytes)} bytes (source ${limit.source}; default ${String(DEFAULT_BRIDGE_MAX_REQUEST_BYTES)}, floor ${String(MINIMUM_BRIDGE_MAX_REQUEST_BYTES)}, maximum ${String(MAXIMUM_BRIDGE_MAX_REQUEST_BYTES)}).`, `Set ${BRIDGE_MAX_REQUEST_BYTES_SETTING} in the environment that starts REA to a decimal integer from ${String(MINIMUM_BRIDGE_MAX_REQUEST_BYTES)} to ${String(MAXIMUM_BRIDGE_MAX_REQUEST_BYTES)}, then restart REA.`, - "Restart Hopper or Cutter from that same environment so the bridge uses this ceiling.", + peer === "Ghidra" + ? `The Ghidra bridge enforces no request ceiling of its own, so only the REA client applies ${BRIDGE_MAX_REQUEST_BYTES_SETTING} to Ghidra requests.` + : "Restart Hopper or Cutter from that same environment so the bridge uses this ceiling.", ].join(" "); const limitFacts = ( @@ -132,13 +142,13 @@ export const bridgeRequestOversizeError = ( operation: string, limit: BridgeMaxRequestBytes, payloadBytes: number, - peer: "Hopper" | "Cutter", + peer: BridgePeer, ): AnalysisResourceConstraintError => { - const recovery = bridgeMaxRequestBytesRecovery(limit); + const recovery = bridgeMaxRequestBytesRecovery(limit, peer); const refusal = peer === "Cutter" ? `Cutter request exceeds its authenticated frame limit of ${String(limit.maximumBytes)} bytes; no command was sent.` - : `Hopper request exceeds its ${String(limit.maximumBytes)}-byte payload limit; no request was sent.`; + : `${peer} request exceeds its ${String(limit.maximumBytes)}-byte payload limit; no request was sent.`; return new AnalysisResourceConstraintError( operation, "transport", @@ -153,10 +163,10 @@ export const bridgeRequestLimitMismatchError = ( operation: string, limit: BridgeMaxRequestBytes, bridgeBytes: number, - peer: "Hopper" | "Cutter", + peer: BridgePeer, sent = peer === "Cutter" ? "No command was sent." : "No request was sent.", ): AnalysisResourceConstraintError => { - const recovery = bridgeMaxRequestBytesRecovery(limit); + const recovery = bridgeMaxRequestBytesRecovery(limit, peer); return new AnalysisResourceConstraintError( operation, "transport", @@ -173,7 +183,7 @@ export const bridgeRequestLimitMismatchError = ( export const bridgeResponseOversizeError = ( limit: BridgeMaxRequestBytes, ): AnalysisResourceConstraintError => { - const recovery = bridgeMaxRequestBytesRecovery(limit); + const recovery = bridgeMaxRequestBytesRecovery(limit, "Cutter"); return new AnalysisResourceConstraintError( "cutter_command", "transport", diff --git a/src/ghidra/GhidraClient.ts b/src/ghidra/GhidraClient.ts index 0aaa344507..7bcbaac147 100644 --- a/src/ghidra/GhidraClient.ts +++ b/src/ghidra/GhidraClient.ts @@ -40,6 +40,7 @@ import { ProviderProcessSupervisor, } from "../process/ProviderProcess.js"; import { DEFAULT_GHIDRA_STARTUP_TIMEOUT_MS } from "../config/providerStartupTimeout.js"; +import { DEFAULT_BRIDGE_MAX_REQUEST } from "../config/bridgeMaxRequestBytes.js"; import type { GhidraClientOptions, GhidraRequestOptions, @@ -199,6 +200,8 @@ export class GhidraClient { pending: this.#pending, logger: this.#logger, failure: this.#failure, + bridgeMaxRequestBytes: + options.bridgeMaxRequestBytes ?? DEFAULT_BRIDGE_MAX_REQUEST, }); this.#requestQueue = new GhidraRequestQueue( async (method, parameters, requestOptions) => { diff --git a/src/ghidra/GhidraClientTypes.ts b/src/ghidra/GhidraClientTypes.ts index aae31eb5d1..2a4c2b9788 100644 --- a/src/ghidra/GhidraClientTypes.ts +++ b/src/ghidra/GhidraClientTypes.ts @@ -1,4 +1,5 @@ import type { Logger } from "pino"; +import type { BridgeMaxRequestBytes } from "../config/bridgeMaxRequestBytes.js"; import type { Result } from "../domain/result.js"; import type { LinuxCgroupMemoryState } from "../process/LinuxCgroupMemory.js"; import type { GhidraLauncher } from "./GhidraLauncher.js"; @@ -49,4 +50,6 @@ export interface GhidraClientOptions { readonly logger?: Logger; /** Observes REA's memory cgroup around a run; defaults to the Linux cgroup v2 reader. */ readonly readMemoryCgroup?: () => Promise; + /** Effective request ceiling; defaults to the configured bridge ceiling. */ + readonly bridgeMaxRequestBytes?: BridgeMaxRequestBytes; } diff --git a/src/ghidra/GhidraClientWire.test.ts b/src/ghidra/GhidraClientWire.test.ts new file mode 100644 index 0000000000..bdd1ad04a9 --- /dev/null +++ b/src/ghidra/GhidraClientWire.test.ts @@ -0,0 +1,141 @@ +import type { Socket } from "node:net"; + +import pino from "pino"; +import { describe, expect, it, vi } from "vitest"; + +import type { BridgeMaxRequestBytes } from "../config/bridgeMaxRequestBytes.js"; +import { AnalysisResourceConstraintError } from "../domain/analysisErrorCore.js"; +import type { JsonValue } from "../domain/jsonValue.js"; +import { ok, type Result } from "../domain/result.js"; +import { PendingOperations } from "../process/PendingOperations.js"; +import { GhidraWire } from "./GhidraClientWire.js"; +import { + bindGhidraSessionFailure, + type GhidraSessionError, +} from "./GhidraSessionError.js"; + +const failure = bindGhidraSessionFailure(() => ({ + target_path: "/tmp/wire-fixture", +})); + +const limit = (maximumBytes: number): BridgeMaxRequestBytes => ({ + maximumBytes, + source: "setting", +}); + +type WireHarness = { + readonly writes: string[]; + readonly pending: PendingOperations< + number, + Result + >; + readonly wire: GhidraWire; +}; + +const harness = (ceiling: BridgeMaxRequestBytes): WireHarness => { + const writes: string[] = []; + const socket = { + destroyed: false, + write: (payload: string, callback?: (cause?: Error | null) => void) => { + writes.push(payload); + callback?.(null); + return true; + }, + } as unknown as Socket; + const pending = new PendingOperations< + number, + Result + >(); + return { + writes, + pending, + wire: new GhidraWire({ + getSocket: () => socket, + getToken: () => "b".repeat(64), + nextId: () => 1, + pending, + logger: pino({ level: "silent" }), + failure, + bridgeMaxRequestBytes: ceiling, + }), + }; +}; + +describe("GhidraWire request ceiling", () => { + it("refuses escaped oversized input before encoding, pending registration or write", async () => { + const { writes, pending, wire } = harness(limit(8192)); + const params = { pattern: "\u0000".repeat(1500) }; + const payloadBytes = Buffer.byteLength( + JSON.stringify({ + id: 1, + token: "b".repeat(64), + method: "search_strings", + params, + }), + ); + expect(Buffer.byteLength(params.pattern)).toBeLessThan(8192); + expect(payloadBytes).toBeGreaterThan(8192); + const stringify = vi.spyOn(JSON, "stringify"); + try { + const refused = await wire.request("search_strings", params, {}); + if (refused.ok) throw new Error("Expected a request ceiling refusal"); + expect(refused.error.message).toContain("REA_BRIDGE_MAX_REQUEST_BYTES"); + expect(refused.error.message).toContain("8192"); + expect(refused.error.message).toContain("Ghidra request exceeds"); + const constraint = refused.error.cause; + if (!(constraint instanceof AnalysisResourceConstraintError)) + throw new Error("Expected the typed transport constraint"); + expect(constraint.reportedLimits).toMatchObject({ + max_request_bytes: 8192, + payload_bytes: payloadBytes, + }); + expect(constraint.retryable).toBe(false); + expect(writes.length).toBe(0); + expect(pending.has(1)).toBe(false); + const encodings = stringify.mock.calls.filter( + ([value]: readonly unknown[]) => + typeof value === "object" && + value !== null && + "params" in value && + value.params === params, + ).length; + expect(encodings).toBe(0); + } finally { + stringify.mockRestore(); + } + }); + + it.each([true, false])( + "enforces the exact UTF-8 JSON payload boundary (admitted: %s)", + async (admitted) => { + const params = { pattern: '前įž€"\\\u0000😀\ud800'.repeat(1024) }; + const encoded = JSON.stringify({ + id: 1, + token: "b".repeat(64), + method: "search_strings", + params, + }); + const payloadBytes = Buffer.byteLength(encoded); + const { writes, pending, wire } = harness( + limit(payloadBytes - (admitted ? 0 : 1)), + ); + const dispatched = wire.request("search_strings", params, {}); + pending.settle(1, ok("ok")); + const result = await dispatched; + if (admitted) { + if (!result.ok) throw result.error; + expect(result.value).toBe("ok"); + expect(writes.length).toBe(1); + expect(Buffer.byteLength(writes[0] ?? "")).toBe(payloadBytes + 1); + expect(writes[0] === encoded + "\n").toBe(true); + } else { + if (result.ok) throw new Error("Expected a request ceiling refusal"); + expect(result.error.cause).toBeInstanceOf( + AnalysisResourceConstraintError, + ); + expect(writes.length).toBe(0); + } + expect(pending.has(1)).toBe(false); + }, + ); +}); diff --git a/src/ghidra/GhidraClientWire.ts b/src/ghidra/GhidraClientWire.ts index 86db6f14b4..9cdee89501 100644 --- a/src/ghidra/GhidraClientWire.ts +++ b/src/ghidra/GhidraClientWire.ts @@ -1,6 +1,11 @@ import type { Socket } from "node:net"; +import { + bridgeRequestOversizeError, + type BridgeMaxRequestBytes, +} from "../config/bridgeMaxRequestBytes.js"; import type { JsonValue } from "../domain/jsonValue.js"; +import { jsonValueByteLength } from "../domain/jsonValueByteLength.js"; import { err, type Result } from "../domain/result.js"; import type { Logger } from "pino"; import type { PendingOperations } from "../process/PendingOperations.js"; @@ -21,6 +26,8 @@ export interface GhidraWireOptions { >; readonly logger: Logger; readonly failure: GhidraSessionFailureFactory; + /** Effective Ghidra request ceiling; the bridge itself enforces none. */ + readonly bridgeMaxRequestBytes: BridgeMaxRequestBytes; } /** Owns the socket write and pending-reply lifecycle for one bridge session. */ @@ -56,6 +63,22 @@ export class GhidraWire { ), ); const timeoutMs = options.timeoutMs; + const payloadValue = { id, token, method, params }; + const payloadBytes = jsonValueByteLength(payloadValue); + // Refuse before registering the operation and before any socket write, so + // an oversized request never reaches the bridge or occupies a pending key. + if (payloadBytes > this.#options.bridgeMaxRequestBytes.maximumBytes) { + const oversize = bridgeRequestOversizeError( + method, + this.#options.bridgeMaxRequestBytes, + payloadBytes, + "Ghidra", + ); + return err( + this.#options.failure("admission", oversize.message, oversize), + ); + } + const payload = JSON.stringify(payloadValue); const response = this.#options.pending.wait(id, { ...(timeoutMs === undefined ? {} @@ -74,22 +97,19 @@ export class GhidraWire { ...(options.signal === undefined ? {} : { signal: options.signal }), cancelledValue: () => err(this.#cancelled(options.signal)), }); - socket.write( - `${JSON.stringify({ id, token, method, params })}\n`, - (cause) => { - if (cause !== undefined && cause !== null) - this.#options.pending.settle( - id, - err( - this.#options.failure( - "process", - "Ghidra bridge write failed", - cause, - ), + socket.write(`${payload}\n`, (cause) => { + if (cause !== undefined && cause !== null) + this.#options.pending.settle( + id, + err( + this.#options.failure( + "process", + "Ghidra bridge write failed", + cause, ), - ); - }, - ); + ), + ); + }); const result = await response; this.#options.logger[result.ok ? "debug" : "warn"]( { method, status: result.ok ? "ok" : "error" }, diff --git a/src/ghidra/GhidraProviderClient.ts b/src/ghidra/GhidraProviderClient.ts index f0cdbbf392..3bd543683d 100644 --- a/src/ghidra/GhidraProviderClient.ts +++ b/src/ghidra/GhidraProviderClient.ts @@ -39,6 +39,7 @@ import { AnalysisInputError, AnalysisResourceConstraintError, AnalysisTimeoutError, + resourceConstraintRecovery, } from "../domain/analysisErrorCore.js"; import type { AnalysisError } from "../domain/analysisErrorBase.js"; import type { EvidenceLocation } from "../domain/evidence.js"; @@ -349,6 +350,7 @@ export const createGhidraProviderClient = (input: { ); const client = clientFactory({ startupTimeoutMs: config.ghidraStartupTimeoutMs, + bridgeMaxRequestBytes: config.bridgeMaxRequestBytes, platform: installation.platform, launcher: new GhidraHeadlessLauncher({ environment: input.environment, @@ -897,6 +899,17 @@ const projectPrimarySessionError = ( const { cause: physicalCause, origin } = artifactFilePrimaryFacts( failure.cause, ); + // A client-side ceiling refusal travels as the cause; re-attribute it to this + // operation so callers see a typed resource constraint rather than a generic + // session failure, matching how Hopper and Cutter report the same ceiling. + if (physicalCause instanceof AnalysisResourceConstraintError) + return new AnalysisResourceConstraintError( + operation, + physicalCause.resource, + physicalCause.reason, + physicalCause.reportedLimits, + { ...context, ...resourceConstraintRecovery(physicalCause) }, + ); if ( origin === "owned-or-classified" && physicalCause instanceof AnalysisAccessDeniedError diff --git a/src/ghidra/GhidraSessionError.ts b/src/ghidra/GhidraSessionError.ts index aac58d6f56..7e5b2b3dcb 100644 --- a/src/ghidra/GhidraSessionError.ts +++ b/src/ghidra/GhidraSessionError.ts @@ -20,6 +20,7 @@ const MAX_COMMAND_OUTPUT_BYTES = 8 * 1024; /** Stable failure phase for one private Ghidra headless session. */ export type GhidraSessionFailureKind = + | "admission" | "cancelled" | "timeout" | "start" diff --git a/tests/boundary/mcp/ghidraRequestCeilingMcp.test.ts b/tests/boundary/mcp/ghidraRequestCeilingMcp.test.ts new file mode 100644 index 0000000000..eef1059887 --- /dev/null +++ b/tests/boundary/mcp/ghidraRequestCeilingMcp.test.ts @@ -0,0 +1,111 @@ +import { access } from "node:fs/promises"; + +import { expect, it, vi } from "vitest"; + +import { AnalysisProviderRegistry } from "../../../src/application/binary/AnalysisProviderRegistry.js"; +import { composeBinarySession } from "../../../src/application/binary/BinarySessionComposition.js"; +import { parseConfig } from "../../../src/config/parseConfig.js"; +import { GhidraClient } from "../../../src/ghidra/GhidraClient.js"; +import { ghidraProfileInstallationHost } from "../../../src/ghidra/GhidraInstallation.fixture.js"; +import { GhidraProvider } from "../../../src/ghidra/GhidraProvider.js"; +import type { GhidraProviderClientFactory } from "../../../src/ghidra/GhidraProviderClient.js"; +import { silentLogger } from "../../../src/logger.js"; +import { createApplicationMcpHarness } from "../../fixtures/applicationMcpHarness.js"; +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; +import { GhidraFixtureLauncher } from "../providers/ghidra/ghidraClient.fixture.js"; + +it("reports local Ghidra admission through SDK MCP without dispatching or losing the warm session", async () => { + const launcher = new GhidraFixtureLauncher(); + let raw: GhidraClient | undefined; + const factory: GhidraProviderClientFactory = (options) => { + raw = new GhidraClient({ + ...options, + platform: process.platform, + transport: "authenticated-loopback-tcp", + launcher, + }); + return raw; + }; + const config = parseConfig({ + GHIDRA_INSTALL_DIR: "/fixture/ghidra", + REA_GHIDRA_STARTUP_TIMEOUT_MS: "10000", + REA_BRIDGE_MAX_REQUEST_BYTES: "8192", + }); + if (!config.ok) throw config.error; + const provider = new GhidraProvider( + config.value, + silentLogger, + {}, + ghidraProfileInstallationHost, + factory, + ); + const session = composeBinarySession( + new AnalysisProviderRegistry([provider]), + ); + const harness = await createApplicationMcpHarness({ + session, + serverOptions: { logger: silentLogger }, + clientIdentity: { name: "ghidra-request-ceiling", version: "1" }, + }); + try { + const opened = await harness.client.callTool({ + name: "open_binary", + arguments: { path: process.execPath, provider_id: "ghidra" }, + }); + expect(opened.isError).not.toBe(true); + const first = await harness.client.callTool({ + name: "list_strings", + arguments: {}, + }); + expect(first.isError).not.toBe(true); + const refused = await harness.client.callTool({ + name: "search_strings", + arguments: { pattern: "\u0000".repeat(1500) }, + }); + expect(refused.isError).toBe(true); + const projected = parseMcpToolError(refused); + expect(projected).toMatchObject({ + error: { + code: "resource_constraint", + details: { + operation: "search_strings", + resource: "transport", + reported_limits: { max_request_bytes: 8192 }, + diagnostics: { failure_kind: "admission" }, + }, + remediation: { + action: expect.stringContaining("REA_BRIDGE_MAX_REQUEST_BYTES"), + }, + }, + }); + expect(projected.error.remediation?.action).toContain( + "only the REA client applies", + ); + expect( + projected.error.remediation?.action.includes("Restart Hopper or Cutter"), + ).toBe(false); + expect(raw?.isReady()).toBe(true); + const after = await harness.client.callTool({ + name: "list_strings", + arguments: {}, + }); + expect(after.isError).not.toBe(true); + await vi.waitFor(() => + expect( + launcher.requests.filter((method) => method === "list_strings"), + ).toHaveLength(2), + ); + expect( + launcher.requests.filter((method) => method === "search_strings"), + ).toHaveLength(0); + expect(launcher.processes).toHaveLength(1); + for (const token of launcher.tokens) + expect(JSON.stringify(refused).includes(token)).toBe(false); + } finally { + await harness.close(); + for (const root of launcher.runtimeRoots) + await expect(access(root)).rejects.toMatchObject({ code: "ENOENT" }); + for (const child of launcher.processes) + expect(child.exitCode !== null || child.signalCode !== null).toBe(true); + } +}, 30_000);