From f11f5b1a1a9f084cdf4b3bdad312df9da3e0b34c Mon Sep 17 00:00:00 2001 From: LogicDuke Date: Wed, 12 Aug 2026 13:43:14 +0200 Subject: [PATCH 01/20] feat: add Commander-Claude process transport boundary --- .../010-commander-claude-bridge.md | 260 +++++ src/adapters/agent-transport.ts | 989 ++++++++++++++++ src/adapters/process-transport.ts | 806 +++++++++++++ tests/adapters/process-transport.test.ts | 724 ++++++++++++ tests/adapters/transport-fixtures.ts | 428 +++++++ tests/adapters/transport-invariants.test.ts | 1004 +++++++++++++++++ 6 files changed, 4211 insertions(+) create mode 100644 docs/architecture/010-commander-claude-bridge.md create mode 100644 src/adapters/agent-transport.ts create mode 100644 src/adapters/process-transport.ts create mode 100644 tests/adapters/process-transport.test.ts create mode 100644 tests/adapters/transport-fixtures.ts create mode 100644 tests/adapters/transport-invariants.test.ts diff --git a/docs/architecture/010-commander-claude-bridge.md b/docs/architecture/010-commander-claude-bridge.md new file mode 100644 index 0000000..283e914 --- /dev/null +++ b/docs/architecture/010-commander-claude-bridge.md @@ -0,0 +1,260 @@ +# Commander ↔ Claude Bridge: Local Process Transport (PR 010) + +Status: V1. Superseded only by an explicit architecture decision. + +## Purpose + +PR 010 implements the process-communication seam that the frozen V1 pipeline's +provider adapters will sit on: + + validated process specification -> one child process -> one AgentExchange + +It answers exactly one question: + +> What did the operating system do when asked to run this exact program with +> this exact argument vector, and what bytes did it write? + +**PR 010 does not eliminate the manual copy/paste loop between the operator-facing +AgentBridge layer and an external agent.** It supplies only the transport +required for that later end-to-end capability. Decoding a transcript, building an +`AgentReport`, and normalizing it through PR 006 remain separate responsibilities +for later bounded PRs. + +## The layer is dormant, and dormant is not unforgeable + +`invokeAgentProcess` is **not exported from `src/index.ts`**, is not re-exported +by any barrel, and has no production caller. That is a statement about wiring, +not a security property: a source module can still be imported by an internal +module or by deep path, and nothing about its absence from the package root +makes it unreachable. + +The accurate state of PR 010: + +- the low-level transport exists and is tested; +- it is not exported from the package root; +- it is not wired into any production orchestration path; +- no production caller invokes it; +- it performs **no policy authorization**; +- a later adapter must enforce an unforgeable, single-use authorization + capability before invoking it. + +### Why the capability is not in this PR + +`GateDecision` is a structural TypeScript interface over a frozen plain object. +It carries no brand, no `unique symbol`, no class identity, and no registry +membership — `src/` contains no `Symbol`, `WeakMap`, `WeakSet`, or brand field +anywhere. A caller can therefore construct an object literal that satisfies every +field, including `mayExecuteAutonomously: true`, and it is indistinguishable at +runtime from one `evaluateActionRequest` produced. **Accepting a `GateDecision` +parameter would be security theatre**, so this transport accepts none. + +Closing that gap needs a new unforgeable capability — a module-private registry +that only an `authorizeAgentCommunication` function can add to, minted from a +single `evaluateActionRequest` call, bound to one specification and consumed +once. That belongs to the later adapter PR, not here. `evaluateActionRequest` +remains the single authority computation, and this layer neither calls it nor +restates its vocabulary. + +## Trust boundary + +| Party | Owns | +| --- | --- | +| **This transport** | validating the specification's shape; spawning one process without a shell; writing stdin and closing it; capturing two bounded byte streams; enforcing a deadline and cancellation; terminating; reporting | +| **The external agent** | everything it does inside the working directory it was assigned, under its own credentials — including editing, committing, or pushing within a Git worktree given to it | +| **Nobody, ever, here** | policy, authority, provider identity, prompt content, transcript interpretation, persistence, logging | + +AgentBridge remains read-only against managed repositories because *AgentBridge's +own process* writes nothing: this layer imports no filesystem API, runs no Git +command, and creates no file. Spawning an external agent in its assigned worktree +does **not** make AgentBridge the repository writer — the agent acts under its own +authority, exactly as `006-agent-invocation-boundary.md` describes. A working +directory is therefore **not** rejected for being a managed-repository worktree, +and this PR adds no managed-root discovery and no repository policy. + +## No shell, on any path + +`spawn` is called with `shell: false` at both call sites, and the module contains +no `exec`, `execSync`, `cmd.exe /c`, `powershell -Command`, or composed command +line — including on the Windows termination path. A test counts the `spawn(` +call sites in the comment-stripped source and requires an equal number of +`shell: false` options, so a third spawn cannot be added without one. + +The executable must be an **absolute path to a directly spawnable binary**. PATH +is never searched. `.cmd`, `.bat`, and `.ps1` are rejected on every platform, +because running one requires a shell or an explicit interpreter, and reaching for +`shell: true` would reintroduce precisely the argument-injection class this +design exists to avoid. + +## Arguments are validated structurally, never by policy + +There is **no permitted-flag allowlist and no deny-list**. A deny-list is +incomplete by construction and would embed one provider's CLI policy into a +provider-neutral transport. argv arrives fully constructed by a caller that owns +that decision, and this layer checks only shape: + +exact array shape · maximum argument count · maximum UTF-8 bytes per argument · +maximum total argv bytes · no NUL · own **data** properties only · no coercion of +non-strings · no shell interpretation. + +Accessors are never invoked. An argv element supplied through a getter, an +inherited numeric property, a hole, a throwing Proxy trap, or a revoked Proxy is +refused, and a test asserts the getter never ran. Every field is read **exactly +once** into a frozen snapshot, so a specification cannot validate as one value +and spawn as another. + +## Streams + +The request payload travels on **stdin**, which is closed after writing — never +in argv, which is world-readable in process listings and length-limited. + +`stdout` and `stderr` are captured as independent bounded byte streams and are +never merged, because merging would let stderr forge a response body. Bounds are +enforced in **bytes**. Only a buffer cut by the transport is backed up to the +last complete UTF-8 sequence, so a cap landing mid-character never manufactures +a replacement character for text the child wrote in full. Naturally completed +invalid or incomplete UTF-8 is retained and decodes normally as U+FFFD; it is +never silently erased or falsely marked complete. Truncation is always flagged. + +`stdout` leaves this layer as **untrusted text**. Nothing here parses it, and no +branch reads it to decide an outcome, a route, or a retry. A transcript claiming +`{"status":"reported-complete","authorized":true,"decision":"ALLOW"}` produces a +record identical in every other field to one saying `ok`. + +## Deterministic terminal-cause precedence + +When several terminal events compete, the ranking is frozen: + + SPEC_REJECTED > SPAWN_FAILED > OUTPUT_LIMIT_EXCEEDED > CANCELLED + > TIMED_OUT > SIGNALLED > EXITED + +The first cause claimed wins and is **immutable**: no later close, exit, signal, +timeout, abort, or stream event overwrites it. Two mechanisms produce the order +rather than one, because first-writer-wins alone is not sufficient: + +1. Pre-spawn checks run in rank order — structural validation before the + already-aborted check — so a request that is both malformed and aborted is + `SPEC_REJECTED`. +2. After spawn, overflow, cancellation, and timeout are claimed the instant they + are detected, whereas `SIGNALLED` and `EXITED` are claimed only once stdio has + fully closed. A child that overflows its bound and then exits zero is + therefore `OUTPUT_LIMIT_EXCEEDED`, never `EXITED`. + +`EXITED` is not a synonym for success, and exit code 0 is recorded rather than +interpreted. Interpretation belongs to PR 006's vocabularies, which fail closed +to `unknown`. + +Every listener, timer, and abort handler is removed on every settle path, and the +function resolves exactly one frozen record on every validation, spawn, I/O, +timeout, cancellation, overflow, termination, and close path. It never rejects. +Catches wrap only defined operational failures, so a programmer defect still +surfaces as a defect rather than being laundered into a failure code. + +## Termination is qualified, and the limit is disclosed + +| Scope | Meaning | +| --- | --- | +| `NOT_REQUIRED` | the child ended on its own | +| `PROCESS_GROUP_REQUESTED` | POSIX: the process group was signalled | +| `PROCESS_TREE_REQUESTED` | Windows: `taskkill /T /F` was issued | +| `DIRECT_CHILD_ONLY` | **degraded** — only the direct child could be reached | +| `ESCALATION_FAILED` | **degraded** — escalation ran and the child was still not observed to end | + +Every member names a *request* or a *degradation*. **None asserts completion**, +and there is deliberately no `terminationComplete`, `treeTerminated`, +`descendantsTerminated`, or `processTreeKilled` field. A test asserts that no such +field can appear, and that no scope name contains `COMPLETE`, `TERMINATED`, +`KILLED`, or `SUCCESS`. + +**POSIX.** The child is spawned `detached`, making it a process-group leader. +Termination signals `SIGTERM` to the group, waits only the bounded grace period, +then escalates `SIGKILL` to the group, and reaps the direct child. `ESRCH` is +treated as "already gone". If the group cannot be signalled, the direct child is +signalled instead and the scope degrades to `DIRECT_CHILD_ONLY`. + +**Windows.** `taskkill.exe` is spawned **directly** — `shell: false`, a validated +absolute path, and the fixed argument vector `/PID /T /F`, whose +only variable this module produced itself. No caller-controlled argument reaches +it. The system directory is resolved from `SystemRoot` (or `windir`) and +validated as absolute, NUL-free, and bounded before use; `C:\Windows` is never +assumed, and the resolved value is never added to the child environment, the +transcript, an error, or the exchange. If the helper reaches its first timeout, +it is killed and observed through a second bounded exit wait before the attempt +returns. The direct child is then waited on. If `taskkill` cannot start, fails, +or reaches either timeout, +the direct child is terminated and the scope degrades to `DIRECT_CHILD_ONLY` — +descendants are **not** claimed. + +### The escape, stated plainly + +A descendant that **deliberately detaches itself** — `setsid` on POSIX, +re-parenting or `CREATE_BREAKAWAY_FROM_JOB` on Windows — is in neither the POSIX +process group nor the Windows process tree, and survives. **Absolute +process-tree termination is not claimed and is not achievable** under the frozen +constraints: it would require a Windows Job Object (a native addon) or Linux +cgroups / PID namespaces (single-platform). The invariant this layer does uphold: + +> Ordinary descendants are targeted through the available process-group or +> process-tree mechanism, and every exchange records whether that request was +> issued or termination degraded to the direct child. Completion for every +> descendant is never claimed. + +Both halves are tested. Termination of an *ordinary* descendant is verified +cross-platform by a heartbeat file that must stop growing. The escape itself is +demonstrated by a POSIX-only test in which a deliberately detached grandchild +keeps writing — the limitation is pinned by a passing assertion, not by prose. + +## Environment + +The child environment comes only from the structurally validated record the caller +supplied. This transport never merges it with `process.env` and never reads +`process.env` to populate it; the only two `process.env` reads in the module are +`SystemRoot` and `windir`, used solely to locate `taskkill.exe`, and a test pins +that count at two. + +On Windows, `uv_spawn` would copy eleven sensitive names from the parent when +they are absent: `HOMEDRIVE`, `HOMEPATH`, `LOGONSERVER`, `PATH`, `SYSTEMDRIVE`, +`SYSTEMROOT`, `TEMP`, `USERDOMAIN`, `USERNAME`, `USERPROFILE`, and `WINDIR`. +The transport prevents that fallback by requiring every name as an own validated +data property before spawn. Matching is case-insensitive, empty values are +permitted, and missing or case-insensitively duplicated names fail with distinct +rejection reasons. AgentBridge never obtains or fills their values from +`process.env`. Windows may still synthesize per-drive pseudo-variables such as +`=C:`; these are operating-system entries rather than inherited parent values +and are excluded from the exact-record comparison in the Windows test. + +No credential appears in a returned record, an error, a fixture, or a serialized +exchange, and this layer contains no logging of any kind. It introduces no +credential storage and no secret resolution. + +## Bounds + +| Bound | Value | Rationale | +| --- | --- | --- | +| `MAX_ARGV_COUNT` | 64 | a real invocation uses a handful | +| `MAX_ARG_BYTES` | 4 096 | per argument, UTF-8 | +| `MAX_ARGV_TOTAL_BYTES` | 30 000 | Windows caps a composed command line at 32 767 characters; this binds first on every platform | +| `MAX_PATH_BYTES` | 4 096 | executable and working directory | +| `MAX_STDIN_BYTES` | 1 048 576 | the payload channel | +| `MAX_STDOUT_BYTES_CEILING` | 8 388 608 | the caller's cap is measured against this | +| `MAX_STDERR_BYTES_CEILING` | 1 048 576 | diagnostics only | +| `MAX_ENV_ENTRIES` | 64 | | +| `MAX_ENV_KEY_BYTES` | 256 | equals PR 005's and PR 006's `MAX_IDENTIFIER_LENGTH`; pinned by a test | +| `MAX_ENV_VALUE_BYTES` | 32 768 | | +| `MIN`/`MAX_TIMEOUT_MS` | 1 / 3 600 000 | required; no default to forget | +| `MIN`/`MAX_GRACE_MS` | 0 / 60 000 | | + +## Non-goals + +No policy, authority, gate, capability, `SpawnGrant`, or `GateDecision` handling. +No report decoding, JSON parsing, `AgentReport` construction, or call to +`ingestInvocationReport`. No completion, finding, freshness, or merge judgment. +No Review Ingestion or Evidence Store persistence. No Autoflow integration. No +Commander type or service. No Claude-specific code, provider routing, prompt +template, or second provider adapter. No flag allowlist or deny-list. No Git or +filesystem mutation by AgentBridge. No logging, retries, queues, scheduling, +metrics, or telemetry. No HTTP, SDK, MCP, WebSocket, or remote execution. No +identifier generation, clock read, or timestamp. No managed-root discovery or +repository policy configuration. No new dependency, and no change to +`src/domain/**`, `src/index.ts`, `README.md`, or the package manifests. + +This is one layer of the frozen V1 pipeline, not the pipeline. diff --git a/src/adapters/agent-transport.ts b/src/adapters/agent-transport.ts new file mode 100644 index 0000000..fd8986a --- /dev/null +++ b/src/adapters/agent-transport.ts @@ -0,0 +1,989 @@ +/** + * Provider-neutral local process transport contract. + * + * This module describes *how to ask the operating system to run one process and + * hand back what it wrote*. It contains no policy, no authority, no provider + * vocabulary, and no I/O: every export here is a type, a frozen vocabulary, a + * bound, or a pure reader. `node:child_process` lives in `process-transport.ts` + * and nowhere else. + * + * What this contract deliberately does **not** contain, and must never gain: + * + * - A `GateDecision`, `ActionRequest`, capability, grant, or any other + * authorization input. PR 003's `evaluateActionRequest` remains the single + * authority computation, and this seam performs none of it. A later adapter + * must enforce an unforgeable, single-use authorization capability *before* + * invoking the transport. + * - Provider identity, provider routing, prompt text, flag allowlists, or flag + * deny-lists. A deny-list would be both incomplete and provider-specific; + * argv arrives already constructed by a caller that owns that policy. + * - Any interpretation of what the child wrote. `stdout` and `stderr` leave here + * as untrusted text. Decoding them into an `AgentReport`, parsing JSON, + * judging completion, or calling `ingestInvocationReport` belong to a later + * bounded PR. + * + * Two inputs meet here and are kept strictly apart: + * + * - **Trusted for shape** — the {@link AgentProcessSpec} and + * {@link TransportLimits} supplied by the caller. They are still validated + * structurally, because a "trusted" object can still be a Proxy, carry + * accessors, or hold values of the wrong runtime type. + * - **Untrusted entirely** — everything the child process writes. It is + * captured, bounded, and echoed. It is never parsed and never reaches a + * decision. + */ + +/** + * Intrinsics captured at module load, before any untrusted property access is + * possible. + * + * Validation reads caller-supplied objects that may be Proxies or carry + * accessors, and such a trap can repoint prototype methods while it runs. + * Capturing first removes that lever. Same pattern as `evidence.ts`, + * `review.ts`, and `agent-invocation.ts`. + */ +const objectFreeze = Object.freeze; +const objectDefineProperty = Object.defineProperty; +const objectCreate = Object.create; +const objectGetOwnPropertyDescriptor = Object.getOwnPropertyDescriptor; +const objectGetOwnPropertyNames = Object.getOwnPropertyNames; +const objectGetOwnPropertySymbols = Object.getOwnPropertySymbols; +const arrayIsArray = Array.isArray; +const numberIsInteger = Number.isInteger; +const reflectApply = Reflect.apply; +// Captured unbound on purpose and invoked through `Reflect.apply`, so neither a +// poisoned prototype method nor a poisoned `Function.prototype.call` is on the +// path. `this` is supplied explicitly at every call site. `Buffer.byteLength` +// is a static that ignores `this`; it is captured for the same reason. +/* eslint-disable @typescript-eslint/unbound-method */ +const bufferByteLength = Buffer.byteLength; +// Node's Buffer prototype is typed through `any`; the runtime method is captured +// with the precise call signature used below. +// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment +const bufferSubarray: (this: Buffer, start: number, end?: number) => Buffer = + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + Buffer.prototype.subarray; +const stringIndexOf = String.prototype.indexOf; +const stringSlice = String.prototype.slice; +const stringToLowerCase = String.prototype.toLowerCase; +const stringCharCodeAt = String.prototype.charCodeAt; +const numberToString = Number.prototype.toString; +const abortSignalAborted: ((this: AbortSignal) => boolean) | undefined = + Object.getOwnPropertyDescriptor(AbortSignal.prototype, 'aborted')?.get; +/* eslint-enable @typescript-eslint/unbound-method */ + +/** Append by defining an own element, bypassing inherited index setters. */ +function append(list: T[], value: T): void { + objectDefineProperty(list, list.length, { + value, + writable: true, + enumerable: true, + configurable: true, + }); +} + +/** + * Which absolute-path grammar applies. + * + * Passed in rather than read from `process.platform`, so this module stays pure + * and both grammars are testable on either host. + */ +export type TransportPlatform = 'win32' | 'posix'; + +/** + * Why an exchange ended. + * + * This records the *initiating cause*, independently of what termination then + * achieved. A child that was killed because its output overflowed is + * `OUTPUT_LIMIT_EXCEEDED`, not `SIGNALLED`: the signal was ours, and reporting + * it as an external signal would erase the reason. + * + * `EXITED` is not a synonym for success. It means the process ran to completion + * and `exitCode` is set; a zero exit code is recorded, never interpreted. + */ +export const TRANSPORT_OUTCOME = objectFreeze({ + /** Ran to completion. `exitCode` is set. Says nothing about correctness. */ + EXITED: 'EXITED', + /** Died by a signal this transport did not send. */ + SIGNALLED: 'SIGNALLED', + /** The deadline elapsed. This transport terminated it. */ + TIMED_OUT: 'TIMED_OUT', + /** The caller's `AbortSignal` fired. This transport terminated it. */ + CANCELLED: 'CANCELLED', + /** A stream bound was reached. This transport terminated it. */ + OUTPUT_LIMIT_EXCEEDED: 'OUTPUT_LIMIT_EXCEEDED', + /** The operating system refused to start the process. */ + SPAWN_FAILED: 'SPAWN_FAILED', + /** Structural validation refused the request. Nothing was spawned. */ + SPEC_REJECTED: 'SPEC_REJECTED', +} as const); + +export type TransportOutcome = + (typeof TRANSPORT_OUTCOME)[keyof typeof TRANSPORT_OUTCOME]; + +/** Every member of the {@link TransportOutcome} union. */ +export const TRANSPORT_OUTCOMES: readonly TransportOutcome[] = objectFreeze([ + TRANSPORT_OUTCOME.EXITED, + TRANSPORT_OUTCOME.SIGNALLED, + TRANSPORT_OUTCOME.TIMED_OUT, + TRANSPORT_OUTCOME.CANCELLED, + TRANSPORT_OUTCOME.OUTPUT_LIMIT_EXCEEDED, + TRANSPORT_OUTCOME.SPAWN_FAILED, + TRANSPORT_OUTCOME.SPEC_REJECTED, +]); + +/** + * Terminal-cause precedence, highest first. + * + * When several terminal events compete, the earlier member wins and the winning + * cause is **immutable**: no later close, exit, signal, timeout, abort, or + * stream event may overwrite it. + * + * Two mechanisms produce this ordering rather than one, because a single + * first-writer-wins rule is not sufficient on its own: + * + * 1. The pre-spawn checks run in this order — structural validation first, then + * an already-aborted signal — so a request that is both malformed and + * aborted is `SPEC_REJECTED`. + * 2. After spawn, `OUTPUT_LIMIT_EXCEEDED`, `CANCELLED`, and `TIMED_OUT` are + * claimed the instant they are detected, whereas `SIGNALLED` and `EXITED` + * are claimed only once the child's stdio has fully closed. A child that + * overflows its bound and then exits zero is therefore + * `OUTPUT_LIMIT_EXCEEDED`, never `EXITED` — which is the whole point of + * ranking overflow above exit. + */ +export const TERMINAL_CAUSE_PRECEDENCE: readonly TransportOutcome[] = objectFreeze([ + TRANSPORT_OUTCOME.SPEC_REJECTED, + TRANSPORT_OUTCOME.SPAWN_FAILED, + TRANSPORT_OUTCOME.OUTPUT_LIMIT_EXCEEDED, + TRANSPORT_OUTCOME.CANCELLED, + TRANSPORT_OUTCOME.TIMED_OUT, + TRANSPORT_OUTCOME.SIGNALLED, + TRANSPORT_OUTCOME.EXITED, +]); + +/** + * What termination was *asked* of the operating system. + * + * Every member is deliberately phrased as a request or a degradation. **None + * asserts completion**, because completion is not provable from either + * mechanism this transport can use: `kill(-pgid, ...)` reaches a POSIX process + * group, and `taskkill /T /F` walks the parent-child links Windows recorded, and + * a descendant that deliberately detached itself is in neither. + * + * There is deliberately no `terminationComplete`, `treeTerminated`, + * `descendantsTerminated`, or `allDescendantsTerminated` field anywhere in this + * contract, and a test asserts that none can appear. + * + * The direct child is the only process whose termination this transport + * observes. A degraded scope means descendants were *not* reached, and + * `PROCESS_GROUP_REQUESTED` / `PROCESS_TREE_REQUESTED` mean the request was + * issued — never that it succeeded for every descendant. + */ +export const TERMINATION_SCOPE = objectFreeze({ + /** The child ended on its own. This transport terminated nothing. */ + NOT_REQUIRED: 'NOT_REQUIRED', + /** POSIX: the process group was signalled. Detached descendants escape. */ + PROCESS_GROUP_REQUESTED: 'PROCESS_GROUP_REQUESTED', + /** Windows: `taskkill /T /F` was issued. Re-parented descendants escape. */ + PROCESS_TREE_REQUESTED: 'PROCESS_TREE_REQUESTED', + /** Degraded: only the direct child could be reached. */ + DIRECT_CHILD_ONLY: 'DIRECT_CHILD_ONLY', + /** Degraded: escalation ran and the direct child was still not observed to end. */ + ESCALATION_FAILED: 'ESCALATION_FAILED', +} as const); + +export type TerminationScope = + (typeof TERMINATION_SCOPE)[keyof typeof TERMINATION_SCOPE]; + +/** Every member of the {@link TerminationScope} union. */ +export const TERMINATION_SCOPES: readonly TerminationScope[] = objectFreeze([ + TERMINATION_SCOPE.NOT_REQUIRED, + TERMINATION_SCOPE.PROCESS_GROUP_REQUESTED, + TERMINATION_SCOPE.PROCESS_TREE_REQUESTED, + TERMINATION_SCOPE.DIRECT_CHILD_ONLY, + TERMINATION_SCOPE.ESCALATION_FAILED, +]); + +/** + * Scopes that mean descendants were not reached. + * + * Exported so a caller can branch on degradation without matching strings, and + * so the qualified guarantee is expressible in data rather than only in prose. + */ +export const DEGRADED_TERMINATION_SCOPES: readonly TerminationScope[] = objectFreeze([ + TERMINATION_SCOPE.DIRECT_CHILD_ONLY, + TERMINATION_SCOPE.ESCALATION_FAILED, +]); + +/** + * Why structural validation refused a request. + * + * Every member describes *shape*. None describes permission, provider policy, + * or intent: this transport has no opinion about which flags are acceptable, + * only about whether it was handed a well-formed argv at all. + */ +export const TRANSPORT_REJECTION = objectFreeze({ + SPEC_UNREADABLE: 'SPEC_UNREADABLE', + LIMITS_UNREADABLE: 'LIMITS_UNREADABLE', + + EXECUTABLE_INVALID: 'EXECUTABLE_INVALID', + EXECUTABLE_NOT_ABSOLUTE: 'EXECUTABLE_NOT_ABSOLUTE', + EXECUTABLE_SUFFIX_FORBIDDEN: 'EXECUTABLE_SUFFIX_FORBIDDEN', + + WORKING_DIRECTORY_INVALID: 'WORKING_DIRECTORY_INVALID', + WORKING_DIRECTORY_NOT_ABSOLUTE: 'WORKING_DIRECTORY_NOT_ABSOLUTE', + + ARGV_NOT_ARRAY: 'ARGV_NOT_ARRAY', + ARGV_UNREADABLE: 'ARGV_UNREADABLE', + ARGV_COUNT_EXCEEDED: 'ARGV_COUNT_EXCEEDED', + ARGUMENT_UNREADABLE: 'ARGUMENT_UNREADABLE', + ARGUMENT_NOT_STRING: 'ARGUMENT_NOT_STRING', + ARGUMENT_CONTAINS_NUL: 'ARGUMENT_CONTAINS_NUL', + ARGUMENT_BYTES_EXCEEDED: 'ARGUMENT_BYTES_EXCEEDED', + ARGV_TOTAL_BYTES_EXCEEDED: 'ARGV_TOTAL_BYTES_EXCEEDED', + + ENVIRONMENT_NOT_RECORD: 'ENVIRONMENT_NOT_RECORD', + ENVIRONMENT_UNREADABLE: 'ENVIRONMENT_UNREADABLE', + ENVIRONMENT_COUNT_EXCEEDED: 'ENVIRONMENT_COUNT_EXCEEDED', + ENVIRONMENT_ENTRY_INVALID: 'ENVIRONMENT_ENTRY_INVALID', + ENVIRONMENT_NAME_DUPLICATED: 'ENVIRONMENT_NAME_DUPLICATED', + ENVIRONMENT_REQUIRED_VARIABLE_MISSING: 'ENVIRONMENT_REQUIRED_VARIABLE_MISSING', + ENVIRONMENT_BYTES_EXCEEDED: 'ENVIRONMENT_BYTES_EXCEEDED', + + STDIN_NOT_STRING: 'STDIN_NOT_STRING', + STDIN_BYTES_EXCEEDED: 'STDIN_BYTES_EXCEEDED', + + TIMEOUT_OUT_OF_RANGE: 'TIMEOUT_OUT_OF_RANGE', + GRACE_OUT_OF_RANGE: 'GRACE_OUT_OF_RANGE', + STDOUT_LIMIT_OUT_OF_RANGE: 'STDOUT_LIMIT_OUT_OF_RANGE', + STDERR_LIMIT_OUT_OF_RANGE: 'STDERR_LIMIT_OUT_OF_RANGE', + ABORT_SIGNAL_INVALID: 'ABORT_SIGNAL_INVALID', +} as const); + +export type TransportRejection = + (typeof TRANSPORT_REJECTION)[keyof typeof TRANSPORT_REJECTION]; + +/** + * V1 bounds. + * + * Every unbounded dimension is capped **before** anything is spawned, following + * the rule established in PR 005 and PR 006. + * + * `MAX_ARGV_TOTAL_BYTES` is the load-bearing one. Windows composes argv into a + * single command line for `CreateProcess`, which caps at 32 767 characters, so + * the total bound binds before `MAX_ARGV_COUNT * MAX_ARG_BYTES` ever could and + * keeps every platform below the strictest operating-system limit. + * + * `MAX_ENV_KEY_BYTES` equals PR 005's and PR 006's `MAX_IDENTIFIER_LENGTH`; a + * test pins the three together. + */ +export const TRANSPORT_BOUNDS = objectFreeze({ + /** Arguments permitted in one argv vector. */ + MAX_ARGV_COUNT: 64, + /** UTF-8 bytes permitted in one argument. */ + MAX_ARG_BYTES: 4_096, + /** UTF-8 bytes permitted across the whole argv vector. */ + MAX_ARGV_TOTAL_BYTES: 30_000, + /** UTF-8 bytes permitted in `executablePath` and `workingDirectory`. */ + MAX_PATH_BYTES: 4_096, + /** UTF-8 bytes permitted in the stdin payload. */ + MAX_STDIN_BYTES: 1_048_576, + /** Ceiling the caller's `maxStdoutBytes` is measured against. */ + MAX_STDOUT_BYTES_CEILING: 8_388_608, + /** Ceiling the caller's `maxStderrBytes` is measured against. */ + MAX_STDERR_BYTES_CEILING: 1_048_576, + /** Entries permitted in the child environment. */ + MAX_ENV_ENTRIES: 64, + /** UTF-8 bytes permitted in one environment key. */ + MAX_ENV_KEY_BYTES: 256, + /** UTF-8 bytes permitted in one environment value. */ + MAX_ENV_VALUE_BYTES: 32_768, + MIN_TIMEOUT_MS: 1, + MAX_TIMEOUT_MS: 3_600_000, + MIN_GRACE_MS: 0, + MAX_GRACE_MS: 60_000, +} as const); + +/** + * Executable suffixes that cannot be spawned without a shell. + * + * `.cmd` and `.bat` are interpreted by `cmd.exe` and `.ps1` by PowerShell, so + * running one requires `shell: true` or an explicit interpreter — and + * `shell: true` reintroduces exactly the argument-injection class this + * transport exists to avoid. They are rejected on every platform, not only on + * Windows, so the rule cannot be sidestepped by where the code happens to run. + */ +const FORBIDDEN_EXECUTABLE_SUFFIXES: readonly string[] = objectFreeze([ + '.cmd', + '.bat', + '.ps1', +]); + +/** Variables libuv otherwise copies from the parent on Windows. */ +const WINDOWS_REQUIRED_ENVIRONMENT_NAMES: readonly string[] = objectFreeze([ + 'HOMEDRIVE', + 'HOMEPATH', + 'LOGONSERVER', + 'PATH', + 'SYSTEMDRIVE', + 'SYSTEMROOT', + 'TEMP', + 'USERDOMAIN', + 'USERNAME', + 'USERPROFILE', + 'WINDIR', +]); + +/** + * One process to run. Every field is required; nothing has a default. + * + * `workingDirectory` is whatever absolute path the caller assigns, and this + * transport does not care whether it is a managed-repository worktree. What an + * external agent does inside its own assigned worktree, under its own + * credentials, is that agent's authority — documented in + * `docs/architecture/006-agent-invocation-boundary.md`. This transport itself + * writes no file and runs no Git command. + * + * Deliberately absent, and never to be added: credentials, tokens, secrets, + * prompt templates, provider identity, repository identity, callbacks, streams, + * file handles, API clients, or any authorization object. + */ +export interface AgentProcessSpec { + /** Absolute path to a directly spawnable executable. Never PATH-searched. */ + readonly executablePath: string; + /** Fully constructed argv. Never composed, never interpolated. */ + readonly args: readonly string[]; + /** Absolute path the child runs in. */ + readonly workingDirectory: string; + /** + * The child's environment. This transport never merges it with its own + * `process.env`, and never reads `process.env` to populate it. + * + * On Windows the caller must explicitly provide every name libuv would + * otherwise copy from the parent environment. Missing names and + * case-insensitive duplicates are rejected before spawn; empty values are + * permitted. The transport never obtains or fills those values itself. + */ + readonly environment: Readonly>; + /** Payload written to the child's stdin, after which stdin is closed. */ + readonly stdin: string; +} + +/** Bounds and cancellation for one exchange. Only `signal` is optional. */ +export interface TransportLimits { + /** Deadline in milliseconds. Required; there is no default to forget. */ + readonly timeoutMs: number; + /** Milliseconds between the polite and the forceful termination step. */ + readonly graceMs: number; + readonly maxStdoutBytes: number; + readonly maxStderrBytes: number; + /** External cancellation. The one optional field. */ + readonly signal?: AbortSignal; +} + +/** + * The result of one exchange. Frozen, JSON-serializable, lossless on round trip. + * + * `stdout` and `stderr` are **untrusted text**. Nothing in this transport reads + * them, and nothing downstream may treat them as an `AgentReport` until a later + * bounded PR normalizes them through PR 006's `ingestInvocationReport`. + * + * There is deliberately no `success`, `status`, `ok`, `complete`, `report`, + * `claims`, `authorized`, `decision`, `freshness`, `duration`, or timestamp + * field, and no field asserting that termination finished. + */ +export interface AgentExchange { + /** The initiating cause, independent of what termination achieved. */ + readonly outcome: TransportOutcome; + /** Non-null only when `outcome` is `SPEC_REJECTED`. */ + readonly rejection: TransportRejection | null; + /** Exit status when the process ran to completion. */ + readonly exitCode: number | null; + /** Signal name when the process died by signal. */ + readonly terminatingSignal: string | null; + /** Untrusted child stdout, bounded and decoded at a complete UTF-8 boundary. */ + readonly stdout: string; + /** Untrusted child stderr. Never merged with stdout. */ + readonly stderr: string; + readonly stdoutTruncated: boolean; + readonly stderrTruncated: boolean; + /** Source bytes retained behind `stdout`, after bounding and boundary trim. */ + readonly stdoutBytes: number; + /** Source bytes retained behind `stderr`, after bounding and boundary trim. */ + readonly stderrBytes: number; + /** What termination was asked of the OS. Never a claim that it finished. */ + readonly terminationScope: TerminationScope; +} + +/** A specification whose every field has been read exactly once and validated. */ +export interface ValidatedInvocation { + readonly executablePath: string; + readonly args: readonly string[]; + readonly workingDirectory: string; + readonly environment: Readonly>; + readonly stdin: string; + readonly timeoutMs: number; + readonly graceMs: number; + readonly maxStdoutBytes: number; + readonly maxStderrBytes: number; + readonly signal: AbortSignal | null; +} + +/** Either a refusal or a fully snapshotted invocation. Never both. */ +export type InvocationReadResult = + | { readonly rejection: TransportRejection; readonly value: null } + | { readonly rejection: null; readonly value: ValidatedInvocation }; + +/** + * Read one **own data** property of an untrusted object. + * + * Accessors are not invoked: a getter is a caller-controlled function, and + * running one during validation would let a specification validate as one value + * and spawn as another. An accessor, an inherited value, or a throwing trap all + * read as `undefined`, which then fails the field's own type check. + */ +function readOwnData(target: object, key: string): unknown { + try { + const descriptor = objectGetOwnPropertyDescriptor(target, key); + if (descriptor === undefined) { + return undefined; + } + if (!('value' in descriptor)) { + return undefined; + } + return descriptor.value; + } catch { + return undefined; + } +} + +/** True when the value is a non-array object that can be probed at all. */ +function isReadableObject(value: unknown): value is object { + if (typeof value !== 'object' || value === null) { + return false; + } + try { + return !arrayIsArray(value); + } catch { + return false; + } +} + +/** UTF-8 byte length, computed without invoking any caller-supplied method. */ +export function utf8ByteLength(value: string): number { + const length: unknown = reflectApply(bufferByteLength, Buffer, [value, 'utf8']); + return typeof length === 'number' && numberIsInteger(length) ? length : 0; +} + +/** True when the string contains a NUL, which no OS accepts in argv or a path. */ +export function containsNul(value: string): boolean { + const index: unknown = reflectApply(stringIndexOf, value, ['\u0000']); + return typeof index !== 'number' || index !== -1; +} + +/** + * True when the path is absolute under the given grammar. + * + * Implemented by character inspection rather than `node:path`, so this module + * stays free of Node imports and both grammars are checkable on either host. A + * bare command name and every relative path fail here, which is what keeps PATH + * out of the picture entirely. + */ +export function isAbsolutePath(value: string, platform: TransportPlatform): boolean { + if (value.length === 0) { + return false; + } + if (platform === 'posix') { + return reflectApply(stringCharCodeAt, value, [0]) === 0x2f; + } + const first = reflectApply(stringCharCodeAt, value, [0]); + const isUnc = + (first === 0x5c || first === 0x2f) && + (reflectApply(stringCharCodeAt, value, [1]) === 0x5c || + reflectApply(stringCharCodeAt, value, [1]) === 0x2f); + if (isUnc) { + return true; + } + const isLetter = + (first >= 0x41 && first <= 0x5a) || (first >= 0x61 && first <= 0x7a); + const separator = reflectApply(stringCharCodeAt, value, [2]); + return ( + isLetter && + reflectApply(stringCharCodeAt, value, [1]) === 0x3a && + (separator === 0x5c || separator === 0x2f) + ); +} + +/** True when the path ends in a suffix that cannot be spawned without a shell. */ +function hasForbiddenSuffix(value: string): boolean { + if (value.length < 4) { + return false; + } + const tail: unknown = reflectApply(stringSlice, value, [value.length - 4]); + if (typeof tail !== 'string') { + return true; + } + const lowered: unknown = reflectApply(stringToLowerCase, tail, []); + if (typeof lowered !== 'string') { + return true; + } + for (let index = 0; index < FORBIDDEN_EXECUTABLE_SUFFIXES.length; index += 1) { + if (FORBIDDEN_EXECUTABLE_SUFFIXES[index] === lowered) { + return true; + } + } + return false; +} + +/** A refusal, shaped for {@link InvocationReadResult}. */ +function refuse(rejection: TransportRejection): InvocationReadResult { + return { rejection, value: null }; +} + +/** Narrow an untrusted value to an in-range integer, or `null`. */ +function readBoundedInteger(value: unknown, min: number, max: number): number | null { + if (typeof value !== 'number') { + return null; + } + if (!numberIsInteger(value)) { + return null; + } + return value >= min && value <= max ? value : null; +} + +/** Validate an untrusted path field once, in a fixed order of failure reasons. */ +function checkPath( + value: unknown, + platform: TransportPlatform, + invalid: TransportRejection, + notAbsolute: TransportRejection, +): TransportRejection | null { + if (typeof value !== 'string' || value.length === 0) { + return invalid; + } + if (containsNul(value)) { + return invalid; + } + if (utf8ByteLength(value) > TRANSPORT_BOUNDS.MAX_PATH_BYTES) { + return invalid; + } + if (!isAbsolutePath(value, platform)) { + return notAbsolute; + } + return null; +} + +/** + * Snapshot and validate argv. + * + * Elements are read through own **data** descriptors, so a hostile array cannot + * supply a value via a getter, via an inherited numeric property, or via a hole. + * The vector is rebuilt into a fresh array with indexed appends, so neither a + * poisoned iterator nor an inherited index setter is on the path between + * validation and spawn. + */ +function readArgs(raw: unknown): { + readonly rejection: TransportRejection | null; + readonly value: readonly string[]; +} { + let isArray = false; + try { + isArray = arrayIsArray(raw); + } catch { + return { rejection: TRANSPORT_REJECTION.ARGV_UNREADABLE, value: [] }; + } + if (!isArray) { + return { rejection: TRANSPORT_REJECTION.ARGV_NOT_ARRAY, value: [] }; + } + + let rawLength: unknown; + try { + rawLength = (raw as { readonly length: unknown }).length; + } catch { + return { rejection: TRANSPORT_REJECTION.ARGV_UNREADABLE, value: [] }; + } + if (typeof rawLength !== 'number' || !numberIsInteger(rawLength) || rawLength < 0) { + return { rejection: TRANSPORT_REJECTION.ARGV_UNREADABLE, value: [] }; + } + if (rawLength > TRANSPORT_BOUNDS.MAX_ARGV_COUNT) { + return { rejection: TRANSPORT_REJECTION.ARGV_COUNT_EXCEEDED, value: [] }; + } + + const args: string[] = []; + let totalBytes = 0; + for (let index = 0; index < rawLength; index += 1) { + let descriptor; + try { + const indexName = reflectApply(numberToString, index, []); + descriptor = objectGetOwnPropertyDescriptor(raw as object, indexName); + } catch { + return { rejection: TRANSPORT_REJECTION.ARGUMENT_UNREADABLE, value: [] }; + } + if (descriptor === undefined || !('value' in descriptor)) { + return { rejection: TRANSPORT_REJECTION.ARGUMENT_UNREADABLE, value: [] }; + } + const element: unknown = descriptor.value; + if (typeof element !== 'string') { + return { rejection: TRANSPORT_REJECTION.ARGUMENT_NOT_STRING, value: [] }; + } + if (containsNul(element)) { + return { rejection: TRANSPORT_REJECTION.ARGUMENT_CONTAINS_NUL, value: [] }; + } + const bytes = utf8ByteLength(element); + if (bytes > TRANSPORT_BOUNDS.MAX_ARG_BYTES) { + return { rejection: TRANSPORT_REJECTION.ARGUMENT_BYTES_EXCEEDED, value: [] }; + } + totalBytes += bytes; + if (totalBytes > TRANSPORT_BOUNDS.MAX_ARGV_TOTAL_BYTES) { + return { rejection: TRANSPORT_REJECTION.ARGV_TOTAL_BYTES_EXCEEDED, value: [] }; + } + append(args, element); + } + + return { rejection: null, value: objectFreeze(args) }; +} + +/** + * Snapshot and validate the child environment. + * + * The result is a fresh null-prototype object built with `defineProperty`, so + * nothing inherited and no accessor survives into what is handed to `spawn`. + * Own symbol keys are a refusal rather than a silent omission: a caller that + * attached one meant something by it, and quietly dropping it would hide the + * mismatch between what was asked for and what the child receives. + */ +function readEnvironment(raw: unknown, platform: TransportPlatform): { + readonly rejection: TransportRejection | null; + readonly value: Readonly>; +} { + const empty: Readonly> = objectFreeze( + objectCreate(null) as Record, + ); + if (!isReadableObject(raw)) { + return { rejection: TRANSPORT_REJECTION.ENVIRONMENT_NOT_RECORD, value: empty }; + } + + let symbols: readonly symbol[]; + let names: readonly string[]; + try { + symbols = objectGetOwnPropertySymbols(raw); + names = objectGetOwnPropertyNames(raw); + } catch { + return { rejection: TRANSPORT_REJECTION.ENVIRONMENT_UNREADABLE, value: empty }; + } + if (symbols.length > 0) { + return { rejection: TRANSPORT_REJECTION.ENVIRONMENT_ENTRY_INVALID, value: empty }; + } + if (names.length > TRANSPORT_BOUNDS.MAX_ENV_ENTRIES) { + return { rejection: TRANSPORT_REJECTION.ENVIRONMENT_COUNT_EXCEEDED, value: empty }; + } + + const environment = objectCreate(null) as Record; + const normalizedNames = objectCreate(null) as Record; + for (let index = 0; index < names.length; index += 1) { + const key = names[index]; + if (typeof key !== 'string' || key.length === 0) { + return { rejection: TRANSPORT_REJECTION.ENVIRONMENT_ENTRY_INVALID, value: empty }; + } + if (containsNul(key) || reflectApply(stringIndexOf, key, ['=']) !== -1) { + return { rejection: TRANSPORT_REJECTION.ENVIRONMENT_ENTRY_INVALID, value: empty }; + } + if (utf8ByteLength(key) > TRANSPORT_BOUNDS.MAX_ENV_KEY_BYTES) { + return { rejection: TRANSPORT_REJECTION.ENVIRONMENT_BYTES_EXCEEDED, value: empty }; + } + + if (platform === 'win32') { + const normalized = reflectApply(stringToLowerCase, key, []); + if (typeof normalized !== 'string') { + return { rejection: TRANSPORT_REJECTION.ENVIRONMENT_ENTRY_INVALID, value: empty }; + } + if (objectGetOwnPropertyDescriptor(normalizedNames, normalized) !== undefined) { + return { rejection: TRANSPORT_REJECTION.ENVIRONMENT_NAME_DUPLICATED, value: empty }; + } + objectDefineProperty(normalizedNames, normalized, { + value: true, + writable: false, + enumerable: true, + configurable: false, + }); + } + + let descriptor; + try { + descriptor = objectGetOwnPropertyDescriptor(raw, key); + } catch { + return { rejection: TRANSPORT_REJECTION.ENVIRONMENT_UNREADABLE, value: empty }; + } + if (descriptor === undefined || !('value' in descriptor)) { + return { rejection: TRANSPORT_REJECTION.ENVIRONMENT_ENTRY_INVALID, value: empty }; + } + const value: unknown = descriptor.value; + if (typeof value !== 'string') { + return { rejection: TRANSPORT_REJECTION.ENVIRONMENT_ENTRY_INVALID, value: empty }; + } + if (containsNul(value)) { + return { rejection: TRANSPORT_REJECTION.ENVIRONMENT_ENTRY_INVALID, value: empty }; + } + if (utf8ByteLength(value) > TRANSPORT_BOUNDS.MAX_ENV_VALUE_BYTES) { + return { rejection: TRANSPORT_REJECTION.ENVIRONMENT_BYTES_EXCEEDED, value: empty }; + } + + objectDefineProperty(environment, key, { + value, + writable: false, + enumerable: true, + configurable: false, + }); + } + + if (platform === 'win32') { + for (let index = 0; index < WINDOWS_REQUIRED_ENVIRONMENT_NAMES.length; index += 1) { + const required = WINDOWS_REQUIRED_ENVIRONMENT_NAMES[index]; + if (required === undefined) { + return { + rejection: TRANSPORT_REJECTION.ENVIRONMENT_REQUIRED_VARIABLE_MISSING, + value: empty, + }; + } + const normalized = reflectApply(stringToLowerCase, required, []); + if ( + typeof normalized !== 'string' || + objectGetOwnPropertyDescriptor(normalizedNames, normalized) === undefined + ) { + return { + rejection: TRANSPORT_REJECTION.ENVIRONMENT_REQUIRED_VARIABLE_MISSING, + value: empty, + }; + } + } + } + + return { rejection: null, value: objectFreeze(environment) }; +} + +/** + * Narrow an untrusted value to something usable as an `AbortSignal`. + * + * The captured platform getter performs the brand check without consulting + * caller-controlled properties or methods. Cross-realm signals with compatible + * platform internal slots remain accepted. + */ +function readSignal(raw: unknown): { + readonly rejection: TransportRejection | null; + readonly value: AbortSignal | null; +} { + if (raw === undefined || raw === null) { + return { rejection: null, value: null }; + } + if (typeof raw !== 'object') { + return { rejection: TRANSPORT_REJECTION.ABORT_SIGNAL_INVALID, value: null }; + } + if (abortSignalAborted === undefined) { + return { rejection: TRANSPORT_REJECTION.ABORT_SIGNAL_INVALID, value: null }; + } + try { + const aborted: unknown = reflectApply(abortSignalAborted, raw, []); + if (typeof aborted !== 'boolean') { + return { rejection: TRANSPORT_REJECTION.ABORT_SIGNAL_INVALID, value: null }; + } + } catch { + return { rejection: TRANSPORT_REJECTION.ABORT_SIGNAL_INVALID, value: null }; + } + return { rejection: null, value: raw as AbortSignal }; +} + +/** + * Validate a specification and its limits, reading every field exactly once. + * + * Pure, total, and deterministic: it never throws, never spawns, never touches + * the filesystem, and returns the same refusal for the same malformed input. + * + * **Single-read discipline.** Every field is read once into a local and the + * snapshot is what later reaches `spawn`. A getter that returns one value when + * validated and another when used cannot exist here, because accessors are + * never invoked and the original object is never consulted again. + * + * Fields are checked in a fixed order, so a request with several problems + * always reports the same one. + */ +export function readInvocation( + spec: AgentProcessSpec, + limits: TransportLimits, + platform: TransportPlatform, +): InvocationReadResult { + const rawSpec: unknown = spec; + if (!isReadableObject(rawSpec)) { + return refuse(TRANSPORT_REJECTION.SPEC_UNREADABLE); + } + const rawLimits: unknown = limits; + if (!isReadableObject(rawLimits)) { + return refuse(TRANSPORT_REJECTION.LIMITS_UNREADABLE); + } + + const rawExecutable: unknown = readOwnData(rawSpec, 'executablePath'); + const executableFailure = checkPath( + rawExecutable, + platform, + TRANSPORT_REJECTION.EXECUTABLE_INVALID, + TRANSPORT_REJECTION.EXECUTABLE_NOT_ABSOLUTE, + ); + if (executableFailure !== null) { + return refuse(executableFailure); + } + const executablePath = rawExecutable as string; + if (hasForbiddenSuffix(executablePath)) { + return refuse(TRANSPORT_REJECTION.EXECUTABLE_SUFFIX_FORBIDDEN); + } + + const rawWorkingDirectory: unknown = readOwnData(rawSpec, 'workingDirectory'); + const workingDirectoryFailure = checkPath( + rawWorkingDirectory, + platform, + TRANSPORT_REJECTION.WORKING_DIRECTORY_INVALID, + TRANSPORT_REJECTION.WORKING_DIRECTORY_NOT_ABSOLUTE, + ); + if (workingDirectoryFailure !== null) { + return refuse(workingDirectoryFailure); + } + + const argsResult = readArgs(readOwnData(rawSpec, 'args')); + if (argsResult.rejection !== null) { + return refuse(argsResult.rejection); + } + + const environmentResult = readEnvironment( + readOwnData(rawSpec, 'environment'), + platform, + ); + if (environmentResult.rejection !== null) { + return refuse(environmentResult.rejection); + } + + const rawStdin: unknown = readOwnData(rawSpec, 'stdin'); + if (typeof rawStdin !== 'string') { + return refuse(TRANSPORT_REJECTION.STDIN_NOT_STRING); + } + if (utf8ByteLength(rawStdin) > TRANSPORT_BOUNDS.MAX_STDIN_BYTES) { + return refuse(TRANSPORT_REJECTION.STDIN_BYTES_EXCEEDED); + } + + const timeoutMs = readBoundedInteger( + readOwnData(rawLimits, 'timeoutMs'), + TRANSPORT_BOUNDS.MIN_TIMEOUT_MS, + TRANSPORT_BOUNDS.MAX_TIMEOUT_MS, + ); + if (timeoutMs === null) { + return refuse(TRANSPORT_REJECTION.TIMEOUT_OUT_OF_RANGE); + } + const graceMs = readBoundedInteger( + readOwnData(rawLimits, 'graceMs'), + TRANSPORT_BOUNDS.MIN_GRACE_MS, + TRANSPORT_BOUNDS.MAX_GRACE_MS, + ); + if (graceMs === null) { + return refuse(TRANSPORT_REJECTION.GRACE_OUT_OF_RANGE); + } + const maxStdoutBytes = readBoundedInteger( + readOwnData(rawLimits, 'maxStdoutBytes'), + 0, + TRANSPORT_BOUNDS.MAX_STDOUT_BYTES_CEILING, + ); + if (maxStdoutBytes === null) { + return refuse(TRANSPORT_REJECTION.STDOUT_LIMIT_OUT_OF_RANGE); + } + const maxStderrBytes = readBoundedInteger( + readOwnData(rawLimits, 'maxStderrBytes'), + 0, + TRANSPORT_BOUNDS.MAX_STDERR_BYTES_CEILING, + ); + if (maxStderrBytes === null) { + return refuse(TRANSPORT_REJECTION.STDERR_LIMIT_OUT_OF_RANGE); + } + + const signalResult = readSignal(readOwnData(rawLimits, 'signal')); + if (signalResult.rejection !== null) { + return refuse(signalResult.rejection); + } + + return { + rejection: null, + value: objectFreeze({ + executablePath, + args: argsResult.value, + workingDirectory: rawWorkingDirectory as string, + environment: environmentResult.value, + stdin: rawStdin, + timeoutMs, + graceMs, + maxStdoutBytes, + maxStderrBytes, + signal: signalResult.value, + }), + }; +} + +/** + * Drop a trailing incomplete UTF-8 sequence. + * + * Bounding happens in bytes, so a cap can land in the middle of a multi-byte + * character. Decoding that directly would emit U+FFFD for a character the child + * actually wrote in full — the transcript would misrepresent its own source. The + * partial tail is dropped instead, and the caller already knows the value was + * cut because truncation is flagged separately. + * + * Only a *trailing partial* sequence is removed. Genuinely invalid UTF-8 + * elsewhere in the buffer is left alone and decodes to U+FFFD, because it is not + * an artefact of bounding and hiding it would be a different kind of lie. + */ +export function trimPartialUtf8(buffer: Buffer): Buffer { + const length = buffer.length; + if (length === 0) { + return buffer; + } + const last = buffer[length - 1]; + if (last === undefined || last < 0x80) { + return buffer; + } + + let start = length - 1; + let steps = 0; + while (start >= 0 && steps < 3) { + const byte = buffer[start]; + if (byte === undefined) { + return buffer; + } + if ((byte & 0xc0) !== 0x80) { + break; + } + start -= 1; + steps += 1; + } + if (start < 0) { + return reflectApply(bufferSubarray, buffer, [0, 0]); + } + + const lead = buffer[start]; + if (lead === undefined) { + return buffer; + } + let expected = 0; + if ((lead & 0x80) === 0x00) { + expected = 1; + } else if ((lead & 0xe0) === 0xc0) { + expected = 2; + } else if ((lead & 0xf0) === 0xe0) { + expected = 3; + } else if ((lead & 0xf8) === 0xf0) { + expected = 4; + } else { + // A continuation byte or an invalid lead in the final position is not a + // recoverable sequence, so the run is dropped rather than guessed at. + return reflectApply(bufferSubarray, buffer, [0, start]); + } + + const available = length - start; + return available >= expected + ? buffer + : reflectApply(bufferSubarray, buffer, [0, start]); +} diff --git a/src/adapters/process-transport.ts b/src/adapters/process-transport.ts new file mode 100644 index 0000000..23187cd --- /dev/null +++ b/src/adapters/process-transport.ts @@ -0,0 +1,806 @@ +/** + * The one place in AgentBridge that starts an operating-system process. + * + * validated specification -> one child process -> one frozen AgentExchange + * + * This module is **dormant in PR 010**. It is not exported from `src/index.ts`, + * it is not re-exported by any barrel, and no production code invokes it. That + * is a statement about wiring, not about safety: a source module can still be + * imported by an internal module or by deep path, so nothing here should be read + * as "unreachable by construction". Before any production caller invokes it, a + * later adapter must enforce an unforgeable, single-use authorization capability + * derived from PR 003's `evaluateActionRequest`. **This module performs no + * policy authorization of its own and must never gain any.** + * + * Scope: process communication only. Nothing here parses stdout, builds an + * `AgentReport`, calls `ingestInvocationReport`, judges completion, evaluates + * freshness, computes policy, persists, logs, retries, queues, or generates an + * identifier. `stdout` and `stderr` leave as untrusted text. + * + * What the *child* does inside its assigned working directory — including + * editing, committing, or pushing within a Git worktree it was given — is that + * agent's own authority under its own credentials, exactly as + * `docs/architecture/006-agent-invocation-boundary.md` describes. AgentBridge + * itself writes no file and runs no Git command: this module imports no + * filesystem API at all. + * + * ## No shell, on any path + * + * `spawn` is always called with `shell: false`. There is no `exec`, no + * `execSync`, no `cmd.exe /c`, no `powershell -Command`, and no composed command + * line anywhere in this file — including the Windows termination path, where + * `taskkill.exe` is spawned directly from a validated absolute path with a fixed + * argument vector whose only variable is a decimal PID this module produced + * itself. + * + * ## Termination is qualified, and says so + * + * Descendant termination is attempted through a POSIX process group or through + * `taskkill /T /F`, and the resulting {@link TerminationScope} records what was + * *requested*, never that it completed. A descendant that deliberately detaches + * itself — `setsid` on POSIX, re-parenting on Windows — is outside the guarantee + * this transport can offer. Absolute process-tree termination is **not claimed** + * and would require a Windows Job Object or Linux cgroups, both of which need + * either a native addon or a single-platform mechanism. + */ + +import { ChildProcess, spawn } from 'node:child_process'; +import { EventEmitter } from 'node:events'; +import { Readable, Writable } from 'node:stream'; + +import { + type AgentExchange, + type AgentProcessSpec, + containsNul, + isAbsolutePath, + readInvocation, + TERMINATION_SCOPE, + type TerminationScope, + TRANSPORT_BOUNDS, + TRANSPORT_OUTCOME, + type TransportLimits, + type TransportOutcome, + type TransportPlatform, + type TransportRejection, + trimPartialUtf8, + utf8ByteLength, +} from './agent-transport.js'; + +/** + * Intrinsics captured at module load, before any child output can be observed. + * Same pattern as the domain boundaries. + */ +const objectFreeze = Object.freeze; +const objectDefineProperty = Object.defineProperty; +const reflectApply = Reflect.apply; +const NativePromise = Promise; +const scheduleTimeout = setTimeout; +const cancelTimeout = clearTimeout; +const runtimeProcess = process; +// `Buffer.isBuffer` and `Buffer.concat` are statics that ignore `this`, captured +// so a later reassignment of the global cannot change how child output is read. +/* eslint-disable @typescript-eslint/unbound-method */ +const bufferIsBuffer = Buffer.isBuffer; +const bufferConcat = Buffer.concat; +// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment +const bufferSubarray: (this: Buffer, start: number, end?: number) => Buffer = + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + Buffer.prototype.subarray; +// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment +const bufferToString: (this: Buffer, encoding: BufferEncoding) => string = + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + Buffer.prototype.toString; +const stringCharCodeAt = String.prototype.charCodeAt; +const numberToString = Number.prototype.toString; +const eventTargetAddEventListener = EventTarget.prototype.addEventListener; +const eventTargetRemoveEventListener = EventTarget.prototype.removeEventListener; +const eventEmitterOn = EventEmitter.prototype.on; +const eventEmitterRemoveListener = EventEmitter.prototype.removeListener; +const eventEmitterRemoveAllListeners = EventEmitter.prototype.removeAllListeners; +const readableOn = Readable.prototype.on; +const writableEnd = Writable.prototype.end; +const childProcessKill = ChildProcess.prototype.kill; +const processKill = process.kill; +const abortSignalAborted: ((this: AbortSignal) => boolean) | undefined = + Object.getOwnPropertyDescriptor(AbortSignal.prototype, 'aborted')?.get; +/* eslint-enable @typescript-eslint/unbound-method */ + +/** + * Bound on how long the Windows tree-kill helper may run before it is itself + * abandoned and the direct-child fallback is used. Independent of the caller's + * grace period, so a caller cannot make termination unbounded by supplying a + * large one, and cannot make it unreliable by supplying zero. + */ +const TASKKILL_TIMEOUT_MS = 5_000; + +function resolved(value: T): Promise { + return new NativePromise((resolve) => { + resolve(value); + }); +} + +function onEvent( + emitter: EventEmitter, + event: string, + listener: (...args: never[]) => void, +): void { + reflectApply(eventEmitterOn, emitter, [event, listener]); +} + +function removeEventListener( + emitter: EventEmitter, + event: string, + listener: (...args: never[]) => void, +): void { + reflectApply(eventEmitterRemoveListener, emitter, [event, listener]); +} + +function removeAllEvents(emitter: EventEmitter): void { + reflectApply(eventEmitterRemoveAllListeners, emitter, []); +} + +function onReadableData(readable: Readable, listener: (chunk: unknown) => void): void { + // Readable overrides EventEmitter.on to enter flowing mode for `data`. + reflectApply(readableOn, readable, ['data', listener]); +} + +/** Append by defining an own element, bypassing inherited index setters. */ +function append(list: T[], value: T): void { + objectDefineProperty(list, list.length, { + value, + writable: true, + enumerable: true, + configurable: true, + }); +} + +/** A bounded byte accumulator for one stream. */ +interface Sink { + readonly chunks: Buffer[]; + readonly limit: number; + bytes: number; + truncated: boolean; +} + +function createSink(limit: number): Sink { + return { chunks: [], limit, bytes: 0, truncated: false }; +} + +/** + * Add a chunk, keeping at most `limit` bytes. + * + * Returns true once the bound has been reached, which is what promotes the + * exchange to `OUTPUT_LIMIT_EXCEEDED`. A stream that lands exactly on the bound + * is **not** truncated; the next byte is what makes it so. + */ +function pushChunk(sink: Sink, chunk: Buffer): boolean { + if (sink.bytes >= sink.limit) { + sink.truncated = true; + return true; + } + const room = sink.limit - sink.bytes; + if (chunk.length > room) { + append(sink.chunks, reflectApply(bufferSubarray, chunk, [0, room])); + sink.bytes = sink.limit; + sink.truncated = true; + return true; + } + append(sink.chunks, chunk); + sink.bytes += chunk.length; + return false; +} + +/** Join, trim only transport-cut UTF-8, and decode natural invalid bytes. */ +function decodeSink(sink: Sink): { readonly text: string; readonly bytes: number } { + const joined = bufferConcat(sink.chunks); + const retained = sink.truncated ? trimPartialUtf8(joined) : joined; + return { + text: reflectApply(bufferToString, retained, ['utf8']), + bytes: retained.length, + }; +} + +/** Read a validated signal through the captured platform brand-checking getter. */ +function readAbortState(signal: AbortSignal): boolean | null { + if (abortSignalAborted === undefined) { + return null; + } + try { + const state: unknown = reflectApply(abortSignalAborted, signal, []); + return typeof state === 'boolean' ? state : null; + } catch { + return null; + } +} + +/** Register without consulting caller-controlled signal properties. */ +function addAbortListener(signal: AbortSignal, listener: EventListener): boolean { + try { + reflectApply(eventTargetAddEventListener, signal, ['abort', listener, { once: true }]); + return true; + } catch { + return false; + } +} + +/** Best-effort cleanup through the captured platform intrinsic. */ +function removeAbortListener(signal: AbortSignal, listener: EventListener): void { + try { + reflectApply(eventTargetRemoveEventListener, signal, ['abort', listener]); + } catch { + // A platform failure cannot be allowed to reject an otherwise total exchange. + } +} + +/** An exchange that never reached the operating system. */ +function unspawnedExchange( + outcome: TransportOutcome, + rejection: TransportRejection | null, +): AgentExchange { + return objectFreeze({ + outcome, + rejection, + exitCode: null, + terminatingSignal: null, + stdout: '', + stderr: '', + stdoutTruncated: false, + stderrTruncated: false, + stdoutBytes: 0, + stderrBytes: 0, + terminationScope: TERMINATION_SCOPE.NOT_REQUIRED, + }); +} + +/** True when a caught value is a POSIX "no such process" error. */ +function isNoSuchProcess(error: unknown): boolean { + if (typeof error !== 'object' || error === null) { + return false; + } + const code: unknown = (error as { readonly code?: unknown }).code; + return code === 'ESRCH'; +} + +/** Signal the child's own process group. True when the group was reached. */ +function signalProcessGroup(pid: number, signal: NodeJS.Signals): boolean { + try { + reflectApply(processKill, runtimeProcess, [-pid, signal]); + return true; + } catch (error: unknown) { + // ESRCH means the group is already gone, which is the state we wanted. + return isNoSuchProcess(error); + } +} + +/** Signal only the direct child, ignoring an already-dead process. */ +function killDirectChild(child: ChildProcess, signal?: NodeJS.Signals): void { + try { + reflectApply(childProcessKill, child, signal === undefined ? [] : [signal]); + } catch { + // The child already exited; there is nothing left to signal. + } +} + +/** True when the child has already been observed to end. */ +function hasEnded(child: ChildProcess): boolean { + return child.exitCode !== null || child.signalCode !== null; +} + +/** Resolve true when the child ends within `ms`, false when it outlives it. */ +function waitForExit(child: ChildProcess, ms: number): Promise { + if (hasEnded(child)) { + return resolved(true); + } + return new NativePromise((resolve) => { + let done = false; + const finish = (value: boolean): void => { + if (done) { + return; + } + done = true; + cancelTimeout(timer); + removeEventListener(child, 'exit', onExit); + resolve(value); + }; + const onExit = (): void => { + finish(true); + }; + const timer = scheduleTimeout(() => { + finish(false); + }, ms); + onEvent(child, 'exit', onExit); + }); +} + +/** + * Locate `taskkill.exe` from the Windows system directory. + * + * `C:\Windows` is **not** assumed. The directory comes from the transport's own + * `SystemRoot` (or `windir`) and is validated as an absolute, NUL-free, bounded + * path before use; anything else yields `null`, which degrades termination + * honestly rather than guessing at a path. + * + * This value is read for this internal operation only. It is never added to the + * child's environment, never written into an exchange, and never echoed + * anywhere — the child environment remains exactly what the caller supplied. + */ +function resolveTaskkill(): { readonly executable: string; readonly systemRoot: string } | null { + const raw: unknown = runtimeProcess.env['SystemRoot'] ?? runtimeProcess.env['windir']; + if (typeof raw !== 'string' || raw.length === 0) { + return null; + } + if (containsNul(raw)) { + return null; + } + if (utf8ByteLength(raw) > TRANSPORT_BOUNDS.MAX_PATH_BYTES) { + return null; + } + if (!isAbsolutePath(raw, 'win32')) { + return null; + } + const last = reflectApply(stringCharCodeAt, raw, [raw.length - 1]); + const separator = last === 0x5c || last === 0x2f ? '' : '\\'; + return { executable: `${raw}${separator}System32\\taskkill.exe`, systemRoot: raw }; +} + +/** + * Ask Windows to end the child's process tree. + * + * Spawned directly — no shell, no PATH search, no composed command line, and no + * caller-controlled argument. The only variable is a decimal PID this module + * produced. Resolves true only when `taskkill` actually ran to a conclusive + * exit; exit code 128 counts, because it means the target was already gone. + */ +function runTaskkill( + taskkill: { readonly executable: string; readonly systemRoot: string }, + pid: number, +): Promise { + return new NativePromise((resolve) => { + let killer: ChildProcess; + try { + const decimalPid = reflectApply(numberToString, pid, []); + killer = spawn(taskkill.executable, ['/PID', decimalPid, '/T', '/F'], { + stdio: 'ignore', + shell: false, + windowsHide: true, + windowsVerbatimArguments: false, + env: { SystemRoot: taskkill.systemRoot }, + }); + } catch { + resolve(false); + return; + } + + let done = false; + let reapTimer: NodeJS.Timeout | null = null; + const finish = (value: boolean): void => { + if (done) { + return; + } + done = true; + cancelTimeout(timer); + if (reapTimer !== null) { + cancelTimeout(reapTimer); + } + removeAllEvents(killer); + resolve(value); + }; + const timer = scheduleTimeout(() => { + if (hasEnded(killer)) { + finish(false); + return; + } + killDirectChild(killer); + // Observe the helper's exit after killing it. The second bound preserves + // totality even if the operating system never reports a terminal event. + reapTimer = scheduleTimeout(() => { + finish(false); + }, TASKKILL_TIMEOUT_MS); + }, TASKKILL_TIMEOUT_MS); + onEvent(killer, 'error', () => { + finish(false); + }); + onEvent(killer, 'exit', (code: number | null) => { + finish(code === 0 || code === 128); + }); + }); +} + +/** + * POSIX termination: signal the process group, then escalate. + * + * The child was spawned `detached`, so it leads its own process group and + * `kill(-pid, ...)` reaches its ordinary descendants. A descendant that called + * `setsid` itself has left that group and is not reached — which is why the + * returned scope says *requested*, never *completed*. + */ +async function terminatePosix( + child: ChildProcess, + pid: number, + graceMs: number, +): Promise { + let groupReached = signalProcessGroup(pid, 'SIGTERM'); + if (!groupReached) { + killDirectChild(child, 'SIGTERM'); + } + if (await waitForExit(child, graceMs)) { + return groupReached + ? TERMINATION_SCOPE.PROCESS_GROUP_REQUESTED + : TERMINATION_SCOPE.DIRECT_CHILD_ONLY; + } + + if (!signalProcessGroup(pid, 'SIGKILL')) { + killDirectChild(child, 'SIGKILL'); + groupReached = false; + } + await waitForExit(child, graceMs); + return groupReached + ? TERMINATION_SCOPE.PROCESS_GROUP_REQUESTED + : TERMINATION_SCOPE.DIRECT_CHILD_ONLY; +} + +/** + * Windows termination: ask `taskkill /T /F`, and fall back honestly. + * + * Returns only after the `taskkill` attempt has finished or reached its own + * bounded failure path, and after the direct child has been waited on. When + * `taskkill` cannot start, fails, or times out, the direct child is terminated + * and the scope degrades to `DIRECT_CHILD_ONLY` — descendants are not claimed. + */ +async function terminateWindows( + child: ChildProcess, + pid: number, + graceMs: number, +): Promise { + const taskkill = resolveTaskkill(); + if (taskkill === null) { + killDirectChild(child); + await waitForExit(child, graceMs); + return TERMINATION_SCOPE.DIRECT_CHILD_ONLY; + } + + const issued = await runTaskkill(taskkill, pid); + if (!issued) { + killDirectChild(child); + await waitForExit(child, graceMs); + return TERMINATION_SCOPE.DIRECT_CHILD_ONLY; + } + + await waitForExit(child, graceMs); + return TERMINATION_SCOPE.PROCESS_TREE_REQUESTED; +} + +/** Dispatch termination to the platform strategy. */ +async function terminate( + child: ChildProcess, + platform: TransportPlatform, + graceMs: number, +): Promise { + const pid = child.pid; + if (pid === undefined) { + // Never started, so nothing beyond the handle can be reached. Reported as + // degraded rather than as a successful group or tree request. + return TERMINATION_SCOPE.DIRECT_CHILD_ONLY; + } + return platform === 'posix' + ? terminatePosix(child, pid, graceMs) + : terminateWindows(child, pid, graceMs); +} + +/** + * Run one process exchange. + * + * **Total.** Resolves to exactly one frozen {@link AgentExchange} on every + * validation, spawn, I/O, timeout, cancellation, overflow, termination, and + * close path. It never rejects and never throws by design. Catches are placed + * only around defined operational failures — `spawn`, `kill`, a broken stdin + * pipe, a hostile `AbortSignal` getter — so a programmer defect still surfaces + * as a defect rather than being laundered into a failure code. + * + * **Deterministic precedence.** The first terminal cause to be claimed wins and + * is immutable; see `TERMINAL_CAUSE_PRECEDENCE`. Overflow, cancellation, and + * timeout are claimed the moment they are detected, while `SIGNALLED` and + * `EXITED` are claimed only after stdio closes, so a child that overflows its + * bound and then exits zero is reported as `OUTPUT_LIMIT_EXCEEDED`. + * + * **No policy.** Nothing here decides whether this process should run. That + * question belongs to `evaluateActionRequest` and to a later adapter that must + * hold an unforgeable capability before calling this function. + * + * @param spec Process specification. Validated structurally; never trusted to + * be well-typed at runtime. + * @param limits Bounds and optional cancellation for this exchange. + */ +export function invokeAgentProcess( + spec: AgentProcessSpec, + limits: TransportLimits, +): Promise { + const platform: TransportPlatform = + runtimeProcess.platform === 'win32' ? 'win32' : 'posix'; + + // Precedence step 1: structural validation runs before the abort check, so a + // request that is both malformed and already aborted is SPEC_REJECTED. + const read = readInvocation(spec, limits, platform); + if (read.rejection !== null) { + return resolved( + unspawnedExchange(TRANSPORT_OUTCOME.SPEC_REJECTED, read.rejection), + ); + } + const invocation = read.value; + + let abortPending = false; + let abortDispatch: (() => void) | null = null; + const onAbort: EventListener = () => { + if (abortDispatch === null) { + abortPending = true; + return; + } + abortDispatch(); + }; + if (invocation.signal !== null) { + const beforeRegistration = readAbortState(invocation.signal); + if (beforeRegistration === null) { + return resolved( + unspawnedExchange( + TRANSPORT_OUTCOME.SPEC_REJECTED, + 'ABORT_SIGNAL_INVALID', + ), + ); + } + if (beforeRegistration) { + return resolved(unspawnedExchange(TRANSPORT_OUTCOME.CANCELLED, null)); + } + if (!addAbortListener(invocation.signal, onAbort)) { + return resolved( + unspawnedExchange( + TRANSPORT_OUTCOME.SPEC_REJECTED, + 'ABORT_SIGNAL_INVALID', + ), + ); + } + const afterRegistration = readAbortState(invocation.signal); + if (afterRegistration === null || afterRegistration) { + removeAbortListener(invocation.signal, onAbort); + return resolved( + unspawnedExchange( + afterRegistration === null + ? TRANSPORT_OUTCOME.SPEC_REJECTED + : TRANSPORT_OUTCOME.CANCELLED, + afterRegistration === null ? 'ABORT_SIGNAL_INVALID' : null, + ), + ); + } + } + + return new NativePromise((resolve) => { + let child: ChildProcess; + try { + child = spawn(invocation.executablePath, invocation.args, { + cwd: invocation.workingDirectory, + env: invocation.environment, + stdio: ['pipe', 'pipe', 'pipe'], + shell: false, + windowsHide: true, + windowsVerbatimArguments: false, + // POSIX only: makes the child a process-group leader so its ordinary + // descendants can be signalled together. On Windows `detached` would + // allocate a new console instead, which does not help termination. + detached: platform === 'posix', + }); + } catch { + if (invocation.signal !== null) { + removeAbortListener(invocation.signal, onAbort); + } + resolve(unspawnedExchange(TRANSPORT_OUTCOME.SPAWN_FAILED, null)); + return; + } + + const stdoutSink = createSink(invocation.maxStdoutBytes); + const stderrSink = createSink(invocation.maxStderrBytes); + + let cause: TransportOutcome | null = null; + let settled = false; + let closed = false; + let terminating = false; + let exitCode: number | null = null; + let terminatingSignal: string | null = null; + let terminationScope: TerminationScope = TERMINATION_SCOPE.NOT_REQUIRED; + let deadline: NodeJS.Timeout | null = null; + let notifyClosed: (() => void) | null = null; + + /** First writer wins. A claimed cause is never overwritten. */ + const claim = (next: TransportOutcome): boolean => { + if (cause !== null) { + return false; + } + cause = next; + return true; + }; + + const dispatchAbort = (): void => { + if (claim(TRANSPORT_OUTCOME.CANCELLED)) { + void runTermination(); + } + }; + + const cleanup = (): void => { + if (deadline !== null) { + cancelTimeout(deadline); + deadline = null; + } + if (notifyClosed !== null) { + // Releases the bounded close-wait timer so no timer outlives the + // exchange, even on a path that settles while that wait is pending. + const notify = notifyClosed; + notifyClosed = null; + notify(); + } + if (invocation.signal !== null) { + removeAbortListener(invocation.signal, onAbort); + } + if (child.stdout !== null) { + removeAllEvents(child.stdout); + } + if (child.stderr !== null) { + removeAllEvents(child.stderr); + } + if (child.stdin !== null) { + removeAllEvents(child.stdin); + } + removeAllEvents(child); + }; + + const settle = (): void => { + if (settled) { + return; + } + settled = true; + cleanup(); + const out = decodeSink(stdoutSink); + const err = decodeSink(stderrSink); + resolve( + objectFreeze({ + outcome: cause ?? TRANSPORT_OUTCOME.EXITED, + rejection: null, + exitCode, + terminatingSignal, + stdout: out.text, + stderr: err.text, + stdoutTruncated: stdoutSink.truncated, + stderrTruncated: stderrSink.truncated, + stdoutBytes: out.bytes, + stderrBytes: err.bytes, + terminationScope, + }), + ); + }; + + /** Resolve when stdio closes, or after a bounded wait — whichever is first. */ + function awaitClose(ms: number): Promise { + if (closed) { + return resolved(undefined); + } + return new NativePromise((resolveWait) => { + const waiter = scheduleTimeout(() => { + notifyClosed = null; + resolveWait(); + }, ms); + notifyClosed = (): void => { + cancelTimeout(waiter); + resolveWait(); + }; + }); + } + + /** + * Terminate, then settle. + * + * Settling is deferred until termination has finished reporting, so an + * exchange can never resolve with `NOT_REQUIRED` while a kill it initiated + * is still in flight. + * + * **This always settles.** Waiting for `close` alone is not safe: a + * descendant that inherited the stdio pipes keeps them open after the direct + * child is gone, and one that escaped termination keeps them open forever, + * so `close` may never arrive. Once termination has reported, stdio gets one + * bounded chance to close and the exchange resolves regardless. Totality + * outranks a complete transcript, and the transcript is already known to be + * partial whenever this path runs. + */ + async function runTermination(): Promise { + if (terminating) { + return; + } + terminating = true; + terminationScope = await terminate(child, platform, invocation.graceMs); + terminating = false; + + if (!closed) { + if (!hasEnded(child)) { + terminationScope = TERMINATION_SCOPE.ESCALATION_FAILED; + } + await awaitClose(invocation.graceMs); + } + settle(); + } + + const onStdout = (chunk: unknown): void => { + if (!bufferIsBuffer(chunk)) { + return; + } + if (pushChunk(stdoutSink, chunk) && claim(TRANSPORT_OUTCOME.OUTPUT_LIMIT_EXCEEDED)) { + void runTermination(); + } + }; + + const onStderr = (chunk: unknown): void => { + if (!bufferIsBuffer(chunk)) { + return; + } + if (pushChunk(stderrSink, chunk) && claim(TRANSPORT_OUTCOME.OUTPUT_LIMIT_EXCEEDED)) { + void runTermination(); + } + }; + + if (child.stdout !== null) { + onReadableData(child.stdout, onStdout); + } + if (child.stderr !== null) { + onReadableData(child.stderr, onStderr); + } + + onEvent(child, 'error', () => { + // Only a failure to start is terminal on its own. A post-spawn error such + // as a broken pipe is recorded by the close path instead. + if (child.pid === undefined) { + claim(TRANSPORT_OUTCOME.SPAWN_FAILED); + settle(); + } + }); + + onEvent(child, 'exit', (code: number | null, signalName: NodeJS.Signals | null) => { + exitCode = code; + terminatingSignal = signalName; + }); + + onEvent(child, 'close', () => { + closed = true; + // Claimed here rather than on 'exit', so output that arrives between exit + // and close can still promote the exchange to OUTPUT_LIMIT_EXCEEDED. + claim( + terminatingSignal !== null + ? TRANSPORT_OUTCOME.SIGNALLED + : TRANSPORT_OUTCOME.EXITED, + ); + if (notifyClosed !== null) { + const notify = notifyClosed; + notifyClosed = null; + notify(); + } + if (!terminating) { + settle(); + } + }); + + const stdin = child.stdin; + if (stdin !== null) { + onEvent(stdin, 'error', () => { + // A child that exits before reading breaks the pipe. That is the + // child's behaviour, not a transport failure, and the close path + // decides the outcome. + }); + reflectApply(writableEnd, stdin, [invocation.stdin, 'utf8']); + } + + abortDispatch = dispatchAbort; + if (abortPending || (invocation.signal !== null && readAbortState(invocation.signal))) { + dispatchAbort(); + } + + deadline = scheduleTimeout(() => { + if (claim(TRANSPORT_OUTCOME.TIMED_OUT)) { + void runTermination(); + } + }, invocation.timeoutMs); + }); +} diff --git a/tests/adapters/process-transport.test.ts b/tests/adapters/process-transport.test.ts new file mode 100644 index 0000000..673da1e --- /dev/null +++ b/tests/adapters/process-transport.test.ts @@ -0,0 +1,724 @@ +import { readdirSync, existsSync, readFileSync, statSync } from 'node:fs'; +import { join } from 'node:path'; + +import { describe, expect, it } from 'vitest'; + +import { + type AgentExchange, + type AgentProcessSpec, + type TransportLimits, +} from '../../src/adapters/agent-transport.js'; +import { invokeAgentProcess } from '../../src/adapters/process-transport.js'; +import { + ascii, + baseEnvironment, + delay, + FORBIDDEN_EXECUTABLES, + heartbeatStub, + makeLimits, + makeSpec, + makeTempDirectory, + NODE_EXECUTABLE, + removeTempDirectory, + SHELL_METACHARACTER_ARGUMENTS, + SHELL_ONLY_EXECUTABLES, + STUB, + withSignal, +} from './transport-fixtures.js'; + +const onPosix = it.skipIf(process.platform === 'win32'); + +/** Run a stub script with optional extra arguments. */ +function runStub( + script: string, + extra: readonly string[] = [], + specOverrides: Partial> = {}, + limits: TransportLimits = makeLimits(), +): Promise { + return invokeAgentProcess( + makeSpec({ + args: ['-e', script, ...extra], + ...(specOverrides.workingDirectory === undefined + ? {} + : { workingDirectory: specOverrides.workingDirectory }), + ...(specOverrides.environment === undefined + ? {} + : { environment: specOverrides.environment }), + ...(specOverrides.stdin === undefined ? {} : { stdin: specOverrides.stdin }), + }), + limits, + ); +} + +describe('invokeAgentProcess — success', () => { + it('runs a process to completion and captures stdout exactly', async () => { + const exchange = await runStub(STUB.WRITE_OK); + + expect(exchange.outcome).toBe('EXITED'); + expect(exchange.exitCode).toBe(0); + expect(exchange.terminatingSignal).toBeNull(); + expect(exchange.stdout).toBe('ok'); + expect(exchange.stderr).toBe(''); + expect(exchange.stdoutTruncated).toBe(false); + expect(exchange.stderrTruncated).toBe(false); + expect(exchange.rejection).toBeNull(); + expect(exchange.terminationScope).toBe('NOT_REQUIRED'); + }); + + it('delivers the stdin payload verbatim and closes stdin', async () => { + const payload = 'line one\nline two\nunicode: é中文 \u{1F600}'; + const exchange = await runStub(STUB.ECHO_STDIN, [], { stdin: payload }); + + expect(exchange.outcome).toBe('EXITED'); + expect(exchange.stdout).toBe(payload); + }); + + it('closes stdin so a child waiting on end-of-file completes', async () => { + const exchange = await runStub(STUB.STDIN_EOF, [], { stdin: 'anything' }); + + expect(exchange.outcome).toBe('EXITED'); + expect(exchange.stdout).toBe('eof'); + }); + + it('accepts an empty stdin payload', async () => { + const exchange = await runStub(STUB.ECHO_STDIN, [], { stdin: '' }); + + expect(exchange.outcome).toBe('EXITED'); + expect(exchange.stdout).toBe(''); + }); + + it('records an empty stdout with a zero exit as a valid exchange', async () => { + const exchange = await runStub(''); + + expect(exchange.outcome).toBe('EXITED'); + expect(exchange.exitCode).toBe(0); + expect(exchange.stdout).toBe(''); + expect(exchange.stdoutBytes).toBe(0); + }); + + it('captures both streams without merging either into the other', async () => { + const exchange = await runStub(STUB.BOTH_STREAMS); + + expect(exchange.stdout).toBe('OUT-AOUT-B'); + expect(exchange.stderr).toBe('ERR-AERR-B'); + expect(exchange.stdout).not.toContain('ERR-'); + expect(exchange.stderr).not.toContain('OUT-'); + }); + + it('runs the child in the working directory it was given', async () => { + const directory = makeTempDirectory(); + try { + const exchange = await runStub(STUB.PRINT_CWD, [], { workingDirectory: directory }); + + expect(exchange.outcome).toBe('EXITED'); + expect(exchange.stdout.toLowerCase()).toBe(directory.toLowerCase()); + } finally { + removeTempDirectory(directory); + } + }); + + it('accepts a zero-argument argv', async () => { + const exchange = await invokeAgentProcess( + makeSpec({ args: ['--version'] }), + makeLimits(), + ); + + expect(exchange.outcome).toBe('EXITED'); + expect(exchange.exitCode).toBe(0); + }); + + it('reports source bytes that match the decoded stdout for valid UTF-8', async () => { + const exchange = await runStub(STUB.MULTIBYTE, ['3']); + + expect(exchange.outcome).toBe('EXITED'); + expect(exchange.stdoutBytes).toBe(Buffer.byteLength(exchange.stdout, 'utf8')); + }); +}); + +describe('invokeAgentProcess — failure', () => { + it('reports SPAWN_FAILED for an absolute path that does not exist', async () => { + const missing = join(makeTempDirectory(), 'no-such-agent-binary'); + const exchange = await invokeAgentProcess( + makeSpec({ executablePath: missing }), + makeLimits(), + ); + + expect(exchange.outcome).toBe('SPAWN_FAILED'); + expect(exchange.rejection).toBeNull(); + expect(exchange.stdout).toBe(''); + expect(exchange.terminationScope).toBe('NOT_REQUIRED'); + }); + + it.each([1, 2, 127, 255])('records exit code %i without interpreting it', async (code) => { + const exchange = await runStub(STUB.EXIT_WITH, [String(code)]); + + expect(exchange.outcome).toBe('EXITED'); + expect(exchange.exitCode).toBe(code); + expect(exchange.terminatingSignal).toBeNull(); + }); + + it('records a non-zero exit alongside stderr without merging the two', async () => { + const exchange = await runStub(STUB.STDERR_ONLY); + + expect(exchange.outcome).toBe('EXITED'); + expect(exchange.exitCode).toBe(3); + expect(exchange.stdout).toBe(''); + expect(exchange.stderr).toBe('diagnostic'); + }); + + it('times out a child that never exits', async () => { + const exchange = await runStub( + STUB.SLEEP, + [], + {}, + makeLimits({ timeoutMs: 400, graceMs: 200 }), + ); + + expect(exchange.outcome).toBe('TIMED_OUT'); + expect(exchange.terminationScope).not.toBe('NOT_REQUIRED'); + }); + + it('escalates past a child that ignores SIGTERM', async () => { + const exchange = await runStub( + STUB.IGNORE_SIGTERM, + [], + {}, + makeLimits({ timeoutMs: 400, graceMs: 300 }), + ); + + expect(exchange.outcome).toBe('TIMED_OUT'); + expect(['PROCESS_GROUP_REQUESTED', 'PROCESS_TREE_REQUESTED', 'DIRECT_CHILD_ONLY']).toContain( + exchange.terminationScope, + ); + }, 15_000); + + it('cancels a running child when the signal fires', async () => { + const controller = new AbortController(); + setTimeout(() => { + controller.abort(); + }, 250); + + const exchange = await invokeAgentProcess( + makeSpec({ args: ['-e', STUB.SLEEP] }), + withSignal(makeLimits({ timeoutMs: 15_000, graceMs: 200 }), controller.signal), + ); + + expect(exchange.outcome).toBe('CANCELLED'); + expect(exchange.terminationScope).not.toBe('NOT_REQUIRED'); + }, 15_000); + + it('never spawns when the signal is already aborted', async () => { + const exchange = await invokeAgentProcess( + makeSpec({ args: ['-e', STUB.WRITE_OK] }), + withSignal(makeLimits(), AbortSignal.abort()), + ); + + expect(exchange.outcome).toBe('CANCELLED'); + expect(exchange.stdout).toBe(''); + expect(exchange.terminationScope).toBe('NOT_REQUIRED'); + }); + + it('rejects structural signal lookalikes without invoking hostile methods', async () => { + let invoked = false; + const hostile = { + aborted: false, + addEventListener(): never { + invoked = true; + throw new Error('hostile addEventListener'); + }, + removeEventListener(): never { + invoked = true; + throw new Error('hostile removeEventListener'); + }, + } as unknown as AbortSignal; + const exchange = await invokeAgentProcess( + makeSpec({ args: ['-e', STUB.WRITE_OK] }), + withSignal(makeLimits(), hostile), + ); + + expect(exchange.outcome).toBe('SPEC_REJECTED'); + expect(exchange.rejection).toBe('ABORT_SIGNAL_INVALID'); + expect(exchange.stdout).toBe(''); + expect(invoked).toBe(false); + }); + + it('ignores hostile own event methods on a genuine AbortSignal', async () => { + const controller = new AbortController(); + Object.defineProperty(controller.signal, 'addEventListener', { + value(): never { throw new Error('own add'); }, + }); + Object.defineProperty(controller.signal, 'removeEventListener', { + value(): never { throw new Error('own remove'); }, + }); + setTimeout(() => { + controller.abort(); + }, 25); + + const exchange = await invokeAgentProcess( + makeSpec({ args: ['-e', STUB.SLEEP] }), + withSignal(makeLimits({ graceMs: 100 }), controller.signal), + ); + expect(exchange.outcome).toBe('CANCELLED'); + }); + + it('closes the immediate-abort registration race before timeout', async () => { + const controller = new AbortController(); + const pending = invokeAgentProcess( + makeSpec({ args: ['-e', STUB.SLEEP] }), + withSignal(makeLimits({ timeoutMs: 50, graceMs: 100 }), controller.signal), + ); + controller.abort(); + + const exchange = await pending; + expect(exchange.outcome).toBe('CANCELLED'); + }); + + it('terminates a child whose stdout floods past the bound', async () => { + const exchange = await runStub( + STUB.FLOOD_STDOUT, + [], + {}, + makeLimits({ timeoutMs: 15_000, graceMs: 300, maxStdoutBytes: 4_096 }), + ); + + expect(exchange.outcome).toBe('OUTPUT_LIMIT_EXCEEDED'); + expect(exchange.stdoutTruncated).toBe(true); + expect(exchange.stdoutBytes).toBeLessThanOrEqual(4_096); + }, 15_000); + + it('terminates a child whose stderr floods past the bound', async () => { + const exchange = await runStub( + STUB.FLOOD_STDERR, + [], + {}, + makeLimits({ timeoutMs: 15_000, graceMs: 300, maxStderrBytes: 4_096 }), + ); + + expect(exchange.outcome).toBe('OUTPUT_LIMIT_EXCEEDED'); + expect(exchange.stderrTruncated).toBe(true); + expect(exchange.stderrBytes).toBeLessThanOrEqual(4_096); + }, 15_000); + + onPosix('reports an externally signalled child as SIGNALLED', async () => { + const exchange = await runStub(STUB.SELF_KILL); + + expect(exchange.outcome).toBe('SIGNALLED'); + expect(exchange.exitCode).toBeNull(); + expect(exchange.terminatingSignal).toBe('SIGKILL'); + }); + + it('survives a child that exits without reading stdin', async () => { + const exchange = await runStub(STUB.EXIT_IMMEDIATELY, [], { stdin: ascii(100_000) }); + + expect(exchange.outcome).toBe('EXITED'); + expect(exchange.exitCode).toBe(0); + }); + + it('settles even when a descendant inherits the stdio pipes', async () => { + // The direct child exits at once while a descendant holds stdout and + // stderr. Whether `close` still arrives is a platform detail — Windows + // releases the handles here, a POSIX host may not — so this asserts the + // property that must hold either way: the exchange settles, within the + // deadline, as one frozen record. Waiting on `close` alone could hang. + const exchange = await runStub( + STUB.LEAK_STDIO_THEN_EXIT, + [], + {}, + makeLimits({ timeoutMs: 700, graceMs: 300 }), + ); + + expect(['EXITED', 'TIMED_OUT']).toContain(exchange.outcome); + expect(Object.isFrozen(exchange)).toBe(true); + }, 20_000); + + it('still enforces the deadline when the child closes stdout early', async () => { + const exchange = await runStub( + STUB.CLOSE_STDOUT_KEEP_RUNNING, + [], + {}, + makeLimits({ timeoutMs: 400, graceMs: 300 }), + ); + + expect(exchange.outcome).toBe('TIMED_OUT'); + }, 15_000); +}); + +describe('invokeAgentProcess — adversarial', () => { + it('uses captured Buffer methods after validation poisons the prototype', async () => { + const subarray = Object.getOwnPropertyDescriptor(Buffer.prototype, 'subarray'); + const toString = Object.getOwnPropertyDescriptor(Buffer.prototype, 'toString'); + const target = makeSpec({ args: ['-e', STUB.WRITE_OK] }); + const hostile = new Proxy(target, { + getOwnPropertyDescriptor(object, key) { + Object.defineProperty(Buffer.prototype, 'subarray', { + value(): never { throw new Error('poisoned subarray'); }, + configurable: true, + }); + Object.defineProperty(Buffer.prototype, 'toString', { + value(): never { throw new Error('poisoned toString'); }, + configurable: true, + }); + return Reflect.getOwnPropertyDescriptor(object, key); + }, + }); + let exchange: AgentExchange; + try { + exchange = await invokeAgentProcess(hostile, makeLimits()); + } finally { + if (subarray !== undefined) { + Object.defineProperty(Buffer.prototype, 'subarray', subarray); + } + if (toString !== undefined) { + Object.defineProperty(Buffer.prototype, 'toString', toString); + } + } + expect(exchange.outcome).toBe('EXITED'); + expect(exchange.stdout).toBe('ok'); + }); + + // A leading positional stops `node` parsing later `--`-prefixed payloads as + // its own options. That is the stub interpreter's argument grammar, not the + // transport's: the transport composes nothing and interprets nothing. + const FIRST_POSITIONAL = 'ARGV0'; + + it.each(SHELL_METACHARACTER_ARGUMENTS)( + 'passes %j through as one verbatim argv element', + async (payload) => { + const exchange = await runStub(STUB.PRINT_ARGV, [FIRST_POSITIONAL, payload]); + + expect(exchange.outcome).toBe('EXITED'); + expect(JSON.parse(exchange.stdout)).toEqual([FIRST_POSITIONAL, payload]); + }, + ); + + it('passes an entire hostile argv vector through unchanged', async () => { + const exchange = await runStub(STUB.PRINT_ARGV, [ + FIRST_POSITIONAL, + ...SHELL_METACHARACTER_ARGUMENTS, + ]); + + expect(exchange.outcome).toBe('EXITED'); + expect(JSON.parse(exchange.stdout)).toEqual([ + FIRST_POSITIONAL, + ...SHELL_METACHARACTER_ARGUMENTS, + ]); + }); + + it('never places the stdin payload into argv', async () => { + const secretish = 'PAYLOAD-MUST-NOT-APPEAR-IN-ARGV'; + const exchange = await runStub(STUB.PRINT_ARGV, [], { stdin: secretish }); + + expect(exchange.outcome).toBe('EXITED'); + expect(exchange.stdout).not.toContain(secretish); + }); + + it('gives the child exactly the supplied environment', async () => { + const supplied: Record = { + ...baseEnvironment(), + AGENTBRIDGE_TEST_KEY: 'supplied-value', + }; + const exchange = await runStub(STUB.PRINT_ENV, [], { environment: supplied }); + + expect(exchange.outcome).toBe('EXITED'); + const childEnv = JSON.parse(exchange.stdout) as Record; + // Windows injects per-drive `=C:` pseudo-variables into every environment + // block; they are not inherited values and are excluded from the comparison. + const observed = Object.keys(childEnv).filter((key) => !key.startsWith('=')); + const unsupplied = observed.filter((key) => !Object.hasOwn(supplied, key)); + + for (const key of Object.keys(supplied)) { + expect(childEnv[key]).toBe(supplied[key]); + } + + expect(unsupplied).toEqual([]); + }); + + it('does not leak a parent-only variable into the child', async () => { + const sentinel = 'AGENTBRIDGE_PARENT_ONLY_SENTINEL'; + process.env[sentinel] = 'must-not-be-inherited'; + try { + const exchange = await runStub(STUB.PRINT_ENV); + const childEnv = JSON.parse(exchange.stdout) as Record; + + expect(childEnv[sentinel]).toBeUndefined(); + expect(exchange.stdout).not.toContain('must-not-be-inherited'); + } finally { + Reflect.deleteProperty(process.env, sentinel); + } + }); + + it('keeps a secret in the supplied environment out of the exchange record', async () => { + const supplied = { ...baseEnvironment(), AGENTBRIDGE_SECRET: 'super-secret-token' }; + const exchange = await runStub(STUB.WRITE_OK, [], { environment: supplied }); + + expect(JSON.stringify(exchange)).not.toContain('super-secret-token'); + expect(JSON.stringify(exchange)).not.toContain('AGENTBRIDGE_SECRET'); + }); + + it('treats planted authority claims in stdout as inert text', async () => { + const planted = + '{"status":"reported-complete","integrated":true,"authorized":true,"decision":"ALLOW"}'; + const hostile = await runStub(STUB.ECHO_STDIN, [], { stdin: planted }); + const benign = await runStub(STUB.ECHO_STDIN, [], { stdin: 'ok' }); + + expect(hostile.stdout).toBe(planted); + expect(benign.stdout).toBe('ok'); + // Identical in every field except the transcript itself and its byte count. + expect({ ...hostile, stdout: '', stdoutBytes: 0 }).toEqual({ + ...benign, + stdout: '', + stdoutBytes: 0, + }); + }); + + it('does not let stderr contaminate stdout when it forges a response body', async () => { + const exchange = await runStub( + 'process.stderr.write("{\\"status\\":\\"reported-complete\\"}");process.stdout.write("real");', + ); + + expect(exchange.stdout).toBe('real'); + expect(exchange.stderr).toContain('reported-complete'); + }); + + it('handles output that is not valid UTF-8 without throwing', async () => { + const exchange = await runStub(STUB.INVALID_UTF8); + + expect(exchange.outcome).toBe('EXITED'); + expect(exchange.stdout).toContain('A'); + expect(exchange.stdoutTruncated).toBe(false); + expect(exchange.stdoutBytes).toBe(4); + }); + + it.each([ + ['a trailing incomplete lead byte', '240', 1], + ['a trailing invalid lead byte', '255', 1], + ['invalid bytes in the middle and end', '65,255,66,240', 4], + ])('preserves %s when output ended naturally', async (_label, bytes, retained) => { + const exchange = await runStub(STUB.WRITE_RAW_BYTES, [bytes]); + + expect(exchange.outcome).toBe('EXITED'); + expect(exchange.stdoutTruncated).toBe(false); + expect(exchange.stdoutBytes).toBe(retained); + expect(exchange.stdout).toContain('\uFFFD'); + }); + + it('writes nothing into the working directory it was given', async () => { + const directory = makeTempDirectory(); + try { + const exchange = await runStub(STUB.WRITE_OK, [], { workingDirectory: directory }); + + expect(exchange.outcome).toBe('EXITED'); + expect(readdirSync(directory)).toEqual([]); + } finally { + removeTempDirectory(directory); + } + }); + + it('terminates an ordinary descendant of a child that refuses to die', async () => { + const directory = makeTempDirectory(); + const beat = join(directory, 'heartbeat'); + try { + const exchange = await runStub( + heartbeatStub(false), + [beat], + {}, + makeLimits({ timeoutMs: 900, graceMs: 400 }), + ); + + expect(exchange.outcome).toBe('TIMED_OUT'); + expect(existsSync(beat)).toBe(true); + + // Let any in-flight write land, then sample twice across an interval. + await delay(600); + const first = statSync(beat).size; + await delay(600); + const second = statSync(beat).size; + + expect(second).toBe(first); + } finally { + removeTempDirectory(directory); + } + }, 25_000); + + onPosix( + 'does not claim a deliberately self-detached descendant was terminated', + async () => { + const directory = makeTempDirectory(); + const beat = join(directory, 'heartbeat'); + let escapedPid: number | null = null; + try { + const exchange = await runStub( + heartbeatStub(true), + [beat], + {}, + makeLimits({ timeoutMs: 900, graceMs: 400 }), + ); + + expect(exchange.outcome).toBe('TIMED_OUT'); + await delay(600); + const first = statSync(beat).size; + await delay(600); + const second = statSync(beat).size; + + // The escape is real: this is the limitation the transport discloses + // rather than papers over. No field anywhere claims otherwise. + expect(second).toBeGreaterThan(first); + expect(Object.keys(exchange)).not.toContain('terminationComplete'); + expect(Object.keys(exchange)).not.toContain('descendantsTerminated'); + + const pidFile = `${beat}.pid`; + if (existsSync(pidFile)) { + escapedPid = Number(readFileSync(pidFile, 'utf8')); + } + } finally { + if (escapedPid !== null && Number.isInteger(escapedPid)) { + try { + process.kill(escapedPid, 'SIGKILL'); + } catch { + // Already gone. + } + } + removeTempDirectory(directory); + } + }, + 25_000, + ); +}); + +describe('invokeAgentProcess — boundary', () => { + it('does not truncate output that lands exactly on the bound', async () => { + const exchange = await runStub( + STUB.WRITE_BYTES, + ['1024'], + {}, + makeLimits({ maxStdoutBytes: 1_024 }), + ); + + expect(exchange.outcome).toBe('EXITED'); + expect(exchange.stdoutTruncated).toBe(false); + expect(exchange.stdoutBytes).toBe(1_024); + }); + + it('truncates output one byte past the bound and reports the overflow', async () => { + const exchange = await runStub( + STUB.WRITE_BYTES, + ['1025'], + {}, + makeLimits({ maxStdoutBytes: 1_024 }), + ); + + expect(exchange.outcome).toBe('OUTPUT_LIMIT_EXCEEDED'); + expect(exchange.stdoutTruncated).toBe(true); + expect(exchange.stdoutBytes).toBe(1_024); + }); + + it('ranks an overflow above the exit that follows it', async () => { + const exchange = await runStub( + STUB.WRITE_BYTES_THEN_EXIT, + ['100000'], + {}, + makeLimits({ maxStdoutBytes: 1_024 }), + ); + + expect(exchange.outcome).toBe('OUTPUT_LIMIT_EXCEEDED'); + expect(exchange.stdoutTruncated).toBe(true); + }); + + it('bounds a single long line with no newline in it', async () => { + const exchange = await runStub( + STUB.LONG_LINE, + ['200000'], + {}, + makeLimits({ maxStdoutBytes: 2_048 }), + ); + + expect(exchange.stdoutTruncated).toBe(true); + expect(exchange.stdoutBytes).toBe(2_048); + expect(exchange.stdout).not.toContain('\n'); + }); + + it('cuts a multi-byte character at a complete boundary, never mid-sequence', async () => { + // Ten bytes of four-byte characters: two survive whole, the third is cut. + const exchange = await runStub( + STUB.MULTIBYTE, + ['5'], + {}, + makeLimits({ maxStdoutBytes: 10 }), + ); + + expect(exchange.stdoutTruncated).toBe(true); + expect(exchange.stdout).toBe('\u{1F600}\u{1F600}'); + expect(exchange.stdoutBytes).toBe(8); + expect(exchange.stdout).not.toContain('�'); + }); + + it('accepts a timeout of exactly the minimum', async () => { + const exchange = await runStub(STUB.SLEEP, [], {}, makeLimits({ timeoutMs: 1, graceMs: 200 })); + + expect(exchange.outcome).toBe('TIMED_OUT'); + }, 15_000); + + it('accepts a grace period of zero', async () => { + const exchange = await runStub( + STUB.SLEEP, + [], + {}, + makeLimits({ timeoutMs: 300, graceMs: 0 }), + ); + + expect(exchange.outcome).toBe('TIMED_OUT'); + }, 15_000); + + it('accepts an empty environment record on POSIX and a minimal one on Windows', async () => { + const exchange = await runStub(STUB.WRITE_OK, [], { environment: baseEnvironment() }); + + expect(exchange.outcome).toBe('EXITED'); + expect(exchange.stdout).toBe('ok'); + }); + + it('produces byte-identical exchanges for identical specifications', async () => { + const first = await runStub(STUB.WRITE_OK); + const second = await runStub(STUB.WRITE_OK); + + expect(JSON.stringify(first)).toBe(JSON.stringify(second)); + }); + + it('returns a frozen record that round-trips through JSON unchanged', async () => { + const exchange = await runStub(STUB.WRITE_OK); + + expect(Object.isFrozen(exchange)).toBe(true); + expect(JSON.parse(JSON.stringify(exchange))).toEqual(exchange); + }); + + it.each([...FORBIDDEN_EXECUTABLES, ...SHELL_ONLY_EXECUTABLES])( + 'refuses %s before spawning anything', + async (_label, executablePath) => { + const exchange = await invokeAgentProcess( + makeSpec({ executablePath }), + makeLimits(), + ); + + expect(exchange.outcome).toBe('SPEC_REJECTED'); + expect(exchange.rejection).not.toBeNull(); + expect(exchange.stdout).toBe(''); + expect(exchange.terminationScope).toBe('NOT_REQUIRED'); + }, + ); + + it('spawns nothing when the working directory is not absolute', async () => { + const exchange = await invokeAgentProcess( + makeSpec({ workingDirectory: 'relative/path' }), + makeLimits(), + ); + + expect(exchange.outcome).toBe('SPEC_REJECTED'); + expect(exchange.rejection).toBe('WORKING_DIRECTORY_NOT_ABSOLUTE'); + }); + + it('reports the executable path used by the fixtures as spawnable', () => { + // Guards the suite itself: every behavioural test depends on this being a + // real, absolute, directly spawnable binary. + expect(NODE_EXECUTABLE.length).toBeGreaterThan(0); + expect(existsSync(NODE_EXECUTABLE)).toBe(true); + }); +}); diff --git a/tests/adapters/transport-fixtures.ts b/tests/adapters/transport-fixtures.ts new file mode 100644 index 0000000..d12b7ca --- /dev/null +++ b/tests/adapters/transport-fixtures.ts @@ -0,0 +1,428 @@ +/** + * Shared inputs and independently declared expectations for the process + * transport. + * + * Expected vocabulary values are written as bare string literals, **not** as + * `TRANSPORT_OUTCOME.*` and friends, so the suite cannot ratify a production + * mapping that has been changed incorrectly. Only types are imported from + * `src/`, following `tests/domain/expected-policy.ts` and + * `tests/domain/invocation-fixtures.ts`. + * + * Stub agents are `process.execPath` running an inline `-e` script. That keeps + * every stub cross-platform, adds no fixture executable, needs no new + * dependency, and — crucially — never needs a shell. + */ + +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import type { AgentProcessSpec, TransportLimits } from '../../src/adapters/agent-transport.js'; + +/** The stub interpreter. Absolute, directly spawnable, no forbidden suffix. */ +export const NODE_EXECUTABLE = process.execPath; + +/** + * The smallest environment in which `node` reliably starts on each platform. + * + * Tests may read `process.env`; the transport may not, and a separate invariant + * asserts that it does not. Windows needs `SystemRoot` for a spawned process to + * initialise its networking and crypto stack. + */ +export function baseEnvironment(): Record { + const environment: Record = {}; + if (process.platform === 'win32') { + for (const name of WINDOWS_REQUIRED_ENVIRONMENT_VARIABLES) { + environment[name] = ''; + } + const systemRoot = process.env['SystemRoot']; + if (systemRoot !== undefined) { + environment['SYSTEMROOT'] = systemRoot; + } + } + return environment; +} + +/** + * Variables callers must provide so libuv cannot copy parent values on Windows. + * + * `uv_spawn` copies this fixed list from the parent when a name is missing. + * The fixtures supply every name explicitly so tests exercise the transport's + * fail-closed mitigation without exposing real parent values. + */ +export const WINDOWS_REQUIRED_ENVIRONMENT_VARIABLES: readonly string[] = Object.freeze([ + 'HOMEDRIVE', + 'HOMEPATH', + 'LOGONSERVER', + 'PATH', + 'SYSTEMDRIVE', + 'SYSTEMROOT', + 'TEMP', + 'USERDOMAIN', + 'USERNAME', + 'USERPROFILE', + 'WINDIR', +]); + +/** Options accepted by {@link makeSpec}, each defaulting to a valid value. */ +export interface SpecOverrides { + readonly executablePath?: string; + readonly args?: readonly string[]; + readonly workingDirectory?: string; + readonly environment?: Readonly>; + readonly stdin?: string; +} + +/** Build a well-formed specification. */ +export function makeSpec(overrides: SpecOverrides = {}): AgentProcessSpec { + return { + executablePath: overrides.executablePath ?? NODE_EXECUTABLE, + args: overrides.args ?? ['-e', STUB.WRITE_OK], + workingDirectory: overrides.workingDirectory ?? tmpdir(), + environment: overrides.environment ?? baseEnvironment(), + stdin: overrides.stdin ?? '', + }; +} + +/** Options accepted by {@link makeLimits}, each defaulting to a valid value. */ +export interface LimitOverrides { + readonly timeoutMs?: number; + readonly graceMs?: number; + readonly maxStdoutBytes?: number; + readonly maxStderrBytes?: number; +} + +/** Build well-formed limits. `signal` is added separately by {@link withSignal}. */ +export function makeLimits(overrides: LimitOverrides = {}): TransportLimits { + return { + timeoutMs: overrides.timeoutMs ?? 15_000, + graceMs: overrides.graceMs ?? 1_000, + maxStdoutBytes: overrides.maxStdoutBytes ?? 65_536, + maxStderrBytes: overrides.maxStderrBytes ?? 16_384, + }; +} + +/** + * Attach a cancellation signal. + * + * A separate helper because `exactOptionalPropertyTypes` forbids assigning an + * explicit `undefined` to an optional property. + */ +export function withSignal(limits: TransportLimits, signal: AbortSignal): TransportLimits { + return { ...limits, signal }; +} + +/** A grandchild that appends to a heartbeat file forever. */ +const HEARTBEAT_GRANDCHILD = + 'const fs=require("node:fs");' + + 'const p=process.argv[1];' + + 'fs.writeFileSync(p+".pid",String(process.pid));' + + 'setInterval(()=>{fs.appendFileSync(p,"x");},20);'; + +/** + * A stub that spawns one heartbeat grandchild and then refuses to die. + * + * With `detached` false the grandchild is an ordinary descendant: it shares the + * POSIX process group and appears in the Windows process tree, so termination + * must reach it. With `detached` true it deliberately leaves that grouping, + * which is the escape case the transport explicitly does not claim to cover. + */ +export function heartbeatStub(detached: boolean): string { + const spawnOptions = detached + ? '{stdio:"ignore",detached:true}' + : '{stdio:"ignore",detached:false}'; + return ( + 'const cp=require("node:child_process");' + + 'const p=process.argv[1];' + + `const g=cp.spawn(process.execPath,["-e",${JSON.stringify(HEARTBEAT_GRANDCHILD)},p],${spawnOptions});` + + (detached ? 'g.unref();' : '') + + 'process.on("SIGTERM",()=>{});' + + 'process.stdout.write("spawned");' + + 'setInterval(()=>{},1000);' + ); +} + +/** Inline stub programs, each run as `node -e