fix(worker): admit the compartment payload pair so a secondmate leg can dispatch

[ruby-dlee]

The defect

A live secondmate compartment (azaccept) was admitted, assigned slot 2, and booted. Its first session leg then refused with exactly:

ELASTIC WORKER REFUSED: payload staging entry is not in the reviewed set: fm-secondmate-session.py

The producer and the validator disagreed about what a compartment stages.

• bin/fm-spawn.sh:4467-4468 copies two extra files into the payload for a KIND=secondmate spawn: fm-secondmate-session.py and fm-secondmate-spawn.pi-ext.ts.
• bin/fm-worker-lifecycle.py PAYLOAD_FILE_BOUNDS admitted only repo.bundle and brief.md, and staged_directory_manifest raises for any staged entry with no bound.

Every compartment leg dispatch has always refused. R2/R3 could never have worked. The reviewed-set check is correct; the allowlist was incomplete, so this PR does not weaken or remove that check.

The three design decisions

1. Bounds: 256 KiB and 64 KiB

entry	measured	bound	headroom
fm-secondmate-session.py	45,142 B	256 KiB (262,144)	5.8x
fm-secondmate-spawn.pi-ext.ts	3,867 B	64 KiB (65,536)	16.9x

The allowlist bounds what can be staged onto a worker, so a generous bound silently weakens a security control. Both numbers are chosen so that headroom buys against ordinary source growth and nothing else:

• The session runner gets 256 KiB, the same ceiling brief.md already carries. It is the one file here under active development, and a bound that a normal year of growth can trip is an outage waiting to happen. 256 KiB is still small enough that nothing but source text fits.
• The pi extension gets 64 KiB, a quarter of the text ceiling. It declares one spawn intent and is not expected to grow like the runner, so it gets the tighter number rather than inheriting the runner's.

Both are three orders of magnitude below repo.bundle's 512 MiB, so the marginal widening of the total staging surface is about 0.06 percent. The suite pins both numbers and asserts each file actually fits under its own bound today, so a bound can never be quietly set below the file it exists to admit.

2. Split by lane, do not flatten one set

PAYLOAD_FILE_BOUNDS stays exactly {repo.bundle, brief.md}. A new COMPARTMENT_PAYLOAD_FILE_BOUNDS adds the two files, and payload_contract(role) selects between them.

Admitting compartment-only files into one universal set would widen what any worker may receive, including ordinary crewmates that have no business receiving a session runner. The split costs one function and one dict; it buys keeping the ordinary lane exactly as narrow as it is today. That is the cheaper side of the trade by a wide margin.

The lane is chosen from the worker's durable role, resolved under the controller lock with the queue item and worker record in hand, never from the payload's own contents. A payload must never select the rules it is judged by. This moved the manifest computation from before the inventory call to inside the lock, which is why command_execute shows a hunk move; create_worker_record copies the item's role onto the worker, so the two are cross-checked and a disagreement fails closed.

That role is cloud-attested, not merely local JSON. expected_tags binds firstmate-role: secondmate-compartment into the VM's own cloud tags when the worker's role is secondmate; resources_exact compares those tags against what the cloud actually reports and returns a mismatch reason; classify_worker only reaches return "assigned" after that comparison passes; and command_execute refuses anything not classified assigned before it ever reads the role. So selecting the lane from worker["role"] is backed by the VM's attested tags, which is a materially stronger guarantee than "resolved under the controller lock" alone. (Chain verified by reading bin/fm-worker-lifecycle.py end to end, not assumed.)

3. Both files are REQUIRED, not merely admitted

The compartment monitor's leg-1 argv runs fm-secondmate-session.py and passes --pi-ext .../fm-secondmate-spawn.pi-ext.ts by literal path. A compartment whose staging silently lost either file would dispatch a leg that cannot possibly work, and the failure would surface on a booted VM instead of at the controller. staged_directory_manifest already takes required=(), so making them required costs nothing and turns a silent broken leg into a loud local refusal.

The deeper problem: three places, one contract

