diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index 0a23a980..3e04e138 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -8348,3 +8348,78 @@ Two worked instances the same day. **#74** went green on 2026-07-30 and sat unme **Source:** stuck-CI triage, 2026-08-01. Instance 1 measured across 11 windows-2025 runs; instance 2 reported and diagnosed by the HA-construct-recheck session from PR #129's sqlserver leg. --- +## 339. Sandbox IPC codec: parent executed child-chosen code (ADR 0087 MFW2) + +> ✅ **SHIPPED 2026-08-01 (ADR 0087 MFW2 amendment, `c0d61b94`).** ADR 0087's `mode=subprocess` boundary was **bypassable**: the worker pickled a Handler's raw return into `{"ok": True, "result": ...}` and the **parent** called `pickle.loads` on it, so a Handler returning an object with a custom `__reduce__` executed arbitrary code in the engine process — the one holding the DEK, the audit chain and every live socket. Proven end-to-end. Both legs now use a **closed-tag, non-executing codec** ([`pipeline/_sandbox_codec.py`](../messagefoundry/pipeline/_sandbox_codec.py)): `json.loads` with no object hooks plus a literal tag dispatch over a closed constructor set, so the decode path cannot name a type, import a module, `getattr` on child data, or reach `__reduce__`. A **restricted unpickler was evaluated and disproved** — a `BUILD` opcode over an allowlisted frozen dataclass yields `Send(to=42, message=[])` with `__post_init__` never running. Adversarial review then found a **second, independent break in the correlation defense the codec introduced**: the request id was a per-spawn nonce + counter, disclosed to the child, and only `(id, phase)` was checked — a Handler could pre-stage a forged frame for the next id and have it consumed as an **unrelated** message's result (silent misdelivery to any registered outbound, no `ERROR`, no disposition anomaly, deterministic). Closed three ways: a fresh `secrets.token_hex(16)` per dispatch, binding of the whole `(id, phase, name)` triple, and an unsolicited-frame check fatal to the worker. `mode=off` stays default and byte-identical. Suppression hygiene: the `nosec B403` and the repo's **only** `nosemgrep` are deleted, not reworded — zero `pickle`/`marshal` remains in `messagefoundry/` or `tee/`. + +**Cluster:** Security & Compliance. **Priority:** P1. **Verdict:** shipped. **Severity:** high (blast radius: full bypass of the isolation boundary), low (likelihood: requires Handler-authoring rights, i.e. an admin — but distrusting admin code is the sandbox's entire purpose). + +**Relationship to #197:** #197 shipped the boundary; this fixes the transport that made it bypassable. It does **not** widen ASVS 15.2.5 — it restores the address-space half to what #197 claimed. Confinement is still address-space only. + +**Net effect on correctness:** measured against a HEAD baseline on a 41-check `mode=off` vs `mode=subprocess` parity sweep, the codec is a **win** — HEAD had 10 divergences, MFW2 has 3, two of which reproduce identically at HEAD. It also fixes `mode=subprocess` being outright **DOA for ADR 0013 loopback re-ingress** (`CapturedResponse` lived in the forbidden `messagefoundry.store`, so `pickle.loads` killed the child). + +**Residuals (stated in ADR 0087, not left implicit):** ~1.2–1.4× marshalling cost on a graph with a large reference table (~162 msg/s per lane at 20k entries — inside the existing ~60 msg/s per-interface end-to-end bound, so not the binding constraint); a compromised worker can still deny its **own** feed via a wrong-guess forged frame or a grandchild on the inherited fd 1 (fail-closed, kill+respawn); a contract-violating **mutating Router** diverges between modes (pre-existing at HEAD, pinned by a test that asserts the inequality on purpose so a future change cannot silently close it without deleting the residual); a Handler's exception reaches the operator wrapped in `SandboxError` (same disposition, `last_error` text only; pre-existing). + +**OPEN — owner decisions, deliberately not taken here:** + +1. **Erratum / advisory.** The shipped `mode=subprocess` did not deliver the boundary it advertised, for anyone who opted in. Whether that warrants a security advisory or release note is a disclosure call, not an engineering one. +2. **`docs/adr/README.md:121` still reads "Closes the WP-L3-17 residual (residual-closure)"** while `:175` (ADR 0144) says "15.2.5 stays **Partial**". These contradict, and :175 is the correct position — confinement is address-space only until [ADR 0147](adr/0147-hardened-runtime-isolation-for-router-handler-code-ipc-brokered-sandbox-extends-adr-0087.md) lands. **The edit was NOT made: a live session (`claude/adr-asvs-scorecard-as-data`) is concurrently rewriting that file's ASVS claims**, and the worktree guard blocked the write rather than risk losing its work. It must be made by whoever lands that branch. The same closure claim appears in **#197's banner above** and was likewise left alone. +3. **Private-vault doc pass.** `docs/security/THREAT-MODEL.md` 15.1.5 row, `ASVS-L3-REMEDIATION-PLAN.md` WP-L3-17, and theme 6 of `ASVS-L3-RISK-ACCEPTANCE-REGISTER.md` all describe the pre-MFW2 boundary. `tests/test_threat_model_doc_drift.py` had **pinned the literal token `pickle`** in that row — which after this change would have *required* the doc to assert a mechanism the code no longer has — so the anchor moved to `_sandbox_codec`. Those 89 drift tests skip in every checkout (the directory is gitignored with zero tracked files), so **CI cannot catch this drift in either direction**; the vault edit is a manual, coupled follow-up. + +**Not fixed here, deliberately (each wants its own item):** process-group kill (a grandchild inherits fd 1 and outlives `proc.kill()`); the unframed inherited stderr; and the latent tuple/set-of-`Send`s accept-and-drop, which this codec **preserves on purpose** (it describes such an item as `{"o": "other"}` and rebuilds an inert `Ignored()` so `_partition` stays the sole filter) rather than changing routing behaviour inside a security fix. + +**Related:** #197 (the boundary), ADR 0087 (amended in place — AC-8..AC-15, no new number), ADR 0147 (its broker now explicitly rides this codec's closed grammar), ADR 0104 (AC-4 re-worded off pickle; the copy-on-Send guarantee is strengthened — `Send` carries encoded text, so the parent's rebuild is a provable no-op), ADR 0144 (the static half of the same 15.2.5 defense). + +**Source:** evaluation of an unscoped "recheck the pickle sandbox" suggestion, 2026-08-01; defect confirmed by proof-of-concept before any code was written. + +--- + +## 341. Handler returning a tuple or set of Sends delivers nothing, silently + +> 🚧 **Status OPEN (filed 2026-08-01).** `_partition` ([pipeline/dryrun.py:112](../messagefoundry/pipeline/dryrun.py)) narrows with `items = result if isinstance(result, list) else [result]`. A **`list`** of `Send`s works; a **tuple** or **set** does not — the container itself becomes the single item, matches none of the three `isinstance` filters, and yields `([], [], [])`. **Verified live:** `_partition((send, send))[0] == []`. The Handler ran, returned deliveries, and **nothing is delivered and nothing errors** — the message finalizes `FILTERED` (every handler ran but delivered nothing), which is indistinguishable from a handler deliberately declining it. That is an **accept-and-drop**, the one thing CLAUDE.md §12 forbids outright. + +**Cluster:** Correctness / data loss. **Priority:** P1. **Verdict:** build (small). **Severity:** high (silent PHI non-delivery, no operator signal), medium (likelihood: `return (Send(...), Send(...))` is a natural idiom and a one-character difference from the working form). + +**Why it is not merely cosmetic:** the disposition is not `ERROR` and not `UNROUTED` — it is `FILTERED`, a *legitimate* outcome. So the count-and-log invariant is satisfied on paper (the message is counted and logged) while the operator is told the handler chose to drop it. Nothing in the store, the console, or an alert distinguishes this from intent. + +**Fix direction (not yet decided):** accept any non-`str` iterable in `_partition`, **or** fail loud on a non-`list` container. Failing loud is probably right — silently accepting a tuple widens the contract, whereas a `ValueError` routes to `ERROR`/dead-letter and tells the author exactly what happened. Whichever is chosen, `SetState`/`SetMeta` must behave identically, and `HandlerFn`'s type hint should be widened or tightened to match so mypy catches it at authoring time. + +**Pre-existing, not introduced by #339.** ADR 0087's MFW2 codec **deliberately preserves** this behaviour rather than changing routing semantics inside a security fix: it describes such an item as `{"o": "other"}` and rebuilds an inert `Ignored()`, so `_partition` stays the sole filter and `mode=off` / `mode=subprocess` agree. Fixing `_partition` fixes both modes at once. + +**Related:** #339 (found during its adversarial review), ADR 0005 / ADR 0081 (`SetState` / `SetMeta`), CLAUDE.md §12. + +**Source:** adversarial review of the ADR 0087 sandbox codec, 2026-08-01; confirmed by direct execution of `_partition`. + +--- + +## 342. Sandbox worker kill does not reap a grandchild holding the response pipe + +> 🚧 **Status OPEN (filed 2026-08-01).** `SandboxSession._kill` ([pipeline/sandbox.py:323](../messagefoundry/pipeline/sandbox.py)) calls `proc.kill()`, which terminates **only the direct worker child**. Admin-authored Handler code running in that child can spawn a grandchild, which **inherits fd 1 — the response pipe** — and survives the kill. It can then write frames onto a pipe the parent believes belongs to a freshly-spawned worker, and it leaks as an orphan process for the engine's lifetime. + +**Cluster:** Security & Compliance. **Priority:** P2. **Verdict:** build (small). **Severity:** medium, low (likelihood: requires Handler-authoring rights, i.e. the same admin threat model as #339). + +**Bounded by the #339 correlation fix, not closed by it:** a grandchild cannot make the parent accept a *forged answer* — the per-dispatch `secrets.token_hex(16)` id is unguessable and the unsolicited-frame check is fatal to the worker. So the residual is **availability and process hygiene**, not misdelivery: the orphan can force repeated kill+respawn cycles on its own feed (each dead-lettering the message in hand, fail-closed) and accumulate leaked processes. + +**Fix direction:** spawn into a job object on Windows (`CREATE_NEW_PROCESS_GROUP` + a kill-on-close job) and a process group on POSIX (`start_new_session=True`, then `killpg`), so the whole tree dies with the worker. Note the platform asymmetry is the same one ADR 0147 already documents for confinement, so the two should be designed together rather than twice. + +**Related:** #339, ADR 0087 (residual now stated there), ADR 0147 (OS-level confinement — the natural home for the job-object work), #343 (the sibling fd-2 issue). + +**Source:** adversarial review of the ADR 0087 sandbox codec, 2026-08-01. + +--- + +## 343. Sandbox child stderr is inherited unframed into the engine log stream + +> 🚧 **Status OPEN (filed 2026-08-01).** The worker is spawned with `stderr=None` ([pipeline/sandbox.py:266](../messagefoundry/pipeline/sandbox.py)), so the child's stderr is the **engine's own stderr**, unframed and unattributed. fd 1 is the IPC channel and is strictly framed; fd 2 has no such discipline. Admin-authored Handler code can therefore write arbitrary bytes straight into the engine's log stream — including forged log lines, ANSI control sequences, or content that breaks whatever consumes those logs (NSSM captures stdout/stderr to files; see [docs/SERVICE.md](SERVICE.md)). + +**Cluster:** Security & Compliance. **Priority:** P3. **Verdict:** build (small). **Severity:** medium (log forgery / audit confusion), low (likelihood — same admin threat model). + +**Two distinct problems, worth separating when fixed:** (a) **attribution** — a line from a sandboxed Handler is indistinguishable from an engine line, so an operator cannot tell which inbound produced it; (b) **PHI** — a Handler that `print()`s a message body to stderr writes a full payload into the general log at whatever level the operator is running, which CLAUDE.md §9 forbids for INFO and above. (b) is the one that matters for a PHI deployment. + +**Fix direction:** capture the child's stderr (`stderr=subprocess.PIPE`) and relay it through the engine's stdlib logger on a reader thread, prefixed with the inbound + worker identity and rate-limited. That also removes the interleaving hazard of two processes writing one fd concurrently. + +**Adjacent, same fd-discipline root:** a Handler that `print()`s to **stdout** is a latent landmine in both the pre- and post-#339 code — it lands in the `TextIOWrapper` buffer rather than the `BufferedWriter` the frames use, so it happens not to corrupt a frame today. That is luck, not design, and should be closed with this item (redirect the child's `sys.stdout` to stderr at bootstrap, leaving the raw fd 1 exclusively for frames). + +**Related:** #339, #342 (sibling fd-1 issue), CLAUDE.md §9 (PHI logging), ADR 0087. + +**Source:** adversarial review of the ADR 0087 sandbox codec, 2026-08-01. diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 81608b70..9108c60a 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -797,9 +797,42 @@ Router/Handler in a **persistent per-inbound worker child** (never a per-message forbidden-import guard (socket/store/crypto), the resource caps below, and a **fail-closed** refusal of the live `db_lookup`/`fhir_lookup` bridges (they re-enter the event loop — a subprocess boundary breaks that; a Handler needing live enrichment runs with `mode=off`). An isolation denial (forbidden op, cap -overrun, worker crash) routes the message to `ERROR`/dead-letter **post-ACK** (no NAK, never dropped). +overrun, worker crash, a rejected frame) routes the message to `ERROR`/dead-letter **post-ACK** (no NAK, +never dropped). **Read once at engine start — a `/config/reload` does NOT re-read it (restart to change).** +**The pipe itself is a control, and it is not configurable.** Both directions speak a **non-executing +frame codec**: a closed tag set decoded with `json.loads` + `bytes.decode` and a literal tag match over a +fixed handful of types. Nothing is serialized by naming a type, so nothing a worker writes can construct +an arbitrary object — or run arbitrary code — in the engine. Each dispatch carries a fresh random +request id that a response must echo along with its phase and handler name, *and* a frame that turns up +outside a dispatch drops the worker — so a worker cannot pre-stage the answer to a later call. Fixed +bounds: **64 MiB** per frame and per frame header, **65536** out-of-band segments, and value nesting +depth **256**; each is a fail-closed rejection, not a truncation. The graph's code-set tables are sent +by the engine once per worker start, so a sandboxed `code_set(...)` always resolves to the value the +engine itself is serving — an unreloaded edit to `codesets/` changes nothing until you reload, exactly +as with `mode=off`. + +**What this boundary does NOT cover.** It confines the *address space*, not the machine: a sandboxed +Router/Handler can still import `os`/`subprocess`/`ctypes`, open files, and spawn processes as the +service account — the forbidden-import guard is defence-in-depth (a module imported before it is +installed keeps a live reference), never a compensating control. All of one inbound's Routers/Handlers +share a single worker, so this does **not** confine one Handler from another — the boundary is between +admin code and the engine, exactly as `mode=off` shares an address space. A grandchild the Handler +spawns inherits the response pipe and outlives the worker's kill; the codec plus the request-answer +binding is what makes that harmless (it can still force a respawn, i.e. dead-letter messages on that +inbound), not the process teardown. The child's **stderr is inherited by the engine**, so a +Handler that prints goes into the engine's log unparsed and un-redacted. ADR 0072 Router/Handler +tracing does not compose with `mode=subprocess` (the sandbox branch precedes the tracer branch), and a +`mode=subprocess` graph cannot use the ADR-0071 fused thread-hop path (it is hard-disabled). + +**Check this before you flip the switch.** Turning the sandbox on is behaviour-neutral for a graph +whose Routers are pure — which is what the engine requires of them anyway. It is **not** neutral for a +Router that *mutates* the message it was handed: in-process that mutation is visible to an `accepts=` +predicate (they share one object), and across the pipe it is not, so such a graph can route +differently under `mode=subprocess`. Grep your Routers for writes to the message before enabling this +on a live feed; a Router that only reads is unaffected. + | Key | Type | Default | Notes | |---|---|---|---| | `mode` | enum | `off` | `off` (in-process, byte-identical, no subprocess) or `subprocess` (persistent per-inbound worker child). | diff --git a/docs/adr/0087-sandbox-subprocess-isolation.md b/docs/adr/0087-sandbox-subprocess-isolation.md index cbccc4b6..f691f23c 100644 --- a/docs/adr/0087-sandbox-subprocess-isolation.md +++ b/docs/adr/0087-sandbox-subprocess-isolation.md @@ -48,8 +48,12 @@ in-process, byte-identically and with zero overhead. - **Persistent per-inbound worker, never a per-message fork.** The child is spawned lazily on first dispatch, reused across messages, and reaped at `stop()`. It loads **its own** `Registry` from the same `config_dir` (the unchanged safe-source loader) and looks the Router/Handler up **by name** — - the "fn-selector"; the parent marshals `(phase, name, payload, run_context)` over a length-prefixed - pickle pipe and gets back the **raw** return value. + the "fn-selector"; the parent marshals `(id, phase, name, payload, run_context)` over a + length-prefixed **non-executing** pipe codec and gets back a *description* of the return value. +- **The IPC codec is part of the boundary, not plumbing (amended — see "IPC codec" below).** Both + legs speak **MFW2** (`pipeline/_sandbox_codec.py`): a segmented, closed-tag JSON wire whose decode + path is `json.loads` + `bytes.decode` and a literal tag match. Nothing is pickled in either + direction, in either process. - **The boundary is the win.** The child constructs only the message *graph* — never the store, DEK, crypto, or sockets — so admin code physically cannot reach the parent's secrets/audit chain across the process boundary. Defence-in-depth on top: a **forbidden-import guard** (a `sys.meta_path` @@ -57,16 +61,22 @@ in-process, byte-identically and with zero overhead. purged so a cached import re-triggers it), a **parent-enforced wall-clock cap** (the authoritative bound on every platform — the parent kills a worker that overruns it) plus a POSIX `RLIMIT_CPU`/`RLIMIT_AS` backstop inside the child where `resource` exists (a no-op on Windows). + The import guard is **defence-in-depth only** and must never be cited as a compensating control: a + module imported before the finder goes up keeps a live reference (`urllib.request.socket` is the + real socket module inside a sandboxed Handler), so the address-space boundary and the codec are the + load-bearing controls. - **Interposition at the `route_only`/`transform_one` seam.** A `sandbox`/`run_context` pair threads through those two functions; `sandbox=None`/`mode=off` is the existing in-process line verbatim (so it **composes** with the ADR 0072 `tracer`). The live `wiring_runner` dispatch sites build the per-phase `RunContext` on the loop (as today) and pass it — `loop.run_in_executor`/`to_thread` do not copy contextvars across a process, so the RunContext is **re-marshalled** and the child re-establishes `run_contexts(rc, phase)` itself. -- **Engine-side validation stays engine-side.** The worker returns only the raw Router/Handler - result; the fail-closed unknown-handler / unknown-outbound validation in `route_only`/ - `transform_one` runs in the **parent**, so a compromised worker cannot smuggle an unknown - destination past the graph. +- **Engine-side validation stays engine-side.** The worker describes only the *shape* of the + Router/Handler result; the fail-closed unknown-handler / unknown-outbound validation in + `route_only`/`transform_one` runs in the **parent**, so a compromised worker cannot smuggle an + unknown destination past the graph. That claim is only load-bearing because the codec builds + nothing but plain data *before* that validation runs — under the original pickle pipe the name + validation ran long after `pickle.loads` had already executed whatever the child sent. - **`db_lookup`/`fhir_lookup` in the sandbox = FORBIDDEN, fail-closed (this PR).** They bridge back onto the engine event loop via `run_coroutine_threadsafe`, which a subprocess boundary breaks. A sandboxed Handler that calls one gets a clear `SandboxError` → `ERROR`/dead-letter. A Handler that @@ -82,6 +92,92 @@ in-process, byte-identically and with zero overhead. chicken-and-egg (the worker itself must load the graph) and out of scope. `_assert_safe_config_source` is **not weakened**. +### IPC codec (amendment — MFW2, replaces the pickle pipe) + +The original decision marshalled both directions with `pickle`, justified as "a private pipe between +the engine and its own spawned worker — never external/untrusted data". **That premise was false in +the child→parent direction**, and CLAUDE.md §11 forbids a compensating control resting on a false +premise. Two facts make it false: + +1. The child runs exactly the code this ADR exists to distrust. A Handler returning an object with a + custom `__reduce__` executed arbitrary code **in the engine parent**, because the parent's reader + thread called `pickle.loads` on the response frame *before* any envelope inspection. That is a + complete bypass of the address-space boundary, inside this ADR's own threat model. +2. `subprocess`/`os`/`ctypes` are importable inside the sandbox, and a **grandchild** the Handler + spawns inherits fd 1 (the response pipe) and survives `proc.kill()`. It can write a frame at any + later moment. + +So **both legs are untrusted by contract** and both speak the same schema — one review, one fuzz +target, no `pickle` import left to mis-suppress: + +- **Wire.** Outer `>I length || body` (64 MiB cap, unchanged). The body is *segmented*: + `>I header_len || header_json || ( >I blob_len || blob_bytes )*`, so a large message payload never + round-trips through JSON string escaping. Bounds: header ≤ 64 MiB, ≤ 65536 segments, value nesting + ≤ 256 — each an independent fail-closed rejection. The header bound is deliberately the frame bound + and nothing tighter: a request header carries the whole `reference_view`, so a 16 MiB header cap was + in practice a ~700k-entry ceiling on reference tables that dead-lettered **every** message on a + bigger graph while `mode=off` served it, and the outer framing already refuses an over-cap frame + before a byte is parsed. A fail-closed cap must be sized against real traffic or it is an outage. +- **Closed grammar.** Containers are tagged (`{"l":…}` list, `{"p":…}` tuple, `{"d":[[k,v],…]}` dict, + `{"b":…}` bytes, `{"f":…}` non-finite float, `{"D"/"E"/"T":…}` datetime/date/time), so a bare array + is illegal at value level and nothing needs escaping. Decoding is `json.loads` + `bytes.decode` + + a literal tag match calling ordinary constructors — it cannot name a type, import a module, or + reach `__reduce__`. An ADR 0028 `mfb64:v1:` body is already an ASCII `str` and rides as one; it is + explicitly **not** re-wrapped through the bytes tag. +- **Segment discipline.** References are strictly sequential and exactly-once (a cursor; decode + succeeds only if every segment was consumed), so the one silent failure mode of an indirection + table — cross-wiring one `Send`'s PHI body onto another `Send`'s destination — is unreachable by + construction, not merely untested. +- **A frame answers a request, or the worker dies.** Every dispatch mints a fresh + `secrets.token_hex(16)` id and a response must echo the whole `(id, phase, name)` triple; *and* the + parent treats a frame queued **before** a dispatch — or left over **after** its answer — as fatal. + Both halves are needed, and a first attempt at this shipped only the weaker one: a derivable id + (a per-spawn nonce plus a counter, both visible to the child) let the code running dispatch *N* + compute *N+1*'s id and pre-stage its answer, so a benign sibling Handler's delivery was silently + replaced by attacker-chosen content on a message it never saw — for `phase="accepts"`, a + routing-verdict flip with no `ERROR` and no disposition anomaly. A random per-dispatch id is not + derivable, and the worker only learns it from the request frame, which the parent writes only after + proving nothing is already queued. A **dead peer** is deliberately not the same event: EOF is a + parent-private sentinel with no wire form, so a worker cannot manufacture one and it proves nothing + about trust. It reaps the worker without failing a call that already holds a correlation-proven + answer — treating it as a forgery would fail closed on a signal that carries no information. + **What this is not.** It does not confine one Handler from another *inside* a worker: code running a + dispatch is handed that dispatch's id, and it could equally rebind a sibling in the child's own + registry. `mode=off` draws no such line either — see the residuals. +- **Result parity.** The child materialises a router result with `_handler_names`' own logic (so a + documented-supported **generator Router**, which is unpicklable, now works under `mode=subprocess`) + and reproduces `_partition`'s *exact* input container for a transform result — including describing + an item `_partition` would ignore rather than omitting it, so a tuple/set/int return still drops and + a `Send` **subclass** still delivers, byte-identically to `mode=off`. +- **`Send` carries encoded text, never a live `Message`.** The sole parent-side consumer already + reduces it to a `str`, so the parent's `Send(...)` rebuild is a provable no-op for ADR 0104's + copy-on-Send choke point instead of taking a second snapshot. +- **`code_sets` are hoisted off the per-dispatch frame** (they were ~430 KB / ~4.6 ms per message on a + realistic crosswalk) and travel **once per spawn in the `boot` frame** instead. The engine's tables + are the source of truth on both sides, so the child cannot diverge from `mode=off`. A first attempt + let the child re-read `codesets/` itself and pinned a SHA-256 digest to *detect* the divergence — + which converted a fail-open hazard into a worse fail-closed one: after any routine respawn (a + wall-cap kill, a crash) following an unreloaded `codesets/` edit, every message on that inbound + dead-lettered permanently, burning a full config load per attempt. Sending the tables removes the + hazard rather than detecting it. +- **A lookup table travels in one of two forms, and they decode identically.** A crosswalk is + overwhelmingly `str -> short str`; those tables ride as a single plain JSON object so the C + encoder/decoder does the walk, and anything else falls back to the tagged per-entry form. The + compact form is an *encoding*, not a grammar relaxation — the decoder still proves every value is a + `str`. Without it the per-entry Python walk over `reference_view` made `mode=subprocess` ~5× + slower per message than the pickle it replaced on a 20k-entry table. With it, that table costs + ~1.4× the pickle round-trip (4.5 ms vs 3.3 ms of marshalling; ~6.2 ms end-to-end per dispatch, + ~0.19 ms with no reference view) — the standing, measured price of a non-executing wire, and well + inside the ~60 msg/s per-interface end-to-end bound the pipeline already has. +- **`CapturedResponse` relocated** to the store-free `config/response.py` (re-exported from + `store/store.py`). It is what `response_view` carries, and `messagefoundry.store` is on the + forbidden-import list — so `mode=subprocess` plus a LOOPBACK inbound with a correlated reply was + **100% non-functional** before this amendment. +- **Framing vs decoding are separated.** The parent's daemon reader thread does framing only; a + rejection there would have been a silent reader death and a wall-cap *hang*, not a fail-closed + error. Decoding happens on the dispatch thread inside its existing `try`. `SandboxCodecError` + subclasses `SandboxError`, so the documented "raises `SandboxError`" contract still holds. + ## Acceptance Criteria - **AC-1** — WHERE `[sandbox].mode=off` (the default), THE SYSTEM SHALL run a Router and a Handler @@ -104,11 +200,64 @@ in-process, byte-identically and with zero overhead. → `tests/test_sandbox.py::test_run_context_reaches_the_worker` - **AC-7** — WHERE `[sandbox].mode=subprocess` and the engine passes its **real** `RunContext` (the store's live `MappingProxyType` `reference_view`/`state_view`), THE SYSTEM SHALL snapshot those - views to picklable dicts and process the message (route + deliver) rather than fail marshalling — + views to marshallable dicts and process the message (route + deliver) rather than fail marshalling — i.e. the control processes real traffic against the default SQLite store, not just an empty `RunContext`. → `tests/test_sandbox.py::test_subprocess_marshals_live_store_run_context`, - `tests/test_sandbox.py::test_picklable_run_context_snapshots_mappingproxy_views` + `tests/test_sandbox.py::test_run_context_codec_snapshots_mappingproxy_views` + +### Acceptance Criteria — IPC codec amendment (MFW2) + +- **AC-8** — IF a sandboxed Handler returns an object carrying a hostile `__reduce__`, THEN THE + SYSTEM SHALL NOT execute it in the engine parent, and SHALL resolve the message byte-identically to + `mode=off`. + → `tests/test_sandbox.py::test_a_handler_returning_a_reduce_gadget_does_not_execute_in_the_engine`, + `tests/test_sandbox_codec.py::test_a_reduce_gadget_never_reaches_the_parent` +- **AC-9** — WHERE a frame is malformed, over a cap, out of the closed value grammar, or out of + segment sequence, THE SYSTEM SHALL reject it with a `SandboxError` (never a cross-wired value, + never a hang, never a non-`SandboxError` escape). + → `tests/test_sandbox_codec.py::test_hostile_frames_fail_closed`, + `tests/test_sandbox_codec.py::test_blob_reference_discipline_is_exactly_once_and_sequential` +- **AC-10** — WHEN a worker writes an extra/forged response frame — whether *before* its own answer or + staged *between* dispatches — THE SYSTEM SHALL NOT consume it as a later dispatch's answer; it SHALL + drop that worker and fail the call closed, and the later dispatch SHALL resolve identically to + `mode=off`. Request ids SHALL be unpredictable and never reused. A **dead peer** (EOF) is NOT a + frame — it has no wire form, so a worker cannot manufacture one — and SHALL drop the worker without + failing a call that already has a correlation-proven answer. + → `tests/test_sandbox.py::test_a_desynced_or_forged_response_is_rejected`, + `tests/test_sandbox.py::test_a_frame_staged_between_dispatches_can_never_answer_the_next_one`, + `tests/test_sandbox.py::test_a_dead_peer_is_not_treated_as_a_forged_frame`, + `tests/test_sandbox.py::test_request_ids_are_unpredictable_and_never_reused`, + `tests/test_sandbox_codec.py::test_hostile_frames_fail_closed[name_mismatch]` +- **AC-11** — WHERE `[sandbox].mode=subprocess`, THE SYSTEM SHALL route a **generator** Router + identically to `mode=off` (it previously dead-lettered every message), and SHALL preserve + `_partition` parity for every other return shape — a `Send` subclass still delivers, a tuple/set/int + still drops. + → `tests/test_sandbox.py::test_generator_router_routes_under_mode_subprocess`, + `tests/test_sandbox_codec.py::test_partition_parity_table` +- **AC-12** — WHERE the engine publishes code-set tables, THE SYSTEM SHALL serve **those** tables to a + sandboxed Router/Handler — including after a transparent respawn that follows a `codesets/` edit + made without a `/config/reload` — and the per-dispatch frame SHALL carry no code-set bytes. + → `tests/test_sandbox.py::test_an_unreloaded_codeset_edit_does_not_brick_the_inbound`, + `tests/test_sandbox.py::test_the_engines_code_sets_win_over_the_childs_own_load`, + `tests/test_sandbox.py::test_code_sets_reach_the_child_without_travelling_per_dispatch`, + `tests/test_sandbox.py::test_code_sets_are_loaded_by_the_child_and_resolve`, + `tests/test_sandbox_codec.py::test_code_sets_round_trip_through_the_boot_frame` +- **AC-13** — WHERE a sandboxed Handler reads a captured reply (`response_get`), THE SYSTEM SHALL + deliver the same result as `mode=off` (previously the child died on the forbidden + `messagefoundry.store` import, making the feature combination non-functional). + → `tests/test_sandbox.py::test_response_view_reaches_a_sandboxed_handler` +- **AC-14** — WHERE a value crosses the boundary that `mode=off` accepts — a large `reference_view`, + a deeply nested `SetState` value — THE SYSTEM SHALL marshal it rather than dead-letter it, so no + codec bound is a mode-dependent behaviour change. + → `tests/test_sandbox_codec.py::test_the_header_cap_is_the_frame_cap_and_nothing_tighter`, + `tests/test_sandbox_codec.py::test_a_large_reference_view_round_trips`, + `tests/test_sandbox_codec.py::test_a_deeply_nested_value_is_not_a_mode_dependent_dead_letter` +- **AC-15** — WHERE a Router *mutates* its payload (a contract violation — Routers must be pure), THE + SYSTEM's routing decision under `mode=subprocess` MAY differ from `mode=off`, and that difference + SHALL be pinned by a test rather than left to drift. This is the one place `[sandbox].mode` is not + transparent; it is a residual, not a guarantee — see "Out of scope / honest residuals". + → `tests/test_sandbox.py::test_a_mutating_router_is_the_one_documented_mode_divergence` ## Options considered @@ -125,30 +274,82 @@ in-process, byte-identically and with zero overhead. ## Consequences **Positive** — Genuine hard isolation of admin code from the DEK/audit-chain/sockets when enabled; -closes the heaviest WP-L3-17 (15.2.5) residual as a **residual-closure**. Default-off means zero -overhead and byte-identical behaviour for existing deployments; the whole existing test suite is -unaffected (sandbox is `None`/off everywhere). +closes the heaviest WP-L3-17 (15.2.5) residual as a **residual-closure**. That closure is only as +good as the pipe: as originally shipped, the parent's `pickle.loads` of the child's frame let admin +code cross the boundary it claims to enforce, so the MFW2 amendment is what makes the claim true +rather than an enhancement on top of it. Default-off means zero overhead and byte-identical behaviour +for existing deployments; the whole existing test suite is unaffected (sandbox is `None`/off +everywhere). -**Negative / risks** — When enabled, each message pays a pickle round-trip to the worker and the +**Negative / risks** — When enabled, each message pays a codec round-trip to the worker and the per-inbound worker serializes that inbound's Router/Handler calls (matching the per-inbound worker cadence). A Handler needing live enrichment cannot use the sandbox this PR. The engine builds the `RunContext` `reference_view`/`state_view` as live `types.MappingProxyType` windows onto the store -caches, which are **not** picklable; `run_sandboxed` therefore snapshots them to plain point-in-time -`dict`s (`_picklable_run_context`) before the frame crosses the pipe — the read-only content a -router/transform would have seen at that instant (re-run stability makes a point-in-time copy the -contract anyway), so `mode=subprocess` processes real messages against the default SQLite store. -Snapshotting copies the reference/state caches per dispatch, an accepted cost of the opt-in isolation -mode. Any residually-unpicklable view (a future exotic value) still fails closed (`SandboxError`), -never silently degrading. +caches; `enc_run_context` snapshots them to plain point-in-time rows **by construction** as it walks +them onto the wire — the read-only content a router/transform would have seen at that instant (re-run +stability makes a point-in-time copy the contract anyway), so `mode=subprocess` processes real +messages against the default SQLite store. Snapshotting copies the reference/state caches per +dispatch, an accepted cost of the opt-in isolation mode (`code_sets`, the largest of the three, is +hoisted out of the per-dispatch frame entirely). A value outside the closed grammar fails closed +(`SandboxError`), never silently degrading — and the reverse is also true, so a Handler returning an +exotic object now reports a *codec* rejection rather than the pickle error text it used to. **Out of scope / honest residuals** — - **DEK-in-worker:** the child never constructs the store/DEK, so there is no DEK in the worker to strip; if a future change loads store state at registry-build time, that must stay out of the child. +- **The boundary confines the address space, not the machine.** `os`/`subprocess`/`ctypes`/`sqlite3`/ + `http.client` are importable inside the sandbox, so a Handler can still read and write files, open + network connections, and spawn processes **as the service account**. The forbidden-import guard + narrows the obvious paths but is bypassable (a module imported before it goes up keeps a live + reference) and carries no compensating-control weight. Least-privilege for the service account + remains the host control that bounds this. ADR 0147 tracks the fuller closure. +- **A grandchild the Handler spawns inherits fd 1** and outlives `proc.kill()`. It can write frames + into the parent's reader at any later moment. What makes that harmless is the *codec* (nothing it + writes can construct an arbitrary object) plus the request-answer binding (nothing it writes can be + taken as an answer, and its mere presence kills the worker) — not the process teardown. On POSIX, + `/proc//fd/1` is additionally openable by any same-UID process subject to + `ptrace_scope`/`hidepid`. What it **can** still do is force a kill-and-respawn, i.e. dead-letter + messages on that inbound: a compromised worker can deny its own feed. +- **One Handler is not confined from another inside a worker.** All of an inbound's Routers/Handlers + share one child and one `Registry`, so admin code can rebind a sibling in-process. This seam does + not change that — `mode=off` shares an address space too — and any claim that the pipe protects + handler-to-handler integrity is false. The boundary drawn here is between admin code and the + **engine**. Per-Handler confinement would need a worker per Handler. +- **The child's stderr is inherited by the engine** (`stderr=None`), unframed and unparsed: a + sandboxed Handler that prints writes straight into the engine's log. That is a log-injection / + PHI-to-log surface, not a frame surface. +- **ADR 0072 tracing does not compose with `mode=subprocess`.** In `_accepted`/`route_only`/ + `transform_one` the sandbox branch precedes the tracer branch, so a traced dry-run produces no + Router/Handler trace when the sandbox is on. It composes with `mode=off` as stated above. +- **A *mutating* Router is the one place `mode` is not transparent.** Every dispatch marshals the + payload, so the child rebuilds its own object; in-process, one object is *shared*. A Router that + mutates its payload — which CLAUDE.md §2 forbids (routers/transforms must be pure) and + `HandlerAccepts` restates for predicates — therefore reaches an `accepts=` predicate under + `mode=off` and not under `mode=subprocess`, so the two modes can **route differently**; on a non-HL7 + `route_message` (dry-run / `check` / Test Bench, where one writable `RawMessage` is shared with the + handlers too) it can also change the delivered bytes. This predates the codec — the original pickle + pipe copied the payload per dispatch identically — and closing it would mean returning the payload + from every router/predicate dispatch, roughly doubling the wire cost of the routing stage to + reproduce a documented authoring hazard. Pinned by AC-15 so it cannot drift silently. If a graph + needs a Router-derived fact at the predicate, pass it through the message, not through mutation. +- **A Handler's own exception reaches the operator wrapped.** In-process a Handler raise propagates as + itself; under `mode=subprocess` it is reported across the pipe and re-raised as `SandboxError` + carrying `": "`. The **disposition is identical** (both land in the router/transform + worker's `except Exception` → `ERROR`/dead-letter); only the `last_error` text differs, and no + traceback crosses. +- **Run-scoped sinks do not cross back.** The `#162` unmapped-capture buffer drains inside the child + against a sink the child never installs. No production sink exists today, so nothing is lost — but + any future run-scoped sink (capture, metrics, audit) will silently no-op under `mode=subprocess` + until the codec grows a side-channel for it. - **`db_lookup`/`fhir_lookup` forward-over-IPC** — deferred; sandboxed live-enrichment Handlers run with `mode=off`. - **Load-time top-level config exec** — not sandboxed; unchanged `_assert_safe_config_source` gate. -- **Combining `mode=subprocess` with the ADR 0071 B5 SS-only thread-hop fusion** — the fused SS path - is orthogonal/default-off and not wired to the session in this PR; sandbox is honored on the - standard async dispatch. +- **`mode=subprocess` and the ADR 0071 B5 SS-only thread-hop fusion are mutually exclusive, and the + sandbox wins.** The fused twins call `route_only`/`transform_one` with no `sandbox=`, so fusion + would run the Router, the Handler *and* the `accepts=` predicate in the engine process — silently + outside the guard the operator turned on. `_open_fused_pool` therefore **disables fusion** with a + loud warning when both are configured. That is fail-closed, not free: a graph tuned for fusion loses + it the moment the sandbox goes on (an IPC round-trip inside a fused hop would have negated the + fusion anyway). - **Least-privilege service account as the default** — remains environment-delegated (host control). diff --git a/docs/adr/0104-copy-on-send-outbound-message-model-recognition-first-handler-message-type-and-hl7-field-picker.md b/docs/adr/0104-copy-on-send-outbound-message-model-recognition-first-handler-message-type-and-hl7-field-picker.md index 712278da..e44fd710 100644 --- a/docs/adr/0104-copy-on-send-outbound-message-model-recognition-first-handler-message-type-and-hl7-field-picker.md +++ b/docs/adr/0104-copy-on-send-outbound-message-model-recognition-first-handler-message-type-and-hl7-field-picker.md @@ -2,7 +2,7 @@ **Status:** Accepted (2026-07-13) — owner-ratified. Q1 copy-on-Send + Q2 `message_type_of` built (PR #995, BACKLOG #230); Q3 §2.3 Steps picker **shipped** (PR #1001, commit `5b90a695`, authored 2026-07-13 UTC — P1 picker + P2 scoping + P3 round-trip badges); the §2.3 **Step 1 inline-autocomplete extension** is the outstanding remainder *(erratum 2026-07-16: this line previously said "Q3 §2.3 HL7 field picker building" — the picker had shipped)*. The §2.3 adoption gate is satisfied by the owner's Steps-view endorsement; the five §3 spec blockers are closed by the build; the copy-on-Send **default-flip** is now **resolved** (2026-07-14, §8.1): default-**ON**, on genuine copy-on-write. **Deciders:** owner + IDE/DX + engine working group -**Related:** ADR **0076** (typed action vocabulary + Steps lens — the `.py`-only-artifact discipline this inherits), ADR **0089** (recognition-first lens over native `Message` idioms — the Steps surface the picker edits), ADR **0084** (`accepts=` router-stage seam — the enforcement seam Q2 reuses), ADR **0004** (payload-agnostic ingress / `RawMessage` — why Q2 is HL7-only), ADR **0057** (inline Step-A fast-path — the second execution path Q1/Q2 must hold on), ADR **0087** (subprocess sandbox — the pickle boundary `Send.message` must survive), ADR **0072** (traced dry-run — live values beside Steps rows), ADR **0005 / 0081** (`SetState` / `SetMeta` — the declarative-op precedent), ADR **0010 / 0043** (`db_lookup` / `fhir_lookup` — raise in the router phase), CLAUDE.md **§2** (reliability/purity), **§8** (HL7 conventions), **§12** (the #26 bright line), BACKLOG **#26** (declined visual/declarative authoring; the Steps-over-`.py` carve-out). Backing evaluation + competitor comparison: [`docs/research/message-model-eval.md`](../research/message-model-eval.md). +**Related:** ADR **0076** (typed action vocabulary + Steps lens — the `.py`-only-artifact discipline this inherits), ADR **0089** (recognition-first lens over native `Message` idioms — the Steps surface the picker edits), ADR **0084** (`accepts=` router-stage seam — the enforcement seam Q2 reuses), ADR **0004** (payload-agnostic ingress / `RawMessage` — why Q2 is HL7-only), ADR **0057** (inline Step-A fast-path — the second execution path Q1/Q2 must hold on), ADR **0087** (subprocess sandbox — the IPC boundary `Send.message` must survive), ADR **0072** (traced dry-run — live values beside Steps rows), ADR **0005 / 0081** (`SetState` / `SetMeta` — the declarative-op precedent), ADR **0010 / 0043** (`db_lookup` / `fhir_lookup` — raise in the router phase), CLAUDE.md **§2** (reliability/purity), **§8** (HL7 conventions), **§12** (the #26 bright line), BACKLOG **#26** (declined visual/declarative authoring; the Steps-over-`.py` carve-out). Backing evaluation + competitor comparison: [`docs/research/message-model-eval.md`](../research/message-model-eval.md). **Code references** are `origin/main @ a79e14b`; line numbers drift — locate exactly at implementation time. --- @@ -94,7 +94,7 @@ class Message: 2. **Batch / non-MSH leading segment.** A BHS/FHS-led `Message` has no single message type to match. (`field("MSH-9")` is **id-based** — it locates an `MSH` segment by id, not a positional `BHS-9` — so the load-bearing guard is the **leading-segment check** `segments()[0] != "MSH"`, combined with a multi-`MSH` count guard for a bare batch; corrected during the ADR 0104 build.) `message_type_of` must **raise** on both. State the **transport-dependent split contract**: **File-source ingress splits batches** (via [`parsing/split.py`](../../messagefoundry/parsing/split.py) `split_batch`, wired in [`transports/file.py`](../../messagefoundry/transports/file.py) — the "Tier 2.2" batch split; **not** ADR 0082, which is *outbound* batch aggregation), so each split message carries its own MSH-9 and matches normally; **MLLP does not split** (no batch handling in [`transports/mllp.py`](../../messagefoundry/transports/mllp.py)), so a batch over MLLP arrives as one `Message` and surfaces as a loud ERROR against an HL7-type-enforced handler. 3. **Fail loud on non-HL7.** A synthesized `getattr(m,"message_type",None)` returns `None` on `RawMessage` → silent UNROUTED, whereas the hand-written guard raises. `message_type_of` raises on any message lacking MSH-9 (§2.2); `check` warns when a `message_type`-bearing handler is bound to a non-`hl7v2` inbound. 4. **Structural-clone snapshot.** copy-on-Send/`copy()` must be a structural clone satisfying dual-backend `snapshot.encode() == source.encode()` over the escaping / repetition / custom-separator / Z-segment corpus — including a `set()` trailing-whitespace terminal field, an appended trailing-whitespace segment, and a **python-hl7 fallback-produced source** (§2.1). -5. **Execution-path matrix.** The snapshot invariant and the enforcement predicate must hold on **both** the split path (`dryrun.py` `transform_one`) **and** the fused inline fast-path (ADR 0057), and across the **subprocess sandbox** pickle boundary (ADR 0087) — see §4. +5. **Execution-path matrix.** The snapshot invariant and the enforcement predicate must hold on **both** the split path (`dryrun.py` `transform_one`) **and** the fused inline fast-path (ADR 0057), and across the **subprocess sandbox** IPC boundary (ADR 0087) — see §4. ## 4. Purity / re-run, fan-out, and the execution-path matrix @@ -102,7 +102,7 @@ class Message: **The one honest behavior change.** copy-on-Send moves the delivery snapshot from **handoff-time** to **Send-construction-time**. For single-Send handlers and fan-out constructed at `return` with no interleaved post-construction mutation, bytes are **identical**. The only regression surface is a handler that constructs a `Send`, then mutates the *same* `Message` before returning, *relying on* the late mutation reaching that already-constructed Send (today's last-write-collapse-to-all-destinations). That reliance is almost always the bug this fixes, but it is a behavior change → **gated on an AST estate scan** for "construct Send, then mutate its referenced Message before return" before the default flips, and a **throughput benchmark** (copy-on-write keeps the common no-post-mutation path zero-copy). -**Sandbox (ADR 0087).** Under `[sandbox].mode=subprocess` the Handler runs in a child and returns `Send`s over a length-prefixed pickle pipe; `send.message.encode()` runs in the **parent**. Every `Send.message` (and the snapshot) must remain **picklable** — an invariant the build must assert. Cost note: today `[Send("A",msg),Send("B",msg)]` sharing one object pickle-memoizes to **one** serialized message; **N independent snapshots serialize N** — copy-on-write bounds this to divergent-fan-out cases, and the throughput benchmark must measure it (the 45M/day path is latency/CPU-sensitive). This cost is also why the distinct `OutboundMessage` type is rejected: its true ripple is not just "grows the `Send.message` union" — it also touches `_partition` narrowing, the `isinstance(send.message, str) else .encode()` branch, the pickle boundary, and mypy-strict narrowing at every Send/transform site. copy-on-Send incurs none of that. +**Sandbox (ADR 0087).** Under `[sandbox].mode=subprocess` the Handler runs in a child and returns `Send`s over a length-prefixed **non-executing IPC codec** (ADR 0087's MFW2 amendment replaced the original pickle pipe — a Handler's `__reduce__` executed in the engine parent). Every `Send.message` (and the snapshot) must remain **describable by that codec** — an invariant the build must assert. `send.message.encode()` now runs in the **child**, at describe time, and the `Send` carries encoded text; that relative instant is the same one the in-process path evaluates it at, and it makes the parent's `Send(...)` rebuild a provable no-op for this choke point rather than a second snapshot. Cost note: the codec does **not** memoize a shared object the way pickle did, so `[Send("A",msg),Send("B",msg)]` sharing one object already serializes **twice** — N independent snapshots therefore cost no more per-Send than the shared case, and the throughput benchmark measures the whole fan-out either way (the 45M/day path is latency/CPU-sensitive). The distinct `OutboundMessage` type is still rejected on ripple: it touches `_partition` narrowing, the `isinstance(send.message, str) else .encode()` branch, the IPC codec's item grammar, and mypy-strict narrowing at every Send/transform site. copy-on-Send incurs none of that. **Inline fast-path (ADR 0057).** The fused path runs `route_only` + a single handler's `transform_one` inline in the router worker and materializes deliveries there, gated on M-single/M-deliver/no-state/no-passthrough. The snapshot invariant must be asserted on **both** paths. A single handler declined by Q2's `accepts=` predicate yields `names=[]` → the M-single fallback → the split path → UNROUTED; the build walks that sequence. @@ -118,8 +118,9 @@ class Message: → `tests/test_message_copy.py::test_structural_clone_encode_parity_both_backends` - **AC-3** — THE SYSTEM SHALL implement `copy()`/the snapshot as a structural clone of the parsed model, NOT via `Message.parse(self.encode())`, and SHALL preserve the source's parser backend (no built-in↔python-hl7 switch on clone). → `tests/test_message_copy.py::test_clone_preserves_backend` -- **AC-4** — WHEN a fan-out `Send.message` is marshalled across the subprocess-sandbox pipe (ADR 0087), THE SYSTEM SHALL pickle and re-encode it in the parent without error. - → `tests/test_sandbox_fanout.py::test_send_message_picklable_across_pipe` +- **AC-4** — WHEN a fan-out `Send.message` is marshalled across the subprocess-sandbox pipe (ADR 0087), THE SYSTEM SHALL round-trip it through the IPC codec and re-encode it in the parent without error. (Re-worded: the pipe no longer pickles — see ADR 0087's MFW2 amendment.) + → `tests/test_copy_on_send.py::test_snapshotted_send_message_survives_the_sandbox_codec`, + `tests/test_sandbox_codec.py::test_copy_on_send_rebuild_is_a_provable_no_op` - **AC-5** — THE SYSTEM SHALL match `message_type_of(spec)` component-wise on `message_code` + `trigger_event` read through the message's own MSH-2, matching a 3-component `ADT^A01^ADT_A01` and a message whose MSH-2 uses a non-standard component separator. → `tests/test_message_type_of.py::test_component_wise_match_three_component_and_custom_sep` - **AC-6** — IF a message enforced by `accepts=message_type_of(...)` lacks an MSH-9 (a `RawMessage` or a BHS/FHS-led batch), THEN THE SYSTEM SHALL raise → record `ERROR`/dead-letter (never silently decline to UNROUTED, never accept-and-drop). @@ -137,7 +138,7 @@ class Message: ## 6. Options considered -**Q1.** 1. **copy-on-Send structural snapshot + `copy()` sugar** — **CHOSEN** (fixes the whole estate with zero edits; the only signal separating divergence from same-content is mutation-between-Sends, which the snapshot captures). 2. *Opt-in `copy()` + static aliasing lint* — Rejected: 0/1,283 adoption predicts ~nil use; the lint provably false-fires on the correct archive+downstream idiom and misses helper/loop-laundered aliasing. 3. *Distinct `OutboundMessage` type* — Rejected: rewrites the estate, and ripples through `_partition`, the `.encode()` branch, the pickle boundary, and mypy-strict narrowing. 4. *`def handle(inbound, outbound)` / hard read-only inbound* — Rejected: breaks all 1,283 in-place-mutate sites; strict immutability is an ergonomics choice, not verified parity table-stakes. 5. *`parse(encode())` clone* — Rejected: silent backend switch on a fallback-parsed source. +**Q1.** 1. **copy-on-Send structural snapshot + `copy()` sugar** — **CHOSEN** (fixes the whole estate with zero edits; the only signal separating divergence from same-content is mutation-between-Sends, which the snapshot captures). 2. *Opt-in `copy()` + static aliasing lint* — Rejected: 0/1,283 adoption predicts ~nil use; the lint provably false-fires on the correct archive+downstream idiom and misses helper/loop-laundered aliasing. 3. *Distinct `OutboundMessage` type* — Rejected: rewrites the estate, and ripples through `_partition`, the `.encode()` branch, the IPC boundary, and mypy-strict narrowing. 4. *`def handle(inbound, outbound)` / hard read-only inbound* — Rejected: breaks all 1,283 in-place-mutate sites; strict immutability is an ergonomics choice, not verified parity table-stakes. 5. *`parse(encode())` clone* — Rejected: silent backend switch on a fallback-parsed source. **Q2.** 1. **Recognition-first descriptive type + `accepts=message_type_of(...)` enforcement** — **CHOSEN.** 2. *Engine-interpreted `@handler(message_type=…, enforce=True)`* — Rejected: compiling a decorator string into routing behavior is a second declarative execution surface (ADR 0076 bright line); whole-field matching silently UNROUTES 3-component types. 3. *Required declaration* — Rejected: `RawMessage` has no MSH-9 (ADR 0004). diff --git a/docs/adr/0147-hardened-runtime-isolation-for-router-handler-code-ipc-brokered-sandbox-extends-adr-0087.md b/docs/adr/0147-hardened-runtime-isolation-for-router-handler-code-ipc-brokered-sandbox-extends-adr-0087.md index 66ba4870..b03f509f 100644 --- a/docs/adr/0147-hardened-runtime-isolation-for-router-handler-code-ipc-brokered-sandbox-extends-adr-0087.md +++ b/docs/adr/0147-hardened-runtime-isolation-for-router-handler-code-ipc-brokered-sandbox-extends-adr-0087.md @@ -46,6 +46,22 @@ questions this ADR turns on: lookups through a parent-held IPC broker and (2) confines the worker at the OS level (default-deny egress/filesystem/imports), per platform.** Proposed (design only — no code in this lane). +> **Prerequisite (added 2026-08-01, BACKLOG #339).** This ADR was written while the ADR 0087 pipe still +> **pickled**, and its Context below therefore understates the residual it was addressing: the pipe had a +> third limit — the parent called `pickle.loads` on frames the *child* wrote, so a Handler's `__reduce__` +> executed in the engine parent and the address-space boundary was bypassable outright. That is fixed +> (a closed-tag, non-executing codec on both legs, `pipeline/_sandbox_codec.py`), which is what makes the +> two limits below the *remaining* ones rather than the only ones. +> +> **This is load-bearing for the broker, not incidental.** The broker's direction of travel is *child +> requests → parent acts*, which is precisely the direction that must never be able to name a callable. +> A broker built on the old pickled pipe would have re-opened the hole it is meant to help close. So +> **the brokered request/response MUST ride the existing closed grammar** — a new frame type in the +> closed tag set, with the same request/answer binding the codec already enforces (fresh per-dispatch +> `secrets` id, and the full `(id, phase, name)` triple checked before any payload is interpreted) — and +> **not** an ad-hoc serializer beside it. "The broker adds a typed IPC protocol surface to secure" under +> *Negative / risks* composes with this; the dependency is now explicit rather than implied. + - **(1) IPC request-broker for `db_lookup`/`fhir_lookup`.** The trusted **parent** holds the DB/HTTP capability. A sandboxed Handler's `db_lookup`/`fhir_lookup` call, instead of bridging to the loop in-process, marshals a **typed request** over the existing ADR 0087 length-prefixed pipe; the **parent** diff --git a/docs/adr/README.md b/docs/adr/README.md index fd25ef93..3ae66028 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -118,7 +118,7 @@ what is withheld and what you can request. | [0083](0083-mtls-client-certificate-identity.md) | mTLS client-certificate identity (BACKLOG #200) — a **verified** peer cert's subject/SAN maps to a MessageFoundry principal via an explicit deny-by-default allow-list (`[api].tls_client_cert_identities`), rooted in `CERT_REQUIRED` and namespace-qualified against spoofing. An **attested service-to-service** identity (no MFA/session/step-up). **Activated** (PLAN-9 Wave 3): a fork-free scope-populating shim (`api/tls_client_cert.py`) surfaces the verified peer cert post-handshake, and a fenced cert-only dependency `require_service_cert` gates one non-interactive route (`GET /service/identity`) — never a bearer/PHI/step-up route (refuses PHI-view perms at build), so a cert-identity can never bypass step-up | Accepted (2026-07-10) — owner ratified; model+resolver built (#200), **activated** PLAN-9 Wave 3 | | [0084](0084-accepts-router-seam.md) | `accepts=` Router-stage seam — let a Handler declare a **pure** router-time applicability predicate (`Callable[[Message \| RawMessage], bool]`) so the Router declines it **before** a routed row is materialized, recovering the `2` transactions per self-filtering handler the ADR 0051 `txn/msg = 3 + 2H + 2N` model charges (the ADT hub's `wasted == 32`, ~63% of its durable writes). Purity enforced by construction (router stage already makes `db_lookup`/`fhir_lookup` raise — ADR 0010/0043); additive + default-identical (`accepts=None` = today). **Crux:** an all-declined message finalizes **`UNROUTED`** not `FILTERED` — count-and-log intact (still `RECEIVED` at ingress, still a final logged disposition, never accept-and-drop), only per-destination FILTERED granularity lost (optional `message_events` declined-handler mitigation). Ships an **advisory** `accepts-candidate` lint (flags a `@handler` opening with a guard-filter `if …: return []`). Spec + lint stub only, no engine build (build = BACKLOG #213) | **Accepted (2026-07-11, owner-ratified)** — `FILTERED → UNROUTED` accepted; `message_events` declined-handler mitigation deferred from v1, gated behind the #63 verbosity gate when built | | [0085](0085-direct-hisp-smime-connector.md) | Direct-Project S/MIME-over-SMTP outbound connector (DIRECT-HISP, BACKLOG #157) — PR1 **outbound send only**: a new `ConnectorType.DIRECT` + `DirectDestination` that **SIGNs then ENCRYPTs** the Handler body via core `cryptography` `serialization.pkcs7` (no new dep — `endesive` rejected, `dnspython` deferred) and submits it as `application/pkcs7-mime; smime-type=enveloped-data` over the reused EMAIL STARTTLS/`refuse_cleartext_credentials` posture. All signing key+cert / per-partner recipient cert / trust anchor loaded + cross-validated at construction (fail loud): key↔cert public-key match + one-level `verify_directly_issued_by` chain check. New fail-closed `[egress].allowed_direct` host gate (kept separate from `allowed_smtp`). Inbound Direct mail / MDN / DNS-CERT discovery / IHE XDR **deferred** to later phases | Accepted (2026-07-10) — PR1 outbound-only, later phases deferred | -| [0087](0087-sandbox-subprocess-isolation.md) | Router/Handler subprocess isolation (SANDBOX, BACKLOG #197, ASVS 15.2.5 / WP-L3-17) — an **opt-in** `[sandbox]` section: `mode=off` (default) runs Routers/Handlers in-process **byte-identically, zero overhead**; `mode=subprocess` runs each inbound's Router/Handler in a **persistent per-inbound worker child** (`pipeline/sandbox.py` + `_sandbox_worker.py`; stdlib-only, no new dep — RestrictedPython rejected), never a per-message fork. The OS-process boundary denies admin code reach to the parent's DEK/audit-chain/sockets (the child loads only the message *graph*); on top: a forbidden-import guard (socket/store/crypto), a parent-enforced wall-clock cap (+ POSIX `RLIMIT_CPU`/`RLIMIT_AS`), and a **fail-closed** refusal of the live `db_lookup`/`fhir_lookup` bridges. Interposed at the `route_only`/`transform_one` seam (the in-process `mode=off` path composes with the ADR 0072 tracer; `mode=subprocess` bypasses it); engine-side handler/outbound-name validation stays engine-side; a denial → `ERROR`/dead-letter **post-ACK** (no NAK, never dropped). Closes the WP-L3-17 residual (residual-closure). **Residuals:** default-off; live-lookup forward-over-IPC deferred; load-time top-level exec not sandboxed (unchanged safe-source DACL gate) | Accepted (2026-07-10) — opt-in subprocess isolation built (#197) | +| [0087](0087-sandbox-subprocess-isolation.md) | Router/Handler subprocess isolation (SANDBOX, BACKLOG #197, ASVS 15.2.5 / WP-L3-17) — an **opt-in** `[sandbox]` section: `mode=off` (default) runs Routers/Handlers in-process **byte-identically, zero overhead**; `mode=subprocess` runs each inbound's Router/Handler in a **persistent per-inbound worker child** (`pipeline/sandbox.py` + `_sandbox_worker.py` + `_sandbox_codec.py`; stdlib-only, no new dep — RestrictedPython rejected), never a per-message fork. The OS-process boundary denies admin code reach to the parent's DEK/audit-chain/sockets (the child loads only the message *graph*); on top: a forbidden-import guard (socket/store/crypto), a parent-enforced wall-clock cap (+ POSIX `RLIMIT_CPU`/`RLIMIT_AS`), and a **fail-closed** refusal of the live `db_lookup`/`fhir_lookup` bridges. Interposed at the `route_only`/`transform_one` seam (the in-process `mode=off` path composes with the ADR 0072 tracer; `mode=subprocess` bypasses it); engine-side handler/outbound-name validation stays engine-side; a denial → `ERROR`/dead-letter **post-ACK** (no NAK, never dropped). **MFW2 amendment (BACKLOG #339):** the IPC pipe originally pickled, so a Handler's `__reduce__` executed in the engine parent and the boundary was bypassable outright; both legs now use a closed-tag, non-executing codec. Consistent with the [0144](0144-security-lint-gate-over-admin-authored-router-handler-config.md) row below, **15.2.5 stays Partial** — this is the *address-space* half (the child still reaches `os`/`subprocess` and the host); OS-level default-deny confinement is [ADR 0147](0147-hardened-runtime-isolation-for-router-handler-code-ipc-brokered-sandbox-extends-adr-0087.md), still Proposed. **Residuals:** default-off; live-lookup forward-over-IPC deferred; load-time top-level exec not sandboxed (unchanged safe-source DACL gate); confinement is address-space only; worker kill does not reap a grandchild (#342) | Accepted (2026-07-10) — opt-in subprocess isolation built (#197) | | [0088](0088-apiclient-service-cli-extraction.md) | Extract a Qt-free / FastAPI-free `apiclient/` engine-client library + a `messagefoundry service {install,start,stop,status}` CLI (BACKLOG #103, reusable-core half) — `apiclient/client.py` is the verbatim former `console/client.py` body (deps: `httpx` + lazy `truststore` + the pure `api/` pydantic models), the canonical client shared by the console, the headless load/acceptance harness, and future clients; `service.py` is the verbatim former `console/service_control.py` (stdlib-only Windows SCM control), surfaced on the CLI. `console/client.py` + `console/service_control.py` become thin re-export shims (no behaviour change); harness client imports repoint to `messagefoundry.apiclient`, Qt-widget imports stay on `console/`. Explicitly the reusable-core half of #103 — the console is **kept**; deleting `console/`, rehoming the Qt widgets, and dropping the `[console]` extra remain **deferred**. Does **not** supersede [ADR 0032](0032-console-desktop-launch.md); no new dependency | Accepted (2026-07-10) — built (PLAN-9 Wave 3) | | [0086](0086-deterministic-corepoint-import.md) | Deterministic Corepoint action-list import → code-first Handlers (BACKLOG #105) — the **inverse** of ADR 0076's typed vocabulary + lens: a pure, stdlib-only engine importer (`messagefoundry/corepoint_import.py`) + a `messagefoundry import corepoint --out ` CLI that parses a Corepoint action-list export and emits one code-first `@router`/`@handler` module per channel, calling the ADR 0076 vocabulary + `Send`. The `.py` stays the only artifact/execution path (#26). **Amended 2026-07-24 (§2(a′)/(b′)/(c′)): the input schema is VALIDATED and it is XML** — root ``, logic at `/`, a recursive control-flow tree whose `@Data` is rich-text markup that must be stripped before any statement classifies; `` is a section label (never an action) and `@Disabled` is preserved as a comment, never live. Parsed through defusedxml (`forbid_dtd`/`forbid_entities`/`forbid_external`); `parse_any()` sniffs XML vs. the superseded synthetic JSON model. The mapping is the inverse of ADR 0076 §2 and deliberately narrow — a field path is never guessed, and control flow is emitted as real Python with inert placeholder conditions. Unmapped verbs emit in-place `# TODO: Corepoint …` + best-effort `msg.set` stub (count-and-log — never dropped). Untrusted export values ride across as `json.dumps`-escaped literals (no code injection). Correctness gate: emitted modules pass `messagefoundry check` + round-trip through `lens parse`. Optional `ide/` wrapper deferred | Accepted (2026-07-10), amended 2026-07-24 — owner ratified; engine importer + CLI built, schema validated (#105) | | [0089](0089-recognition-first-lens-native-idioms.md) | Recognition-first lens (Phase A, BACKLOG #222) -- recognize the native Message API (msg.set / msg.field-copy / msg.delete_segments) as editable set_field/copy_field/delete_segment rows, so a Handler authored in the native API (not the ADR 0076 actions.py wrappers) renders as editable Steps without being rewritten. Extends the byte-stable rewrite to EDIT and INSERT those native forms import-free (insert_row exempt from the editable-kind guard; inserts indent to the anchor code line). Roadmap phases A-E; a production-estate scan measured ~66% opaque rows before, ~42% editable after Phase A | Accepted (2026-07-13) — owner-ratified; Phase A built + adopted | diff --git a/messagefoundry/config/response.py b/messagefoundry/config/response.py index 52353bfb..e52a1caf 100644 --- a/messagefoundry/config/response.py +++ b/messagefoundry/config/response.py @@ -16,9 +16,14 @@ correlation lineage — at which point this accessor resolves a real reply. The view it reads is **immutable committed** state, so it is re-run-stable by construction (ADR 0009). -**Layering (information hiding, CLAUDE.md §4).** This config-layer module owns only the active-view -holder + the accessor + the publish helpers; it does **not** import the store. The runner bridges the -two by publishing a store-derived view here around each run, so the config layer stays store-free. +**Layering (information hiding, CLAUDE.md §4).** This config-layer module owns the active-view holder, +the accessor, the publish helpers, and the :class:`CapturedResponse` *record shape* the view carries; +it does **not** import the store. The runner bridges the two by publishing a store-derived view here +around each run, so the config layer stays store-free. The record lives here rather than in +``store/store.py`` (which re-exports it, so every existing import path keeps working) because a +sandbox worker child has to rebuild it — and ``messagefoundry.store`` is on the sandbox's +forbidden-import list, so a store-homed record made ``[sandbox].mode=subprocess`` plus a correlated +LOOPBACK reply non-functional. """ from __future__ import annotations @@ -26,9 +31,11 @@ from collections.abc import Iterator, Mapping from contextlib import contextmanager from contextvars import ContextVar +from dataclasses import dataclass, field from typing import Any __all__ = [ + "CapturedResponse", "ResponseView", "response_get", "set_active", @@ -36,6 +43,31 @@ "activated", ] + +@dataclass(frozen=True) +class CapturedResponse: + """One captured request/response reply (ADR 0013), as returned by ``correlate_response`` for the + API/console read surface. ``body``/``detail`` are decrypted here; ``body`` is ``None`` once + retention has nulled it (the row is kept, like a purged ``messages.raw``).""" + + message_id: str + destination_name: str + response_seq: int + outcome: str + detail: str | None + captured_at: float + body: str | None + # ADR 0021 "Response Sent": 'response' (outbound reply, the default) | 'ack_sent' (inbound ACK we + # returned). ack_code/ack_phase are non-PHI disposition metadata, populated only for ack_sent rows. + kind: str = "response" + ack_code: str | None = None + ack_phase: str | None = None + # BACKLOG #154 (ADR 0013 amendment): the captured allow-listed HTTP response headers ({} when none / + # once retention nulls them). Decrypted + JSON-decoded here; a re-ingressed Handler reads it via + # response_get(dest).headers. + headers: Mapping[str, str] = field(default_factory=dict) + + #: The engine-published read view: a read-only mapping ``{destination_name: latest_reply}`` for the #: message in scope (Increment 2 populates it). The value shape is whatever the runner publishes; the #: config layer only reads it, so it needs no store import. ``None`` means "no active view". diff --git a/messagefoundry/pipeline/_sandbox_codec.py b/messagefoundry/pipeline/_sandbox_codec.py new file mode 100644 index 00000000..5fccae07 --- /dev/null +++ b/messagefoundry/pipeline/_sandbox_codec.py @@ -0,0 +1,1138 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""MFW2 — the non-executing IPC codec for the sandbox pipe (ADR 0087, BACKLOG #197). + +**Why a codec at all.** The sandbox exists to put an OS address-space boundary between the engine +(the DEK, the tamper-evident audit chain, every live socket) and admin-authored Router/Handler code. +A codec that can *name a type* while decoding — ``pickle`` is the canonical one — hands that boundary +straight back: a Handler returning an object with a custom ``__reduce__`` executes arbitrary code in +whichever process loads the frame. So **both legs are untrusted by contract**: + +* **child → parent** is untrusted because the child runs exactly the code the sandbox exists to + distrust, and because a *grandchild* the Handler spawns inherits fd 1 (the response pipe) and + survives ``proc.kill()`` — it can write a frame at any later moment. +* **parent → child** is untrusted because the same reasoning read in the other direction is the only + reason the first bullet is a boundary at all; a single schema, one review, one fuzz target. + +Decoding here reduces to :func:`json.loads` plus :meth:`bytes.decode` over a **closed tag set**, then +a literal ``if``/``elif`` match that calls the ordinary constructors of a fixed handful of types. +Nothing in the decode path can name a type, import a module, or reach ``__reduce__``. + +**Frame shape.** :func:`sandbox._write_frame` owns the outer ``>I length || body`` framing and the +64 MiB :data:`MAX_FRAME` ceiling (defined here, so the two layers cannot drift apart). This module owns +the *body*, which is **segmented** so a big message payload never round-trips through JSON string +escaping:: + + >I header_len || header_json || ( >I blob_len || blob_bytes ) * + +``header_json`` is the whole envelope; any long ``str`` slot is replaced by ``{"$": i}`` and carried +verbatim in blob ``i``. Blob references are **strictly sequential and exactly-once** — the decoder +holds a cursor, ``{"$": i}`` is legal only when ``i`` is the cursor, and decode succeeds only if every +blob was consumed. That makes the one dangerous failure mode of an indirection table (an off-by-one +silently cross-wiring one ``Send``'s body onto another ``Send``'s destination) *structurally* +unreachable rather than merely untested. The inline-vs-blob threshold is encoder-side **policy**: every +``str`` slot accepts either form, so the two ends can never disagree about it. + +**Non-goal (ADR 0028).** A binary body is ALREADY an ordinary ASCII ``str`` in ``RawMessage.raw`` (the +self-describing ``mfb64:v1:`` marker). It rides this codec as a plain ``str`` slot and MUST NOT +be re-wrapped through the ``{"b": ...}`` bytes tag — double-encoding would break +:attr:`RawMessage.raw_bytes` at the far end. + +**Fail-closed.** Every rejection raises :class:`SandboxCodecError`, a :class:`SandboxError` subclass, +so the caller routes it to ``ERROR``/dead-letter **post-ACK** exactly like any other isolation fault — +never a NAK, never an accept-and-drop, never a crashed connection. +""" + +from __future__ import annotations + +import base64 +import datetime as _dt +import json +import math +import struct +from collections.abc import Mapping, Sequence +from dataclasses import dataclass +from typing import Any, Final + +from messagefoundry.config.code_sets import CodeSet, UnmappedKind, UnmappedPolicy +from messagefoundry.config.models import ContentType +from messagefoundry.config.response import CapturedResponse +from messagefoundry.config.run_context import RunContext +from messagefoundry.config.wiring import Send, SetMeta, SetState, WiringError +from messagefoundry.parsing.message import Message, RawMessage + +__all__ = [ + "MAX_FRAME", + "SandboxError", + "SandboxCodecError", + "Ignored", + "Request", + "Response", + "build_payload", + "decode_boot", + "decode_boot_reply", + "decode_frame", + "decode_request", + "decode_response", + "encode_bootfail", + "encode_boot", + "encode_fail", + "encode_frame", + "encode_ok", + "encode_ready", + "encode_request", + "enc_value", + "dec_value", +] + + +class SandboxError(RuntimeError): + """A sandboxed Router/Handler was denied, timed out, crashed, or could not be marshalled. + + Raised only on the ``subprocess`` path (``off`` never raises this). The caller treats it exactly + like any other post-ACK router/transform failure: ``ERROR``/dead-letter, no NAK. + + Defined **here**, not in :mod:`messagefoundry.pipeline.sandbox`, only because + :class:`SandboxCodecError` must subclass it and the codec is the lower layer of the pair; the + public name stays ``messagefoundry.pipeline.sandbox.SandboxError`` (re-exported there).""" + + +class SandboxCodecError(SandboxError): + """A sandbox IPC frame was rejected: malformed, over a cap, or outside the closed value grammar. + + A :class:`SandboxError` subclass so ``route_only``/``transform_one``'s documented "raises + SandboxError" contract keeps meaning what it says for every rejection.""" + + +# --- caps -------------------------------------------------------------------- + +_LEN: Final = struct.Struct(">I") +#: The one wire ceiling, imported by :mod:`messagefoundry.pipeline.sandbox` as its ``_MAX_FRAME`` so the +#: outer framing and the body decoder cannot drift apart. +MAX_FRAME: Final = 64 * 1024 * 1024 +#: Deliberately the SAME number, not a tighter one. A tighter header cap is fail-CLOSED against traffic +#: it has no business judging: a request header carries the whole ``reference_view`` (an ADR 0006 +#: snapshot), so a 16 MiB header cap silently became a ~700k-entry ceiling on reference tables — +#: every message on a bigger graph dead-lettered forever, where ``mode=off`` served them fine. Since +#: :func:`sandbox._read_frame_bytes` already refuses a frame over :data:`MAX_FRAME` before a byte is +#: parsed, a separate lower bound buys no protection; it only buys a mode-dependent outage. +_MAX_HEADER: Final = MAX_FRAME +_MAX_BLOBS: Final = 65536 +#: Own nesting counter for the value grammar, independent of (and below) ``RecursionError``. Each level +#: is TWO JSON levels (``{"l":[…]}``), so this must stay well under half the recursion limit — but it +#: must also clear anything ``mode=off`` accepts, since ``SetState``'s own ``json.dumps`` validator +#: tolerates deep values and a tighter cap here would be a mode-dependent dead-letter. +_MAX_DEPTH: Final = 256 +#: Encoder-side POLICY only — a ``str`` at least this long travels out of band. Every ``str`` slot +#: decodes an inline string or a reference interchangeably, so the two ends cannot disagree. +_BLOB_MIN: Final = 4096 + +_WIRE_VERSION: Final = 1 + + +# --- framing ----------------------------------------------------------------- + + +def _reject_nonfinite(token: str) -> Any: + """``json.loads(parse_constant=...)`` hook: a bare ``NaN``/``Infinity`` literal never rides the wire + (non-finite floats travel as the explicit ``{"f": ...}`` tag instead).""" + raise SandboxCodecError(f"non-finite JSON literal {token!r} is not allowed in a sandbox frame") + + +def encode_frame(header: Mapping[str, Any], blobs: Sequence[str]) -> bytes: + """Serialize one frame body: the JSON header plus its out-of-band ``str`` segments.""" + try: + text = json.dumps(header, ensure_ascii=False, allow_nan=False, separators=(",", ":")) + except (TypeError, ValueError, RecursionError) as exc: + raise SandboxCodecError(f"sandbox frame header is not encodable: {exc}") from exc + head = text.encode("utf-8", "surrogatepass") + if len(head) > _MAX_HEADER: + raise SandboxCodecError(f"sandbox frame header too large: {len(head)} bytes") + if len(blobs) > _MAX_BLOBS: + raise SandboxCodecError(f"sandbox frame carries {len(blobs)} segments (> {_MAX_BLOBS})") + parts: list[bytes] = [_LEN.pack(len(head)), head] + for blob in blobs: + body = blob.encode("utf-8", "surrogatepass") + try: + parts.append(_LEN.pack(len(body))) + except struct.error as exc: # a single segment past 4 GiB + raise SandboxCodecError(f"sandbox frame segment too large: {exc}") from exc + parts.append(body) + return b"".join(parts) + + +def decode_frame(body: bytes) -> tuple[dict[str, Any], list[str]]: + """Parse one frame body into ``(header, blobs)``. Never constructs anything but JSON primitives.""" + try: + if len(body) < _LEN.size: + raise SandboxCodecError("truncated sandbox frame (no header length)") + (head_len,) = _LEN.unpack_from(body, 0) + if head_len > _MAX_HEADER: + raise SandboxCodecError(f"sandbox frame header too large: {head_len} bytes") + end = _LEN.size + head_len + if len(body) < end: + raise SandboxCodecError("truncated sandbox frame header") + header = json.loads( + body[_LEN.size : end].decode("utf-8", "surrogatepass"), + parse_constant=_reject_nonfinite, + ) + if not isinstance(header, dict): + raise SandboxCodecError("sandbox frame header must be a JSON object") + blobs: list[str] = [] + pos, total = end, len(body) + while pos < total: + if pos + _LEN.size > total: + raise SandboxCodecError("truncated sandbox frame segment length") + (blob_len,) = _LEN.unpack_from(body, pos) + pos += _LEN.size + if pos + blob_len > total: + raise SandboxCodecError("truncated sandbox frame segment") + if len(blobs) >= _MAX_BLOBS: + raise SandboxCodecError(f"sandbox frame carries > {_MAX_BLOBS} segments") + blobs.append(str(memoryview(body)[pos : pos + blob_len], "utf-8", "surrogatepass")) + pos += blob_len + except (ValueError, UnicodeDecodeError, RecursionError, struct.error) as exc: + # RecursionError is NOT a ValueError, and it is exactly what a depth-100000 header raises; + # without it the rejection would escape the fail-closed SandboxError contract. + raise SandboxCodecError(f"malformed sandbox frame: {type(exc).__name__}: {exc}") from exc + return header, blobs + + +class _Blobs: + """Encoder-side segment accumulator. ``ref()`` is the ONLY thing that appends, and it appends at + the same instant it emits the reference — so segment order IS schema walk order.""" + + __slots__ = ("items",) + + def __init__(self) -> None: + self.items: list[str] = [] + + def ref(self, text: object) -> Any: + if not isinstance(text, str): + raise SandboxCodecError(f"expected a str slot, got {type(text).__name__}") + if len(text) < _BLOB_MIN: + return text + self.items.append(text) + return {"$": len(self.items) - 1} + + +class _Reader: + """Decoder-side segment cursor enforcing the **strictly sequential, exactly-once** discipline. + + Out-of-range, duplicated, skipped and trailing-unconsumed segments are all rejected *by + construction*: a reference is legal only at the cursor, and :meth:`finish` demands the cursor + reached the end. That is what makes a cross-wired PHI body (one ``Send``'s payload landing on + another ``Send``'s destination) unreachable rather than merely untested.""" + + __slots__ = ("blobs", "cursor") + + def __init__(self, blobs: Sequence[str]) -> None: + self.blobs = blobs + self.cursor = 0 + + def text(self, node: Any) -> str: + if isinstance(node, str): + return node + if isinstance(node, dict) and len(node) == 1 and "$" in node: + index = node["$"] + if not isinstance(index, int) or isinstance(index, bool): + raise SandboxCodecError("sandbox segment reference must be an integer") + if index != self.cursor or index >= len(self.blobs): + raise SandboxCodecError( + f"sandbox segment reference {index} is out of sequence (expected {self.cursor})" + ) + self.cursor += 1 + return self.blobs[index] + raise SandboxCodecError("sandbox frame slot is neither a string nor a segment reference") + + def finish(self) -> None: + if self.cursor != len(self.blobs): + raise SandboxCodecError( + f"sandbox frame carries {len(self.blobs)} segments but consumed {self.cursor}" + ) + + +# --- the closed value grammar ------------------------------------------------ + + +def enc_value(value: object, blobs: _Blobs, _depth: int = 0) -> Any: + """Describe one genuinely-``Any`` value (a reference/code-set entry, a ``SetState`` value). + + Containers are **wrapped** in a single-key tag object, so a bare JSON array is illegal at value + level and a legitimate list can never be mistaken for a tag — an unambiguous grammar with zero + escaping. ``blobs`` is required: an "inline everything" variant would be a second encoding of the + same value that no caller wants and nobody fuzzes.""" + if _depth > _MAX_DEPTH: + raise SandboxCodecError(f"sandbox value nesting exceeds {_MAX_DEPTH}") + if value is None or isinstance(value, bool): + return value + if isinstance(value, int): + return value + if isinstance(value, float): + # A tag, not a narrowing: SetState.__post_init__ validates with json.dumps' DEFAULT + # allow_nan=True, so SetState(ns, k, float("nan")) constructs today. Refusing it here would + # turn a working Handler into a new dead-letter purely on upgrade. + if math.isnan(value): + return {"f": "nan"} + if math.isinf(value): + return {"f": "inf" if value > 0 else "-inf"} + return value + if isinstance(value, str): + return blobs.ref(value) + if isinstance(value, bytes | bytearray): + return {"b": base64.b64encode(bytes(value)).decode("ascii")} + # datetime BEFORE date — datetime is a date subclass, and tomllib produces all three natively, so a + # .toml code set with a date value is a legitimate shape plain JSON refuses. + if isinstance(value, _dt.datetime): + return {"D": value.isoformat()} + if isinstance(value, _dt.date): + return {"E": value.isoformat()} + if isinstance(value, _dt.time): + return {"T": value.isoformat()} + if isinstance(value, list): + return {"l": [enc_value(item, blobs, _depth + 1) for item in value]} + if isinstance(value, tuple): + # Preserved as its own tag: a JSON-array shortcut would silently flatten a tuple to a list, + # a value-shape change mode=off does not make. + return {"p": [enc_value(item, blobs, _depth + 1) for item in value]} + if isinstance(value, Mapping): + return { + "d": [ + [enc_value(k, blobs, _depth + 1), enc_value(v, blobs, _depth + 1)] + for k, v in value.items() + ] + } + raise SandboxCodecError( + f"value of type {type(value).__name__} cannot cross the sandbox boundary" + ) + + +_FLOAT_TAGS: Final[dict[str, float]] = {"nan": math.nan, "inf": math.inf, "-inf": -math.inf} + + +def dec_value(node: Any, reader: _Reader, _depth: int = 0) -> Any: + """Rebuild one value from the closed grammar. A closed literal match — no getattr, no import.""" + if _depth > _MAX_DEPTH: + raise SandboxCodecError(f"sandbox value nesting exceeds {_MAX_DEPTH}") + if node is None or isinstance(node, bool | int | float | str): + return node + if isinstance(node, list): + raise SandboxCodecError("a bare array is not a sandbox value (containers are tagged)") + if not isinstance(node, dict) or len(node) != 1: + raise SandboxCodecError("a sandbox value object must carry exactly one tag") + ((tag, arg),) = node.items() + if tag == "$": + return reader.text(node) + if tag == "l": + return [dec_value(item, reader, _depth + 1) for item in _tag_list(tag, arg)] + if tag == "p": + return tuple(dec_value(item, reader, _depth + 1) for item in _tag_list(tag, arg)) + if tag == "d": + out: dict[Any, Any] = {} + for pair in _tag_list(tag, arg): + if not isinstance(pair, list) or len(pair) != 2: + raise SandboxCodecError("a sandbox dict entry must be a [key, value] pair") + key = dec_value(pair[0], reader, _depth + 1) + value = dec_value(pair[1], reader, _depth + 1) + try: + out[key] = value + except ( + TypeError + ) as exc: # an unhashable decoded key must not escape as a bare TypeError + raise SandboxCodecError(f"sandbox dict key is unhashable: {exc}") from exc + return out + if tag == "b": + if not isinstance(arg, str): + raise SandboxCodecError("sandbox bytes tag must carry a base64 string") + try: + return base64.b64decode(arg, validate=True) + except (ValueError, TypeError) as exc: + raise SandboxCodecError(f"sandbox bytes tag is not valid base64: {exc}") from exc + if tag == "f": + if not isinstance(arg, str) or arg not in _FLOAT_TAGS: + raise SandboxCodecError(f"unknown sandbox float tag {arg!r}") + return _FLOAT_TAGS[arg] + if tag in ("D", "E", "T"): + if not isinstance(arg, str): + raise SandboxCodecError(f"sandbox {tag!r} tag must carry an ISO-8601 string") + try: + if tag == "D": + return _dt.datetime.fromisoformat(arg) + if tag == "E": + return _dt.date.fromisoformat(arg) + return _dt.time.fromisoformat(arg) + except ValueError as exc: + raise SandboxCodecError(f"sandbox {tag!r} tag is not ISO-8601: {exc}") from exc + raise SandboxCodecError(f"unknown sandbox value tag {tag!r}") + + +def _tag_list(tag: str, arg: Any) -> list[Any]: + if not isinstance(arg, list): + raise SandboxCodecError(f"sandbox {tag!r} tag must carry an array") + return arg + + +# --- small typed slot helpers ------------------------------------------------- + + +def _req_str(node: Any, what: str) -> str: + if not isinstance(node, str): + raise SandboxCodecError(f"sandbox {what} must be a string") + return node + + +def _opt_str(node: Any, what: str) -> str | None: + if node is None: + return None + return _req_str(node, what) + + +def _req_int(node: Any, what: str) -> int: + if not isinstance(node, int) or isinstance(node, bool): + raise SandboxCodecError(f"sandbox {what} must be an integer") + return node + + +def _req_float(node: Any, what: str) -> float: + if isinstance(node, bool) or not isinstance(node, int | float): + raise SandboxCodecError(f"sandbox {what} must be a number") + return float(node) + + +def _opt_float(node: Any, what: str) -> float | None: + if node is None: + return None + return _req_float(node, what) + + +def _req_bool(node: Any, what: str) -> bool: + if not isinstance(node, bool): + raise SandboxCodecError(f"sandbox {what} must be a boolean") + return node + + +def _req_list(node: Any, what: str) -> list[Any]: + if not isinstance(node, list): + raise SandboxCodecError(f"sandbox {what} must be an array") + return node + + +def _req_obj(node: Any, what: str) -> dict[str, Any]: + if not isinstance(node, dict): + raise SandboxCodecError(f"sandbox {what} must be an object") + return node + + +# --- the payload (parent -> child) -------------------------------------------- + + +def build_payload(raw: str | bytes, content_type: str) -> Message | RawMessage: + """The object a Router/Handler receives (ADR 0004): a mutable HL7 :class:`Message` for ``hl7v2``, + a verbatim :class:`RawMessage` otherwise. + + The single definition of that construction, shared by :func:`dryrun._payload` and the sandbox + child, so the child rebuilds the **same** object the parent would have built rather than a + look-alike that could drift. It lives here (not in ``dryrun``) because the child must not import + ``dryrun`` — that would pull ``messagefoundry.store`` into the sandboxed process, which is exactly + what the forbidden-import guard denies (and what "the child loads the graph, not the store" + means).""" + if content_type == ContentType.HL7V2.value: + return Message.parse(raw) + return RawMessage(raw if isinstance(raw, str) else raw.decode("utf-8"), content_type) + + +def enc_payload( + payload: object, origin: tuple[str | bytes, str] | None, blobs: _Blobs +) -> dict[str, Any]: + """Describe the Router/Handler input. + + ``origin`` is the ``(raw, content_type)`` the PARENT derived the payload from — passed only when + the caller's own ``payload`` argument was ``None`` (all three live engine call sites), so the child + can call the identical :func:`build_payload` and get a byte-faithful object. Otherwise the + caller's actual object is described (``dryrun.route_message(payload=shared)`` is public API).""" + if origin is not None: + raw, content_type = origin + if isinstance(raw, str): + return {"k": "origin", "enc": "s", "raw": blobs.ref(raw), "ct": content_type} + return { + "k": "origin", + "enc": "b", + "raw": blobs.ref(base64.b64encode(raw).decode("ascii")), + "ct": content_type, + } + if isinstance(payload, Message): + return {"k": "obj", "kind": "hl7", "text": blobs.ref(payload.encode())} + if isinstance(payload, RawMessage): + return { + "k": "obj", + "kind": "raw", + "text": blobs.ref(payload.raw), + "ct": payload.content_type, + } + raise SandboxCodecError( + f"a sandbox payload must be a Message or RawMessage, got {type(payload).__name__}" + ) + + +def dec_payload(node: Any, reader: _Reader) -> Message | RawMessage: + """Rebuild the Router/Handler input in the child (see :func:`enc_payload` for the two forms).""" + obj = _req_obj(node, "payload") + kind = obj.get("k") + if kind == "origin": + text = reader.text(obj.get("raw")) + content_type = _req_str(obj.get("ct"), "payload content type") + enc = obj.get("enc") + if enc == "s": + return build_payload(text, content_type) + if enc == "b": + try: + return build_payload(base64.b64decode(text, validate=True), content_type) + except (ValueError, TypeError) as exc: + raise SandboxCodecError(f"sandbox payload is not valid base64: {exc}") from exc + raise SandboxCodecError(f"unknown sandbox payload encoding {enc!r}") + if kind == "obj": + text = reader.text(obj.get("text")) + shape = obj.get("kind") + if shape == "hl7": + return Message.parse(text) + if shape == "raw": + return RawMessage(text, _req_str(obj.get("ct"), "payload content type")) + raise SandboxCodecError(f"unknown sandbox payload shape {shape!r}") + raise SandboxCodecError(f"unknown sandbox payload kind {kind!r}") + + +# --- the run context (parent -> child) ---------------------------------------- + + +def enc_run_context(rc: RunContext, blobs: _Blobs) -> dict[str, Any]: + """Describe the run-scoped views for the child. + + Snapshotting is **by construction** here: the engine's live ``MappingProxyType`` windows onto the + store caches are walked into plain wire rows at this instant, which is exactly the read-only + content an in-process run would have seen (and re-run stability makes a point-in-time copy the + contract anyway). There is deliberately no second marshalling layer. + + ``code_sets`` is **absent by design** — the engine's tables ride the one-per-spawn ``boot`` frame + instead (see :func:`enc_code_sets`). Re-sending them per dispatch was ~430 KB and ~4.6 ms of pure + marshalling per message on a realistic crosswalk.""" + return { + "reference_view": _enc_reference_view(rc.reference_view, blobs), + "state_view": _enc_state_view(rc.state_view, blobs), + "response_view": _enc_response_view(rc.response_view, blobs), + "active_environment": rc.active_environment, + "ingest_time": rc.ingest_time, + "message_id": rc.message_id, + "snapshot_on_send": bool(rc.snapshot_on_send), + } + + +def dec_run_context(node: Any, reader: _Reader) -> RunContext: + """Rebuild the child-side :class:`RunContext` (``code_sets`` stays ``None`` — the worker + substitutes its OWN registry's tables).""" + obj = _req_obj(node, "run context") + # The three view slots are read in the SAME order enc_run_context wrote them: segment order is + # schema walk order, so this ordering is load-bearing, not stylistic. + reference_view = _dec_reference_view(obj.get("reference_view"), reader) + state_view = _dec_state_view(obj.get("state_view"), reader) + response_view = _dec_response_view(obj.get("response_view"), reader) + return RunContext( + code_sets=None, + reference_view=reference_view, + state_view=state_view, + response_view=response_view, + active_environment=_opt_str(obj.get("active_environment"), "active_environment"), + ingest_time=_opt_float(obj.get("ingest_time"), "ingest_time"), + message_id=_opt_str(obj.get("message_id"), "message_id"), + snapshot_on_send=_req_bool(obj.get("snapshot_on_send"), "snapshot_on_send"), + ) + + +def _enc_table(what: str, table: Mapping[Any, Any], blobs: _Blobs) -> Any: + """One lookup table (an ADR 0006 reference set, a code set), with a fast path for the shape that + dominates real traffic. + + A crosswalk is overwhelmingly ``str -> short str``. Describing those entry by entry through + :func:`enc_value` costs a per-entry Python round trip in BOTH directions, which made the + ``reference_view`` the single largest per-message cost of ``mode=subprocess`` (measurably worse than + the pickle it replaced). When every entry fits that shape the table travels as ONE plain JSON + object and the C encoder/decoder does the walk. This is an ENCODING choice, not a grammar change: + the decoder still proves every value is a ``str`` before it is used, and either form decodes, so the + two ends can never disagree about which one to use.""" + if all( + isinstance(k, str) and isinstance(v, str) and len(v) < _BLOB_MIN for k, v in table.items() + ): + return {"s": dict(table)} + return {"a": [[_req_str(k, f"{what} key"), enc_value(v, blobs)] for k, v in table.items()]} + + +def _dec_table(node: Any, what: str, reader: _Reader) -> dict[str, Any]: + """Rebuild one lookup table from either form (see :func:`_enc_table`).""" + obj = _req_obj(node, what) + if len(obj) != 1: + # Exactly one form, like every other tagged node here — a frame carrying both would decode + # under one of them and leave the other silently unread. + raise SandboxCodecError(f"a {what} must carry exactly one table form") + flat = obj.get("s") + if flat is not None: + entries = _req_obj(flat, what) + # JSON object keys are strings by construction; the VALUES are what a forged frame could vary, + # so they carry the whole type proof for this form. + if not all(type(value) is str for value in entries.values()): + raise SandboxCodecError(f"{what}: every compact-table value must be a string") + return entries + out: dict[str, Any] = {} + for entry in _req_list(obj.get("a"), f"{what} entries"): + kv = _req_list(entry, f"{what} entry") + if len(kv) != 2: + raise SandboxCodecError(f"a {what} entry must be [key, value]") + out[_req_str(kv[0], f"{what} key")] = dec_value(kv[1], reader) + return out + + +def _enc_reference_view(view: object, blobs: _Blobs) -> Any: + if view is None: + return None + if not isinstance(view, Mapping): + raise SandboxCodecError(f"reference_view must be a mapping, got {type(view).__name__}") + rows: list[Any] = [] + for name, table in view.items(): + if not isinstance(name, str): + raise SandboxCodecError("reference_view names must be strings") + if not isinstance(table, Mapping): + raise SandboxCodecError(f"reference set {name!r} must be a mapping") + rows.append([name, _enc_table("reference set", table, blobs)]) + return {"refs": rows} + + +def _dec_reference_view(node: Any, reader: _Reader) -> dict[str, dict[str, Any]] | None: + if node is None: + return None + rows = _req_list(_req_obj(node, "reference_view").get("refs"), "reference_view rows") + out: dict[str, dict[str, Any]] = {} + for row in rows: + pair = _req_list(row, "reference_view row") + if len(pair) != 2: + raise SandboxCodecError("a reference_view row must be [name, entries]") + name = _req_str(pair[0], "reference name") + out[name] = _dec_table(pair[1], "reference set", reader) + return out + + +def _enc_state_view(view: object, blobs: _Blobs) -> Any: + if view is None: + return None + if not isinstance(view, Mapping): + raise SandboxCodecError(f"state_view must be a mapping, got {type(view).__name__}") + rows: list[Any] = [] + for key, value in view.items(): + if not (isinstance(key, tuple) and len(key) == 2 and all(isinstance(p, str) for p in key)): + raise SandboxCodecError("state_view keys must be (namespace, key) string tuples") + rows.append([key[0], key[1], enc_value(value, blobs)]) + return {"state": rows} + + +def _dec_state_view(node: Any, reader: _Reader) -> dict[tuple[str, str], Any] | None: + if node is None: + return None + rows = _req_list(_req_obj(node, "state_view").get("state"), "state_view rows") + out: dict[tuple[str, str], Any] = {} + for row in rows: + triple = _req_list(row, "state_view row") + if len(triple) != 3: + raise SandboxCodecError("a state_view row must be [namespace, key, value]") + # The key is rebuilt as a TUPLE deliberately: state_get() does a dict lookup on + # (namespace, key), so a list/joined key would turn every state_get into a SILENT miss + # returning its default — fail-open, inverting a suppression/dedup Handler with no ERROR, + # no dead-letter and no disposition anomaly. + out[(_req_str(triple[0], "state namespace"), _req_str(triple[1], "state key"))] = dec_value( + triple[2], reader + ) + return out + + +def _enc_response_view(view: object, blobs: _Blobs) -> Any: + if view is None: + return None + if not isinstance(view, Mapping): + raise SandboxCodecError(f"response_view must be a mapping, got {type(view).__name__}") + rows: list[Any] = [] + for dest, reply in view.items(): + if not isinstance(dest, str): + raise SandboxCodecError("response_view destinations must be strings") + if not isinstance(reply, CapturedResponse): + raise SandboxCodecError( + f"response_view[{dest!r}] must be a CapturedResponse, got {type(reply).__name__}" + ) + rows.append( + [ + dest, + { + "message_id": reply.message_id, + "destination_name": reply.destination_name, + "response_seq": reply.response_seq, + "outcome": reply.outcome, + "detail": reply.detail, + "captured_at": reply.captured_at, + "body": None if reply.body is None else blobs.ref(reply.body), + "kind": reply.kind, + "ack_code": reply.ack_code, + "ack_phase": reply.ack_phase, + "headers": [[k, v] for k, v in reply.headers.items()], + }, + ] + ) + return {"replies": rows} + + +def _dec_response_view(node: Any, reader: _Reader) -> dict[str, CapturedResponse] | None: + if node is None: + return None + rows = _req_list(_req_obj(node, "response_view").get("replies"), "response_view rows") + out: dict[str, CapturedResponse] = {} + for row in rows: + pair = _req_list(row, "response_view row") + if len(pair) != 2: + raise SandboxCodecError("a response_view row must be [destination, reply]") + rec = _req_obj(pair[1], "captured reply") + headers: dict[str, str] = {} + for entry in _req_list(rec.get("headers"), "captured reply headers"): + kv = _req_list(entry, "captured reply header") + if len(kv) != 2: + raise SandboxCodecError("a captured reply header must be [name, value]") + headers[_req_str(kv[0], "header name")] = _req_str(kv[1], "header value") + body_node = rec.get("body") + out[_req_str(pair[0], "response destination")] = CapturedResponse( + message_id=_req_str(rec.get("message_id"), "reply message_id"), + destination_name=_req_str(rec.get("destination_name"), "reply destination_name"), + response_seq=_req_int(rec.get("response_seq"), "reply response_seq"), + outcome=_req_str(rec.get("outcome"), "reply outcome"), + detail=_opt_str(rec.get("detail"), "reply detail"), + captured_at=_req_float(rec.get("captured_at"), "reply captured_at"), + body=None if body_node is None else reader.text(body_node), + kind=_req_str(rec.get("kind"), "reply kind"), + ack_code=_opt_str(rec.get("ack_code"), "reply ack_code"), + ack_phase=_opt_str(rec.get("ack_phase"), "reply ack_phase"), + headers=headers, + ) + return out + + +# --- the result (child -> parent) --------------------------------------------- + + +class Ignored: + """A child result item :func:`dryrun._partition` would ignore, described rather than omitted. + + The encoder never silently drops an item it does not recognise — a silent omission would be an + accept-and-drop (CLAUDE.md §12). Rebuilding the ignored slot keeps ``_partition`` the SOLE filter, + so a tuple/set/int Handler return still resolves to ``([], [], [])`` byte-identically to + ``mode=off``.""" + + __slots__ = () + + def __repr__(self) -> str: # pragma: no cover - diagnostics only + return "Ignored()" + + +def enc_result(phase: str, result: object, blobs: _Blobs) -> dict[str, Any]: + """Describe a Router/Handler/predicate return value, phase-typed.""" + if phase == "accepts": + # Strict: the caller coerces with bool() BEFORE this point (see _sandbox_worker), which is what + # stops a truthy non-bool such as an re.Match from ever reaching the encoder. + if not isinstance(result, bool): + raise SandboxCodecError(f"accepts verdict must be a bool, got {type(result).__name__}") + return {"r": "bool", "b": result} + if phase == "router": + # Materialise with _handler_names' OWN logic HERE, in the child: that is what makes a + # generator Router (documented-supported, and unpicklable) work under mode=subprocess. The + # parent's _handler_names then sees a list[str] and its list(...) is a no-op copy. + try: + if result is None: + names: list[Any] = [] + elif isinstance(result, str): + names = [result] + else: + names = list(result) # type: ignore[call-overload] + except TypeError as exc: + raise SandboxCodecError(f"router result is not iterable: {exc}") from exc + for item in names: + if not isinstance(item, str): + raise SandboxCodecError( + f"router returned a non-string handler name ({type(item).__name__})" + ) + return {"r": "names", "n": names} + if phase == "transform": + # `shape` reproduces _partition's EXACT input container so the parent hands the unmodified + # _partition the same thing mode=off would. Do NOT normalise an iterable into a list: that + # would start delivering Sends mode=off drops. + if result is None: + return {"r": "items", "shape": "none"} + if isinstance(result, list): + return {"r": "items", "shape": "list", "i": [_enc_item(it, blobs) for it in result]} + return {"r": "items", "shape": "one", "i": _enc_item(result, blobs)} + raise SandboxCodecError(f"unknown sandbox phase {phase!r}") + + +def dec_result(phase: str, node: Any, reader: _Reader) -> object: + """Rebuild a Router/Handler/predicate return value in the parent.""" + obj = _req_obj(node, "result") + tag = obj.get("r") + if phase == "accepts": + if tag != "bool": + raise SandboxCodecError(f"expected an accepts verdict, got {tag!r}") + return _req_bool(obj.get("b"), "accepts verdict") + if phase == "router": + if tag != "names": + raise SandboxCodecError(f"expected router names, got {tag!r}") + return [_req_str(item, "handler name") for item in _req_list(obj.get("n"), "router names")] + if phase == "transform": + if tag != "items": + raise SandboxCodecError(f"expected transform items, got {tag!r}") + shape = obj.get("shape") + if shape == "none": + return None + if shape == "one": + return _dec_item(obj.get("i"), reader) + if shape == "list": + return [_dec_item(it, reader) for it in _req_list(obj.get("i"), "transform items")] + raise SandboxCodecError(f"unknown transform result shape {shape!r}") + raise SandboxCodecError(f"unknown sandbox phase {phase!r}") + + +def _enc_item(item: object, blobs: _Blobs) -> dict[str, Any]: + # isinstance, in _partition's order — NOT type identity, so a user SUBCLASS of Send still + # delivers exactly as it does in-process. + if isinstance(item, Send): + # `.encode()` is called at DESCRIBE time — the same relative instant mode=off evaluates + # dryrun.py's `send.message if isinstance(..., str) else send.message.encode()`, which is the + # SOLE parent-side consumer of Send.message. Carrying text (never a live Message) is what makes + # the parent's Send() rebuild a provable no-op for ADR 0104's copy-on-Send choke point. + try: + text = item.message if isinstance(item.message, str) else item.message.encode() + except (AttributeError, TypeError, ValueError) as exc: + raise SandboxCodecError(f"Send message is not encodable: {exc}") from exc + if not isinstance(text, str): + raise SandboxCodecError( + f"Send message encoded to {type(text).__name__}, expected a str" + ) + return {"o": "send", "to": _req_str(item.to, "Send destination"), "m": blobs.ref(text)} + if isinstance(item, SetState): + return { + "o": "state", + "ns": _req_str(item.namespace, "SetState namespace"), + "key": _req_str(item.key, "SetState key"), + "v": enc_value(item.value, blobs), + } + if isinstance(item, SetMeta): + return { + "o": "meta", + "key": _req_str(item.key, "SetMeta key"), + "v": blobs.ref(item.value), + } + return {"o": "other"} + + +def _dec_item(node: Any, reader: _Reader) -> object: + obj = _req_obj(node, "transform item") + tag = obj.get("o") + # The NORMAL constructors over a CLOSED four-tag literal match — no getattr, no import, no + # registry keyed by a wire string. + try: + if tag == "send": + return Send(_req_str(obj.get("to"), "Send destination"), reader.text(obj.get("m"))) + if tag == "state": + return SetState( + _req_str(obj.get("ns"), "SetState namespace"), + _req_str(obj.get("key"), "SetState key"), + dec_value(obj.get("v"), reader), + ) + if tag == "meta": + return SetMeta(_req_str(obj.get("key"), "SetMeta key"), reader.text(obj.get("v"))) + except (WiringError, TypeError, ValueError) as exc: + # The child ALREADY ran these constructors successfully (a Handler had to build the object to + # return it), so a raise here cannot be an authoring fault — it is a codec bug or a forged + # frame. Reporting it as a WiringError would put a FALSE diagnosis in the operator's last_error. + raise SandboxCodecError(f"sandbox result item could not be rebuilt: {exc}") from exc + if tag == "other": + return Ignored() + raise SandboxCodecError(f"unknown sandbox result item tag {tag!r}") + + +# --- the code-set tables (parent -> child, once per spawn) -------------------- + + +def enc_code_sets(code_sets: object, blobs: _Blobs) -> Any: + """Describe the ENGINE's live ``{name: CodeSet}`` tables for the worker's bootstrap frame. + + Sent **once per spawn**, not per dispatch: re-sending them with every message was ~430 KB and + ~4.6 ms of pure marshalling on a realistic crosswalk. Sending them at all — rather than letting the + child use its OWN ``load_config`` of the same directory — is what keeps ``mode=subprocess`` + byte-identical to ``mode=off``: after a transparent respawn (a wall-cap kill, a crash) the child + re-reads ``codesets/`` from disk, so an operator who edits a crosswalk without a ``/config/reload`` + would otherwise have the child serve a value the engine never would. The engine's tables are the + single source of truth on both sides. + + ``None`` means "the caller published no tables" and leaves the child on its own bootstrap load (the + pre-existing behaviour for a session constructed without them); an empty mapping means "the engine + has none", which is a different statement.""" + if code_sets is None: + return None + if not isinstance(code_sets, Mapping): + raise SandboxCodecError(f"code_sets must be a mapping, got {type(code_sets).__name__}") + rows: list[Any] = [] + for name, table in code_sets.items(): + if not isinstance(name, str): + raise SandboxCodecError("code set names must be strings") + if not isinstance(table, Mapping): + raise SandboxCodecError(f"code set {name!r} must be a mapping") + policy = getattr(table, "policy", None) + declared = policy if isinstance(policy, UnmappedPolicy) else UnmappedPolicy() + rows.append( + [ + name, + declared.kind.value, + declared.default_value, + _enc_table(f"code set {name!r}", table, blobs), + ] + ) + return {"sets": rows} + + +def dec_code_sets(node: Any, reader: _Reader) -> dict[str, CodeSet] | None: + """Rebuild the engine's code-set tables in the child (the ordinary :class:`CodeSet` constructor).""" + if node is None: + return None + rows = _req_list(_req_obj(node, "code_sets").get("sets"), "code_sets rows") + out: dict[str, CodeSet] = {} + for row in rows: + quad = _req_list(row, "code_sets row") + if len(quad) != 4: + raise SandboxCodecError("a code_sets row must be [name, kind, default_value, entries]") + name = _req_str(quad[0], "code set name") + kind = _req_str(quad[1], "code set policy kind") + default_value = _opt_str(quad[2], "code set policy default_value") + entries = _dec_table(quad[3], f"code set {name!r}", reader) + try: + # UnmappedKind rejects an unknown kind and UnmappedPolicy.__post_init__ rejects an + # inconsistent (kind, default_value) pair — both ValueError, both folded into the codec's + # fail-closed contract rather than escaping as a config-layer diagnosis. + policy = UnmappedPolicy(kind=UnmappedKind(kind), default_value=default_value) + except ValueError as exc: + raise SandboxCodecError( + f"code set {name!r} carries an invalid unmapped policy: {exc}" + ) from exc + out[name] = CodeSet(name, entries, policy) + return out + + +# --- envelopes ---------------------------------------------------------------- + + +def _envelope(header: dict[str, Any], kind: str) -> dict[str, Any]: + return {"v": _WIRE_VERSION, "t": kind, **header} + + +def _open(body: bytes, allowed: tuple[str, ...]) -> tuple[dict[str, Any], _Reader, str]: + header, blobs = decode_frame(body) + if header.get("v") != _WIRE_VERSION: + raise SandboxCodecError(f"unsupported sandbox wire version {header.get('v')!r}") + kind = header.get("t") + if not isinstance(kind, str) or kind not in allowed: + raise SandboxCodecError(f"unexpected sandbox frame type {kind!r}") + return header, _Reader(blobs), kind + + +def encode_boot( + *, + config_dir: str, + forbidden: Sequence[str], + cpu_seconds: float, + mem_mb: int | None, + code_sets: object, +) -> bytes: + """The one-per-spawn bootstrap frame. + + ``env`` is deliberately NOT marshalled: ``load_config()`` takes only a directory and the worker + never read it — a dead field on the wire is a field nobody validates.""" + blobs = _Blobs() + header = _envelope( + { + "config_dir": config_dir, + "forbidden": list(forbidden), + "cpu_seconds": float(cpu_seconds), + "mem_mb": None if mem_mb is None else int(mem_mb), + "code_sets": enc_code_sets(code_sets, blobs), + }, + "boot", + ) + return encode_frame(header, blobs.items) + + +@dataclass(frozen=True) +class Boot: + """The decoded bootstrap frame (child side).""" + + config_dir: str + forbidden: tuple[str, ...] + cpu_seconds: float + mem_mb: int | None + code_sets: dict[str, CodeSet] | None + + +def decode_boot(body: bytes) -> Boot: + header, reader, _ = _open(body, ("boot",)) + config_dir = _req_str(header.get("config_dir"), "config_dir") + forbidden = tuple( + _req_str(m, "forbidden module") for m in _req_list(header.get("forbidden"), "forbidden") + ) + cpu_seconds = _req_float(header.get("cpu_seconds"), "cpu_seconds") + mem_mb = None if header.get("mem_mb") is None else _req_int(header.get("mem_mb"), "mem_mb") + code_sets = dec_code_sets(header.get("code_sets"), reader) + reader.finish() + return Boot( + config_dir=config_dir, + forbidden=forbidden, + cpu_seconds=cpu_seconds, + mem_mb=mem_mb, + code_sets=code_sets, + ) + + +def encode_ready() -> bytes: + return encode_frame(_envelope({}, "ready"), ()) + + +def encode_bootfail(error: str) -> bytes: + blobs = _Blobs() + header = _envelope({"error": blobs.ref(error)}, "bootfail") + return encode_frame(header, blobs.items) + + +def decode_boot_reply(body: bytes) -> tuple[bool, str]: + """``(ready, error)`` — ``error`` is type-checked to ``str`` BEFORE the parent interpolates it, so a + child-supplied object's ``__format__``/``__repr__`` is never invoked in the engine.""" + header, reader, kind = _open(body, ("ready", "bootfail")) + if kind == "ready": + reader.finish() + return True, "" + error = reader.text(header.get("error")) + reader.finish() + return False, error + + +def encode_request( + *, + request_id: str, + phase: str, + name: str, + payload: object, + origin: tuple[str | bytes, str] | None, + run_context: RunContext, +) -> bytes: + blobs = _Blobs() + header = _envelope( + { + "id": request_id, + "phase": phase, + "name": name, + "payload": enc_payload(payload, origin, blobs), + "rc": enc_run_context(run_context, blobs), + }, + "req", + ) + return encode_frame(header, blobs.items) + + +@dataclass(frozen=True) +class Request: + """The decoded dispatch request (child side).""" + + request_id: str + phase: str + name: str + payload: Message | RawMessage + run_context: RunContext + + +def decode_request(body: bytes) -> Request: + header, reader, _ = _open(body, ("req",)) + # Walk order matches encode_request: payload BEFORE rc. + request_id = _req_str(header.get("id"), "request id") + phase = _req_str(header.get("phase"), "request phase") + name = _req_str(header.get("name"), "request name") + payload = dec_payload(header.get("payload"), reader) + run_context = dec_run_context(header.get("rc"), reader) + reader.finish() + return Request( + request_id=request_id, phase=phase, name=name, payload=payload, run_context=run_context + ) + + +def encode_ok(*, request_id: str, phase: str, name: str, result: object) -> bytes: + blobs = _Blobs() + header = _envelope( + { + "id": request_id, + "phase": phase, + "name": name, + "result": enc_result(phase, result, blobs), + }, + "ok", + ) + return encode_frame(header, blobs.items) + + +def encode_fail(*, request_id: str, phase: str, name: str, kind: str, error: str) -> bytes: + blobs = _Blobs() + header = _envelope( + {"id": request_id, "phase": phase, "name": name, "kind": kind, "error": blobs.ref(error)}, + "fail", + ) + return encode_frame(header, blobs.items) + + +@dataclass(frozen=True) +class Response: + """The decoded dispatch response (parent side).""" + + ok: bool + result: object + kind: str + error: str + + +def decode_response(body: bytes, *, request_id: str, phase: str, name: str) -> Response: + """Decode one response frame and check it answers the request the parent actually made. + + The whole ``(id, phase, name)`` triple is bound, so a frame produced for a *different* call can + never be read as this one's answer. **What this is and is not:** ``request_id`` is a fresh + :func:`secrets.token_hex` the parent mints per dispatch and reveals only inside that dispatch's + request frame, so a frame written *before* the request — by the previous Handler, or by a grandchild + that inherited fd 1 and outlived ``proc.kill()`` — cannot carry a matching id. It is **not** a + defence against the code executing the current request: that code is handed the id, and it could + equally just replace its sibling in the child's own registry. Confining one Handler from another + inside a shared worker is not a boundary this seam draws (``mode=off`` does not draw it either); + what it draws is the address-space boundary to the ENGINE, and the non-executing grammar below. + :meth:`sandbox.SandboxSession.dispatch` supplies the other half — an unsolicited frame queued + before or after a dispatch is fatal to the worker — so a pre-staged answer has no window at all.""" + header, reader, kind = _open(body, ("ok", "fail")) + if _req_str(header.get("id"), "response id") != request_id: + raise SandboxCodecError( + f"sandbox response correlation mismatch (expected request {request_id!r})" + ) + if _req_str(header.get("phase"), "response phase") != phase: + raise SandboxCodecError(f"sandbox response phase mismatch (expected {phase!r})") + if _req_str(header.get("name"), "response name") != name: + raise SandboxCodecError(f"sandbox response name mismatch (expected {name!r})") + if kind == "ok": + if "result" not in header: + raise SandboxCodecError("sandbox 'ok' frame carries no result") + result = dec_result(phase, header["result"], reader) + reader.finish() + return Response(ok=True, result=result, kind="", error="") + fail_kind = header.get("kind") + if fail_kind not in ("denied", "error"): + raise SandboxCodecError(f"unknown sandbox failure kind {fail_kind!r}") + error = reader.text(header.get("error")) + reader.finish() + return Response(ok=False, result=None, kind=fail_kind, error=error) diff --git a/messagefoundry/pipeline/_sandbox_worker.py b/messagefoundry/pipeline/_sandbox_worker.py index e2ef1042..6af42b12 100644 --- a/messagefoundry/pipeline/_sandbox_worker.py +++ b/messagefoundry/pipeline/_sandbox_worker.py @@ -3,19 +3,26 @@ """The sandbox worker child process (ADR 0087, BACKLOG #197). Launched by :class:`messagefoundry.pipeline.sandbox.SandboxSession` as ``python -m -messagefoundry.pipeline._sandbox_worker``. It speaks a tiny length-prefixed pickle protocol on -stdin/stdout (``sandbox._read_frame`` / ``_write_frame``): - -1. **Bootstrap** — reads one frame ``{config_dir, env, forbidden, cpu_seconds, mem_mb}``, loads the - message :class:`~messagefoundry.config.wiring.Registry` from ``config_dir`` (the same loader the - engine uses — it executes admin config under the unchanged safe-source gate), applies the POSIX - resource caps where available, installs the forbidden-import guard, and replies ``{ready: True}`` - (or ``{ready: False, error}`` on any failure). -2. **Serve** — for each subsequent request frame ``{phase, name, payload, run_context}`` it looks the - Router/Handler up in *its own* registry, re-establishes the run-scoped context providers for the - phase, runs the function on the unpickled payload, and replies ``{ok: True, result}`` — or - ``{ok: False, kind, error}`` on a denial (forbidden import, a live ``db_lookup``/``fhir_lookup``, - an unpicklable result) or a plain handler error. +messagefoundry.pipeline._sandbox_worker``. It speaks the length-prefixed **MFW2** codec +(:mod:`messagefoundry.pipeline._sandbox_codec`) on stdin/stdout — a closed-tag JSON+segment wire whose +decode path cannot name a type, import a module, or reach ``__reduce__``. Nothing is pickled in either +direction, in either process. + +1. **Bootstrap** — reads one ``boot`` frame ``{config_dir, forbidden, cpu_seconds, mem_mb, + code_sets}``, loads the message :class:`~messagefoundry.config.wiring.Registry` from ``config_dir`` + (the same loader the engine uses — it executes admin config under the unchanged safe-source gate), + **adopts the engine's code-set tables** from the frame in place of its own re-read of ``codesets/`` + (when the parent published any — a session constructed without them leaves the child on its own + load), applies the POSIX resource caps where available, installs the forbidden-import guard, and + replies ``ready`` (or ``bootfail`` on any failure). +2. **Serve** — for each subsequent ``req`` frame ``{id, phase, name, payload, rc}`` it rebuilds the + payload with the engine's own constructor, looks the Router/Handler up in *its own* registry, + re-establishes the run-scoped context providers for the phase (substituting the boot-carried code + sets, which the per-dispatch frame deliberately does not repeat), runs the function, and + **describes** the result back as ``ok`` — or ``fail`` with ``kind`` ``denied`` (forbidden import, a + live ``db_lookup``/``fhir_lookup``, an undescribable result) or ``error`` (a plain handler raise). + The reply echoes the request's ``id``, ``phase`` and ``name`` so the parent can prove the frame + answers the call it made. stdout is the binary IPC channel — **nothing else may write to it**. Logging and any diagnostics go to stderr (inherited by the engine). The engine parent enforces the wall-clock cap and kills a @@ -26,8 +33,8 @@ import logging import math -import pickle # nosec B403 — pickle only carries IPC frames between the engine and its own spawned sandbox worker, never external/untrusted input import sys +from dataclasses import replace from typing import Any # stdout is the IPC channel; keep the root logger on stderr so a stray log line can't corrupt a frame. @@ -46,7 +53,7 @@ def __init__(self, prefixes: tuple[str, ...]) -> None: self._prefixes = prefixes def find_spec(self, name: str, path: Any = None, target: Any = None) -> None: - from messagefoundry.pipeline.sandbox import SandboxError + from messagefoundry.pipeline._sandbox_codec import SandboxError for prefix in self._prefixes: if name == prefix or name.startswith(prefix + "."): @@ -86,18 +93,22 @@ def _install_import_guard(forbidden: tuple[str, ...]) -> None: sys.meta_path.insert(0, _ForbiddenImportFinder(forbidden)) -def _run_one(registry: Any, req: dict[str, Any]) -> dict[str, Any]: - """Execute one router/handler request and build the response dict (never raises).""" +def _run_one(registry: Any, req: Any, code_sets: Any) -> tuple[bool, object, str, str]: + """Execute one router/handler request; return ``(ok, result, kind, error)``. Never raises.""" from messagefoundry.config.db_lookup import DbLookupError from messagefoundry.config.fhir_lookup import FhirLookupError - from messagefoundry.config.run_context import RunContext, run_contexts - from messagefoundry.pipeline.sandbox import SandboxError + from messagefoundry.config.run_context import run_contexts + from messagefoundry.pipeline._sandbox_codec import SandboxError - phase = req.get("phase") - name = req.get("name") - payload = req.get("payload") - rc = req.get("run_context") - run_context = rc if isinstance(rc, RunContext) else RunContext() + phase = req.phase + name = req.name + # code_sets never travel per dispatch — the ENGINE's tables arrived once in the boot frame, so + # publish those. Falling back to this child's own load only happens when the parent published + # none at all (a session constructed without them); using the child's copy when the engine HAS + # one would let a `codesets/` edit made without a /config/reload diverge from mode=off. + run_context = replace( + req.run_context, code_sets=registry.code_sets if code_sets is None else code_sets + ) if phase == "router": fn = registry.routers.get(name) @@ -115,50 +126,82 @@ def _run_one(registry: Any, req: dict[str, Any]) -> dict[str, Any]: fn = registry.handler_accepts.get(name) phase_key = "router" else: - return {"ok": False, "kind": "error", "error": f"unknown sandbox phase {phase!r}"} + return False, None, "error", f"unknown sandbox phase {phase!r}" if fn is None: - return {"ok": False, "kind": "error", "error": f"no such {phase} {name!r} in registry"} + return False, None, "error", f"no such {phase} {name!r} in registry" try: with run_contexts(run_context, phase=phase_key): - result = fn(payload) + result = fn(req.payload) if phase == "accepts": # HandlerAccepts is contractually ``(msg) -> bool`` and the PARENT coerces the verdict with - # ``bool(...)`` (dryrun._accepted). Coerce HERE too, BEFORE the result is pickled back: a + # ``bool(...)`` (dryrun._accepted). Coerce HERE too, BEFORE the result is described back: a # predicate that returns a truthy NON-bool (a natural shape the parent's ``bool()`` sanctions, - # e.g. ``re.search(...)`` -> re.Match) would otherwise be marshalled raw and crash the child on - # an unpicklable object — content-dependent, since a non-match returns picklable None. Coercing - # to the contract type here makes ``[sandbox].mode`` never change the routing decision (ADR 0087). + # e.g. ``re.search(...)`` -> re.Match) would otherwise be rejected by the codec's strict + # JSON-bool slot — content-dependent, since a non-match returns None. Coercing to the + # contract type here is what lets the parent demand a strict bool, and makes + # ``[sandbox].mode`` never change the routing decision (ADR 0087). result = bool(result) except (DbLookupError, FhirLookupError) as exc: # db_lookup/fhir_lookup bridge back onto the engine event loop (run_coroutine_threadsafe), # which a subprocess boundary breaks — forbidden + fail-closed for this PR (ADR 0087). - return { - "ok": False, - "kind": "denied", - "error": f"{type(exc).__name__}: live db_lookup/fhir_lookup is forbidden inside the " + return ( + False, + None, + "denied", + f"{type(exc).__name__}: live db_lookup/fhir_lookup is forbidden inside the " "sandbox (ADR 0087) — run this Handler with [sandbox].mode=off if it needs live enrichment", - } + ) except SandboxError as exc: - return {"ok": False, "kind": "denied", "error": str(exc)} + return False, None, "denied", str(exc) except Exception as exc: # noqa: BLE001 — a handler raise is content, reported not crashed - return {"ok": False, "kind": "error", "error": f"{type(exc).__name__}: {exc}"} - return {"ok": True, "result": result} + return False, None, "error", f"{type(exc).__name__}: {exc}" + return True, result, "", "" + + +def _respond(registry: Any, req: Any, code_sets: Any) -> bytes: + """Build the response frame for one request. Never raises — an undescribable result is reported.""" + from messagefoundry.pipeline import _sandbox_codec as codec + from messagefoundry.pipeline._sandbox_codec import SandboxError + + ok, result, kind, error = _run_one(registry, req, code_sets) + if ok: + try: + return codec.encode_ok( + request_id=req.request_id, phase=req.phase, name=req.name, result=result + ) + except SandboxError as exc: + # A result outside the closed grammar (an exotic Send payload, a Handler returning an + # object that is not describable) — report it instead of dying so the worker survives for + # the next message. + return codec.encode_fail( + request_id=req.request_id, + phase=req.phase, + name=req.name, + kind="error", + error=f"unmarshallable result: {exc}", + ) + return codec.encode_fail( + request_id=req.request_id, phase=req.phase, name=req.name, kind=kind, error=error + ) def main() -> int: - from messagefoundry.pipeline.sandbox import SandboxError, _read_frame, _write_frame + from messagefoundry.pipeline import _sandbox_codec as codec + from messagefoundry.pipeline._sandbox_codec import SandboxError + from messagefoundry.pipeline.sandbox import _read_frame_bytes, _write_frame stdin = sys.stdin.buffer stdout = sys.stdout.buffer - boot = _read_frame(stdin) - if not isinstance(boot, dict): + frame = _read_frame_bytes(stdin) + if frame is None: return 0 # parent closed the pipe before bootstrap — nothing to do try: + boot = codec.decode_boot(frame) from messagefoundry.config.wiring import load_config - registry = load_config(boot["config_dir"]) + registry = load_config(boot.config_dir) # Pre-import every module the serve loop touches BEFORE the guard goes up, so a first-time # (transitive) import of an engine helper can't be misread as a forbidden user import. Once # cached in sys.modules, a later `import` short-circuits ahead of the meta_path finder. @@ -166,39 +209,48 @@ def main() -> int: import messagefoundry.config.fhir_lookup # noqa: F401 import messagefoundry.config.run_context # noqa: F401 - _apply_resource_caps(float(boot.get("cpu_seconds", 2.0)), boot.get("mem_mb")) - _install_import_guard(tuple(boot.get("forbidden", ()))) + _apply_resource_caps(boot.cpu_seconds, boot.mem_mb) + _install_import_guard(boot.forbidden) except Exception as exc: # noqa: BLE001 — report a bootstrap failure, do not crash silently try: # noqa: SIM105 - _write_frame(stdout, {"ready": False, "error": f"{type(exc).__name__}: {exc}"}) + _write_frame(stdout, codec.encode_bootfail(f"{type(exc).__name__}: {exc}")) except (OSError, SandboxError): pass return 1 try: - _write_frame(stdout, {"ready": True}) + _write_frame(stdout, codec.encode_ready()) except (OSError, SandboxError): return 1 while True: - req = _read_frame(stdin) - if req is None: + frame = _read_frame_bytes(stdin) + if frame is None: return 0 # parent closed the pipe — clean shutdown - if not isinstance(req, dict): - continue - resp = _run_one(registry, req) + try: + req = codec.decode_request(frame) + except SandboxError as exc: + # We have no trustworthy correlation id to answer with, so we cannot report this as a + # per-message error. Exit: the parent reads EOF, fails THIS message closed (SandboxError → + # dead-letter) and respawns a clean child, rather than hanging to the wall cap. + log.error("sandbox worker: undecodable request frame (%s)", exc) + return 1 + resp = _respond(registry, req, boot.code_sets) try: _write_frame(stdout, resp) - except (OSError, SandboxError, pickle.PicklingError, TypeError) as exc: - # A result that will not pickle (e.g. an exotic Send payload, or a Handler returning an - # unpicklable object) — report it instead of dying so the worker survives for the next - # message. pickle.dumps raises TypeError/PicklingError on an unmarshallable object; those are - # caught here (not just OSError/SandboxError) so the child's own "survives for the next - # message" contract actually holds — otherwise an unpicklable result kills the child and the - # parent reads EOF, dead-letters this message, and pays a full config-reload respawn next. + except (OSError, SandboxError) as exc: + # An over-cap response frame (a huge fan-out) raises in _write_frame, not at describe time — + # report it instead of dying so the child's "survives for the next message" contract holds, + # rather than the parent reading EOF and paying a full config-reload respawn. try: _write_frame( stdout, - {"ok": False, "kind": "error", "error": f"unmarshallable result: {exc}"}, + codec.encode_fail( + request_id=req.request_id, + phase=req.phase, + name=req.name, + kind="error", + error=f"unmarshallable result: {exc}", + ), ) except (OSError, SandboxError): return 1 diff --git a/messagefoundry/pipeline/dryrun.py b/messagefoundry/pipeline/dryrun.py index ba3d0aa2..9e2bd5de 100644 --- a/messagefoundry/pipeline/dryrun.py +++ b/messagefoundry/pipeline/dryrun.py @@ -44,6 +44,7 @@ summarize, validate, ) +from messagefoundry.pipeline._sandbox_codec import build_payload from messagefoundry.pipeline.sandbox import SandboxMode, SandboxSession, run_sandboxed from messagefoundry.store import MessageStatus @@ -123,10 +124,13 @@ def _payload(raw: str | bytes, content_type: str) -> Message | RawMessage: *mutates* its :class:`Message`, so every consumer must get its own parse (one parse per consumer is also cheaper than parse-once-then-deep-copy — python-hl7's parse beats deep-copying its nested list tree). A :class:`RawMessage` is read-only, so it is safe to *share* across consumers of the - same message (see :func:`_shareable_payload`).""" - if content_type == ContentType.HL7V2.value: - return Message.parse(raw) - return RawMessage(raw if isinstance(raw, str) else raw.decode("utf-8"), content_type) + same message (see :func:`_shareable_payload`). + + The construction itself lives in :func:`~messagefoundry.pipeline._sandbox_codec.build_payload` so + the sandbox worker child rebuilds the **same** object rather than a look-alike that could drift + (the child must not import this module — it would pull ``messagefoundry.store`` into the sandboxed + process, which the forbidden-import guard denies).""" + return build_payload(raw, content_type) def _shareable_payload(raw: str | bytes, content_type: str) -> Message | RawMessage | None: @@ -212,6 +216,7 @@ def _accepted( tracer: TraceHook | None, sandbox: SandboxSession | None, run_context: RunContext | None, + origin: tuple[str | bytes, str] | None = None, ) -> list[str]: """Drop the handlers whose ``accepts=`` predicate declines this message (ADR 0084). @@ -254,6 +259,7 @@ def _accepted( name=hname, run_context=run_context, session=sandbox, + origin=origin, ) ) elif tracer is not None: @@ -322,10 +328,19 @@ def route_only( handler-name validation below still runs **engine-side** on the returned result. """ route = registry.routers[ic.router] + # ADR 0087: only when WE derived the payload from `raw` may the sandbox child rebuild it from + # (raw, content_type) — a caller-supplied `payload` (route_message's shared RawMessage; public + # API) is described as the object it actually is, never assumed to equal _payload(raw, ct). + derived = payload is None if payload is None: payload = _payload(raw, ic.content_type.value) + # Built only on the sandboxed branch (`_accepted` reads it only there too), so `mode=off` keeps + # its "not even an allocation" promise rather than paying for a seam it never crosses. + origin: tuple[str | bytes, str] | None = None raw_result: list[str] | str | None if sandbox is not None and sandbox.mode is SandboxMode.SUBPROCESS: + if derived: + origin = (raw, ic.content_type.value) raw_result = cast( "list[str] | str | None", run_sandboxed( @@ -335,6 +350,7 @@ def route_only( name=ic.router, run_context=run_context, session=sandbox, + origin=origin, ), ) else: @@ -354,7 +370,13 @@ def route_only( if not registry.handler_accepts: return names return _accepted( - registry, names, payload, tracer=tracer, sandbox=sandbox, run_context=run_context + registry, + names, + payload, + tracer=tracer, + sandbox=sandbox, + run_context=run_context, + origin=origin, ) @@ -404,6 +426,8 @@ def transform_one( directly. The hook returns the Handler's result unchanged, so the deliveries are byte-identical. """ handle: HandlerFn = registry.handlers[hname] + # See route_only: `origin` is passed only when this call derived the payload itself. + derived = payload is None if payload is None: payload = _payload(raw, content_type) raw_result: Send | SetState | SetMeta | list[Send | SetState | SetMeta] | None @@ -417,6 +441,7 @@ def transform_one( name=hname, run_context=run_context, session=sandbox, + origin=(raw, content_type) if derived else None, ), ) else: diff --git a/messagefoundry/pipeline/sandbox.py b/messagefoundry/pipeline/sandbox.py index 588d94c6..41a92925 100644 --- a/messagefoundry/pipeline/sandbox.py +++ b/messagefoundry/pipeline/sandbox.py @@ -12,12 +12,30 @@ * ``off`` (default) — :func:`run_sandboxed` calls the Router/Handler **in-process**, byte-identically and with **zero** overhead (no subprocess, no marshalling). The isolation seam is invisible. * ``subprocess`` — the Router/Handler runs in a **persistent per-inbound worker subprocess** - (:mod:`messagefoundry.pipeline._sandbox_worker`). The parent marshals ``(phase, name, payload, - run_context)`` over a length-prefixed pickle pipe; the worker looks the function up in **its own** + (:mod:`messagefoundry.pipeline._sandbox_worker`). The parent marshals ``(id, phase, name, payload, + run_context)`` over a length-prefixed **non-executing** pipe codec + (:mod:`messagefoundry.pipeline._sandbox_codec`); the worker looks the function up in **its own** freshly-loaded :class:`~messagefoundry.config.wiring.Registry`, re-establishes the run-scoped - context providers, runs the function, and returns its raw result. The worker is *long-lived* (one - child per inbound), never a per-message fork — a fork per message would destroy the throughput - target. + context providers (over the ENGINE's code-set tables, which arrive once in the boot frame — the + child's own re-read of ``codesets/`` is never authoritative), runs the function, and describes its + result back. The worker is *long-lived* (one child per inbound), never a per-message fork — a fork + per message would destroy the throughput target. + +**Both legs of the pipe are untrusted by contract.** The child runs exactly the code the sandbox +exists to distrust, and a grandchild it spawns inherits fd 1 (the response pipe) and outlives +``proc.kill()``. So the wire is **MFW2**, a closed-tag JSON+segment codec whose decode path is +``json.loads`` plus ``bytes.decode`` and a literal tag match — it cannot name a type, import a module, +or reach ``__reduce__``. Nothing is pickled in either direction. + +**Frames answer requests, never the other way round.** Each dispatch mints a fresh +:func:`secrets.token_hex` request id and binds the whole ``(id, phase, name)`` triple on the way back, +and :meth:`SandboxSession.dispatch` treats a frame that is queued *before* a request — or left over +*after* its answer — as fatal to that worker. Together those close the window in which a pre-staged +frame could be read as the NEXT dispatch's answer (for ``phase="accepts"``, a routing-verdict flip on a +message the attacker never saw: no ``ERROR``, no disposition anomaly). What they do **not** do is +confine one Handler from another *inside* a worker: code running a dispatch is handed that dispatch's +id, and it could just as easily rebind a sibling in the child's own registry. ``mode=off`` draws no such +line either — the boundary this seam draws is to the **engine**, not between admin functions. **What isolation buys (and its honest limits).** The child is a *separate OS process*: even if the admin code opens a socket or spins the CPU, it cannot touch the parent's DEK, audit chain, or @@ -26,43 +44,51 @@ forbidden-import guard (``socket``/store/crypto), a wall-clock cap enforced by the parent (plus POSIX ``RLIMIT_CPU``/``RLIMIT_AS`` when available), and a fail-closed refusal of the live ``db_lookup``/``fhir_lookup`` bridges (they re-enter the event loop, which a process boundary -breaks — forwarding them over IPC is a documented next-phase residual). +breaks — forwarding them over IPC is a documented next-phase residual). The import guard is +**defence-in-depth only** — a module imported before it goes up keeps a live reference, so the +address-space boundary and the codec are the load-bearing controls. **Fail-closed.** Any isolation denial — a forbidden import/op, a resource cap exceeded, a worker -crash, or an unmarshallable payload/run-context — raises :class:`SandboxError`. The caller (the -router/transform worker) routes it to ``ERROR``/dead-letter **post-ACK** via the existing +crash, a rejected frame, or an unmarshallable payload/run-context — raises :class:`SandboxError`. The +caller (the router/transform worker) routes it to ``ERROR``/dead-letter **post-ACK** via the existing ``_apply_router_internal_error`` / ``_apply_transform_internal_error`` paths — never a NAK, never an accept-and-drop, never a crashed connection. -**Engine-side validation stays engine-side.** The worker returns only the *raw* Router/Handler -return value; the fail-closed handler-name / outbound-name validation (see -:func:`messagefoundry.pipeline.dryrun.route_only` / ``transform_one``) runs in the parent on that -result, so a compromised worker cannot smuggle an unknown destination past the graph. +**Engine-side validation stays engine-side.** The worker describes only the *shape* of the +Router/Handler return value; the fail-closed handler-name / outbound-name validation (see +:func:`messagefoundry.pipeline.dryrun.route_only` / ``transform_one``) runs in the parent on the +rebuilt result, so a compromised worker cannot smuggle an unknown destination past the graph. That +validation is now genuinely downstream of decoding: the decoder builds nothing but a closed set of +plain data types before it runs. **Layering (CLAUDE.md §4).** This is a pure ``pipeline/`` library — no ``api/`` or ``console/`` -imports. It depends only on ``config`` (the :class:`RunContext` shape) and the stdlib. +imports. It depends only on ``config`` (the :class:`RunContext` shape), its own codec, and the stdlib. """ from __future__ import annotations import enum import logging -import pickle # nosec B403 — pickle only carries IPC frames between the engine and its own spawned sandbox worker, never external/untrusted input import queue +import secrets import struct import subprocess import sys import threading from collections.abc import Callable, Mapping -from dataclasses import dataclass, replace +from dataclasses import dataclass from pathlib import Path -from typing import Any +from typing import Any, Final +from messagefoundry.config.code_sets import CodeSet from messagefoundry.config.run_context import RunContext +from messagefoundry.pipeline import _sandbox_codec as codec +from messagefoundry.pipeline._sandbox_codec import SandboxCodecError, SandboxError __all__ = [ "SandboxMode", "SandboxError", + "SandboxCodecError", "SandboxPolicy", "SandboxSession", "run_sandboxed", @@ -102,16 +128,9 @@ class SandboxMode(str, enum.Enum): # noqa: UP042 SUBPROCESS = "subprocess" # persistent per-inbound worker child -class SandboxError(RuntimeError): - """A sandboxed Router/Handler was denied, timed out, crashed, or could not be marshalled. - - Raised only on the ``subprocess`` path (``off`` never raises this). The caller treats it exactly - like any other post-ACK router/transform failure: ``ERROR``/dead-letter, no NAK.""" - - @dataclass(frozen=True) class SandboxPolicy: - """Resolved ``[sandbox]`` policy. Pure data (picklable) so the caps travel to the worker. + """Resolved ``[sandbox]`` policy. Pure data so the caps travel to the worker. ``mode=off`` (default) is the zero-overhead, byte-identical parity mode. ``wall_seconds`` is the **authoritative** cap on every platform — the parent kills a worker that overruns it (so a @@ -128,16 +147,34 @@ class SandboxPolicy: forbidden_modules: tuple[str, ...] = DEFAULT_FORBIDDEN_MODULES -# --- length-prefixed pickle framing over the worker pipe --------------------- +# --- length-prefixed framing over the worker pipe ---------------------------- +# The OUTER frame only. The body's schema (and every type that may cross) lives in _sandbox_codec. _LEN = struct.Struct(">I") -_MAX_FRAME = 64 * 1024 * 1024 # 64 MiB ceiling — a hostile frame length can't force a huge alloc +#: 64 MiB ceiling — a hostile frame length can't force a huge alloc. Sourced from the codec so the +#: outer framing and the body decoder's own header bound are the SAME number by construction. +_MAX_FRAME: Final = codec.MAX_FRAME + + +class _Eof: + """The dead-peer sentinel the reader thread enqueues on EOF. + + A parent-PRIVATE singleton with **no wire representation**: the old ``{"__eof__": True}`` frame was + structurally indistinguishable from one the child could forge, handing a compromised worker a free + kill-and-respawn lever (which also reset its own state).""" + __slots__ = () -def _write_frame(stream: Any, obj: object) -> None: - """Pickle ``obj`` and write it length-prefixed. Raises on an unpicklable object (fail-closed) or a - broken pipe; the caller maps either to :class:`SandboxError`.""" - body = pickle.dumps(obj, protocol=pickle.HIGHEST_PROTOCOL) + def __repr__(self) -> str: # pragma: no cover - diagnostics only + return "" + + +_EOF: Final = _Eof() + + +def _write_frame(stream: Any, body: bytes) -> None: + """Write one length-prefixed frame body. Raises on an over-cap frame (fail-closed) or a broken + pipe; the caller maps either to :class:`SandboxError`.""" if len(body) > _MAX_FRAME: raise SandboxError(f"sandbox frame too large: {len(body)} bytes") stream.write(_LEN.pack(len(body))) @@ -158,21 +195,20 @@ def _read_exact(stream: Any, n: int) -> bytes | None: return b"".join(chunks) -def _read_frame(stream: Any) -> Any: - """Read one length-prefixed pickled frame, or ``None`` on EOF. Sentinel used to signal a dead peer.""" +def _read_frame_bytes(stream: Any) -> bytes | None: + """Read one length-prefixed frame **body**, or ``None`` on EOF / a dead peer. + + Deliberately does **no** decoding: on the parent this runs on the daemon reader thread, whose + ``except`` catches only ``OSError`` — a rejection raised there would kill the reader silently and + the dispatch would HANG to the wall cap instead of failing closed. Decoding happens on the + dispatch thread, inside its existing ``try``.""" header = _read_exact(stream, _LEN.size) if header is None: return None (length,) = _LEN.unpack(header) if length > _MAX_FRAME: return None # a corrupt/hostile length — treat as a dead peer - body = _read_exact(stream, length) - if body is None: - return None - # `body` is an IPC frame read from a private pipe whose other end is our own - # spawned sandbox worker (child) / engine (parent) — never external/untrusted - # data — so the pickle-deserialization risk (B301 / semgrep) does not apply here. - return pickle.loads(body) # nosec B301 # nosemgrep: mf-no-insecure-deserialization + return _read_exact(stream, length) # --- the persistent worker session (parent side) ----------------------------- @@ -187,10 +223,24 @@ class SandboxSession: ``mode=off`` sessions never spawn anything — :meth:`dispatch` isn't called on them (the caller branches on :attr:`mode`).""" - def __init__(self, policy: SandboxPolicy, *, config_dir: str | Path, env: str | None) -> None: + def __init__( + self, + policy: SandboxPolicy, + *, + config_dir: str | Path, + env: str | None, + code_sets: Mapping[str, CodeSet] | None = None, + ) -> None: self.policy = policy self._config_dir = str(Path(config_dir)) + # Kept for the caller's signature (engine.py resolves and passes it), but NOT marshalled: the + # worker never read it — `load_config()` takes only a directory — and a dead field on the wire + # is a field nobody validates. self._env = env + # The ENGINE's live code-set tables, sent once per spawn in the boot frame so the child serves + # exactly what mode=off would rather than its own re-read of `codesets/` (see + # codec.enc_code_sets). `None` = the caller published none, so the child keeps its own load. + self._code_sets = code_sets self._proc: subprocess.Popen[bytes] | None = None self._responses: queue.Queue[Any] = queue.Queue() self._lock = threading.Lock() @@ -225,45 +275,50 @@ def _spawn(self) -> None: try: _write_frame( proc.stdin, - { - "config_dir": self._config_dir, - "env": self._env, - "forbidden": list(self.policy.forbidden_modules), - "cpu_seconds": self.policy.cpu_seconds, - "mem_mb": self.policy.mem_mb, - }, + codec.encode_boot( + config_dir=self._config_dir, + forbidden=self.policy.forbidden_modules, + cpu_seconds=self.policy.cpu_seconds, + mem_mb=self.policy.mem_mb, + code_sets=self._code_sets, + ), ) except (OSError, SandboxError) as exc: self._kill(proc) raise SandboxError(f"failed to bootstrap sandbox worker: {exc}") from exc try: - ready = self._responses.get(timeout=self.policy.startup_seconds) + frame = self._responses.get(timeout=self.policy.startup_seconds) except queue.Empty: self._kill(proc) raise SandboxError( f"sandbox worker did not start within {self.policy.startup_seconds}s" ) from None - if isinstance(ready, dict) and ready.get("__eof__"): + if frame is _EOF: self._kill(proc) raise SandboxError("sandbox worker exited during bootstrap") - if not (isinstance(ready, dict) and ready.get("ready")): + try: + ready, detail = codec.decode_boot_reply(frame) + except SandboxError as exc: + self._kill(proc) + raise SandboxError(f"sandbox worker bootstrap frame was rejected: {exc}") from exc + if not ready: self._kill(proc) - detail = ready.get("error") if isinstance(ready, dict) else repr(ready) raise SandboxError(f"sandbox worker bootstrap failed: {detail}") self._proc = proc def _reader_loop(self, stdout: Any, sink: queue.Queue[Any]) -> None: - """Drain the child's stdout into ``sink``. Runs on a daemon thread; a fresh queue per spawn - means a stale reader's writes are harmlessly ignored.""" + """Drain the child's stdout into ``sink`` — **framing only**, never decoding (see + :func:`_read_frame_bytes`). Runs on a daemon thread; a fresh queue per spawn means a stale + reader's writes are harmlessly ignored.""" try: while True: - frame = _read_frame(stdout) - if frame is None: - sink.put({"__eof__": True}) + body = _read_frame_bytes(stdout) + if body is None: + sink.put(_EOF) return - sink.put(frame) + sink.put(body) except OSError: - sink.put({"__eof__": True}) + sink.put(_EOF) def _kill(self, proc: subprocess.Popen[bytes] | None) -> None: if proc is None: @@ -285,34 +340,104 @@ def close(self) -> None: self._closed = True self._kill(self._proc) + def _reject_unsolicited(self, proc: subprocess.Popen[bytes] | None, when: str) -> None: + """Drop the worker if anything is pending that no outstanding request asked for. + + The protocol is strictly one request, one frame, so a FRAME queued **before** a dispatch or + left over **after** its answer was written by something other than the call we made — a + Handler writing straight to fd 1, or a grandchild that inherited it and outlived + ``proc.kill()``. Letting such a frame sit in the queue is the whole exploit: the next dispatch + would take it as its own answer. It is not an authoring accident either — ``print()`` goes + through the text wrapper, not the frame writer — so there is no benign case to preserve. Drop + the worker and dead-letter the message in hand. + + :data:`_EOF` is the opposite case and must NOT be treated the same way. It is a parent-private + singleton with no wire form (see :class:`_Eof`), so a worker cannot manufacture one — it + carries no trust information at all, only "the peer is gone". Dead-lettering on it would fail + closed against a signal that proves nothing: a child that crashes in the instant *after* + writing a correct, correlation-proven answer would lose a message it had already answered, and + a child that died between dispatches would lose the next one instead of respawning + transparently. Reap it and let the caller carry on.""" + try: + stray = self._responses.get_nowait() + except queue.Empty: + return + self._kill(proc) + if stray is _EOF: + return + raise SandboxError( + f"sandbox worker wrote an unsolicited response frame {when} a dispatch — " + "the pipe is not trustworthy" + ) + + def _live_worker(self) -> subprocess.Popen[bytes]: + """The session's worker — spawned or respawned as needed — on a pipe nothing is waiting on. + + The unsolicited check runs **after** the liveness check, not before: a killed worker's queue is + discarded wholesale by :meth:`_spawn`, so its leftovers are neither reachable nor evidence + about the *new* child. Only a REUSED worker can hand us a leftover, and only there is one worth + judging. A leftover EOF reaps it (see :meth:`_reject_unsolicited`), so spawn once more rather + than dead-letter this message on a peer that merely died.""" + if self._proc is None or self._proc.poll() is not None: + self._spawn() + self._reject_unsolicited(self._proc, "before") + if self._proc is None: + self._spawn() + proc = self._proc + assert proc is not None + return proc + # -- dispatch ------------------------------------------------------------- - def dispatch(self, phase: str, name: str, payload: object, run_context: RunContext) -> object: - """Run ``name`` on ``payload`` in the worker; return its raw result. + def dispatch( + self, + phase: str, + name: str, + payload: object, + run_context: RunContext, + *, + origin: tuple[str | bytes, str] | None = None, + ) -> object: + """Run ``name`` on ``payload`` in the worker; return its rebuilt result. ``phase`` is ``"router"``, ``"transform"``, or ``"accepts"`` (ADR 0084) — for ``"accepts"``, ``name`` keys the HANDLER whose predicate is being run and the worker re-establishes the ROUTER - run-context phase (a predicate is a router-stage peek). Serialized against concurrent callers. Any - isolation fault (crash / timeout / denial / marshalling failure) raises :class:`SandboxError`; - a plain, still-alive worker survives a *denied* call so the next message reuses it.""" + run-context phase (a predicate is a router-stage peek). ``origin`` is the ``(raw, + content_type)`` the caller derived ``payload`` from, when it did; the child then rebuilds a + byte-faithful object with the identical constructor. Serialized against concurrent callers. Any + isolation fault (crash / timeout / denial / codec rejection) raises :class:`SandboxError`; + a plain, still-alive worker survives a *denied* call so the next message reuses it. The one + thing it does **not** survive is writing a frame nobody asked for + (see :meth:`_reject_unsolicited`) — that is a lost worker and a dead-lettered message.""" with self._lock: if self._closed: raise SandboxError("sandbox session is closed") - if self._proc is None or self._proc.poll() is not None: - self._spawn() - proc = self._proc - assert proc is not None and proc.stdin is not None + proc = self._live_worker() + assert proc.stdin is not None + # A FRESH unpredictable id per dispatch, not a counter: the worker learns it only when it + # reads this request frame, which the check above proves is the first thing it can answer. + # A derivable id (a per-spawn nonce plus a sequence) let the code running dispatch N + # compute N+1's and pre-stage its answer. + request_id = secrets.token_hex(16) try: _write_frame( proc.stdin, - {"phase": phase, "name": name, "payload": payload, "run_context": run_context}, + codec.encode_request( + request_id=request_id, + phase=phase, + name=name, + payload=payload, + origin=origin, + run_context=run_context, + ), ) - except (OSError, SandboxError, pickle.PicklingError, TypeError) as exc: - # Unpicklable payload/run-context, or a broken pipe: fail closed and reset the worker. + except (OSError, SandboxError) as exc: + # A payload/run-context outside the closed grammar, or a broken pipe: fail closed and + # reset the worker. self._kill(proc) raise SandboxError(f"failed to marshal sandbox {phase} {name!r}: {exc}") from exc try: - resp = self._responses.get(timeout=self.policy.wall_seconds) + frame = self._responses.get(timeout=self.policy.wall_seconds) except queue.Empty: # Wall cap exceeded — the authoritative resource bound on every platform. Kill the # runaway child (a busy-loop can't wedge intake) and fail closed. @@ -320,44 +445,28 @@ def dispatch(self, phase: str, name: str, payload: object, run_context: RunConte raise SandboxError( f"sandbox {phase} {name!r} exceeded the {self.policy.wall_seconds}s wall cap" ) from None - if isinstance(resp, dict) and resp.get("__eof__"): + if frame is _EOF: self._kill(proc) raise SandboxError(f"sandbox worker crashed while running {phase} {name!r}") - if not (isinstance(resp, dict) and resp.get("ok")): - detail = resp.get("error") if isinstance(resp, dict) else repr(resp) - raise SandboxError(f"sandbox denied {phase} {name!r}: {detail}") - return resp["result"] - - -def _snapshot_view(view: Any) -> Any: - """Coerce one run-scoped view to a picklable point-in-time snapshot for the worker pipe. - - The engine builds ``reference_view``/``state_view`` (and, later, ``response_view``) as live - :class:`types.MappingProxyType` windows onto the store's caches - (:meth:`Store.reference_view`/:meth:`Store.state_view`) — and a ``mappingproxy`` is **not** - picklable, so a live view would fail-closed at marshal time (dead-lettering every message) rather - than reach the worker. Snapshot it to a plain ``dict`` (both levels of the ``{name: {key: value}}`` - reference view), which is exactly the read-only content the router/transform would have seen at - this instant in-process — and re-run stability (CLAUDE.md §2) makes a point-in-time copy the - contract anyway. Non-mapping / already-picklable views (``None``, a plain ``dict``) pass through - unchanged (a plain ``dict`` is still copied so the worker can't observe a later parent mutation).""" - if isinstance(view, Mapping): - return {k: (dict(v) if isinstance(v, Mapping) else v) for k, v in view.items()} - return view - - -def _picklable_run_context(rc: RunContext) -> RunContext: - """Return a marshalling-safe copy of ``rc`` with its live store views snapshotted to plain dicts. - - The mapping fields the engine fills from the live store are the only unpicklable members; the - scalar fields and ``code_sets`` (a ``{name: CodeSet}`` of ``__slots__`` mappings) already pickle, - so they pass through :func:`dataclasses.replace` unchanged.""" - return replace( - rc, - reference_view=_snapshot_view(rc.reference_view), - state_view=_snapshot_view(rc.state_view), - response_view=_snapshot_view(rc.response_view), - ) + try: + # Decoding happens HERE, on the dispatch thread, inside this try — not on the daemon + # reader thread, where a rejection would be a silent reader death and a wall-cap hang. + resp = codec.decode_response(frame, request_id=request_id, phase=phase, name=name) + except SandboxError as exc: + # A desynchronized or forged frame: the worker's stream can no longer be trusted to + # line up with our requests, so drop it and respawn on the next message. + self._kill(proc) + raise SandboxError( + f"sandbox {phase} {name!r} returned an invalid frame: {exc}" + ) from exc + # One request, one frame: a second FRAME already queued means the worker is writing frames + # nobody asked for, so this answer is not trustworthy either. (A queued EOF is only a dead + # peer — it reaps the worker and leaves this proven answer standing.) + self._reject_unsolicited(proc, "after") + if not resp.ok: + verdict = "denied" if resp.kind == "denied" else "failed" + raise SandboxError(f"sandbox {verdict} {phase} {name!r}: {resp.error}") + return resp.result def run_sandboxed( @@ -368,16 +477,20 @@ def run_sandboxed( name: str, run_context: RunContext | None, session: SandboxSession | None, + origin: tuple[str | bytes, str] | None = None, ) -> object: - """Run ``fn`` on ``payload`` under the isolation policy of ``session`` and return its raw result. + """Run ``fn`` on ``payload`` under the isolation policy of ``session`` and return its result. With ``session is None`` or ``session.mode is OFF`` this is exactly ``fn(payload)`` — in-process, - byte-identical, zero overhead (the parity default). With ``session.mode is SUBPROCESS`` the call - is marshalled to the persistent worker via :meth:`SandboxSession.dispatch`, enforcing the - forbidden-import / resource caps and raising :class:`SandboxError` on any violation. The live - ``RunContext`` is snapshotted to a picklable form first (:func:`_picklable_run_context`) so the - store-backed ``MappingProxyType`` views the engine always passes cross the process boundary.""" + byte-identical, zero overhead (the parity default); nothing above that branch touches ``payload``, + ``origin``, or the codec. With ``session.mode is SUBPROCESS`` the call is marshalled to the + persistent worker via :meth:`SandboxSession.dispatch`, enforcing the forbidden-import / resource + caps and raising :class:`SandboxError` on any violation. + + ``origin`` is the ``(raw, content_type)`` the caller built ``payload`` from — pass it **only** when + the caller derived the payload itself, so the child can rebuild it byte-faithfully; ``None`` makes + the codec describe the caller's actual object instead.""" if session is None or session.mode is SandboxMode.OFF: return fn(payload) rc = run_context if run_context is not None else RunContext() - return session.dispatch(phase, name, payload, _picklable_run_context(rc)) + return session.dispatch(phase, name, payload, rc, origin=origin) diff --git a/messagefoundry/pipeline/wiring_runner.py b/messagefoundry/pipeline/wiring_runner.py index 46515da5..1c96ec4b 100644 --- a/messagefoundry/pipeline/wiring_runner.py +++ b/messagefoundry/pipeline/wiring_runner.py @@ -2532,7 +2532,11 @@ def _sandbox_for(self, name: str) -> SandboxSession | None: session = self._sandbox_sessions.get(name) if session is None: env = self._sandbox_config_source[1] if self._sandbox_config_source else None - session = SandboxSession(policy, config_dir=cfg_dir, env=env) + # The engine's code-set tables travel once per spawn in the boot frame (not per dispatch), + # so the child serves exactly what mode=off would rather than its own re-read of codesets/. + session = SandboxSession( + policy, config_dir=cfg_dir, env=env, code_sets=self.registry.code_sets + ) self._sandbox_sessions[name] = session return session diff --git a/messagefoundry/store/store.py b/messagefoundry/store/store.py index 30ff15c0..a846c233 100644 --- a/messagefoundry/store/store.py +++ b/messagefoundry/store/store.py @@ -56,6 +56,11 @@ import aiosqlite from messagefoundry.config.models import RetryPolicy + +# The captured-reply record lives in the (store-free) config layer so a sandbox worker child — which +# may not import `messagefoundry.store` — can rebuild the SAME class the engine publishes. Re-exported +# here so every existing `from messagefoundry.store.store import CapturedResponse` keeps working. +from messagefoundry.config.response import CapturedResponse as CapturedResponse # re-export from messagefoundry.config.settings import StoreBackend from messagefoundry.parsing.binary import strip_documents as _strip_documents from messagefoundry.redaction import safe_text @@ -536,30 +541,6 @@ class ClaimedHeads: rearm: frozenset[str] -@dataclass(frozen=True) -class CapturedResponse: - """One captured request/response reply (ADR 0013), as returned by ``correlate_response`` for the - API/console read surface. ``body``/``detail`` are decrypted here; ``body`` is ``None`` once - retention has nulled it (the row is kept, like a purged ``messages.raw``).""" - - message_id: str - destination_name: str - response_seq: int - outcome: str - detail: str | None - captured_at: float - body: str | None - # ADR 0021 "Response Sent": 'response' (outbound reply, the default) | 'ack_sent' (inbound ACK we - # returned). ack_code/ack_phase are non-PHI disposition metadata, populated only for ack_sent rows. - kind: str = "response" - ack_code: str | None = None - ack_phase: str | None = None - # BACKLOG #154 (ADR 0013 amendment): the captured allow-listed HTTP response headers ({} when none / - # once retention nulls them). Decrypted + JSON-decoded here; a re-ingressed Handler reads it via - # response_get(dest).headers. - headers: Mapping[str, str] = field(default_factory=dict) - - @dataclass(frozen=True) class ReplyWaitState: """One metadata-only snapshot for a synchronous-reply wait tick (ADR 0154 D3). diff --git a/scripts/security/crypto_inventory_check.py b/scripts/security/crypto_inventory_check.py index 55ff4e07..d38b935a 100644 --- a/scripts/security/crypto_inventory_check.py +++ b/scripts/security/crypto_inventory_check.py @@ -189,6 +189,13 @@ # shards disagree on a lane's owner). Deterministic placement, not a security control, no secret # material involved. "messagefoundry/pipeline/sharding.py": frozenset({"hashlib"}), + # ADR 0087 (#197) — secrets = a fresh 16-byte token_hex per DISPATCH, carried as that call's + # request id and bound on the way back. This IS a security control: a grandchild the sandboxed + # Handler spawns inherits fd 1 (the response pipe) and outlives the worker's kill, so a DERIVABLE + # id (a per-spawn nonce plus a counter) would let the code running one call pre-stage the answer to + # the next — for an `accepts=` predicate, a routing-verdict flip with no ERROR and no disposition + # anomaly. Unpredictability is the whole property, hence secrets rather than random. + "messagefoundry/pipeline/sandbox.py": frozenset({"secrets"}), # ADR 0049 (#60): the .mfbak DR-backup archive codec — a chunked AES-256-GCM streaming framing # (cryptography AESGCM) keyed by the existing store DEK, with a SHA-256 (hashlib) header digest bound # as per-frame AAD + the one-way key_id fingerprint. Net-new crypto surface; the store DEK key source diff --git a/tests/test_copy_on_send.py b/tests/test_copy_on_send.py index 2fda9fb3..6fc0f2f3 100644 --- a/tests/test_copy_on_send.py +++ b/tests/test_copy_on_send.py @@ -7,12 +7,12 @@ the caller's reference and a divergent fan-out collapses to last-write, exactly as before), and the propagation guards: the run-context provider activates only in the transform phase, all three transform-phase ``RunContext`` builders thread the flag (the fused silent-miss backstop), the sandbox -marshalling carries the scalar, and a snapshotted ``Send.message`` is picklable on both backends. +marshalling carries the scalar, and a snapshotted ``Send.message`` survives the sandbox IPC codec on +both backends. """ from __future__ import annotations -import pickle from pathlib import Path import pytest @@ -33,8 +33,15 @@ ) from messagefoundry.parsing._backend import backend from messagefoundry.parsing.message import Message +from messagefoundry.pipeline._sandbox_codec import ( + _Blobs, + _Reader, + dec_result, + dec_run_context, + enc_result, + enc_run_context, +) from messagefoundry.pipeline.dryrun import dry_run, transform_one -from messagefoundry.pipeline.sandbox import _picklable_run_context _ADT = ( "MSH|^~\\&|SENDA|SENDF|RECV|RFAC|20200101||ADT^A01^ADT_A01|MSG1|P|2.5\r" @@ -174,24 +181,37 @@ def test_all_three_transform_builders_thread_the_flag() -> None: assert src.count("snapshot_on_send=self._snapshot_on_send") == 3 -def test_picklable_run_context_carries_flag() -> None: - """AC-4: the sandbox marshalling copy carries the scalar ``snapshot_on_send`` to the child unchanged - (no ``sandbox.py`` edit needed — it rides ``dataclasses.replace``).""" - assert _picklable_run_context(RunContext(snapshot_on_send=True)).snapshot_on_send is True - assert _picklable_run_context(RunContext()).snapshot_on_send is False +def _rc_round_trip(rc: RunContext) -> RunContext: + blobs = _Blobs() + return dec_run_context(enc_run_context(rc, blobs), _Reader(blobs.items)) + + +def test_marshalled_run_context_carries_flag() -> None: + """AC-4: the sandbox marshalling carries the scalar ``snapshot_on_send`` to the child unchanged. + Re-pointed at the MFW2 codec — the pickle path it used to assert no longer exists.""" + assert _rc_round_trip(RunContext(snapshot_on_send=True)).snapshot_on_send is True + assert _rc_round_trip(RunContext()).snapshot_on_send is False @pytest.mark.parametrize("builtin", [True, False], ids=["builtin", "python_hl7"]) -def test_snapshotted_send_message_is_picklable(builtin: bool) -> None: - """AC-4: a snapshotted ``Send.message`` survives the child→parent pickle round-trip on both backends - and still encodes identically (so ``send.message.encode()`` succeeds in the parent).""" +def test_snapshotted_send_message_survives_the_sandbox_codec(builtin: bool) -> None: + """AC-4: a snapshotted ``Send.message`` survives the child→parent marshalling on both backends and + still encodes identically (so ``send.message.encode()`` succeeds in the parent). + + Re-pointed from pickle to the MFW2 codec, which is the path the engine actually uses now; leaving + it on pickle would have kept asserting a route the engine no longer takes (a false green).""" with backend(builtin=builtin): msg = Message.parse(_ADT) with run_contexts(RunContext(snapshot_on_send=True), phase="transform"): send = Send("OB_A", msg) assert send.message is not msg - restored = pickle.loads(pickle.dumps(send, protocol=pickle.HIGHEST_PROTOCOL)) - assert restored.message.encode() == msg.encode() + blobs = _Blobs() + node = enc_result("transform", send, blobs) + restored = dec_result("transform", node, _Reader(blobs.items)) + # The codec carries the ENCODED text, so the parent's Send.message is already the str + # dryrun's `send.message if isinstance(..., str) else .encode()` would have produced. + assert isinstance(restored, Send) + assert restored.message == msg.encode() def test_provider_registered_before_unmapped_capture() -> None: diff --git a/tests/test_sandbox.py b/tests/test_sandbox.py index ea916f8b..783a3a62 100644 --- a/tests/test_sandbox.py +++ b/tests/test_sandbox.py @@ -5,25 +5,41 @@ Proves the crux parity property (``mode=off`` — and a benign ``subprocess`` round-trip — are byte-identical to a direct in-process call) plus the isolation guarantees: a forbidden op is denied, a runaway is capped without wedging intake, a sandboxed ``db_lookup``/``fhir_lookup`` fails closed, -and the marshalled :class:`RunContext` reaches the worker. Synthetic HL7 only.""" +the marshalled :class:`RunContext` reaches the worker, a ``__reduce__`` gadget returned by a Handler +never executes in the engine parent, a desynchronized/forged/pre-staged response frame can never +answer a later dispatch, and the engine's code-set tables — not the child's own re-read of +``codesets/`` — are what a sandboxed Handler resolves against. Synthetic HL7 only.""" from __future__ import annotations +import math +import queue import time from pathlib import Path from types import MappingProxyType +from typing import Any import pytest -from messagefoundry.config.run_context import RunContext +from messagefoundry.config.code_sets import CodeSet +from messagefoundry.config.response import CapturedResponse +from messagefoundry.config.run_context import RunContext, run_contexts from messagefoundry.config.wiring import Registry, load_config +from messagefoundry.pipeline import _sandbox_codec as codec +from messagefoundry.pipeline._sandbox_codec import ( + _Blobs, + _Reader, + dec_run_context, + enc_run_context, + encode_request, +) from messagefoundry.pipeline.dryrun import route_only, transform_one from messagefoundry.pipeline.sandbox import ( + _EOF, SandboxError, SandboxMode, SandboxPolicy, SandboxSession, - _picklable_run_context, ) from messagefoundry.store.store import MessageStore @@ -32,11 +48,13 @@ _GRAPH = """ from messagefoundry import ( - inbound, outbound, router, handler, MLLP, Send, - db_lookup, current_environment, reference, + inbound, outbound, router, handler, MLLP, Send, SetState, + db_lookup, current_environment, reference, response_get, ) inbound("IB_T", MLLP(port=19311), router="r") +inbound("IB_GEN", MLLP(port=19313), router="r_gen") +inbound("IB_MUT", MLLP(port=19315), router="r_mut") outbound("OB_T", MLLP(host="127.0.0.1", port=19312)) @@ -45,6 +63,94 @@ def r(msg): return "h_ok" +@router("r_mut") +def r_mut(msg): + # CONTRACT-VIOLATING on purpose (CLAUDE.md §2: routers must be pure). In-process the predicate + # below sees this mutation because it is handed the SAME object; across the pipe it cannot. + msg.set("MSH-6", "ROUTER_TOUCHED") + return ["h_sees_mutation"] + + +def _sees_mutation(msg): + return msg.field("MSH-6") == "ROUTER_TOUCHED" + + +@handler("h_sees_mutation", accepts=_sees_mutation) +def h_sees_mutation(msg): + return Send("OB_T", str(msg)) + + +@router("r_gen") +def r_gen(msg): + # A GENERATOR router is documented-supported (route_only routes the yielded values). It is not + # picklable, so before the codec it dead-lettered every message under mode=subprocess. + yield "h_ok" + + +class _Gadget: + \"\"\"A Handler return value whose __reduce__ writes to the filesystem of whichever process + deserializes it. Harmless under a non-executing codec; a full engine-parent escape under pickle.\"\"\" + + def __reduce__(self): + import os + + return (os.makedirs, (__SENTINEL__,)) + + +@handler("h_gadget") +def h_gadget(msg): + return _Gadget() + + +@handler("h_state") +def h_state(msg): + return [SetState("ns", "tup", (1, 2)), SetState("ns", "nan", float("nan"))] + + +@handler("h_reply") +def h_reply(msg): + rep = response_get("OB_T") + return Send("OB_T", rep.body if rep is not None else "NOREPLY") + + +@handler("h_forge") +def h_forge(msg): + # Write an EXTRA well-formed response frame straight onto fd 1 (the response pipe), then answer + # normally. Without correlation the parent would consume this pre-staged frame as the NEXT + # dispatch's answer. + import os + + from messagefoundry.pipeline import _sandbox_codec as _codec + + frame = _codec.encode_ok( + request_id="guess:1", phase="transform", name="h_ok", result=Send("OB_T", "FORGED") + ) + os.write(1, len(frame).to_bytes(4, "big") + frame) + return Send("OB_T", "REAL") + + +@handler("h_late_forge") +def h_late_forge(msg): + # The forgery that a correlation check ALONE cannot stop: stage the frame LATER, after this + # dispatch's own answer has been consumed, so it is sitting in the parent's queue when the NEXT + # dispatch starts. A grandchild that inherited fd 1 and outlived proc.kill() can do exactly this at + # any moment. The frame is addressed to a plausible next call (h_ok, transform) — only the id it + # cannot know. + import os + import threading + + from messagefoundry.pipeline import _sandbox_codec as _codec + + def _stage(): + frame = _codec.encode_ok( + request_id="guess:2", phase="transform", name="h_ok", result=Send("OB_T", "FORGED") + ) + os.write(1, len(frame).to_bytes(4, "big") + frame) + + threading.Timer(1.5, _stage).start() + return Send("OB_T", "REAL") + + @handler("h_ref") def h_ref(msg): # Reads a live reference snapshot the engine publishes via store.reference_view() — proving the @@ -83,9 +189,15 @@ def h_lookup(msg): """ +def _sentinel_path(config_dir: str | Path) -> Path: + """Where the ``__reduce__`` gadget would land if anything deserialized it.""" + return Path(config_dir) / "PWNED_IN_THE_PARENT" + + @pytest.fixture def graph(tmp_path: Path) -> tuple[Registry, str]: - (tmp_path / "graph.py").write_text(_GRAPH, encoding="utf-8") + source = _GRAPH.replace("__SENTINEL__", repr(str(_sentinel_path(tmp_path)))) + (tmp_path / "graph.py").write_text(source, encoding="utf-8") registry = load_config(tmp_path) return registry, str(tmp_path) @@ -270,24 +382,392 @@ async def test_subprocess_marshals_live_store_run_context( assert deliver == [("OB_T", "1")] # the snapshotted reference view reached and served the child -def test_picklable_run_context_snapshots_mappingproxy_views() -> None: - """Unit-level guard on the snapshot helper: it converts the store's unpicklable - ``MappingProxyType`` views (both levels of the reference view) to plain, picklable dicts while - preserving content, and leaves the scalar fields untouched.""" - import pickle +def _rc_round_trip(rc: RunContext) -> RunContext: + """Marshal a RunContext through the real codec (the shape the parent puts on the wire).""" + blobs = _Blobs() + node = enc_run_context(rc, blobs) + return dec_run_context(node, _Reader(blobs.items)) + +def test_run_context_codec_snapshots_mappingproxy_views() -> None: + """Unit-level guard on the run-context marshalling: the store's live ``MappingProxyType`` views + (both levels of the reference view) are snapshotted to plain dicts by construction, the + ``state_view``'s ``(namespace, key)`` TUPLE keys survive as tuples, and the scalar fields ride + through untouched. Replaces the old pickle round-trip — nothing is pickled any more.""" rc = RunContext( reference_view=MappingProxyType({"codes": MappingProxyType({"A": "1"})}), state_view=MappingProxyType({("ns", "k"): "v"}), active_environment="prod", + snapshot_on_send=True, ) - with pytest.raises(TypeError): - pickle.dumps(rc) # the live engine shape is DOA across the pipe - - safe = _picklable_run_context(rc) - round_tripped = pickle.loads(pickle.dumps(safe)) # now marshals - assert type(safe.reference_view) is dict and type(safe.reference_view["codes"]) is dict - assert type(safe.state_view) is dict + round_tripped = _rc_round_trip(rc) + assert type(round_tripped.reference_view) is dict + assert type(round_tripped.reference_view["codes"]) is dict + assert type(round_tripped.state_view) is dict assert round_tripped.reference_view == {"codes": {"A": "1"}} assert round_tripped.state_view == {("ns", "k"): "v"} + assert all(isinstance(k, tuple) for k in round_tripped.state_view) assert round_tripped.active_environment == "prod" + assert round_tripped.snapshot_on_send is True + # code_sets are deliberately NOT on the wire — the child substitutes its own bootstrap load. + assert round_tripped.code_sets is None + + +# --- the IPC boundary itself (MFW2 codec) ------------------------------------- + + +def _session(config_dir: str, **kw: object) -> SandboxSession: + return SandboxSession( + SandboxPolicy(mode=SandboxMode.SUBPROCESS, wall_seconds=15.0), + config_dir=config_dir, + env=None, + **kw, # type: ignore[arg-type] + ) + + +def test_a_handler_returning_a_reduce_gadget_does_not_execute_in_the_engine( + graph: tuple[Registry, str], +) -> None: + """The confirmed defect, end to end through a real spawned worker. + + ``h_gadget`` returns an object whose ``__reduce__`` creates a directory. Under the old pickle pipe + the ENGINE PARENT ran it while unpickling the response frame — a full address-space-boundary + bypass by admin-authored Handler code, which is the exact thing the sandbox exists to prevent. + Under MFW2 the gadget is an item ``_partition`` would ignore: it is described as ``{"o": "other"}`` + and rebuilt as an inert placeholder, so nothing runs and the message simply delivers nothing.""" + registry, config_dir = graph + sentinel = _sentinel_path(config_dir) + assert not sentinel.exists() + session = _session(config_dir) + try: + deliveries, state_ops, meta_ops, declined = transform_one( + registry, "h_gadget", RAW, sandbox=session, run_context=RunContext() + ) + # Parity with mode=off, which drops an unrecognised return value the same way. + assert (deliveries, state_ops, meta_ops, declined) == ([], [], [], []) + assert transform_one(registry, "h_gadget", RAW) == ( + deliveries, + state_ops, + meta_ops, + declined, + ) + # And the worker survived — a described-but-ignored result is not a fault. + assert _deliveries(registry, "h_ok", sandbox=session, run_context=RunContext()) + finally: + session.close() + assert not sentinel.exists(), "a Handler's __reduce__ executed in the engine parent" + + +def test_generator_router_routes_under_mode_subprocess(graph: tuple[Registry, str]) -> None: + """A generator Router is documented-supported and unpicklable, so ``mode=subprocess`` used to + dead-letter every message it routed. The child now materialises the router result with + ``_handler_names``' own logic, so it routes identically to ``mode=off``. This is a behaviour change + in the DELIVERING direction, hence an explicit test rather than a side effect.""" + registry, config_dir = graph + ic = registry.inbound["IB_GEN"] + assert route_only(registry, ic, RAW) == ["h_ok"] + session = _session(config_dir) + try: + assert route_only(registry, ic, RAW, sandbox=session, run_context=RunContext()) == ["h_ok"] + finally: + session.close() + + +def test_a_mutating_router_is_the_one_documented_mode_divergence( + graph: tuple[Registry, str], +) -> None: + """AC-15 — pins the single place ``[sandbox].mode`` is NOT transparent, so it cannot drift silently. + + Every dispatch marshals the payload, so the child rebuilds its own object; in-process the Router + and every ``accepts=`` predicate share ONE object. A Router that *mutates* its payload — which + CLAUDE.md §2 forbids — is therefore visible to the predicate under ``mode=off`` and invisible under + ``mode=subprocess``, and the two modes route differently. This predates the codec (the original + pickle pipe copied the payload per dispatch the same way); closing it would mean returning the + payload from every router/predicate dispatch, roughly doubling the routing stage's wire cost to + reproduce a documented authoring hazard. Asserted as an inequality **on purpose**: if a future + change makes the modes agree here, this test fails and the ADR residual gets deleted deliberately + rather than going stale.""" + registry, config_dir = graph + ic = registry.inbound["IB_MUT"] + assert route_only(registry, ic, RAW) == ["h_sees_mutation"] + session = _session(config_dir) + try: + assert route_only(registry, ic, RAW, sandbox=session, run_context=RunContext()) == [] + finally: + session.close() + + +def test_a_desynced_or_forged_response_is_rejected(graph: tuple[Registry, str]) -> None: + """``h_forge`` writes an extra well-formed response frame onto the inherited fd 1 before answering. + + Without correlation the parent would take that pre-staged frame as the answer to the NEXT dispatch + — for ``phase="accepts"`` that is a routing-verdict flip on a message the attacker never saw, the + one failure with no ERROR and no disposition anomaly. A per-dispatch ``secrets`` id makes the + forgery unguessable and the mismatch fatal to that worker. (The phase and name halves of the same + check are pinned at frame level by + ``test_sandbox_codec.py::test_hostile_frames_fail_closed[phase_mismatch|name_mismatch]``.)""" + registry, config_dir = graph + session = _session(config_dir) + try: + with pytest.raises(SandboxError, match="invalid frame"): + _deliveries(registry, "h_forge", sandbox=session, run_context=RunContext()) + # The desynchronized worker was killed; the next message gets a clean child and the right answer. + assert _deliveries(registry, "h_ok", sandbox=session, run_context=RunContext()) == ( + _deliveries(registry, "h_ok") + ) + finally: + session.close() + + +def test_a_frame_staged_between_dispatches_can_never_answer_the_next_one( + graph: tuple[Registry, str], +) -> None: + """The confirmed contract break: correlation ALONE does not stop a pre-staged answer. + + ``h_late_forge`` answers normally, then writes a second, perfectly well-formed ``ok`` frame a + moment LATER — so it is queued when the next, unrelated dispatch begins. A derivable request id + (a per-spawn nonce plus a counter) made that frame the next call's answer: a benign Handler's + delivery silently replaced by attacker-chosen content on a message it never saw, with no ERROR and + no disposition anomaly. Two things close it, and this test proves both: the id is now a fresh + ``secrets`` token the worker only learns from the request itself, and a frame queued before a + dispatch is fatal to the worker rather than consumed.""" + registry, config_dir = graph + session = _session(config_dir) + try: + # The forging dispatch itself succeeds — its own answer was honest. + assert _deliveries(registry, "h_late_forge", sandbox=session, run_context=RunContext()) == [ + ("OB_T", "REAL") + ] + time.sleep(2.5) # let the staged frame land in the parent's queue + # The next dispatch refuses to run against a pipe that already has a frame on it. + with pytest.raises(SandboxError, match="unsolicited"): + _deliveries(registry, "h_ok", sandbox=session, run_context=RunContext()) + # ...and the victim message is never delivered attacker-chosen content: a clean child answers. + assert _deliveries(registry, "h_ok", sandbox=session, run_context=RunContext()) == ( + _deliveries(registry, "h_ok") + ) + finally: + session.close() + + +def test_a_dead_peer_is_not_treated_as_a_forged_frame(graph: tuple[Registry, str]) -> None: + """The unsolicited-frame guard must fail closed on a FRAME and stay open on a corpse. + + A frame is the exploit: only a writer on fd 1 can produce one, so it is fatal. ``_EOF`` is a + parent-private singleton with **no wire form**, so a worker cannot manufacture one — it proves + only that the peer is gone. Conflating the two fails closed against a signal carrying no trust + information: a child that crashes in the instant *after* writing a correct, correlation-proven + answer would lose a message it had already answered.""" + registry, config_dir = graph + session = _session(config_dir) + try: + expected = _deliveries(registry, "h_ok", sandbox=session, run_context=RunContext()) + proc = session._proc + assert proc is not None + + # A dead peer: the worker is dropped (it is gone), but the answer in hand still stands. + session._responses.put(_EOF) + session._reject_unsolicited(proc, "after") # must NOT raise + assert session._proc is None + + # A frame in the very same slot is still fatal, and still named as unsolicited. + # Rebind the queue first, for the same reason `_spawn` does: the kill above makes the dead + # worker's reader thread hit EOF on a closed stdout and push a `_EOF` of its OWN. On a + # fast-teardown platform that lands BEFORE the frame planted here, so `_reject_unsolicited` + # — which drains exactly one item — would consume the benign corpse signal and correctly not + # raise, failing this assertion for a reason that has nothing to do with the frame. Windows + # tears down slowly enough to hide it; Linux does not. Dropping the dead generation's sink + # makes the planted frame provably the first item. + session._responses = queue.Queue() + session._responses.put(b"\x00\x00\x00\x02{}") + with pytest.raises(SandboxError, match="unsolicited"): + session._reject_unsolicited(session._proc, "after") + + # Either way the next message is served by a clean child, identically to mode=off. + assert _deliveries(registry, "h_ok", sandbox=session, run_context=RunContext()) == expected + finally: + session.close() + + +def test_request_ids_are_unpredictable_and_never_reused( + graph: tuple[Registry, str], monkeypatch: pytest.MonkeyPatch +) -> None: + """The id has to be unguessable to code that has already seen an earlier one — a per-spawn nonce + plus a counter let the Handler running dispatch N compute N+1's and pre-stage its answer.""" + registry, config_dir = graph + seen: list[str] = [] + real_encode = codec.encode_request + + def _spy(*, request_id: str, **kw: Any) -> bytes: + seen.append(request_id) + return real_encode(request_id=request_id, **kw) + + monkeypatch.setattr(codec, "encode_request", _spy) + session = _session(config_dir) + try: + for _ in range(3): + _deliveries(registry, "h_ok", sandbox=session, run_context=RunContext()) + finally: + session.close() + assert len(set(seen)) == 3 + assert all(len(rid) == 32 for rid in seen) # secrets.token_hex(16), not a counter + # No id is derivable from another: no shared prefix, no ordering relationship. + assert len({rid[:8] for rid in seen}) == 3 + + +def test_response_view_reaches_a_sandboxed_handler(graph: tuple[Registry, str]) -> None: + """New capability, not a regression guard: ``response_view`` carries ``CapturedResponse`` objects, + which used to live in ``messagefoundry.store`` — a FORBIDDEN import inside the child, raised from + inside its frame read (outside any ``try``). So ``[sandbox].mode=subprocess`` plus a LOOPBACK + inbound with a correlated reply was 100% non-functional. The record now lives in the store-free + config layer, so the child rebuilds the identical class.""" + registry, config_dir = graph + reply = CapturedResponse( + message_id="m1", + destination_name="OB_T", + response_seq=1, + outcome="ok", + detail=None, + captured_at=1.0, + body="MSA|AA|MSG00001", + headers={"x-trace": "abc"}, + ) + rc = RunContext(response_view={"OB_T": reply}) + session = _session(config_dir) + try: + deliver = _deliveries(registry, "h_reply", sandbox=session, run_context=rc) + finally: + session.close() + assert deliver == [("OB_T", "MSA|AA|MSG00001")] + # Identical to mode=off. In-process the CALLER activates the run-scoped providers (the sandbox + # child re-establishes them on the far side of the pipe), so bracket the comparison run the same way. + with run_contexts(rc, phase="transform"): + assert deliver == _deliveries(registry, "h_reply", run_context=rc) + + +def test_setstate_tuple_and_nonfinite_values_survive_mode_subprocess( + graph: tuple[Registry, str], +) -> None: + """Two parity gaps a plain-JSON codec would have shipped as silent narrowings: a tuple value + flattened to a list, and ``float('nan')`` (which ``SetState`` accepts today, because its validator + calls ``json.dumps`` with the default ``allow_nan=True``) turned into a fresh dead-letter.""" + registry, config_dir = graph + session = _session(config_dir) + try: + _, ops, _, _ = transform_one( + registry, "h_state", RAW, sandbox=session, run_context=RunContext() + ) + finally: + session.close() + by_key = {op.key: op.value for op in ops} + assert by_key["tup"] == (1, 2) and isinstance(by_key["tup"], tuple) + assert math.isnan(by_key["nan"]) + _, off_ops, _, _ = transform_one(registry, "h_state", RAW) + assert [(o.namespace, o.key) for o in ops] == [(o.namespace, o.key) for o in off_ops] + + +# --- the code-set hoist keeps the engine authoritative ------------------------ + + +def test_code_sets_reach_the_child_without_travelling_per_dispatch() -> None: + """The tables ride the one-per-spawn boot frame; the per-dispatch request frame must carry none of + their bytes (that redundancy was ~430 KB and ~4.6 ms per message).""" + name = "SECRET_CODE_SET_NAME" + rc = RunContext(code_sets={name: CodeSet(name, {"A": "SECRET_CODE_SET_VALUE"})}) + frame = encode_request( + request_id="n:1", + phase="router", + name="r", + payload=None, + origin=(RAW, "hl7v2"), + run_context=rc, + ) + assert b"SECRET_CODE_SET_VALUE" not in frame + assert name.encode() not in frame + + +def _codeset_graph(tmp_path: Path, mapped: str) -> Registry: + (tmp_path / "codesets").mkdir(exist_ok=True) + (tmp_path / "codesets" / "cs.csv").write_text(f"key,value\nA,{mapped}\n", encoding="utf-8") + (tmp_path / "graph.py").write_text( + "from messagefoundry import inbound, outbound, router, handler, MLLP, Send, code_set\n" + 'inbound("IB_C", MLLP(port=19331), router="r")\n' + 'outbound("OB_C", MLLP(host="127.0.0.1", port=19332))\n' + '@router("r")\n' + "def r(msg):\n" + ' return "h"\n' + '@handler("h")\n' + "def h(msg):\n" + ' return Send("OB_C", code_set("cs")["A"])\n', + encoding="utf-8", + ) + return load_config(tmp_path) + + +def test_code_sets_are_loaded_by_the_child_and_resolve(tmp_path: Path) -> None: + """The positive half of the hoist: a call-time ``code_set(...)`` inside a sandboxed Handler + resolves against the ENGINE's tables, with none of them on the per-dispatch wire.""" + registry = _codeset_graph(tmp_path, "MAPPED") + session = _session(str(tmp_path), code_sets=registry.code_sets) + try: + ds, _, _, _ = transform_one( + registry, + "h", + RAW, + sandbox=session, + run_context=RunContext(code_sets=registry.code_sets), + ) + finally: + session.close() + assert [(d.to, d.payload) for d in ds] == [("OB_C", "MAPPED")] + + +def test_an_unreloaded_codeset_edit_does_not_brick_the_inbound(tmp_path: Path) -> None: + """Regression for the fail-closed cure that was worse than the disease. + + Hoisting the tables to the child's OWN bootstrap load is fail-OPEN (an operator who edits + ``codesets/`` without a ``POST /config/reload`` would get a value the engine never published), and + pinning a digest to detect that turned it into a hard, permanent outage: every message on the + inbound dead-lettered forever after the next ROUTINE respawn (a wall-cap kill, a worker crash), + burning a full config load per attempt. Sending the engine's tables in the boot frame removes both + — the child cannot diverge because it is not the source. The sequence below is the real one: + dispatch, edit the file on disk with no reload, force a respawn, dispatch again.""" + registry = _codeset_graph(tmp_path, "ENGINE_VALUE") + session = _session(str(tmp_path), code_sets=registry.code_sets) + rc = RunContext(code_sets=registry.code_sets) + try: + first, _, _, _ = transform_one(registry, "h", RAW, sandbox=session, run_context=rc) + assert [(d.to, d.payload) for d in first] == [("OB_C", "ENGINE_VALUE")] + + # An operator edits the crosswalk and does NOT reload; then anything routine drops the worker. + (tmp_path / "codesets" / "cs.csv").write_text( + "key,value\nA,EDITED_ON_DISK\n", encoding="utf-8" + ) + session._kill(session._proc) # exactly what a wall-cap kill or a crash does + + second, _, _, _ = transform_one(registry, "h", RAW, sandbox=session, run_context=rc) + finally: + session.close() + # Still the engine's value, still delivering — byte-identical to mode=off, which reads the live + # registry the same way. + assert [(d.to, d.payload) for d in second] == [("OB_C", "ENGINE_VALUE")] + # In-process the CALLER activates the run-scoped providers (the sandbox child re-establishes them + # on the far side of the pipe), so bracket the comparison run the same way. + with run_contexts(rc, phase="transform"): + off, _, _, _ = transform_one(registry, "h", RAW, run_context=rc) + assert [(d.to, d.payload) for d in second] == [(d.to, d.payload) for d in off] + + +def test_the_engines_code_sets_win_over_the_childs_own_load(graph: tuple[Registry, str]) -> None: + """The same rule stated the other way: what the parent publishes is what the child serves, even + when the child's own ``load_config`` found different tables (here: none at all).""" + registry, config_dir = graph + assert registry.code_sets == {} # this graph ships none + session = _session(config_dir, code_sets={"cs": CodeSet("cs", {"A": "1"})}) + try: + # The bootstrap succeeds (no skew to detect) and the worker serves normally. + assert _deliveries(registry, "h_ok", sandbox=session, run_context=RunContext()) == ( + _deliveries(registry, "h_ok") + ) + finally: + session.close() diff --git a/tests/test_sandbox_codec.py b/tests/test_sandbox_codec.py new file mode 100644 index 00000000..a29bfb9c --- /dev/null +++ b/tests/test_sandbox_codec.py @@ -0,0 +1,865 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""MFW2 — the non-executing sandbox IPC codec (ADR 0087, BACKLOG #197). + +The regression suite for the pickle escape: the sandbox's whole purpose is an address-space boundary, +and a parent that ``pickle.loads`` the child's frame hands it straight back (a Handler returning an +object with a custom ``__reduce__`` executed arbitrary code IN THE ENGINE PARENT). These tests pin the +replacement wire: a closed value grammar, a strictly-sequential exactly-once segment discipline, and +fail-closed rejection of every malformed/hostile frame. Synthetic HL7 only.""" + +from __future__ import annotations + +import datetime as dt +import json +import math +import pickle +import struct +from collections.abc import Iterator +from types import MappingProxyType +from typing import Any + +import pytest + +from messagefoundry.config.code_sets import CodeSet, UnmappedKind, UnmappedPolicy +from messagefoundry.config.models import ContentType +from messagefoundry.config.response import CapturedResponse +from messagefoundry.config.run_context import RunContext, run_contexts +from messagefoundry.config.state import state_get +from messagefoundry.config.wiring import Send, SetMeta, SetState, WiringError +from messagefoundry.parsing.message import Message, RawMessage +from messagefoundry.pipeline import _sandbox_codec as codec +from messagefoundry.pipeline import sandbox +from messagefoundry.pipeline._sandbox_codec import ( + _MAX_DEPTH, + _MAX_HEADER, + MAX_FRAME, + Ignored, + SandboxCodecError, + SandboxError, + _Blobs, + _dec_item, + _Reader, + dec_code_sets, + dec_payload, + dec_result, + dec_run_context, + dec_value, + decode_boot, + decode_frame, + decode_request, + decode_response, + enc_code_sets, + enc_payload, + enc_result, + enc_run_context, + enc_value, + encode_boot, + encode_frame, + encode_ok, + encode_request, +) +from messagefoundry.pipeline.dryrun import _partition + +RAW = "MSH|^~\\&|SEND|F|RECV|F|20240101120000||ADT^A01|MSG00001|P|2.3\rPID|1||900001||DOE^JANE\r" + +# --- the __reduce__ gadget ---------------------------------------------------- + +#: Module-level tripwire. A gadget that executes appends to it; every test asserting "nothing ran" +#: asserts this list is still empty. +EXECUTED: list[str] = [] + + +def _detonate(tag: str) -> str: + EXECUTED.append(tag) + return tag + + +class Gadget: + """A perfectly ordinary object whose ``__reduce__`` runs code the moment anything unpickles it.""" + + def __reduce__(self) -> tuple[Any, ...]: + return (_detonate, ("pwned",)) + + +@pytest.fixture(autouse=True) +def _clear_tripwire() -> Iterator[None]: + EXECUTED.clear() + yield + EXECUTED.clear() + + +# --- frame helpers ------------------------------------------------------------ + + +def _body(header: Any, blobs: tuple[str, ...] = ()) -> bytes: + """A frame body from a JSON-able header object (bypasses the envelope builders on purpose).""" + return _body_text(json.dumps(header, ensure_ascii=False, separators=(",", ":")), blobs) + + +def _body_text(text: str, blobs: tuple[str, ...] = ()) -> bytes: + head = text.encode("utf-8", "surrogatepass") + parts = [struct.pack(">I", len(head)), head] + for blob in blobs: + raw = blob.encode("utf-8", "surrogatepass") + parts.append(struct.pack(">I", len(raw))) + parts.append(raw) + return b"".join(parts) + + +#: The handler/router name every frame in this module is built for. ``decode_response`` binds the whole +#: ``(id, phase, name)`` triple, so the tests have to name it too. +NAME = "h" + + +def _ok( + result: Any, phase: str = "transform", request_id: str = "r:1", name: str = NAME +) -> dict[str, Any]: + return {"v": 1, "t": "ok", "id": request_id, "phase": phase, "name": name, "result": result} + + +def _decode( + body: bytes, *, request_id: str = "r:1", phase: str = "transform", name: str = NAME +) -> Any: + """``decode_response`` with this module's default correlation triple.""" + return decode_response(body, request_id=request_id, phase=phase, name=name) + + +def _rt_transform(result: object) -> object: + blobs = _Blobs() + node = enc_result("transform", result, blobs) + return dec_result("transform", node, _Reader(blobs.items)) + + +# --- (1) THE REGRESSION TEST: a __reduce__ gadget never reaches the parent ----- + + +def test_a_reduce_gadget_never_reaches_the_parent() -> None: + """The confirmed defect, both halves. + + (a) CHILD side — describing a Handler return value never invokes ``__reduce__``: a gadget is an + item ``_partition`` would ignore, so it is described as ``{"o": "other"}`` and the parent rebuilds + an inert :class:`Ignored`. + + (b) PARENT side — the EXACT bytes the old child wrote (``pickle.dumps({"ok": True, "result": + gadget})``) are fed to both codecs. ``pickle.loads`` — which is literally what the old + ``sandbox._read_frame`` did on the reader thread, before any envelope inspection — DETONATES the + gadget in this process. ``decode_frame`` refuses the same bytes with the tripwire untouched.""" + # (a) describing the gadget executes nothing, and the parent rebuilds an inert placeholder. + blobs = _Blobs() + node = enc_result("transform", Gadget(), blobs) + assert node == {"r": "items", "shape": "one", "i": {"o": "other"}} + assert isinstance(dec_result("transform", node, _Reader(blobs.items)), Ignored) + assert EXECUTED == [] + + # (b) the exact frame body the OLD child produced for a gadget-returning Handler. + hostile = pickle.dumps({"ok": True, "result": Gadget()}, protocol=pickle.HIGHEST_PROTOCOL) + + # The old parent path, run here so the test is self-proving: this is the bug. + # nosec B301 — this load is the DEFECT being regression-tested, executed here on purpose against a + # locally-built gadget so the assertion below has something real to catch. Not a product path. + pickle.loads(hostile) # nosec B301 + assert EXECUTED == ["pwned"], "the gadget must actually be live for this test to mean anything" + EXECUTED.clear() + + # The new parent path refuses it and executes nothing. + with pytest.raises(SandboxCodecError): + decode_frame(hostile) + with pytest.raises(SandboxCodecError): + _decode(hostile) + assert EXECUTED == [] + + # A hostile OBJECT GRAPH inside an otherwise well-formed frame is refused too: no wire tag can + # name a module, a type, or a reduce callable. + for smuggled in ( + {"__reduce__": ["tests.test_sandbox_codec", "_detonate"]}, + {"py/object": "tests.test_sandbox_codec.Gadget"}, + {"o": "send", "to": "OB", "m": {"!!python/name": "os.system"}}, + ): + with pytest.raises(SandboxCodecError): + _decode(_body(_ok({"r": "items", "shape": "one", "i": smuggled}))) + assert EXECUTED == [] + + +def test_sandbox_codec_error_is_a_sandbox_error() -> None: + """So ``route_only``/``transform_one``'s documented "raises SandboxError" — and every existing + ``pytest.raises(SandboxError, ...)`` — keeps meaning what it says for a codec rejection.""" + assert issubclass(SandboxCodecError, SandboxError) + + +# --- (2) the decoder constructs only a closed set of types -------------------- + +_ALLOWED_TYPES = { + Send, + SetState, + SetMeta, + Ignored, + CapturedResponse, + Message, + RawMessage, + bool, + int, + float, + str, + bytes, + list, + tuple, + dict, + dt.datetime, + dt.date, + dt.time, + type(None), +} + + +def _walk_types(obj: object, seen: set[type]) -> None: + seen.add(type(obj)) + if isinstance(obj, dict): + for k, v in obj.items(): + _walk_types(k, seen) + _walk_types(v, seen) + elif isinstance(obj, list | tuple): + for item in obj: + _walk_types(item, seen) + elif isinstance(obj, Send): + _walk_types(obj.to, seen) + _walk_types(obj.message, seen) + elif isinstance(obj, SetState): + _walk_types(obj.value, seen) + elif isinstance(obj, CapturedResponse): + _walk_types(dict(obj.headers), seen) + _walk_types(obj.body, seen) + + +def test_decoder_constructs_only_the_closed_type_set() -> None: + corpus: list[object] = [] + + # a full transform result + corpus.append( + _rt_transform( + [ + Send("OB_A", "x" * 5000), + SetState("ns", "k", {"a": [1, 2.5, True, None, ("t",)]}), + SetMeta("mk", "mv"), + object(), + ] + ) + ) + corpus.append(_rt_transform(None)) + # a router + accepts result + for phase, result in (("router", ["h1", "h2"]), ("accepts", True)): + blobs = _Blobs() + corpus.append(dec_result(phase, enc_result(phase, result, blobs), _Reader(blobs.items))) + # a run context with all three views + rc = RunContext( + reference_view=MappingProxyType({"codes": MappingProxyType({"A": b"\x00\x01"})}), + state_view=MappingProxyType({("ns", "k"): [1, "two"]}), + response_view={ + "OB_A": CapturedResponse( + message_id="m1", + destination_name="OB_A", + response_seq=1, + outcome="ok", + detail=None, + captured_at=1.5, + body="MSH|", + kind="response", + ack_code="AA", + ack_phase="CA", + headers={"content-type": "text/plain"}, + ) + }, + active_environment="prod", + ingest_time=2.0, + message_id="m1", + snapshot_on_send=True, + ) + blobs = _Blobs() + decoded_rc = dec_run_context(enc_run_context(rc, blobs), _Reader(blobs.items)) + corpus.extend([decoded_rc.reference_view, decoded_rc.state_view, decoded_rc.response_view]) + # both payload forms + for origin in ((RAW, ContentType.HL7V2.value), (b"{}", "json")): + blobs = _Blobs() + corpus.append(dec_payload(enc_payload(None, origin, blobs), _Reader(blobs.items))) + blobs = _Blobs() + corpus.append(dec_payload(enc_payload(Message.parse(RAW), None, blobs), _Reader(blobs.items))) + + seen: set[type] = set() + for item in corpus: + _walk_types(item, seen) + assert seen <= _ALLOWED_TYPES, f"decoder produced unexpected types: {seen - _ALLOWED_TYPES}" + + +# --- (3) the segment discipline is exactly-once and strictly sequential ------- + +_BIG_A = "A" * 5000 +_BIG_B = "B" * 5000 + + +def _two_sends(ref_a: Any, ref_b: Any) -> dict[str, Any]: + return _ok( + { + "r": "items", + "shape": "list", + "i": [ + {"o": "send", "to": "OB_A", "m": ref_a}, + {"o": "send", "to": "OB_B", "m": ref_b}, + ], + } + ) + + +def test_sequential_segment_references_decode() -> None: + """The happy path the discipline has to keep working: two out-of-band bodies, in order.""" + resp = _decode(_body(_two_sends({"$": 0}, {"$": 1}), (_BIG_A, _BIG_B))) + sends, _, _ = _partition(resp.result) # type: ignore[arg-type] + assert [(s.to, s.message) for s in sends] == [("OB_A", _BIG_A), ("OB_B", _BIG_B)] + + +@pytest.mark.parametrize( + ("case", "body"), + [ + ("out_of_range", _body(_two_sends({"$": 0}, {"$": 7}), (_BIG_A, _BIG_B))), + ("duplicate", _body(_two_sends({"$": 0}, {"$": 0}), (_BIG_A, _BIG_B))), + ("skipped", _body(_two_sends({"$": 1}, {"$": 0}), (_BIG_A, _BIG_B))), + ("swapped", _body(_two_sends({"$": 1}, {"$": 0}), (_BIG_A, _BIG_B))), + ("unconsumed_trailing", _body(_two_sends("inline", "inline"), (_BIG_A, _BIG_B))), + ("partially_consumed", _body(_two_sends({"$": 0}, "inline"), (_BIG_A, _BIG_B))), + ("non_integer_ref", _body(_two_sends({"$": "0"}, {"$": 1}), (_BIG_A, _BIG_B))), + ("bool_ref", _body(_two_sends({"$": False}, {"$": 1}), (_BIG_A, _BIG_B))), + ], +) +def test_blob_reference_discipline_is_exactly_once_and_sequential(case: str, body: bytes) -> None: + """The PHI cross-wire guard. An indirection table's worst failure is silent: an off-by-one puts one + ``Send``'s body on another ``Send``'s destination with no error at all. References are legal only + at the cursor and every segment must be consumed, so each of these is a rejection **by + construction** rather than by test coverage — note ``swapped`` in particular is REFUSED, not + quietly cross-wired.""" + with pytest.raises(SandboxCodecError): + _decode(body) + + +# --- (4) hostile frames fail closed ------------------------------------------ + + +def _deep_json(depth: int) -> str: + return "[" * depth + "]" * depth + + +def _too_many_blobs() -> bytes: + header = json.dumps(_ok({"r": "items", "shape": "none"}), separators=(",", ":")).encode() + parts = [struct.pack(">I", len(header)), header] + parts.extend([struct.pack(">I", 0)] * 70000) + return b"".join(parts) + + +_HOSTILE: list[tuple[str, bytes]] = [ + ("truncated_outer", b"\x00\x00"), + ("truncated_header", struct.pack(">I", 4096) + b"{}"), + ("header_over_cap", struct.pack(">I", _MAX_HEADER + 1) + b"{}"), + ("too_many_segments", _too_many_blobs()), + ("truncated_segment", _body(_ok({"r": "items", "shape": "none"})) + struct.pack(">I", 99)), + ("header_not_an_object", _body([1, 2, 3])), + ( + "bad_version", + _body({"v": 2, "t": "ok", "id": "r:1", "phase": "transform", "name": NAME, "result": {}}), + ), + ("unknown_type", _body({"v": 1, "t": "nope", "id": "r:1"})), + ( + "ok_without_result", + _body({"v": 1, "t": "ok", "id": "r:1", "phase": "transform", "name": NAME}), + ), + ("id_mismatch", _body(_ok({"r": "items", "shape": "none"}, request_id="other:9"))), + ("phase_mismatch", _body(_ok({"r": "names", "n": []}, phase="router"))), + # The forgery that made the correlation check load-bearing: a frame for a DIFFERENT handler, + # correctly addressed to this request otherwise. Without the name binding it would be delivered as + # this call's answer (an attacker-chosen Send on a message the victim handler never saw). + ("name_mismatch", _body(_ok({"r": "items", "shape": "none"}, name="h_other"))), + ("missing_name", _body({"v": 1, "t": "ok", "id": "r:1", "phase": "transform", "result": {}})), + ( + "unknown_value_tag", + _body( + _ok( + { + "r": "items", + "shape": "one", + "i": {"o": "state", "ns": "n", "key": "k", "v": {"zz": 1}}, + } + ) + ), + ), + ( + "two_key_wrapper", + _body( + _ok( + { + "r": "items", + "shape": "one", + "i": {"o": "state", "ns": "n", "key": "k", "v": {"l": [], "p": []}}, + } + ) + ), + ), + ( + "bare_array_value", + _body( + _ok( + {"r": "items", "shape": "one", "i": {"o": "state", "ns": "n", "key": "k", "v": [1]}} + ) + ), + ), + ( + "unhashable_dict_key", + _body( + _ok( + { + "r": "items", + "shape": "one", + "i": {"o": "state", "ns": "n", "key": "k", "v": {"d": [[{"l": []}, 1]]}}, + } + ) + ), + ), + ( + "invalid_base64", + _body( + _ok( + { + "r": "items", + "shape": "one", + "i": {"o": "state", "ns": "n", "key": "k", "v": {"b": "!!!!"}}, + } + ) + ), + ), + ("deep_nesting", _body_text(_deep_json(100000))), + ( + "literal_nan", + _body_text( + '{"v":1,"t":"ok","id":"r:1","phase":"transform","x":NaN,"result":{"r":"items","shape":"none"}}' + ), + ), + ( + "non_str_error", + _body( + { + "v": 1, + "t": "fail", + "id": "r:1", + "phase": "transform", + "name": NAME, + "kind": "error", + "error": 12, + } + ), + ), + ( + "unknown_fail_kind", + _body( + { + "v": 1, + "t": "fail", + "id": "r:1", + "phase": "transform", + "name": NAME, + "kind": "boom", + "error": "x", + } + ), + ), + ("unknown_item_tag", _body(_ok({"r": "items", "shape": "one", "i": {"o": "exec"}}))), + ("unknown_shape", _body(_ok({"r": "items", "shape": "generator", "i": []}))), + ("non_bool_accepts", _body(_ok({"r": "bool", "b": 1}, phase="transform"))), + ("empty_body", b""), +] + + +@pytest.mark.parametrize(("case", "body"), _HOSTILE, ids=[c for c, _ in _HOSTILE]) +def test_hostile_frames_fail_closed(case: str, body: bytes) -> None: + with pytest.raises(SandboxCodecError): + _decode(body) + assert EXECUTED == [] + + +def test_recursion_error_is_not_a_value_error() -> None: + """Pins the reason ``decode_frame`` catches ``RecursionError`` explicitly: it is a ``RuntimeError``, + so a ``except ValueError`` would have let a deep-nesting rejection escape the fail-closed contract + exactly as the old bare ``KeyError`` did.""" + assert not issubclass(RecursionError, ValueError) + with pytest.raises(RecursionError): + json.loads(_deep_json(100000)) + + +# --- (5) the value grammar round-trips exactly ------------------------------- + + +def _rt_value(value: object) -> object: + blobs = _Blobs() + return dec_value(enc_value(value, blobs), _Reader(blobs.items)) + + +def test_value_grammar_round_trips_exactly() -> None: + assert _rt_value("a\udce9b") == "a\udce9b" # a lone surrogate survives (surrogatepass) + assert _rt_value("L" * 9000) == "L" * 9000 # and so does an out-of-band long string + + tup = _rt_value((1, "two", (3,))) + assert tup == (1, "two", (3,)) and isinstance(tup, tuple) and isinstance(tup[2], tuple) + + keyed = _rt_value({1: "int", ("a", "b"): "tuple", None: "none", 2.5: "float"}) + assert keyed == {1: "int", ("a", "b"): "tuple", None: "none", 2.5: "float"} + + assert _rt_value(b"\x00\xff\x10") == b"\x00\xff\x10" + assert math.isnan(_rt_value(float("nan"))) # type: ignore[arg-type] + assert _rt_value(float("inf")) == math.inf + assert _rt_value(float("-inf")) == -math.inf + # tomllib produces these natively, so a .toml code set with a date value is a legitimate shape + # plain JSON refuses. + assert _rt_value(dt.datetime(2026, 8, 1, 12, 30, 5)) == dt.datetime(2026, 8, 1, 12, 30, 5) + assert _rt_value(dt.date(2026, 8, 1)) == dt.date(2026, 8, 1) + assert _rt_value(dt.time(12, 30, 5)) == dt.time(12, 30, 5) + + +def test_a_setstate_tuple_value_stays_a_tuple() -> None: + """A JSON-text shortcut would flatten it to a list — a silent value-shape change mode=off does not + make.""" + restored = _rt_transform(SetState("ns", "k", (1, 2))) + assert isinstance(restored, SetState) and restored.value == (1, 2) + assert isinstance(restored.value, tuple) + + +def test_adr0028_binary_body_is_not_double_encoded() -> None: + """An ``mfb64:v1:`` body is ALREADY an ordinary ASCII ``str`` — it must ride as a plain + string slot, never be re-wrapped through the bytes tag (that would break ``raw_bytes``).""" + payload = b"\x00\x01\x02binary\xffbody" + original = RawMessage.from_bytes(payload, "application/octet-stream") + assert original.raw.startswith("mfb64:v1:") + + blobs = _Blobs() + node = enc_payload(original, None, blobs) + assert node["kind"] == "raw" + restored = dec_payload(node, _Reader(blobs.items)) + assert isinstance(restored, RawMessage) + assert restored.raw == original.raw # byte-identical, NOT re-base64'd + assert restored.raw_bytes == payload + + # and the same body carried back as a Send survives the result leg too + out = _rt_transform(Send("OB_A", original)) + assert isinstance(out, Send) and out.message == original.raw + + +# --- (6) state_view tuple keys stay tuples ----------------------------------- + + +def test_state_view_tuple_keys_round_trip() -> None: + """``state_get`` does a dict lookup on ``(namespace, key)``; a list/joined key would turn every read + into a SILENT miss returning its default — fail-open, inverting a suppression/dedup Handler with no + ERROR, no dead-letter and no disposition anomaly.""" + rc = RunContext(state_view=MappingProxyType({("ns", "k"): "hit"})) + blobs = _Blobs() + decoded = dec_run_context(enc_run_context(rc, blobs), _Reader(blobs.items)) + with run_contexts(decoded, phase="transform"): + assert state_get("ns", "k", "MISS") == "hit" + + +# --- (7) _partition parity — the codec never changes what delivers ------------ + + +class _SubSend(Send): + """A user subclass. ``_partition`` selects with ``isinstance``, so this DELIVERS in-process.""" + + +def _generator_result() -> Any: + def gen() -> Iterator[Send]: + yield Send("OB_A", "x") + + return gen() + + +@pytest.mark.parametrize( + ("case", "make"), + [ + ("none", lambda: None), + ("bare_send", lambda: Send("OB_A", "x")), + ("mixed_list", lambda: [Send("OB_A", "x"), SetState("n", "k", 1), SetMeta("m", "v"), 7]), + ("send_subclass", lambda: _SubSend("OB_A", "x")), + ("tuple_of_sends", lambda: (Send("OB_A", "x"), Send("OB_B", "y"))), + ("set_of_sends", lambda: {Send("OB_A", "x")}), + ("bare_int", lambda: 7), + ("generator", _generator_result), + ], +) +def test_partition_parity_table(case: str, make: Any) -> None: + """``_partition`` stays the SOLE filter: the codec reproduces its exact input container, so the + ``(sends, state, meta)`` shape is identical to ``mode=off`` for every return shape — including the + ones ``mode=off`` silently drops. Normalising an iterable into a list here would start delivering + ``Send``\\ s the current engine does not (shipping PHI it drops today).""" + direct = _partition(make()) + through_codec = _partition(_rt_transform(make())) # type: ignore[arg-type] + assert [len(x) for x in through_codec] == [len(x) for x in direct] + if case == "send_subclass": + assert len(direct[0]) == 1 and through_codec[0][0].to == "OB_A" # still DELIVERS + if case in ("tuple_of_sends", "set_of_sends", "bare_int", "generator", "none"): + assert [len(x) for x in direct] == [0, 0, 0] # still DROPS, in both modes + + +# --- (8) parent-side constructor faults are wrapped -------------------------- + + +def test_parent_rebuild_wraps_constructor_faults() -> None: + """A forged row that trips ``SetState.__post_init__`` must surface as a codec rejection, not as a + ``WiringError``: the child already ran that constructor successfully, so a raise here cannot be an + authoring fault, and a ``WiringError`` would put a FALSE diagnosis in the operator's last_error.""" + with pytest.raises(SandboxCodecError) as excinfo: + _dec_item({"o": "state", "ns": "", "key": "k", "v": 1}, _Reader(())) + assert isinstance(excinfo.value.__cause__, WiringError) + assert isinstance(excinfo.value, SandboxError) # the documented contract still holds + + +# --- (9) the Send rebuild is a provable copy-on-Send no-op -------------------- + + +def test_copy_on_send_rebuild_is_a_provable_no_op() -> None: + """The parent rebuilds with the NORMAL ``Send`` constructor, inside the transform run context — so + ``Send.__post_init__`` re-consults ADR 0104's copy-on-Send flag. Because the wire always carries a + ``str``, ``snapshot_payload`` returns the identical object and the guard (``if snap is not + self.message``) never rebinds: the choke point is honoured, not bypassed by an ``object.__new__`` + hack. Str-carriage is therefore a load-bearing invariant, not an optimisation.""" + blobs = [_BIG_A] + reader = _Reader(blobs) + with run_contexts(RunContext(snapshot_on_send=True), phase="transform"): + item = _dec_item({"o": "send", "to": "OB_A", "m": {"$": 0}}, reader) + assert isinstance(item, Send) + assert item.message is blobs[0] # identity: no snapshot copy was taken + + +# --- the header cap must not become a mode-dependent ceiling ------------------ + + +def test_the_header_cap_is_the_frame_cap_and_nothing_tighter() -> None: + """Regression for a fail-CLOSED cap sized by aesthetics rather than by traffic. + + ``_MAX_HEADER`` used to be 16 MiB, a quarter of the frame ceiling. Because the request header + carries the whole ADR 0006 ``reference_view``, that silently capped reference tables at roughly + 700k entries: past it EVERY message on the inbound dead-lettered forever with "frame header too + large" while ``mode=off`` served them fine, and there is no ``[sandbox]`` knob to raise it. The + outer framing already refuses anything over :data:`MAX_FRAME` before a byte is parsed, so a + tighter header bound bought the divergence and nothing else.""" + assert _MAX_HEADER == MAX_FRAME == sandbox._MAX_FRAME + # A header well past the old 16 MiB cap now decodes. + header, blobs = decode_frame(encode_frame({"v": 1, "t": "ready", "pad": "x" * 17_000_000}, ())) + assert len(header["pad"]) == 17_000_000 + assert blobs == [] + + +def test_a_large_reference_view_round_trips(monkeypatch: pytest.MonkeyPatch) -> None: + """The functional half: a realistic 20k-entry crosswalk marshals and comes back intact — through + the compact table form AND (forced here) through the general per-entry form, because both must + produce identical results or ``mode=subprocess`` would resolve lookups differently.""" + table = {f"K{i:05d}": f"V{i:05d}" for i in range(20_000)} + rc = RunContext(reference_view={"crosswalk": table}) + + def _round_trip() -> Any: + frame = encode_request( + request_id="r:1", + phase="router", + name="r", + payload=None, + origin=(RAW, ContentType.HL7V2.value), + run_context=rc, + ) + assert len(frame) > 300_000 # the shape this has to survive + return decode_request(frame).run_context.reference_view + + assert _round_trip() == {"crosswalk": table} + # Force the general form by making the fast path's "short string" test fail for every entry. + monkeypatch.setattr(codec, "_BLOB_MIN", 1) + assert _round_trip() == {"crosswalk": table} + + +def _req_body(table: Any) -> bytes: + """A hand-built ``req`` frame carrying one reference table — the frame type a lookup table rides.""" + return _body( + { + "v": 1, + "t": "req", + "id": "r:1", + "phase": "transform", + "name": NAME, + "payload": {"k": "obj", "kind": "raw", "text": "x", "ct": "json"}, + "rc": { + "reference_view": {"refs": [["codes", table]]}, + "snapshot_on_send": False, + }, + } + ) + + +def test_a_forged_compact_table_is_rejected_on_the_request_frame() -> None: + """The compact form carries its WHOLE type proof in the values, so the proof has to be exercised + where a table actually rides: the ``req`` frame. + + This lived in the ``_HOSTILE`` response-frame table and passed for the wrong reason — a ``req`` + frame handed to ``decode_response`` is refused as an unexpected frame *type* long before any table + is parsed, so the assertion never reached the code it was named after.""" + # The control: the same frame with a legal table decodes, so the rejections below are about the + # value type and nothing else. + good = decode_request(_req_body({"s": {"A": "1"}})) + assert good.run_context.reference_view == {"codes": {"A": "1"}} + for forged in ({"A": 1}, {"A": None}, {"A": ["x"]}, {"A": {"b": "AA=="}}): + with pytest.raises(SandboxCodecError, match="must be a string"): + decode_request(_req_body({"s": forged})) + + +def test_the_compact_table_form_is_only_an_encoding() -> None: + """The fast path is an encoding choice, not a grammar change: the general form decodes to exactly + the same table, and the compact form still proves every value is a ``str`` (a forged + ``{"s": {"A": 1}}`` is rejected — see + ``test_a_forged_compact_table_is_rejected_on_the_request_frame``).""" + blobs = _Blobs() + compact = codec._enc_table("t", {"A": "1", "B": "2"}, blobs) + assert compact == {"s": {"A": "1", "B": "2"}} + assert codec._dec_table(compact, "t", _Reader(blobs.items)) == {"A": "1", "B": "2"} + # A mixed-type table falls back to the general per-entry form, unchanged. + blobs = _Blobs() + general = codec._enc_table("t", {"A": "1", "B": 2}, blobs) + assert general == {"a": [["A", "1"], ["B", 2]]} + assert codec._dec_table(general, "t", _Reader(blobs.items)) == {"A": "1", "B": 2} + + +def test_a_realistic_crosswalk_actually_takes_the_compact_form() -> None: + """The throughput half of the same fast path, pinned structurally rather than by a wall clock. + + Describing a `str -> short str` crosswalk entry by entry costs a Python round trip per entry in + BOTH directions, which made the `reference_view` the dominant per-message cost of + ``mode=subprocess`` — measurably worse than the pickle it replaced. A refactor can silently route + around the fast path without changing a single decoded value, so assert the *shape* on the wire: a + realistic crosswalk must ride as one JSON object, not as a per-entry array.""" + table = {f"K{i:05d}": f"V{i:05d}" for i in range(2_000)} + frame = encode_request( + request_id="r:1", + phase="router", + name="r", + payload=None, + origin=(RAW, ContentType.HL7V2.value), + run_context=RunContext(reference_view={"crosswalk": table}), + ) + assert b'"s":{"K00000":"V00000"' in frame # the compact form + assert b'["K00000","V00000"]' not in frame # not the per-entry form + assert decode_request(frame).run_context.reference_view == {"crosswalk": table} + + +@pytest.mark.parametrize( + ("case", "table"), + [ + ("lone surrogate value", {"A": "lone\udcff"}), + ("lone surrogate key", {"k\udcff": "v"}), + ("NUL and C0 controls", {"a\x00b": "c\x01d\x7f"}), + ("astral plane", {"\U0001f600": "\U0001f9ea"}), + ("empty table", {}), + ("empty strings", {"": ""}), + ("JSON metacharacters", {'a"\\b': 'c"\\d'}), + ], +) +def test_the_compact_form_survives_exotic_but_legal_text(case: str, table: dict[str, str]) -> None: + """The compact form pushes keys AND values through JSON escaping instead of the segment path, so + every text shape a real crosswalk can hold has to survive it — a mangled key is a SILENT lookup + miss under ``mode=subprocess`` (fail-open) rather than a dead-letter.""" + blobs = _Blobs() + node = codec._enc_table("t", table, blobs) + assert "s" in node, case # every case above is str -> short str, so the fast path applies + assert codec._dec_table(node, "t", _Reader(blobs.items)) == table + + +# --- (10) the engine's code-set tables travel in the boot frame --------------- + + +def _code_sets( + values: dict[str, Any], *, policy: UnmappedPolicy | None = None +) -> dict[str, CodeSet]: + return {"cs": CodeSet("cs", values, policy)} + + +def _boot_round_trip(code_sets: object) -> Any: + return decode_boot( + encode_boot( + config_dir="cfg", + forbidden=("socket",), + cpu_seconds=2.0, + mem_mb=512, + code_sets=code_sets, + ) + ).code_sets + + +def test_code_sets_round_trip_through_the_boot_frame() -> None: + """The ENGINE's tables are what the child serves, so they must survive the boot frame exactly — + values, and the declared unmapped policy that decides what a MISS returns.""" + rebuilt = _boot_round_trip(_code_sets({"A": "1", "B": "2"})) + assert rebuilt is not None + assert dict(rebuilt["cs"]) == {"A": "1", "B": "2"} + assert rebuilt["cs"].name == "cs" + assert rebuilt["cs"].policy == UnmappedPolicy() + + flagged = _boot_round_trip( + _code_sets({"A": "1"}, policy=UnmappedPolicy(kind=UnmappedKind.DEFAULT, default_value="D")) + ) + assert flagged is not None + assert flagged["cs"].translate("MISS") == "D" # the policy, not just the table, crossed + + # A non-str value (a .toml code set) falls back to the general form and keeps its type. + typed = _boot_round_trip(_code_sets({"A": 1, "B": dt.date(2026, 1, 1)})) + assert typed is not None + assert dict(typed["cs"]) == {"A": 1, "B": dt.date(2026, 1, 1)} + + # "no tables published" and "the engine has none" are DIFFERENT statements: the first leaves the + # child on its own bootstrap load, the second tells it there are none. + assert _boot_round_trip(None) is None + assert _boot_round_trip({}) == {} + + +def test_a_deeply_nested_value_is_not_a_mode_dependent_dead_letter() -> None: + """``mode=off`` accepts whatever ``SetState``'s own ``json.dumps`` validator accepts, so a tight + codec depth cap was a divergence in the DELIVERING direction: a value nested 80 deep set state + in-process and dead-lettered under ``mode=subprocess``. The cap now clears anything a real + transform builds — while still bounding the walk, fail-closed, well short of ``RecursionError``.""" + value: Any = "leaf" + for _ in range(200): + value = [value] + op = SetState("ns", "deep", value) # constructs at mode=off, so it must cross the wire too + rebuilt = _rt_transform(op) + assert isinstance(rebuilt, SetState) + assert rebuilt.value == value + + runaway: Any = "leaf" + for _ in range(_MAX_DEPTH + 2): + runaway = [runaway] + with pytest.raises(SandboxCodecError, match="nesting exceeds"): + enc_value(runaway, _Blobs()) + + +def test_a_forged_code_set_policy_fails_closed() -> None: + """The boot frame is decoded under the same closed contract as everything else.""" + blobs = _Blobs() + node = enc_code_sets(_code_sets({"A": "1"}), blobs) + node["sets"][0][1] = "not-a-kind" + with pytest.raises(SandboxCodecError, match="invalid unmapped policy"): + dec_code_sets(node, _Reader(blobs.items)) + + +# --- framing round-trip ------------------------------------------------------ + + +def test_encode_frame_round_trips_header_and_segments() -> None: + header, blobs = decode_frame(encode_frame({"v": 1, "t": "ready"}, ("one", "two"))) + assert header == {"v": 1, "t": "ready"} + assert blobs == ["one", "two"] + + +def test_encode_ok_rejects_an_undescribable_result() -> None: + """Fail-closed at describe time: an exotic ``Send`` payload cannot silently become something else.""" + with pytest.raises(SandboxCodecError): + encode_ok(request_id="r:1", phase="transform", name=NAME, result=Send("OB_A", object())) # type: ignore[arg-type] + with pytest.raises(SandboxCodecError): + encode_ok(request_id="r:1", phase="accepts", name=NAME, result="truthy") + with pytest.raises(SandboxCodecError): + encode_ok(request_id="r:1", phase="router", name=NAME, result=[1, 2]) diff --git a/tests/test_threat_model_doc_drift.py b/tests/test_threat_model_doc_drift.py index a29ec9e8..bcf80991 100644 --- a/tests/test_threat_model_doc_drift.py +++ b/tests/test_threat_model_doc_drift.py @@ -79,7 +79,12 @@ "**In-process (default) or subprocess-isolated execution": "sandbox", "**`db_lookup`**": "db_lookup", "**`fhir_lookup`**": "fhir_lookup", - "**Sandbox IPC deserialization**": "pickle", + # The pipe no longer pickles (ADR 0087 MFW2, BACKLOG #339): a Handler's `__reduce__` executed in + # the engine parent. Pinning "pickle" here would now REQUIRE the private doc to keep asserting a + # mechanism the code does not have, so the anchor moved to the module that replaced it. This is a + # COUPLED edit — `docs/security/THREAT-MODEL.md` lives in the vault (gitignored, absent from every + # checkout, so these tests skip here and in CI) and its 15.1.5 row must be rewritten to match. + "**Sandbox IPC deserialization**": "_sandbox_codec", "**Other subprocess invocations**": "shell=False", }