feat(crosscheck): refuse a model image that does not attest the dispatched harness - #276
Conversation
|
Recorded after merge: the honest limit in this PR's body saying the two ARM api-version strings were never exercised against live ARM is now RETIRED. An independent review exercised them read-only against the real resources. Both api-versions are valid: Against the configured image, the guard behaves as intended on real data:
One consequence worth stating because it changes how the code should be read: the source-follow is LOAD-BEARING IN PRODUCTION, not a corner case. The live gallery version carries no tags at all, so every real dispatch takes two GETs and the follow is the step that actually proves the harness. Two follow-ups the same review named, neither blocking and neither implemented here:
|
The gap and its history
The Azure crosscheck compartment lane boots a model VM from the gallery image named by
model_image_idand dispatches a reviewer harness (piorcodex) onto it. The image build writes attestation tags onto the managed image it distributes:pi-tarball-sha256,node-tarball-sha256,codex-cli-sha256,claude-cli-sha256, every digest taken from the pinned closure indocs/azure-crosscheck/model-image-closure.jsonthroughdistribute.artifactTagsindocs/azure-crosscheck/model-image.json.Nothing read them. PR #246 recorded this as an open gap, and
docs/azure-crosscheck.mdstill carried the line "nothing reads those tags" onmain.#246 exists because of what that costs. Every Pi reviewer that reached a live model VM died on
pi: command not found, one paid VM per attempt, because admission never compared the harness it was about to dispatch against what the configured image actually carries. The tag was the only thing that could have caught it, and it was write-only.The lane is currently off (
"enabled": falsein$FM_HOME/config/crosscheck-azure.json), which is what makes this the right moment to close the read: flippingenabledback to true is exactly the event this guard exists for.Note on the current image, so this PR does not re-propagate a refuted claim: the image in
model_image_idtoday (gallery version1.0.1787092687, from managed imageimg-fm7c799d-ccm-1.0.1787091895) DOES carrypi, taggedpi-tarball-sha256a69a1859...andnode-tarball-sha256d60acfe0..., matching the tracked closure forpi-coding-agent0.84.1 and Node v22.23.2. The lane is off because of the flag, not because of any image problem. This change asserts nothing to the contrary; it makes admission able to check that for itself instead of a document asserting it.What the guard does
require_model_image_attests_harness(azure, config["harness"])inbin/fm-crosscheck-azure.py, called from_run_azure_review_in_laneafter the lane is held and the foundation preflight passes, and before the capacity reservation, before any staged blob, and before the model VM. A refusal therefore costs nothing.pibindspi-tarball-sha256andnode-tarball-sha256;codexbindscodex-cli-sha256. Pi ships a#!/usr/bin/env nodeentrypoint and declaresengines.node >= 22.19.0, so an image carryingpiwithout the pinned Node fails the reviewer at launch for the same reason and at the same cost as an image carrying nopi.az rest --method get, the idiom the file already uses), and follows the version's source managed image exactly once when a required tag is absent there, because the build tags the managed image and gallery promotion is a separate operator step that need not carryartifactTags.claudelane, an empty or unexpected value) is refused rather than defaulting to admitted.The fail-closed choice
An unreadable image, an unreadable source managed image, an unreadable tag object, and an unreadable pinned closure are all refusals. This guard exists to stand between a wrong image and a paid VM, so it may never admit on ambiguity. The one case deliberately NOT treated as ambiguity is an ARM resource that reports no
tagsat all: that is not unreadable, it is an image that attests nothing, so it refuses as absence with the absence message.The refusal surfaces as
CrosscheckToolErrorrather than a hard error, so the reviewer roster rotates. That is correct here and not a softening: the same image can legitimately attest a different harness, which is exactly what rotation is for. With no reviewers left it raises as any other tool failure does.Scope
Preflight refusal only. When the check passes the lane does exactly what it did before: no changed parameters, no changed provisioning, no changed identity or ledger fields. The ordinary worker lane is untouched, and
bin/fm-worker-authority.py/bin/fm-worker-lifecycle.pyand their suites are untouched.Test evidence
tests/fm-crosscheck-azure.test.shgoes from 15 units to 16. Registrations in this suite are bare words that exit 0 on a typo, so every registered name was checked against every definition: 16 defined, 16 registered, no name registered-but-undefined, none defined-but-unregistered, no duplicates. All 16 execute and printok -.Run as CI runs it, never as a bare
bash:The new unit is hermetic: it substitutes
m.azwith an exact-resource-id ARM responder that records every URL, and reads the expected digests out of the tracked closure file rather than from a constant, so it cannot pass by comparing a literal to itself. It covers, in order:refusing before any model VMin the message; missingnode-tarball-sha256refuses on its own.absent.tagsshapes (a string, a list, an object with a non-string value) all refuse; a resource with notagskey refuses as absence.claudeand other unmapped harnesses refuse.reserve_model_capacity,upload_blob,provision_model_vm, andsubmit_model_run, and passesconfig["harness"]rather than a literal.nothing reads those tags.Mutation proofs
Five mutations, each applied to a clean tree, run through
tests/run-one.py, proven red, then reverted withgit checkout --and the tree confirmed clean.M1: delete the guard call from
_run_azure_review_in_lane.M2: make absence admit (
if value is None: continue).M3: make an unreadable read admit (an unreadable ARM GET returns the pinned closure digests as if the image had been read and attested).
M4: presence only, a mismatched digest admits (
if value != pinnedbecomesif False).M5: the call site checks a hardcoded harness (
require_model_image_attests_harness(azure, "pi")).After reverting all five, the suite is green again at 16/16.
A weaker earlier form of M3 (an unreadable read returning empty tags) also went red, but by degrading to the absence refusal rather than by admitting; it was replaced with the admitting form above so the proof shows a real admission being caught.
bin/fm-lint.shwas run to completion over the full repo.Honest limits
artifactTagsoutsidebin/fm-crosscheck-azure-image.sh.2023-07-03for gallery image versions,2024-03-01for managed images) were not exercised against live ARM in this session; the lane is off and no billable or live Azure call was made. If either were wrong,az restfails and the guard REFUSES, which is the safe direction, and the refusal prints the ARM diagnostic verbatim.claudelane is refused rather than checked. The image still carriesclaude-cli-sha256, but the adapter has no claude credential lane, so mapping it would be dead code that reads as support.