This is the same shape as the other drift defects. The anti-recurrence control is now effect-shaped, with a static companion.

These guards are DRIFT DETECTORS, not the security control. An earlier revision of this body claimed the effect-shaped check catches an unadmitted file "no matter how the staging was written". That is overstated and review proved it: a conditional non-literal site injected into the real fm-spawn.sh

if [ -f "$WT/data/extras.tar" ]; then
  tar -xf "$WT/data/extras.tar" -C "$STATE/$ID.cloud-payload" || exit 1
fi

slips both controls. It is text-blind to the static guard, and invisible to the effect-shaped one because that scenario never makes the condition true. Five of eleven attempted bypasses defeat the static guard, all of them forms that refer to the directory as a whole (variable alias, tar -C, subshell cd, cp -R src/., a PDIR variable). My comment calling such a reference "the directory ITSELF: stages nothing" was an assumption rather than a fact, and is corrected in this PR.

What they actually do: the effect-shaped check catches an unadmitted file no matter how the staging was written within the code path its scenario exercises, and the static guard fails closed on the sites it can read while making no claim about the ones it cannot.

The real control is elsewhere and is closed independently of both. staged_directory_manifest refuses any unadmitted entry at dispatch however it was spelled, and staged_directory_archive iterates only manifest entries and re-verifies sha256 and bytes before anything leaves the controller. Dotfiles are skipped by the validator but manifest-bound in the archive, so there is no hole there either. A future drifting staging site therefore reproduces the outage class this PR fixes, not unadmitted bytes reaching a guest.

The effect-shaped check. tests/fm-secondmate-cloud-monitor.test.sh already stages a real compartment payload through the real fm-spawn.sh. It now also runs the production validator over the actual staged directory, so it bounds what may be present at all rather than naming files it expects.

The static guard is kept, but fails closed. The first version of this guard was syntax-shaped and it was wrong. It matched only a literal basename written immediately after the literal directory path, so two spellings staged an unadmitted file while it printed green:

EVIL_NAME=evil.sh; cp ... "$STATE/$ID.cloud-payload/$EVIL_NAME"   -> was GREEN
cp "$SCRIPT_DIR/fm-brief.sh" "$STATE/$ID.cloud-payload/"          -> was GREEN

The second is the ordinary "copy into the directory" idiom, no contrivance at all. A guard whose whole purpose is catching drift, that recognizes only today's syntax, is the same class of weakness as the four defects it exists to catch. It now classifies every occurrence of the payload directory and goes red on any it cannot read, so its silence is never ambiguous between "nothing unadmitted" and "I could not parse that".

The third encoding is closed too. bin/fm-secondmate-cloud-monitor.sh:278,282 names these same two files by absolute guest path in the leg argv. Producer, validator and consumer are three places holding one contract. The guard now pins the argv basenames to the reviewed set in both directions: the argv cannot name a file the bounds do not admit, and cannot name a file fm-spawn.sh does not stage.

I still did not move the names into a shared data file that fm-spawn.sh parses. Reasons unchanged and open to being overruled: the bounds are a security control currently reviewed as literals directly above their enforcement point, and fm-spawn.sh stages with plain cp inside a umask 077 subshell with no interpreter in the hot path. With an effect-shaped check over the real directory plus a fail-closed static guard plus the argv pinned, the drift class is covered without that coupling.

Consequences of moving the manifest under the lock

Resolving the lane from the worker's durable role means the manifest is now computed inside the controller lock instead of before the inventory call. Two measured consequences, both judged non-blocking in review, disclosed here so a reader does not have to rediscover them:

1. Failure ordering changed for a payload that is both oversized and on an unassigned task. Base refused with payload staging entry exceeds its byte bound: repo.bundle and contacted no provider; head refuses with execute requires one exact assigned task generation after an inventory round-trip. (Measured by the reviewer; it follows directly from the reordering.) A caller sees a different, still-correct refusal and one extra read-only provider call.
2. Hashing cost inside the exclusive lock. Read plus sha256 of a repo.bundle at its full 512 MiB bound measures 0.275s on this host (independently measured here; the reviewer measured 0.22s). Callers wait on the lock rather than fail, and the refusal precedes make_action, slot_lease and claim_pending, so no slot is ever wedged by it.

