From cd3a4008773c81631eae3db76212517ed37a0aa7 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Sun, 2 Aug 2026 10:17:43 -0500 Subject: [PATCH] backlog: close #339's stale OPEN item, correct #197's banner, file #346 Three ledger corrections trailing PR #136 (55bbafb1, the ADR 0087 sandbox codec fix). 1. #339 OPEN item 2 said "The edit was NOT made" about docs/adr/README.md:121. 08d898bc MADE it, in that same PR, once claude/adr-asvs-scorecard-as-data merged (ADR 0156, PR #120) and the worktree guard released. The item is CORRECTED rather than deleted: it read "was NOT made" for the life of the branch and at least four sessions saw it in that state, so the record should show what happened, not just end up right. 2. #197's banner still claimed "Closes the WP-L3-17 (ASVS 15.2.5) residual". That was the LAST copy of the claim in this file - README.md:121 was fixed by 08d898bc and the private ASVS-L3-REMEDIATION-PLAN / THREAT-MODEL rows in the vault. Two reasons it is wrong, both verified against the code rather than inferred from the ADR: (a) DEFAULT_FORBIDDEN_MODULES blocks socket/ssl/asyncio/multiprocessing and the secret-bearing packages but NOT os/subprocess, so a sandboxed Handler still reaches host command execution - confinement is address-space only; (b) until #339 the pipe pickled the child's return and the ENGINE PARENT deserialized it, so the boundary was bypassable outright. Banner also gains _sandbox_codec.py, which it did not name. 3. #346 filed - the sandbox import boundary is enforced only at runtime, under an off-by-default flag. A type the child must construct or receive cannot live under a DEFAULT_FORBIDDEN_MODULES prefix; CapturedResponse violated that and made mode=subprocess + ADR 0013 loopback re-ingress DOA until #339 relocated it. The guard runs in the child, at dispatch time, only under mode=subprocess - so a re-violation gives a green suite, a byte-identical mode=off, and breakage ONLY for installs that enabled the sandbox for security reasons. It fails selectively against the population least able to report it. Number allocated via scripts/coord/alloc.ps1, never grepped. Measured for the item rather than asserted: `git grep -l FORBIDDEN_MODULES -- tests/` is EMPTY, and _sandbox_codec.py's imports are today all under config/ and parsing/, so the invariant currently HOLDS - this is about keeping it, not repairing it. Verified by falsification, not by a green run: probing #346's banner to a non-banner line failed the checker at exactly BACKLOG.md:8429 naming item #346, so the gate demonstrably sees this item. Reverted; 269 items, each declaring exactly one status, exit 0. --- docs/BACKLOG.md | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index 3e04e138..62996a5c 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -6365,7 +6365,7 @@ Two findings are worth surfacing here. **Posture B scores worse on Fails than Po ## 197. Runtime sandbox for admin-authored Router/Handler code -> ✅ **SHIPPED 2026-07-10 (ADR 0087, PLAN-9 Wave 3).** Opt-in `[sandbox]` subprocess isolation built: `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 DEK/audit-chain/sockets (the child loads only the message *graph*); plus 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 the tracer — see residuals); engine-side handler/outbound-name validation stays engine-side; a denial → `ERROR`/dead-letter **post-ACK** (no NAK, never dropped). Wired live through `wiring_runner`/`engine`/`app`; RunContext re-marshalled across the boundary. **Closes the WP-L3-17 (ASVS 15.2.5) residual** (residual-closure — `ASVS-L3-REMEDIATION-PLAN.md` + `THREAT-MODEL.md` flipped). **Deferred residuals:** default-off (opt-in); the ADR 0072 protocol-tracer is not forwarded across the subprocess boundary (`mode=subprocess` bypasses it; `mode=off` composes); `db_lookup`/`fhir_lookup` forward-over-IPC (sandboxed live-enrichment Handlers run `mode=off`); load-time top-level config exec not sandboxed (unchanged safe-source DACL gate); least-privilege service account default is environment-delegated. +> ✅ **SHIPPED 2026-07-10 (ADR 0087, PLAN-9 Wave 3).** Opt-in `[sandbox]` subprocess isolation built: `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 DEK/audit-chain/sockets (the child loads only the message *graph*); plus 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 the tracer — see residuals); engine-side handler/outbound-name validation stays engine-side; a denial → `ERROR`/dead-letter **post-ACK** (no NAK, never dropped). Wired live through `wiring_runner`/`engine`/`app`; RunContext re-marshalled across the boundary. **Does NOT close the WP-L3-17 (ASVS 15.2.5) residual — corrected 2026-08-02 (BACKLOG #339).** Two independent reasons, both verified rather than inferred: (a) confinement is **address-space only** — `DEFAULT_FORBIDDEN_MODULES` blocks socket/ssl/asyncio/multiprocessing and the secret-bearing packages but **not `os`/`subprocess`**, so a sandboxed Handler still reaches host command execution; and (b) until #339 the IPC transport pickled the child's return value and the *engine parent* deserialized it, so the boundary was bypassable outright by any Handler with a custom `__reduce__`. OS-level default-deny is [ADR 0147](adr/0147-hardened-runtime-isolation-for-router-handler-code-ipc-brokered-sandbox-extends-adr-0087.md), still Proposed. The private `ASVS-L3-REMEDIATION-PLAN.md` WP-L3-17 row and `THREAT-MODEL.md` 15.1.5 row were flipped on the original claim and are corrected to match. **Deferred residuals:** default-off (opt-in); the ADR 0072 protocol-tracer is not forwarded across the subprocess boundary (`mode=subprocess` bypasses it; `mode=off` composes); `db_lookup`/`fhir_lookup` forward-over-IPC (sandboxed live-enrichment Handlers run `mode=off`); load-time top-level config exec not sandboxed (unchanged safe-source DACL gate); least-privilege service account default is environment-delegated. **Cluster:** Security & Compliance. **Priority:** P3. **Verdict:** build (large). **Severity:** high (blast radius), low (likelihood). @@ -8363,7 +8363,7 @@ Two worked instances the same day. **#74** went green on 2026-07-30 and sat unme **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. +2. ~~**`docs/adr/README.md:121` still reads "Closes the WP-L3-17 residual (residual-closure)"**~~ — **DONE 2026-08-01, not open.** The contradiction with `:175` (ADR 0144, "15.2.5 stays **Partial**") was real and :175 was 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 landed as `08d898bc` in this same PR, once `claude/adr-asvs-scorecard-as-data` merged (ADR 0156, PR #120) and the worktree guard released; the row now records the MFW2 amendment and agrees with `:175`. **This entry is corrected rather than deleted because it read "the edit was NOT made" for the life of the branch, and at least four sessions saw it in that state.** The same closure claim in **#197's banner above** is corrected in the same pass as this note — it was the last copy of it in this file. 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. @@ -8423,3 +8423,23 @@ Two worked instances the same day. **#74** went green on 2026-07-30 and sat unme **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. + +--- + +## 346. The sandbox import boundary is enforced only at runtime, under an off-by-default flag + +> 🚧 **Status OPEN (filed 2026-08-02).** A type the sandbox child must **construct or receive** cannot live under a prefix on `DEFAULT_FORBIDDEN_MODULES` ([pipeline/sandbox.py](../messagefoundry/pipeline/sandbox.py)) — the child's import guard raises and the dispatch fails. That rule is real, it has already been violated once in shipped code, and **nothing enforces it**. `CapturedResponse` lived in `messagefoundry.store`; the child could not import it, which made `mode=subprocess` + ADR 0013 loopback re-ingress **DOA** until #339 relocated it to [config/response.py](../messagefoundry/config/response.py). The only guard runs **in the child, at dispatch time, and only when `[sandbox].mode=subprocess`** — which is not the default, so a re-violation is invisible to a green suite. + +**Cluster:** Correctness / test coverage. **Priority:** P2. **Verdict:** build (small). **Severity:** medium (blast radius: a feature is DOA for everyone who opted in), medium (likelihood: the codec's constructor set is precisely the surface that grows as the payload model does). + +**Why it fails selectively — the reason this wants a test and not a comment.** The population that could report the breakage is the population *not* running the default. A future violation yields a green CI suite, a byte-identical `mode=off`, and a hard failure **only** on installs that turned the sandbox on for security reasons. The failure mode is inverted: the more security-conscious the deployment, the worse its experience, and the quieter the signal reaching the maintainer. + +**Measured, not assumed (2026-08-02):** `git grep -l "FORBIDDEN_MODULES" -- tests/` returns nothing — no test references the constant in any form. [`_sandbox_codec.py`](../messagefoundry/pipeline/_sandbox_codec.py) imports exactly the types the two ends construct (`CodeSet`/`UnmappedKind`/`UnmappedPolicy`, `ContentType`, `CapturedResponse`, `RunContext`, `Send`/`SetMeta`/`SetState`/`WiringError`, `Message`/`RawMessage`), today all under `config/` and `parsing/` — so the invariant **currently holds**. This item is about keeping it that way, not repairing it. + +**Fix direction.** A static test that walks the imports of `_sandbox_codec.py` and `_sandbox_worker.py` (stdlib `ast`, transitively across first-party modules) and asserts none resolves under a `DEFAULT_FORBIDDEN_MODULES` prefix. Anchor it on the **constant**, never a copied list — two copies of a rule drift, and the copy that drifts is the one nobody is testing. + +**Measurement discipline — the part that decides whether this is worth building.** The test must be demonstrated to **fail** against a deliberately introduced forbidden import *before* it is trusted. An import-walker that silently resolves nothing passes for exactly the same reason a correct one does, so a green run proves neither. Have it report what it walked, not merely that it walked. + +**Related:** #339 (surfaced it; relocated `CapturedResponse`), ADR 0087 (the boundary), ADR 0013 (the loopback re-ingress that was DOA), #342 / #343 (the other two findings the #339 review filed but did not fix). + +**Source:** adversarial review of the ADR 0087 sandbox codec, 2026-08-01; the `CapturedResponse` violation is measured, not hypothetical.