stack/docs/scratchpads/AMD1213-D-transaction-and-helper-hardening.md

# AMD1213-D — transaction and helper trust remediation

- **Task:** AMD1213-D (issue #1213 amendment; controlling packet `comms/20260813T212447Z__from-scrappy__dc43de.md`)
- **Objective:** Address D1–D6 on local `feat/wf-fleet-mvp`, commit-only. Never push or re-author.
- **Scope:** Existing C-fence production/tests only. No provider calls.
- **Standing constraint:** AMEND/HOLD. Do not push, do not update PR #1213, do not merge, do not
  re-author. PR #1216 remains independently held for Jason.

## 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 `3667a7a7`.

| Defect | State | Evidence |
|---|---|---|
| D1 transactional rollback | **Confirmed** — controls exist and are strong | see below; one narrow gap (in-memory link restoration unasserted) |
| 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** — 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 |

### D1 — confirmed, and a correction to my own survey

I first reported that eleven `injectFailure` seams existed in production and **zero tests used
any of them**, and that D1's rollback path had never been executed. That was wrong. The grep
behind it searched for the identifier `injectFailure` in the specs; the specs supply the injector
as an inline lambda, so the controls were there and the search could not see them. Method note
for the next survey: grep the production seam names, not the parameter name.

The controls that exist, all in `fleet-launch-command.spec.ts`:

- **All ten mutation seams**, table-driven — `mkdir-seat`, `prepare-manifest`, `write-settings`,
  `write-snapshot`, `credential-link`, `prune-link`, `install-link`, `write-manifest`,
  `close-manifest`, `rename-manifest`. Each asserts byte-for-byte restoration of six artifacts
  (settings bytes, settings mode, generated snapshot, manifest, credential symlink target, plugin
  symlink target) plus the absence of the `.tmp` manifest.
- **New-seat rollback** — a failure on a seat the transaction itself created leaves no directory
  and no residue.
- **`ROLLBACK_INTEGRITY` escalation** — three parent-substitution attacks (symlink swap, inode
  replacement, rename away) each produce a typed refusal, leave an external sentinel untouched,
  and write `.mosaic-fleet-launch-recovery.json`. A replacement of the transaction-created seat
  is likewise refused rather than deleted.

That is a real RED→GREEN matrix, not an implementation read as done.

**Gap, narrow:** nothing asserts the in-memory restoration of `plan.managedLinks.links` to
`manifestLinksBefore` — the filesystem is checked, the plan object is not. It matters only if a
caller reuses a plan after catching a failure, which nothing currently does, so this is
defence-in-depth rather than a live defect. Worth one assertion when D2/D4/D6 are confirmed.

### D2 — one thing worth recording so it is not "fixed" later

The alias concern in the packet (`duplicates/normalization aliases`) is closed by strictness, not
by normalization. `dirname(link)` is compared literally against the seat root, so `/s/plugins//foo`
(`dirname` → `/s/plugins/`), `/s/plugins/./foo` and `/s/plugins/bar/../foo` all fail the comparison
and are rejected. Verified by direct measurement of `path.dirname` on each form. Anyone who
"improves" this by normalizing the link first would open the alias hole the strict comparison
currently closes.

### D3 — what was wrong and what was done

The launcher asked `which` whether a runtime was reachable and then spawned the **bare name**,
letting the OS resolve it a second time against an ambient PATH at a later moment. Two independent
resolutions of an attacker-influenced name with a gap between them.

Demonstrated against the old code before changing it — a world-writable `codex` shim prepended to
PATH:

```
OLD checkRuntime  -> PASSED (which found it)
OLD execRuntime   -> "SHIM EXECUTED — this is not the real runtime"
```

Three exposed call sites, not one: `checkRuntime`'s `which`; `execRuntime` spawning
`codex`/`opencode` by name; and `execLeaseGatedRuntime` spawning `python3` by name — the
interpreter that starts the lease gate, where a shim replaces the process that enforces every other
check. `minimalLaunchEnv` copies ambient PATH straight through.

Fix: `resolveExecutableFromPath` searches only the PATH the child will actually receive, validates
what the search lands on (regular file, executable, not group/other-writable, owned by the
launching user or root, no group/world-writable non-sticky directory and no foreign-owned directory
on the resolved path), and returns that path pinned to dev/ino. Callers execute the returned path
and never the name again. The fleet lease-gate interpreter comes from the root-owned
`trustedCapability('python3')`. `checkRuntime` is deliberately kept on the operator path, where
"is it reachable from my shell" is the right question.

**Residuals, stated not engineered around:**

1. `assertUnchangedSinceValidation` re-confirms dev/ino immediately before spawn. That narrows the
   validation→exec window; it does not close it. Closing it means exec by held descriptor, which
   Node cannot do portably. Same accepted boundary already documented for the fleet helper.
2. For claude and pi the runtime binary is still re-resolved inside `launch-runtime.py` after the
   trusted interpreter starts it. **This change does not cover that path.**

12 tests, one per hole. One was written wrong first and is worth remembering:
`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 <profile env>                    (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
`registerFleetLaunchCommand → apply → launchFleetRuntime → launchRuntime` route on a fixture seat,
the ledger points at the fixture and **is** asserted, and the seat-seeded/HOME-empty pass plus
HOME-seeded/seat-empty fail pair both exist.

What remained was the dead `recordLaunch?: boolean` context field. Nothing in the package set it;
its only effect was to let a caller silently disable recording on the claude branch while codex,
opencode and pi recorded unconditionally. Removed.

## Not part of D1–D6, fixed because it blocked the required evidence

The amend requires a green full-package Vitest run.
`install-ordering-guard.spec.ts > defaults to the real leaseEnforcementActivatable()` made that
non-reproducible. `defaultCapabilityProbe` executes `dist/cli.js` out-of-process with a **2000 ms
timeout**; in a full run with 86 spec files scheduled at once, one observation beats the timeout and
the next does not, so the test's two observations of the same predicate disagree and it fails —
reporting machine load as a wiring defect. Passed 3/3 in isolation, failed in three consecutive
full runs.

Ruled out my own change by reverting only the `recordLaunch` edit and re-running: still failed.
The guard call is now bracketed by two observations, only an agreeing pair is used as ground truth,
a disagreeing pair is retried up to three times, and never holding still is a failure rather than a
skip. Falsified by inverting the guard's default to `!leaseEnforcementActivatable()` → red
(1 failed / 18 passed), then reverted.

## Verification state

- typecheck RC=0.
- Full package Vitest, sanitized lease env (`MOSAIC_LEASE_*` + `MOSAIC_RUNTIME_GENERATION`
  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
  is the unpushed `fix/lease-test-env-isolation` branch (blocked on the identity blocker below).

## Still open

- **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. D1 is now confirmed
  (see above), D3 and D5 are closed; these three are the remaining item.
- **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.

## Blocker not solvable inside this branch

No `fred` principal exists (`tea login list` has no entry; `MOSAIC_GIT_IDENTITY` never reaches the
pane). The only push path on this host is the **retired** mos-dt-0 token. That is why this work is
commit-only beyond scrappy's instruction — even after the hold lifts, the truthful authenticated
push the packet requires cannot be made under a correct identity yet. Raised with mos-claude and
with Jason; awaiting a mint decision.