Proof, executed

All runs used env -i with FM_*/AZURE*/ARM_* scrubbed (26 such vars were live in the ambient shell), no ~/.fm-azure/fleet.env, and the fixture provider. az vm list -g rg-firstmate-pilot-eastus-001 returns [], count 0: nothing was created.

1. Reproduced before fixing, driving the real staged_directory_manifest over a fixture payload holding all four names at the live byte sizes:

staged: ['brief.md', 'fm-secondmate-session.py', 'fm-secondmate-spawn.pi-ext.ts', 'repo.bundle']
bounds admit: ['brief.md', 'repo.bundle']
REFUSED: payload staging entry is not in the reviewed set: fm-secondmate-session.py

2. Passes after the fix, manifest carrying all four entries:

brief.md                       sha256=0198880ca7b99cb0620a18bdb7e8899d1a6d5b89b8c4e79cbfca7ce3067c5e28 bytes=4384
fm-secondmate-session.py       sha256=79ea7848860c6073d4bede1b55b105df758f29defac11f05b16f904b763698f9 bytes=45142
fm-secondmate-spawn.pi-ext.ts  sha256=b4ca4fd7b88c671b3c9e9708b6a418f27a8e54b73073f7c2f8b694db94b75c94 bytes=3867
repo.bundle                    sha256=d481103800df96a6aece158dea6e2b9ed5ef4bdd57a2bc542aefde689aec4fcd bytes=10042238

The suite additionally drives a real execute on a genuinely assigned role=secondmate compartment and asserts the digest-bound request the provider receives carries all four entries with matching sha256/bytes, so the new command_execute wiring is covered by an executed path and not only by the helper.

3. ADMIT-red mutations.

Mutation A, remove the two new bounds entries. Red in three independent places, each the defect itself:

• structural guard: bin/fm-spawn.sh stages payload entries the reviewed set does not admit: ['fm-secondmate-session.py', 'fm-secondmate-spawn.pi-ext.ts']
• with the guard neutered so the manifest assertions are reached: lifecycle.LifecycleError: payload staging entry is not in the reviewed set: fm-secondmate-session.py
• with the whole helper skipped so the real CLI is reached: AssertionError: ELASTIC WORKER REFUSED: payload staging entry is not in the reviewed set: fm-secondmate-spawn.pi-ext.ts, the live azaccept string.

Guard bypasses, the two that were proven to slip past the first version. Both now go red in both guards, each naming the unadmitted file:

bypass	static guard (fail closed)	effect-shaped check
.../cloud-payload/$EVIL_NAME	red: cannot classify these bin/fm-spawn.sh sites ... line 4469: EVIL_NAME=evil.sh; cp ...	red: fm-spawn.sh staged ['evil.sh'] into the compartment payload, which the reviewed set does not admit
cp src .../cloud-payload/	red: cannot classify these bin/fm-spawn.sh sites ... line 4469: cp "$SCRIPT_DIR/fm-brief.sh" "$STATE/$ID.cloud-payload/"	red: fm-spawn.sh staged ['fm-brief.sh'] into the compartment payload, which the reviewed set does not admit

Mutation B, stage a literal unadmitted name (fm-secondmate-tools.json): red naming it.

The role-disagreement refusal, which nothing pinned until now. Review deleted the check outright and both suites stayed fully green (37/37 and 50/50), so the one new behavior in this change was indistinguishable from its own absence. It is now reached through the real CLI by editing only the queue item in controller.json, leaving the worker record and its cloud-attested VM tags intact, which is exactly the drift it fails closed on. Two independent mutations prove it load-bearing:

mutation	result
delete the check outright	red: a worker/item role disagreement (author) was not refused
keep it but fail open on an absent item role	red: a worker/item role disagreement (None) was not refused

