diff --git a/bin/fm-azure-validation.py b/bin/fm-azure-validation.py index dc89b6bcc2e..2a565b79b13 100755 --- a/bin/fm-azure-validation.py +++ b/bin/fm-azure-validation.py @@ -2639,6 +2639,16 @@ def same_stable_identity(recorded, live, kind): ) +def disk_is_attached(resource): + properties = resource.get("properties", resource) + return bool( + resource.get("managedBy") + or properties.get("managedBy") + or resource.get("managedByExtended") + or properties.get("managedByExtended") + ) + + def expected_tags(state, selected): request = state["request"] return { @@ -3843,8 +3853,7 @@ def wait_exact_disk_detached(env, disk_id, recorded, label): live = immutable_identity(disk, "disk") if not same_stable_identity(recorded, live, "disk"): raise ValidationError("{} disk stable identity changed".format(label)) - managed_by = disk.get("managedBy") or disk.get("properties", {}).get("managedBy") - if not managed_by: + if not disk_is_attached(disk): return live time.sleep(5) raise ValidationError("{} disk did not detach after bounded reconciliation".format(label)) @@ -4538,15 +4547,12 @@ def build_purge_plan(env, state): raise ValidationError("retained worktree disk is absent before purge planning") verify_cleanup_resource(state, worktree, "disk", "worktree") worktree_identity = immutable_identity(worktree, "disk") - worktree_etag = worktree.get("etag") or worktree.get("properties", {}).get("etag") if ( not same_stable_identity(recorded_worktree, worktree_identity, "disk") - or not worktree_etag - or worktree.get("managedBy") - or worktree.get("properties", {}).get("managedBy") + or not worktree_identity.get("etag") + or disk_is_attached(worktree) ): raise ValidationError("retained worktree disk is not exact and detached") - worktree_identity["etag"] = worktree_etag identity_id = state["resources"].get("identity_id") exists, identity_resource = read_resource(env, identity_id, "identity") if not exists: @@ -4698,11 +4704,9 @@ def delete_planned_purge_resource(env, state, resource_id, kind, planned_identit live = immutable_identity(resource, kind) if not same_stable_identity(planned_identity, live, kind): raise ValidationError("planned {} stable identity changed".format(label)) - if planned_identity.get("etag"): - raw_etag = resource.get("etag") or resource.get("properties", {}).get("etag") - if raw_etag != planned_identity["etag"]: - raise ValidationError("planned {} ETag changed".format(label)) - if kind == "disk" and (resource.get("managedBy") or resource.get("properties", {}).get("managedBy")): + if planned_identity.get("etag") and live.get("etag") != planned_identity["etag"]: + raise ValidationError("planned {} mutation identity changed".format(label)) + if kind == "disk" and disk_is_attached(resource): raise ValidationError("planned {} disk reattached before deletion".format(label)) arguments = [ "rest", "--method", "delete", diff --git a/docs/azure-validation.md b/docs/azure-validation.md index 17a77dfdb2d..9ac41e30e57 100644 --- a/docs/azure-validation.md +++ b/docs/azure-validation.md @@ -98,8 +98,11 @@ Its digest binds all of these identities before queueing: - Trusted guest and shard-bridge digests. Azure tags repeat the non-secret home, task, generation, validation, cell, fence, branch, head, worktree, credential-lease, SKU-family, processor-reservation, and cost-attribution bindings. -The host records every resource id, ETag, VM instance id, NIC `resourceGuid`, disk `uniqueId`, identity client/principal id, and guest boot id before accepting a result or deleting anything. -ETags guard the current mutation, while stable VM/NIC/disk identities remain authoritative across legitimate attach, detach, and power-state ETag changes. +The host records every resource id, every ETag Azure returns, VM instance id, NIC `resourceGuid`, disk `uniqueId`, identity client/principal id, and guest boot id before accepting a result or deleting anything. +Where Azure returns an ETag, it guards the current mutation, while stable VM/NIC/disk identities remain authoritative across legitimate attach, detach, and power-state ETag changes. +Managed-disk GETs can omit an ETag from both the body and response headers, so the disk `uniqueId` is stored in the identity record's `etag` compatibility field and sent through the existing `If-Match` deletion path. +The pinned Azure Compute [`2023-10-02` Disk Delete contract](https://github.com/Azure/azure-rest-api-specs/blob/main/specification/compute/resource-manager/Microsoft.Compute/Compute/stable/2023-10-02/disk.json) declares no `If-Match` parameter, so this fallback is a stable identity pin and not a claim of provider-enforced atomic ETag compare-and-swap. +A successful delete with that extra header proves only that Azure accepted the request, not that it compared the value instead of ignoring it. The guest independently re-reads IMDS and requires the exact VM instance plus worktree and credential disk ids before unlocking either disk. The credential disk's recorded LUKS UUID is checked after unlock. The newly created worktree LUKS UUID is retained in the durable run identity, re-proved on every response or replacement, and repeated in every result. @@ -476,14 +479,15 @@ The pinned shared-provider inventory must show every exact censused reservation Because provider inventory is not fence-bound, any provider-active id without an exact controller reservation also refuses; an unrelated active reservation is ignored only when its controller record proves a different fence. An already-released exact constituent is accepted with its cleanup receipt, while any admitted constituent with no dispatch lineage and a queued or reserved status is listed separately for exact release. -It then proves the worktree disk is detached with its recorded stable identity and current ETag, proves the private container and its complete two-role inventory, and proves the cell identity has only its exact container and auth-share grants. +It then proves the worktree disk is detached with its recorded stable identity under the managed-disk limitation above, requiring both the single-owner `managedBy` field and the shared-disk `managedByExtended` attachment list to be empty. +It proves the private container and its complete two-role inventory, and proves the cell identity has only its exact container and auth-share grants. Those identities, every verified shard lineage, and only the remaining capacity constituents are sealed into an immutable purge plan. The state durably enters non-replaceable `purging` with the plan digest before the first deletion. Any remaining exact capacity constituent is released first and then read back as durably released and provider-inactive or absent before the retained disk, RBAC, container, or identity is touched. Every retry repeats the complete fresh allocator/provider census before attempting a remaining release, and the census is repeated after release before artifact deletion, so a new same-fence reservation can never fall outside the sealed plan. Retries never rebuild or widen that plan. -They accept only remaining subsets of its disk, role, container, identity, and capacity identities, use the stored ETags for conditional deletion, and persist progress after every boundary. +They accept only remaining subsets of its disk, role, container, identity, and capacity identities, use the stored mutation identities through the existing deletion-header path, and persist progress after every boundary. A partial or ambiguous attempt remains `purging`; `replace` and `retain-failure` cannot reclaim it. Completion retains the local state as a `purged` tombstone with the immutable plan and digest, and repeating the exact command is a no-op. diff --git a/tests/fm-azure-validation.test.sh b/tests/fm-azure-validation.test.sh index 3b6b399bb28..dbb5bd36ed7 100755 --- a/tests/fm-azure-validation.test.sh +++ b/tests/fm-azure-validation.test.sh @@ -1180,7 +1180,7 @@ state={ "ttl_schedule_id":"/control/ttl","safety_run_command_id":"/control/safety", "worktree_disk_id":worktree_id,"identity_id":identity_id,"identity_principal_id":principal, "identities":{ - "worktree":{"id":worktree_id.lower(),"etag":"work-etag","unique_id":"work-unique"}, + "worktree":{"id":worktree_id.lower(),"etag":"work-unique","unique_id":"work-unique"}, "identity":{"id":identity_id.lower(),"etag":None,"client_id":client,"principal_id":principal}, }, }, @@ -1222,7 +1222,7 @@ retry_runner["shared_capacity_reservation"]={"reservation_id":retry_one, "cleanup_receipt":"sha256:"+"8"*64} (runner_dir/(retry_one+".json")).write_text(json.dumps(retry_runner)) resources={ - worktree_id:{"id":worktree_id,"etag":"work-etag","properties":{"uniqueId":"work-unique"}, + worktree_id:{"id":worktree_id,"properties":{"uniqueId":"work-unique"}, "tags":{"validation-cell":cell,"fence":fence}}, identity_id:{"id":identity_id,"properties":{"clientId":client,"principalId":principal}, "tags":{"validation-cell":cell,"fence":fence}}, @@ -1258,7 +1258,7 @@ def az(_env,args,**kwargs): assert_sealed_before_mutation(); url=args[args.index("--url")+1] wanted=next(resource_id for resource_id in list(resources) if resource_id in url) if wanted==worktree_id: - assert "If-Match=work-etag" in args + assert "If-Match=work-unique" in args # Reproduce the former TOCTOU at the exact first irreversible artifact # mutation. The allocator must already have made this fence permanently # inadmissible, so the attempted reserve cannot enter the fresh census. @@ -1383,6 +1383,27 @@ else: raise AssertionError("purge admitted a passed result under a corrupt retai assert not mutations state=m.load_state(env,cell); state.pop("result"); m.save_state(env,state) +# The live managed-disk shape carries no raw body ETag. Its exact id, tags, +# stable uniqueId, and detached state remain mandatory before plan sealing. +resources[worktree_id]["properties"]["uniqueId"]="foreign-unique" +try: m.purge_retained(env,args()) +except m.ValidationError as exc: assert "immutable identity changed" in str(exc) +else: raise AssertionError("purge admitted a retained disk with the wrong uniqueId") +assert not mutations and m.load_state(env,cell)["phase"]=="failed-retained" +resources[worktree_id]["properties"]["uniqueId"]="work-unique" +resources[worktree_id]["managedBy"]="/foreign/vm" +try: m.purge_retained(env,args()) +except m.ValidationError as exc: assert "not exact and detached" in str(exc) +else: raise AssertionError("purge admitted a reattached retained disk") +assert not mutations and m.load_state(env,cell)["phase"]=="failed-retained" +resources[worktree_id].pop("managedBy") +resources[worktree_id]["managedByExtended"]=["/foreign/shared-vm"] +try: m.purge_retained(env,args()) +except m.ValidationError as exc: assert "not exact and detached" in str(exc) +else: raise AssertionError("purge admitted a shared disk with an extended attachment") +assert not mutations and m.load_state(env,cell)["phase"]=="failed-retained" +resources[worktree_id].pop("managedByExtended") + # Provider inventory is not fence-bound, so an active id without an exact # controller row cannot be classified as unrelated and must refuse. The # unrelated active id above remains admissible because its controller fence is @@ -1468,11 +1489,26 @@ else: raise AssertionError("injected destructive boundary did not fail") partial=m.load_state(env,cell); assert partial["phase"]=="purging" sealed=copy.deepcopy(partial["purge"]["plan"]); digest=partial["purge"]["plan_digest"] assert digest==m.sha256_bytes(m.canonical_bytes(sealed)) +assert sealed["worktree"]["identity"]=={ + "id":worktree_id.lower(),"etag":"work-unique","unique_id":"work-unique", +} assert partial["purge"]["progress"]=={ "worktree_absent":False,"container_roles_absent":False,"container_absent":False, "auth_share_roles_absent":False,"identity_absent":False, "capacity_fence_retired":False,"released_capacity":[], } +# A shared-disk attachment appearing after plan sealing must be caught by the +# fresh pre-delete read before the Azure mutation call. +resources[worktree_id]["managedByExtended"]=["/foreign/shared-vm"] +before_extended=copy.deepcopy(mutations) +try: + m.delete_planned_purge_resource( + env,partial,worktree_id,"disk",sealed["worktree"]["identity"],"worktree" + ) +except m.ValidationError as exc: assert "reattached before deletion" in str(exc) +else: raise AssertionError("pre-delete recheck admitted an extended disk attachment") +assert mutations==before_extended +resources[worktree_id].pop("managedByExtended") assert m.replacement_allowed(partial,"absent-proven")[0] is False assert set(resources)=={worktree_id,identity_id,container_scope} and len(roles)==3 assert lock_events[-4:]==[("enter",cell+"-shards"),("enter",cell),("exit",cell),("exit",cell+"-shards")]