fix(sandbox): the engine parent executed code chosen by the sandboxed child (BACKLOG #339, ADR 0087)

[wshallwshall]

The defect

ADR 0087's opt-in [sandbox].mode=subprocess exists to put an address-space boundary between admin-authored Router/Handler code and the engine's DEK, audit chain, and sockets. The boundary was bypassable.

The worker pickled a Handler's raw return value into {"ok": True, "result": ...} and the parent called pickle.loads on it. A Handler returning an object with a custom __reduce__ therefore executed arbitrary code in the engine process. Proven end-to-end before any code was written, and again against the finished tree.

Three things made it more than a nit:

• sandbox.py:40 and ADR 0087 both already reasoned about "a compromised worker" — and named engine-side handler/outbound-name validation as the defense. That runs long after pickle.loads has executed the payload.
• The # nosec B301 # nosemgrep suppression was justified as "never external/untrusted data" — true for the child reading the parent, false for the parent reading the child, which is the direction the threat model distrusts. That suppression is what kept two scanners quiet.
• The child's own except (PicklingError, TypeError) guard doesn't help: a __reduce__ object pickles fine. The danger is entirely on the load side.

mode=off is the default and was never affected.

The fix

A segmented, closed-tag, non-executing codec (pipeline/_sandbox_codec.py) replaces pickle on both legs. The decode path is json.loads with no object hooks plus a literal tag dispatch over a closed constructor set — it 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.

A second, independent break — found by adversarial review

The codec introduced a correlation defense, and that defense was itself bypassable. The request id was a per-spawn nonce plus a counter, disclosed to the child; decode_response checked only (id, phase). A malicious Handler could pre-stage a forged frame for the next id and have it consumed as an unrelated message's result — attacker-chosen delivery to any registered outbound, no ERROR, no disposition anomaly. Deterministic, 3/3, end-to-end through a real spawned worker.

Closed three ways: a fresh secrets.token_hex(16) per dispatch, binding of the whole (id, phase, name) triple, and an unsolicited-frame check that is fatal to the worker.

This would have shipped as a "fix" without the adversarial pass.

Verification

• Self-proving regression test. test_a_reduce_gadget_never_reaches_the_parent runs pickle.loads on the hostile bytes and asserts the tripwire fired — so if the gadget ever goes inert the test fails loudly instead of passing vacuously — then asserts the new codec refuses the identical bytes with nothing executed.
• End-to-end forgery test. test_a_frame_staged_between_dispatches_can_never_answer_the_next_one stages a real second frame through a spawned worker, sleeps for it to land, and asserts the next dispatch refuses it and the victim message still gets its true answer.
• Parity measured against a HEAD baseline. On a 41-check mode=off vs mode=subprocess sweep the codec is a net correctness win — HEAD had 10 divergences, this has 3, two of which reproduce identically at HEAD. It also fixes mode=subprocess being outright DOA for ADR 0013 loopback re-ingress.

Suppression hygiene

The # nosec B403 and the repo's only # nosemgrep are deleted, not reworded. Zero pickle/marshal remains in messagefoundry/ or tee/, so .semgrep/messagefoundry.yml's "starts clean" header is now true without a suppression propping it up.

Residuals — stated, not implied

mode=off stays default and byte-identical. Honest residuals now in ADR 0087: ~1.2–1.4× marshalling cost on a large reference table; a compromised worker can still deny its own feed (fail-closed); a contract-violating mutating Router diverges between modes (pre-existing, pinned by a test asserting the inequality on purpose); a Handler's exception reaches the operator wrapped.

15.2.5 stays Partial. This is the address-space half — DEFAULT_FORBIDDEN_MODULES does not block os/subprocess, so a sandboxed Handler still reaches host execution. OS-level confinement is ADR 0147, still Proposed. The ADR index row previously claimed full closure and contradicted the ADR 0144 row; corrected here.

Ledger

Implements BACKLOG #339. Also files #341 (P1: a Handler returning a tuple of Sends delivers nothing, silently — pre-existing, verified live), #342, #343 — filed, deliberately unclaimed, and not fixed here.

ADR 0087 amended in place (AC-8..AC-15); ADR 0147 pinned to this codec as a prerequisite; ADR 0104's AC-4 re-worded off pickle. No ADR/BACKLOG number grepped — all allocated via scripts/coord/alloc.ps1.

Notes for reviewers

• Rebased three times today; currently 0 behind main.
• Local suite: 9,900 passed. The only failures were environment artifacts (stale venv metadata, a Windows path-mangling issue in a test's own temp-file creation), both proven unrelated.
• Adds ~1,393 test lines that spawn real worker subprocesses. Sized against the windows-2025 cap raised by ci: raise the Windows step cap (1.06x margin, claimed 2x) + report PRs that can never merge #131 — two tests are irreducible by design (one sleeps 2.5s to let a forged frame land; the other waits out the wall cap it tests).