b91b702a531388dfc7712bf3e94850ed6ad25fad
2
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 fred
|
b91b702a53 |
fix(launch): drop the dead recordLaunch test seam; stop a load-sensitive spec reporting CPU load as a defect
Two changes, both about a test seam that alters production behaviour. AMD1213-D defect D5 objected that `launchFleetRuntimeForTest` was an exported production API that also set `recordLaunch:false`, changing a second production branch beyond the two the card authorized. Most of that is already closed in this tree: the exported helper is gone, and the specs now enter through the real `registerFleetLaunchCommand -> apply -> launchFleetRuntime -> launchRuntime` route on a fixture seat, with the ledger pointed at the fixture and asserted (`fleet-launch-command.spec.ts` asserts `events.ndjson` contains the record). The seat-seeded/HOME-empty pass and HOME-seeded/seat-empty fail pair both exist. What remained was the `recordLaunch?: boolean` context field itself. Nothing in the package sets it -- it is a dead switch whose only effect was to let a caller silently disable launch recording on the claude branch while codex, opencode and pi recorded unconditionally. Removed, so all four branches record the same way and the asymmetry cannot be reintroduced by passing a flag. The second change is unrelated to D1-D6 and is called out as such. It is here because the amend's required evidence includes a green full-package Vitest run, and one spec made that non-reproducible. `install-ordering-guard.spec.ts` proves that `guardClaudeSettingsWiring` really delegates to `leaseEnforcementActivatable()` by comparing the guard's outcome against its own call to the same predicate. That predicate is not deterministic: `defaultCapabilityProbe` runs `dist/cli.js` out-of-process with a 2000 ms timeout. In a full-package run with 86 spec files scheduled at once, one observation beats that timeout and the next does not, the two disagree, and the test fails -- reporting machine load as a wiring defect. It passed in isolation every time, which is why it read as a flake. Measured rather than assumed. The failure reproduced in three consecutive full runs and passed 3/3 in isolation. It was NOT caused by the recordLaunch removal above: reverting only that edit and re-running the full suite still failed, which is what ruled my own change out. The guard call is now bracketed by two observations of the predicate, and only a pair that agrees is used as ground truth; a disagreeing pair is retried, up to three attempts, and never holding still is itself a failure rather than a skip. This does not weaken the assertion -- a real delegation failure is stable and survives every attempt while load noise is not. Falsified: inverting the guard's default to `!leaseEnforcementActivatable()` turns the test red (1 failed / 18 passed), so the retry did not blunt what the test detects. The inversion was reverted and the file confirmed clean. Verification: typecheck RC=0. Three consecutive full-package runs, RC=0, 86 files / 1619 tests passed, 0 failed, under the sanitized lease environment (MOSAIC_LEASE_* and MOSAIC_RUNTIME_GENERATION stripped). Commit-only per scrappy's controlling packet (comms 20260813T212447Z dc43de): not pushed, PR #1213 not updated, nothing re-authored. |
||
|
fred
|
585dac7a5d |
fix(launch): resolve the runtime binary once and execute the object that was checked
AMD1213-D defect D3. The fleet launch path 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 in between is not a check.
Measured against the old code before changing it. A world-writable shim named
`codex` prepended to PATH:
OLD checkRuntime -> PASSED (which found it)
OLD execRuntime -> "SHIM EXECUTED -- this is not the real runtime"
The probe satisfied the check and then supplied the thing that ran.
Three call sites were exposed, not one: `checkRuntime`'s `which`; `execRuntime`
spawning 'codex'/'opencode' by name; and `execLeaseGatedRuntime` spawning
'python3' by name -- the interpreter that starts the lease gate itself, where a
shim does not bypass one check, it replaces the process that enforces all of
them. `minimalLaunchEnv` copies ambient PATH straight through, so the child
inherits the same search.
The fix: `resolveExecutableFromPath` searches only the PATH the child will
actually receive, validates the object the search lands on (regular file,
executable, not group/other-writable, owned by the launching user or root, with
no group/world-writable non-sticky directory and no foreign-owned directory on
its resolved path), and returns that path pinned to its dev/ino. Callers execute
the returned path, never the name again. Rules that are each a hole if dropped:
a relative PATH entry is skipped, since it resolves against wherever the
launcher was started; the first name match decides the outcome and an unsafe
first match is a refusal rather than a reason to keep looking, because falling
through would let a planted binary silently downgrade the search to whatever
came after it; a symlink is followed and the real file is what gets validated
and executed, since validating the link and executing the name repeats the
original bug one level down.
`checkRuntime` is kept unchanged on the operator path. `which` proves
reachability from the operator's own shell, which is the right question there
and the wrong one for a seat. The fleet lease-gate interpreter now comes from
the root-owned `trustedCapability('python3')` the helper already requires.
Two residuals, stated rather than engineered around:
* `assertUnchangedSinceValidation` re-confirms dev/ino immediately before
spawn. That narrows the validation-to-exec window; it does not close it.
Closing it means executing a held descriptor and Node has no portable way to
exec by descriptor. A same-UID replacement landing inside the remaining
window is the same accepted boundary already documented for the fleet
helper.
* 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.
Twelve tests in launch.spec.ts, each written against a specific hole: safe
resolution; world-writable binary; safe binary under a world-writable
directory; no fall-through past an unsafe first match; relative PATH entry
ignored; symlink followed and real file validated; symlink to an unsafe target
refused; non-executable refused; directory sharing the name refused; a path
rather than a name refused; no PATH declared; not-found reported as not-found
rather than resolving something else.
One of those tests was written wrong first and is worth recording: creating the
open directory with `mkdirSync(path, { mode: 0o777 })` gets masked by the umask
to 0o755, so the case passed while testing nothing. It creates at 0o755 and
chmods after.
Verification: typecheck RC=0. Full package suite 1615 passed / 4 failed / 1619.
The four failures are the pre-existing host lease-identity leak into spawned
hooks, not this change -- the same spec re-run with only the five MOSAIC_LEASE_*
and MOSAIC_RUNTIME_GENERATION variables stripped from the environment, with no
code change, is 20/20.
Scope note: this commit carries the uncommitted D1/D4/D6 work already present in
the tree alongside D3, because it is interleaved in the same files and is one
amend package. D2 and D5 are not yet assessed.
Commit-only per scrappy's controlling packet (comms 20260813T212447Z dc43de):
not pushed, PR #1213 not updated, nothing re-authored.
|