diff --git a/bin/fm-worker-lifecycle.py b/bin/fm-worker-lifecycle.py index fccf9950121..6c85875510f 100755 --- a/bin/fm-worker-lifecycle.py +++ b/bin/fm-worker-lifecycle.py @@ -3558,13 +3558,49 @@ def command_capacity_release(env, args): print("specialized capacity reservation released after exact zero-compute proof") +# What an ORDINARY crewmate payload may contain: the repository as a +# credential-free bundle, plus the one task file its entrypoint reads. This set +# is deliberately NOT widened for the compartment lane; see below. PAYLOAD_FILE_BOUNDS = { "repo.bundle": 512 * 1024 * 1024, "brief.md": 256 * 1024, } +PAYLOAD_REQUIRED = ("repo.bundle", "brief.md") +# What a SECONDMATE COMPARTMENT payload may contain: the ordinary set plus the +# two files bin/fm-spawn.sh stages only for KIND=secondmate - the session runner +# and the spawn-intent pi extension, which the compartment monitor's leg argv +# names by path at /mnt/task/.fm-task/. Bounds are the smallest round numbers +# leaving real headroom over the measured sizes (45142 B and 3867 B on the +# azaccept compartment): a bound is a security control, so headroom buys against +# ordinary source growth, not against a file becoming a different KIND of thing. +COMPARTMENT_PAYLOAD_FILE_BOUNDS = { + **PAYLOAD_FILE_BOUNDS, + "fm-secondmate-session.py": 256 * 1024, + "fm-secondmate-spawn.pi-ext.ts": 64 * 1024, +} +# Both are REQUIRED, not merely admitted: the leg argv runs the runner and +# passes --pi-ext, so a compartment whose staging silently lost either file +# would dispatch a leg that cannot work. Refuse at the controller instead. +COMPARTMENT_PAYLOAD_REQUIRED = PAYLOAD_REQUIRED + ( + "fm-secondmate-session.py", + "fm-secondmate-spawn.pi-ext.ts", +) ACCOUNT_TOTAL_BOUND = 1024 * 1024 +def payload_contract(role): + """The one owner of "what may a payload for this lane contain". + + Returns (bounds, required) for the worker's durable role. Splitting by lane + rather than flattening one set keeps the ordinary crewmate lane exactly as + narrow as it is today: an author worker that somehow staged a session + runner is still refused. + """ + if role == "secondmate": + return COMPARTMENT_PAYLOAD_FILE_BOUNDS, COMPARTMENT_PAYLOAD_REQUIRED + return PAYLOAD_FILE_BOUNDS, PAYLOAD_REQUIRED + + def staged_directory_manifest(label, directory, bounds=None, total_bound=None, required=()): """Digest one flat staging directory into {name: {sha256, bytes}}. @@ -3626,14 +3662,6 @@ def command_execute(env, args): if outcome_root.is_symlink() or not outcome_root.is_dir(): raise LifecycleError("outcome directory is unavailable: {}".format(args.outcome_dir)) payload_manifest = account_manifest = None - if args.payload_dir is not None: - payload_manifest = staged_directory_manifest( - "payload", args.payload_dir, bounds=PAYLOAD_FILE_BOUNDS, - required=("repo.bundle", "brief.md"), - ) - account_manifest = staged_directory_manifest( - "account", args.account_dir, total_bound=ACCOUNT_TOTAL_BOUND, - ) inventory = provider_call(env, "inventory")["inventory"] with contextlib.ExitStack() as stack: with controller_lock(env): @@ -3651,6 +3679,24 @@ def command_execute(env, args): classification, reason = classify_worker(worker, cloud) if classification != "assigned": raise LifecycleError("execute refuses a non-assigned or ambiguous worker: {}".format(reason)) + if args.payload_dir is not None: + # The staging contract is chosen by the worker's DURABLE role, so it + # is resolved here (under the one controller lock, with the queue + # item and worker record in hand) rather than from the payload's own + # contents - a payload must never select the rules it is judged by. + # create_worker_record copies the item's role onto the worker, so a + # disagreement means the two records have drifted: fail closed. + worker_role = worker.get("role", "author") + if worker_role != item.get("role", "author"): + raise LifecycleError( + "execute refuses a worker whose role disagrees with its queue item") + bounds, required = payload_contract(worker_role) + payload_manifest = staged_directory_manifest( + "payload", args.payload_dir, bounds=bounds, required=required, + ) + account_manifest = staged_directory_manifest( + "account", args.account_dir, total_bound=ACCOUNT_TOTAL_BOUND, + ) request = { "schema": EXECUTION_SCHEMA, **worker["bindings"], diff --git a/tests/fm-secondmate-cloud-monitor.test.sh b/tests/fm-secondmate-cloud-monitor.test.sh index e8ae59f0458..eef59f6bea5 100755 --- a/tests/fm-secondmate-cloud-monitor.test.sh +++ b/tests/fm-secondmate-cloud-monitor.test.sh @@ -2898,6 +2898,42 @@ PY assert_present "$SP_HOME/state/helm.cloud-payload/brief.md" "payload lacks the brief" assert_present "$SP_HOME/state/helm.cloud-payload/fm-secondmate-session.py" "payload lacks the session runner" assert_present "$SP_HOME/state/helm.cloud-payload/fm-secondmate-spawn.pi-ext.ts" "payload lacks the pi extension" + # EFFECT-shaped, not syntax-shaped: run the REAL lifecycle validator over the + # REAL directory the REAL fm-spawn.sh just produced. The assertions above name + # files they expect to be present; this one bounds what may be present at all, + # so a producer that stages an unadmitted file goes red here no matter HOW the + # staging was spelled (literal name, shell variable, trailing-slash cp). That + # distinction matters: the defect this closes was a producer and a validator + # drifting apart, and a guard that recognizes only today's syntax is the same + # class of weakness as the defect. + python3 - "$SP_HOME/state/helm.cloud-payload" "$ROOT/bin/fm-worker-lifecycle.py" \ + <<'PY' || fail "the staged compartment payload is not admitted by the reviewed set" +import importlib.util +import sys +from pathlib import Path + +payload = Path(sys.argv[1]) +spec = importlib.util.spec_from_file_location("lifecycle", sys.argv[2]) +module = importlib.util.module_from_spec(spec) +spec.loader.exec_module(module) +bounds, required = module.payload_contract("secondmate") + +# Deliberately NOT filtering dotfiles, though staged_directory_manifest skips +# them: nothing should ever put a dotfile here, so this is stricter than the +# validator on purpose and a stray one goes red instead of travelling unseen. +staged = sorted(entry.name for entry in payload.iterdir()) +unadmitted = [name for name in staged if name not in bounds] +assert not unadmitted, ( + "fm-spawn.sh staged {} into the compartment payload, which the reviewed set " + "does not admit; every leg dispatch would refuse. Staged: {}".format( + unadmitted, staged)) + +# The validator itself is the authority, so this cannot drift from what the +# controller enforces at dispatch: it bounds bytes and requires the pair too. +manifest = module.staged_directory_manifest( + "payload", payload, bounds=bounds, required=required) +assert sorted(manifest) == staged, (sorted(manifest), staged) +PY assert_present "$SP_HOME/state/helm.cloud-account/auth.json" "account staging lacks the auth projection" # Durable leg config rode into the persisted compartment environment. assert_grep 'FM_SECONDMATE_LEG_SECONDS=7200' "$SP_HOME/state/helm.cloud-env" "leg config was not persisted for the monitor" diff --git a/tests/fm-worker-lifecycle.test.sh b/tests/fm-worker-lifecycle.test.sh index 63429252f6c..0a2c85c7595 100755 --- a/tests/fm-worker-lifecycle.test.sh +++ b/tests/fm-worker-lifecycle.test.sh @@ -983,6 +983,9 @@ if request["operation"] == "mutate": "type": action["type"], "slot": action["slot"], "key": key, "outcome_expected": bool((action.get("request") or {}).get("outcome_expected")), "outcome_dir": action.get("outcome_dir"), + # Additive: the digest-bound staged manifest the provider actually + # receives, so a caller can assert what a lane staged. + "payload_files": (action.get("request") or {}).get("payload_files"), }) if key in state["seen"]: result = state["seen"][key] @@ -3591,6 +3594,111 @@ assert state["queue"]["smc-1@gen-s1"]["status"] == "assigned" smc_slot = str(state["queue"]["smc-1@gen-s1"]["slot"]) assert state["workers"][smc_slot]["role"] == "secondmate" +# A REAL compartment leg dispatch through command_execute: this is the path +# that refused on the live azaccept run with "payload staging entry is not in +# the reviewed set: fm-secondmate-session.py", because command_execute judged a +# compartment payload against the ordinary crewmate set. The lane now comes +# from the worker's durable role, resolved under the controller lock. +import hashlib +smc_assignment = state["workers"][smc_slot]["assignment_generation"] +staging = Path(env["FM_HOME"]) / "compartment-staging" +payload_dir = staging / "payload" +account_dir = staging / "account" +for directory in (payload_dir, account_dir): + directory.mkdir(parents=True) +(account_dir / "auth.json").write_text("{}\n") +COMPARTMENT_PAYLOAD = { + "repo.bundle": b"bundle-fixture", + "brief.md": b"brief\n", + "fm-secondmate-session.py": b"# session runner\n", + "fm-secondmate-spawn.pi-ext.ts": b"// spawn intent\n", +} + + +def stage_compartment(payload): + for stale in payload_dir.iterdir(): + stale.unlink() + for name, body in payload.items(): + (payload_dir / name).write_bytes(body) + + +def compartment_execute(check=True): + return run( + "execute", "--task", "smc-1", "--task-generation", "gen-s1", + "--assignment-generation", smc_assignment, "--wall-seconds", "60", + "--payload-dir", str(payload_dir), "--account-dir", str(account_dir), + "--confirm-execute", "--confirm-subscription", env["FM_AZURE_SUBSCRIPTION_ID"], + "--", "/usr/bin/true", check=check) + + +# Refusals first: each is raised before any provider claim is minted, so the +# slot is never wedged by one. +for missing in ("fm-secondmate-session.py", "fm-secondmate-spawn.pi-ext.ts"): + stage_compartment({name: body for name, body in COMPARTMENT_PAYLOAD.items() + if name != missing}) + refused = compartment_execute(check=False) + assert refused.returncode != 0 and "lacks required {}".format(missing) in refused.stderr, ( + refused.stderr) +stage_compartment(dict(COMPARTMENT_PAYLOAD, **{"id_rsa": b"key"})) +refused = compartment_execute(check=False) +assert refused.returncode != 0 and "not in the reviewed set: id_rsa" in refused.stderr, refused.stderr +stage_compartment(dict(COMPARTMENT_PAYLOAD, **{ + "fm-secondmate-spawn.pi-ext.ts": b"x" * (64 * 1024 + 1)})) +refused = compartment_execute(check=False) +assert refused.returncode != 0 and ( + "exceeds its byte bound: fm-secondmate-spawn.pi-ext.ts" in refused.stderr), refused.stderr + +# ...and the exact live compartment payload now dispatches, with the +# digest-bound request carrying all four staged entries. +stage_compartment(COMPARTMENT_PAYLOAD) +dispatched = json.loads(compartment_execute().stdout) +assert dispatched["schema"] == "fm.worker-execution-result/v1", dispatched +executed = [entry for entry in + json.loads(Path(env["FIXTURE_STATE"]).read_text())["calls"] + if entry["type"] == "execute"][-1] +assert sorted(executed["payload_files"]) == sorted(COMPARTMENT_PAYLOAD), executed +for name, body in COMPARTMENT_PAYLOAD.items(): + assert executed["payload_files"][name] == { + "sha256": hashlib.sha256(body).hexdigest(), "bytes": len(body)}, executed + +# The lane is selected from the worker's DURABLE role, and the two records that +# carry it must agree. create_worker_record copies the item's role onto the +# worker, so a disagreement means durable state has drifted and the controller +# must not guess which side is right. Reached through the real CLI by editing +# ONLY the queue item, leaving the worker record and its cloud-attested VM tags +# intact, which is exactly the drift this fails closed on. +controller_file = Path(env["FM_HOME"]) / "state/azure-workers/controller.json" + + +def rewrite_item_role(value): + durable = json.loads(controller_file.read_text()) + item = durable["queue"]["smc-1@gen-s1"] + if value is None: + item.pop("role", None) + else: + item["role"] = value + # The worker record keeps role=secondmate; only the queue item moves. + assert durable["workers"][smc_slot]["role"] == "secondmate", durable["workers"][smc_slot] + controller_file.write_text(json.dumps(durable, sort_keys=True, separators=(",", ":"))) + + +stage_compartment(COMPARTMENT_PAYLOAD) +for drifted in ("author", None): + rewrite_item_role(drifted) + refused = compartment_execute(check=False) + assert refused.returncode != 0 and "role disagrees with its queue item" in refused.stderr, ( + "a worker/item role disagreement ({}) was not refused: {}".format( + drifted, refused.stderr)) + # The refusal precedes make_action/slot_lease/claim_pending, so it must not + # have wedged the slot or left a durable claim behind. + assert smc_slot not in controller_state()["pending_actions"], controller_state()["pending_actions"] + +# Positive control: with the records agreeing again the SAME call succeeds, so +# the refusals above are caused by the disagreement and nothing else. +rewrite_item_role("secondmate") +agreed = compartment_execute() +assert json.loads(agreed.stdout)["schema"] == "fm.worker-execution-result/v1", agreed.stdout + # The compartment cap (default 2): a third compartment refuses. run("request", "--task", "smc-2", "--task-generation", "gen-s2", "--home-binding", binding(31), "--account-binding", binding(32), @@ -6812,7 +6920,192 @@ PY pass "idle workers deallocate unattended at the threshold, are loudly listed, and still exit through the ordinary release" } +compartment_payload_contract() { + # The producer (bin/fm-spawn.sh) and this validator encode the same contract. + # They drifted once: fm-spawn.sh staged the compartment session runner and pi + # extension, PAYLOAD_FILE_BOUNDS admitted neither, and every compartment leg + # dispatch refused with "payload staging entry is not in the reviewed set". + # The first block is the structural guard that makes that drift a red test + # instead of a booted VM that cannot work. + python3 - "$CONTROLLER" "$ROOT/bin/fm-spawn.sh" "$ROOT/bin/fm-secondmate-cloud-monitor.sh" \ + <<'PY' || fail "compartment payload contract failed" +import hashlib +import importlib.util +import re +import sys +import tempfile +from pathlib import Path + +spec = importlib.util.spec_from_file_location("lifecycle", sys.argv[1]) +module = importlib.util.module_from_spec(spec) +spec.loader.exec_module(module) +spawn = Path(sys.argv[2]).read_text(encoding="utf-8") +monitor = Path(sys.argv[3]).read_text(encoding="utf-8") + +# STRUCTURAL GUARD, FAIL CLOSED: every basename bin/fm-spawn.sh writes into the +# cloud payload directory must be admitted by the compartment bounds. +# +# SCOPE: this is a DRIFT DETECTOR, not the security control. It reads text, so +# any staging that refers to the directory as a whole (tar -C, cd, cp -R src/., +# a variable alias) is invisible to it; it fails closed on the ones it can see +# but cannot claim to see all of them. The control that actually bounds what +# reaches a guest is staged_directory_manifest at dispatch, which refuses an +# unadmitted entry however it was spelled. +# +# This classifies EVERY occurrence of the payload directory and refuses the ones +# it cannot read, because a guard whose silence is ambiguous between "nothing +# unadmitted" and "I could not parse that" is not a control at all. An earlier +# version matched only a literal basename written straight after the literal +# directory path, so `.../cloud-payload/$NAME` and the ordinary +# `cp src .../cloud-payload/` idiom both slipped past it while it printed green. +# The authoritative check is effect-shaped and lives in +# tests/fm-secondmate-cloud-monitor.test.sh, which runs this same validator over +# the directory a real fm-spawn.sh actually produced; this one is the cheap +# static companion that names the offending line. +spawn_lines = spawn.splitlines() +staged_names = set() +unreadable = [] +for site in re.finditer(r"\.cloud-payload", spawn): + tail = spawn[site.end():] + if tail[:1] == '"': + # The directory as a WHOLE. Today's sites are install -d, rm -rf and + # --payload-dir, none of which stage a file. That is a property of the + # current callers, NOT of the form: `tar -C