diff --git a/docs/design/rea-limit-inventory.md b/docs/design/rea-limit-inventory.md index 3bdfa8d3a2..45f9290b0d 100644 --- a/docs/design/rea-limit-inventory.md +++ b/docs/design/rea-limit-inventory.md @@ -99,6 +99,7 @@ reports the current value, source, and range. A rejected Windows ChangeTime laun | Environment | Owner | Default | Floor | Maximum | Applies | Fields | | ------------------------------------------ | ------------------------------------------- | ----------------------: | ------: | -----------: | ------------------------------------------------------------------------------------------------------------ | ---------------------------------------------------------------- | | `REA_BRIDGE_MAX_REQUEST_BYTES` | `src/config/bridgeMaxRequestBytes.ts` | 16777216 | 7464 | 536870887 | each bridge request, from the accepted operation snapshot | `update_mode: "runtime"`, `pending_restart: false` | +| `REA_BRIDGE_MAX_REPLY_BYTES` | `src/config/bridgeMaxReplyBytes.ts` | 268435456 | 1048576 | 536870888 | each Hopper or Ghidra request, from the accepted operation snapshot | `update_mode: "runtime"`, `pending_restart: false` | | `REA_CAPTURE_PACKET_JSON_EXPANSION_BYTES` | `src/config/capturePacketJsonExpansion.ts` | 137438953472 | 1048576 | 137438953472 | each packet-capture reply before parsing, from the accepted operation snapshot | `update_mode: "runtime"`, `pending_restart: false` | | `REA_DEBUGGER_COMMAND_DEADLINE_MS` | `src/domain/analysisResourceLimits.ts` | 120000 | 1 | 2147483647 | each command, from the accepted operation snapshot | `update_mode: "runtime"`, `pending_restart: false` | | `REA_DEBUGGER_MAX_COMMAND_OUTPUT_BYTES` | `src/domain/analysisResourceLimits.ts` | 16777216 | 1 | 89303718 | each command, from the accepted operation snapshot | `update_mode: "runtime"`, `pending_restart: false` | diff --git a/docs/reverse-engineering-tools.md b/docs/reverse-engineering-tools.md index 8a4306c17a..6cdf62a0c7 100644 --- a/docs/reverse-engineering-tools.md +++ b/docs/reverse-engineering-tools.md @@ -314,7 +314,12 @@ never sends its authenticated frame does not hold the bridge accept loop. At most 8 connection threads run. A further accept is closed, and a failure to start a thread closes that socket without stopping the listener. A status `completed_connection` is the longest prefix of accepted connections that have terminated. It does not treat an earlier connection as finished because a later status reached the command lock first. Scalar values in one Cutter reply are encoded in bounded batches. A command reply that does not fit keeps its completion state and document generation, and reports an oversized path as unknown. Cutter response frames use this same ceiling and -also exclude the terminating LF. A command response the client discards because it exceeds that ceiling stays an unknown execution and sets `output_truncated`, so Evidence records the omitted output. Hopper response reads are unchanged. +also exclude the terminating LF. A command response the client discards because it exceeds that ceiling stays an unknown execution and sets `output_truncated`, so Evidence records the omitted output. Hopper and Ghidra reply lines are bounded separately by +`REA_BRIDGE_MAX_REPLY_BYTES` (`bridge.max_reply_bytes`): default 268435456 bytes +(256 MiB), from 1048576 to 536870888. The client admits the value with each +request. A reply line, or an unterminated tail, above it is refused as a typed +`transport` resource constraint naming the setting; REA discards the bytes and +closes that bridge connection rather than retaining them. `cutter_command` reports `output_truncated` and `document_generation` as `null` when the plugin did not declare them, for example when no reply was read, the reply was malformed, or an older plugin omitted them. A null generation means diff --git a/scripts/verify/architecture-guard-baselines.json b/scripts/verify/architecture-guard-baselines.json index 764f3aa482..7d74c35c39 100644 --- a/scripts/verify/architecture-guard-baselines.json +++ b/scripts/verify/architecture-guard-baselines.json @@ -373,6 +373,18 @@ "src/server/createServer.ts": 1, "src/windows/WindowsNativeLoader.ts": 1 }, + "hand-caught-configuration-error": { + "src/application/binary/BinarySession.ts": 3, + "src/application/McpClientRegistration.ts": 1, + "src/application/McpRegistrationCatalog.ts": 1, + "src/application/McpRegistrationGit.ts": 1, + "src/application/SetupClientConfiguration.ts": 5, + "src/application/SupportedClients.ts": 1, + "src/application/Uninstall.ts": 2, + "src/application/WebScriptExportService.ts": 1, + "src/ghidra/GhidraProviderClient.ts": 1, + "src/ida/IdaConfiguration.ts": 1 + }, "abort-without-reason": {}, "completed-abort-reclassification": { "src/application/EvidenceInputResolver.ts": 1, diff --git a/scripts/verify/architecture-guards.mjs b/scripts/verify/architecture-guards.mjs index ca3f3c878c..cf4c546a58 100644 --- a/scripts/verify/architecture-guards.mjs +++ b/scripts/verify/architecture-guards.mjs @@ -377,6 +377,17 @@ checkCounted( ["src/gdb/GdbMiRecord.ts"], ); +// A runtime setting view throws the ConfigurationError of a malformed launch +// seed; readRuntimeSetting in src/config/runtimeAnalysisSettings.ts returns it +// as a Result. Catching it by hand mixes it into each site's own failures. +checkCounted( + "hand-caught ConfigurationError (read runtime settings with readRuntimeSetting from src/config/runtimeAnalysisSettings)", + "hand-caught-configuration-error", + files, + /\bcause instanceof ConfigurationError\b/gu, + ["src/config/runtimeAnalysisSettings.ts"], +); + // Byte-format parsers read untrusted input. Bounds, truncation and failure // kind belong to BinaryReader (src/native/metadata/BinaryContainer.ts) and // the ParseFailure algebra (src/domain/parseFailure.ts), not to each parser. diff --git a/src/application/AnalysisSettings.test.ts b/src/application/AnalysisSettings.test.ts index 03eb5ef1f7..33d03bd43c 100644 --- a/src/application/AnalysisSettings.test.ts +++ b/src/application/AnalysisSettings.test.ts @@ -181,7 +181,11 @@ it("exposes shared provider controls and preserves the explicit unbounded decomp store .inspect({ provider: "ghidra", operation: "search_strings" }) .settings.map(({ setting }) => setting), - ).toEqual(["ghidra.startup_timeout_ms", "bridge.max_request_bytes"]); + ).toEqual([ + "ghidra.startup_timeout_ms", + "bridge.max_request_bytes", + "bridge.max_reply_bytes", + ]); for (const operation of ["list_cutter_sessions", "cutter_command"] as const) expect( store diff --git a/src/application/SetupRegistrationEnvironment.ts b/src/application/SetupRegistrationEnvironment.ts index ed213d38b7..1974775a34 100644 --- a/src/application/SetupRegistrationEnvironment.ts +++ b/src/application/SetupRegistrationEnvironment.ts @@ -60,6 +60,7 @@ const SELECTED_REGISTRATION_SETTINGS = [ "REA_GHIDRA_VALUE_FLOW_MAX_EDGES", "REA_GHIDRA_DECOMPILE_MAX_PAYLOAD_MIB", "REA_BRIDGE_MAX_REQUEST_BYTES", + "REA_BRIDGE_MAX_REPLY_BYTES", "REA_CAPTURE_DECODER_OLD_GENERATION_MIB", "REA_CAPTURE_PACKET_JSON_EXPANSION_BYTES", "REA_GHIDRA_DECOMPILE_TIMEOUT_SECONDS", diff --git a/src/config/analysisSettings.ts b/src/config/analysisSettings.ts index 2e0fff5887..b3bd9cfbd6 100644 --- a/src/config/analysisSettings.ts +++ b/src/config/analysisSettings.ts @@ -22,6 +22,7 @@ import { captureDecoderOldGenerationMiBSchema } from "./captureDecoderHeap.js"; import { capturePacketJsonExpansionBytesSchema } from "./capturePacketJsonExpansion.js"; import { parseNativeMatchingAllowance } from "./nativeMatchingAllowance.js"; import { bridgeMaxRequestBytesSchema } from "./bridgeMaxRequestBytes.js"; +import { bridgeMaxReplyBytesSchema } from "./bridgeMaxReplyBytes.js"; import { debuggerCommandDeadlineMsSchema, debuggerMaxCommandOutputBytesSchema, @@ -51,6 +52,7 @@ const environmentSchemas: Readonly>> = REA_CAPTURE_PACKET_JSON_EXPANSION_BYTES: capturePacketJsonExpansionBytesSchema, REA_BRIDGE_MAX_REQUEST_BYTES: bridgeMaxRequestBytesSchema, + REA_BRIDGE_MAX_REPLY_BYTES: bridgeMaxReplyBytesSchema, REA_DEBUGGER_COMMAND_DEADLINE_MS: debuggerCommandDeadlineMsSchema, REA_DEBUGGER_MAX_COMMAND_OUTPUT_BYTES: debuggerMaxCommandOutputBytesSchema, REA_HOPPER_CALLER_SCAN_BUDGET_MS: providerStartupTimeoutSchema, diff --git a/src/config/bridgeMaxReplyBytes.test.ts b/src/config/bridgeMaxReplyBytes.test.ts new file mode 100644 index 0000000000..eac610089d --- /dev/null +++ b/src/config/bridgeMaxReplyBytes.test.ts @@ -0,0 +1,57 @@ +import { expect, it } from "vitest"; + +import { analysisSettingDefinition } from "../domain/analysisSettingDefinitions.js"; +import { runWithAnalysisOperationSnapshot } from "../domain/analysisOperationSnapshot.js"; +import { GHIDRA_OPERATIONS } from "../ghidra/GhidraProviderMetadata.js"; +import { HOPPER_OPERATIONS } from "../hopper/HopperProviderMetadata.js"; +import { + BRIDGE_MAX_REPLY_BYTES_SETTING, + DEFAULT_BRIDGE_MAX_REPLY, +} from "./bridgeMaxReplyBytes.js"; +import { ConfigurationError } from "../domain/configurationErrors.js"; +import { parseConfig } from "./parseConfig.js"; +import { runtimeAnalysisConfig } from "./runtimeAnalysisSettings.js"; + +it("advertises the reply ceiling for every Hopper and Ghidra operation", () => { + const advertised = new Set( + analysisSettingDefinition("bridge.max_reply_bytes").operations, + ); + expect( + [...HOPPER_OPERATIONS, ...GHIDRA_OPERATIONS].filter( + (operation) => !advertised.has(operation), + ), + ).toEqual([]); +}); + +it("resolves the default, a launch seed, and a runtime override", () => { + const unset = parseConfig({}); + if (!unset.ok) throw unset.error; + expect(unset.value.bridgeMaxReplyBytes).toEqual(DEFAULT_BRIDGE_MAX_REPLY); + + const seeded = parseConfig({ [BRIDGE_MAX_REPLY_BYTES_SETTING]: "2097152" }); + if (!seeded.ok) throw seeded.error; + expect(seeded.value.bridgeMaxReplyBytes).toEqual({ + maximumBytes: 2_097_152, + source: "setting", + }); + + const runtime = runtimeAnalysisConfig(seeded.value); + runWithAnalysisOperationSnapshot( + { revision: 1, overrides: { "bridge.max_reply_bytes": 4_194_304 } }, + () => { + expect({ ...runtime.bridgeMaxReplyBytes }).toEqual({ + maximumBytes: 4_194_304, + source: "runtime", + }); + }, + ); +}); + +it("rejects a launch seed below the floor when the ceiling is read", () => { + const parsed = parseConfig({ [BRIDGE_MAX_REPLY_BYTES_SETTING]: "1024" }); + if (!parsed.ok) throw parsed.error; + const runtime = runtimeAnalysisConfig(parsed.value); + expect(() => runtime.bridgeMaxReplyBytes.maximumBytes).toThrow( + ConfigurationError, + ); +}); diff --git a/src/config/bridgeMaxReplyBytes.ts b/src/config/bridgeMaxReplyBytes.ts new file mode 100644 index 0000000000..7b83889955 --- /dev/null +++ b/src/config/bridgeMaxReplyBytes.ts @@ -0,0 +1,92 @@ +import { positiveDecimalIntegerSchema } from "./positiveDecimalInteger.js"; + +import { analysisSettingLimit } from "../domain/analysisSettingDefinitions.js"; +import { AnalysisResourceConstraintError } from "../domain/analysisErrorCore.js"; +import type { ConfigurationError } from "../domain/configurationErrors.js"; +import type { Result } from "../domain/result.js"; +import { readRuntimeSetting } from "./runtimeAnalysisSettings.js"; + +import { + DEFAULT_BRIDGE_MAX_REPLY_BYTES, + MAXIMUM_BRIDGE_MAX_REPLY_BYTES, + MINIMUM_BRIDGE_MAX_REPLY_BYTES, +} from "../domain/analysisResourceLimits.js"; + +export const BRIDGE_MAX_REPLY_BYTES_SETTING = "REA_BRIDGE_MAX_REPLY_BYTES"; + +/** + * Largest Hopper or Ghidra reply line REA reads, in UTF-8 bytes before the LF. + * + * Measured 2026-11-14 over the fake-bridge client tests (ghidraClient, + * ghidraTransport, ghidraInventoryMcp, ghidraReopen, hopperClient.progress, + * hopperRegexFailure, hopperBridgeRequestLimit): 287 reply lines. Apart from + * two deliberate stress replies, the largest was a 1466-byte handshake. The + * stress replies are a 16 MiB Hopper echo and the largest inventory reply, a + * 67109019-byte Ghidra `list_strings` page holding one 64 MiB string. The + * 256 MiB default keeps four times that headroom while bounding what a + * misbehaving bridge can make REA retain. + */ +export interface BridgeMaxReplyBytes { + readonly maximumBytes: number; + readonly source: "default" | "setting" | "runtime"; +} + +export const DEFAULT_BRIDGE_MAX_REPLY: BridgeMaxReplyBytes = { + maximumBytes: DEFAULT_BRIDGE_MAX_REPLY_BYTES, + source: "default", +}; + +/** + * Copy the effective ceiling when a request is admitted. Socket reads run + * outside the request's async context, where a runtime override is not + * visible, so each request carries the value it admitted and the reader + * enforces it only while that request is the one on the wire. A runtime view + * refuses a malformed launch seed once no override covers it (for example + * after the override is reset), so admission returns that refusal. + */ +export const admitBridgeReplyLimit = ( + limit: BridgeMaxReplyBytes, +): Result => + readRuntimeSetting(() => ({ + maximumBytes: limit.maximumBytes, + source: limit.source, + })); + +/** Bridges whose replies the shared JSON-line reader bounds. */ +export type BridgeReplyPeer = "Hopper" | "Ghidra"; + +const bridgeMaxReplyBytesConstraint = `${BRIDGE_MAX_REPLY_BYTES_SETTING} must be a positive decimal integer from ${String(MINIMUM_BRIDGE_MAX_REPLY_BYTES)} to ${String(MAXIMUM_BRIDGE_MAX_REPLY_BYTES)} bytes; the default is ${String(DEFAULT_BRIDGE_MAX_REPLY_BYTES)} bytes (256 MiB). A value below ${String(MINIMUM_BRIDGE_MAX_REPLY_BYTES)} bytes cannot carry a bridge handshake or a full inventory page, and a larger value cannot be decoded as one string.`; + +/** Optional decimal byte count for the Hopper and Ghidra reply line ceiling. */ +export const bridgeMaxReplyBytesSchema = positiveDecimalIntegerSchema( + MINIMUM_BRIDGE_MAX_REPLY_BYTES, + MAXIMUM_BRIDGE_MAX_REPLY_BYTES, + bridgeMaxReplyBytesConstraint, +); + +const bridgeMaxReplyBytesRecovery = (limit: BridgeMaxReplyBytes): string => + `The effective bridge.max_reply_bytes value is ${String(limit.maximumBytes)} bytes (source ${limit.source}; default ${String(DEFAULT_BRIDGE_MAX_REPLY_BYTES)}, floor ${String(MINIMUM_BRIDGE_MAX_REPLY_BYTES)}, maximum ${String(MAXIMUM_BRIDGE_MAX_REPLY_BYTES)}). Request a smaller page or narrower selection, or raise it with update_analysis_settings; the launch seed is ${BRIDGE_MAX_REPLY_BYTES_SETTING}. REA closed that bridge connection instead of retaining the reply, so the provider session must start again before its next request.`; + +/** Refuse one reply line that exceeds the effective ceiling; nothing past it was kept. */ +export const bridgeReplyOversizeError = ( + peer: BridgeReplyPeer, + limit: BridgeMaxReplyBytes, + observedBytes: number, +): AnalysisResourceConstraintError => { + const recovery = bridgeMaxReplyBytesRecovery(limit); + return new AnalysisResourceConstraintError( + `${peer.toLowerCase()}_bridge_reply`, + "transport", + `${peer} bridge reply exceeded the ${String(limit.maximumBytes)}-byte reply limit: one reply line reached ${String(observedBytes)} bytes, and REA stopped reading it. ${recovery}`, + { max_reply_bytes: limit.maximumBytes, observed_bytes: observedBytes }, + { + remediationAction: recovery, + retryable: false, + limit: analysisSettingLimit( + "bridge.max_reply_bytes", + limit.maximumBytes, + limit.source, + ), + }, + ); +}; diff --git a/src/config/bridgeMaxRequestBytes.ts b/src/config/bridgeMaxRequestBytes.ts index c3a47dcadc..ad7d35120e 100644 --- a/src/config/bridgeMaxRequestBytes.ts +++ b/src/config/bridgeMaxRequestBytes.ts @@ -4,6 +4,7 @@ import { analysisSettingLimit } from "../domain/analysisSettingDefinitions.js"; import { AnalysisResourceConstraintError } from "../domain/analysisErrorCore.js"; import { ConfigurationError } from "../domain/configurationErrors.js"; import { err, ok, type Result } from "../domain/result.js"; +import { readRuntimeSetting } from "./runtimeAnalysisSettings.js"; import { DEFAULT_BRIDGE_MAX_REQUEST_BYTES, @@ -88,6 +89,19 @@ export const parseBridgeMaxRequestBytes = ( ); }; +/** + * Copy the effective request ceiling when a request is admitted, in the + * caller's async context, so a queued request is written under the ceiling it + * was admitted with. A malformed launch seed no override covers is refused. + */ +export const admitBridgeRequestLimit = ( + limit: BridgeMaxRequestBytes, +): Result => + readRuntimeSetting(() => ({ + maximumBytes: limit.maximumBytes, + source: limit.source, + })); + /** How to read and change the effective bridge request ceiling. */ export const bridgeRequestLimit = (limit: BridgeMaxRequestBytes) => analysisSettingLimit( diff --git a/src/config/kunaProjectDeadline.test.ts b/src/config/kunaProjectDeadline.test.ts index 912eee604b..2d5d80b08d 100644 --- a/src/config/kunaProjectDeadline.test.ts +++ b/src/config/kunaProjectDeadline.test.ts @@ -4,7 +4,6 @@ import { kunaProjectDeadline, readKunaConfiguration, } from "../kuna/KunaConfiguration.js"; -import { acceptedExportDeadline } from "../kuna/KunaProjectExport.js"; it("keeps the deadline a failed export accepted after the override is reset", () => { const environment = { @@ -38,6 +37,5 @@ it("keeps the deadline a failed export accepted after the override is reset", () expect(() => kunaProjectDeadline(configuration)).toThrow( /no runtime override is active/, ); - expect(acceptedExportDeadline(configuration, accepted)).toEqual(accepted); expect(accepted).toMatchObject({ ms: 5000, source: "runtime" }); }); diff --git a/src/config/parseConfig.ts b/src/config/parseConfig.ts index 3649358202..5e7b148ca9 100644 --- a/src/config/parseConfig.ts +++ b/src/config/parseConfig.ts @@ -19,6 +19,7 @@ import { DEFAULT_HOPPER_STARTUP_TIMEOUT_MS, } from "../domain/analysisResourceLimits.js"; import { DEFAULT_BRIDGE_MAX_REQUEST } from "./bridgeMaxRequestBytes.js"; +import { DEFAULT_BRIDGE_MAX_REPLY } from "./bridgeMaxReplyBytes.js"; import { hopperCallerScanBudgetFromEnvironment } from "./hopperCallerScanBudget.js"; import { DEFAULT_REGEX_MATCHING_DEADLINE } from "./regexMatchingDeadline.js"; import { DEFAULT_REGEX_WORKER_HEAP_REQUEST } from "./regexWorkerHeapRequest.js"; @@ -138,6 +139,7 @@ export const parseConfig = ( const valueFlowEdges = admitted("ghidra.value_flow_max_edges"); const callerScan = admitted("hopper.caller_scan_budget_ms"); const bridgeBytes = admitted("bridge.max_request_bytes"); + const bridgeReplyBytes = admitted("bridge.max_reply_bytes"); const regexDeadline = admitted("regex.matching_deadline_ms"); const regexHeap = admitted("regex.worker_heap_request_mib"); return ok({ @@ -249,6 +251,15 @@ export const parseConfig = ( maximumBytes: bridgeBytes, source: "setting", }, + bridgeMaxReplyBytes: + bridgeReplyBytes === undefined || + bridgeReplyBytes === null || + environment.REA_BRIDGE_MAX_REPLY_BYTES === undefined + ? DEFAULT_BRIDGE_MAX_REPLY + : { + maximumBytes: bridgeReplyBytes, + source: "setting", + }, regexMatchingDeadline: regexDeadline === undefined || regexDeadline === null || diff --git a/src/config/runtimeAnalysisSettings.ts b/src/config/runtimeAnalysisSettings.ts index 1299acbf79..c791257b95 100644 --- a/src/config/runtimeAnalysisSettings.ts +++ b/src/config/runtimeAnalysisSettings.ts @@ -3,8 +3,8 @@ import { currentAnalysisOperationSnapshot, currentAnalysisSettingOverride, } from "../domain/analysisOperationSnapshot.js"; -import type { ConfigurationError } from "../domain/configurationErrors.js"; -import { ok, type Result } from "../domain/result.js"; +import { ConfigurationError } from "../domain/configurationErrors.js"; +import { err, ok, type Result } from "../domain/result.js"; import type { AppConfig } from "./types.js"; import { DEFAULT_GHIDRA_VALUE_FLOW_MAX_OPERATIONS, @@ -25,6 +25,23 @@ import { type NativeMatchingAllowance, } from "./nativeMatchingAllowance.js"; +/** + * Read typed runtime settings at a request boundary. The views below throw the + * `ConfigurationError` of a malformed launch seed whenever no override covers + * it, which can first happen after startup (an override is reset); a caller + * that admits settings per request receives that refusal as a value. + */ +export const readRuntimeSetting = ( + read: () => T, +): Result => { + try { + return ok(read()); + } catch (cause: unknown) { + if (cause instanceof ConfigurationError) return err(cause); + throw cause; + } +}; + /** * Typed views read the enclosing operation snapshot, never the ambient * environment. A malformed launch seed is rejected here unless that snapshot @@ -38,6 +55,7 @@ export const runtimeAnalysisConfig = ( const heap = config.regexWorkerHeapRequest; const scan = config.hopperCallerScanBudget; const bridge = config.bridgeMaxRequestBytes; + const bridgeReply = config.bridgeMaxReplyBytes; const overrides = () => (read === undefined ? currentAnalysisOperationSnapshot()?.overrides @@ -158,6 +176,20 @@ export const runtimeAnalysisConfig = ( return bridge.source; }, }, + bridgeMaxReplyBytes: { + get maximumBytes() { + return ( + overrides()["bridge.max_reply_bytes"] ?? + (admit("bridge.max_reply_bytes"), bridgeReply.maximumBytes) + ); + }, + get source() { + if (overrides()["bridge.max_reply_bytes"] !== undefined) + return "runtime"; + admit("bridge.max_reply_bytes"); + return bridgeReply.source; + }, + }, }; }; diff --git a/src/config/types.ts b/src/config/types.ts index 5e615e2876..dc67dca197 100644 --- a/src/config/types.ts +++ b/src/config/types.ts @@ -9,6 +9,7 @@ import type { McpInputSchemaProfile } from "./mcpInputSchemaProfile.js"; import type { McpOutputSchemaProfile } from "./mcpOutputSchemaProfile.js"; import type { McpToolCatalogProfile } from "./mcpToolCatalogProfile.js"; import type { BridgeMaxRequestBytes } from "./bridgeMaxRequestBytes.js"; +import type { BridgeMaxReplyBytes } from "./bridgeMaxReplyBytes.js"; import type { RegexMatchingDeadline } from "./regexMatchingDeadline.js"; import type { RegexWorkerHeapRequest } from "./regexWorkerHeapRequest.js"; import type { HopperCallerScanBudget } from "./hopperCallerScanBudget.js"; @@ -54,6 +55,8 @@ export interface AppConfig { readonly referenceSourcePolicy: ReferenceSourcePolicy; /** Hopper and Cutter request payload ceiling resolved at startup. */ readonly bridgeMaxRequestBytes: BridgeMaxRequestBytes; + /** Hopper and Ghidra reply line ceiling resolved at startup. */ + readonly bridgeMaxReplyBytes: BridgeMaxReplyBytes; /** Supervised ECMAScript matching deadline resolved at startup. */ readonly regexMatchingDeadline: RegexMatchingDeadline; /** Regex worker old-generation request resolved at startup. */ diff --git a/src/contracts/advertisedDialectBaseline.ts b/src/contracts/advertisedDialectBaseline.ts index 4a584f0d52..66a63940f4 100644 --- a/src/contracts/advertisedDialectBaseline.ts +++ b/src/contracts/advertisedDialectBaseline.ts @@ -34,7 +34,7 @@ export const ADVERTISED_INPUT_DIALECT_BASELINE: Readonly< "input input-depth": 33, "input keyword $schema": 256, "input keyword allOf": 99, - "input keyword const": 1915, + "input keyword const": 1917, "input keyword default": 803, "input keyword else": 6, "input keyword examples": 253, diff --git a/src/domain/analysisResourceLimits.ts b/src/domain/analysisResourceLimits.ts index 38fbdbff57..afbdb5af83 100644 --- a/src/domain/analysisResourceLimits.ts +++ b/src/domain/analysisResourceLimits.ts @@ -196,6 +196,18 @@ export const DEFAULT_BRIDGE_MAX_REQUEST_BYTES = 16 * 1024 * 1024; export const MINIMUM_BRIDGE_MAX_REQUEST_BYTES = 7464; export const MAXIMUM_BRIDGE_MAX_REQUEST_BYTES = 536_870_888 - 1; +/** + * Hopper and Ghidra reply line ceiling, in UTF-8 bytes before the framing LF. + * The client refuses a longer line, or an unterminated tail past it, instead + * of retaining it. The default is far above the largest reply the packaged + * bridge fixtures produce (see `bridgeMaxReplyBytes.ts`). The floor admits the + * bridge handshake and a full inventory page. A line decodes to at most as many + * UTF-16 code units as it has bytes, so the maximum is Node's string ceiling. + */ +export const DEFAULT_BRIDGE_MAX_REPLY_BYTES = 256 * 1024 * 1024; +export const MINIMUM_BRIDGE_MAX_REPLY_BYTES = 1024 * 1024; +export const MAXIMUM_BRIDGE_MAX_REPLY_BYTES = NODE_MAX_STRING_LENGTH; + /** MCP client SDK default request timeout. JEB uses it when the setting is unset. */ export const DEFAULT_JEB_MCP_TIMEOUT_MS = 60_000; diff --git a/src/domain/analysisSettingDefinitions.ts b/src/domain/analysisSettingDefinitions.ts index f1c83a0246..b86dde49b1 100644 --- a/src/domain/analysisSettingDefinitions.ts +++ b/src/domain/analysisSettingDefinitions.ts @@ -34,6 +34,9 @@ import { DEFAULT_BRIDGE_MAX_REQUEST_BYTES, MINIMUM_BRIDGE_MAX_REQUEST_BYTES, MAXIMUM_BRIDGE_MAX_REQUEST_BYTES, + DEFAULT_BRIDGE_MAX_REPLY_BYTES, + MINIMUM_BRIDGE_MAX_REPLY_BYTES, + MAXIMUM_BRIDGE_MAX_REPLY_BYTES, DEFAULT_DEBUGGER_COMMAND_DEADLINE_MS, MINIMUM_DEBUGGER_COMMAND_DEADLINE_MS, DEFAULT_DEBUGGER_MAX_COMMAND_OUTPUT_BYTES, @@ -111,6 +114,54 @@ const windowsChangeTimeOperations = [ "inspect_recorded_crash", ] as const; +/** Every Hopper and Ghidra bridge operation; their replies share one line reader. */ +const hopperAndGhidraBridgeOperations = [ + "open_binary", + "address_name", + "comment", + "current_address", + "current_procedure", + "current_document", + "goto_address", + "inline_comment", + "list_bookmarks", + "list_documents", + "list_names", + "list_procedures", + "list_segments", + "list_strings", + "next_address", + "prev_address", + "procedure_address", + "procedure_assembly", + "procedure_callees", + "procedure_callers", + "procedure_info", + "read_function_instructions", + "read_bytes", + "address_to_file_offset", + "procedure_references", + "procedure_pseudo_code", + "resolve_containing_procedure", + "search_procedures", + "search_strings", + "set_address_name", + "set_addresses_names", + "set_bookmark", + "set_comment", + "set_inline_comment", + "unset_bookmark", + "xrefs", + "analyze_function", + "binary_overview", + "inspect_native_load_image", + "search_bytes", + "annotate_native_function", + "inspect_native_data_type", + "inspect_native_instruction", + "resolve_native_call_targets", +] as const; + /** Metadata aggregates existing owner definitions; no environment mutation or engine discovery. */ export const ANALYSIS_SETTINGS = { "ghidra.decompile_max_payload_mib": { @@ -383,54 +434,23 @@ export const ANALYSIS_SETTINGS = { // Cutter bridge calls. bridgeMaxRequestBytes.test.ts fails when a registry // gains an operation that is missing here. operations: [ - "open_binary", - "address_name", - "comment", - "current_address", - "current_procedure", - "current_document", - "goto_address", - "inline_comment", - "list_bookmarks", - "list_documents", - "list_names", - "list_procedures", - "list_segments", - "list_strings", - "next_address", - "prev_address", - "procedure_address", - "procedure_assembly", - "procedure_callees", - "procedure_callers", - "procedure_info", - "read_function_instructions", - "read_bytes", - "address_to_file_offset", - "procedure_references", - "procedure_pseudo_code", - "resolve_containing_procedure", - "search_procedures", - "search_strings", - "set_address_name", - "set_addresses_names", - "set_bookmark", - "set_comment", - "set_inline_comment", - "unset_bookmark", - "xrefs", - "analyze_function", - "binary_overview", - "inspect_native_load_image", - "search_bytes", - "annotate_native_function", - "inspect_native_data_type", - "inspect_native_instruction", - "resolve_native_call_targets", + ...hopperAndGhidraBridgeOperations, "list_cutter_sessions", "cutter_command", ], }, + "bridge.max_reply_bytes": { + updateMode: "runtime", + environment: "REA_BRIDGE_MAX_REPLY_BYTES", + defaultValue: DEFAULT_BRIDGE_MAX_REPLY_BYTES, + minimum: MINIMUM_BRIDGE_MAX_REPLY_BYTES, + maximum: MAXIMUM_BRIDGE_MAX_REPLY_BYTES, + unit: "bytes", + appliesAt: "next_operation", + provider: "hopper", + relatedProviders: ["ghidra"], + operations: hopperAndGhidraBridgeOperations, + }, "debugger.command_deadline_ms": { updateMode: "runtime", ...timer, diff --git a/src/domain/analysisSettings.ts b/src/domain/analysisSettings.ts index 66555a194c..c6d70130c5 100644 --- a/src/domain/analysisSettings.ts +++ b/src/domain/analysisSettings.ts @@ -19,6 +19,7 @@ export interface AnalysisSettingValues { readonly "wasm.command_timeout_ms": number; readonly "native.matching_representation_bytes": number; readonly "bridge.max_request_bytes": number; + readonly "bridge.max_reply_bytes": number; readonly "debugger.command_deadline_ms": number; readonly "debugger.max_command_output_bytes": number; readonly "hopper.caller_scan_budget_ms": number; diff --git a/src/gdb/GdbSessionManager.ts b/src/gdb/GdbSessionManager.ts index 5695044ba2..ce03089f8f 100644 --- a/src/gdb/GdbSessionManager.ts +++ b/src/gdb/GdbSessionManager.ts @@ -45,7 +45,7 @@ import type { } from "../domain/commandObservation.js"; import { currentAnalysisOperationSnapshot } from "../domain/analysisOperationSnapshot.js"; import { err, ok, type Result } from "../domain/result.js"; -import { ConfigurationError } from "../domain/configurationErrors.js"; +import { readRuntimeSetting } from "../config/runtimeAnalysisSettings.js"; import { ProviderProcessSupervisor, spawnOwnedProviderProcess, @@ -378,9 +378,6 @@ export class GdbSessionManager { if (!parsed.ok) throw parsed.error; return parsed.value; } - get #commandTimeoutMs(): number { - return this.#commandDeadline.value; - } get #commandOutput(): DebuggerAdmissionLimit { const override = currentAnalysisOperationSnapshot()?.overrides[ @@ -1016,6 +1013,13 @@ export class GdbSessionManager { session.knownInferiorIdentities.clear(); if (commandMayChangeInferiorOutput(command)) session.inferiorTtyVerified = false; + const accepted = readRuntimeSetting(() => ({ + deadline: this.#commandDeadline, + output: this.#commandOutput, + })); + if (!accepted.ok) return accepted; + const { deadline: acceptedDeadline, output: acceptedOutput } = + accepted.value; try { const result = await this.#request( session, @@ -1023,6 +1027,7 @@ export class GdbSessionManager { signal, "gdb_console", command, + accepted.value, ); const evidence = createEvidence(undefined, session.provider, { predicateType: "rea.debugger.console-observation", @@ -1069,7 +1074,7 @@ export class GdbSessionManager { ] : []), "MI records are returned as decoded text; their semantics depend on GDB version and target.", - ...gdbRetainedOutputLimitations(result, this.#commandOutput), + ...gdbRetainedOutputLimitations(result, acceptedOutput), ...(result.output_invalid_utf8 ? [ "MI stream bytes were not valid UTF-8; returned text contains replacement characters.", @@ -1080,7 +1085,6 @@ export class GdbSessionManager { }); return ok({ result, evidence }); } catch (cause) { - if (cause instanceof ConfigurationError) return err(cause); if (cause instanceof AnalysisCancelledError) return err(cause); if (cause instanceof AnalysisCapabilityUnavailableError) return err(cause as AnalysisError); @@ -1106,7 +1110,7 @@ export class GdbSessionManager { "The active GDB inferior identity is not established by the debugger session identifier.", limitations: [ "GDB exited before returning a correlated MI result record; only records retained for the active command are returned raw.", - ...gdbRetainedOutputLimitations(result, this.#commandOutput), + ...gdbRetainedOutputLimitations(result, acceptedOutput), ], }); const stopped = await this.#stopDebuggerOnly(session); @@ -1123,9 +1127,8 @@ export class GdbSessionManager { (limit) => limit.kind === "command_bytes", ) ? { - diagnostics: debuggerAdmissionDiagnostics( - this.#commandOutput, - ), + diagnostics: + debuggerAdmissionDiagnostics(acceptedOutput), } : {}), }, @@ -1134,11 +1137,11 @@ export class GdbSessionManager { return ok({ result, evidence }); } return err( - new AnalysisTimeoutError("gdb_console", this.#commandTimeoutMs, { + new AnalysisTimeoutError("gdb_console", acceptedDeadline.value, { cause, ...configurableDeadlineGuidance( "The GDB MI command", - this.#commandDeadline, + acceptedDeadline, ), }), ); @@ -1815,6 +1818,10 @@ export class GdbSessionManager { signal?: AbortSignal, operation: "gdb_console" | "start_gdb_session" = "gdb_console", evidenceCommand: string = command, + accepted?: { + readonly deadline: DebuggerAdmissionLimit; + readonly output: DebuggerAdmissionLimit; + }, ): Promise { const stdin = session.launched.process.stdin; if (session.exited) @@ -1834,9 +1841,9 @@ export class GdbSessionManager { return Promise.reject(new Error(message)); } const token = session.nextToken++; - const acceptedDeadline = this.#commandDeadline; + const acceptedDeadline = accepted?.deadline ?? this.#commandDeadline; const acceptedOutput = command.startsWith("-interpreter-exec console ") - ? this.#commandOutput + ? (accepted?.output ?? this.#commandOutput) : debuggerControlReplyOutput; return new Promise((resolve, reject) => { let abortListener!: () => void; diff --git a/src/ghidra/GhidraClient.ts b/src/ghidra/GhidraClient.ts index cb75c3306d..fb0d292de5 100644 --- a/src/ghidra/GhidraClient.ts +++ b/src/ghidra/GhidraClient.ts @@ -34,7 +34,10 @@ import { type LinuxCgroupMemoryState, } from "../process/LinuxCgroupMemory.js"; import { PendingOperations } from "../process/PendingOperations.js"; -import { Utf8NdjsonLineBuffer } from "../process/Utf8NdjsonLineBuffer.js"; +import { + JsonLineBridgeReader, + type JsonLineBridgeFailure, +} from "../process/JsonLineBridgeReader.js"; import { AbortedWait, waitForAbortable } from "../process/AbortablePromise.js"; import { SharedStartupAttempt } from "../process/SharedStartupAttempt.js"; import { PrivateRuntimeRoot } from "../process/PrivateRuntimeRoot.js"; @@ -48,6 +51,10 @@ import { } from "../process/ProviderProcess.js"; import { DEFAULT_GHIDRA_STARTUP_TIMEOUT_MS } from "../domain/analysisResourceLimits.js"; import { DEFAULT_BRIDGE_MAX_REQUEST } from "../config/bridgeMaxRequestBytes.js"; +import { + bridgeReplyOversizeError, + DEFAULT_BRIDGE_MAX_REPLY, +} from "../config/bridgeMaxReplyBytes.js"; import type { GhidraClientOptions, GhidraRequestOptions, @@ -63,7 +70,6 @@ import { type GhidraDiagnosticsScope, } from "./GhidraDiagnostics.js"; import { GhidraLaunchError, type GhidraLaunch } from "./GhidraLauncher.js"; -import { GhidraResponseRouter } from "./GhidraResponseRouter.js"; import { GhidraRequestQueue } from "./GhidraRequestQueue.js"; import { bindGhidraSessionFailure, @@ -113,6 +119,12 @@ import { type GhidraEndpoint, } from "./GhidraTransport.js"; import { GhidraWire } from "./GhidraClientWire.js"; +import { + ghidraRemoteError, + ghidraReplyEnvelope, + GhidraRemoteError, + type GhidraBridgeResponse, +} from "./protocol.js"; import { completeGhidraStartupHandshake } from "./GhidraClientStartup.js"; import { ghidraSessionRoot } from "./GhidraSessionRoot.js"; import { @@ -128,7 +140,6 @@ const SHUTDOWN_TIMEOUT_MS = 10_000; */ const OUTPUT_DRAIN_MS = 1_000; const SESSION_ROOT = ghidraSessionRoot(); -const GHIDRA_RESPONSE_NOT_UTF8 = "Ghidra bridge response is not valid UTF-8"; type GhidraStartupAttempt = SharedStartupAttempt; @@ -187,28 +198,35 @@ export class GhidraClient { ); readonly #wire: GhidraWire; readonly #requestQueue: GhidraRequestQueue; - readonly #responseRouter = new GhidraResponseRouter({ - pending: this.#pending, - nextId: () => this.#nextId, - remoteFailure: (failure) => - this.#failure("remote", failure.message, failure, { - remoteCode: failure.code, - }), - protocolFailure: (message, cause) => this.#abortProtocol(message, cause), + readonly #replies = new JsonLineBridgeReader< + GhidraBridgeResponse, + GhidraRemoteError + >({ + envelope: ghidraReplyEnvelope, + maxReplyBytes: () => this.#wire.replyLimit().maximumBytes, + deliver: (response) => { + if (!this.#pending.has(response.id)) return false; + this.#pending.settle( + response.id, + response.ok + ? ok(response.result) + : err(this.#remoteFailure(ghidraRemoteError(response.error))), + ); + return true; + }, + wasSent: (id) => id < this.#nextId, + rejectInFlight: (cause) => { + if (this.#pending.size === 0) return false; + this.#failAll(this.#remoteFailure(cause)); + return true; + }, + fail: (failure) => this.#abortReplyStream(failure), }); - readonly #responseLines = new Utf8NdjsonLineBuffer(); readonly #onSocketData = (chunk: Buffer): void => { - for (const line of this.#responseLines.push(chunk)) { - if (!line.ok) { - this.#abortProtocol(GHIDRA_RESPONSE_NOT_UTF8, line.error); - return; - } - this.#responseRouter.route(line.value); - } + this.#replies.push(chunk); }; readonly #onSocketEnd = (): void => { - const tail = this.#responseLines.finish(); - if (!tail.ok) this.#abortProtocol(GHIDRA_RESPONSE_NOT_UTF8, tail.error); + this.#replies.finish(); }; readonly #onSocketError = (): void => { this.#failAll(this.#failure("process", "Ghidra bridge socket failed")); @@ -239,6 +257,8 @@ export class GhidraClient { failure: this.#failure, bridgeMaxRequestBytes: options.bridgeMaxRequestBytes ?? DEFAULT_BRIDGE_MAX_REQUEST, + bridgeMaxReplyBytes: + options.bridgeMaxReplyBytes ?? DEFAULT_BRIDGE_MAX_REPLY, }); this.#requestQueue = new GhidraRequestQueue( async (method, parameters, requestOptions) => { @@ -403,7 +423,7 @@ export class GhidraClient { ): Promise> { const started = await this.start(options.signal); if (!started.ok) return started; - const result = await this.#requestQueue.run("ping", {}, options); + const result = await this.#enqueue("ping", {}, options); if (!result.ok) return result; return this.#parseSessionInfo(result.value); } @@ -438,6 +458,20 @@ export class GhidraClient { return this.#callTool(operation, parameters, options); } + /** Queue one request with the ceilings admitted in the caller's context. */ + #enqueue( + method: string, + parameters: JsonValue, + options: GhidraRequestOptions, + ): Promise> { + const limits = this.#wire.admitLimits(); + if (!limits.ok) return Promise.resolve(limits); + return this.#requestQueue.run(method, parameters, { + ...options, + limits: limits.value, + }); + } + async #callTool( operation: GhidraOperation, parameters: Readonly>, @@ -455,7 +489,7 @@ export class GhidraClient { typeof parameters.pattern === "string" ? parameters.pattern : undefined; - return this.#requestQueue.run( + return this.#enqueue( operation, pattern !== undefined ? { @@ -972,6 +1006,11 @@ export class GhidraClient { async #startWithin( deadline: ProviderStartupDeadline, ): Promise { + // Admit the bridge ceilings before acquiring anything: an invalid launch + // seed is a configuration refusal, not a failure after Ghidra started. + // The admitted ceilings are kept for this session's cleanup requests. + const limits = this.#wire.admitSessionLimits(); + if (!limits.ok) return limits; let packedProgram: GhidraPackedProgram | undefined; try { packedProgram = await this.#checkpoint?.acquire( @@ -1158,10 +1197,16 @@ export class GhidraClient { if (owner === undefined || runtime === undefined) return; await this.#finishCheckpointStaging(signal); const path = join(runtime.path, `checkpoint-${randomUUID()}.gzf`); + // Checkpoints run at startup and during close, so they use the session's + // admitted ceilings rather than the current settings. + const limits = this.#wire.sessionLimits(); const saved = await this.#wire.request( "checkpoint_program", { path, force }, - signal === undefined ? {} : { signal }, + { + ...(signal === undefined ? {} : { signal }), + ...(limits === undefined ? {} : { limits }), + }, ); if (!saved.ok) { if (saved.error.kind === "remote") @@ -1261,7 +1306,7 @@ export class GhidraClient { this.#detachSocket = undefined; this.#socket?.destroy(); this.#socket = undefined; - this.#responseLines.reset(); + this.#replies.reset(); } } @@ -1364,8 +1409,16 @@ export class GhidraClient { } const socket = this.#socket; if (!forceStop && socket !== undefined && !socket.destroyed) { + const limits = this.#wire.sessionLimits(); const shutdown = await this.#wire - .request("shutdown", {}, { timeoutMs: SHUTDOWN_TIMEOUT_MS }) + .request( + "shutdown", + {}, + { + timeoutMs: SHUTDOWN_TIMEOUT_MS, + ...(limits === undefined ? {} : { limits }), + }, + ) .catch((cause: unknown) => err( this.#failure("process", "Ghidra shutdown request failed", cause), @@ -1430,7 +1483,7 @@ export class GhidraClient { this.#endpointPath = undefined; this.#token = undefined; this.#runId = undefined; - this.#responseLines.reset(); + this.#replies.reset(); if (forcedStopFailure !== undefined) throw forcedStopFailure.cause; this.#cleanupRelease.confirm(); this.#cleanupRelease = new AnalysisCleanupRelease(); @@ -1523,6 +1576,43 @@ export class GhidraClient { this.#failAll(this.#failure("process", "Ghidra process exited")); } + #remoteFailure(failure: GhidraRemoteError): GhidraSessionError { + return this.#failure("remote", failure.message, failure, { + remoteCode: failure.code, + }); + } + + #abortReplyStream(failure: JsonLineBridgeFailure): void { + switch (failure.kind) { + case "reply_oversize": { + const oversize = bridgeReplyOversizeError( + "Ghidra", + this.#wire.replyLimit(), + failure.observedBytes, + ); + this.#abortProtocol(oversize.message, oversize); + return; + } + case "utf8": + this.#abortProtocol( + "Ghidra bridge response is not valid UTF-8", + failure.cause, + ); + return; + case "envelope": + this.#abortProtocol(failure.cause.message, failure.cause); + return; + case "unknown_id": + this.#abortProtocol("Ghidra returned an unknown response id"); + return; + case "connection_rejected": + this.#abortProtocol( + `Ghidra rejected the connection: ${failure.cause.message}`, + failure.cause, + ); + } + } + #abortProtocol(message: string, cause?: Error): void { const failure = this.#failure("protocol", message, cause); this.#requestQueue.failQueued(failure); diff --git a/src/ghidra/GhidraClientTypes.ts b/src/ghidra/GhidraClientTypes.ts index 26cf22dcc4..c9f7feb1ff 100644 --- a/src/ghidra/GhidraClientTypes.ts +++ b/src/ghidra/GhidraClientTypes.ts @@ -1,6 +1,7 @@ import type { Logger } from "pino"; import type { GhidraCheckpointSelection } from "./GhidraCheckpoint.js"; import type { BridgeMaxRequestBytes } from "../config/bridgeMaxRequestBytes.js"; +import type { BridgeMaxReplyBytes } from "../config/bridgeMaxReplyBytes.js"; import type { Result } from "../domain/result.js"; import type { LinuxCgroupMemoryState } from "../process/LinuxCgroupMemory.js"; import type { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js"; @@ -21,6 +22,19 @@ export interface GhidraRequestOptions { readonly signal?: AbortSignal; /** Internal startup/shutdown deadline; analysis calls have no fixed deadline. */ readonly timeoutMs?: number; + /** + * Ceilings admitted when the request entered the FIFO. The wire applies + * them only when this request is written, because the queue may drain it + * outside the caller's settings context. Omitted admits them at write time, + * for requests the caller writes directly. + */ + readonly limits?: GhidraAdmittedLimits; +} + +/** Bridge ceilings copied from the settings when one request was admitted. */ +export interface GhidraAdmittedLimits { + readonly requestLimit: BridgeMaxRequestBytes; + readonly replyLimit: BridgeMaxReplyBytes; } /** Safe lifecycle telemetry for one isolated Ghidra headless process. */ @@ -66,4 +80,6 @@ export interface GhidraClientOptions { }) => Promise; /** Effective request ceiling; defaults to the configured bridge ceiling. */ readonly bridgeMaxRequestBytes?: BridgeMaxRequestBytes; + /** Effective reply line ceiling; defaults to the configured bridge ceiling. */ + readonly bridgeMaxReplyBytes?: BridgeMaxReplyBytes; } diff --git a/src/ghidra/GhidraClientWire.test.ts b/src/ghidra/GhidraClientWire.test.ts index c4a2a566bc..288193b139 100644 --- a/src/ghidra/GhidraClientWire.test.ts +++ b/src/ghidra/GhidraClientWire.test.ts @@ -1,10 +1,15 @@ import type { Socket } from "node:net"; +import { + DEFAULT_BRIDGE_MAX_REPLY, + type BridgeMaxReplyBytes, +} from "../config/bridgeMaxReplyBytes.js"; 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 { ConfigurationError } from "../domain/configurationErrors.js"; import type { JsonValue } from "../domain/jsonValue.js"; import { ok, type Result } from "../domain/result.js"; import { PendingOperations } from "../process/PendingOperations.js"; @@ -32,8 +37,12 @@ type WireHarness = { readonly wire: GhidraWire; }; -const harness = (ceiling: BridgeMaxRequestBytes): WireHarness => { +const harness = ( + ceiling: BridgeMaxRequestBytes, + replyCeiling: BridgeMaxReplyBytes = DEFAULT_BRIDGE_MAX_REPLY, +): WireHarness => { const writes: string[] = []; + let nextId = 0; const socket = { destroyed: false, write: (payload: string, callback?: (cause?: Error | null) => void) => { @@ -52,15 +61,108 @@ const harness = (ceiling: BridgeMaxRequestBytes): WireHarness => { wire: new GhidraWire({ getSocket: () => socket, getToken: () => "b".repeat(64), - nextId: () => 1, + nextId: () => (nextId += 1), pending, logger: pino({ level: "silent" }), failure, bridgeMaxRequestBytes: ceiling, + bridgeMaxReplyBytes: replyCeiling, }), }; }; +describe("GhidraWire reply ceiling", () => { + it("admits the reply ceiling in the request's settings context", async () => { + let override = 2 * 1024 * 1024; + const { wire } = harness(limit(8192), { + get maximumBytes() { + return override; + }, + source: "runtime", + }); + expect(wire.replyLimit()).toEqual(DEFAULT_BRIDGE_MAX_REPLY); + + void wire.request("list_strings", {}, { timeoutMs: 1 }); + override = 4 * 1024 * 1024; + + expect(wire.replyLimit()).toEqual({ + maximumBytes: 2 * 1024 * 1024, + source: "runtime", + }); + }); + + it("activates a queued request's admitted ceiling only when it is written", async () => { + const { wire } = harness(limit(512), { + maximumBytes: 8 * 1024 * 1024, + source: "setting", + }); + const admitted: BridgeMaxReplyBytes = { + maximumBytes: 3 * 1024 * 1024, + source: "runtime", + }; + void wire.request("ping", {}, { timeoutMs: 1 }); + + const refused = await wire.request( + "list_strings", + { pattern: "x".repeat(1024) }, + { + timeoutMs: 1, + limits: { + requestLimit: limit(512), + replyLimit: { maximumBytes: 1, source: "runtime" }, + }, + }, + ); + expect(refused.ok).toBe(false); + expect( + wire.replyLimit(), + "a refused request leaves the written request's ceiling in force", + ).toEqual({ maximumBytes: 8 * 1024 * 1024, source: "setting" }); + + void wire.request( + "ping", + {}, + { + timeoutMs: 1, + limits: { requestLimit: limit(512), replyLimit: admitted }, + }, + ); + expect(wire.replyLimit()).toEqual(admitted); + }); +}); + +describe("GhidraWire ceiling admission", () => { + it.each([ + ["request", "REA_BRIDGE_MAX_REQUEST_BYTES"], + ["reply", "REA_BRIDGE_MAX_REPLY_BYTES"], + ] as const)( + "returns a %s seed refused after startup as an admission failure", + async (which, setting) => { + // A malformed seed is refused by the runtime view once the override + // that covered it at startup is reset. + const refusal = new ConfigurationError(`${setting} is invalid`, { + settings: [{ setting, constraint: "a positive decimal integer" }], + }); + const refused = { + get maximumBytes(): number { + throw refusal; + }, + source: "setting" as const, + }; + const { writes, pending, wire } = + which === "request" ? harness(refused) : harness(limit(8192), refused); + + const result = await wire.request("ping", {}, {}); + + if (result.ok) throw new Error("Expected a configuration refusal"); + expect(result.error.kind).toBe("admission"); + expect(result.error.cause).toBe(refusal); + expect(writes).toEqual([]); + expect(pending.size).toBe(0); + }, + ); +}); + describe("GhidraWire request ceiling", () => { it("refuses escaped oversized input before encoding, pending registration or write", async () => { const { writes, pending, wire } = harness(limit(8192)); diff --git a/src/ghidra/GhidraClientWire.ts b/src/ghidra/GhidraClientWire.ts index 7981cda32a..8415c45042 100644 --- a/src/ghidra/GhidraClientWire.ts +++ b/src/ghidra/GhidraClientWire.ts @@ -1,15 +1,25 @@ import type { Socket } from "node:net"; import { + admitBridgeRequestLimit, bridgeRequestOversizeError, type BridgeMaxRequestBytes, } from "../config/bridgeMaxRequestBytes.js"; +import { + admitBridgeReplyLimit, + DEFAULT_BRIDGE_MAX_REPLY, + type BridgeMaxReplyBytes, +} from "../config/bridgeMaxReplyBytes.js"; import type { JsonValue } from "../domain/jsonValue.js"; import { jsonValueByteLength } from "../domain/jsonValueByteLength.js"; -import { err, type Result } from "../domain/result.js"; +import { err, ok, type Result } from "../domain/result.js"; +import type { ConfigurationError } from "../domain/configurationErrors.js"; import type { Logger } from "pino"; import type { PendingOperations } from "../process/PendingOperations.js"; -import type { GhidraRequestOptions } from "./GhidraClientTypes.js"; +import type { + GhidraAdmittedLimits, + GhidraRequestOptions, +} from "./GhidraClientTypes.js"; import type { GhidraSessionError, GhidraSessionFailureFactory, @@ -28,16 +38,64 @@ export interface GhidraWireOptions { readonly failure: GhidraSessionFailureFactory; /** Effective Ghidra request ceiling; the bridge itself enforces none. */ readonly bridgeMaxRequestBytes: BridgeMaxRequestBytes; + /** Effective reply line ceiling, admitted with each request. */ + readonly bridgeMaxReplyBytes: BridgeMaxReplyBytes; } /** Owns the socket write and pending-reply lifecycle for one bridge session. */ export class GhidraWire { readonly #options: GhidraWireOptions; + #replyLimit: BridgeMaxReplyBytes = DEFAULT_BRIDGE_MAX_REPLY; + #sessionLimits: GhidraAdmittedLimits | undefined; constructor(options: GhidraWireOptions) { this.#options = options; } + /** Reply ceiling of the request last written; the default before any. */ + replyLimit(): BridgeMaxReplyBytes { + return this.#replyLimit; + } + + /** + * Copy the effective request and reply ceilings in the admitting caller's + * context. A malformed launch seed no override covers is an admission + * refusal carrying its `ConfigurationError`. + */ + admitLimits(): Result { + const requestLimit = admitBridgeRequestLimit( + this.#options.bridgeMaxRequestBytes, + ); + if (!requestLimit.ok) return err(this.#refused(requestLimit.error)); + const replyLimit = admitBridgeReplyLimit(this.#options.bridgeMaxReplyBytes); + if (!replyLimit.ok) return err(this.#refused(replyLimit.error)); + return ok({ + requestLimit: requestLimit.value, + replyLimit: replyLimit.value, + }); + } + + /** + * Admit the ceilings a session starts with and keep them for its cleanup. + * Cleanup must not depend on the current settings: an override reset after + * startup can expose a malformed launch seed, and refusing the checkpoint or + * shutdown request then would leave the JVM and its files behind. + */ + admitSessionLimits(): Result { + const limits = this.admitLimits(); + if (limits.ok) this.#sessionLimits = limits.value; + return limits; + } + + /** Ceilings admitted when the current session started, for its cleanup. */ + sessionLimits(): GhidraAdmittedLimits | undefined { + return this.#sessionLimits; + } + + #refused(cause: ConfigurationError): GhidraSessionError { + return this.#options.failure("admission", cause.message, cause); + } + /** Write one authenticated request and return its correlated reply. */ async request( method: string, @@ -78,13 +136,17 @@ export class GhidraWire { ? {} : { decompile_max_payload_mib: options.decompileMaxPayloadMiB }), }; + const limits = + options.limits === undefined ? this.admitLimits() : ok(options.limits); + if (!limits.ok) return limits; + const { requestLimit, replyLimit } = limits.value; 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) { + if (payloadBytes > requestLimit.maximumBytes) { const oversize = bridgeRequestOversizeError( method, - this.#options.bridgeMaxRequestBytes, + requestLimit, payloadBytes, "Ghidra", ); @@ -93,6 +155,9 @@ export class GhidraWire { ); } const payload = JSON.stringify(payloadValue); + // Only the request being written owns the next reply's ceiling; a + // refused or still-queued request never changes it. + this.#replyLimit = replyLimit; const response = this.#options.pending.wait(id, { ...(timeoutMs === undefined ? {} diff --git a/src/ghidra/GhidraProvider.behavior.test.ts b/src/ghidra/GhidraProvider.behavior.test.ts index 07e8782150..68dd3417e3 100644 --- a/src/ghidra/GhidraProvider.behavior.test.ts +++ b/src/ghidra/GhidraProvider.behavior.test.ts @@ -109,6 +109,7 @@ import { GhidraAdmissionError } from "./GhidraAdmissionError.js"; import { silentLogger } from "../logger.js"; import { ProviderCleanupError } from "../domain/providerCleanupError.js"; import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; +import { ConfigurationError } from "../domain/configurationErrors.js"; import { ProviderStartupDeadline } from "../process/ProviderDeadline.js"; import { createFunctionCoordinateContext, @@ -4733,6 +4734,42 @@ it.each([ }, ); +it("keeps a refused setting's configuration error through the provider boundary", async () => { + const refusal = new ConfigurationError("invalid bridge reply ceiling", { + settings: [ + { + setting: "REA_BRIDGE_MAX_REPLY_BYTES", + constraint: "a positive decimal integer", + }, + ], + }); + const failure = new GhidraSessionError( + "admission", + refusal.message, + {}, + { cause: refusal }, + ); + const ghidra = provider(installationHost(), () => ({ + start: async () => err(failure), + callTool: async () => err(failure), + close: async () => ok(null), + })); + onTestFinished(async () => { + await ghidra.closeOwnedResources(); + }); + const target = executableTarget("elf", "x86_64"); + const profile = await ghidra.resolveAnalysisProfile(target); + if (!profile.ok || profile.value.profile === null) + throw new Error("Expected profile"); + + const result = await ghidra + .createClient(target, profile.value.profile) + .execute("list_documents", {}); + + expect(result.ok ? undefined : result.error).toBe(refusal); + expect(projectAnalysisError(refusal).code).toBe("configuration_invalid"); +}); + describe("Ghidra startup deadline for a finished invocation", () => { const startupLimit = { setting: "ghidra.startup_timeout_ms", diff --git a/src/ghidra/GhidraProviderClient.ts b/src/ghidra/GhidraProviderClient.ts index 403f27e46b..e348c0b387 100644 --- a/src/ghidra/GhidraProviderClient.ts +++ b/src/ghidra/GhidraProviderClient.ts @@ -1,3 +1,4 @@ +import { readRuntimeSetting } from "../config/runtimeAnalysisSettings.js"; import { bindProcessInspectionCancellations } from "../providers/OwnedCommandCleanup.js"; import { filterGhidraLiteralCandidates, @@ -616,17 +617,18 @@ export const createGhidraProviderClient = (input: { }), ); // An invalid decompile seed is refused only by operations that use it. - let launcherDecompileTimeout: number | undefined; - try { - launcherDecompileTimeout = config.ghidraDecompileTimeoutSeconds; - } catch (cause: unknown) { - if (!(cause instanceof ConfigurationError)) throw cause; - } + const decompileTimeout = readRuntimeSetting( + () => config.ghidraDecompileTimeoutSeconds, + ); + const launcherDecompileTimeout = decompileTimeout.ok + ? decompileTimeout.value + : undefined; const client = clientFactory({ ...(checkpoint.data === undefined ? {} : { checkpoint: checkpoint.data }), startupTimeoutMs: config.ghidraStartupTimeoutMs, readStartupTimeoutMs: () => config.ghidraStartupTimeoutMs, bridgeMaxRequestBytes: config.bridgeMaxRequestBytes, + bridgeMaxReplyBytes: config.bridgeMaxReplyBytes, platform: installation.platform, ...(project === undefined ? {} : { existingProject: true }), launcher: new GhidraHeadlessLauncher({ @@ -1055,13 +1057,9 @@ export const createGhidraProviderClient = (input: { } : {}), }); - let operationLimits: ReturnType; - try { - operationLimits = selectLimits(); - } catch (cause: unknown) { - if (cause instanceof ConfigurationError) return err(cause); - throw cause; - } + const admittedLimits = readRuntimeSetting(selectLimits); + if (!admittedLimits.ok) return admittedLimits; + const operationLimits = admittedLimits.value; // A report binds before any query runs. The client shares one startup // attempt, so `callTool` reuses this launch; a restart is checked again. const started = await startSession(operation, options); @@ -1354,6 +1352,9 @@ const projectPrimarySessionError = ( ? {} : { partialObservation: failure.partialObservation }), }; + // A refused setting (for example the reply ceiling admitted before launch) + // is already the caller-facing refusal with its setting and allowed values. + if (failure.cause instanceof ConfigurationError) return failure.cause; if (failure.kind === "checkpoint") { if (failure.cause instanceof AnalysisError) return analysisClientFailureForOperation( diff --git a/src/ghidra/GhidraResponseRouter.test.ts b/src/ghidra/GhidraResponseRouter.test.ts deleted file mode 100644 index 9724a921f4..0000000000 --- a/src/ghidra/GhidraResponseRouter.test.ts +++ /dev/null @@ -1,117 +0,0 @@ -import { expect, it, vi } from "vitest"; - -import type { JsonValue } from "../domain/jsonValue.js"; -import { err, ok, type Result } from "../domain/result.js"; -import { Utf8NdjsonLineBuffer } from "../process/Utf8NdjsonLineBuffer.js"; -import { PendingOperations } from "../process/PendingOperations.js"; -import { GhidraResponseRouter } from "./GhidraResponseRouter.js"; -import { GhidraSessionError } from "./GhidraSessionError.js"; - -it("routes later Ghidra response lines after an unknown response id", async () => { - const pending = new PendingOperations< - number, - Result - >(); - const protocolFailure = vi.fn(); - const router = new GhidraResponseRouter({ - pending, - nextId: () => 2, - remoteFailure: (failure) => - new GhidraSessionError( - "remote", - failure.message, - {}, - { - remoteCode: failure.code, - }, - ), - protocolFailure: (message, cause) => - protocolFailure( - new GhidraSessionError("protocol", message, {}, { cause }), - ), - }); - const response = pending.wait(1, { - cancelledValue: () => - err(new GhidraSessionError("cancelled", "request cancelled")), - }); - const lines = new Utf8NdjsonLineBuffer(); - - for (const line of lines.push( - Buffer.from( - '{"id":2,"ok":true,"result":null}\n{"id":1,"ok":true,"result":"settled"}\n', - ), - )) - if (line.ok) router.route(line.value); - - expect(protocolFailure).toHaveBeenCalledOnce(); - expect(protocolFailure).toHaveBeenCalledWith( - expect.objectContaining({ kind: "protocol" }), - ); - await expect(response).resolves.toEqual(ok("settled")); -}); - -const routerFor = (nextId: number) => { - const pending = new PendingOperations< - number, - Result - >(); - const protocolFailures: GhidraSessionError[] = []; - const router = new GhidraResponseRouter({ - pending, - nextId: () => nextId, - remoteFailure: (failure) => - new GhidraSessionError( - "remote", - failure.message, - {}, - { remoteCode: failure.code }, - ), - protocolFailure: (message, cause) => - protocolFailures.push( - new GhidraSessionError("protocol", message, {}, { cause }), - ), - }); - return { pending, protocolFailures, router }; -}; -const invalidRequestReply = (id: number) => - `{"id":${String(id)},"ok":false,"error":{"code":"invalid_request","message":"Bridge request is invalid"}}`; - -it("fails the request in flight with the bridge's connection-level invalid_request", async () => { - const { pending, protocolFailures, router } = routerFor(5); - const response = pending.wait(4, { - cancelledValue: () => - err(new GhidraSessionError("cancelled", "request cancelled")), - }); - - router.route(invalidRequestReply(0)); - - const settled = await response; - expect( - settled.ok ? undefined : settled.error, - "an unattributable rejection reaches the caller as the remote invalid_request", - ).toMatchObject({ kind: "remote", remoteCode: "invalid_request" }); - expect(protocolFailures).toEqual([]); -}); - -it("reports a connection-level rejection with nothing in flight as a protocol failure", () => { - const { protocolFailures, router } = routerFor(5); - - router.route(invalidRequestReply(0)); - - expect(protocolFailures).toEqual([ - expect.objectContaining({ - kind: "protocol", - message: expect.stringContaining("Bridge request is invalid"), - }), - ]); -}); - -it("never accepts a success on the reserved connection id", () => { - const { protocolFailures, router } = routerFor(5); - - router.route('{"id":0,"ok":true,"result":null}'); - - expect(protocolFailures).toEqual([ - expect.objectContaining({ kind: "protocol" }), - ]); -}); diff --git a/src/ghidra/GhidraResponseRouter.ts b/src/ghidra/GhidraResponseRouter.ts deleted file mode 100644 index ca6ef0243b..0000000000 --- a/src/ghidra/GhidraResponseRouter.ts +++ /dev/null @@ -1,63 +0,0 @@ -import type { JsonValue } from "../domain/jsonValue.js"; -import { err, type Result } from "../domain/result.js"; -import type { PendingOperations } from "../process/PendingOperations.js"; -import type { GhidraSessionError } from "./GhidraSessionError.js"; -import { - GHIDRA_CONNECTION_FAILURE_ID, - ghidraResponseResult, - parseGhidraResponseLine, -} from "./protocol.js"; -import type { GhidraRemoteError } from "./protocol.js"; - -/** Dependencies for correlating validated bridge responses. */ -export interface GhidraResponseRouterOptions { - readonly pending: PendingOperations< - number, - Result - >; - readonly nextId: () => number; - readonly remoteFailure: (failure: GhidraRemoteError) => GhidraSessionError; - readonly protocolFailure: (message: string, cause?: Error) => void; -} - -/** Routes validated response IDs without giving the wire protocol client state. */ -export class GhidraResponseRouter { - readonly #options: GhidraResponseRouterOptions; - - constructor(options: GhidraResponseRouterOptions) { - this.#options = options; - } - - /** Parse and settle one complete response line. */ - route(line: string): void { - const parsed = parseGhidraResponseLine(line); - if (!parsed.ok) { - this.#options.protocolFailure(parsed.error.message, parsed.error); - return; - } - const result = ghidraResponseResult(parsed.value); - if (parsed.value.id === GHIDRA_CONNECTION_FAILURE_ID && !result.ok) { - // The bridge rejected a request it could not attribute and closes the - // connection, so every request in flight fails with its remote reason. - if (this.#options.pending.size === 0) - this.#options.protocolFailure( - `Ghidra rejected the connection: ${result.error.message}`, - result.error, - ); - else { - const failure = this.#options.remoteFailure(result.error); - this.#options.pending.failAll(() => err(failure)); - } - return; - } - if (!this.#options.pending.has(parsed.value.id)) { - if (parsed.value.id >= this.#options.nextId()) - this.#options.protocolFailure("Ghidra returned an unknown response id"); - return; - } - this.#options.pending.settle( - parsed.value.id, - result.ok ? result : err(this.#options.remoteFailure(result.error)), - ); - } -} diff --git a/src/ghidra/protocol.ts b/src/ghidra/protocol.ts index f3958e0382..7594fa817f 100644 --- a/src/ghidra/protocol.ts +++ b/src/ghidra/protocol.ts @@ -1,8 +1,9 @@ import { GhidraAdmissionError } from "./GhidraAdmissionError.js"; import { z } from "zod"; -import { ownedJsonValueSchema, type JsonValue } from "../domain/jsonValue.js"; +import { ownedJsonValueSchema } from "../domain/jsonValue.js"; import { err, ok, type Result } from "../domain/result.js"; +import type { JsonLineBridgeEnvelope } from "../process/JsonLineBridgeReader.js"; const responseSchema = z.discriminatedUnion("ok", [ z.strictObject({ @@ -11,8 +12,8 @@ const responseSchema = z.discriminatedUnion("ok", [ result: ownedJsonValueSchema, }), z.strictObject({ - // Id 0 is reserved for a connection-level failure: the bridge could not - // parse the request, so it cannot name the request id it rejects. + // Id 0 is BRIDGE_CONNECTION_FAILURE_ID: the bridge could not parse the + // request, so it cannot name the request id it rejects. id: z.int().nonnegative(), ok: z.literal(false), error: z @@ -24,9 +25,6 @@ const responseSchema = z.discriminatedUnion("ok", [ }), ]); -/** Response id the bridge uses for a failure not tied to one request. */ -export const GHIDRA_CONNECTION_FAILURE_ID = 0; - /** Validated response emitted by REA's packaged Ghidra script. */ export type GhidraBridgeResponse = z.infer; @@ -41,8 +39,13 @@ export class GhidraRemoteError extends Error { } } +/** The remote failure one rejected request carries. */ +export const ghidraRemoteError = ( + error: Extract["error"], +): GhidraRemoteError => new GhidraRemoteError(error.code, error.message); + /** Parse one complete Java-bridge response without accepting extra fields. */ -export const parseGhidraResponseLine = ( +const parseGhidraResponseLine = ( line: string, ): Result => { try { @@ -56,10 +59,13 @@ export const parseGhidraResponseLine = ( } }; -/** Project one validated response into its result or remote failure. */ -export const ghidraResponseResult = ( - response: GhidraBridgeResponse, -): Result => - response.ok - ? ok(response.result) - : err(new GhidraRemoteError(response.error.code, response.error.message)); +/** Ghidra's `ok`-discriminated envelope for the shared JSON-line reader. */ +export const ghidraReplyEnvelope: JsonLineBridgeEnvelope< + GhidraBridgeResponse, + GhidraRemoteError +> = { + parse: parseGhidraResponseLine, + id: (response) => response.id, + remoteFailure: (response) => + response.ok ? undefined : ghidraRemoteError(response.error), +}; diff --git a/src/hopper/HopperCallerReconciliation.test.ts b/src/hopper/HopperCallerReconciliation.test.ts index fbf9cc7dbc..7194a759d5 100644 --- a/src/hopper/HopperCallerReconciliation.test.ts +++ b/src/hopper/HopperCallerReconciliation.test.ts @@ -75,7 +75,7 @@ const base = { }; const referenceResult = { procedure: target, - direction: "incoming", + direction: "incoming" as const, reference_kinds_available: false, unresolved_calls: [], references: [], @@ -94,7 +94,7 @@ it("builds one forward index for successive queries in a workflow", async () => client, "procedure_callers", { procedure: "name" }, - base, + ok(base), options, 250, ); @@ -102,7 +102,7 @@ it("builds one forward index for successive queries in a workflow", async () => client, "procedure_references", { procedure: "name", direction: "incoming" }, - referenceResult, + ok(referenceResult), options, 250, ); @@ -157,7 +157,7 @@ it("builds one forward index for successive queries in a workflow", async () => client, "procedure_callers", { procedure: "name" }, - base, + ok(base), options, 250, ); @@ -198,7 +198,7 @@ it("links each compact call site to its owning caller address", async () => { { callTool } as unknown as HopperClient, "procedure_callers", { procedure: "name" }, - base, + ok(base), undefined, 250, ); @@ -237,7 +237,7 @@ it("merges paged call sites and pins the selected target", async () => { client, "procedure_references", { procedure: "name", direction: "incoming" }, - referenceResult, + ok(referenceResult), undefined, 250, ); @@ -267,7 +267,7 @@ it("does not reuse a previous scan", async () => { client, "procedure_callers", { procedure: "name" }, - base, + ok(base), undefined, 250, ); @@ -292,7 +292,7 @@ it("does not reuse a previous scan", async () => { client, "procedure_callers", { procedure: "name" }, - base, + ok(base), undefined, 250, ); @@ -309,7 +309,7 @@ it("stops between pages on cancellation", async () => { { callTool } as unknown as HopperClient, "procedure_callers", {}, - base, + ok(base), { signal: controller.signal }, 250, ); @@ -350,7 +350,7 @@ it("preserves cancellation while reporting the last page", async () => { { callTool } as unknown as HopperClient, "procedure_callers", {}, - base, + ok(base), { signal: controller.signal, progress }, 250, ); @@ -393,7 +393,7 @@ it("retains completed pages when a later scan page fails", async () => { { callTool } as unknown as HopperClient, "procedure_callers", {}, - base, + ok(base), undefined, 250, ); @@ -426,7 +426,7 @@ it("reports a changing inventory as incomplete", async () => { { callTool } as unknown as HopperClient, "procedure_callers", {}, - base, + ok(base), undefined, 250, ); @@ -452,7 +452,7 @@ it("retains original callers and reports a procedure that exceeds the page allow { callTool } as unknown as HopperClient, "procedure_callers", {}, - { ...base, callers: ["0x1000"] }, + ok({ ...base, callers: ["0x1000"] }), undefined, 250, ); @@ -507,7 +507,7 @@ it("refuses expanded reference copies that exceed the retained scan budget", asy { callTool } as unknown as HopperClient, "procedure_references", { procedure: "name", direction: "incoming" }, - referenceResult, + ok(referenceResult), undefined, 250, ); @@ -541,7 +541,7 @@ it("reports truncated target instructions and native callers", async () => { { callTool } as unknown as HopperClient, "procedure_callers", {}, - { ...base, instructions_complete: false, callers_complete: false }, + ok({ ...base, instructions_complete: false, callers_complete: false }), undefined, 250, ); diff --git a/src/hopper/HopperCallerReconciliation.ts b/src/hopper/HopperCallerReconciliation.ts index f1eb500b9c..3d2e09bf89 100644 --- a/src/hopper/HopperCallerReconciliation.ts +++ b/src/hopper/HopperCallerReconciliation.ts @@ -18,7 +18,10 @@ import { procedureIdentitySchema } from "../domain/native/procedureInfo.js"; import { nativeCallClassificationSchema } from "../domain/native/nativeCallReference.js"; import { err, ok, type Result } from "../domain/result.js"; import type { HopperClient } from "./HopperClient.js"; -import { parseHopperReply, type HopperReply } from "./HopperReplyContracts.js"; +import { + HopperReplyContractError, + type HopperReply, +} from "./HopperReplyContracts.js"; import { analysisClientFailureForOperation } from "../providers/AnalysisClientFailure.js"; import { hopperCallerScanBudgetForMilliseconds, @@ -167,11 +170,14 @@ async function buildIndex( : reportScanProgress(options, start, total), }, }; - const page = await client.callTool( - "rea_caller_scan_page", - args, - pageOptions, - ); + const page = await client.callTool("rea_caller_scan_page", args, { + ...pageOptions, + operation, + }); + if (!page.ok && page.error instanceof HopperReplyContractError) + return options?.signal?.aborted + ? fail(cancelled(), { code: "cancelled" }, page.error.reply) + : fail(page.error, { code: "invalid_page" }, page.error.reply); if (!page.ok) return options?.signal?.aborted ? fail(cancelled(), { code: "cancelled" }) @@ -179,16 +185,7 @@ async function buildIndex( analysisClientFailureForOperation(page.error, "hopper", operation), { code: "page_failure" }, ); - const decoded = parseHopperReply( - "rea_caller_scan_page", - page.value, - operation, - ); - if (!decoded.ok) - return options?.signal?.aborted - ? fail(cancelled(), { code: "cancelled" }, page.value) - : fail(decoded.error, { code: "invalid_page" }, page.value); - const value = decoded.value; + const value = page.value; if ( value.start !== start || value.next < start || @@ -201,14 +198,14 @@ async function buildIndex( (value.oversized && value.next !== start) ) return options?.signal?.aborted - ? fail(cancelled(), { code: "cancelled" }, page.value) + ? fail(cancelled(), { code: "cancelled" }, value) : fail( new AnalysisOutputError( operation, "Invalid Hopper caller scan progress", ), { code: "invalid_progress" }, - page.value, + value, ); if (total !== undefined && value.total !== total) { stop = { code: "inventory_changed" }; @@ -329,11 +326,17 @@ const compactForwardDestinations = ( }; }; +/** The validated native reply each reconciled operation starts from. */ +export type HopperCallerNativeReply = + | HopperReply<"rea_caller_base"> + | HopperReply<"analyze_function"> + | HopperReply<"procedure_references">; + export const reconcileHopperCallers = async ( client: HopperClient, operation: Operation, parameters: Readonly>, - original: JsonValue, + nativeReply: Result, options: ExecutionOptions | undefined, budget: number | HopperCallerScanBudget, ): Promise< @@ -347,6 +350,10 @@ export const reconcileHopperCallers = async ( AnalysisError > > => { + // The native reply exactly as Hopper sent it, kept when it missed its contract. + const original: JsonValue = nativeReply.ok + ? nativeReply.value + : nativeReply.error.reply; const scanBudget = typeof budget === "number" ? hopperCallerScanBudgetForMilliseconds(budget) @@ -393,13 +400,7 @@ export const reconcileHopperCallers = async ( ); return error; }; - // Each native reply is validated once by its bridge reply contract. - const nativeReply = - operation === "procedure_callers" - ? parseHopperReply("rea_caller_base", original, operation) - : operation === "analyze_function" - ? parseHopperReply("analyze_function", original, operation) - : parseHopperReply("procedure_references", original, operation); + // The client validated the native reply against its method contract. if (!nativeReply.ok) return err( retainNative(nativeReply.error, { scan: null, destinations: null }), @@ -410,43 +411,35 @@ export const reconcileHopperCallers = async ( if (parameters.document !== undefined) args.document = parameters.document; if (parameters.procedure !== undefined) args.procedure = parameters.procedure; // The callers base already carries its scan target. - const targetResponse = + const selected = base !== undefined - ? ok(original) - : await client.callTool( - "rea_caller_scan_target", - args, - reconciliationChildOptions(options), - ); - if (!targetResponse.ok) - return err( - retainNative( - analysisClientFailureForOperation( - targetResponse.error, - "hopper", + ? ok(base) + : await client.callTool("rea_caller_scan_target", args, { + ...reconciliationChildOptions(options), operation, - ), - { scan: null, destinations: null }, - ), - ); - if (options?.signal?.aborted) + }); + if ( + options?.signal?.aborted && + (selected.ok || selected.error instanceof HopperReplyContractError) + ) return err( retainNative(cancelledBy(operation, options.signal), { scan: null, destinations: null, }), ); - const selected = - base !== undefined - ? ok(base) - : parseHopperReply( - "rea_caller_scan_target", - targetResponse.value, - operation, - ); if (!selected.ok) return err( - retainNative(selected.error, { scan: null, destinations: null }), + retainNative( + selected.error instanceof HopperReplyContractError + ? selected.error + : analysisClientFailureForOperation( + selected.error, + "hopper", + operation, + ), + { scan: null, destinations: null }, + ), ); const { target, instructions, instructions_complete } = selected.value; if ( diff --git a/src/hopper/HopperClient.ts b/src/hopper/HopperClient.ts index 43b6f508d1..d0e6ec636a 100644 --- a/src/hopper/HopperClient.ts +++ b/src/hopper/HopperClient.ts @@ -1,3 +1,7 @@ +import { + JsonLineBridgeReader, + type JsonLineBridgeFailure, +} from "../process/JsonLineBridgeReader.js"; import { analysisSettingLimit } from "../domain/analysisSettingDefinitions.js"; import { randomBytes, randomUUID } from "node:crypto"; import { access } from "node:fs/promises"; @@ -12,6 +16,16 @@ import { AnalysisCapabilityUnavailableError, OwnerCancellation, } from "../domain/analysisErrorCore.js"; +import { + hopperBridgeReplyContracts, + parseHopperReply, + type HopperBridgeReplyContracts, + type ValueKeepingContracts, + type HopperContractReply, + type HopperReplyContracts, + parseHopperServerInfo, + type HopperServerInfo, +} from "./HopperReplyContracts.js"; import { hopperCancelledBySignal, type HopperCancelledError, @@ -24,7 +38,7 @@ import { HopperTimeoutError, } from "../domain/hopperErrors.js"; import type { AnalysisError } from "../domain/analysisErrorBase.js"; -import { ConfigurationError } from "../domain/configurationErrors.js"; +import { readRuntimeSetting } from "../config/runtimeAnalysisSettings.js"; import { hopperStartupFailure, type HopperStartupFailureDiagnostic, @@ -65,10 +79,6 @@ import { type HopperOwnedResources, } from "./HopperCleanup.js"; import { HOPPER_SHUTDOWN_TIMEOUT_MS } from "./HopperSessionValues.js"; -import { - parseHopperServerInfo, - type HopperServerInfo, -} from "./HopperReplyContracts.js"; import { connectHopperSocketOnce } from "./HopperSocketConnection.js"; import { hopperLauncherFailureDiagnostic, @@ -77,19 +87,26 @@ import { } from "./HopperProcessDiagnostic.js"; import { DEFAULT_HOPPER_STARTUP_TIMEOUT_MS } from "../domain/analysisResourceLimits.js"; import { + admitBridgeRequestLimit, bridgeRequestLimitMismatchError, bridgeRequestOversizeError, DEFAULT_BRIDGE_MAX_REQUEST, type BridgeMaxRequestBytes, } from "../config/bridgeMaxRequestBytes.js"; +import { + admitBridgeReplyLimit, + bridgeReplyOversizeError, + DEFAULT_BRIDGE_MAX_REPLY, + type BridgeMaxReplyBytes, +} from "../config/bridgeMaxReplyBytes.js"; import { HopperRequestQueue, type HopperRequestActivity, } from "./HopperRequestQueue.js"; -import { HopperResponseStream } from "./HopperResponseStream.js"; import { type HopperBridgeEvent, type HopperBridgeMessage, + hopperReplyEnvelope, responseResult, } from "./protocol.js"; @@ -99,8 +116,15 @@ const HOPPER_PROCESS_DIAGNOSTIC_BYTES = 8 * 1024 * 1024; const SESSION_ROOT = process.platform === "darwin" ? "/tmp" : tmpdir(); /** Dependencies, deadlines, and redacted diagnostics for one bridge client. */ -export interface HopperClientOptions { +export interface HopperClientOptions< + Contracts extends HopperReplyContracts = HopperBridgeReplyContracts, +> { readonly launcher: BridgeLauncher; + /** + * Reply contract for each callable method. Production uses the bridge's + * `hopperReplySchemas`; transport tests supply their own methods. + */ + readonly replyContracts?: Contracts & ValueKeepingContracts; readonly runId?: string; readonly startupTimeoutMs?: number; readonly readStartupTimeoutMs?: () => number; @@ -112,10 +136,18 @@ export interface HopperClientOptions { | undefined; /** Effective request ceiling. Omitted uses the 16 MiB default. */ readonly bridgeMaxRequestBytes?: BridgeMaxRequestBytes; + /** Effective reply line ceiling. Omitted uses the 256 MiB default. */ + readonly bridgeMaxReplyBytes?: BridgeMaxReplyBytes; readonly onDiagnostic?: (event: HopperDiagnostic) => void; readonly logger?: Logger; } +/** Bridge ceilings admitted when a session started, kept for its cleanup. */ +interface HopperSessionLimits { + readonly requestLimit: BridgeMaxRequestBytes; + readonly replyLimit: BridgeMaxReplyBytes; +} + type HopperStartupResult = Result; type HopperStartupAttempt = SharedStartupAttempt; type HopperStartupOutcome = @@ -139,13 +171,19 @@ type HopperClientCloseOptions = { * only terminates launch processes explicitly marked as owned; the normal * Hopper launcher does not confer ownership of the GUI application. */ -export class HopperClient { +export class HopperClient< + Contracts extends HopperReplyContracts = HopperBridgeReplyContracts, +> { readonly #options: Required< Pick > & - HopperClientOptions; + HopperClientOptions; + readonly #contracts: Contracts & ValueKeepingContracts; readonly #requests: HopperRequestQueue; - readonly #responses: HopperResponseStream; + readonly #responses: JsonLineBridgeReader< + HopperBridgeMessage, + HopperRemoteError + >; readonly #logger: Logger; #socket: Socket | undefined; readonly #resources: HopperOwnedResources = { @@ -156,6 +194,9 @@ export class HopperClient { externalApplication: undefined, }; #token: string | undefined; + /** Reply ceiling admitted with the request on the wire; the default before any. */ + #replyLimit: BridgeMaxReplyBytes = DEFAULT_BRIDGE_MAX_REPLY; + #sessionLimits: HopperSessionLimits | undefined; readonly #lineage = new ProviderRunLineage(); #nextId = 1; #closing = false; @@ -214,8 +255,16 @@ export class HopperClient { this.#rememberProcessFailure(error, []); }; - constructor(options: HopperClientOptions) { + constructor(options: HopperClientOptions) { this.#logger = options.logger ?? silentLogger; + // The default type argument is the production table this default supplies. + // Without a table the type argument is its default, the production table. + // A caller that names another type argument but omits its table gets a + // typed refusal per method (see #typedReply), never a throw. + this.#contracts = + options.replyContracts ?? + (hopperBridgeReplyContracts as unknown as Contracts & + ValueKeepingContracts); this.#options = { ...options, startupTimeoutMs: @@ -224,7 +273,7 @@ export class HopperClient { options.bridgeMaxRequestBytes ?? DEFAULT_BRIDGE_MAX_REQUEST, }; this.#requests = new HopperRequestQueue( - ({ id, method, params }, failed) => { + ({ id, method, params, requestLimit, replyLimit }, failed) => { const socket = this.#socket; const token = this.#token; if (socket === undefined || socket.destroyed || token === undefined) { @@ -247,13 +296,14 @@ export class HopperClient { return; } const payloadBytes = Buffer.byteLength(payload); - if (payloadBytes > this.#options.bridgeMaxRequestBytes.maximumBytes) { + const admitted = requestLimit ?? DEFAULT_BRIDGE_MAX_REQUEST; + if (payloadBytes > admitted.maximumBytes) { this.#requests.accept( id, err( bridgeRequestOversizeError( method, - this.#options.bridgeMaxRequestBytes, + admitted, payloadBytes, "Hopper", ), @@ -261,16 +311,21 @@ export class HopperClient { ); return; } + // This request now owns the wire, so its admitted ceiling bounds the + // next reply; a queued or refused request never changes it. + this.#replyLimit = replyLimit ?? DEFAULT_BRIDGE_MAX_REPLY; socket.write(`${payload}\n`, (cause) => { if (cause !== undefined && cause !== null) failed(); }); }, ); - this.#responses = new HopperResponseStream({ - accept: (message) => this.#acceptBridgeMessage(message), - hasQueued: (id) => this.#requests.hasQueued(id), - nextRequestId: () => this.#nextId, - abort: (message, cause) => this.#abortProtocol(message, cause), + this.#responses = new JsonLineBridgeReader({ + envelope: hopperReplyEnvelope, + maxReplyBytes: () => this.#replyLimit.maximumBytes, + deliver: (message) => this.#acceptBridgeMessage(message), + wasSent: (id) => id < this.#nextId && !this.#requests.hasQueued(id), + rejectInFlight: (cause) => this.#requests.failActive(cause), + fail: (failure) => this.#abortReplyStream(failure), }); } @@ -447,15 +502,13 @@ export class HopperClient { new HopperProtocolError("Hopper client is already started"), ); } - let startupTimeoutMs: number; - try { - startupTimeoutMs = + const startupTimeout = readRuntimeSetting( + () => this.#options.readStartupTimeoutMs?.() ?? - this.#options.startupTimeoutMs; - } catch (cause: unknown) { - if (cause instanceof ConfigurationError) return err(cause); - throw cause; - } + this.#options.startupTimeoutMs, + ); + if (!startupTimeout.ok) return startupTimeout; + const startupTimeoutMs = startupTimeout.value; this.#operationFailure = undefined; this.#startupFailureResult = undefined; this.#resources.shutdownConfirmed = false; @@ -484,6 +537,22 @@ export class HopperClient { async #startWithin( deadline: ProviderStartupDeadline, ): Promise> { + // Admit both bridge ceilings before acquiring anything: an invalid launch + // seed is a configuration refusal, not a failure after Hopper started. + // Cleanup requests reuse these, so a setting reset after startup cannot + // refuse the shutdown that releases Hopper's document and files. + const requestLimit = admitBridgeRequestLimit( + this.#options.bridgeMaxRequestBytes, + ); + if (!requestLimit.ok) return requestLimit; + const replyLimit = admitBridgeReplyLimit( + this.#options.bridgeMaxReplyBytes ?? DEFAULT_BRIDGE_MAX_REPLY, + ); + if (!replyLimit.ok) return replyLimit; + this.#sessionLimits = { + requestLimit: requestLimit.value, + replyLimit: replyLimit.value, + }; try { this.#resources.runtimeRoot = await PrivateRuntimeRoot.create({ parent: SESSION_ROOT, @@ -664,34 +733,66 @@ export class HopperClient { }; } - /** Invoke one declared operation, returning timeout and cancellation as values. */ - async callTool( - name: string, + /** + * Invoke one bridge method and validate its reply against that method's + * contract. Timeout, cancellation and a reply outside the contract + * (`HopperReplyContractError`, which keeps the raw reply) return as values. + * `operation` attributes a failure to the analyst operation the method serves. + */ + async callTool( + method: M, arguments_: Readonly> = {}, options: { readonly signal?: AbortSignal; /** Deadline after startup, including waiting for Hopper's serial bridge. */ readonly timeoutMs?: number; readonly progress?: ProgressReporter; + readonly operation?: string; } = {}, - ): Promise> { + ): Promise, AnalysisError>> { const started = await this.start(options.signal); if (!started.ok) return started; - return this.#normalizeToolResult( - name, - await this.#request(name, arguments_, options), + return this.#typedReply( + method, + options.operation ?? method, + await this.#request(method, arguments_, options), ); } /** Dispatch against the connected bridge without invoking startup or retry. */ - callToolIfReady( - name: string, + callToolIfReady( + method: M, arguments_: Readonly> = {}, options: { readonly signal?: AbortSignal } = {}, - ): Promise | undefined> { + ): Promise< + Result, AnalysisError> | undefined + > { if (!this.isReady()) return Promise.resolve(undefined); - return this.#request(name, arguments_, options).then((result) => - this.#normalizeToolResult(name, result), + return this.#request(method, arguments_, options).then((result) => + this.#typedReply(method, method, result), + ); + } + + #typedReply( + method: M, + operation: string, + result: Result, + ): Result, AnalysisError> { + const normalized = this.#normalizeToolResult(method, result); + if (!normalized.ok) return normalized; + if (this.#contracts[method] === undefined) + return err( + new AnalysisCapabilityUnavailableError( + "hopper", + operation, + `No reply contract is installed for the Hopper bridge method ${method}; its reply was not read.`, + ), + ); + return parseHopperReply( + this.#contracts, + method, + normalized.value, + operation, ); } @@ -752,6 +853,17 @@ export class HopperClient { } } + /** + * The ceilings this session admitted at start, for its cleanup requests + * (reauthentication and shutdown): a setting refused after startup must not + * block the cleanup that releases Hopper's document and files. + */ + #cleanupLimits(): { readonly limits?: HopperSessionLimits } { + return this.#sessionLimits === undefined + ? {} + : { limits: this.#sessionLimits }; + } + /** Reauthenticate this run after late readiness or a lost transport. */ async #reconnectForCleanup(): Promise { if (this.#socket?.destroyed === true) this.#releaseTransport(); @@ -804,6 +916,7 @@ export class HopperClient { { signal: controller.signal, timeoutMs: HOPPER_SHUTDOWN_TIMEOUT_MS, + ...this.#cleanupLimits(), }, ); if ( @@ -831,7 +944,14 @@ export class HopperClient { logger: this.#logger, onDiagnostic: this.#options.onDiagnostic, request: (method) => - this.#request(method, {}, { timeoutMs: HOPPER_SHUTDOWN_TIMEOUT_MS }), + this.#request( + method, + {}, + { + timeoutMs: HOPPER_SHUTDOWN_TIMEOUT_MS, + ...this.#cleanupLimits(), + }, + ), retainDocument: options.retainDocument === true, releaseTransport: () => this.#releaseTransport(), }); @@ -1076,6 +1196,8 @@ export class HopperClient { readonly signal?: AbortSignal; readonly timeoutMs?: number; readonly progress?: ProgressReporter; + /** Ceilings already admitted, such as the session's for cleanup. */ + readonly limits?: HopperSessionLimits; }, ): Promise> { const id = this.#nextId++; @@ -1114,22 +1236,37 @@ export class HopperClient { const payloadValue = { id, token, method, params }; // Count escaped JSON before the request can wait in the FIFO. A rejected // payload is not serialized, and an admitted one is serialized only when - // it reaches the wire. + // it reaches the wire. Both ceilings are admitted here, in the caller's + // context; a queued request is written later outside it. + const requestLimit = + options.limits === undefined + ? admitBridgeRequestLimit(this.#options.bridgeMaxRequestBytes) + : ok(options.limits.requestLimit); + if (!requestLimit.ok) return requestLimit; + const replyLimit = + options.limits === undefined + ? admitBridgeReplyLimit( + this.#options.bridgeMaxReplyBytes ?? DEFAULT_BRIDGE_MAX_REPLY, + ) + : ok(options.limits.replyLimit); + if (!replyLimit.ok) return replyLimit; const measured = jsonValueByteLength( payloadValue, - this.#options.bridgeMaxRequestBytes.maximumBytes, + requestLimit.value.maximumBytes, ); - if (measured > this.#options.bridgeMaxRequestBytes.maximumBytes) + if (measured > requestLimit.value.maximumBytes) return err( bridgeRequestOversizeError( method, - this.#options.bridgeMaxRequestBytes, + requestLimit.value, measured, "Hopper", ), ); const startedAt = performance.now(); const result = await this.#requests.run(id, method, params, { + requestLimit: requestLimit.value, + replyLimit: replyLimit.value, ...(options.signal === undefined ? {} : { signal: options.signal }), ...(options.timeoutMs === undefined ? {} @@ -1258,6 +1395,38 @@ export class HopperClient { } } + #abortReplyStream(failure: JsonLineBridgeFailure): void { + switch (failure.kind) { + case "reply_oversize": + this.#failAll( + bridgeReplyOversizeError( + "Hopper", + this.#replyLimit, + failure.observedBytes, + ), + ); + this.#socket?.destroy(); + return; + case "utf8": + this.#abortProtocol( + "Hopper bridge response is not valid UTF-8", + failure.cause, + ); + return; + case "envelope": + this.#abortProtocol(failure.cause.message, failure.cause); + return; + case "unknown_id": + this.#abortProtocol("Hopper returned an unknown response id"); + return; + case "connection_rejected": + this.#abortProtocol( + `Hopper rejected the connection: ${failure.cause.message}`, + failure.cause, + ); + } + } + #abortProtocol(message: string, cause?: Error): void { this.#failAll(new HopperProtocolError(message, { cause })); this.#socket?.destroy(); diff --git a/src/hopper/HopperNameInventory.test.ts b/src/hopper/HopperNameInventory.test.ts index 6416216165..21ec377b3b 100644 --- a/src/hopper/HopperNameInventory.test.ts +++ b/src/hopper/HopperNameInventory.test.ts @@ -6,6 +6,32 @@ import { HopperClient } from "./HopperClient.js"; import { HopperProvider } from "./HopperProvider.js"; import { projectHopperNameInventory } from "./HopperNameInventory.js"; +import { + hopperReplySchemas, + parseHopperReply as parseHopperReplyWith, + type HopperReplyMethod, +} from "./HopperReplyContracts.js"; +import type { JsonValue } from "../domain/jsonValue.js"; + +/** + * Stand in for the client's own reply check: `HopperClient.callTool` validates + * each reply against its method contract before the provider reads it. + */ +const validatedReply = + (reply: JsonValue) => + ( + method: HopperReplyMethod, + _arguments?: unknown, + options?: { readonly operation?: string }, + ) => + Promise.resolve( + parseHopperReplyWith( + hopperReplySchemas, + method, + reply, + options?.operation ?? method, + ), + ); describe("Hopper name inventory labels", () => { it("tags stripped class-object labels without rewriting source values", () => { @@ -148,7 +174,7 @@ describe("Hopper list_names reply contract", () => { if (!config.ok) throw config.error; const callTool = vi .spyOn(HopperClient.prototype, "callTool") - .mockResolvedValue(ok(reply)); + .mockImplementation(validatedReply(reply) as never); const client = new HopperProvider( config.value, silentLogger, diff --git a/src/hopper/HopperProvider.ts b/src/hopper/HopperProvider.ts index 20209792ec..aa9abf3891 100644 --- a/src/hopper/HopperProvider.ts +++ b/src/hopper/HopperProvider.ts @@ -25,7 +25,7 @@ import type { Logger } from "pino"; import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; import type { AnalysisError } from "../domain/analysisErrorBase.js"; import { AnalysisCapabilityUnavailableError } from "../domain/analysisErrorCore.js"; -import { err, ok } from "../domain/result.js"; +import { err, ok, type Result } from "../domain/result.js"; import { AnalysisCleanupLease } from "../artifacts/AnalysisCleanupLease.js"; import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js"; import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; @@ -44,14 +44,17 @@ import { HopperRegexSearch } from "./HopperRegexSearch.js"; import { HopperClient } from "./HopperClient.js"; import { projectHopperNameInventory } from "./HopperNameInventory.js"; import { - hasHopperReplyContract, + HopperReplyContractError, hopperBridgeMethod, - parseHopperReply, + hopperReplyJson, + isHopperOperation, + type HopperReplyMethod, } from "./HopperReplyContracts.js"; import { hopperMachOImageSchema } from "./HopperMachOImage.js"; import { reconcileHopperCallers, reconciliationChildOptions, + type HopperCallerNativeReply, } from "./HopperCallerReconciliation.js"; import { analysisClientFailureForOperation } from "../providers/AnalysisClientFailure.js"; @@ -334,6 +337,7 @@ export class HopperProvider implements AnalysisProviderCandidate { readStartupTimeoutMs: () => this.config.hopperStartupTimeoutMs, readStartupTimeoutSource: () => this.config.hopperStartupTimeoutSource, bridgeMaxRequestBytes: this.config.bridgeMaxRequestBytes, + bridgeMaxReplyBytes: this.config.bridgeMaxReplyBytes, logger: this.logger.child({ layer: "bridge" }), onDiagnostic: (diagnostic) => this.logger.info(diagnostic, "Hopper bridge reported a diagnostic"), @@ -416,102 +420,114 @@ export class HopperProvider implements AnalysisProviderCandidate { execute: async (operation, parameters, options) => { try { return await sourceReads.run(async () => { + if (!isHopperOperation(operation)) + return err( + new AnalysisCapabilityUnavailableError( + "hopper", + operation, + "Hopper does not serve this operation.", + ), + ); const willReconcile = operation === "procedure_callers" || operation === "analyze_function" || (operation === "procedure_references" && parameters.direction === "incoming"); - const isRegexSearch = + const callOptions = { + ...(willReconcile + ? reconciliationChildOptions(options) + : options), + operation, + }; + // The client validates each reply against its method contract. + const call = (method: M) => + client.callTool(method, parameters, callOptions); + if (operation === "list_names") { + const names = await call("list_names"); + if (!names.ok) return names; + const inventory = projectHopperNameInventory(names.value); + return ok( + projectExecution( + operation, + inventory.result, + inventory.changed ? names.value : null, + ), + ); + } + if (operation === "address_to_file_offset") { + const offset = await call("address_to_file_offset"); + if (!offset.ok) return offset; + const mapped = await mapHopperFileOffset(target, offset.value, { + ...(options?.signal === undefined + ? {} + : { signal: options.signal }), + retainFailedClose: retainSourceDescriptor, + onCleanupMarker: (marker) => { + if (!(marker.cause instanceof RegularFileCleanupFailure)) + return; + sourceLeases + .get(marker.cause.owner) + ?.bindCleanupError(marker); + }, + }); + if (!mapped.ok) return mapped; + return ok( + projectExecution(operation, mapped.value, offset.value), + ); + } + if (willReconcile) { + const native = + operation === "procedure_callers" + ? await call("rea_caller_base") + : operation === "analyze_function" + ? await call("analyze_function") + : await call("procedure_references"); + // A reply outside its contract keeps its raw reply as partial + // reconciliation Evidence; other failures return directly. + let nativeReply: Result< + HopperCallerNativeReply, + HopperReplyContractError + >; + if (native.ok) nativeReply = native; + else if (native.error instanceof HopperReplyContractError) + nativeReply = err(native.error); + else + return operation === "procedure_callers" + ? err( + analysisClientFailureForOperation( + native.error, + "hopper", + operation, + ), + ) + : native; + const reconciled = await reconcileHopperCallers( + client, + operation, + parameters, + nativeReply, + options, + this.config.hopperCallerScanBudget, + ); + if (!reconciled.ok) return reconciled; + return ok( + projectExecution( + operation, + reconciled.value.result, + reconciled.value.rawResult, + reconciled.value.reason, + ), + ); + } + const result = (operation === "search_strings" || operation === "search_procedures") && - parameters.mode === "regex"; - const result = isRegexSearch - ? await regexSearch.execute(operation, parameters, options) - : await client.callTool( - hopperBridgeMethod(operation), - parameters, - willReconcile ? reconciliationChildOptions(options) : options, - ); - if (!result.ok) - return operation === "procedure_callers" - ? err( - analysisClientFailureForOperation( - result.error, - "hopper", - operation, - ), - ) - : result; - // Each bridge reply is validated once against its method contract. - // Reconciliation validates the replies it reads, and regex search - // rows come from an inventory validated when it was read. - const names = - operation === "list_names" - ? parseHopperReply(operation, result.value) - : undefined; - if (names !== undefined && !names.ok) return names; - const offset = - operation === "address_to_file_offset" - ? parseHopperReply(operation, result.value) - : undefined; - if (offset !== undefined && !offset.ok) return offset; - if ( - names === undefined && - offset === undefined && - !willReconcile && - !isRegexSearch && - hasHopperReplyContract(operation) - ) { - const replied = parseHopperReply(operation, result.value); - if (!replied.ok) return replied; - } - const mapped = - offset !== undefined - ? await mapHopperFileOffset(target, offset.value, { - ...(options?.signal === undefined - ? {} - : { signal: options.signal }), - retainFailedClose: retainSourceDescriptor, - onCleanupMarker: (marker) => { - if (!(marker.cause instanceof RegularFileCleanupFailure)) - return; - sourceLeases - .get(marker.cause.owner) - ?.bindCleanupError(marker); - }, - }) - : result; - if (!mapped.ok) return mapped; - const reconciled = - operation === "procedure_callers" || - operation === "analyze_function" || - (operation === "procedure_references" && - parameters.direction === "incoming") - ? await reconcileHopperCallers( - client, - operation, - parameters, - mapped.value, - options, - this.config.hopperCallerScanBudget, - ) - : undefined; - if (reconciled !== undefined && !reconciled.ok) return reconciled; - const reconciledResult = reconciled?.value.result ?? mapped.value; - const inventory = - names === undefined - ? { result: reconciledResult, changed: false } - : projectHopperNameInventory(names.value); + parameters.mode === "regex" + ? await regexSearch.execute(operation, parameters, options) + : await call(hopperBridgeMethod(operation)); + if (!result.ok) return result; return ok( - projectExecution( - operation, - inventory.result, - reconciled?.value.rawResult ?? - (inventory.changed || mapped !== result - ? result.value - : null), - reconciled?.value.reason, - ), + projectExecution(operation, hopperReplyJson(result.value)), ); }); } catch (cause: unknown) { @@ -528,10 +544,6 @@ export class HopperProvider implements AnalysisProviderCandidate { options?.signal === undefined ? {} : { signal: options.signal }, ); if (result === undefined || !result.ok) return result; - if (hasHopperReplyContract(operation)) { - const replied = parseHopperReply(operation, result.value); - if (!replied.ok) return replied; - } return ok(projectExecution(operation, result.value)); }, runtimeLineageSnapshots: () => { diff --git a/src/hopper/HopperRegexSearch.test.ts b/src/hopper/HopperRegexSearch.test.ts index 5ba6467836..7fe5ac194d 100644 --- a/src/hopper/HopperRegexSearch.test.ts +++ b/src/hopper/HopperRegexSearch.test.ts @@ -1,5 +1,9 @@ import { expect, it, vi } from "vitest"; import { HopperRegexSearch } from "./HopperRegexSearch.js"; +import { + hopperReplySchemas, + parseHopperReply, +} from "./HopperReplyContracts.js"; import { err, ok } from "../domain/result.js"; import { AnalysisCapabilityUnavailableError, @@ -104,17 +108,23 @@ it.each(["search_strings", "search_procedures"] as const)( it.each(["search_strings", "search_procedures"] as const)( "rejects malformed canonical metadata in the %s inventory", async (operation) => { + // The client validates each reply against its method contract. const search = new HopperRegexSearch({ - callTool: async () => - ok([ - { - address: "0x1", - value: "matched", - ...(operation === "search_strings" - ? { string: { encoding: false } } - : { procedure: { external: "unknown" } }), - }, - ]), + callTool: async (method, _arguments, options) => + parseHopperReply( + hopperReplySchemas, + method, + [ + { + address: "0x1", + value: "matched", + ...(operation === "search_strings" + ? { string: { encoding: false } } + : { procedure: { external: "unknown" } }), + }, + ], + options?.operation ?? method, + ), }); try { expect( diff --git a/src/hopper/HopperRegexSearch.ts b/src/hopper/HopperRegexSearch.ts index ed56f6d484..73dac17d54 100644 --- a/src/hopper/HopperRegexSearch.ts +++ b/src/hopper/HopperRegexSearch.ts @@ -8,7 +8,10 @@ import type { JsonValue } from "../domain/jsonValue.js"; import { err, ok, type Result } from "../domain/result.js"; import { nativeSearchInput } from "../domain/native/nativeSearch.js"; import type { HopperClient } from "./HopperClient.js"; -import { parseHopperReply } from "./HopperReplyContracts.js"; +import { + HopperReplyContractError, + type HopperReply, +} from "./HopperReplyContracts.js"; import { DEFAULT_REGEX_MATCHING_DEADLINE, type RegexMatchingDeadline, @@ -25,6 +28,20 @@ const inputSchema = z.strictObject({ mode: z.literal("regex"), }); /** Match collected native facts off Hopper's API thread, with cancellable lifetime. */ +/** The client capability regex search needs: one validated inventory read. */ +export interface HopperInventoryReader { + callTool( + method: "list_strings" | "list_procedures", + arguments_: Readonly>, + options: Parameters[2], + ): Promise< + Result< + HopperReply<"list_strings"> | HopperReply<"list_procedures">, + AnalysisError + > + >; +} + export class HopperRegexSearch { readonly #active = new Map< AbortController, @@ -32,7 +49,7 @@ export class HopperRegexSearch { >(); constructor( - private readonly client: Pick, + private readonly client: HopperInventoryReader, private readonly deadline: RegexMatchingDeadline = DEFAULT_REGEX_MATCHING_DEADLINE, private readonly heapRequest: RegexWorkerHeapRequest = DEFAULT_REGEX_WORKER_HEAP_REQUEST, ) {} @@ -98,23 +115,26 @@ export class HopperRegexSearch { }, }), }; - const inventory = await this.client.callTool( - operation === "search_strings" ? "list_strings" : "list_procedures", + const inventoryArguments = input.data.document === undefined ? {} - : { document: input.data.document }, - inventoryOptions, + : { document: input.data.document }; + const inventory = await this.client.callTool( + operation === "search_strings" ? "list_strings" : "list_procedures", + inventoryArguments, + { ...inventoryOptions, operation }, ); if (!inventory.ok) return err( - analysisClientFailureForOperation(inventory.error, "hopper", operation), + inventory.error instanceof HopperReplyContractError + ? inventory.error + : analysisClientFailureForOperation( + inventory.error, + "hopper", + operation, + ), ); - const parsed = - operation === "search_strings" - ? parseHopperReply("list_strings", inventory.value, operation) - : parseHopperReply("list_procedures", inventory.value, operation); - if (!parsed.ok) return parsed; - const rows: readonly (typeof parsed.value)[number][] = parsed.value; + const rows: readonly { readonly value: string }[] = inventory.value; if (options.signal?.aborted) return err(cancelledBy(operation, options.signal)); await options.progress diff --git a/src/hopper/HopperReplyContracts.test.ts b/src/hopper/HopperReplyContracts.test.ts index 162aae1c73..126b3bcf46 100644 --- a/src/hopper/HopperReplyContracts.test.ts +++ b/src/hopper/HopperReplyContracts.test.ts @@ -1,3 +1,4 @@ +import { z } from "zod"; import { describe, expect, it, vi } from "vitest"; import { parseConfig } from "../config/parseConfig.js"; @@ -9,16 +10,44 @@ import { HopperClient } from "./HopperClient.js"; import { HopperProvider } from "./HopperProvider.js"; import { HOPPER_OPERATIONS } from "./HopperProviderMetadata.js"; import { - hasHopperReplyContract, hopperBridgeMethod, - parseHopperReply, + parseHopperReply as parseHopperReplyWith, + HopperReplyContractError, + hopperReplySchemas, + type HopperReplyMethod, parseHopperServerInfo, parseHopperShutdownReply, } from "./HopperReplyContracts.js"; +/** + * Stand in for the client's own reply check: `HopperClient.callTool` validates + * each reply against its method contract before the provider reads it. + */ +const validatedReply = + (reply: JsonValue) => + ( + method: HopperReplyMethod, + _arguments?: unknown, + options?: { readonly operation?: string }, + ) => + Promise.resolve( + parseHopperReplyWith( + hopperReplySchemas, + method, + reply, + options?.operation ?? method, + ), + ); + +const parseHopperReply = ( + method: M, + value: JsonValue, + operation: string = method, +) => parseHopperReplyWith(hopperReplySchemas, method, value, operation); + it("gives every Hopper operation's bridge method a reply contract", () => { const missing = HOPPER_OPERATIONS.map(hopperBridgeMethod).filter( - (method) => !hasHopperReplyContract(method), + (method) => !Object.hasOwn(hopperReplySchemas, method), ); expect(missing, "no Hopper reply reaches Evidence unvalidated").toEqual([]); @@ -111,10 +140,20 @@ it("names the requesting operation and the bridge method in a contract failure", expect(parsed.ok ? undefined : parsed.error).toMatchObject({ _tag: "AnalysisOutputError", operation: "search_procedures", + method: "list_procedures", message: expect.stringContaining("list_procedures contract"), }); }); +it("keeps the raw reply on a contract failure as partial evidence", () => { + const reply = [{ address: "0x10" }]; + const parsed = parseHopperReply("list_procedures", reply); + + if (parsed.ok) throw new Error("Expected a contract failure"); + expect(parsed.error).toBeInstanceOf(HopperReplyContractError); + expect(parsed.error.reply).toBe(reply); +}); + it("names each failing field without echoing reported text", () => { const region = { name: "secret-name", @@ -227,7 +266,9 @@ describe("provider entry paths apply the reply contract", () => { "unlisted source fields survive", ).toEqual(valid); - callTool.mockResolvedValueOnce(ok(structuredClone(malformed))); + callTool.mockImplementationOnce( + validatedReply(structuredClone(malformed)) as never, + ); const rejected = await client.execute(operation, parameters); expect(rejected.ok ? undefined : rejected.error).toMatchObject({ _tag: "AnalysisOutputError", @@ -248,7 +289,7 @@ describe("provider entry paths apply the reply contract", () => { async (operation) => { const ready = vi .spyOn(HopperClient.prototype, "callToolIfReady") - .mockResolvedValue(ok([{ unexpected: true }])); + .mockImplementation(validatedReply([{ unexpected: true }]) as never); const client = providerClient(); try { const result = await client.executeIfReady?.(operation, {}); @@ -267,6 +308,19 @@ describe("provider entry paths apply the reply contract", () => { ); }); +it("refuses, at compile time, a contract that would change the reply", () => { + const coercing = { count: z.object({ count: z.coerce.number() }) }; + const defaulted = { label: z.object({ label: z.string().default("x") }) }; + // @ts-expect-error A coercion promises a number the raw reply does not hold. + const coerced = parseHopperReplyWith(coercing, "count", { count: "1" }, "op"); + // @ts-expect-error A default promises a field the raw reply may lack. + const filled = parseHopperReplyWith(defaulted, "label", {}, "op"); + + // The raw reply is returned, which is why such contracts are refused. + expect(coerced).toEqual({ ok: true, value: { count: "1" } }); + expect(filled).toEqual({ ok: true, value: {} }); +}); + describe("Hopper health reply", () => { const health = { name: "REA Hopper bridge", diff --git a/src/hopper/HopperReplyContracts.ts b/src/hopper/HopperReplyContracts.ts index e4bbe6bceb..ba37e444ad 100644 --- a/src/hopper/HopperReplyContracts.ts +++ b/src/hopper/HopperReplyContracts.ts @@ -26,12 +26,25 @@ import { procedureInfoSchema, } from "../domain/native/procedureInfo.js"; import { err, ok, type Result } from "../domain/result.js"; -import type { HOPPER_OPERATIONS } from "./HopperProviderMetadata.js"; +import { HOPPER_OPERATIONS } from "./HopperProviderMetadata.js"; -type HopperOperation = (typeof HOPPER_OPERATIONS)[number]; +/** Every operation the provider dispatches to the bridge, including `health`. */ +export type HopperOperation = (typeof HOPPER_OPERATIONS)[number] | "health"; + +const hopperOperations: ReadonlySet = new Set([ + ...HOPPER_OPERATIONS, + "health", +]); + +/** Whether Hopper serves this operation. */ +export const isHopperOperation = ( + operation: string, +): operation is HopperOperation => hopperOperations.has(operation); /** The bridge method that serves an analyst operation. */ -export const hopperBridgeMethod = (operation: string): string => +export const hopperBridgeMethod = ( + operation: HopperOperation, +): HopperReplyMethod => operation === "procedure_callers" ? "rea_caller_base" : operation; /** Every bridge method whose reply REA reads. */ @@ -169,40 +182,93 @@ export const hopperReplySchemas = { }), } as const satisfies Record; -export type HopperReplyMethod = keyof typeof hopperReplySchemas; +/** The production table's type; `HopperClient` defaults to it. */ +export type HopperBridgeReplyContracts = typeof hopperReplySchemas; +export type HopperReplyMethod = keyof HopperBridgeReplyContracts; export type HopperReply = z.output< (typeof hopperReplySchemas)[M] >; -export const hasHopperReplyContract = ( - method: string, -): method is HopperReplyMethod => Object.hasOwn(hopperReplySchemas, method); +/** + * A validated reply as JSON. `parseHopperReply` returns the bridge's decoded + * JSON value unchanged, so every reply is a `JsonValue`; zod's output types + * only spell optional members in a way `JsonValue` does not. + */ +export const hopperReplyJson = ( + reply: HopperReply | JsonValue, +): JsonValue => reply as JsonValue; + +/** A method-keyed reply contract table, such as `hopperReplySchemas`. */ +export type HopperReplyContracts = Readonly>; + +/** + * A table whose contracts validate a reply without changing it: each + * schema's input type is its output type. `parseHopperReply` returns the raw + * reply, so a coercion, default or transform would promise a type the reply + * does not have; such a schema fails to type-check here. + */ +export type ValueKeepingContracts = { + readonly [M in keyof Contracts]: [z.input] extends [ + z.output, + ] + ? Contracts[M] + : never; +}; + +/** The production table, checked to keep every reply value unchanged. */ +export const hopperBridgeReplyContracts: ValueKeepingContracts = + hopperReplySchemas; + +/** The validated reply a contract table assigns to one method. */ +export type HopperContractReply< + Contracts extends HopperReplyContracts, + M extends keyof Contracts, +> = z.output; + +/** + * A bridge reply outside its method's contract. `reply` is the reply exactly + * as the bridge sent it, kept as partial evidence for callers that retain it. + */ +export class HopperReplyContractError extends AnalysisOutputError { + constructor( + operation: string, + readonly method: string, + readonly reply: JsonValue, + cause: z.ZodError, + ) { + super( + operation, + `Hopper bridge output did not match the ${method} contract`, + { + cause, + diagnostics: outputContractDiagnostics(cause, reply), + }, + ); + } +} /** * Validate one bridge reply against its method's contract. The source value is * returned unchanged, so Evidence keeps every reported field; a reply outside - * the contract is an output failure, never passed through. + * the contract is an output failure, never passed through. Only the client + * and this module's lifecycle readers call this; every other consumer receives + * the typed reply. */ -export const parseHopperReply = ( +export const parseHopperReply = < + Contracts extends HopperReplyContracts, + M extends keyof Contracts & string, +>( + contracts: Contracts & ValueKeepingContracts, method: M, value: JsonValue, - operation: string = method, -): Result, AnalysisOutputError> => { - const parsed = hopperReplySchemas[method].safeParse(value); + operation: string, +): Result, HopperReplyContractError> => { + const parsed = contracts[method]!.safeParse(value); return parsed.success ? // The schemas carry no transforms, so the validated source is the // parsed value without the object copy that drops unlisted fields. - ok(value as HopperReply) - : err( - new AnalysisOutputError( - operation, - `Hopper bridge output did not match the ${method} contract`, - { - cause: parsed.error, - diagnostics: outputContractDiagnostics(parsed.error, value), - }, - ), - ); + ok(value as HopperContractReply) + : err(new HopperReplyContractError(operation, method, value, parsed.error)); }; /** @@ -219,7 +285,7 @@ export const parseHopperShutdownReply = ( "confirmed" | "cleanup_required" | "unconfirmed", AnalysisOutputError > => { - const reply = parseHopperReply(method, value); + const reply = parseHopperReply(hopperReplySchemas, method, value, method); if (!reply.ok) return reply; const { analysis_stopped, document_closed, document_retained } = reply.value; if (reply.value.cleanup_required === true) return ok("cleanup_required"); @@ -248,7 +314,7 @@ export const parseHopperServerInfo = ( value: JsonValue, expectedRunId: string, ): Result => { - const reply = parseHopperReply("health", value); + const reply = parseHopperReply(hopperReplySchemas, "health", value, "health"); if (!reply.ok || reply.value.run_id !== expectedRunId) return err( new HopperProtocolError( diff --git a/src/hopper/HopperRequestQueue.ts b/src/hopper/HopperRequestQueue.ts index d072232104..8b49ed7ff6 100644 --- a/src/hopper/HopperRequestQueue.ts +++ b/src/hopper/HopperRequestQueue.ts @@ -13,6 +13,8 @@ import { HopperRemoteError, HopperTimeoutError, } from "../domain/hopperErrors.js"; +import type { BridgeMaxReplyBytes } from "../config/bridgeMaxReplyBytes.js"; +import type { BridgeMaxRequestBytes } from "../config/bridgeMaxRequestBytes.js"; import type { JsonValue } from "../domain/jsonValue.js"; import { err, type Result } from "../domain/result.js"; @@ -23,6 +25,14 @@ interface HopperRequestQueueOptions { /** Caller deadline from admission, including time waiting in the FIFO. */ readonly timeoutMs?: number; readonly progress?: ProgressReporter; + /** + * Ceilings admitted with this request in the caller's context. They are + * handed to the sender only when the request reaches the wire, so queued + * work cannot change the ceiling of the request Hopper is answering, and a + * write outside the caller's context never reads the settings again. + */ + readonly requestLimit?: BridgeMaxRequestBytes; + readonly replyLimit?: BridgeMaxReplyBytes; } export interface HopperRequestActivity { @@ -38,6 +48,8 @@ type RequestSender = ( readonly id: number; readonly method: string; readonly params: JsonValue; + readonly requestLimit: BridgeMaxRequestBytes | undefined; + readonly replyLimit: BridgeMaxReplyBytes | undefined; }, failed: () => void, ) => void; @@ -46,6 +58,8 @@ interface QueuedRequest { readonly id: number; readonly method: string; readonly params: JsonValue; + readonly requestLimit: BridgeMaxRequestBytes | undefined; + readonly replyLimit: BridgeMaxReplyBytes | undefined; readonly signal: AbortSignal | undefined; readonly progress: ProgressReporter | undefined; readonly resolve: (result: HopperRequestResult) => void; @@ -110,6 +124,8 @@ export class HopperRequestQueue { id, method, params, + requestLimit: options.requestLimit, + replyLimit: options.replyLimit, signal: options.signal, progress: options.progress, resolve, @@ -180,6 +196,16 @@ export class HopperRequestQueue { return true; } + /** + * Fail the request on the wire with a rejection the bridge could not + * attribute by id. False when no request is on the wire. + */ + failActive(error: HopperError): boolean { + const active = this.#active; + if (active === undefined || !active.transmitted) return false; + return this.accept(active.id, err(error)); + } + /** Fail the active wire request and every request still waiting in the FIFO. */ failAll(error: HopperError): void { const active = this.#active; @@ -269,6 +295,8 @@ export class HopperRequestQueue { id: entry.id, method: entry.method, params: entry.params, + requestLimit: entry.requestLimit, + replyLimit: entry.replyLimit, }, () => this.accept( diff --git a/src/hopper/HopperResponseStream.test.ts b/src/hopper/HopperResponseStream.test.ts deleted file mode 100644 index d72b6626c2..0000000000 --- a/src/hopper/HopperResponseStream.test.ts +++ /dev/null @@ -1,76 +0,0 @@ -import { describe, expect, it, vi } from "vitest"; - -import { HopperResponseStream } from "./HopperResponseStream.js"; - -describe("Hopper response stream", () => { - it("stops on malformed input and resumes with buffered response lines", () => { - const accepted: number[] = []; - const abort = vi.fn(); - const stream = new HopperResponseStream({ - accept: (message) => { - accepted.push(message.id); - return true; - }, - hasQueued: () => false, - nextRequestId: () => 4, - abort, - }); - - stream.push(Buffer.from('not json\n{"id":2,"result":null}\n')); - expect(accepted).toEqual([]); - expect(abort).toHaveBeenCalledOnce(); - - stream.push(Buffer.from('{"id":3')); - expect(accepted).toEqual([2]); - stream.push(Buffer.from(',"result":null}\n')); - expect(accepted).toEqual([2, 3]); - expect(abort).toHaveBeenCalledOnce(); - }); - - it("stops on a future response id and retains later complete lines", () => { - const accepted: number[] = []; - const abort = vi.fn(); - const stream = new HopperResponseStream({ - accept: (message) => { - if (message.id >= 3) return false; - accepted.push(message.id); - return true; - }, - hasQueued: () => false, - nextRequestId: () => 3, - abort, - }); - - stream.push( - Buffer.from('{"id":3,"result":null}\n{"id":1,"result":null}\n'), - ); - expect(accepted).toEqual([]); - expect(abort).toHaveBeenCalledWith( - "Hopper returned an unknown response id", - ); - - stream.push(Buffer.from('{"id":2')); - expect(accepted).toEqual([1]); - stream.push(Buffer.from(',"result":null}\n')); - expect(accepted).toEqual([1, 2]); - expect(abort).toHaveBeenCalledOnce(); - }); - - it("discards incomplete lines on reset", () => { - const accepted: number[] = []; - const stream = new HopperResponseStream({ - accept: (message) => { - accepted.push(message.id); - return true; - }, - hasQueued: () => false, - nextRequestId: () => 2, - abort: vi.fn(), - }); - - stream.push(Buffer.from('{"id":1,"result":null}')); - stream.reset(); - stream.push(Buffer.from('{"id":1,"result":null}\n')); - expect(accepted).toEqual([1]); - }); -}); diff --git a/src/hopper/HopperResponseStream.ts b/src/hopper/HopperResponseStream.ts deleted file mode 100644 index 69dc35b96a..0000000000 --- a/src/hopper/HopperResponseStream.ts +++ /dev/null @@ -1,57 +0,0 @@ -import { - parseBridgeMessageLine, - type HopperBridgeMessage, -} from "./protocol.js"; -import { Utf8NdjsonLineBuffer } from "../process/Utf8NdjsonLineBuffer.js"; - -interface HopperResponseStreamOptions { - readonly accept: (message: HopperBridgeMessage) => boolean; - readonly hasQueued: (id: number) => boolean; - readonly nextRequestId: () => number; - readonly abort: (message: string, cause?: Error) => void; -} - -const NOT_UTF8 = "Hopper bridge response is not valid UTF-8"; - -/** Incrementally validates Hopper NDJSON response frames. */ -export class HopperResponseStream { - readonly #lines = new Utf8NdjsonLineBuffer(); - - constructor(readonly options: HopperResponseStreamOptions) {} - - push(chunk: Buffer): void { - for (const line of this.#lines.push(chunk)) { - if (!line.ok) { - this.options.abort(NOT_UTF8, line.error); - break; - } - if (!this.#acceptLine(line.value)) break; - } - } - - finish(): void { - const tail = this.#lines.finish(); - if (!tail.ok) this.options.abort(NOT_UTF8, tail.error); - } - - reset(): void { - this.#lines.reset(); - } - - #acceptLine(line: string): boolean { - const parsed = parseBridgeMessageLine(line); - if (!parsed.ok) { - this.options.abort(parsed.error.message, parsed.error); - return false; - } - if (this.options.accept(parsed.value)) return true; - if ( - parsed.value.id >= this.options.nextRequestId() || - this.options.hasQueued(parsed.value.id) - ) { - this.options.abort("Hopper returned an unknown response id"); - return false; - } - return true; - } -} diff --git a/src/hopper/protocol.ts b/src/hopper/protocol.ts index 5f36f5d888..e1e8143d98 100644 --- a/src/hopper/protocol.ts +++ b/src/hopper/protocol.ts @@ -7,6 +7,7 @@ import { import { ownedJsonValueSchema, type JsonValue } from "../domain/jsonValue.js"; import { err, ok, type Result } from "../domain/result.js"; import { safeParseJson } from "../domain/safeJson.js"; +import type { JsonLineBridgeEnvelope } from "../process/JsonLineBridgeReader.js"; const remoteErrorSchema = z .object({ @@ -83,7 +84,7 @@ export type HopperBridgeEvent = z.infer; export type HopperBridgeMessage = z.infer; /** Parse one complete Hopper NDJSON response or event line. */ -export const parseBridgeMessageLine = ( +const parseBridgeMessageLine = ( line: string, ): Result => { const decodedResult = safeParseJson(line); @@ -106,14 +107,28 @@ export const parseBridgeMessageLine = ( ); }; +const hopperRemoteError = ( + error: z.infer, +): HopperRemoteError => + new HopperRemoteError(error.code, error.message, { + diagnosticType: error.type, + }); + /** Project a parsed response into its result or expected remote failure. */ export const responseResult = ( response: HopperResponse, ): Result => "error" in response - ? err( - new HopperRemoteError(response.error.code, response.error.message, { - diagnosticType: response.error.type, - }), - ) + ? err(hopperRemoteError(response.error)) : ok(response.result); + +/** Hopper's `result | error` plus events envelope for the shared JSON-line reader. */ +export const hopperReplyEnvelope: JsonLineBridgeEnvelope< + HopperBridgeMessage, + HopperRemoteError +> = { + parse: parseBridgeMessageLine, + id: (message) => message.id, + remoteFailure: (message) => + "error" in message ? hopperRemoteError(message.error) : undefined, +}; diff --git a/src/kuna/KunaCopy.test.ts b/src/kuna/KunaCopy.test.ts index f89cba940b..d5d5973f5b 100644 --- a/src/kuna/KunaCopy.test.ts +++ b/src/kuna/KunaCopy.test.ts @@ -17,6 +17,7 @@ import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js"; import { copyAndHash, kunaCopyFailureFacts } from "./KunaCommand.js"; import { createTestTempDirectory } from "../../tests/fixtures/temporaryDirectory.js"; import { awaitAcknowledgement } from "../../tests/fixtures/lifecycleAcknowledgement.js"; +import { withKunaProjectDeadline } from "../../tests/boundary/providers/kuna/kunaProjectRequest.js"; vi.mock("node:fs/promises", { spy: true }); @@ -305,11 +306,13 @@ it("keeps staging until both copy descriptors close, then retires the exact work await cache.close(); vi.restoreAllMocks(); }); - const built = await cache.build({ - installation, - targetPath, - targetSha256: createHash("sha256").update("owned input").digest("hex"), - }); + const built = await cache.build( + withKunaProjectDeadline({ + installation, + targetPath, + targetSha256: createHash("sha256").update("owned input").digest("hex"), + }), + ); if (built.ok) throw new Error( "Expected copy descriptor cleanup failure before any engine call", diff --git a/src/kuna/KunaProjectCache.ts b/src/kuna/KunaProjectCache.ts index 20b3439281..b82f2e61c2 100644 --- a/src/kuna/KunaProjectCache.ts +++ b/src/kuna/KunaProjectCache.ts @@ -241,8 +241,8 @@ export interface KunaProjectRequest { readonly targetSha256: string; readonly signal?: AbortSignal; readonly onStatus?: (status: KunaExportStatus) => void; - /** Deadline accepted by the session that started this export. */ - readonly deadline?: KunaDeadline; + /** Deadline accepted by the operation that started this export. */ + readonly deadline: KunaDeadline; } /** One build whose process tree REA could not prove stopped. */ @@ -1082,7 +1082,7 @@ export class KunaProjectCache { copyResources, retainFailedFileClose: (owner, resource) => this.#retainHashFile(owner, key, staging, resource), - ...(request.deadline === undefined ? {} : { deadline: request.deadline }), + deadline: request.deadline, }); if (outcome.kind === "retained") { const retained: RetainedProcess = { diff --git a/src/kuna/KunaProjectExport.ts b/src/kuna/KunaProjectExport.ts index f63646f21c..5162518769 100644 --- a/src/kuna/KunaProjectExport.ts +++ b/src/kuna/KunaProjectExport.ts @@ -43,11 +43,7 @@ import { runKunaCommand, type KunaProcessLauncher, } from "./KunaCommand.js"; -import { - kunaProjectDeadline, - type KunaConfiguration, - type KunaDeadline, -} from "./KunaConfiguration.js"; +import type { KunaDeadline } from "./KunaConfiguration.js"; import type { KunaInstallation } from "./KunaInstallation.js"; import { KUNA_DIAGNOSTIC_BYTES } from "./KunaLimits.js"; import { @@ -67,12 +63,6 @@ import { /** Operation name every export and cache failure reports. */ export const KUNA_EXPORT_OPERATION = "kuna:decompile-project"; -/** The deadline an export already accepted, otherwise the current snapshot. */ -export const acceptedExportDeadline = ( - configuration: KunaConfiguration, - retained?: KunaDeadline, -): KunaDeadline => retained ?? kunaProjectDeadline(configuration); - const INVENTORY_FILE = "functions.json"; /** Exact flat file layout emitted for the selected target's native basename. */ @@ -160,7 +150,7 @@ export interface KunaExportRequest { readonly onStatus: (status: KunaExportStatus) => void; readonly launcher?: KunaProcessLauncher | undefined; /** Deadline accepted when the export started. Later setting changes do not rewrite it. */ - readonly deadline?: KunaDeadline; + readonly deadline: KunaDeadline; } const message = (cause: unknown): string => @@ -283,11 +273,7 @@ export const exportKunaProject = async ( request: KunaExportRequest, ): Promise => { const { installation, signal } = request; - const { configuration } = installation; - const acceptedDeadline = acceptedExportDeadline( - configuration, - request.deadline, - ); + const acceptedDeadline = request.deadline; const deadline = Deadline.after(acceptedDeadline.ms); const input = join(request.workspace, "input", basename(request.targetPath)); await mkdir(join(request.workspace, "input"), { mode: 0o700 }); @@ -365,7 +351,7 @@ export const exportKunaProject = async ( "-o", request.output, "--jobs", - String(configuration.jobs), + String(installation.configuration.jobs), ], // Kuna prints one summary line per export on stdout. KUNA_DIAGNOSTIC_BYTES, diff --git a/src/kuna/KunaReferenceQueries.ts b/src/kuna/KunaReferenceQueries.ts index 6ad204d989..5a64e944c8 100644 --- a/src/kuna/KunaReferenceQueries.ts +++ b/src/kuna/KunaReferenceQueries.ts @@ -1,3 +1,4 @@ +import { readRuntimeSetting } from "../config/runtimeAnalysisSettings.js"; import type { AnalysisError } from "../domain/analysisErrorBase.js"; import { AnalysisResourceConstraintError } from "../domain/analysisErrorCore.js"; import type { JsonValue } from "../domain/jsonValue.js"; @@ -101,6 +102,11 @@ export class KunaReferenceQueries { return this.#loadGraph(installation, operation, signal); } if (this.#pendingGraph === undefined) { + // Read in the requesting operation's snapshot, refused as a value. + const deadline = readRuntimeSetting(() => + kunaProjectDeadline(installation.configuration), + ); + if (!deadline.ok) return deadline; const pending = this.work.start( (producerSignal) => this.work.scan( @@ -113,7 +119,7 @@ export class KunaReferenceQueries { "--functions", "", ], - deadline: kunaProjectDeadline(installation.configuration), + deadline: deadline.value, }, new KunaCallGraphReader(), producerSignal, @@ -250,13 +256,17 @@ export class KunaReferenceQueries { { measuredExhaustion: true }, ), ); + const deadline = readRuntimeSetting(() => + kunaProjectDeadline(installation.configuration), + ); + if (!deadline.ok) return deadline; pending = this.work.start( async (producerSignal) => { const run = await this.work.run({ installation, operation: "xrefs", arguments: (input) => ["xrefs", input, "--to", address, "--json"], - deadline: kunaProjectDeadline(installation.configuration), + deadline: deadline.value, outputBytes: KUNA_XREF_OUTPUT_BYTES, signal: producerSignal, }); diff --git a/src/kuna/KunaSessionClient.ts b/src/kuna/KunaSessionClient.ts index c0cc9b808b..8f83c4dcf8 100644 --- a/src/kuna/KunaSessionClient.ts +++ b/src/kuna/KunaSessionClient.ts @@ -1,3 +1,5 @@ +import { readRuntimeSetting } from "../config/runtimeAnalysisSettings.js"; +import { ConfigurationError } from "../domain/configurationErrors.js"; import { snapshotRegexConfiguration, type RegexConfigurationSnapshot, @@ -24,7 +26,6 @@ import { type RetainedResourceScope, } from "../application/AnalysisProvider.js"; import { AnalysisError } from "../domain/analysisErrorBase.js"; -import { ConfigurationError } from "../domain/configurationErrors.js"; import { AnalysisCancelledError, AnalysisCapabilityUnavailableError, @@ -591,8 +592,10 @@ export class KunaSessionClient implements AnalysisClient { parameters.analysis !== "independent_fresh") || operation === "procedure_assembly" || operation === "analyze_function" - ) - await this.#startProject(installation.value); + ) { + const project = await this.#startProject(installation.value); + if (!project.ok) return project; + } const answer = await this.#answer( operation, parameters, @@ -1421,7 +1424,8 @@ export class KunaSessionClient implements AnalysisClient { : kunaEntryLimitations(index.layout, placement), }); } - await this.#startProject(installation); + const project = await this.#startProject(installation); + if (!project.ok) return project; return this.#withProject( "address_name", installation, @@ -1476,7 +1480,8 @@ export class KunaSessionClient implements AnalysisClient { if (!isUnavailableProject(cause) || recovered) throw cause; recovered = true; this.#invalidateProject(reader); - await this.#startProject(installation); + const project = await this.#startProject(installation); + if (!project.ok) return project; continue; } } @@ -1658,13 +1663,20 @@ export class KunaSessionClient implements AnalysisClient { admitted(); return err(cancelledBy(operation, signal)); } - let deadline: KunaDeadline; + let selected: Result; try { - deadline = kunaFunctionDeadline(installation.configuration); + selected = readRuntimeSetting(() => + kunaFunctionDeadline(installation.configuration), + ); } catch (cause: unknown) { admitted(); throw cause; } + if (!selected.ok) { + admitted(); + return selected; + } + const deadline = selected.value; const run = await this.#commands.run({ installation, arguments: (input) => ["decompile", input, address, "--addr"], @@ -1762,9 +1774,13 @@ export class KunaSessionClient implements AnalysisClient { operation, signal, ); + // A deadline refusal is the request's own setting failure with its own + // recovery, not a missing fact the dossier can report as unavailable. if ( !relationships.ok && - (relationships.error.cleanupIncomplete || signal?.aborted === true) + (relationships.error.cleanupIncomplete || + relationships.error instanceof ConfigurationError || + signal?.aborted === true) ) return relationships; const callAvailability = relationships.ok @@ -2087,21 +2103,40 @@ export class KunaSessionClient implements AnalysisClient { } } - /** Reuse a verified export, or start building one in the background. */ - #startProject(installation: KunaInstallation): Promise { - if (this.#project !== undefined) return Promise.resolve(); + /** + * Reuse a verified export, or start building one in the background. The + * export's deadline is read once, in the first requesting operation's + * snapshot; a malformed launch seed without an override is returned as that + * request's refusal, and the next request reads it again. + */ + async #startProject( + installation: KunaInstallation, + ): Promise> { + if (this.#project !== undefined) return ok(null); + let deadline = this.#acceptedProjectDeadline; + if (deadline === undefined) { + const admitted = readRuntimeSetting(() => + kunaProjectDeadline(installation.configuration), + ); + if (!admitted.ok) return admitted; + deadline = admitted.value; + this.#acceptedProjectDeadline = deadline; + } // Concurrent first requests share one lookup, which verifies every // export file's digest on a fresh server. - this.#starting ??= this.#lookupOrBuild(installation).finally(() => { - this.#starting = undefined; - }); - return this.#starting; + this.#starting ??= this.#lookupOrBuild(installation, deadline).finally( + () => { + this.#starting = undefined; + }, + ); + await this.#starting; + return ok(null); } - async #lookupOrBuild(installation: KunaInstallation): Promise { - this.#acceptedProjectDeadline ??= kunaProjectDeadline( - installation.configuration, - ); + async #lookupOrBuild( + installation: KunaInstallation, + deadline: KunaDeadline, + ): Promise { const key = kunaProjectKey({ targetSha256: this.#target.sha256, targetBasename: basename(this.#target.path), @@ -2130,9 +2165,7 @@ export class KunaSessionClient implements AnalysisClient { targetPath: this.#target.path, targetSha256: this.#target.sha256, signal: this.#session.signal, - ...(this.#acceptedProjectDeadline === undefined - ? {} - : { deadline: this.#acceptedProjectDeadline }), + deadline, onStatus: (status) => { this.#status = status; if (status.inventory !== null) listed(status.inventory); @@ -2174,7 +2207,8 @@ export class KunaSessionClient implements AnalysisClient { } catch (cause: unknown) { if (!isUnavailableProject(cause) || attempt !== 0) throw cause; this.#invalidateProject(reader.value); - await this.#startProject(installation); + const project = await this.#startProject(installation); + if (!project.ok) return project; } } } @@ -2417,13 +2451,11 @@ export class KunaSessionClient implements AnalysisClient { if (exported !== undefined) return ok(exported); if (this.#session.signal.aborted) return err(cancelledBy("list_procedures", this.#session.signal)); - let deadline: KunaDeadline; - try { - deadline = kunaFunctionDeadline(installation.configuration); - } catch (cause: unknown) { - if (cause instanceof ConfigurationError) return err(cause); - throw cause; - } + const admitted = readRuntimeSetting(() => + kunaFunctionDeadline(installation.configuration), + ); + if (!admitted.ok) return admitted; + const deadline = admitted.value; const run = await this.#commands.run({ installation, arguments: (input) => ["functions", input, "--json"], diff --git a/src/kuna/KunaStringQueries.ts b/src/kuna/KunaStringQueries.ts index f1d2dec0a7..3b84655e8f 100644 --- a/src/kuna/KunaStringQueries.ts +++ b/src/kuna/KunaStringQueries.ts @@ -1,3 +1,4 @@ +import { readRuntimeSetting } from "../config/runtimeAnalysisSettings.js"; import type { RegexConfigurationSnapshot } from "../process/RegexMatcher.js"; import type { AnalysisError } from "../domain/analysisErrorBase.js"; import { @@ -118,6 +119,12 @@ export class KunaStringQueries { return this.#load(installation, operation, signal); } if (this.#pending === undefined) { + // Read the deadline in the requesting operation's snapshot; a malformed + // launch seed without an override is refused here as a value. + const deadline = readRuntimeSetting(() => + kunaProjectDeadline(installation.configuration), + ); + if (!deadline.ok) return deadline; const pending = this.work.start( (producerSignal) => this.work.scan( @@ -136,7 +143,7 @@ export class KunaStringQueries { "any", "--no-xrefs", ], - deadline: kunaProjectDeadline(installation.configuration), + deadline: deadline.value, }, new KunaStringInventoryReader(), producerSignal, diff --git a/src/process/JsonLineBridgeReader.test.ts b/src/process/JsonLineBridgeReader.test.ts new file mode 100644 index 0000000000..12ce10e68f --- /dev/null +++ b/src/process/JsonLineBridgeReader.test.ts @@ -0,0 +1,166 @@ +import { describe, expect, it } from "vitest"; + +import { + ghidraReplyEnvelope, + type GhidraRemoteError, +} from "../ghidra/protocol.js"; +import { hopperReplyEnvelope } from "../hopper/protocol.js"; +import { + JsonLineBridgeReader, + type JsonLineBridgeFailure, +} from "./JsonLineBridgeReader.js"; + +type Response = Parameters[0]; + +const readerFor = ({ + inFlight = [] as number[], + nextId = 5, + maxReplyBytes = 1024, +} = {}) => { + const delivered: number[] = []; + const rejected: GhidraRemoteError[] = []; + const failures: JsonLineBridgeFailure[] = []; + const pending = new Set(inFlight); + const reader = new JsonLineBridgeReader({ + envelope: ghidraReplyEnvelope, + maxReplyBytes: () => maxReplyBytes, + deliver: (response) => { + if (!pending.delete(response.id)) return false; + delivered.push(response.id); + return true; + }, + wasSent: (id) => id < nextId, + rejectInFlight: (cause) => { + if (pending.size === 0) return false; + pending.clear(); + rejected.push(cause); + return true; + }, + fail: (failure) => failures.push(failure), + }); + return { reader, delivered, rejected, failures }; +}; + +const success = (id: number) => + `{"id":${String(id)},"ok":true,"result":null}\n`; +const invalidRequest = (id: number) => + `{"id":${String(id)},"ok":false,"error":{"code":"invalid_request","message":"Bridge request is invalid"}}\n`; + +describe("JSON-line bridge reader", () => { + it("fails the requests in flight with a rejection on the reserved id", () => { + const { reader, rejected, failures } = readerFor({ inFlight: [4] }); + + reader.push(Buffer.from(invalidRequest(0))); + + expect(rejected).toMatchObject([ + { code: "invalid_request", message: "Bridge request is invalid" }, + ]); + expect(failures).toEqual([]); + }); + + it("reports a reserved-id rejection with nothing in flight as a connection failure", () => { + const { reader, failures } = readerFor(); + + reader.push(Buffer.from(invalidRequest(0))); + + expect(failures).toMatchObject([ + { + kind: "connection_rejected", + cause: { message: "Bridge request is invalid" }, + }, + ]); + }); + + it("never accepts a success on the reserved id", () => { + const { reader, failures } = readerFor({ inFlight: [4] }); + + reader.push(Buffer.from('{"id":0,"ok":false}\n')); + + expect(failures).toMatchObject([{ kind: "envelope" }]); + }); + + it("refuses a schema-valid success on the reserved id without delivering it", () => { + // Hopper's envelope admits id 0, so the reader's own reserved-id check + // is what keeps this success from reaching a request. + const delivered: unknown[] = []; + const failures: JsonLineBridgeFailure[] = []; + let rejected = false; + const reader = new JsonLineBridgeReader({ + envelope: hopperReplyEnvelope, + maxReplyBytes: () => 1024, + deliver: (message) => (delivered.push(message), true), + wasSent: () => true, + rejectInFlight: () => (rejected = true), + fail: (failure) => failures.push(failure), + }); + + reader.push(Buffer.from('{"id":0,"result":null}\n')); + + expect(delivered).toEqual([]); + expect(rejected).toBe(false); + expect(failures).toEqual([{ kind: "unknown_id", id: 0 }]); + }); + + it("ignores a late reply for a sent id and fails on an id never sent", () => { + const { reader, delivered, failures } = readerFor({ inFlight: [2] }); + + reader.push(Buffer.from(success(1) + success(2) + success(9) + success(2))); + + expect(delivered).toEqual([2]); + expect(failures).toEqual([{ kind: "unknown_id", id: 9 }]); + }); + + it("stops reading after a failure until reset", () => { + const { reader, delivered, failures } = readerFor({ inFlight: [3, 4] }); + + reader.push(Buffer.from(`not json\n${success(3)}`)); + reader.push(Buffer.from(success(4))); + expect(delivered).toEqual([]); + expect(failures).toMatchObject([{ kind: "envelope" }]); + + reader.reset(); + reader.push(Buffer.from(success(4))); + expect(delivered).toEqual([4]); + }); + + it("refuses an unterminated reply past the ceiling without waiting for its LF", () => { + const { reader, failures } = readerFor({ maxReplyBytes: 1024 }); + + reader.push(Buffer.from('{"id":1,"ok":true,"result":"')); + reader.push(Buffer.alloc(1024, 0x78)); + + expect(failures).toEqual([ + { kind: "reply_oversize", maximumBytes: 1024, observedBytes: 1052 }, + ]); + }); + + it("refuses a complete reply line longer than the ceiling", () => { + const { reader, delivered, failures } = readerFor({ + inFlight: [1], + maxReplyBytes: 64, + }); + + reader.push( + Buffer.from(`{"id":1,"ok":true,"result":"${"x".repeat(64)}"}\n`), + ); + + expect(delivered).toEqual([]); + expect(failures).toMatchObject([ + { kind: "reply_oversize", maximumBytes: 64 }, + ]); + }); + + it("reports malformed UTF-8 as its own failure", () => { + const { reader, failures } = readerFor({ inFlight: [1] }); + + reader.push( + Buffer.concat([ + Buffer.from('{"id":1,"ok":true,"result":"'), + Buffer.from([0xff]), + Buffer.from('"}\n'), + ]), + ); + + expect(failures).toMatchObject([{ kind: "utf8" }]); + }); +}); diff --git a/src/process/JsonLineBridgeReader.ts b/src/process/JsonLineBridgeReader.ts new file mode 100644 index 0000000000..ac58b7aba2 --- /dev/null +++ b/src/process/JsonLineBridgeReader.ts @@ -0,0 +1,127 @@ +import type { Result } from "../domain/result.js"; +import { + Utf8NdjsonLineBuffer, + Utf8NdjsonOversizeError, +} from "./Utf8NdjsonLineBuffer.js"; + +/** + * Reply id a REA-owned bridge uses for a failure it cannot attribute to one + * request, such as an unparseable or oversized request frame. No client + * request uses it: both clients number requests from 1. + */ +export const BRIDGE_CONNECTION_FAILURE_ID = 0; + +/** Why the reply stream can no longer be trusted. The connection must close. */ +export type JsonLineBridgeFailure = + | { + readonly kind: "reply_oversize"; + readonly maximumBytes: number; + readonly observedBytes: number; + } + | { readonly kind: "utf8"; readonly cause: Error } + /** The line is not JSON or is outside the bridge envelope. */ + | { readonly kind: "envelope"; readonly cause: Error } + /** A reply names an id REA never sent, or a success on the reserved id. */ + | { readonly kind: "unknown_id"; readonly id: number } + /** The bridge rejected the connection while no request was in flight. */ + | { readonly kind: "connection_rejected"; readonly cause: Error }; + +/** Bridge-specific reply envelope: the only part Ghidra and Hopper do not share. */ +export interface JsonLineBridgeEnvelope { + readonly parse: (line: string) => Result; + readonly id: (message: Message) => number; + /** The remote failure a message carries, or undefined for a result or event. */ + readonly remoteFailure: (message: Message) => Failure | undefined; +} + +export interface JsonLineBridgeReaderOptions { + readonly envelope: JsonLineBridgeEnvelope; + /** Effective reply line ceiling, read per chunk. */ + readonly maxReplyBytes: () => number; + /** Settle the in-flight request or forward its event; false when the id is not in flight. */ + readonly deliver: (message: Message) => boolean; + /** Whether REA already wrote this id; a late reply for it is ignored. */ + readonly wasSent: (id: number) => boolean; + /** Fail every in-flight request with the bridge's reason; false when none. */ + readonly rejectInFlight: (cause: Failure) => boolean; + /** The stream is unusable; the owner fails its requests and closes the socket. */ + readonly fail: (failure: JsonLineBridgeFailure) => void; +} + +/** + * Reads one REA-owned JSON-line bridge connection: bounded UTF-8 framing, + * envelope parsing, and request-id correlation including connection-level + * rejections on the reserved id. After the first failure it ignores input + * until `reset`, because the owner closes the connection. + */ +export class JsonLineBridgeReader { + readonly #options: JsonLineBridgeReaderOptions; + readonly #lines: Utf8NdjsonLineBuffer; + #failed = false; + + constructor(options: JsonLineBridgeReaderOptions) { + this.#options = options; + this.#lines = new Utf8NdjsonLineBuffer(options.maxReplyBytes); + } + + push(chunk: Buffer): void { + if (this.#failed) return; + for (const line of this.#lines.push(chunk)) { + if (!line.ok) { + this.#fail( + line.error instanceof Utf8NdjsonOversizeError + ? { + kind: "reply_oversize", + maximumBytes: line.error.maximumBytes, + observedBytes: line.error.observedBytes, + } + : { kind: "utf8", cause: line.error }, + ); + return; + } + if (!this.#route(line.value)) return; + } + } + + /** Validate the unterminated tail at EOF. */ + finish(): void { + if (this.#failed) return; + const tail = this.#lines.finish(); + if (!tail.ok) this.#fail({ kind: "utf8", cause: tail.error }); + } + + /** Discard buffered bytes and accept input again for a new connection. */ + reset(): void { + this.#lines.reset(); + this.#failed = false; + } + + /** Route one complete line; false once the stream has failed. */ + #route(line: string): boolean { + const { envelope } = this.#options; + const parsed = envelope.parse(line); + if (!parsed.ok) + return this.#fail({ kind: "envelope", cause: parsed.error }); + const message = parsed.value; + const id = envelope.id(message); + if (id === BRIDGE_CONNECTION_FAILURE_ID) { + const cause = envelope.remoteFailure(message); + if (cause === undefined) return this.#fail({ kind: "unknown_id", id }); + // The bridge could not attribute the rejected request and closes the + // connection, so every request in flight fails with its remote reason. + if (!this.#options.rejectInFlight(cause)) + return this.#fail({ kind: "connection_rejected", cause }); + return true; + } + if (this.#options.deliver(message)) return true; + if (this.#options.wasSent(id)) return true; + return this.#fail({ kind: "unknown_id", id }); + } + + #fail(failure: JsonLineBridgeFailure): false { + this.#failed = true; + this.#lines.reset(); + this.#options.fail(failure); + return false; + } +} diff --git a/src/process/RegexMatcher.ts b/src/process/RegexMatcher.ts index dc74691b3b..63ed684b4b 100644 --- a/src/process/RegexMatcher.ts +++ b/src/process/RegexMatcher.ts @@ -1,3 +1,4 @@ +import { readRuntimeSetting } from "../config/runtimeAnalysisSettings.js"; import { ConfigurationError } from "../domain/configurationErrors.js"; import { HOST_PLATFORM } from "../platform/HostPlatform.js"; import { analysisSettingLimit } from "../domain/analysisSettingDefinitions.js"; @@ -454,16 +455,16 @@ export const launchRegexMatchingLimitation = ( deadline: RegexMatchingDeadline = DEFAULT_REGEX_MATCHING_DEADLINE, request: RegexWorkerHeapRequest = DEFAULT_REGEX_WORKER_HEAP_REQUEST, ): string => { - try { - return regexMatchingLimitation( + const limitation = readRuntimeSetting(() => + regexMatchingLimitation( resolveLaunchRegexWorkerHeap(launchExecArgv, launchNodeOptions, request), deadline, request, - ); - } catch (cause: unknown) { - if (!(cause instanceof ConfigurationError)) throw cause; - return "Regex mode is unavailable until its invalid launch configuration is corrected with update_analysis_settings or by restarting REA. Inspect analysis settings for the affected value."; - } + ), + ); + return limitation.ok + ? limitation.value + : "Regex mode is unavailable until its invalid launch configuration is corrected with update_analysis_settings or by restarting REA. Inspect analysis settings for the affected value."; }; /** @@ -599,23 +600,17 @@ export interface RegexConfigurationSnapshot { export const snapshotRegexConfiguration = ( selectedDeadline: RegexMatchingDeadline = DEFAULT_REGEX_MATCHING_DEADLINE, selectedHeap: RegexWorkerHeapRequest = DEFAULT_REGEX_WORKER_HEAP_REQUEST, -): Result => { - try { - return ok({ - deadline: { - deadlineMs: selectedDeadline.deadlineMs, - source: selectedDeadline.source, - }, - heapRequest: { - heapMiB: selectedHeap.heapMiB, - source: selectedHeap.source, - }, - }); - } catch (cause: unknown) { - if (cause instanceof ConfigurationError) return err(cause); - throw cause; - } -}; +): Result => + readRuntimeSetting(() => ({ + deadline: { + deadlineMs: selectedDeadline.deadlineMs, + source: selectedDeadline.source, + }, + heapRequest: { + heapMiB: selectedHeap.heapMiB, + source: selectedHeap.source, + }, + })); /** Match an inventory off the service thread, and prove worker exit before returning. */ export const matchRegexValues = async (input: { diff --git a/src/process/Utf8NdjsonLineBuffer.test.ts b/src/process/Utf8NdjsonLineBuffer.test.ts index 871ccaf820..098c25f728 100644 --- a/src/process/Utf8NdjsonLineBuffer.test.ts +++ b/src/process/Utf8NdjsonLineBuffer.test.ts @@ -2,7 +2,10 @@ import { describe, expect, it, vi } from "vitest"; import { ParseFailureError } from "../domain/parseFailure.js"; import { ok } from "../domain/result.js"; -import { Utf8NdjsonLineBuffer } from "./Utf8NdjsonLineBuffer.js"; +import { + Utf8NdjsonLineBuffer, + Utf8NdjsonOversizeError, +} from "./Utf8NdjsonLineBuffer.js"; const texts = (lines: Iterable<{ ok: boolean; value?: string }>): string[] => Array.from(lines, (line) => { @@ -13,7 +16,7 @@ const texts = (lines: Iterable<{ ok: boolean; value?: string }>): string[] => describe("UTF-8 NDJSON line buffer", () => { it("drains complete buffered lines on an empty push after iteration stops", () => { - const lines = new Utf8NdjsonLineBuffer(); + const lines = new Utf8NdjsonLineBuffer(() => 1024 * 1024); const iterator = lines.push(Buffer.from("first\nsecond\nthird\npartial")); expect(iterator.next()).toEqual({ value: ok("first"), done: false }); iterator.return(undefined); @@ -23,7 +26,7 @@ describe("UTF-8 NDJSON line buffer", () => { }); it("joins fragments and yields trimmed nonblank complete lines", () => { - const lines = new Utf8NdjsonLineBuffer(); + const lines = new Utf8NdjsonLineBuffer(() => 1024 * 1024); expect(texts(lines.push(Buffer.from(" first ")))).toEqual([]); expect( @@ -42,7 +45,7 @@ describe("UTF-8 NDJSON line buffer", () => { throw failure; }); try { - const lines = new Utf8NdjsonLineBuffer(); + const lines = new Utf8NdjsonLineBuffer(() => 1024 * 1024); let observed; try { Array.from(lines.push(Buffer.from("valid\n"))); @@ -56,7 +59,7 @@ describe("UTF-8 NDJSON line buffer", () => { }); it("yields a line that is not UTF-8 as a malformed parse failure and stops", () => { - const lines = new Utf8NdjsonLineBuffer(); + const lines = new Utf8NdjsonLineBuffer(() => 1024 * 1024); const yielded = Array.from( lines.push(Buffer.from([0x61, 0x0a, 0xff, 0x0a, 0x62, 0x0a])), ); @@ -65,7 +68,8 @@ describe("UTF-8 NDJSON line buffer", () => { const failure = yielded[1]; expect(failure?.ok).toBe(false); if (failure === undefined || failure.ok) return; - expect(failure.error).toBeInstanceOf(ParseFailureError); + if (!(failure.error instanceof ParseFailureError)) + throw new Error("expected a malformed parse failure"); expect(failure.error.failure).toMatchObject({ kind: "malformed", format: "ndjson", @@ -74,7 +78,7 @@ describe("UTF-8 NDJSON line buffer", () => { }); it("reports an unfinished tail that is not UTF-8 at EOF", () => { - const lines = new Utf8NdjsonLineBuffer(); + const lines = new Utf8NdjsonLineBuffer(() => 1024 * 1024); expect(texts(lines.push(Buffer.from([0x61, 0x0a, 0xff])))).toEqual(["a"]); const tail = lines.finish(); expect(tail.ok).toBe(false); @@ -85,7 +89,7 @@ describe("UTF-8 NDJSON line buffer", () => { }); it("retains unconsumed complete lines and reset discards them", () => { - const lines = new Utf8NdjsonLineBuffer(); + const lines = new Utf8NdjsonLineBuffer(() => 1024 * 1024); const iterator = lines.push(Buffer.from('invalid\n{"id":2}\n')); expect(iterator.next()).toEqual({ value: ok("invalid"), done: false }); @@ -110,7 +114,7 @@ describe("UTF-8 NDJSON line buffer", () => { }); it("releases the backing allocation when a frame ends at the newline", () => { - const lines = new Utf8NdjsonLineBuffer(); + const lines = new Utf8NdjsonLineBuffer(() => 1024 * 1024); const frame = Buffer.alloc(64 * 1024, 0x78); frame[frame.length - 1] = 0x0a; expect(texts(lines.push(frame))).toEqual([ @@ -125,4 +129,33 @@ describe("UTF-8 NDJSON line buffer", () => { backingBytes: tail.buffer.byteLength, }); }); + + it("discards an unterminated tail once it passes the ceiling", () => { + const lines = new Utf8NdjsonLineBuffer(() => 16); + expect([...lines.push(Buffer.alloc(16, 0x78))]).toEqual([]); + + const refused = [...lines.push(Buffer.from("y"))]; + expect(refused).toHaveLength(1); + const refusal = refused[0]?.ok ? undefined : refused[0]?.error; + expect(refusal).toEqual(new Utf8NdjsonOversizeError(16, 17)); + // The refusal is the shared limit parse failure, with the observed bytes. + expect(refusal).toBeInstanceOf(ParseFailureError); + expect(refusal?.failure).toMatchObject({ + kind: "limit", + format: "ndjson", + limit: { name: "ndjson-line-bytes", maximum: 16 }, + }); + expect(lines.retainedTail()).toEqual({ pendingBytes: 0, backingBytes: 0 }); + }); + + it("reads the ceiling on each push so a runtime change applies to the next chunk", () => { + let maximumBytes = 4; + const lines = new Utf8NdjsonLineBuffer(() => maximumBytes); + const refused = [...lines.push(Buffer.from("12345\n"))]; + expect(refused[0]?.ok ? undefined : refused[0]?.error).toBeInstanceOf( + Utf8NdjsonOversizeError, + ); + maximumBytes = 8; + expect(texts(lines.push(Buffer.from("12345\n")))).toEqual(["12345"]); + }); }); diff --git a/src/process/Utf8NdjsonLineBuffer.ts b/src/process/Utf8NdjsonLineBuffer.ts index 60f1970b90..6953583f0d 100644 --- a/src/process/Utf8NdjsonLineBuffer.ts +++ b/src/process/Utf8NdjsonLineBuffer.ts @@ -1,32 +1,81 @@ import { strictUtf8Decoder } from "../domain/strictUtf8.js"; -import { - malformedInput, - type ParseFailureError, -} from "../domain/parseFailure.js"; +import { malformedInput, ParseFailureError } from "../domain/parseFailure.js"; import { err, ok, type Result } from "../domain/result.js"; -/** A frame that is not UTF-8; a value, so it cannot be confused with a consumer's exception. */ -type Utf8NdjsonLine = Result>; +/** + * One line, or the unterminated tail, reached the caller's byte ceiling: a + * `limit` parse failure that also keeps the bytes observed when framing + * stopped, which the transport error reports. + */ +export class Utf8NdjsonOversizeError extends ParseFailureError<"limit"> { + constructor( + readonly maximumBytes: number, + readonly observedBytes: number, + ) { + super({ + kind: "limit", + format: "ndjson", + detail: `NDJSON line reached ${String(observedBytes)} bytes, above the ${String(maximumBytes)}-byte line limit`, + limit: { name: "ndjson-line-bytes", maximum: maximumBytes }, + }); + this.name = "Utf8NdjsonOversizeError"; + } +} -/** Frame socket bytes before decoding, so later malformed frames cannot erase earlier lines. */ +/** + * A line, or why framing stopped: bytes that are not UTF-8, or a line above + * the ceiling. Failures are values, so they cannot be confused with a + * consumer's exception. + */ +type Utf8NdjsonLine = Result< + string, + ParseFailureError<"malformed"> | Utf8NdjsonOversizeError +>; + +/** + * Frame socket bytes before decoding, so later malformed frames cannot erase + * earlier lines. Every line is bounded: `maxLineBytes` is read on each push so + * a runtime setting change applies to the next chunk. + */ export class Utf8NdjsonLineBuffer { #buffer: Buffer = Buffer.alloc(0); #fragments: Buffer[] = []; + #fragmentBytes = 0; #nextNewline = -1; + readonly #maxLineBytes: () => number; readonly #decoder = strictUtf8Decoder("preserve"); + constructor(maxLineBytes: () => number) { + this.#maxLineBytes = maxLineBytes; + } + /** * Yield each complete trimmed nonblank line; preserve unconsumed lines on * early return. A line that is not UTF-8 is yielded as a failure and ends - * the iteration. + * the iteration. A line or the tail still waiting for its LF above the + * ceiling is yielded as `Utf8NdjsonOversizeError`, after discarding every + * buffered byte, and ends the iteration. */ *push(chunk: Buffer): Generator { + const maximumBytes = this.#maxLineBytes(); this.#fragments.push(chunk); - if (!chunk.includes(10) && this.#nextNewline < 0) return; + this.#fragmentBytes += chunk.length; + if (!chunk.includes(10) && this.#nextNewline < 0) { + const refused = this.#refuseTail(maximumBytes); + if (refused !== undefined) yield err(refused); + return; + } this.#buffer = Buffer.concat([this.#buffer, ...this.#fragments]); this.#fragments = []; + this.#fragmentBytes = 0; if (this.#nextNewline < 0) this.#nextNewline = this.#buffer.indexOf(10); while (this.#nextNewline >= 0) { + if (this.#nextNewline > maximumBytes) { + const observedBytes = this.#nextNewline; + this.reset(); + yield err(new Utf8NdjsonOversizeError(maximumBytes, observedBytes)); + return; + } const encodedLine = this.#buffer.subarray(0, this.#nextNewline); const rest = this.#buffer.subarray(this.#nextNewline + 1); // An empty view would keep the drained frame's allocation for the session. @@ -40,6 +89,8 @@ export class Utf8NdjsonLineBuffer { const trimmed = line.value.trim(); if (trimmed.length > 0) yield ok(trimmed); } + const refused = this.#refuseTail(maximumBytes); + if (refused !== undefined) yield err(refused); } /** Validate the unfinished tail at EOF without treating it as a complete response. */ @@ -50,7 +101,16 @@ export class Utf8NdjsonLineBuffer { return tail.ok ? ok(null) : tail; } - #decode(bytes: Buffer): Utf8NdjsonLine { + #refuseTail(maximumBytes: number): Utf8NdjsonOversizeError | undefined { + const pendingBytes = this.#buffer.length + this.#fragmentBytes; + if (this.#nextNewline >= 0 || pendingBytes <= maximumBytes) + return undefined; + // Drop the refused bytes rather than retaining them until reconnect. + this.reset(); + return new Utf8NdjsonOversizeError(maximumBytes, pendingBytes); + } + + #decode(bytes: Buffer): Result> { try { return ok(this.#decoder.decode(bytes)); } catch (cause: unknown) { @@ -64,6 +124,7 @@ export class Utf8NdjsonLineBuffer { reset(): void { this.#buffer = Buffer.alloc(0); this.#fragments = []; + this.#fragmentBytes = 0; this.#nextNewline = -1; } @@ -72,9 +133,7 @@ export class Utf8NdjsonLineBuffer { readonly pendingBytes: number; readonly backingBytes: number; } { - const pendingBytes = - this.#buffer.length + - this.#fragments.reduce((sum, fragment) => sum + fragment.length, 0); + const pendingBytes = this.#buffer.length + this.#fragmentBytes; const backingBytes = this.#buffer.buffer.byteLength + this.#fragments.reduce( diff --git a/src/rizin/RizinDebugSessionManager.ts b/src/rizin/RizinDebugSessionManager.ts index 364194f875..77d42efe0a 100644 --- a/src/rizin/RizinDebugSessionManager.ts +++ b/src/rizin/RizinDebugSessionManager.ts @@ -1,3 +1,4 @@ +import { readRuntimeSetting } from "../config/runtimeAnalysisSettings.js"; import { RIZIN_DEBUGGER_PROVIDER_IDENTITY } from "./RizinDebuggerProviderIdentity.js"; import { persistentDebuggerHostSupported } from "../process/PersistentDebuggerHost.js"; import { Deadline } from "../domain/deadline.js"; @@ -40,7 +41,6 @@ import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup. import { createEvidence, type Evidence } from "../domain/evidence.js"; import { currentAnalysisOperationSnapshot } from "../domain/analysisOperationSnapshot.js"; import { err, ok, type Result } from "../domain/result.js"; -import { ConfigurationError } from "../domain/configurationErrors.js"; import { ProviderProcessSupervisor, spawnOwnedProviderProcess, @@ -778,15 +778,13 @@ export class RizinDebugSessionManager { }, ), ); - let acceptedDeadline: DebuggerAdmissionLimit; - let acceptedOutput: DebuggerAdmissionLimit; - try { - acceptedDeadline = this.#commandDeadline; - acceptedOutput = this.#commandOutput; - } catch (cause: unknown) { - if (cause instanceof ConfigurationError) return err(cause); - throw cause; - } + const accepted = readRuntimeSetting(() => ({ + deadline: this.#commandDeadline, + output: this.#commandOutput, + })); + if (!accepted.ok) return accepted; + const { deadline: acceptedDeadline, output: acceptedOutput } = + accepted.value; session.pendingCommandCount += 1; let release: (() => void) | undefined; let acquired = false; diff --git a/tests/boundary/filesystem/hopperFatCoordinates.test.ts b/tests/boundary/filesystem/hopperFatCoordinates.test.ts index 6c73b28248..64f7042dc8 100644 --- a/tests/boundary/filesystem/hopperFatCoordinates.test.ts +++ b/tests/boundary/filesystem/hopperFatCoordinates.test.ts @@ -16,7 +16,10 @@ import { AnalysisCleanupLease } from "../../../src/artifacts/AnalysisCleanupLeas import { RegularFileCleanupFailure } from "../../../src/filesystem/OwnedRegularFile.js"; import type { OwnedFileHandle } from "../../../src/filesystem/OwnedFileHandle.js"; import { mapHopperFileOffset } from "../../../src/hopper/HopperFileOffset.js"; -import { parseHopperReply } from "../../../src/hopper/HopperReplyContracts.js"; +import { + hopperReplySchemas, + parseHopperReply, +} from "../../../src/hopper/HopperReplyContracts.js"; import { resolveHopperMachOImage } from "../../../src/hopper/HopperMachOImage.js"; import { createInspectionResources } from "../../fixtures/inspectionResources.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; @@ -50,7 +53,12 @@ const mapOffset = (target: BinaryTarget, observation: JsonValue) => { const scope = new ArtifactResourceScope(); const leases = new WeakMap(); onTestFinished(() => scope.close()); - const reply = parseHopperReply("address_to_file_offset", observation); + const reply = parseHopperReply( + hopperReplySchemas, + "address_to_file_offset", + observation, + "address_to_file_offset", + ); if (!reply.ok) return Promise.resolve(reply); return mapHopperFileOffset(target, reply.value, { retainFailedClose: (owner) => { diff --git a/tests/boundary/providers/ghidra/ghidraCheckpointLifecycle.test.ts b/tests/boundary/providers/ghidra/ghidraCheckpointLifecycle.test.ts index b4d11c505a..033e6e7a05 100644 --- a/tests/boundary/providers/ghidra/ghidraCheckpointLifecycle.test.ts +++ b/tests/boundary/providers/ghidra/ghidraCheckpointLifecycle.test.ts @@ -13,6 +13,8 @@ import { join } from "node:path"; import { ArtifactResourceScope } from "../../../../src/artifacts/ArtifactResourceScope.js"; import { createAnalysisProfile } from "../../../../src/domain/analysisProfile.js"; import { GhidraClient } from "../../../../src/ghidra/GhidraClient.js"; +import type { BridgeMaxReplyBytes } from "../../../../src/config/bridgeMaxReplyBytes.js"; +import { ConfigurationError } from "../../../../src/domain/configurationErrors.js"; import { resolveGhidraCheckpoint } from "../../../../src/ghidra/GhidraCheckpoint.js"; import { GhidraFixtureLauncher } from "./ghidraClient.fixture.js"; import { awaitAcknowledgement } from "../../../fixtures/lifecycleAcknowledgement.js"; @@ -73,6 +75,7 @@ const profile = createAnalysisProfile( ); async function fixture( mode: ConstructorParameters[0] = "checkpoint", + bridgeMaxReplyBytes?: BridgeMaxReplyBytes, ) { const root = await mkdtemp(join(tmpdir(), "rea-checkpoint-lifecycle-")); roots.push(root); @@ -97,6 +100,7 @@ async function fixture( profileDigest: profile.digest, checkpoint: selection, startupTimeoutMs: 10_000, + ...(bridgeMaxReplyBytes === undefined ? {} : { bridgeMaxReplyBytes }), }); clients.push(client); return { root, selection, launcher, client, targetPath, sha256 }; @@ -141,6 +145,29 @@ it.skipIf(process.platform === "win32")( }, ); +it.skipIf(process.platform === "win32")( + "saves the closing checkpoint under the session's ceilings after its seed is refused", + async () => { + // A seed refused after startup (its override reset) must not refuse the + // closing checkpoint and leave the JVM running. + let seedRefused = false; + const f = await fixture("checkpoint", { + get maximumBytes(): number { + if (seedRefused) + throw new ConfigurationError("invalid bridge.max_reply_bytes seed"); + return 1024 * 1024; + }, + source: "runtime", + }); + expect((await f.client.start()).ok).toBe(true); + seedRefused = true; + + expect((await f.client.close()).ok).toBe(true); + expect(f.launcher.requests).toContain("checkpoint_program"); + expect(f.launcher.requests).toContain("shutdown"); + }, +); + it.skipIf(process.platform === "win32")( "keeps a failed normal-close save editable and retries without deleting the previous checkpoint", async () => { diff --git a/tests/boundary/providers/ghidra/ghidraClient.test.ts b/tests/boundary/providers/ghidra/ghidraClient.test.ts index dc1f9f878e..bb621f4c0a 100644 --- a/tests/boundary/providers/ghidra/ghidraClient.test.ts +++ b/tests/boundary/providers/ghidra/ghidraClient.test.ts @@ -1,4 +1,5 @@ import { parseConfig } from "../../../../src/config/parseConfig.js"; +import type { BridgeMaxReplyBytes } from "../../../../src/config/bridgeMaxReplyBytes.js"; import { GhidraProvider } from "../../../../src/ghidra/GhidraProvider.js"; import { fixtureDosLoadImage } from "../../../../src/ghidra/GhidraLoadImage.fixture.js"; import { jsonValueSchema } from "../../../../src/domain/jsonValue.js"; @@ -24,6 +25,7 @@ import { PassThrough } from "node:stream"; import { afterEach, describe, expect, it, onTestFinished, vi } from "vitest"; import pino from "pino"; +import { ConfigurationError } from "../../../../src/domain/configurationErrors.js"; import { withVerifierCleanup } from "../../../../scripts/lib/verifier-cleanup.mjs"; import { awaitAcknowledgement } from "../../../fixtures/lifecycleAcknowledgement.js"; import type { Logger } from "pino"; @@ -128,6 +130,7 @@ const clientFor = ( readonly logger?: Logger; readonly targetPath?: string; readonly targetSha256?: string; + readonly bridgeMaxReplyBytes?: BridgeMaxReplyBytes; } = {}, ): GhidraClient => { const client = new GhidraClient({ @@ -140,6 +143,9 @@ const clientFor = ( startupTimeoutMs: options.startupTimeoutMs ?? 1_000, ...(options.runId === undefined ? {} : { runId: options.runId }), ...(options.logger === undefined ? {} : { logger: options.logger }), + ...(options.bridgeMaxReplyBytes === undefined + ? {} + : { bridgeMaxReplyBytes: options.bridgeMaxReplyBytes }), ...(options.onDiagnostic === undefined ? {} : { onDiagnostic: options.onDiagnostic }), @@ -2759,3 +2765,60 @@ it.each(["search_procedures", "search_strings"] as const)( ).toBe(4); }, ); + +describe("GhidraClient reply ceiling", () => { + it("shuts the bridge down under the session's ceilings after its seed is refused", async () => { + // Cleanup must not depend on the current settings: a seed refused after + // startup would otherwise refuse the shutdown request before writing it. + let seedRefused = false; + const replyCeiling: BridgeMaxReplyBytes = { + get maximumBytes(): number { + if (seedRefused) + throw new ConfigurationError("invalid bridge.max_reply_bytes seed"); + return 1024 * 1024; + }, + source: "runtime", + }; + const logs: string[] = []; + const logger = pino( + { level: "warn" }, + { + write: (line) => { + logs.push(line); + }, + }, + ); + const client = clientFor(new FixtureLauncher(), { + bridgeMaxReplyBytes: replyCeiling, + logger, + }); + await expect(client.start()).resolves.toMatchObject({ ok: true }); + seedRefused = true; + + await expect(client.close()).resolves.toEqual({ ok: true, value: null }); + expect(logs.join("")).not.toContain( + "Ghidra bridge shutdown was not confirmed", + ); + }); + + it("refuses a reply above bridge.max_reply_bytes with a typed limit", async () => { + const client = clientFor(new FixtureLauncher("oversized_result"), { + bridgeMaxReplyBytes: { maximumBytes: 1024 * 1024, source: "setting" }, + }); + + const result = await client.callTool("list_strings", { document: null }); + + expect(result.ok ? undefined : result.error).toMatchObject({ + kind: "protocol", + cause: { + _tag: "AnalysisResourceConstraintError", + resource: "transport", + limit: { + setting: "bridge.max_reply_bytes", + effective_value: 1024 * 1024, + source: "setting", + }, + }, + }); + }); +}); diff --git a/tests/boundary/providers/hopper/fakeHopperReplyContracts.test.ts b/tests/boundary/providers/hopper/fakeHopperReplyContracts.test.ts index e2cc11ff75..ec3e91d423 100644 --- a/tests/boundary/providers/hopper/fakeHopperReplyContracts.test.ts +++ b/tests/boundary/providers/hopper/fakeHopperReplyContracts.test.ts @@ -1,22 +1,33 @@ +import type { JsonValue } from "../../../../src/domain/jsonValue.js"; import { expect, it } from "vitest"; import { - parseHopperReply, - hasHopperReplyContract, + parseHopperReply as parseHopperReplyWith, + hopperReplySchemas, + type HopperReplyMethod, } from "../../../../src/hopper/HopperReplyContracts.js"; import { FAKE_HOPPER_REPLY_METHODS, fakeHopperReply, } from "../../../fixtures/fakeHopperReplies.mjs"; +const parseHopperReply = ( + method: M, + value: JsonValue, + operation: string = method, +) => parseHopperReplyWith(hopperReplySchemas, method, value, operation); + +const isReplyMethod = (method: string): method is HopperReplyMethod => + Object.hasOwn(hopperReplySchemas, method); + // The packaged CLI baseline and provider fixtures run against this fake // bridge; a reply outside the real bridge's contract fails them as unreadable // output instead of exercising the workflow. it.each(FAKE_HOPPER_REPLY_METHODS)( "serves a %s reply the Hopper reply contract admits", (method) => { - expect(hasHopperReplyContract(method)).toBe(true); - if (!hasHopperReplyContract(method)) return; + expect(isReplyMethod(method)).toBe(true); + if (!isReplyMethod(method)) return; const reply = fakeHopperReply(method); expect(reply).toBeDefined(); if (reply === undefined) return; diff --git a/tests/boundary/providers/hopper/hopperBridgeRequestLimit.test.ts b/tests/boundary/providers/hopper/hopperBridgeRequestLimit.test.ts index c2b1f213a2..5b3aec6ed1 100644 --- a/tests/boundary/providers/hopper/hopperBridgeRequestLimit.test.ts +++ b/tests/boundary/providers/hopper/hopperBridgeRequestLimit.test.ts @@ -1,12 +1,13 @@ +import { + transportReplyContracts, + HopperFixtureLauncher, + trackHopperClient, +} from "./hopperClient.fixture.js"; import { bridgeMaxRequestBytesRecovery } from "../../../../src/config/bridgeMaxRequestBytes.js"; import { expect, it, vi } from "vitest"; import { AnalysisResourceConstraintError } from "../../../../src/domain/analysisErrorCore.js"; import { HopperClient } from "../../../../src/hopper/HopperClient.js"; -import { - HopperFixtureLauncher, - trackHopperClient, -} from "./hopperClient.fixture.js"; import { waitFor } from "../../../support/testWait.js"; const limit = (maximumBytes: number) => @@ -16,6 +17,7 @@ it("reports a Hopper ceiling mismatch and releases the fixture", async () => { const launcher = new HopperFixtureLauncher(); const client = trackHopperClient( new HopperClient({ + replyContracts: transportReplyContracts, launcher, startupTimeoutMs: 10_000, bridgeMaxRequestBytes: limit(1_048_576), @@ -50,6 +52,7 @@ it("refuses an oversized Hopper request before the fixture receives it", async ( ); const client = trackHopperClient( new HopperClient({ + replyContracts: transportReplyContracts, launcher, startupTimeoutMs: 10_000, bridgeMaxRequestBytes: limit(8192), @@ -86,6 +89,7 @@ it("refuses a Hopper payload whose JSON expansion exceeds the ceiling before ser ); const client = trackHopperClient( new HopperClient({ + replyContracts: transportReplyContracts, launcher, startupTimeoutMs: 10_000, bridgeMaxRequestBytes: limit(8192), @@ -156,6 +160,7 @@ it("serializes a queued Hopper request only when it reaches the wire", async () const launcher = new HopperFixtureLauncher(); const client = trackHopperClient( new HopperClient({ + replyContracts: transportReplyContracts, launcher, startupTimeoutMs: 10_000, }), @@ -204,6 +209,7 @@ it("sends a Hopper request above 16 MiB when both sides raise the ceiling", asyn ); const client = trackHopperClient( new HopperClient({ + replyContracts: transportReplyContracts, launcher, startupTimeoutMs: 10_000, bridgeMaxRequestBytes: limit(ceiling), diff --git a/tests/boundary/providers/hopper/hopperClient.diagnostics.test.ts b/tests/boundary/providers/hopper/hopperClient.diagnostics.test.ts index db7a53db65..5708bf46a4 100644 --- a/tests/boundary/providers/hopper/hopperClient.diagnostics.test.ts +++ b/tests/boundary/providers/hopper/hopperClient.diagnostics.test.ts @@ -1,10 +1,13 @@ +import { + transportReplyContracts, + HopperFixtureLauncher, +} from "./hopperClient.fixture.js"; import { access } from "node:fs/promises"; import { expect, it, onTestFinished } from "vitest"; import { HopperTimeoutError } from "../../../../src/domain/hopperErrors.js"; import { HopperClient } from "../../../../src/hopper/HopperClient.js"; -import { HopperFixtureLauncher } from "./hopperClient.fixture.js"; import { waitFor } from "../../../support/testWait.js"; const HOPPER_PROCESS_DIAGNOSTIC_BYTES = 8 * 1024 * 1024; @@ -19,6 +22,7 @@ it("bounds long-lived launcher output while keeping the bridge usable", async () let observedStderrBytes = 0; const diagnosticLauncher = new HopperFixtureLauncher(undefined, "noisy"); const client = new HopperClient({ + replyContracts: transportReplyContracts, launcher: diagnosticLauncher, startupTimeoutMs: 10_000, onDiagnostic(event) { @@ -50,6 +54,7 @@ it("reports observed and retained output when capture cuts a token", async () => "noisy_no_socket", ); const client = new HopperClient({ + replyContracts: transportReplyContracts, launcher: diagnosticLauncher, startupTimeoutMs: 1_000, }); diff --git a/tests/boundary/providers/hopper/hopperClient.externalExit.test.ts b/tests/boundary/providers/hopper/hopperClient.externalExit.test.ts index 23aa8994e8..e574a4f743 100644 --- a/tests/boundary/providers/hopper/hopperClient.externalExit.test.ts +++ b/tests/boundary/providers/hopper/hopperClient.externalExit.test.ts @@ -25,7 +25,11 @@ import { HopperClient } from "../../../../src/hopper/HopperClient.js"; import { silentLogger } from "../../../../src/logger.js"; import { PrivateRuntimeRoot } from "../../../../src/process/PrivateRuntimeRoot.js"; import { observeProcessStartIdentity } from "../../../../src/process/ProcessOwnershipObservation.js"; -import { HopperFixtureLauncher } from "./hopperClient.fixture.js"; +import { + HopperFixtureLauncher, + transportReplyContracts, + type TransportHopperClient, +} from "./hopperClient.fixture.js"; const identities = { hopper: { id: "hopper", name: "Hopper fixture", version: null }, @@ -101,11 +105,12 @@ const stopFixture = async (child: ChildProcess): Promise => { class ExternalFixtureBridge { readonly fixture = new HopperFixtureLauncher(); child: ChildProcess | undefined; - readonly client: HopperClient; + readonly client: TransportHopperClient; constructor() { const thisFixture = this; this.client = new HopperClient({ + replyContracts: transportReplyContracts, startupTimeoutMs: 2_000, launcher: { async launch(session: BridgeSession) { diff --git a/tests/boundary/providers/hopper/hopperClient.fixture.ts b/tests/boundary/providers/hopper/hopperClient.fixture.ts index a3f72c4c04..0a15a643f8 100644 --- a/tests/boundary/providers/hopper/hopperClient.fixture.ts +++ b/tests/boundary/providers/hopper/hopperClient.fixture.ts @@ -11,6 +11,11 @@ import type { } from "../../../../src/hopper/BridgeLauncher.js"; import { waitForExit } from "../../../support/process/processFixture.js"; import { HopperClient } from "../../../../src/hopper/HopperClient.js"; +import type { HopperReplyContracts } from "../../../../src/hopper/HopperReplyContracts.js"; +import { + ownedJsonValueSchema, + type JsonValue, +} from "../../../../src/domain/jsonValue.js"; import { waitFor } from "../../../support/testWait.js"; export const hopperFixturePath = fileURLToPath( @@ -119,8 +124,24 @@ export class HopperFixtureLauncher implements BridgeLauncher { } } +/** + * Transport tests call the fake bridge's own methods (`echo`, `hang`, ...), so + * every method's reply contract is any JSON value. Production contracts are + * covered by the provider and reply-contract suites. + */ +export type TransportReplyContracts = Readonly< + Record> +>; +export const transportReplyContracts: TransportReplyContracts = new Proxy( + {}, + { get: () => ownedJsonValueSchema }, +); +export type TransportHopperClient = HopperClient; + /** Register owned client cleanup before any startup assertion can fail. */ -export const trackHopperClient = (client: HopperClient): HopperClient => { +export const trackHopperClient = ( + client: HopperClient, +): HopperClient => { onTestFinished(async () => { await client.close(); }); @@ -130,11 +151,14 @@ export const trackHopperClient = (client: HopperClient): HopperClient => { /** Start a real fixture session with test-scoped cleanup. */ export const startHopperFixtureClient = async ( launcher = new HopperFixtureLauncher(), -): Promise => { + options: Partial[0]> = {}, +): Promise => { const client = trackHopperClient( new HopperClient({ launcher, startupTimeoutMs: 10_000, + ...options, + replyContracts: transportReplyContracts, }), ); const started = await client.start(); diff --git a/tests/boundary/providers/hopper/hopperClient.handshakeRecovery.test.ts b/tests/boundary/providers/hopper/hopperClient.handshakeRecovery.test.ts index 3b24934e67..4d04e04fec 100644 --- a/tests/boundary/providers/hopper/hopperClient.handshakeRecovery.test.ts +++ b/tests/boundary/providers/hopper/hopperClient.handshakeRecovery.test.ts @@ -1,3 +1,7 @@ +import { + transportReplyContracts, + HopperFixtureLauncher, +} from "./hopperClient.fixture.js"; import { execFile, spawn, type ChildProcess } from "node:child_process"; import { access, rm } from "node:fs/promises"; import { setTimeout as delay } from "node:timers/promises"; @@ -7,12 +11,13 @@ import { expect, it, onTestFinished } from "vitest"; import { projectAnalysisError } from "../../../../src/domain/analysisErrorProjection.js"; import { ok } from "../../../../src/domain/result.js"; +import { ConfigurationError } from "../../../../src/domain/configurationErrors.js"; +import type { BridgeMaxReplyBytes } from "../../../../src/config/bridgeMaxReplyBytes.js"; import type { BridgeLauncher, BridgeSession, } from "../../../../src/hopper/BridgeLauncher.js"; import { HopperClient } from "../../../../src/hopper/HopperClient.js"; -import { HopperFixtureLauncher } from "./hopperClient.fixture.js"; import { waitFor } from "../../../support/testWait.js"; const stop = async (child: ChildProcess): Promise => { @@ -66,13 +71,19 @@ const setup = ( identity?: "matching" | "wrong-run" | "wrong-token", healthMode?: "ready" | "gated", shutdownMode?: "acknowledge" | "exit-without-ack", + bridgeMaxReplyBytes?: BridgeMaxReplyBytes, ) => { const launcher = new ExternalBridgeLauncher( identity, healthMode, shutdownMode, ); - const client = new HopperClient({ launcher, startupTimeoutMs: 2_000 }); + const client = new HopperClient({ + replyContracts: transportReplyContracts, + launcher, + startupTimeoutMs: 2_000, + ...(bridgeMaxReplyBytes === undefined ? {} : { bridgeMaxReplyBytes }), + }); onTestFinished(async () => { await client.close(); await Promise.all( @@ -94,6 +105,7 @@ it.each(["missing", "not-listening"] as const)( let session: BridgeSession | undefined; let helper: ChildProcess | undefined; const client = new HopperClient({ + replyContracts: transportReplyContracts, startupTimeoutMs: 2_000, launcher: { launch(value) { @@ -186,30 +198,46 @@ it.each(["wrong-run", "wrong-token"] as const)( }, ); -it("reconnects for authenticated cleanup after a previously valid bridge transport fails", async () => { - const { client, launcher } = setup(); - const started = await client.start(); - if (!started.ok) throw started.error; - const malformed = await client.callTool("malformed"); - expect(malformed).toMatchObject({ - ok: false, - error: { _tag: "HopperProtocolError" }, - }); - const closed = await client.close(); - if (!closed.ok) throw closed.error; - expect(launcher.fixture.requests.map(({ method }) => method)).toEqual([ - "health", - "malformed", - "health", - "shutdown_document", - ]); - await waitFor(() => expect(launcher.fixture.processes[0]?.exitCode).toBe(0)); - const session = launcher.sessions[0]; - if (session === undefined) throw new Error("Fixture did not launch"); - await expect(access(session.directory)).rejects.toMatchObject({ - code: "ENOENT", - }); -}); +it.each([false, true])( + "reconnects for authenticated cleanup after a previously valid bridge transport fails (seed refused after start: %s)", + async (refuseSeed) => { + // Cleanup uses the ceilings admitted at start, so a seed the runtime view + // refuses once its override is reset cannot block reauthentication. + let seedRefused = false; + const { client, launcher } = setup(undefined, undefined, undefined, { + get maximumBytes(): number { + if (seedRefused) + throw new ConfigurationError("invalid reply ceiling seed"); + return 4 * 1024 * 1024; + }, + source: "runtime", + }); + const started = await client.start(); + if (!started.ok) throw started.error; + const malformed = await client.callTool("malformed"); + expect(malformed).toMatchObject({ + ok: false, + error: { _tag: "HopperProtocolError" }, + }); + seedRefused = refuseSeed; + const closed = await client.close(); + if (!closed.ok) throw closed.error; + expect(launcher.fixture.requests.map(({ method }) => method)).toEqual([ + "health", + "malformed", + "health", + "shutdown_document", + ]); + await waitFor(() => + expect(launcher.fixture.processes[0]?.exitCode).toBe(0), + ); + const session = launcher.sessions[0]; + if (session === undefined) throw new Error("Fixture did not launch"); + await expect(access(session.directory)).rejects.toMatchObject({ + code: "ENOENT", + }); + }, +); it("preserves launcher diagnostics after a health deadline and reauthenticates for cleanup", async () => { const { client, launcher } = setup("matching", "gated"); diff --git a/tests/boundary/providers/hopper/hopperClient.health.test.ts b/tests/boundary/providers/hopper/hopperClient.health.test.ts index 92d7941461..274bf7a7da 100644 --- a/tests/boundary/providers/hopper/hopperClient.health.test.ts +++ b/tests/boundary/providers/hopper/hopperClient.health.test.ts @@ -1,3 +1,7 @@ +import { + transportReplyContracts, + HopperFixtureLauncher, +} from "./hopperClient.fixture.js"; import { EventEmitter } from "node:events"; import { PassThrough } from "node:stream"; @@ -11,7 +15,6 @@ import type { BridgeSession, } from "../../../../src/hopper/BridgeLauncher.js"; import { HopperClient } from "../../../../src/hopper/HopperClient.js"; -import { HopperFixtureLauncher } from "./hopperClient.fixture.js"; import { waitFor } from "../../../support/testWait.js"; class ObservedProcess extends EventEmitter { @@ -57,7 +60,11 @@ class LifecycleLauncher implements BridgeLauncher { const start = async (owned: boolean) => { const launcher = new LifecycleLauncher(owned); - const client = new HopperClient({ launcher, startupTimeoutMs: 10_000 }); + const client = new HopperClient({ + replyContracts: transportReplyContracts, + launcher, + startupTimeoutMs: 10_000, + }); onTestFinished(async () => { await client.close(); for (const child of launcher.fixture.processes) child.kill(); @@ -77,6 +84,7 @@ it("reports launch work until the first authenticated handshake completes", asyn releaseLaunch = resolve; }); const client = new HopperClient({ + replyContracts: transportReplyContracts, startupTimeoutMs: 10_000, launcher: { async launch(session) { diff --git a/tests/boundary/providers/hopper/hopperClient.helperFailure.test.ts b/tests/boundary/providers/hopper/hopperClient.helperFailure.test.ts index 67ae5c26da..a07eee5505 100644 --- a/tests/boundary/providers/hopper/hopperClient.helperFailure.test.ts +++ b/tests/boundary/providers/hopper/hopperClient.helperFailure.test.ts @@ -1,3 +1,8 @@ +import { + transportReplyContracts, + type TransportHopperClient, + HopperFixtureLauncher, +} from "./hopperClient.fixture.js"; import { spawn } from "node:child_process"; import { access, writeFile } from "node:fs/promises"; import { join } from "node:path"; @@ -16,17 +21,17 @@ import type { import { HopperApplicationLauncher } from "../../../../src/hopper/BridgeLauncher.js"; import { HopperClient } from "../../../../src/hopper/HopperClient.js"; import { acquireHopperTargetLease } from "../../../../src/process/HopperLease.js"; -import { HopperFixtureLauncher } from "./hopperClient.fixture.js"; // Failure assertions keep the external document unknown. Teardown then publishes // this run's authenticated bridge so normal cleanup can release its runtime/lease. const trackFailedHelperClient = ( launcher: BridgeLauncher, startupTimeoutMs = 10_000, -): HopperClient => { +): TransportHopperClient => { let session: BridgeSession | undefined; let external = false; const client = new HopperClient({ + replyContracts: transportReplyContracts, launcher: { async launch(input, options) { session = input; @@ -151,6 +156,7 @@ it.skipIf(process.platform !== "darwin")( const retry = new HopperFixtureLauncher(); let session: BridgeSession | undefined; const client = new HopperClient({ + replyContracts: transportReplyContracts, launcher: { async launch(input, options) { if (session !== undefined) return retry.launch(input); @@ -224,7 +230,11 @@ it("classifies a pre-handoff Apple Events denial as not retryable from the same ); }, }; - const client = new HopperClient({ launcher, startupTimeoutMs: 10_000 }); + const client = new HopperClient({ + replyContracts: transportReplyContracts, + launcher, + startupTimeoutMs: 10_000, + }); onTestFinished(async () => { const closed = await client.close(); if (!closed.ok) throw closed.error; diff --git a/tests/boundary/providers/hopper/hopperClient.protocol.test.ts b/tests/boundary/providers/hopper/hopperClient.protocol.test.ts index b810f937ac..b58c724e5b 100644 --- a/tests/boundary/providers/hopper/hopperClient.protocol.test.ts +++ b/tests/boundary/providers/hopper/hopperClient.protocol.test.ts @@ -1,24 +1,85 @@ +import { + transportReplyContracts, + type TransportReplyContracts, + HopperFixtureLauncher as FixtureLauncher, + startHopperFixtureClient as startClient, +} from "./hopperClient.fixture.js"; import { once } from "node:events"; -import { describe, expect, it, onTestFinished } from "vitest"; +import { describe, expect, expectTypeOf, it, onTestFinished } from "vitest"; +import { z } from "zod"; import { HopperProcessError } from "../../../../src/domain/hopperErrors.js"; import { HOPPER_OPERATIONS } from "../../../../src/hopper/HopperProviderMetadata.js"; import { HopperClient } from "../../../../src/hopper/HopperClient.js"; +import { HopperReplyContractError } from "../../../../src/hopper/HopperReplyContracts.js"; +import { ConfigurationError } from "../../../../src/domain/configurationErrors.js"; import type { HopperDiagnostic } from "../../../../src/hopper/HopperDiagnostics.js"; import { providerCleanupFailure } from "../../../../src/hopper/HopperDiagnostics.js"; import { projectAnalysisError } from "../../../../src/domain/analysisErrorProjection.js"; -import { - HopperFixtureLauncher as FixtureLauncher, - startHopperFixtureClient as startClient, -} from "./hopperClient.fixture.js"; import { waitFor } from "../../../support/testWait.js"; +const MIB = 1024 * 1024; + describe("HopperClient protocol", () => { + it("refuses a method its installed table has no contract for instead of throwing", async () => { + const launcher = new FixtureLauncher(); + // The type argument names the transport table, but none is installed. + const client = new HopperClient({ + launcher, + startupTimeoutMs: 1_000, + }); + onTestFinished(async () => { + await client.close(); + }); + + expect(await client.callTool("echo", { label: "unread" })).toMatchObject({ + ok: false, + error: { + _tag: "AnalysisCapabilityUnavailableError", + operation: "echo", + }, + }); + }); + + it("validates each reply against its method contract and keeps a rejected reply", async () => { + const launcher = new FixtureLauncher(); + const client = new HopperClient({ + replyContracts: { echo: z.strictObject({ label: z.string() }) }, + launcher, + startupTimeoutMs: 1_000, + }); + onTestFinished(async () => { + await client.close(); + }); + + const accepted = await client.callTool("echo", { label: "typed" }); + expect(accepted).toEqual({ ok: true, value: { label: "typed" } }); + if (accepted.ok) expectTypeOf(accepted.value.label).toBeString(); + + const rejected = await client.callTool( + "echo", + { label: 7 }, + { operation: "list_names" }, + ); + if (rejected.ok) throw new Error("Expected a contract failure"); + expect(rejected.error).toBeInstanceOf(HopperReplyContractError); + expect(rejected.error).toMatchObject({ + _tag: "AnalysisOutputError", + operation: "list_names", + method: "echo", + reply: { label: 7 }, + }); + }); + it("does not start cold and preserves normal dispatch once ready", async () => { const launcher = new FixtureLauncher(); - const client = new HopperClient({ launcher, startupTimeoutMs: 1_000 }); + const client = new HopperClient({ + replyContracts: transportReplyContracts, + launcher, + startupTimeoutMs: 1_000, + }); onTestFinished(async () => { await client.close(); }); @@ -50,6 +111,7 @@ describe("HopperClient protocol", () => { it("uses the caller-assigned provider run identity", async () => { const launcher = new FixtureLauncher(); const client = new HopperClient({ + replyContracts: transportReplyContracts, launcher, runId: "11111111-1111-4111-8111-111111111111", startupTimeoutMs: 1_000, @@ -64,7 +126,11 @@ describe("HopperClient protocol", () => { it("keeps the native socket path below macOS sockaddr_un limits", async () => { const launcher = new FixtureLauncher(); - const client = new HopperClient({ launcher, startupTimeoutMs: 1_000 }); + const client = new HopperClient({ + replyContracts: transportReplyContracts, + launcher, + startupTimeoutMs: 1_000, + }); onTestFinished(async () => { await client.close(); }); @@ -170,6 +236,7 @@ describe("HopperClient response lifecycle", () => { it("rejects a bridge session with the wrong capability token", async () => { const client = new HopperClient({ + replyContracts: transportReplyContracts, launcher: new FixtureLauncher("wrong-token"), startupTimeoutMs: 1_000, }); @@ -222,6 +289,7 @@ describe("HopperClient response lifecycle", () => { const launcher = new FixtureLauncher(); const diagnostics: HopperDiagnostic[] = []; const client = new HopperClient({ + replyContracts: transportReplyContracts, launcher, startupTimeoutMs: 1_000, onDiagnostic: (event) => diagnostics.push(event), @@ -247,6 +315,168 @@ describe("HopperClient response lifecycle", () => { }); }); +describe("HopperClient reply framing", () => { + it("delivers a rejection on the reserved id to the request in flight", async () => { + const client = await startClient(); + + const result = await client.callTool( + "unattributed_invalid_request", + {}, + { timeoutMs: 5_000 }, + ); + + expect( + result.ok ? undefined : result.error, + "an unattributable rejection reaches the caller instead of being dropped", + ).toMatchObject({ + _tag: "HopperRemoteError", + diagnosticType: "invalid_request", + message: expect.stringContaining("Invalid bridge request JSON"), + }); + }); + + it.each([ + ["an unterminated reply", "unterminated_reply", { bytes: 2 * MIB }], + ["a complete reply line", "echo", { label: "x".repeat(2 * MIB) }], + ] as const)( + "refuses %s above bridge.max_reply_bytes and names the setting", + async (_case, method, parameters) => { + const client = await startClient(undefined, { + bridgeMaxReplyBytes: { maximumBytes: MIB, source: "setting" }, + }); + + const result = await client.callTool(method, parameters, { + timeoutMs: 10_000, + }); + + expect(result.ok ? undefined : result.error).toMatchObject({ + _tag: "AnalysisResourceConstraintError", + resource: "transport", + limit: { + setting: "bridge.max_reply_bytes", + effective_value: MIB, + source: "setting", + }, + }); + }, + ); + + it("refuses an invalid reply ceiling seed before launching Hopper", async () => { + const invalid = new ConfigurationError("invalid bridge.max_reply_bytes"); + const launcher = new FixtureLauncher(); + const client = new HopperClient({ + replyContracts: transportReplyContracts, + launcher, + startupTimeoutMs: 10_000, + bridgeMaxReplyBytes: { + get maximumBytes(): number { + throw invalid; + }, + source: "setting", + }, + }); + + const started = await client.start(); + + expect(started.ok ? undefined : started.error).toBe(invalid); + expect(launcher.processes, "no bridge was launched").toEqual([]); + expect((await client.close()).ok).toBe(true); + }); + + it("returns a ceiling seed refused after startup as a configuration error", async () => { + // A malformed seed covered by an override at startup is refused by the + // runtime view once that override is reset. + const refusal = new ConfigurationError("invalid bridge ceiling seed"); + let seedRefused = false; + const setting = (value: number) => ({ + get maximumBytes(): number { + if (seedRefused) throw refusal; + return value; + }, + source: "runtime" as const, + }); + const launcher = new FixtureLauncher(); + const client = await startClient(launcher, { + bridgeMaxRequestBytes: setting(16 * MIB), + bridgeMaxReplyBytes: setting(4 * MIB), + }); + // The gate holds the active reply, so the second request stays queued + // until the seed is refused and is written outside its caller's context. + const active = client.callTool("echo", { label: "active", gate: "held" }); + const held = await launcher.waitForRequest("echo", "held"); + const queued = client.callTool("echo", { label: "queued" }); + await waitFor(() => + expect(client.requestActivity()).toMatchObject({ queuedRequests: 1 }), + ); + seedRefused = true; + + const refused = await client.callTool("echo", { label: "late" }); + expect(refused.ok ? undefined : refused.error).toBe(refusal); + await launcher.release(held); + expect((await active).ok).toBe(true); + expect( + (await queued).ok, + "a queued request is written under the ceilings it was admitted with", + ).toBe(true); + }); + + it("shuts the bridge down under the session's ceilings after its seed is refused", async () => { + // Cleanup must not depend on the current settings: a refused seed would + // otherwise block the shutdown that releases Hopper's document and files. + const refusal = new ConfigurationError("invalid bridge ceiling seed"); + let seedRefused = false; + const setting = (value: number) => ({ + get maximumBytes(): number { + if (seedRefused) throw refusal; + return value; + }, + source: "runtime" as const, + }); + const launcher = new FixtureLauncher(); + const client = await startClient(launcher, { + bridgeMaxRequestBytes: setting(16 * MIB), + bridgeMaxReplyBytes: setting(4 * MIB), + }); + seedRefused = true; + + const closed = await client.close(); + + expect(closed.ok ? undefined : closed.error).toBeUndefined(); + expect( + launcher.requests.map(({ method }) => method), + "the shutdown request reached the bridge", + ).toEqual(expect.arrayContaining([expect.stringMatching(/^shutdown/u)])); + }); + + it("bounds each reply by the ceiling its own request admitted", async () => { + // The getter-backed runtime setting reads whichever caller admits next. + const limit = { maximumBytes: 4 * MIB, source: "runtime" as const }; + const launcher = new FixtureLauncher(); + const client = await startClient(launcher, { bridgeMaxReplyBytes: limit }); + + const large = client.callTool( + "echo", + { label: "x".repeat(2 * MIB), delay: 100 }, + { timeoutMs: 10_000 }, + ); + await waitFor(() => + expect(launcher.requests.map(({ method }) => method)).toContain("echo"), + ); + limit.maximumBytes = MIB; + const queued = client.callTool( + "echo", + { label: "small" }, + { timeoutMs: 10_000 }, + ); + + expect( + (await large).ok, + "a request queued with a lower ceiling does not shrink the active one", + ).toBe(true); + expect((await queued).ok).toBe(true); + }); +}); + it("preserves caller cancellation while a late valid UTF-8 reply drains", async () => { const launcher = new FixtureLauncher(); const client = await startClient(launcher); diff --git a/tests/boundary/providers/hopper/hopperClient.startupFailures.test.ts b/tests/boundary/providers/hopper/hopperClient.startupFailures.test.ts index 9a0c329421..a41c4be0fa 100644 --- a/tests/boundary/providers/hopper/hopperClient.startupFailures.test.ts +++ b/tests/boundary/providers/hopper/hopperClient.startupFailures.test.ts @@ -1,3 +1,8 @@ +import { + transportReplyContracts, + type TransportHopperClient, + HopperFixtureLauncher as FixtureLauncher, +} from "./hopperClient.fixture.js"; import { spawn, type ChildProcess } from "node:child_process"; import { access } from "node:fs/promises"; import { createServer } from "node:net"; @@ -14,8 +19,6 @@ import type { import { HopperClient } from "../../../../src/hopper/HopperClient.js"; import { LINUX_PRIVATE_DISPLAY_DIAGNOSTIC_PREFIX } from "../../../../src/hopper/LinuxPrivateDisplayDiagnostic.js"; -import { HopperFixtureLauncher as FixtureLauncher } from "./hopperClient.fixture.js"; - class SilentLauncher implements BridgeLauncher { launch() { return Promise.resolve( @@ -191,7 +194,7 @@ class BadHealthLauncher implements BridgeLauncher { } } -const clients: HopperClient[] = []; +const clients: TransportHopperClient[] = []; afterEach(async () => { await Promise.all(clients.splice(0).map((client) => client.close())); @@ -200,7 +203,11 @@ afterEach(async () => { describe("HopperClient startup deadline", () => { it("applies one deadline to launcher, socket, and health startup phases", async () => { const launcher = new LateSilentLauncher(); - const client = new HopperClient({ launcher, startupTimeoutMs: 500 }); + const client = new HopperClient({ + replyContracts: transportReplyContracts, + launcher, + startupTimeoutMs: 500, + }); clients.push(client); const result = await client.start(); @@ -235,6 +242,7 @@ describe("HopperClient startup failures", () => { it("cancels bridge startup without waiting for the startup timeout", async () => { const launcher = new CancelThenFixtureLauncher(); const client = new HopperClient({ + replyContracts: transportReplyContracts, launcher, // Keep the retry proof independent from scheduler pressure when the // complete boundary project is starting several child-process fixtures. @@ -276,7 +284,11 @@ describe("HopperClient startup failures", () => { it("retries after cancellation before the launcher starts", async () => { const launcher = new FixtureLauncher(); - const client = new HopperClient({ launcher, startupTimeoutMs: 30_000 }); + const client = new HopperClient({ + replyContracts: transportReplyContracts, + launcher, + startupTimeoutMs: 30_000, + }); clients.push(client); const controller = new AbortController(); @@ -309,7 +321,11 @@ describe("HopperClient startup failures", () => { it("records unknown when the health handshake is invalid", async () => { const launcher = new BadHealthLauncher(); - const client = new HopperClient({ launcher, startupTimeoutMs: 30_000 }); + const client = new HopperClient({ + replyContracts: transportReplyContracts, + launcher, + startupTimeoutMs: 30_000, + }); clients.push(client); const result = await client.start(); @@ -326,6 +342,7 @@ describe("HopperClient startup failures", () => { "reports Linux adapter exit %i during bridge startup", async (exitCode) => { const client = new HopperClient({ + replyContracts: transportReplyContracts, launcher: new ExitingLauncher(exitCode), startupTimeoutMs: 10_000, }); @@ -342,6 +359,7 @@ describe("HopperClient startup failures", () => { it("projects actionable CI remediation for Hopper UI, license, and lifecycle failures", async () => { const client = new HopperClient({ + replyContracts: transportReplyContracts, launcher: new ExitingLauncher(75), startupTimeoutMs: 10_000, }); @@ -362,6 +380,7 @@ describe("HopperClient startup failures", () => { it("preserves the private-display diagnostic from an adapter exit", async () => { const client = new HopperClient({ + replyContracts: transportReplyContracts, launcher: new DiagnosticExitingLauncher(), startupTimeoutMs: 10_000, }); diff --git a/tests/boundary/providers/hopper/hopperClient.startupWaiters.test.ts b/tests/boundary/providers/hopper/hopperClient.startupWaiters.test.ts index 4e0c22e8de..088d6d607e 100644 --- a/tests/boundary/providers/hopper/hopperClient.startupWaiters.test.ts +++ b/tests/boundary/providers/hopper/hopperClient.startupWaiters.test.ts @@ -1,3 +1,9 @@ +import { + transportReplyContracts, + type TransportHopperClient, + HopperFixtureLauncher, + trackHopperClient, +} from "./hopperClient.fixture.js"; import { access } from "node:fs/promises"; import { expect, it, vi } from "vitest"; @@ -10,15 +16,13 @@ import { HopperClient } from "../../../../src/hopper/HopperClient.js"; import { ok } from "../../../../src/domain/result.js"; import { ProviderRunLineage } from "../../../../src/process/ProviderRunLineage.js"; import { waitForExit } from "../../../support/process/processFixture.js"; -import { - HopperFixtureLauncher, - trackHopperClient, -} from "./hopperClient.fixture.js"; import { waitFor } from "../../../support/testWait.js"; it("preserves a warm provider when an already cancelled caller joins", async () => { const launcher = new HopperFixtureLauncher(); - const client = trackHopperClient(new HopperClient({ launcher })); + const client = trackHopperClient( + new HopperClient({ replyContracts: transportReplyContracts, launcher }), + ); await expect(client.start()).resolves.toMatchObject({ ok: true }); const controller = new AbortController(); const reason = { source: "caller", opaque: 7 }; @@ -41,7 +45,11 @@ it("preserves a warm provider when an already cancelled caller joins", async () it("reports provider closure to live startup callers without cancelling their signals", async () => { const launcher = new HopperFixtureLauncher(undefined, "quiet", "gated"); const client = trackHopperClient( - new HopperClient({ launcher, startupTimeoutMs: 10_000 }), + new HopperClient({ + replyContracts: transportReplyContracts, + launcher, + startupTimeoutMs: 10_000, + }), ); const first = new AbortController(); const second = new AbortController(); @@ -67,6 +75,7 @@ it("retains the final caller's cleanup failure and blocks replacement until clos let allowCleanup = false; const client = trackHopperClient( new HopperClient({ + replyContracts: transportReplyContracts, startupTimeoutMs: 10_000, launcher: { async launch(session) { @@ -123,6 +132,7 @@ it.each([0, 1])( const launcher = new HopperFixtureLauncher(); const client = trackHopperClient( new HopperClient({ + replyContracts: transportReplyContracts, startupTimeoutMs: 10_000, launcher: { async launch(session) { @@ -139,7 +149,9 @@ it.each([0, 1])( ); try { await launchPending; - let cancelled: Awaited> | undefined; + let cancelled: + | Awaited> + | undefined; void waits[cancelledIndex]?.then((result) => { cancelled = result; }); @@ -195,7 +207,11 @@ it.each(["cancelled", "rejected"] as const)( }); const launcher = new HopperFixtureLauncher(); const client = trackHopperClient( - new HopperClient({ launcher, startupTimeoutMs: 10_000 }), + new HopperClient({ + replyContracts: transportReplyContracts, + launcher, + startupTimeoutMs: 10_000, + }), ); const controller = new AbortController(); const starting = client.start(controller.signal); @@ -250,7 +266,9 @@ it("keeps a post-connect launcher exit ahead of cancellation during lineage fina await original.call(this, launch); }); const launcher = new HopperFixtureLauncher(); - const client = trackHopperClient(new HopperClient({ launcher })); + const client = trackHopperClient( + new HopperClient({ replyContracts: transportReplyContracts, launcher }), + ); const controller = new AbortController(); const starting = client.start(controller.signal); try { @@ -282,7 +300,11 @@ it("keeps a post-connect launcher exit ahead of cancellation during lineage fina it("keeps an unstarted client reusable after an already cancelled startup wait", async () => { const launcher = new HopperFixtureLauncher(); const client = trackHopperClient( - new HopperClient({ launcher, startupTimeoutMs: 30_000 }), + new HopperClient({ + replyContracts: transportReplyContracts, + launcher, + startupTimeoutMs: 30_000, + }), ); const controller = new AbortController(); controller.abort(); diff --git a/tests/boundary/providers/hopper/hopperRegexFailure.test.ts b/tests/boundary/providers/hopper/hopperRegexFailure.test.ts index 821b00605c..923393db92 100644 --- a/tests/boundary/providers/hopper/hopperRegexFailure.test.ts +++ b/tests/boundary/providers/hopper/hopperRegexFailure.test.ts @@ -1,10 +1,11 @@ +import { HopperClient } from "../../../../src/hopper/HopperClient.js"; import { expect, it } from "vitest"; import { HopperRegexSearch } from "../../../../src/hopper/HopperRegexSearch.js"; import { HopperRemoteError } from "../../../../src/domain/hopperErrors.js"; import { projectAnalysisError } from "../../../../src/domain/analysisErrorProjection.js"; import { HopperFixtureLauncher, - startHopperFixtureClient, + trackHopperClient, } from "./hopperClient.fixture.js"; it.each( @@ -23,7 +24,12 @@ it.each( "acknowledge", failure, ); - const client = await startHopperFixtureClient(launcher); + // Production reply contracts: the inventory reply is validated by the client. + const client = trackHopperClient( + new HopperClient({ launcher, startupTimeoutMs: 10_000 }), + ); + const started = await client.start(); + if (!started.ok) throw started.error; const search = new HopperRegexSearch(client); try { const result = await search.execute(operation, { @@ -71,7 +77,12 @@ it.each(["search_strings", "search_procedures"] as const)( "preserves successful %s search and caller-selected document", async (operation) => { const launcher = new HopperFixtureLauncher(); - const client = await startHopperFixtureClient(launcher); + // Production reply contracts: the inventory reply is validated by the client. + const client = trackHopperClient( + new HopperClient({ launcher, startupTimeoutMs: 10_000 }), + ); + const started = await client.start(); + if (!started.ok) throw started.error; const search = new HopperRegexSearch(client); try { expect( diff --git a/tests/boundary/providers/kuna/kunaCacheCleanupConcurrency.test.ts b/tests/boundary/providers/kuna/kunaCacheCleanupConcurrency.test.ts index 6a59c62392..559770d36c 100644 --- a/tests/boundary/providers/kuna/kunaCacheCleanupConcurrency.test.ts +++ b/tests/boundary/providers/kuna/kunaCacheCleanupConcurrency.test.ts @@ -11,6 +11,7 @@ import { spawnOwnedProviderProcess } from "../../../../src/process/ProviderProce import { createTestTempDirectory } from "../../../fixtures/temporaryDirectory.js"; import { writeFakeKuna, fakeKunaCalls } from "./fakeKuna.js"; import { inspectKunaInstallationOwned } from "./kunaResourceScope.js"; +import { withKunaProjectDeadline } from "./kunaProjectRequest.js"; const test = it.skipIf(process.platform === "win32"); test.each([false, true])( @@ -72,12 +73,14 @@ test.each([false, true])( jobs: installed.value.configuration.jobs, }); keys.push(key); - const pending = cache.build({ - installation: installed.value, - targetPath, - targetSha256, - signal: controllers[index]!.signal, - }); + const pending = cache.build( + withKunaProjectDeadline({ + installation: installed.value, + targetPath, + targetSha256, + signal: controllers[index]!.signal, + }), + ); builds.push(pending); await expect .poll(async () => diff --git a/tests/boundary/providers/kuna/kunaDeadlineRecovery.test.ts b/tests/boundary/providers/kuna/kunaDeadlineRecovery.test.ts index af949f5aa9..f03057bbce 100644 --- a/tests/boundary/providers/kuna/kunaDeadlineRecovery.test.ts +++ b/tests/boundary/providers/kuna/kunaDeadlineRecovery.test.ts @@ -4,52 +4,62 @@ import { session } from "./kunaSessionFixture.js"; const it = test.skipIf(process.platform === "win32"); -it("retries inventory after a reset exposes an invalid function deadline", async () => { - const snapshot = { - revision: 1, - overrides: { "kuna.function_timeout_ms": 1000 }, - } as const; - const fixture = await runWithAnalysisOperationSnapshot(snapshot, () => - session( - "rea-kuna-deadline-inventory-", - {}, - { REA_KUNA_TIMEOUT_MS: "invalid" }, - ), - ); - const client = runWithAnalysisOperationSnapshot(snapshot, () => - fixture.open(), - ); - try { - const ready = await runWithAnalysisOperationSnapshot(snapshot, () => - client.execute("health", {}), - ); - expect(ready.ok).toBe(true); - const refused = await client.execute("list_procedures", {}); - expect(refused).toMatchObject({ - ok: false, - error: { - _tag: "ConfigurationError", - settings: [{ setting: "REA_KUNA_TIMEOUT_MS", update_mode: "runtime" }], - remediationAction: expect.stringContaining("kuna.function_timeout_ms"), - }, - }); - const retried = await runWithAnalysisOperationSnapshot(snapshot, () => - client.execute( - "list_procedures", +it.each([ + ["list_procedures", {}], + ["procedure_pseudo_code", { procedure: "0x1000" }], +] as const)( + "retries %s after a reset exposes an invalid function deadline", + async (operation, parameters) => { + const snapshot = { + revision: 1, + overrides: { "kuna.function_timeout_ms": 1000 }, + } as const; + const fixture = await runWithAnalysisOperationSnapshot(snapshot, () => + session( + "rea-kuna-deadline-inventory-", {}, - { signal: AbortSignal.timeout(3000) }, + { REA_KUNA_TIMEOUT_MS: "invalid" }, ), ); - expect(retried).toMatchObject({ ok: true }); - } finally { - await client.close(); - } -}); + const client = runWithAnalysisOperationSnapshot(snapshot, () => + fixture.open(), + ); + try { + const ready = await runWithAnalysisOperationSnapshot(snapshot, () => + client.execute("health", {}), + ); + expect(ready.ok).toBe(true); + const refused = await client.execute(operation, parameters); + expect(refused).toMatchObject({ + ok: false, + error: { + _tag: "ConfigurationError", + settings: [ + { setting: "REA_KUNA_TIMEOUT_MS", update_mode: "runtime" }, + ], + remediationAction: expect.stringContaining( + "kuna.function_timeout_ms", + ), + }, + }); + const retried = await runWithAnalysisOperationSnapshot(snapshot, () => + client.execute(operation, parameters, { + signal: AbortSignal.timeout(3000), + }), + ); + expect(retried).toMatchObject({ ok: true }); + } finally { + await client.close(); + } + }, +); it.each([ ["list_strings", {}], ["procedure_callers", { procedure: "main" }], ["xrefs", { address: "0x1000" }], + ["procedure_assembly", { procedure: "0x1000" }], + ["analyze_function", { procedure: "0x1000" }], ] as const)( "retries %s after a reset exposes an invalid project deadline", async (operation, parameters) => { @@ -94,3 +104,44 @@ it.each([ } }, ); + +it("refuses analyze_function on a reused export when its call-graph deadline is invalid", async () => { + const snapshot = { + revision: 1, + overrides: { "kuna.project_timeout_ms": 1000 }, + } as const; + const fixture = await runWithAnalysisOperationSnapshot(snapshot, () => + session( + "rea-kuna-deadline-dossier-", + {}, + { REA_KUNA_PROJECT_TIMEOUT_MS: "invalid" }, + ), + ); + const client = runWithAnalysisOperationSnapshot(snapshot, () => + fixture.open(), + ); + try { + // Build the export under the override, without reading the call graph. + const exported = await runWithAnalysisOperationSnapshot(snapshot, () => + client.execute( + "procedure_assembly", + { procedure: "0x1000" }, + { signal: AbortSignal.timeout(3000) }, + ), + ); + expect(exported).toMatchObject({ ok: true }); + // The dossier's call relationships need a graph scan the reset refuses. + const refused = await client.execute("analyze_function", { + procedure: "0x1000", + }); + expect(refused).toMatchObject({ + ok: false, + error: { + _tag: "ConfigurationError", + settings: [{ setting: "REA_KUNA_PROJECT_TIMEOUT_MS" }], + }, + }); + } finally { + await client.close(); + } +}); diff --git a/tests/boundary/providers/kuna/kunaHashOwnerPropagation.test.ts b/tests/boundary/providers/kuna/kunaHashOwnerPropagation.test.ts index 6d4264a98c..6b733b7039 100644 --- a/tests/boundary/providers/kuna/kunaHashOwnerPropagation.test.ts +++ b/tests/boundary/providers/kuna/kunaHashOwnerPropagation.test.ts @@ -205,6 +205,7 @@ import { kunaInstallationCancellation } from "../../../../src/kuna/KunaInstallat import { inspectKunaInstallationOwned } from "./kunaResourceScope.js"; import { fakeKunaCalls } from "./fakeKuna.js"; import { run, session } from "./kunaSessionFixture.js"; +import { withKunaProjectDeadline } from "./kunaProjectRequest.js"; it.skipIf(process.platform === "win32").each([ { operation: "list_strings", input: {}, command: "strings" }, @@ -420,11 +421,13 @@ it.skipIf(process.platform === "win32")( injected.fingerprintReads = 0; injected.failReadStage = "fingerprint"; try { - const result = await cache.build({ - installation: installed.value, - targetPath: fixture.target.path, - targetSha256: fixture.target.sha256, - }); + const result = await cache.build( + withKunaProjectDeadline({ + installation: installed.value, + targetPath: fixture.target.path, + targetSha256: fixture.target.sha256, + }), + ); if (result.ok) throw new Error("injected export hash read passed"); const originatingError = result.error; expect(originatingError).toMatchObject({ diff --git a/tests/boundary/providers/kuna/kunaProjectCache.test.ts b/tests/boundary/providers/kuna/kunaProjectCache.test.ts index 902b6d69ff..426e262927 100644 --- a/tests/boundary/providers/kuna/kunaProjectCache.test.ts +++ b/tests/boundary/providers/kuna/kunaProjectCache.test.ts @@ -40,6 +40,7 @@ import { writeFakeKuna, type FakeKunaBehavior, } from "./fakeKuna.js"; +import { withKunaProjectDeadline } from "./kunaProjectRequest.js"; const it = test.skipIf(process.platform === "win32"); @@ -128,7 +129,9 @@ it.each([1, 2])( "rea-kuna-cache-format-transition-", ); const input = await target("program"); - const initial = await cache.build({ installation, ...input }); + const initial = await cache.build( + withKunaProjectDeadline({ installation, ...input }), + ); if (!initial.ok) throw initial.error; expect((await cache.close()).ok).toBe(true); // Seed the previously shipped manifest and content-key format using a real @@ -183,7 +186,9 @@ it.each([1, 2])( await writeFile(join(unresolved, "sentinel"), "preserve"); const migrated = newCache(installation); try { - const built = await migrated.build({ installation, ...input }); + const built = await migrated.build( + withKunaProjectDeadline({ installation, ...input }), + ); if (!built.ok) throw built.error; await expect(access(legacyEntry)).rejects.toMatchObject({ code: "ENOENT", @@ -199,7 +204,9 @@ it.each([1, 2])( expect(await callsOf(kuna, "decompile-project")).toHaveLength(2); const fresh = newCache(installation); try { - const reused = await fresh.build({ installation, ...input }); + const reused = await fresh.build( + withKunaProjectDeadline({ installation, ...input }), + ); if (!reused.ok) throw reused.error; expect(reused.value.manifest).toEqual(built.value.manifest); expect(await callsOf(kuna, "decompile-project")).toHaveLength(2); @@ -250,7 +257,9 @@ it("publishes a verified export and serves later requests from the cache", async ); const input = await target("program"); - const built = await cache.build({ installation, ...input }); + const built = await cache.build( + withKunaProjectDeadline({ installation, ...input }), + ); expect(built).toMatchObject({ ok: true, @@ -315,7 +324,7 @@ it("publishes a verified export and serves later requests from the cache", async expect(await entries(cacheRoot, "staging")).toEqual([]); // The same process and a fresh one both reuse the published entry. - await cache.build({ installation, ...input }); + await cache.build(withKunaProjectDeadline({ installation, ...input })); expect(await newCache(installation).lookup(built.value.key)).toMatchObject({ functions: built.value.functions, inventory: built.value.inventory, @@ -328,7 +337,9 @@ it("retains a failed warm-cache hash descriptor and retries it at cache close", "rea-kuna-cache-hash-owner-", ); const input = await target("program"); - const built = await cache.build({ installation, ...input }); + const built = await cache.build( + withKunaProjectDeadline({ installation, ...input }), + ); if (!built.ok) throw built.error; const closeCause = new Error("injected warm-cache descriptor close failure"); const close = vi @@ -362,7 +373,9 @@ it.each(["success", "failure"] as const)( `rea-kuna-cache-hash-race-${retryOutcome}-`, ); const input = await target("program"); - const built = await cache.build({ installation, ...input }); + const built = await cache.build( + withKunaProjectDeadline({ installation, ...input }), + ); if (!built.ok) throw built.error; const warmCache = newCache(installation); @@ -467,10 +480,12 @@ it("resolves a hash failure delivered after its retained owner was released", as const { installation, cache, target } = await fixture( "rea-kuna-cache-late-hash-", ); - const built = await cache.build({ - installation, - ...(await target("program")), - }); + const built = await cache.build( + withKunaProjectDeadline({ + installation, + ...(await target("program")), + }), + ); if (!built.ok) throw built.error; const warmCache = newCache(installation); const failureRetained = deferred(); @@ -530,12 +545,16 @@ it("blocks repeated cache reads while a retained hash descriptor cannot close", "rea-kuna-cache-hash-admission-", ); const input = await target("program"); - const built = await cache.build({ installation, ...input }); + const built = await cache.build( + withKunaProjectDeadline({ installation, ...input }), + ); if (!built.ok) throw built.error; - const otherBuilt = await cache.build({ - installation, - ...(await target("other")), - }); + const otherBuilt = await cache.build( + withKunaProjectDeadline({ + installation, + ...(await target("other")), + }), + ); if (!otherBuilt.ok) throw otherBuilt.error; const keys = [built.value.key, otherBuilt.value.key] as const; @@ -637,10 +656,12 @@ it("keeps hash cleanup aggregates pending through retained staging removal", asy let buildFailure: AnalysisError | undefined; const aggregates: ProviderCleanupError[] = []; try { - const built = await cache.build({ - installation, - ...(await target("program")), - }); + const built = await cache.build( + withKunaProjectDeadline({ + installation, + ...(await target("program")), + }), + ); if (built.ok) throw new Error("Expected the retained hash close to fail"); expect(built.error.cleanupIncomplete).toBe(true); buildFailure = built.error; @@ -691,8 +712,8 @@ it("shares one Kuna run between concurrent requests for one export", async () => const input = await target("program"); const [first, second] = await Promise.all([ - cache.build({ installation, ...input }), - cache.build({ installation, ...input }), + cache.build(withKunaProjectDeadline({ installation, ...input })), + cache.build(withKunaProjectDeadline({ installation, ...input })), ]); expect(first.ok && second.ok).toBe(true); @@ -710,11 +731,13 @@ it("preserves a pre-aborted caller reason without launching an export", async () const reason = { caller: "before admission" }; controller.abort(reason); const callsBefore = await fakeKunaCalls(kuna); - const result = await cache.build({ - installation, - ...input, - signal: controller.signal, - }); + const result = await cache.build( + withKunaProjectDeadline({ + installation, + ...input, + signal: controller.signal, + }), + ); if (result.ok) throw new Error("Expected pre-aborted cache request to cancel"); expect(result.error._tag).toBe("AnalysisCancelledError"); @@ -732,16 +755,20 @@ it("stops the run only when every waiting request is cancelled", async () => { const firstCaller = new AbortController(); const secondCaller = new AbortController(); - const first = cache.build({ - installation, - ...input, - signal: firstCaller.signal, - }); - const second = cache.build({ - installation, - ...input, - signal: secondCaller.signal, - }); + const first = cache.build( + withKunaProjectDeadline({ + installation, + ...input, + signal: firstCaller.signal, + }), + ); + const second = cache.build( + withKunaProjectDeadline({ + installation, + ...input, + signal: secondCaller.signal, + }), + ); await expect.poll(() => callsOf(kuna, "decompile-project")).toHaveLength(1); const pid = (await callsOf(kuna, "decompile-project"))[0]?.pid ?? 0; @@ -779,10 +806,12 @@ it("applies one deadline to the inventory and the export together", async () => { environment: { REA_KUNA_PROJECT_TIMEOUT_MS: "800" } }, ); - const result = await cache.build({ - installation, - ...(await target("program")), - }); + const result = await cache.build( + withKunaProjectDeadline({ + installation, + ...(await target("program")), + }), + ); expect(result).toMatchObject({ ok: false, @@ -794,7 +823,9 @@ it("applies one deadline to the inventory and the export together", async () => await setFakeKunaBehavior(kuna, { inventory: "hang" }); expect( - await cache.build({ installation, ...(await target("other")) }), + await cache.build( + withKunaProjectDeadline({ installation, ...(await target("other")) }), + ), ).toMatchObject({ ok: false, error: { _tag: "AnalysisTimeoutError" } }); }); @@ -804,10 +835,12 @@ it("reports a failed export with Kuna's diagnostics and retains nothing", async { project: "exit-failure" }, ); - const result = await cache.build({ - installation, - ...(await target("program")), - }); + const result = await cache.build( + withKunaProjectDeadline({ + installation, + ...(await target("program")), + }), + ); expect(result).toMatchObject({ ok: false, @@ -845,7 +878,9 @@ it.each([ ); expect( - await cache.build({ installation, ...(await target("program")) }), + await cache.build( + withKunaProjectDeadline({ installation, ...(await target("program")) }), + ), ).toMatchObject({ ok: false, error: { @@ -863,7 +898,9 @@ it("refuses to decompile a target that no longer matches its digest", async () = const input = await target("program"); await appendFile(input.targetPath, "patched"); - expect(await cache.build({ installation, ...input })).toMatchObject({ + expect( + await cache.build(withKunaProjectDeadline({ installation, ...input })), + ).toMatchObject({ ok: false, error: { _tag: "AnalysisArtifactChangedError", path: input.targetPath }, }); @@ -875,11 +912,13 @@ it("removes its staging when the target disappears before the copy", async () => await fixture("rea-kuna-vanished-"); const input = await target("program"); - const result = await cache.build({ - installation, - ...input, - targetPath: `${input.targetPath}.gone`, - }); + const result = await cache.build( + withKunaProjectDeadline({ + installation, + ...input, + targetPath: `${input.targetPath}.gone`, + }), + ); expect(result).toMatchObject({ ok: false, @@ -895,7 +934,9 @@ it("discards an entry whose files no longer match their manifest", async () => { const { installation, cacheRoot, cache, target } = await fixture("rea-kuna-tamper-"); const input = await target("program"); - const built = await cache.build({ installation, ...input }); + const built = await cache.build( + withKunaProjectDeadline({ installation, ...input }), + ); if (!built.ok) throw built.error; await appendFile(join(built.value.directory, "program.c"), "// edited\n"); @@ -909,7 +950,9 @@ it("evicts least recently used entries above the cap, never a pinned one", async }); const cache = newCache(installation, 1024 * 1024); const build = async (name: string, ageSeconds: number) => { - const result = await cache.build({ installation, ...(await target(name)) }); + const result = await cache.build( + withKunaProjectDeadline({ installation, ...(await target(name)) }), + ); if (!result.ok) throw result.error; const when = new Date(Date.now() - ageSeconds * 1000); await utimes(join(cacheRoot, "projects", result.value.key), when, when); @@ -938,10 +981,12 @@ it("refuses an export larger than the whole cache may hold", async () => { { asmPadding: 8 * 1024 }, ); - const result = await newCache(installation, 4096).build({ - installation, - ...(await target("program")), - }); + const result = await newCache(installation, 4096).build( + withKunaProjectDeadline({ + installation, + ...(await target("program")), + }), + ); expect(result).toMatchObject({ ok: false, @@ -963,8 +1008,12 @@ it("keeps the first publication when another process exports the same key", asyn // Two cache instances stand in for two REA processes sharing one root. const [first, second] = await Promise.all([ - newCache(installation).build({ installation, ...input }), - newCache(installation).build({ installation, ...input }), + newCache(installation).build( + withKunaProjectDeadline({ installation, ...input }), + ), + newCache(installation).build( + withKunaProjectDeadline({ installation, ...input }), + ), ]); expect(await callsOf(kuna, "decompile-project")).toHaveLength(2); @@ -981,11 +1030,13 @@ it("reports each build phase to every waiting request", async () => { const { installation, cache, target } = await fixture("rea-kuna-phases-"); const statuses: KunaExportStatus[] = []; - const result = await cache.build({ - installation, - ...(await target("program")), - onStatus: (status) => statuses.push(status), - }); + const result = await cache.build( + withKunaProjectDeadline({ + installation, + ...(await target("program")), + onStatus: (status) => statuses.push(status), + }), + ); expect(result.ok).toBe(true); expect( @@ -1014,7 +1065,11 @@ it("sweeps retired entries but preserves staging with unresolved process ownersh await utimes(abandoned, twoDaysAgo, twoDaysAgo); expect( - (await cache.build({ installation, ...(await target("program")) })).ok, + ( + await cache.build( + withKunaProjectDeadline({ installation, ...(await target("program")) }), + ) + ).ok, ).toBe(true); expect(await entries(cacheRoot, "staging")).toEqual([ @@ -1037,7 +1092,9 @@ it.each(["sweep", "close"] as const)( let built; let closed; try { - built = await cache.build({ installation, ...(await target("program")) }); + built = await cache.build( + withKunaProjectDeadline({ installation, ...(await target("program")) }), + ); closed = await cache.close(); } finally { await chmod(retired, 0o700); @@ -1052,10 +1109,12 @@ it.each(["sweep", "close"] as const)( "retained export", ); if (recovery === "sweep") { - const next = await cache.build({ - installation, - ...(await target("other")), - }); + const next = await cache.build( + withKunaProjectDeadline({ + installation, + ...(await target("other")), + }), + ); if (!next.ok) throw next.error; } expect(await cache.close()).toEqual({ ok: true, value: null }); @@ -1092,11 +1151,13 @@ it("keeps staging owned when a cancelled Kuna process cannot be proven stopped", }, }); const caller = new AbortController(); - const build = cache.build({ - installation, - ...(await target("program")), - signal: caller.signal, - }); + const build = cache.build( + withKunaProjectDeadline({ + installation, + ...(await target("program")), + signal: caller.signal, + }), + ); await expect.poll(() => callsOf(kuna, "decompile-project")).toHaveLength(1); caller.abort(); @@ -1124,10 +1185,12 @@ it("keeps staging owned when a cancelled Kuna process cannot be proven stopped", await utimes(retained, twoDaysAgo, twoDaysAgo); await setFakeKunaBehavior(kuna, {}); const other = newCache(installation); - const built = await other.build({ - installation, - ...(await target("other")), - }); + const built = await other.build( + withKunaProjectDeadline({ + installation, + ...(await target("other")), + }), + ); if (!built.ok) throw built.error; expect(await entries(cacheRoot, "staging")).toEqual(staged); expect(await readFile(join(retained, "input", "program"), "utf8")).toBe( @@ -1148,7 +1211,9 @@ it("rebuilds an entry another cache instance evicted after this one verified it" "rea-kuna-evicted-elsewhere-", ); const input = await target("program"); - const built = await cache.build({ installation, ...input }); + const built = await cache.build( + withKunaProjectDeadline({ installation, ...input }), + ); if (!built.ok) throw built.error; // Another process retires the entry the way #discard does. await mkdir(join(cacheRoot, "trash"), { recursive: true }); @@ -1158,7 +1223,9 @@ it("rebuilds an entry another cache instance evicted after this one verified it" ); expect(await cache.lookup(built.value.key)).toBeUndefined(); - const rebuilt = await cache.build({ installation, ...input }); + const rebuilt = await cache.build( + withKunaProjectDeadline({ installation, ...input }), + ); expect(rebuilt.ok).toBe(true); expect(await callsOf(kuna, "decompile-project")).toHaveLength(2); @@ -1193,11 +1260,13 @@ it("reports an abandoned build's unproven cleanup even after a retry replaced it }); const input = await target("program"); const first = new AbortController(); - const firstBuild = cache.build({ - installation, - ...input, - signal: first.signal, - }); + const firstBuild = cache.build( + withKunaProjectDeadline({ + installation, + ...input, + signal: first.signal, + }), + ); const second = new AbortController(); let secondBuild: ReturnType | undefined; let stopping: ReturnType | undefined; @@ -1213,11 +1282,13 @@ it("reports an abandoned build's unproven cleanup even after a retry replaced it ); // A retry for the same key starts while the first is still stopping. - secondBuild = cache.build({ - installation, - ...input, - signal: second.signal, - }); + secondBuild = cache.build( + withKunaProjectDeadline({ + installation, + ...input, + signal: second.signal, + }), + ); await expect.poll(() => callsOf(kuna, "decompile-project")).toHaveLength(2); second.abort(); await secondBuild; @@ -1246,10 +1317,12 @@ it("removes its staging when publication itself fails", async () => { { project: "block-manifest" }, ); - const result = await cache.build({ - installation, - ...(await target("program")), - }); + const result = await cache.build( + withKunaProjectDeadline({ + installation, + ...(await target("program")), + }), + ); expect(result).toMatchObject({ ok: false, @@ -1293,7 +1366,9 @@ it.each(["missing", "corrupt"])( expect(await readFile(join(decoyEntry, "manifest.json"), "utf8")).toBe( "{}", ); - const built = await cache.build({ installation, ...input }); + const built = await cache.build( + withKunaProjectDeadline({ installation, ...input }), + ); if (!built.ok) throw built.error; expect(built.value.directory).toBe(join(intendedEntry, "project")); expect(await reopened.lookup(key)).toEqual(built.value); @@ -1322,10 +1397,12 @@ it("invalidates a warm export when same-length content changes", async () => { "rea-kuna-warm-integrity-", ); try { - const built = await cache.build({ - installation, - ...(await target("program")), - }); + const built = await cache.build( + withKunaProjectDeadline({ + installation, + ...(await target("program")), + }), + ); if (!built.ok) throw built.error; const file = join(built.value.directory, built.value.manifest.files.c.path); const original = await readFile(file, "utf8"); @@ -1341,10 +1418,12 @@ it("refuses escaped manifest paths before reading export files and preserves the const { installation, cacheRoot, cache, target } = await fixture( "rea-kuna-escaped-manifest-", ); - const built = await cache.build({ - installation, - ...(await target("program")), - }); + const built = await cache.build( + withKunaProjectDeadline({ + installation, + ...(await target("program")), + }), + ); if (!built.ok) throw built.error; expect((await cache.close()).ok).toBe(true); const outside = join(dirname(cacheRoot), "saved-project"); @@ -1380,10 +1459,12 @@ it.each(["h", "readme", "inventory"] as const)( const { installation, cache, target } = await fixture( "rea-kuna-role-manifest-", ); - const built = await cache.build({ - installation, - ...(await target("program")), - }); + const built = await cache.build( + withKunaProjectDeadline({ + installation, + ...(await target("program")), + }), + ); if (!built.ok) throw built.error; expect((await cache.close()).ok).toBe(true); const manifest = structuredClone(built.value.manifest); @@ -1412,10 +1493,12 @@ it.each(["entry", "project", "manifest"] as const)( const { installation, cacheRoot, cache, target } = await fixture( "rea-kuna-linked-entry-", ); - const built = await cache.build({ - installation, - ...(await target("program")), - }); + const built = await cache.build( + withKunaProjectDeadline({ + installation, + ...(await target("program")), + }), + ); if (!built.ok) throw built.error; expect((await cache.close()).ok).toBe(true); const reopened = newCache(installation); @@ -1456,10 +1539,12 @@ it("keeps an empty symlinked projects parent outside its removal authority", asy const { installation, cacheRoot, cache, target } = await fixture( "rea-kuna-linked-projects-", ); - const built = await cache.build({ - installation, - ...(await target("program")), - }); + const built = await cache.build( + withKunaProjectDeadline({ + installation, + ...(await target("program")), + }), + ); if (!built.ok) throw built.error; expect((await cache.close()).ok).toBe(true); const outside = join(dirname(cacheRoot), "empty-projects"); @@ -1485,10 +1570,12 @@ it("preserves a legitimate POSIX backslash in the common export basename", async const { installation, cache, target } = await fixture( "rea-kuna-native-basename-", ); - const built = await cache.build({ - installation, - ...(await target("program\\name")), - }); + const built = await cache.build( + withKunaProjectDeadline({ + installation, + ...(await target("program\\name")), + }), + ); if (!built.ok) throw built.error; expect((await cache.close()).ok).toBe(true); const reopened = newCache(installation); @@ -1513,10 +1600,12 @@ it.each(["invalid JSON", "changed producing inputs", "valid rewrite"] as const)( const { installation, cache, target } = await fixture( "rea-kuna-warm-manifest-", ); - const built = await cache.build({ - installation, - ...(await target("program")), - }); + const built = await cache.build( + withKunaProjectDeadline({ + installation, + ...(await target("program")), + }), + ); if (!built.ok) throw built.error; expect(await cache.lookup(built.value.key)).toBeDefined(); const manifest = structuredClone(built.value.manifest); @@ -1549,12 +1638,16 @@ it("separates equal target bytes whose native basenames change the export", asyn const { kuna, installation, cache, target } = await fixture( "rea-kuna-basename-key-", ); - const first = await cache.build({ - installation, - ...(await target("first", "same bytes")), - }); + const first = await cache.build( + withKunaProjectDeadline({ + installation, + ...(await target("first", "same bytes")), + }), + ); const secondInput = await target("second", "same bytes"); - const second = await cache.build({ installation, ...secondInput }); + const second = await cache.build( + withKunaProjectDeadline({ installation, ...secondInput }), + ); if (!first.ok) throw first.error; if (!second.ok) throw second.error; expect(second.value.key).not.toBe(first.value.key); @@ -1571,7 +1664,9 @@ it("separates equal target bytes whose native basenames change the export", asyn expect((await cache.close()).ok).toBe(true); const reopened = newCache(installation); try { - const reused = await reopened.build({ installation, ...secondInput }); + const reused = await reopened.build( + withKunaProjectDeadline({ installation, ...secondInput }), + ); if (!reused.ok) throw reused.error; expect(reused.value.manifest).toEqual(second.value.manifest); expect(await callsOf(kuna, "decompile-project")).toHaveLength(2); diff --git a/tests/boundary/providers/kuna/kunaProjectConcurrency.test.ts b/tests/boundary/providers/kuna/kunaProjectConcurrency.test.ts index f536c09c81..fddd22bf15 100644 --- a/tests/boundary/providers/kuna/kunaProjectConcurrency.test.ts +++ b/tests/boundary/providers/kuna/kunaProjectConcurrency.test.ts @@ -10,6 +10,7 @@ import { KunaProjectCache } from "../../../../src/kuna/KunaProjectCache.js"; import type { KunaExportStatus } from "../../../../src/kuna/KunaProjectExport.js"; import { createTestTempDirectory } from "../../../fixtures/temporaryDirectory.js"; import { fakeKunaCalls, writeFakeKuna } from "./fakeKuna.js"; +import { withKunaProjectDeadline } from "./kunaProjectRequest.js"; // Reproduce concurrent eviction's shared directory snapshot while retaining // real exports, publication, hashing and file removal underneath it. @@ -117,7 +118,7 @@ it.each([true, false])( const releases: (() => void)[] = []; const inputs = await Promise.all([request("first"), request("second")]); const builds = inputs.map(async (input) => { - const result = await cache.build(input); + const result = await cache.build(withKunaProjectDeadline(input)); if (!result.ok) throw result.error; if (pin) releases.push(cache.pin(result.value.key)); return result.value; @@ -156,13 +157,15 @@ it.each(["decompiling", "sealing"] as const)( async (phase) => { const { kuna, cache, request, cacheRoot } = await fixture(); const input = await request("program"); - const result = await cache.build({ - ...input, - onStatus: (status) => { - if (status.phase === phase) - appendFileSync(kuna, "\n// installation replaced\n"); - }, - }); + const result = await cache.build( + withKunaProjectDeadline({ + ...input, + onStatus: (status) => { + if (status.phase === phase) + appendFileSync(kuna, "\n// installation replaced\n"); + }, + }), + ); expect(result).toMatchObject({ ok: false, error: { @@ -184,7 +187,7 @@ it("protects a cache request that arrives after an eviction snapshot starts", as "node:fs/promises", ); const firstInput = await request("first"); - const first = await cache.build(firstInput); + const first = await cache.build(withKunaProjectDeadline(firstInput)); if (!first.ok) throw first.error; const snapshot = barrier(); const touch = barrier(); @@ -199,7 +202,9 @@ it("protects a cache request that arrives after an eviction snapshot starts", as releases.push(cache.pin(result.value.key)); return result.value; }; - const second = pinResult(cache.build(await request("second"))); + const second = pinResult( + cache.build(withKunaProjectDeadline(await request("second"))), + ); let warm: ReturnType | undefined; onTestFinished(async () => { snapshot.release(); @@ -210,7 +215,7 @@ it("protects a cache request that arrives after an eviction snapshot starts", as if (!closed.ok) throw closed.error; }); await expect.poll(() => listing.arrived).toBe(1); - warm = pinResult(cache.build(firstInput)); + warm = pinResult(cache.build(withKunaProjectDeadline(firstInput))); await expect.poll(() => listing.touched).toBe(true); listing.names = await actual.readdir(listing.directory); snapshot.release(); @@ -234,8 +239,10 @@ it("keeps a shared progress callback registered for the remaining waiter", async statuses.push(status); if (status.phase === "decompiling") controller.abort(); }; - const first = cache.build({ ...input, signal: controller.signal, onStatus }); - const second = cache.build({ ...input, onStatus }); + const first = cache.build( + withKunaProjectDeadline({ ...input, signal: controller.signal, onStatus }), + ); + const second = cache.build(withKunaProjectDeadline({ ...input, onStatus })); expect(await first).toMatchObject({ ok: false, error: { _tag: "AnalysisCancelledError" }, @@ -255,12 +262,14 @@ it("lets a joining request cancel from the status replayed to it", async () => { const { cache, request } = await fixture(); const input = await request("program"); const decompiling = barrier(); - const first = cache.build({ - ...input, - onStatus: (status) => { - if (status.phase === "decompiling") decompiling.release(); - }, - }); + const first = cache.build( + withKunaProjectDeadline({ + ...input, + onStatus: (status) => { + if (status.phase === "decompiling") decompiling.release(); + }, + }), + ); let firstSettled = false; void first.then(() => { firstSettled = true; @@ -269,14 +278,16 @@ it("lets a joining request cancel from the status replayed to it", async () => { const leaving = new AbortController(); const replayed: KunaExportStatus[] = []; - const second = await cache.build({ - ...input, - signal: leaving.signal, - onStatus: (status) => { - replayed.push(status); - leaving.abort(); - }, - }); + const second = await cache.build( + withKunaProjectDeadline({ + ...input, + signal: leaving.signal, + onStatus: (status) => { + replayed.push(status); + leaving.abort(); + }, + }), + ); expect(replayed.map(({ phase }) => phase)).toEqual(["decompiling"]); expect(second).toMatchObject({ ok: false, diff --git a/tests/boundary/providers/kuna/kunaProjectRequest.ts b/tests/boundary/providers/kuna/kunaProjectRequest.ts new file mode 100644 index 0000000000..2abcf92069 --- /dev/null +++ b/tests/boundary/providers/kuna/kunaProjectRequest.ts @@ -0,0 +1,13 @@ +import { kunaProjectDeadline } from "../../../../src/kuna/KunaConfiguration.js"; +import type { KunaProjectRequest } from "../../../../src/kuna/KunaProjectCache.js"; + +/** + * A project request carrying the deadline its installation's configuration + * selects now, as the session accepts one before it starts an export. + */ +export const withKunaProjectDeadline = ( + request: Omit, +): KunaProjectRequest => ({ + ...request, + deadline: kunaProjectDeadline(request.installation.configuration), +}); diff --git a/tests/boundary/providers/kuna/kunaPublicationCleanup.test.ts b/tests/boundary/providers/kuna/kunaPublicationCleanup.test.ts index ee8e64709f..522cb58c8a 100644 --- a/tests/boundary/providers/kuna/kunaPublicationCleanup.test.ts +++ b/tests/boundary/providers/kuna/kunaPublicationCleanup.test.ts @@ -15,6 +15,7 @@ import { writeFakeKuna, type FakeKunaBehavior, } from "./fakeKuna.js"; +import { withKunaProjectDeadline } from "./kunaProjectRequest.js"; const fault = vi.hoisted(() => ({ root: "", blocked: false })); vi.mock("node:fs/promises", async (importOriginal) => { @@ -78,7 +79,7 @@ const fixture = async (behavior: FakeKunaBehavior = {}) => { it("retains the completed publication receipt while retrying only stopped staging cleanup", async () => { const { cache, request, kuna } = await fixture(); try { - const built = await cache.build(request); + const built = await cache.build(withKunaProjectDeadline(request)); if (built.ok) throw new Error("Expected retained staging cleanup failure"); expect(built.error).toBeInstanceOf(KunaPublishedProjectCleanupError); if (!(built.error instanceof KunaPublishedProjectCleanupError)) @@ -164,7 +165,7 @@ it("reuses the published export in the same session without hiding pending clean it("preserves the original failed export when staging cleanup also fails", async () => { const { cache, request } = await fixture({ project: "exit-failure" }); try { - const built = await cache.build(request); + const built = await cache.build(withKunaProjectDeadline(request)); if (built.ok) throw new Error("Expected export and cleanup failure"); expect(built.error).not.toBeInstanceOf(KunaPublishedProjectCleanupError); expect(built.error.cleanupIncomplete).toBe(true); diff --git a/tests/boundary/providers/kuna/kunaSelectedCommand.test.ts b/tests/boundary/providers/kuna/kunaSelectedCommand.test.ts index 64255dc677..2b37a93d3a 100644 --- a/tests/boundary/providers/kuna/kunaSelectedCommand.test.ts +++ b/tests/boundary/providers/kuna/kunaSelectedCommand.test.ts @@ -21,6 +21,7 @@ import { inspectKunaInstallationOwned as inspectKunaInstallation, withKunaResourceScope, } from "./kunaResourceScope.js"; +import { withKunaProjectDeadline } from "./kunaProjectRequest.js"; const it = test.skipIf(process.platform === "win32"); const shellArgument = (value: string) => `'${value.replaceAll("'", "'\\''")}'`; @@ -203,11 +204,13 @@ it("keeps shared-cache exports separate for equal wrapper bytes selected beside try { const keys: string[] = []; for (const [index, installation] of installations.entries()) { - const built = await cache.build({ - installation, - targetPath, - targetSha256, - }); + const built = await cache.build( + withKunaProjectDeadline({ + installation, + targetPath, + targetSha256, + }), + ); if (!built.ok) throw built.error; keys.push(built.value.key); expect(built.value.functions[0]?.name).toBe( diff --git a/tests/fixtures/fakeHopper.mjs b/tests/fixtures/fakeHopper.mjs index c21de8cfc9..a6230e67fe 100644 --- a/tests/fixtures/fakeHopper.mjs +++ b/tests/fixtures/fakeHopper.mjs @@ -238,6 +238,21 @@ const server = createServer((socket) => { Buffer.from('"}\n'), ]), ); + } else if (request.method === "unattributed_invalid_request") { + // The real bridge answers on reserved id 0 when it cannot read a + // request's id, for example a frame that is not JSON. + send({ + id: 0, + error: { + code: -32000, + type: "invalid_request", + message: "Invalid bridge request JSON", + }, + }); + } else if (request.method === "unterminated_reply") { + socket.write( + `{"id":${request.id},"result":"${"x".repeat(request.params.bytes)}`, + ); } else if (request.method === "wrong_id") { send({ id: request.id + 100, result: {} }); } else if (bridgeEventMessages !== undefined) {