The second is the subtle one: it still refuses a disagreeing role and is caught only by the absent-role case. The unit also asserts neither refusal leaves a durable claim on the slot, and carries a positive control so the refusals are attributable to the disagreement and nothing else.

Mutation C, rename the file in the monitor's leg argv only, leaving the producer alone: the compartment leg argv names files the reviewed set does not admit, so the guest would be told to run something that never travels: ['fm-secondmate-spawn.ext.ts']

4. Byte bounds still bind, at bound and one past it:

fm-secondmate-session.py       at 262144 B: accepted   at 262145 B: REFUSED ... exceeds its byte bound
fm-secondmate-spawn.pi-ext.ts  at  65536 B: accepted   at  65537 B: REFUSED ... exceeds its byte bound
repo.bundle                    bound + 1:              REFUSED ... exceeds its byte bound

An unreviewed name (id_rsa) is still refused in the compartment lane too: the set widened by exactly two names, it did not open.

5. The ordinary crewmate lane is unchanged. Emitting the author-lane bounds, required set, and manifest over a byte-identical fixture on base and head and diffing them: identical, no output from diff -u. Separately, the base and head assertion lists differ by exactly one added line, with all 36 pre-existing assertions passing on head, including end_to_end_lifecycle, which runs the real crewmate execute with --payload-dir through the reordered code.

6. Suites run via python3 tests/run-one.py <test>, never bare bash.

• tests/fm-worker-lifecycle.test.sh: green on base (36) and on head (37).
• tests/fm-secondmate-cloud-monitor.test.sh: green on head (50 assertions), carrying the new effect-shaped check.
• tests/fm-worker-supervisor.test.sh: green, including "the supervisor stages digest-bound payload, account, and repository exactly".
• tests/fm-lint.test.sh: green with the pinned ShellCheck 0.11.0 resolved. shellcheck --norc -x on the changed test file is clean on base and head.
• The gap disclosed in the first round is closed: keeping HOME and scrubbing only the dangerous FM_*/AZURE*/ARM_* vars with env -u, rather than blanket env -i, both previously unrunnable suites pass. The earlier HOME: unbound variable failures were purely an artifact of the blanket scrub, and reproduced identically on base.

Rebased onto #280

Rebased onto d8154a61 after #280 merged. It applied with no conflicts: #280's account-pool work sits above my hunks in bin/fm-worker-lifecycle.py, and its test units are separate from mine. A clean rebase is not proof of semantic compatibility, so the assumption my change actually depends on was re-verified rather than assumed:

• create_worker_record still writes "role": item.get("role", "author") onto the worker record, which is what the lane selection reads under the lock. feat(worker): lease one Pi account per placement so concurrent crewmates never collide #280 changed account_binding to account_pool_home around it, but did not touch role.
• The end-to-end execute on a genuinely assigned role=secondmate compartment was re-run on top of feat(worker): lease one Pi account per placement so concurrent crewmates never collide #280 and re-proven load-bearing: with the two bounds entries removed and the unit block unregistered, the real CLI still refuses with ELASTIC WORKER REFUSED: payload staging entry is not in the reviewed set: fm-secondmate-spawn.pi-ext.ts.
• Both guard bypasses were re-proven red post-rebase, in both guards.
• fm-worker-placement, fm-spawn-cloud, fm-worker-supervisor, fm-secondmate-cloud-monitor and fm-worker-lifecycle all pass on the rebased head.

Scope

Touches bin/fm-worker-lifecycle.py, tests/fm-worker-lifecycle.test.sh, and tests/fm-secondmate-cloud-monitor.test.sh.

The effect-shaped assertion lives in tests/fm-secondmate-cloud-monitor.test.sh because that is the only place in the repo where a real fm-spawn.sh produces a real payload directory, and the whole point of the finding is that the check must inspect the effect rather than the syntax. #280 previously held that file and has now merged, so there is no longer any ownership contention.

The account staging call passes only total_bound and no per-name bounds, so it has no allowlist of this shape and cannot fail the same way. It is deliberately unchanged.