diff --git a/scripts/CHANGELOG.md b/scripts/CHANGELOG.md index 1811ac2..5c112b5 100644 --- a/scripts/CHANGELOG.md +++ b/scripts/CHANGELOG.md @@ -1,3 +1,32 @@ +## 1.24.0 — a pre-existing backup target is granted too (2026-08-03, R-185) + +**`configure_backup_target` has two arms and only one of them granted.** The Case A arm creates the +storage and calls `felhom-backup-target-apply grant` in the same breath — a box that builds its own +target has always been fine. The **Scenario-F arm** — *"the target already exists, leave it exactly as +it is"* — returned without ever granting. + +So a box whose `felhom-backup` pre-dated the install (created by the vzdump-target-move runbook, or +surviving a reinstall) ended up with `local_backup_target: felhom-backup` while its token held +`FelhomAgentStore` on only `local`, `local-lvm` and `felhom-pbs`. Measured on **both** demo boxes +2026-08-03: the content API answers `{"data":[]}` through the agent's token while root lists three +archives. That tier was invisible to the agent and never restore-tested — and nothing said so, +because an empty listing is also what a brand-new tier returns. + +The reuse arm now ensures the ACL through the same guarded wrapper, so both arms leave the box in the +same state. **Scenario F is unviolated:** the storage DEFINITION is still untouched — granting the +role the agent is supposed to have on the target this script is about to write into `agent.json` is +finishing the job, not retargeting the box. `pveum acl modify` is idempotent, so a box that already +has the grant is unchanged and a box whose token was rotated gets it back. + +`$BACKUP_TARGET_ID` is deliberately **not** added to `PVE_STORAGES`, and the comment now says why: that +list is granted in step 4/5, before the target has been resolved in step 6, and `--acl-storages` +entries are preflight-checked for existence. The grant belongs with the resolution, which is where it +already was for a newly created target. + +**A gate now asserts it** (`hostinstall_gates.py`): every arm of `configure_backup_target` that +resolves the target must also grant on it. Red-proved by reverting the reuse arm — `resolves the +backup target in 2 place(s) but grants in only 1`. + ## v1.23.0 — the installer is published, not pushed (2026-08-03, R-110 + R-183) **Two channels moved off `main` in the same change, because either one left behind makes the other diff --git a/scripts/felhom-host-install.sh b/scripts/felhom-host-install.sh index df5193f..e6fabd7 100644 --- a/scripts/felhom-host-install.sh +++ b/scripts/felhom-host-install.sh @@ -184,7 +184,7 @@ set -euo pipefail -SCRIPT_VERSION="1.23.0" # the SINGLE version source (F-1): -h and the run banners follow it. +SCRIPT_VERSION="1.24.0" # the SINGLE version source (F-1): -h and the run banners follow it. # The hub used to carry a copy for its Setup tab; R-94 DELETED it # (2026-08-02) because the hub cannot know which version a box runs — # the Setup command fetches this script at run time. scripts/ @@ -307,6 +307,14 @@ PVE_PRIVS_STORE="Datastore.Allocate Datastore.AllocateSpace" PVE_PRIVS_BASE="Sys.Audit SDN.Use Datastore.Audit" # Storages the agent reads/writes (archive+dump=local, restore=local-lvm, offsite DR=felhom-pbs). The # offsite felhom-pbs MUST be included or the agent's DR backup 403s (SPIKE residual #1). --acl-storages overrides. +# +# `$BACKUP_TARGET_ID` is deliberately NOT in this list, and the reason matters because the obvious +# tidy-up is to add it (R-185, 2026-08-03). This list is granted in STEP 4/5, before +# configure_backup_target has run in STEP 6 — so at this point the target may not exist yet, and +# --acl-storages entries are preflight-checked for existence. The target's grant therefore belongs +# with the target's RESOLUTION, where it already is for a freshly created one, and now also for a +# pre-existing one. Adding it here would grant on a storage that may not exist and would still leave +# the resolution path as the single owner of that decision, split across two places. PVE_STORAGES=(local local-lvm felhom-pbs) # E-2: the whole-guest backup target storage id, and what configure_backup_target resolved to. # BACKUP_TARGET_RESOLVED feeds agent.json's local_backup_target; "local" means DEGRADED (Case B). @@ -652,8 +660,31 @@ configure_backup_target() { # existing id is an error, and repointing a live target is exactly the silent retarget this # whole arc closes. if pvesm status --storage "$BACKUP_TARGET_ID" >/dev/null 2>&1; then - log_skip " backup target '$BACKUP_TARGET_ID' already exists — leaving it exactly as it is (Scenario F)" + log_skip " backup target '$BACKUP_TARGET_ID' already exists — leaving its DEFINITION exactly as it is (Scenario F)" BACKUP_TARGET_RESOLVED="$BACKUP_TARGET_ID" + # R-185: …but STILL ensure the ACL. "The storage already exists" says nothing about whether + # the agent may READ it, and this early return is where the two came apart. + # + # THE DEFECT THIS CLOSES, measured on both demo boxes 2026-08-03. The CASE A path below + # creates the storage and grants in the same breath, so a box that built its own target is + # fine. A box whose target ALREADY existed — created by the vzdump-target-move runbook, or + # surviving a reinstall — returned here and never granted. The result: `local_backup_target` + # pointed at `felhom-backup` while the token held FelhomAgentStore only on local, local-lvm + # and felhom-pbs, so the API answered `{"data":[]}` for that storage while root saw three + # archives. The tier was invisible to the agent and never restore-tested, and nothing said so + # — because an empty listing is also what a brand-new tier returns. + # + # Scenario F is UNVIOLATED: the storage definition is still untouched. Granting a role the + # agent is supposed to have on the target this same script is about to write into + # agent.json is not "touching the box's target", it is finishing the job. `pveum acl modify` + # is idempotent, so a box that already has the grant is unchanged, and a box that had its + # token rotated gets it back. + if $DRY_RUN; then + log_dry "felhom-backup-target-apply grant $BACKUP_TARGET_ID # R-185: ACL on a pre-existing target" + else + /usr/local/sbin/felhom-backup-target-apply grant "$BACKUP_TARGET_ID" \ + || die "backup target grant failed on the pre-existing target — the agent could not read its own backup tier (R-185)" + fi return 0 fi local mp diff --git a/scripts/hostinstall_gates.py b/scripts/hostinstall_gates.py index 0f9e35d..96c5ed6 100644 --- a/scripts/hostinstall_gates.py +++ b/scripts/hostinstall_gates.py @@ -202,6 +202,37 @@ if wp is not None: fail("manifests/webpage.yaml no longer tracks main for the website — pinning the SITE to " "the installer tag turns every copy edit into a release") +# ── R-185: every path that RESOLVES the backup target must also grant on it ────────────────── +# +# THE DEFECT THIS WOULD HAVE CAUGHT, measured on both demo boxes 2026-08-03. `configure_backup_target` +# has two arms. The CASE A arm creates the storage and grants in the same breath. The Scenario-F arm — +# "the target already exists, leave it alone" — returned WITHOUT granting, so a box whose target +# pre-dated the install pointed `local_backup_target` at a storage its own token could not read. The +# API answered `{"data":[]}` while root saw three archives, and nothing said so, because an empty +# listing is also what a brand-new tier returns. +# +# The assertion is deliberately about the FUNCTION, not about PVE_STORAGES: the target's grant belongs +# with the target's resolution (PVE_STORAGES is granted a step earlier, before the target exists), so +# what must hold is that no arm of that function can resolve a target and skip the grant. +fn = re.search(r'^configure_backup_target\(\)\s*\{(.*?)^\}', src, re.S | re.M) +if not fn: + fail("cannot find configure_backup_target() — the backup-target ACL assertion cannot run, and a " + "check that cannot run must never report OK (R-185)") +else: + body = fn.group(1) + resolutions = len(re.findall(r'BACKUP_TARGET_RESOLVED="\$BACKUP_TARGET_ID"', body)) + grants = len(re.findall(r'felhom-backup-target-apply grant', body)) + if resolutions == 0: + fail("configure_backup_target no longer resolves BACKUP_TARGET_ID anywhere — re-read it") + elif grants >= resolutions: + ok("every arm that resolves the backup target also grants on it (%d resolution(s), %d grant(s))" + % (resolutions, grants)) + else: + fail("configure_backup_target resolves the backup target in %d place(s) but grants in only %d " + "— an arm resolves a target the agent may not READ. That is R-185: the tier's archives are " + "invisible to the agent, it is never restore-tested, and an empty listing looks exactly " + "like a brand-new tier." % (resolutions, grants)) + print() if fails: print("hostinstall gates: %d FAILURE(S)" % len(fails))