From be2d14fb6c04aca92bb2dece9cfcb927da8a216e Mon Sep 17 00:00:00 2001 From: fred Date: Sat, 15 Aug 2026 15:06:31 -0500 Subject: [PATCH] docs(amd1213-d): D3 closed both halves; record the measured child env and the deliberate HOME residual --- ...1213-D-transaction-and-helper-hardening.md | 64 ++++++++++++++++--- 1 file changed, 56 insertions(+), 8 deletions(-) diff --git a/docs/scratchpads/AMD1213-D-transaction-and-helper-hardening.md b/docs/scratchpads/AMD1213-D-transaction-and-helper-hardening.md index 893a546a..edcea5a7 100644 --- a/docs/scratchpads/AMD1213-D-transaction-and-helper-hardening.md +++ b/docs/scratchpads/AMD1213-D-transaction-and-helper-hardening.md @@ -9,13 +9,13 @@ ## Where this actually stands (measured 2026-08-15, not inherited from notes) Everything below was re-measured against the tree rather than trusted from the previous entries, -which understated progress by roughly two defects. Branch head `b91b702a`. +which understated progress by roughly two defects. Branch head `3667a7a7`. | Defect | State | Evidence | |---|---|---| | D1 transactional rollback | Substantially implemented | `fleet-launch-command.ts` snapshots links before mutating, prepares the manifest, and has `injectFailure` seams at `prepare-manifest`, `write-manifest`, `close-manifest`, `rename-manifest`; restores `manifestLinksBefore` on failure | | D2 exact managed-link classes | Substantially implemented | classifier at `fleet-launch-command.ts:598-663`: exact resolved credential, direct one-component plugin/skill only, symlink target and ancestor rejected, `realpath` containment, duplicates rejected | -| D3 ambient-PATH executable resolution | **Closed this pass** (commit `585dac7a`) | see below | +| D3 ambient-PATH executable resolution | **Closed** — executables `585dac7a`, environment `3667a7a7` | see below; one deliberate residual (HOME) | | D4 config check/apply safety | Substantially implemented | `secure_dir` ancestor checks, `read_private` with `O_NOFOLLOW` + fstat, apply via mkstemp + fchmod 0600 + fsync + dev/ino re-check before `os.replace`, compatibility path separated | | D5 bounded test seam | **Closed this pass** (commit `b91b702a`) | see below | | D6 validate-by-path then exec-by-path | Substantially implemented | helper runs as a verified snapshot piped to `bash -s`, not executed by pathname; the same binding applied to the runtime in D3 | @@ -68,6 +68,53 @@ and never the name again. The fleet lease-gate interpreter comes from the root-o `mkdirSync(path, { mode: 0o777 })` is masked by the umask to 0o755, so the world-writable-directory case passed while testing nothing. Create at 0o755, then `chmodSync`. +### D3 environment half — measured, and mostly already true + +Measured before changing anything: the real `fleet launch` route with a shim in place of the +runtime binary, the shim dumping its own environment. The subject is therefore what arrives after +composition **and** after `launch-runtime.py` adds the lease variables — not the object the +launcher builds. Those are different sets. + +The complete child environment for a composed claude seat: + +``` +PATH HOME USER LOGNAME SHELL TERM COLORTERM TMPDIR XDG_RUNTIME_DIR (inherited allowlist) +LANG LC_ALL (fixed, this pass) +CLAUDE_CONFIG_DIR MOSAIC_AGENT_NAME (declared) +MOSAIC_LAUNCH_ID (minted per launch) +MOSAIC_LEASE_BROKER_SOCKET MOSAIC_LEASE_GENERATION_FILE +MOSAIC_LEASE_RUNTIME MOSAIC_LEASE_SESSION_ID +MOSAIC_RECEIPT_OBSERVER_SOCKET MOSAIC_RUNTIME_GENERATION (lease gate) +``` + +Most of the defect was already closed **by construction and untested**. `minimalLaunchEnv` builds +from an empty object over a fixed list, so `BASH_ENV`, `ENV`, `PYTHON*`, `NODE_*`, `NPM_CONFIG_*`, +`LD_PRELOAD`, `LD_LIBRARY_PATH` and provider credentials never reach the child. All sixteen were +planted; none survived, including through the lease gate. The gap was that nothing named the +allowlist, and an allowlist no test names is one careless edit away from being a denylist. + +Fixed: **locale was inherited**, so the same seat emitted different message language, collation and +number/date formatting depending on who started it. Composed launches now pin `C.UTF-8` — not `C`, +which is ASCII and would mangle non-ASCII output. A profile-declared `LANG`/`LC_ALL` still wins, +and a test holds that escape hatch open. The operator path is untouched. + +**Residual, deliberate — `HOME` is still the operator's.** The card is right that this is the +remaining leak: the runtime gets its own config dir, but anything it shells out to (git, ssh, npm) +reads the operator's dotfiles and therefore the operator's credentials. Not changed here, because +a seat whose HOME is a bare directory has no gitconfig and no ssh key, so it cannot commit or push +— and the fleet MVP's proof is a seat carrying a change to a pushed branch. Moving HOME before the +per-agent home is populated improves isolation and breaks the deliverable. **Owner: the +harness-homes design**, which is exactly the track that populates a per-agent home with its own +auth bundle. Do it there, not here. + +Eight tests, each falsified by inverting the property it defends; every inversion hit only its own +test: `BASH_ENV` added to the inherited list → permitted-set + loader-hook killers red (2 failed); +locale pin reverted → locale killer red; ambient `MOSAIC_LAUNCH_ID` reused → launch-id killer red; +`process.env` recorded into the ledger → ledger-value killer red. + +The permitted-name list in the spec is hand-written, not derived from the launcher. Deriving it +would make the test agree with the code by construction and detect nothing. + ### D5 — what remained and what was done Most of D5 was already closed: `launchFleetRuntimeForTest` is gone, specs enter through the real @@ -99,7 +146,9 @@ skip. Falsified by inverting the guard's default to `!leaseEnforcementActivatabl - typecheck RC=0. - Full package Vitest, sanitized lease env (`MOSAIC_LEASE_*` + `MOSAIC_RUNTIME_GENERATION` - stripped): **86 files / 1619 tests passed, 0 failed**, three consecutive runs, RC=0. + stripped): **87 files / 1627 tests passed, 0 failed**, three consecutive runs plus one against + the committed tree, RC=0. That is exactly one file and eight tests above the 86/1619 baseline, + so the D3 environment work moved nothing else. eslint RC=0, prettier clean. - Without that sanitization the suite shows 4 failures in `mutator-gate.acceptance.spec.ts`. Those are the known host lease-identity leak into spawned hooks, **not** a product defect — the same spec re-run with only those five variables stripped and no code change is 20/20. The standing fix @@ -107,12 +156,11 @@ skip. Falsified by inverting the guard's default to `!leaseEnforcementActivatabl ## Still open -- **Remaining D3 surface:** the packet also asks for a capability-minimal *measured child - environment* — construct child env from scratch, fixed locale, exact vars only, and add - `BASH_ENV`/`ENV`, `PYTHON*`, `NODE_*`, `NPM_CONFIG_*`, unexpected-env-name killers. The - executable-resolution half is closed; the environment half is not. - **D1/D2/D4/D6 need confirmation, not assumption.** They read as substantially implemented but I - have not run the packet's full RED→GREEN control matrix against each seam. + have not run the packet's full RED→GREEN control matrix against each seam. This is now the + largest remaining item in the amendment. +- **D3's HOME residual** is routed to harness-homes (see above). It is stated, not engineered + around, and it does not belong to this branch. - Required next evidence per the packet: all D1–D6 observed RED→GREEN controls, framework-shell, build/lint/Prettier/bash -n, fresh current-next merge-tree.