felhom-agent/REPORT.md

REPORT — F1 (HIGH) + F2: never destroy a guest this transaction didn't create (v0.60.0)

Date: 2026-07-02 · Repo: felhom-agent · Class: Risky/supervised (destructive rollback/destroy path) — strictly SAFER. Fixes the pool-effects campaign's F1/F2 (felhom.eu/documentation/audits/CAMPAIGN-pool-effects-2026-07-01.md); F3 is the companion doc in felhom.eu (documentation/runbooks/provisioning.md, commit 6dae9d7).

1. Baseline → target

agent v0.59.0 (55ade9e) → v0.60.0 (commit b9356d6 on main). felhom.eu doc commit 6dae9d7 (no bump).

2. Root cause + what changed

The compensating destroy ran on ASSUMED provenance (Rollback:true/SameTxnCreated), never verified against proof-of-launch. A RestoreLXC that failed synchronously (no UPID — e.g. PVE refusing a pre-existing vmid the pool-blind ListLXC couldn't see) still armed DestroyLXC(vmid) — destroying a guest the transaction never created. The pool ACL's 403 only saved the non-pool subset; an in-pool pre-existing guest would have been destroyed, in-process AND by Recover. Now a RestoreLXC UPID is the sole destroy authorization, in all three destroy paths (the pool ACL is defense-in-depth again):

• F1a internal/reconcile/bringup.go — runBringUp's rollback defer gated on launched (set only after the restore POST is accepted); a no-launch failure closes the owning entry terminal-failed with NO destroy. Pre-restore OpStarted append kept (crash-safety). rollbackBringUp doc updated: only ever called launch-proven.
• F1b internal/reconcile/restoretest.go — runScratchTest's teardownScratch defer gated the same way.
• F1c internal/reconcile/recover.go — the no-UPID "POST never confirmed → abandon" check now runs BEFORE the Scratch/Rollback dispatch: a no-UPID guest-creating entry is abandoned (marked failed, NO destroy) instead of destroy-by-vmid-existence. Recover safe by DESIGN, not by the pool-blind "already gone" accident.
• F2 restoretest.go — RunRestoreTest band-advance: an "already exists" refusal (new pveAlreadyExists, APIError-typed like pveConfigLock — a real restore failure is never misclassified) advances to the next free band vmid (pickScratchVMID + exclude set, bounded by the band width); band exhausted → Skipped (the scheduler raises no record/alert), never FAIL.
• Accepted residual (rule 4, by design): a crash in the one-statement window between obtaining the UPID and journaling it leaks a half-built guest Recover won't destroy — cleanable, preferable to destroying an innocent guest.

3. Green gate

go build ./... && go vet ./... && go test ./... — clean, 17 packages ok (build server go1.26.0 build of the deploy binary also clean).

4. Tests (incl. the red-proofs)

• F1a red-proof TestRunBringUp_NoLaunchNoDestroy (PVE "already exists" APIError + plain synchronous error): asserts DestroyLXC NOT called + entry terminal. Companion verified: with the launched gate reverted it fails destroys=[8000] — the exact destroy-innocent-guest bug.
• F1b TestRunRestoreTest_RestoreNoLaunchNoTeardown (replaces the old …RestoreFailureStillTearsDown, which asserted the BUGGY behavior): no teardown on a no-launch failure. Red-proof verified (destroys=[990000] pre-fix).
• No-regression (Scenario B): TestRunRestoreTest_LaunchedTaskFailureStillTearsDown + a new "restore-task failure after launch" case in TestRunBringUp_CompensatingRollback — a LAUNCHED restore whose task fails still destroys the half-built guest.
• F1c TestRecover_BringUpNoUPIDAbandoned + TestRecover_ScratchNoUPIDAbandoned: a no-UPID Rollback/Scratch entry with a guest PRESENT at the vmid → abandoned via the fail-safe path, no destroy. Red-proof verified. Existing leaked-guest Recover tests updated to carry the UPID (the launch proof) their scenario implies.
• F2 TestRunRestoreTest_AdvancesPastOccupiedBandVMID (squatter at 990000 → PASS at 990001, squatter never destroyed) + …BandFullOfSquattersSkips (Skipped, not FAIL, bounded — one try per band vmid) + TestPickScratchVMID exclude-set coverage.

5. Live proofs (felhom-pve, v0.60.0 deployed)

• Probe A (headline): --selftest=bring-up -vmid 9001 (existing non-pool guest) → PVE refused ("CT 9001 already exists"); no gate decision, no DestroyLXC attempt of any kind (pre-fix run had gate decision class=guest_destroy + a 403'd DELETE); 9001 untouched; journal drained in-process (next run: recover: examined=0).
• Probe D (F2): decoy parked at 990000 → restore-test logged "band VMID occupied … advancing", passed at 990001 (restored+booted+verified+torn-down in 28s), decoy survived, decoy removed.
• Regression: forced post-launch failure (-cores 10000 → config PUT 400 after restore) → compensating rollback destroyed the half-built 9310 ("bring-up: rolled back"); a normal provision of 9310 then passed (boot+running 22s) and tore down cleanly.
• STOP condition (any destroy attempted against 9001 or the decoy) did not trigger.

6. Deployed + published

• felhom-pve: /usr/local/bin/felhom-agent = 0.60.0 (backup felhom-agent.bak-0.59.0), unit active, clean logs. Demo left healthy: pool felhom = {9201}, guests 9001/9100/9201/9999 intact.
• Gitea generic package felhom-agent/0.60.0 published (HTTP 201), download sha256 round-trip verified: 45d19b8799cc764333cd95bb64e0515f2246cdb08c0fb36d9b23409051725831.
• Operator follow-up: bump the hub Day-0 artifact manifest (password-gated UI) to agent 0.60.0 — until then, fresh Day-0 installs still fetch the manifest-pinned older version.

7. F3 doc

felhom.eu/documentation/runbooks/provisioning.md (commit 6dae9d7): pick the provision -vmid from the host's FULL guest list (pct list + qm list), never the pool-only hub inventory; reserved bands documented (9999, 990000–990009 fleet-wide). Campaign doc updated with the F1/F2/F3 resolution banner.

8. Observations (not acted on)

• The hub artifact-manifest bump remains operator-gated (hub v0.29.0 dropdowns make it one click).
• Campaign F4 (restore-test can't read a NON-pool source guest's config for bind-mount neutralization) and F5 (pool-blind eject dependent-scan) remain open as LOW, per the campaign doc.
• The bring-up duplicate-guard (bringup.go ListLXC scan) is still pool-blind — acceptable now that a collision is non-destructive and PVE refuses authoritatively; noted in the F3 runbook.