From 1ea309e14c1bea6310349cf7741dbed7c9e9a008 Mon Sep 17 00:00:00 2001 From: Dongkeun Lee Date: Fri, 21 Aug 2026 03:21:13 -0400 Subject: [PATCH 1/4] fix(spawn): persist every cloud-lane name the closed monitor pane must read A secondmate compartment could never create a child crewmate's worker. The controller admitted the request with every binding right, then the Azure provider refused: FM_AZURE_TENANT_ID is required and must be supplied out of band. SPAWN_CLOUD_ENV_ALLOWLIST is the set written into state/.cloud-env, which the monitors source in a subshell for every lifecycle call; their Herdr panes inherit nothing from the operator's shell, so a name absent from that file is unreachable in production. The allowlist carried 19 names and the deployment path reads 37. The gap was structural, not a typo. The allowlist's own regeneration recipe grepped only bin/fm-worker-lifecycle.py and bin/fm-azure-worker-provider.py, but the deployment runs in bin/fm-azure-pilot.sh, a SUBPROCESS the provider launches from run_pilot_create. Twelve names its require_cloud_environment refuses without, and the parameter file it builds, were invisible to that grep. Seven of them refuse outright; the rest, including the worker VM image id, would have drifted silently to a default. bin/fm-cloud-env-contract.py is now the one owner of that set: it derives the names from the readers, subtracts the seven the provider supplies per placement, and records secret-bearing exclusions explicitly so the allowlist stays a reviewed literal rather than a prefix glob. The new behavior test is effect-shaped. It derives the contract, exports a shape-valid value for every name, runs a real cloud spawn, then sources the persisted file in a scrubbed environment and compares value by value. It never names the reported variable, so a name added to a reader and not to the allowlist goes red on its own. --- bin/fm-cloud-env-contract.py | 152 +++++++++++++++++++++++++++++++++++ bin/fm-spawn.sh | 23 ++++-- docs/scripts.md | 1 + tests/fm-spawn-cloud.test.sh | 101 +++++++++++++++++++++++ 4 files changed, 270 insertions(+), 7 deletions(-) create mode 100755 bin/fm-cloud-env-contract.py diff --git a/bin/fm-cloud-env-contract.py b/bin/fm-cloud-env-contract.py new file mode 100755 index 00000000000..637dc714641 --- /dev/null +++ b/bin/fm-cloud-env-contract.py @@ -0,0 +1,152 @@ +#!/usr/bin/env python3 +"""The ONE derivation of the FM_AZURE_* names a cloud placement must persist. + +WHY THIS FILE EXISTS. `SPAWN_CLOUD_ENV_ALLOWLIST` in bin/fm-spawn.sh is the set +of variables written into `state/.cloud-env`, which the compartment and +crewmate monitors source in a subshell for every lifecycle call they make. Those +monitors run in Herdr panes whose environment is CLOSED: they inherit nothing +from the operator's shell, so a name missing from that file is unreachable in +production no matter what the operator exported. + +The names that file must carry are decided somewhere else entirely - in the code +that READS them on the far side of the pane boundary. Before this module the two +sides learned the set independently, and they drifted: the compartment-child +lane could never create its worker at all, because bin/fm-azure-pilot.sh refuses +`worker-create` without FM_AZURE_TENANT_ID (and six more names) that the +allowlist did not carry. Every hermetic test missed it, because the tests drive +a fixture provider that never shells out to the pilot. + +So: this module derives the required set FROM THE READERS, mechanically. The +allowlist stays an explicit literal in bin/fm-spawn.sh (an operator can audit one +line without running anything), and tests/fm-spawn-cloud.test.sh asserts the +PERSISTED FILE against this derivation, so a name added to a reader without +being added to the allowlist goes red. + +Regenerate the allowlist literal with: + + bin/fm-cloud-env-contract.py --allowlist + +Usage: + fm-cloud-env-contract.py one required name per line + fm-cloud-env-contract.py --allowlist one space-joined line for bin/fm-spawn.sh + fm-cloud-env-contract.py --explain each name with the readers that need it +""" + +from __future__ import annotations + +import pathlib +import re +import sys + +ROOT = pathlib.Path(__file__).resolve().parent.parent + +# The readers on the far side of the closed pane. Every FM_AZURE_* name any of +# them takes from the environment is a name the persisted file has to be able to +# carry, because the compartment-child lane reaches all three from the monitor: +# fm-spawn.sh -> fm-worker-lifecycle.py -> fm-azure-worker-provider.py -> +# fm-azure-pilot.sh (worker-create). The pilot is the one that was missed: it is +# a SUBPROCESS of the provider, so a grep of the provider alone never saw it. +READERS = ( + "bin/fm-azure-pilot.sh", + "bin/fm-azure-worker-provider.py", + "bin/fm-worker-lifecycle.py", +) + +# Names the provider SUPPLIES to the pilot itself, per placement, in +# run_pilot_create's env.update. Persisting an operator's copy of these would be +# inert at best and a stale override at worst, so they are subtracted - and they +# are read out of that call rather than listed here, so the subtraction cannot +# drift from it either. +SUPPLIER = "bin/fm-azure-worker-provider.py" + +# Reviewed exclusions: a FM_AZURE_* name a reader takes from the environment that +# must still NEVER be written to disk, because its VALUE is a credential (or +# names a file holding one). The allowlist exists for exactly this reason - it is +# not a prefix glob - and this tuple is where that judgment is recorded, one +# entry per name with the reason inline. +# +# Empty today, deliberately: no name any of the READERS takes is secret-bearing. +# The shape to expect is the validation lane's FM_AZURE_VALIDATION_*_KEY_FILE +# pair, which names key material and is excluded here by construction because no +# reader above reads it. +SECRET_BEARING_EXCLUSIONS: tuple[str, ...] = () + +NAME = re.compile(r"FM_AZURE_[A-Z0-9_]+") + + +class ContractError(RuntimeError): + pass + + +def read(relative: str) -> str: + path = ROOT / relative + try: + return path.read_text(encoding="utf-8") + except OSError as exc: + raise ContractError("reader {} is unreadable: {}".format(relative, exc)) + + +def reader_names() -> dict[str, set[str]]: + """Every FM_AZURE_* name each reader takes, keyed by name.""" + by_name: dict[str, set[str]] = {} + for relative in READERS: + found = set(NAME.findall(read(relative))) + if not found: + # A reader that suddenly matches nothing means the derivation broke, + # not that the lane stopped needing an environment. Fail loudly: a + # silently empty contract would make the guarding test vacuous. + raise ContractError( + "reader {} yielded no FM_AZURE_ names; the derivation is broken".format(relative) + ) + for name in found: + by_name.setdefault(name, set()).add(relative) + return by_name + + +def provider_supplied() -> set[str]: + source = read(SUPPLIER) + match = re.search(r"def run_pilot_create\(.*?env\.update\((\{.*?\})\)", source, re.S) + if match is None: + raise ContractError( + "run_pilot_create's env.update could not be located in {}; " + "the provider-supplied subtraction cannot be derived".format(SUPPLIER) + ) + supplied = set(NAME.findall(match.group(1))) + if not supplied: + raise ContractError("run_pilot_create supplies no FM_AZURE_ names; the derivation is broken") + return supplied + + +def required() -> dict[str, set[str]]: + by_name = reader_names() + for name in provider_supplied() | set(SECRET_BEARING_EXCLUSIONS): + by_name.pop(name, None) + if not by_name: + raise ContractError("the derived cloud-env contract is empty; the derivation is broken") + return by_name + + +def main(argv: list[str]) -> int: + mode = argv[1] if len(argv) > 1 else "" + if mode not in ("", "--allowlist", "--explain"): + print(__doc__.strip(), file=sys.stderr) + return 2 + try: + contract = required() + except ContractError as exc: + print("fm-cloud-env-contract: {}".format(exc), file=sys.stderr) + return 1 + names = sorted(contract) + if mode == "--allowlist": + print(" ".join(names)) + elif mode == "--explain": + for name in names: + print("{}\t{}".format(name, ",".join(sorted(contract[name])))) + else: + for name in names: + print(name) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main(sys.argv)) diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index d490affd9b9..32eaeaf17b2 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -4505,13 +4505,22 @@ spawn_cloud_record_account_placement() { # # the crewmate entrypoint (CLOUD_WORKER_LAUNCH, the exact launch string every # local backend uses) runs on the worker through a detached bounded # `execute`, whose digest-bound result lands in state/.worker-result.json. -# The exact worker-lane environment names read by bin/fm-worker-lifecycle.py -# and bin/fm-azure-worker-provider.py (regenerate with: -# grep -ohE 'FM_AZURE_[A-Z0-9_]+' bin/fm-worker-lifecycle.py \ -# bin/fm-azure-worker-provider.py | sort -u -# ). An explicit allowlist, not a prefix glob, so a future secret-bearing -# FM_AZURE_* variable can never land on disk silently. -SPAWN_CLOUD_ENV_ALLOWLIST='FM_AZURE_AUTHOR_CAPACITY_MODE FM_AZURE_CAPACITY_PROFILE FM_AZURE_DEPLOYMENT_GENERATION FM_AZURE_NAMING_PREFIX FM_AZURE_OWNER_TAG FM_AZURE_RESOURCE_GROUP FM_AZURE_STORAGE_NAME FM_AZURE_SUBSCRIPTION_ID FM_AZURE_WORKER_ADMISSION_HOURS FM_AZURE_WORKER_ALLOW_UNTRAINED_FORECAST FM_AZURE_WORKER_COMMISSIONING_CEILING_USD FM_AZURE_WORKER_COST_ATTRIBUTION FM_AZURE_WORKER_HOUR_PLANNING_THRESHOLD FM_AZURE_WORKER_IDLE_COOLDOWN_SECONDS FM_AZURE_WORKER_MAX FM_AZURE_WORKER_POLICY_PHASE FM_AZURE_WORKER_STATE_DIR FM_AZURE_WORKER_STEADY_TARGET_USD FM_AZURE_WORKER_WARM_IDLE' +# The exact cloud-lane environment names the far side of the closed pane reads. +# DERIVED, not hand-kept: bin/fm-cloud-env-contract.py is the one owner of that +# set, and tests/fm-spawn-cloud.test.sh asserts the PERSISTED file against it, +# so a name added to a reader without landing here goes red. Regenerate with: +# bin/fm-cloud-env-contract.py --allowlist +# The readers are bin/fm-worker-lifecycle.py, bin/fm-azure-worker-provider.py, +# AND bin/fm-azure-pilot.sh - the pilot is a SUBPROCESS of the provider +# (run_pilot_create), so the grep of the provider alone that used to regenerate +# this line never saw the twelve names the pilot's own require_cloud_environment +# refuses without. That omission made a compartment child's worker impossible to +# create on any machine, in any configuration, which no hermetic test could see +# because they all drive a fixture provider that never shells out to the pilot. +# Still an explicit literal and not a prefix glob: a secret-bearing FM_AZURE_* +# must never land on disk silently, and the contract records that judgment in +# its SECRET_BEARING_EXCLUSIONS rather than leaving it to a pattern. +SPAWN_CLOUD_ENV_ALLOWLIST='FM_AZURE_ADMIN_EMAIL FM_AZURE_ADMIN_SSH_PUBLIC_KEY FM_AZURE_ADMIN_USERNAME FM_AZURE_BUDGET_START_DATE FM_AZURE_CLEANUP_TIMEOUT_SECONDS FM_AZURE_DEPLOYMENT_GENERATION FM_AZURE_KEY_VAULT_NAME FM_AZURE_MUTATION_STATE_DIR FM_AZURE_NAMING_PREFIX FM_AZURE_OPERATOR_DATA_PLANE_IP FM_AZURE_OWNER_TAG FM_AZURE_PROTECT_DURABLE_STATE FM_AZURE_RESOURCE_GROUP FM_AZURE_RUNNER_OPERATOR_OBJECT_ID FM_AZURE_RUNNER_VALIDATION_SKU FM_AZURE_SECONDMATE_MAX FM_AZURE_STEADY_STATE_BUDGET_TARGET_USD FM_AZURE_STORAGE_NAME FM_AZURE_SUBSCRIPTION_ID FM_AZURE_TENANT_ID FM_AZURE_VM_FAMILY FM_AZURE_WORKER_ADMISSION_HOURS FM_AZURE_WORKER_ALLOW_UNTRAINED_FORECAST FM_AZURE_WORKER_COMMISSIONING_CEILING_USD FM_AZURE_WORKER_DAILY_BOUND_OVERRIDE FM_AZURE_WORKER_DAILY_BOUND_USD FM_AZURE_WORKER_HOUR_PLANNING_THRESHOLD FM_AZURE_WORKER_IDLE_COOLDOWN_SECONDS FM_AZURE_WORKER_IDLE_RELEASE_SECONDS FM_AZURE_WORKER_IMAGE_ID FM_AZURE_WORKER_MAX FM_AZURE_WORKER_POLICY_PHASE FM_AZURE_WORKER_SKUS FM_AZURE_WORKER_SLOTS FM_AZURE_WORKER_STATE_DIR FM_AZURE_WORKER_STEADY_TARGET_USD FM_AZURE_WORKER_WARM_IDLE' spawn_cloud_persist_convergence_artifacts() { # A queued request outlives this process, but the entrypoint argv and the # FM_AZURE_* identity environment exist only here. Persist both so the diff --git a/docs/scripts.md b/docs/scripts.md index f1c069ffd8d..a260273da41 100644 --- a/docs/scripts.md +++ b/docs/scripts.md @@ -36,6 +36,7 @@ The shared no-mistakes gate refusal used by every directly invocable mutating co | `fm-worker-supervisor.py` | Execute one exactly bound command and emit one bounded digest-bound guest result | | `fm-worker-authority.py` | Issue release receipts from ordinary endpoint, report, landing, account, and worktree authorities | | `fm-azure-worker-provider.py` | Reconcile exact Azure worker generations through the landed private foundation | +| `fm-cloud-env-contract.py` | Derive the FM_AZURE_* names a cloud placement must persist for the closed monitor pane | | `fm-spawn.sh` | Spawn, native-resume, or provider-neutrally continue crewmates while recording pre-metadata Treehouse acquisition ownership | | `fm-dispatch-select.sh` | Resolve a matched crew-dispatch rule through quota or the deferred legacy pool-summary branch | | `fm-account-directory.sh` | Select a direct Claude/Codex account directory and install its per-profile Herdr hook | diff --git a/tests/fm-spawn-cloud.test.sh b/tests/fm-spawn-cloud.test.sh index 66d7c0e5377..9b08a0b60ec 100755 --- a/tests/fm-spawn-cloud.test.sh +++ b/tests/fm-spawn-cloud.test.sh @@ -789,6 +789,106 @@ test_cloud_switch_off_and_on_share_the_same_base_metadata() { pass "cloud metadata stays additive over the local metadata shape" } +# A shape-valid value for one contract name. The SHAPES are what the readers +# accept (a phase enum, a bounded integer, a directory); the NAMES they are +# keyed off are pattern suffixes, never the specific variable this defect was +# reported as. A future contract name whose shape none of these fit fails the +# spawn below, which is red for a true reason and says so. +cloud_env_contract_sentinel() { # + case $1 in + # Mirrors of the values the rest of this suite's cloud lane runs with, so + # the fixture provider and the controller still agree with each other. + FM_AZURE_SUBSCRIPTION_ID) printf '%s' "$SUB" ;; + FM_AZURE_DEPLOYMENT_GENERATION) printf 'dep-one' ;; + FM_AZURE_OWNER_TAG) printf 'owner' ;; + FM_AZURE_NAMING_PREFIX) printf 'fmtest' ;; + FM_AZURE_WORKER_STATE_DIR) printf '%s' "$HOME_DIR/state/azure-workers" ;; + *_STATE_DIR) printf '%s' "$CASE_DIR/contract-state" ;; + *_POLICY_PHASE) printf 'commissioning' ;; + *_WORKER_MAX) printf '16' ;; + *_SECONDMATE_MAX) printf '2' ;; + *_BOUND_OVERRIDE) date -u +%Y-%m-%d ;; + *_ALLOW_UNTRAINED_FORECAST|*_PROTECT_DURABLE_STATE|*_WARM_IDLE) printf '0' ;; + # The commissioning ceiling is not a free knob: admission refuses any + # value other than the reviewed one. + *_CEILING_USD) printf '1500' ;; + *_HOURS) printf '24' ;; + *_SECONDS|*_USD|*_THRESHOLD) printf '900' ;; + *) printf 'fmcontract-%s' "$1" ;; + esac +} + +test_persisted_cloud_env_carries_the_whole_deployment_read_set() { + # THE CLOSED-PANE CONTRACT, asserted as an EFFECT. + # + # state/.cloud-env is the ONLY channel between the operator's shell and + # the compartment/crewmate monitors, whose Herdr panes inherit nothing. What + # that file must carry is decided on the far side, by the code that reads it + # when a lifecycle call reaches the provider and the provider shells out to + # bin/fm-azure-pilot.sh for the deployment. This test derives that read set + # from those readers (bin/fm-cloud-env-contract.py) and asserts the file the + # spawn ACTUALLY WROTE against it, value by value, through a source in a + # scrubbed environment - the same way a monitor reads it. + # + # Deliberately not a grep for any one variable: a name added to a reader and + # not to SPAWN_CLOUD_ENV_ALLOWLIST goes red here without this test ever + # having heard of it. That is the failure that had to reach a live Azure run + # before, because every provider in this suite is a fixture that never shells + # out to the pilot at all. + local record id out env_file required count name want got missing mismatched + id=cloud-env-c30 + record=$(make_cloud_case env-contract "$id") + read_cloud_case "$record" + required=$("$ROOT/bin/fm-cloud-env-contract.py") \ + || fail "the cloud-env contract could not be derived: $required" + count=$(printf '%s\n' "$required" | grep -c '^FM_AZURE_') + # Vacuity guard: a derivation that silently matched nothing would make every + # assertion below pass while proving nothing at all. + [ "$count" -ge 12 ] || fail "the cloud-env contract derived only $count names; the derivation is broken" + out=$( + while IFS= read -r name; do + [ -n "$name" ] || continue + export "$name=$(cloud_env_contract_sentinel "$name")" + done </dev/null 2>&1 || true; value=$2; printf "%s" "${!value-}"' \ + _ "$env_file" "$name") + if [ -z "$got" ]; then + missing="$missing $name" + elif [ "$got" != "$want" ]; then + mismatched="$mismatched $name" + fi + done < Date: Fri, 21 Aug 2026 04:43:53 -0400 Subject: [PATCH 2/4] fix(spawn): bound the cloud-env allowlist in both directions and anchor the derivation Review found the guard failed loud in the wrong direction. The test asserted contract subset-of persisted and never the reverse, and nothing else in the repo constrained SPAWN_CLOUD_ENV_ALLOWLIST, so adding real secret-bearing names to it with no reader stayed green, and SECRET_BEARING_EXCLUSIONS had no enforcement power at all. Worse, a bare comment in a reader pulled a name into the derived contract, and the remediation the failure printed steered the developer toward persisting it. The test now asserts EQUALITY. Its probe environment is the contract unioned with every name the allowlist currently spells, so an extra name that no reader asks for actually reaches the persisted file and fails there, on the file's own contents rather than on a source read. Both failure messages name SECRET_BEARING_EXCLUSIONS as the lever, and the extraction carries its own vacuity guard so a broken read cannot silently disarm the half of the test that looks for extras. The scan is now comment-blind: Python readers are tokenized, shell readers drop whole-line comments. That removes the steering at its source rather than arguing with it afterwards, and the derived set is unchanged at 37 names because no name was ever comment-only. It over-includes rather than under- includes by design, since a trailing shell comment still counts. provider_supplied is anchored to run_pilot_create's own body, top-level def to top-level def. The previous regex was unbounded at the start, so hoisting the env.update into a helper defined later in the same file matched the helper and returned an identical-looking 37-name set while the real deployment would have run at capacityProfile=foundation with every worker binding left unbound. That refactor now refuses by name. bin/fm-worker-lifecycle.sh joins the scanned readers. It is in the chain and names nothing today, so it was unguarded rather than safe. --- bin/fm-cloud-env-contract.py | 117 ++++++++++++++++++++++++++++++----- tests/fm-spawn-cloud.test.sh | 69 ++++++++++++++------- 2 files changed, 150 insertions(+), 36 deletions(-) diff --git a/bin/fm-cloud-env-contract.py b/bin/fm-cloud-env-contract.py index 637dc714641..cb083cfcbf9 100755 --- a/bin/fm-cloud-env-contract.py +++ b/bin/fm-cloud-env-contract.py @@ -34,9 +34,11 @@ from __future__ import annotations +import io import pathlib import re import sys +import tokenize ROOT = pathlib.Path(__file__).resolve().parent.parent @@ -50,8 +52,16 @@ "bin/fm-azure-pilot.sh", "bin/fm-azure-worker-provider.py", "bin/fm-worker-lifecycle.py", + # In the chain (bin/fm-spawn.sh invokes it as the lifecycle entrypoint) and + # carrying zero non-comment FM_AZURE_ reads today. Scanned anyway so it is + # guarded rather than merely currently harmless. + "bin/fm-worker-lifecycle.sh", ) +# Readers that legitimately name nothing today, so the empty-reader guard below +# does not mistake "guarded and quiet" for "the derivation broke". +MAY_BE_EMPTY = ("bin/fm-worker-lifecycle.sh",) + # Names the provider SUPPLIES to the pilot itself, per placement, in # run_pilot_create's env.update. Persisting an operator's copy of these would be # inert at best and a stale override at worst, so they are subtracted - and they @@ -61,14 +71,24 @@ # Reviewed exclusions: a FM_AZURE_* name a reader takes from the environment that # must still NEVER be written to disk, because its VALUE is a credential (or -# names a file holding one). The allowlist exists for exactly this reason - it is -# not a prefix glob - and this tuple is where that judgment is recorded, one -# entry per name with the reason inline. +# names a file holding one). One entry per name, reason inline. +# +# THIS TUPLE IS THE LEVER THAT KEEPS A NAME OUT. The scan is a regex and cannot +# tell a read from a mention, so code_text strips comments first - but that is a +# reduction of the hazard, not its removal: a trailing shell comment still +# counts, and a reader that genuinely reads a credential-valued name would pull +# it in legitimately. When the guarding test demands such a name be reachable +# from disk, the correct answer is an entry HERE, not a new allowlist entry, +# which is why that test names this tuple in its failure message and asserts the +# persisted set EQUALS the contract rather than merely contains it. # -# Empty today, deliberately: no name any of the READERS takes is secret-bearing. -# The shape to expect is the validation lane's FM_AZURE_VALIDATION_*_KEY_FILE -# pair, which names key material and is excluded here by construction because no -# reader above reads it. +# Empty today: no name any of the READERS currently takes is secret-bearing. +# That is a fact about today's readers, NOT a property of the scan. The fleet +# environment really does hold key material under this prefix - +# FM_AZURE_VALIDATION_CREDENTIAL_KEY_FILE, FM_AZURE_VALIDATION_WORKTREE_KEY_FILE +# and FM_AZURE_GITHUB_TOKEN_FILE (bin/fm-azure-validation.py) - and those stay +# out only because no scanned reader names them. A comment would be enough to +# change that, and this tuple is where the answer goes when it does. SECRET_BEARING_EXCLUSIONS: tuple[str, ...] = () NAME = re.compile(r"FM_AZURE_[A-Z0-9_]+") @@ -86,12 +106,44 @@ def read(relative: str) -> str: raise ContractError("reader {} is unreadable: {}".format(relative, exc)) +def code_text(relative: str, text: str) -> str: + """The reader's source with its COMMENTS removed. + + The scan is a regex over source text and cannot tell a read from a mention, + so without this a bare `# see FM_AZURE_CLIENT_SECRET` inside a reader would + pull that name into the contract and the guarding test would then demand it + be reachable from disk. Steering a developer toward persisting a credential + is the worst thing this module could do, so mentions are stripped before the + scan rather than argued about afterwards. + + Conservative on purpose, in the safe direction. Python is tokenized, which + is exact. Shell drops only whole-line comments, because a `#` inside a shell + string is not a comment and no cheap parse tells them apart; a trailing + `# ... FM_AZURE_X` therefore still counts as a read. That over-includes, + which costs a spurious contract entry that SECRET_BEARING_EXCLUSIONS can + answer - never under-includes, which would silently drop a real name and + recreate the outage this module exists to prevent. + """ + if relative.endswith(".py"): + try: + return "\n".join( + token.string + for token in tokenize.generate_tokens(io.StringIO(text).readline) + if token.type != tokenize.COMMENT + ) + except (tokenize.TokenError, IndentationError, SyntaxError) as exc: + raise ContractError("reader {} could not be tokenized: {}".format(relative, exc)) + return "\n".join( + line for line in text.splitlines() if not line.lstrip().startswith("#") + ) + + def reader_names() -> dict[str, set[str]]: """Every FM_AZURE_* name each reader takes, keyed by name.""" by_name: dict[str, set[str]] = {} for relative in READERS: - found = set(NAME.findall(read(relative))) - if not found: + found = set(NAME.findall(code_text(relative, read(relative)))) + if not found and relative not in MAY_BE_EMPTY: # A reader that suddenly matches nothing means the derivation broke, # not that the lane stopped needing an environment. Fail loudly: a # silently empty contract would make the guarding test vacuous. @@ -103,15 +155,52 @@ def reader_names() -> dict[str, set[str]]: return by_name +def run_pilot_create_body(source: str) -> str: + """Exactly run_pilot_create's own body, top-level def to top-level def. + + The slice matters more than it looks. An unanchored + search from the def to the first env.update will happily run PAST the + end of the function and match an `env.update` in some LATER helper, and + then the subtraction is computed from a call the pilot never receives. + Moving this block into a helper is an ordinary refactor, and the failure it + would cause is silent and severe: run_pilot_create would supply nothing, so + a real deployment would run at capacityProfile=foundation with all four + worker bindings left "unbound", while the contract still printed a + plausible set and every test stayed green. + """ + start = source.find("\ndef run_pilot_create(") + if start < 0: + raise ContractError( + "run_pilot_create is not defined in {}; the provider-supplied " + "subtraction cannot be derived".format(SUPPLIER) + ) + start += 1 + end = len(source) + for match in re.finditer(r"^(?:def |class )", source[start:], re.M): + offset = start + match.start() + if offset > start: + end = offset + break + return source[start:end] + + def provider_supplied() -> set[str]: - source = read(SUPPLIER) - match = re.search(r"def run_pilot_create\(.*?env\.update\((\{.*?\})\)", source, re.S) + # RAW source here, not code_text: the function slicer needs real line layout + # to find its top-level `def` boundaries, and tokenizing flattens it away. + # Comments are stripped from the matched dict below instead. + body = run_pilot_create_body(read(SUPPLIER)) + match = re.search(r"env\.update\((\{.*?\})\)", body, re.S) if match is None: raise ContractError( - "run_pilot_create's env.update could not be located in {}; " - "the provider-supplied subtraction cannot be derived".format(SUPPLIER) + "run_pilot_create's own body no longer contains an env.update; the " + "provider-supplied subtraction cannot be derived from {}. If that " + "call moved into a helper, point this function at the helper - do " + "NOT let the search widen past the function, which is how it would " + "silently read some other call's names.".format(SUPPLIER) ) - supplied = set(NAME.findall(match.group(1))) + supplied = set(NAME.findall("\n".join( + line for line in match.group(1).splitlines() if not line.lstrip().startswith("#") + ))) if not supplied: raise ContractError("run_pilot_create supplies no FM_AZURE_ names; the derivation is broken") return supplied diff --git a/tests/fm-spawn-cloud.test.sh b/tests/fm-spawn-cloud.test.sh index 9b08a0b60ec..a4e69511b4b 100755 --- a/tests/fm-spawn-cloud.test.sh +++ b/tests/fm-spawn-cloud.test.sh @@ -818,8 +818,8 @@ cloud_env_contract_sentinel() { # esac } -test_persisted_cloud_env_carries_the_whole_deployment_read_set() { - # THE CLOSED-PANE CONTRACT, asserted as an EFFECT. +test_persisted_cloud_env_matches_the_deployment_read_set_exactly() { + # THE CLOSED-PANE CONTRACT, asserted as an EFFECT, in BOTH directions. # # state/.cloud-env is the ONLY channel between the operator's shell and # the compartment/crewmate monitors, whose Herdr panes inherit nothing. What @@ -827,31 +827,52 @@ test_persisted_cloud_env_carries_the_whole_deployment_read_set() { # when a lifecycle call reaches the provider and the provider shells out to # bin/fm-azure-pilot.sh for the deployment. This test derives that read set # from those readers (bin/fm-cloud-env-contract.py) and asserts the file the - # spawn ACTUALLY WROTE against it, value by value, through a source in a - # scrubbed environment - the same way a monitor reads it. + # spawn ACTUALLY WROTE equals it. + # + # EQUALITY, not containment, and that is the whole point of the second half. + # A contract name the file cannot carry is an outage - that is the defect this + # test exists for. But an EXTRA name the file carries and no reader wants is + # the opposite failure and the more dangerous one: SPAWN_CLOUD_ENV_ALLOWLIST + # is what keeps a secret-bearing FM_AZURE_* off disk, and a containment-only + # assertion would let anyone widen it to FM_AZURE_GITHUB_TOKEN_FILE or the + # FM_AZURE_VALIDATION_*_KEY_FILE pair and stay green. So the probe environment + # is deliberately WIDER than the contract - it is the contract UNION every + # name the allowlist currently spells - and any probe name that survives into + # the file without a reader asking for it fails here. # # Deliberately not a grep for any one variable: a name added to a reader and - # not to SPAWN_CLOUD_ENV_ALLOWLIST goes red here without this test ever - # having heard of it. That is the failure that had to reach a live Azure run - # before, because every provider in this suite is a fixture that never shells - # out to the pilot at all. - local record id out env_file required count name want got missing mismatched + # not to the allowlist goes red here without this test ever having heard of + # it. That is the failure that had to reach a live Azure run before, because + # every provider in this suite is a fixture that never shells out to the + # pilot at all. + local record id out env_file required declared probe count name want got + local missing extra persisted mismatched id=cloud-env-c30 record=$(make_cloud_case env-contract "$id") read_cloud_case "$record" required=$("$ROOT/bin/fm-cloud-env-contract.py") \ || fail "the cloud-env contract could not be derived: $required" - count=$(printf '%s\n' "$required" | grep -c '^FM_AZURE_') + count=$(printf '%s\n' "$required" | grep -c '^FM_') # Vacuity guard: a derivation that silently matched nothing would make every # assertion below pass while proving nothing at all. [ "$count" -ge 12 ] || fail "the cloud-env contract derived only $count names; the derivation is broken" + # What the allowlist SPELLS, read only to widen the probe environment. It is + # never asserted against directly - the assertions below are all about the + # file the spawn wrote - but without it an extra allowlist name would simply + # be unset at spawn time and leave no trace to catch. + declared=$(sed -n "s/^SPAWN_CLOUD_ENV_ALLOWLIST='\(.*\)'$/\1/p" "$ROOT/bin/fm-spawn.sh" | tr ' ' '\n' | grep '^FM_') + count=$(printf '%s\n' "$declared" | grep -c '^FM_') + # Second vacuity guard: a failed extraction would silently narrow the probe + # back to the contract and disarm the extra-name half of this test. + [ "$count" -ge 12 ] || fail "only $count allowlist names could be read from bin/fm-spawn.sh; the probe environment would be too narrow to detect an extra name" + probe=$(printf '%s\n%s\n' "$required" "$declared" | grep '^FM_' | sort -u) out=$( while IFS= read -r name; do [ -n "$name" ] || continue export "$name=$(cloud_env_contract_sentinel "$name")" - done </dev/null 2>&1 || true; value=$2; printf "%s" "${!value-}"' \ _ "$env_file" "$name") - if [ -z "$got" ]; then - missing="$missing $name" - elif [ "$got" != "$want" ]; then - mismatched="$mismatched $name" - fi + [ "$got" = "$want" ] || mismatched="$mismatched $name" done < Date: Fri, 21 Aug 2026 05:52:25 -0400 Subject: [PATCH 3/4] fix(spawn): slice the supplier with ast and scope the cloud-env equality to the file Both round-two findings were the same species as the original defect: a comment asserting a safety the code did not provide. The provider-supplied slice is now taken by ast, from the top-level FunctionDef or AsyncFunctionDef named run_pilot_create, using end_lineno. The terminator regex it replaces had to enumerate the shapes that end a function and missed two, both of which reopened the hole it existed to close: `async def` was not in the terminator, and a run_pilot_create that is the LAST top-level def fell through to end-of-file and swallowed module-level code after it. Each returned the identical 37 names with rc=0 while the real deployment would have run at capacityProfile=foundation with every worker binding unbound. Both now refuse by name. Decorator, nested def, class-method decoy and a docstring containing "def " all still behave. The test's equality was scoped to the probe, not to the file: it intersected the file's contents with the probe before comparing, so any name written by a path other than the allowlist loop was outside the assertion entirely. The persist block really does have such paths, so they are now named in CLOUD_ENV_NON_ALLOWLIST_EXPORTS and every other name in the file is compared, whatever wrote it and whatever its prefix. A rogue export is also inert unless its variable is set, so the probe additionally carries every literal `export NAME=` the persist block can emit; the guard on that extraction caught its own first version reading one name of three. code_text no longer overstates itself. Tokenizing is exact for `#` only, and a name in a docstring or any string literal still enters the contract, which is the safe direction and now says so. Shell is no longer stripped at all: a `#` line inside a multi-line double-quoted string or an unquoted heredoc body genuinely expands, and dropping it would be an under-include, the one direction that silently loses a real name. The union is unchanged by that choice, and it retires MAY_BE_EMPTY along with the permanent quiet-exemption it granted bin/fm-worker-lifecycle.sh. --- bin/fm-cloud-env-contract.py | 139 +++++++++++++++++++---------------- tests/fm-spawn-cloud.test.sh | 49 ++++++++++-- 2 files changed, 121 insertions(+), 67 deletions(-) diff --git a/bin/fm-cloud-env-contract.py b/bin/fm-cloud-env-contract.py index cb083cfcbf9..7e32e712e53 100755 --- a/bin/fm-cloud-env-contract.py +++ b/bin/fm-cloud-env-contract.py @@ -34,6 +34,7 @@ from __future__ import annotations +import ast import io import pathlib import re @@ -45,23 +46,21 @@ # The readers on the far side of the closed pane. Every FM_AZURE_* name any of # them takes from the environment is a name the persisted file has to be able to # carry, because the compartment-child lane reaches all three from the monitor: -# fm-spawn.sh -> fm-worker-lifecycle.py -> fm-azure-worker-provider.py -> -# fm-azure-pilot.sh (worker-create). The pilot is the one that was missed: it is -# a SUBPROCESS of the provider, so a grep of the provider alone never saw it. +# fm-spawn.sh -> fm-worker-lifecycle.sh -> fm-worker-lifecycle.py -> +# fm-azure-worker-provider.py -> fm-azure-pilot.sh (worker-create). The pilot is +# the one that was missed: it is a SUBPROCESS of the provider, so a grep of the +# provider alone never saw it. READERS = ( "bin/fm-azure-pilot.sh", "bin/fm-azure-worker-provider.py", "bin/fm-worker-lifecycle.py", - # In the chain (bin/fm-spawn.sh invokes it as the lifecycle entrypoint) and - # carrying zero non-comment FM_AZURE_ reads today. Scanned anyway so it is - # guarded rather than merely currently harmless. + # In the chain: bin/fm-spawn.sh invokes it as the lifecycle entrypoint. All + # 16 of its FM_AZURE_ names sit in its header documentation and are covered + # by the readers above, so it adds nothing to the set - but it is scanned, + # not exempted, so the day it grows a real read the contract sees it. "bin/fm-worker-lifecycle.sh", ) -# Readers that legitimately name nothing today, so the empty-reader guard below -# does not mistake "guarded and quiet" for "the derivation broke". -MAY_BE_EMPTY = ("bin/fm-worker-lifecycle.sh",) - # Names the provider SUPPLIES to the pilot itself, per placement, in # run_pilot_create's env.update. Persisting an operator's copy of these would be # inert at best and a stale override at worst, so they are subtracted - and they @@ -107,35 +106,44 @@ def read(relative: str) -> str: def code_text(relative: str, text: str) -> str: - """The reader's source with its COMMENTS removed. + """The reader's source with its `#` COMMENTS removed, where that is safe. The scan is a regex over source text and cannot tell a read from a mention, so without this a bare `# see FM_AZURE_CLIENT_SECRET` inside a reader would pull that name into the contract and the guarding test would then demand it be reachable from disk. Steering a developer toward persisting a credential - is the worst thing this module could do, so mentions are stripped before the - scan rather than argued about afterwards. - - Conservative on purpose, in the safe direction. Python is tokenized, which - is exact. Shell drops only whole-line comments, because a `#` inside a shell - string is not a comment and no cheap parse tells them apart; a trailing - `# ... FM_AZURE_X` therefore still counts as a read. That over-includes, - which costs a spurious contract entry that SECRET_BEARING_EXCLUSIONS can - answer - never under-includes, which would silently drop a real name and - recreate the outage this module exists to prevent. + is the worst thing this module could do. + + WHAT THIS ACTUALLY DOES, precisely, because overstating it is the exact + mistake this module exists to catch: + + - Python is tokenized, and dropping COMMENT tokens is exact FOR `#`. It + is not a claim about mentions in general: a name inside a docstring or + any other string literal is still scanned and still enters the contract. + Docstrings are the natural place to document environment variables, so + that is a live case, not a corner. It over-includes, which is the safe + direction, and SECRET_BEARING_EXCLUSIONS is the answer when it matters. + + - Shell is NOT stripped at all, deliberately. A `#` in shell is only + sometimes a comment: inside a multi-line double-quoted string, or in an + unquoted heredoc body, a line beginning `#$VAR` genuinely expands, and + the obvious `line.lstrip().startswith("#")` filter drops it. That is an + UNDER-include, the one direction that silently loses a real name and + recreates the outage this module exists to prevent. No cheap shell parse + tells the cases apart, so the scan does not try: shell comments count as + reads. Costs a spurious contract entry at worst; the union is unchanged + by this choice today. """ - if relative.endswith(".py"): - try: - return "\n".join( - token.string - for token in tokenize.generate_tokens(io.StringIO(text).readline) - if token.type != tokenize.COMMENT - ) - except (tokenize.TokenError, IndentationError, SyntaxError) as exc: - raise ContractError("reader {} could not be tokenized: {}".format(relative, exc)) - return "\n".join( - line for line in text.splitlines() if not line.lstrip().startswith("#") - ) + if not relative.endswith(".py"): + return text + try: + return "\n".join( + token.string + for token in tokenize.generate_tokens(io.StringIO(text).readline) + if token.type != tokenize.COMMENT + ) + except (tokenize.TokenError, IndentationError, SyntaxError) as exc: + raise ContractError("reader {} could not be tokenized: {}".format(relative, exc)) def reader_names() -> dict[str, set[str]]: @@ -143,7 +151,7 @@ def reader_names() -> dict[str, set[str]]: by_name: dict[str, set[str]] = {} for relative in READERS: found = set(NAME.findall(code_text(relative, read(relative)))) - if not found and relative not in MAY_BE_EMPTY: + if not found: # A reader that suddenly matches nothing means the derivation broke, # not that the lane stopped needing an environment. Fail loudly: a # silently empty contract would make the guarding test vacuous. @@ -156,38 +164,45 @@ def reader_names() -> dict[str, set[str]]: def run_pilot_create_body(source: str) -> str: - """Exactly run_pilot_create's own body, top-level def to top-level def. - - The slice matters more than it looks. An unanchored - search from the def to the first env.update will happily run PAST the - end of the function and match an `env.update` in some LATER helper, and - then the subtraction is computed from a call the pilot never receives. - Moving this block into a helper is an ordinary refactor, and the failure it - would cause is silent and severe: run_pilot_create would supply nothing, so - a real deployment would run at capacityProfile=foundation with all four - worker bindings left "unbound", while the contract still printed a - plausible set and every test stayed green. + """Exactly run_pilot_create's own body, sliced by the PARSER. + + The slice matters more than it looks. Any text search from the def to the + first env.update will happily run PAST the end of the function and match an + env.update in some LATER helper, and then the subtraction is computed from a + call the pilot never receives. Moving that block into a helper is an + ordinary refactor, and the failure it would cause is silent and severe: + run_pilot_create would supply nothing, so a real deployment would run at + capacityProfile=foundation with all four worker bindings left "unbound", + while the contract still printed a plausible set and every test stayed + green. + + Sliced with ast rather than a terminator regex because a regex has to + enumerate the shapes that end a function, and the two it missed both + reopened exactly that hole: `async def` was not in the terminator, and a + run_pilot_create that is the LAST top-level def fell through to end-of-file + and swallowed any module-level code after it. ast.end_lineno knows where the + function ends without anyone having to list the ways it can. """ - start = source.find("\ndef run_pilot_create(") - if start < 0: - raise ContractError( - "run_pilot_create is not defined in {}; the provider-supplied " - "subtraction cannot be derived".format(SUPPLIER) - ) - start += 1 - end = len(source) - for match in re.finditer(r"^(?:def |class )", source[start:], re.M): - offset = start + match.start() - if offset > start: - end = offset - break - return source[start:end] + try: + module = ast.parse(source) + except SyntaxError as exc: + raise ContractError("{} could not be parsed: {}".format(SUPPLIER, exc)) + lines = source.splitlines() + for node in module.body: + if ( + isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) + and node.name == "run_pilot_create" + ): + return "\n".join(lines[node.lineno - 1:node.end_lineno]) + raise ContractError( + "run_pilot_create is not defined at module level in {}; the " + "provider-supplied subtraction cannot be derived".format(SUPPLIER) + ) def provider_supplied() -> set[str]: - # RAW source here, not code_text: the function slicer needs real line layout - # to find its top-level `def` boundaries, and tokenizing flattens it away. - # Comments are stripped from the matched dict below instead. + # RAW source here, not code_text: ast needs real line layout, and tokenizing + # flattens it away. Comments are stripped from the matched dict below. body = run_pilot_create_body(read(SUPPLIER)) match = re.search(r"env\.update\((\{.*?\})\)", body, re.S) if match is None: diff --git a/tests/fm-spawn-cloud.test.sh b/tests/fm-spawn-cloud.test.sh index a4e69511b4b..af2b3b876a6 100755 --- a/tests/fm-spawn-cloud.test.sh +++ b/tests/fm-spawn-cloud.test.sh @@ -789,6 +789,16 @@ test_cloud_switch_off_and_on_share_the_same_base_metadata() { pass "cloud metadata stays additive over the local metadata shape" } +# The names spawn_cloud_persist_convergence_artifacts writes into +# state/.cloud-env from paths OTHER than the SPAWN_CLOUD_ENV_ALLOWLIST loop. +# This list is the ONLY thing that excuses a name from the equality assertion +# below, and it is spelled out here rather than falling out of a filter so that +# adding an export somewhere else in that block is a visible decision instead of +# a silent exemption. The compartment block (FM_SECONDMATE_*) is listed even +# though a crewmate spawn never writes it, so the contract is stated once for +# both lanes rather than depending on which lane a test happens to drive. +CLOUD_ENV_NON_ALLOWLIST_EXPORTS='FM_SPAWN_CLOUD_WALL_SECONDS FM_WORKER_PROVIDER_COMMAND FM_SECONDMATE_LEG_SECONDS FM_SECONDMATE_POLL_SECONDS FM_SECONDMATE_IDLE_SECONDS FM_SECONDMATE_TTL_HOURS FM_SECONDMATE_CHILD_PROJECT' + # A shape-valid value for one contract name. The SHAPES are what the readers # accept (a phase enum, a bounded integer, a directory); the NAMES they are # keyed off are pattern suffixes, never the specific variable this defect was @@ -840,6 +850,17 @@ test_persisted_cloud_env_matches_the_deployment_read_set_exactly() { # name the allowlist currently spells - and any probe name that survives into # the file without a reader asking for it fails here. # + # THE COMPARISON IS SCOPED TO THE FILE, NOT TO THE PROBE. An earlier revision + # intersected the file's contents with the probe before comparing, which put + # every name written by a path OTHER than the allowlist loop outside the + # assertion entirely - a one-line `printf export FM_AZURE_CLIENT_SECRET` + # added anywhere else in spawn_cloud_persist_convergence_artifacts stayed + # green. The persist block really does have such paths (the wall, the + # provider-command override, the compartment leg block), so the exemption is + # spelled out by name in CLOUD_ENV_NON_ALLOWLIST_EXPORTS below and every + # other name in the file, whatever wrote it, is compared. A silent + # consequence of an intersect is how the last one hid. + # # Deliberately not a grep for any one variable: a name added to a reader and # not to the allowlist goes red here without this test ever having heard of # it. That is the failure that had to reach a live Azure run before, because @@ -865,7 +886,23 @@ test_persisted_cloud_env_matches_the_deployment_read_set_exactly() { # Second vacuity guard: a failed extraction would silently narrow the probe # back to the contract and disarm the extra-name half of this test. [ "$count" -ge 12 ] || fail "only $count allowlist names could be read from bin/fm-spawn.sh; the probe environment would be too narrow to detect an extra name" - probe=$(printf '%s\n%s\n' "$required" "$declared" | grep '^FM_' | sort -u) + # Every name the persist block emits as a LITERAL `export NAME=` line, from + # any path, not just the allowlist loop. Also probe-widening only, never + # asserted against: a rogue export is written only when its variable is set, + # so without setting it the rogue line is inert and the file never shows it. + # That inertness is exactly what let a hand-added + # `printf 'export FM_AZURE_CLIENT_SECRET=%q\n'` pass review round two. + local emitted + # grep -o, not an anchored sed: these printfs appear after `||` and inside + # case arms, so a line-start anchor sees one of the three and the guard below + # is what caught that. The `%s` form (the allowlist loop itself) has no + # literal name and is deliberately not matched. + emitted=$(grep -o "printf 'export [A-Za-z_][A-Za-z0-9_]*=" "$ROOT/bin/fm-spawn.sh" \ + | sed "s/^printf 'export //; s/=$//" | sort -u) + count=$(printf '%s\n' "$emitted" | grep -c '^[A-Za-z_]') + # Third vacuity guard, same reason as the other two. + [ "$count" -ge 2 ] || fail "only $count literal export names could be read from bin/fm-spawn.sh; the probe environment would miss a non-allowlist export path" + probe=$(printf '%s\n%s\n%s\n' "$required" "$declared" "$emitted" | grep '^[A-Za-z_]' | sort -u) out=$( while IFS= read -r name; do [ -n "$name" ] || continue @@ -882,16 +919,18 @@ PROBE expect_code 0 $? "the contract cloud spawn should succeed: $out" env_file="$HOME_DIR/state/$id.cloud-env" assert_present "$env_file" "the cloud spawn persisted no environment for the closed pane" - # What the file actually carries, restricted to the probe: the spawn also - # persists wall/provider-command lines this contract has no opinion about. + # EVERY name the file carries, minus the explicitly named non-allowlist + # exports. Nothing is filtered by the probe here, so a name written by any + # other path in the persist block - allowlisted or not, FM_AZURE_ or not - + # lands in the comparison and has to be accounted for. persisted=$(sed -n 's/^export \([A-Za-z_][A-Za-z0-9_]*\)=.*/\1/p' "$env_file" | sort -u \ - | comm -12 - <(printf '%s\n' "$probe")) + | comm -23 - <(printf '%s\n' "$CLOUD_ENV_NON_ALLOWLIST_EXPORTS" | tr ' ' '\n' | grep . | sort -u)) missing=$(comm -23 <(printf '%s\n' "$required" | sort -u) <(printf '%s\n' "$persisted") | tr '\n' ' ') extra=$(comm -13 <(printf '%s\n' "$required" | sort -u) <(printf '%s\n' "$persisted") | tr '\n' ' ') missing=${missing% } extra=${extra% } [ -z "$missing" ] || fail "the persisted cloud-env cannot reach the deployment path: the closed pane never sees $missing (regenerate SPAWN_CLOUD_ENV_ALLOWLIST in bin/fm-spawn.sh with bin/fm-cloud-env-contract.py --allowlist; if a name is here only because some reader MENTIONS it and its value would be a credential, the answer is an entry in SECRET_BEARING_EXCLUSIONS in bin/fm-cloud-env-contract.py, NOT a new allowlist entry)" - [ -z "$extra" ] || fail "the persisted cloud-env writes names no reader on the deployment path asks for: $extra (SPAWN_CLOUD_ENV_ALLOWLIST is what keeps a secret-bearing FM_AZURE_* off disk; drop them, or add the reader that needs them, or record the judgment in SECRET_BEARING_EXCLUSIONS in bin/fm-cloud-env-contract.py)" + [ -z "$extra" ] || fail "the persisted cloud-env writes names no reader on the deployment path asks for: $extra (SPAWN_CLOUD_ENV_ALLOWLIST is what keeps a secret-bearing FM_AZURE_* off disk; drop them, or add the reader that needs them, or record the judgment in SECRET_BEARING_EXCLUSIONS in bin/fm-cloud-env-contract.py. If one is a DELIBERATE non-allowlist export from another path in spawn_cloud_persist_convergence_artifacts, name it in CLOUD_ENV_NON_ALLOWLIST_EXPORTS in this file - deliberately, in the open, never by widening a filter)" mismatched= while IFS= read -r name; do [ -n "$name" ] || continue From 99706e2d3b798eab4bf4358721ae3a36fbf75a1b Mon Sep 17 00:00:00 2001 From: Dongkeun Lee Date: Fri, 21 Aug 2026 06:35:07 -0400 Subject: [PATCH 4/4] test(spawn): keep the contract refusal reason in the cloud-env failure message Found by re-driving the red directions through the sealed entry point rather than through run-one.py. Every ContractError the derivation raises goes to stderr, and the assertion captured stdout only, so a derivation that refuses produced "the cloud-env contract could not be derived:" with nothing after the colon. The async-def hoist was red for the right reason and said nothing about what it was. stderr now goes to its own file and into the message. Not folded into the name list, because that would put refusal text where names are expected. --- tests/fm-spawn-cloud.test.sh | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/fm-spawn-cloud.test.sh b/tests/fm-spawn-cloud.test.sh index af2b3b876a6..ec0e796e104 100755 --- a/tests/fm-spawn-cloud.test.sh +++ b/tests/fm-spawn-cloud.test.sh @@ -871,8 +871,14 @@ test_persisted_cloud_env_matches_the_deployment_read_set_exactly() { id=cloud-env-c30 record=$(make_cloud_case env-contract "$id") read_cloud_case "$record" - required=$("$ROOT/bin/fm-cloud-env-contract.py") \ - || fail "the cloud-env contract could not be derived: $required" + # stderr is captured separately, not folded in: every ContractError the + # derivation raises goes to stderr, so folding it into $required would put the + # refusal text into the name list, and dropping it leaves the failure reading + # "could not be derived:" with nothing after the colon. A sealed re-drive of + # the async-def hoist produced exactly that empty message. + local contract_err="$CASE_DIR/contract.err" + required=$("$ROOT/bin/fm-cloud-env-contract.py" 2>"$contract_err") \ + || fail "the cloud-env contract could not be derived: $(cat "$contract_err" 2>/dev/null)" count=$(printf '%s\n' "$required" | grep -c '^FM_') # Vacuity guard: a derivation that silently matched nothing would make every # assertion below pass while proving nothing at all.