From 03b58cec0aea9ee2f1bd1a280161b3f4f014f10b Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Mon, 3 Aug 2026 19:04:15 +0200 Subject: [PATCH] REPORT + CONTEXT + REUSE: R-185 closed, with the corrected root cause The installer defect was NOT PVE_STORAGES as the row and the task assumed: the create arm of configure_backup_target grants, the Scenario-F reuse arm did not. Also records the measured trap (an ungranted path answers with INHERITED privileges, not empty and not 403), the deviation from the spec's suggested Prober generalisation in favour of the existing poolReadStatus precedent, the hollow test caught before it shipped, and that demo-hp carried the same drift and was fixed. --- CONTEXT.md | 28 ++++ REPORT.md | 382 +++++++++++++++++++++-------------------------------- REUSE.md | 1 + 3 files changed, 182 insertions(+), 229 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index 9790dc7..e0878f7 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -5,6 +5,34 @@ ## Current +- **2026-08-03 — v0.123.0 (R-185): a tier the box cannot READ now says so.** The agent's token had + `FelhomAgentStore` on `local`, `local-lvm`, `felhom-pbs` and **not** on `felhom-backup` — the + storage both demo boxes configure as `local_backup_target`. That storage answered `{"data":[]}` + through the token while root listed three archives, and `pickForThisRun` skipped it as *"no settled + archive yet"* — **which is what a brand-new tier reports**, so the host tier was never + restore-testable and nothing said so. + - **The permission question is asked directly**, because unlike the listing it has a definite + answer: `Client.Permissions` reads `/access/permissions?path=/storage/` **as the agent's + own token**, and `storeGrantStatuses` emits one `capability.Status` per configured tier. It + composes AROUND the sudo prober, the way `poolReadStatus` already does — an API read does not + belong inside a sudo-policy probe. `Status`'s wire shape is untouched, so the hub's critical + degraded alert applies with **no hub change**. + - **MEASURED FIRST, and the obvious reading is wrong:** an ungranted path answers neither empty nor + 403 — it carries the privileges INHERITED from the box-wide `/` grant + (`Sys.Audit, SDN.Use, Datastore.Audit`). Checking path-presence, or `Datastore.Audit`, reports a + blinded storage HEALTHY. The probe tests **`Datastore.AllocateSpace`**; re-measure before ever + changing that constant (`storeGrantRequiredPriv`, red-proved). + - **The probed set comes from `BackupTiers()`, never a fixed list** — a hardcoded probe list is the + defect reproduced inside the fix. Critical, EXCEPT the `local` fallback target (reported, but it + does not page). It never consults content, so it cannot alarm on a newborn tier; it never reports + ok when it could not ask. + - **LIVE:** degraded observed on the still-blind box (hub emailed `agent_capability_degraded`) → + grant applied on **both** demo boxes → token lists 3 and 4 archives → `ok=70 total=70 degraded=0` + and `degraded → ok` at the hub → **the host tier became a due-check candidate for the first time**, + correctly picking the 08-02 archive (08-03 had not settled 24 h). + - **The installer's real defect was NOT `PVE_STORAGES`** — see `felhom.eu` CONTEXT S-22: Case A + grants, the Scenario-F reuse arm did not. Fixed in installer **1.24.0** with a gate. + - **2026-08-03 — v0.122.0 (R-189 · R-188 · R-186): three signals that lied about their own work.** None touches data; all three cost attention, which every other signal depends on. - **R-189 — a passing restore-test no longer vanishes on a restart.** `restore_tests[]` came only diff --git a/REPORT.md b/REPORT.md index 304c4b2..5d8503d 100644 --- a/REPORT.md +++ b/REPORT.md @@ -1,9 +1,9 @@ -# REPORT — R-189 · R-188 · R-186: three ways the signals lied about themselves +# REPORT — R-185: a tier the box cannot READ must say so -**Date:** 2026-08-03 · **Repo:** `felhom-agent` **v0.121.1 → v0.122.0** (`7581f81`) · released, -published, verified by an independent download **and by rebuilding it**, deployed to demo-felhom. -`felhom.eu`: register + docs only, **no hub change and no hub bump** — the hub already reads -`restore_tests[]`; the defect was that the agent stopped sending them. +**Date:** 2026-08-03 · **Repos:** `felhom-agent` **v0.122.0 → v0.123.0** (`fe14bc6`) · `felhom.eu` +installer **1.23.0 → 1.24.0** (`688470c`, tag `installer-v1.24.0`, manifest `4dbe0d9`-era bump) · +**no hub change and no hub bump** — the hub already alerts on a degraded critical capability, which is +why that mechanism was chosen. --- @@ -11,271 +11,195 @@ published, verified by an independent download **and by rebuilding it**, deploye | Repo | `main` @ commit | Version | Matched §1? | |---|---|---|---| -| `felhom-agent` | `3d0a1d615d11` | `v0.121.1` | **yes** | -| `felhom.eu` | `c9a3e48b2106` | hub `v0.91.1` | **yes** | +| `felhom-agent` | `0b28eae7bb14` | `v0.122.0` | **yes** | +| `felhom.eu` | `7a5694341d59` | installer `1.23.0`, both `--ref=installer-v1.23.0` (lines 327, 372) | **yes** | -Highest register ID in use was **R-189**; no new IDs were needed — all three rows already existed. -(Grep confirmed R-190+ free, in case one had been.) +Highest register ID in use **R-189**; R-190+ confirmed free by grep, and none was needed. -## 2. Scenario H — the reproducibility measurement (Part 3, done first on purpose) +## 2. Part 0 — the measurements, before anything was designed against them -**Before**, one commit, same source, same toolchain, same ldflags — the only difference is whether the -tag existed when the build ran: +**The row's three-way observation, reproduced unchanged:** -| build | sha256 | size | embedded module version | -|---|---|---|---| -| default flags, **no tag yet** | `18f4a495…` | 14 085 464 B | `v0.121.2-0.20260803133646-3d0a1d61` | -| default flags, **tagged** | `4a38f394…` | 14 085 440 B | `v0.121.99` | -| `-trimpath -buildvcs=false`, either way | `7ffcdf1d…` | 14 064 574 B | *(none)* | +| leg | result | +|---|---| +| root, `pvesh … /storage/felhom-backup/content` | **3 archives** — 6.1 / 6.2 / 6.3 GB, dated 08-01, 08-02, 08-03 | +| the **agent's token**, same endpoint | `{"data":[]}` | +| the agent's token, `/storage/local/content` | **8 entries** — the token works where it is granted | -**After, on the real release (v0.122.0) — the three values §15.2 asks for:** +So the token is the variable, not the storage. Two further checks removed the obvious alternative +explanation: guest **9201 IS in the `felhom` pool** (so `VM.Backup` is not the discriminator), and +`pveum acl list` showed ACL rows for `/storage/{local,local-lvm,felhom-pbs}` and **none** for +`/storage/felhom-backup`. -| artifact | sha256 | size | -|---|---|---| -| published, downloaded from Gitea | `d5f294e56c1ef59055e8e87fb9135aa477632dbbc56a5d4bff46bbd0466c1edf` | 14 076 649 B | -| rebuild at the tag, #1 | `d5f294e56c1ef59055e8e87fb9135aa477632dbbc56a5d4bff46bbd0466c1edf` | 14 076 649 B | -| rebuild at the tag, #2 | `d5f294e56c1ef59055e8e87fb9135aa477632dbbc56a5d4bff46bbd0466c1edf` | 14 076 649 B | - -**All three identical.** The property is removed-cause, not sequenced-around: `-buildvcs=false` drops -a stamp nothing reads (no `ReadBuildInfo` caller, verified by grep), and the version still comes from -the explicit `-X main.version` ldflag. `-trimpath` additionally makes a rebuild from a different -checkout directory match. - -**A second discrepancy fell out of the measurement and is fixed with it:** `publish-agent.sh`'s -fallback build forced `CGO_ENABLED=0` and produced **13 990 236 B** against the release path's -**14 064 574 B** — a 74 KB difference, i.e. one version name meaning two binaries depending on which -entry point ran. Both paths now use identical flags, each commented with a pointer to the other. - -## 3. Scenario E — a correct release no longer emails a failure (Part 2) - -**Only the tag PUSH moved.** The order is now build → tag **locally** → publish → push tag. The tag is -still created before anything is published, so the build and the tag describe the same commit; it -becomes *visible* — to CI (`on: [push]`) and to any `raw/tag/…` fetch — only once the package is -downloadable. - -**The old ordering's invariant is asserted directly rather than arranged for.** -`check-published-versions.py` now carries two invariants: every tag has an installable package (as -before) **and no published version is missing its tag** (new). The second is a **bounded probe** — -the frontier past the newest tag, where a failed tag push leaves an orphan, plus patch gaps — and it -**prints its probe set on every run**, because a check whose coverage is invisible reads as a -guarantee it is not making. The package listing api was **re-measured**, not assumed: `401` without a -token, so absence still cannot be enumerated, and the script says so in its own output. - -**Live result — this release is the test:** - -| release | CI runs on the release commit | outcome | -|---|---|---| -| v0.121.0 (yesterday) | #12 / #13 | one **green**, one **red** | -| v0.121.1 (yesterday) | #17 / #18 | one **red**, one **green** | -| **v0.122.0 (this one)** | **#21 (task id 96) / #22 (task id 97)** | **both green** | - -**Failure modes made loud rather than tidy:** a publish that succeeds followed by a tag push that -fails now dies printing `git push origin v` — the local tag is already there, so recovery is one -line — and a publish that *fails* deletes the local-only tag so a retry is clean instead of colliding -with step 2's re-release guard. - -## 4. Scenarios F and G — both directions demonstrated, then cleaned up - -**G — a published version with no tag must FAIL.** A real fixture: version **0.121.2** (the frontier, -exactly where a failed tag push lands) published to the live registry with no tag. +**The permission query, asked by the token itself — and the obvious reading is wrong:** ``` -0.121.2 next patch after the newest tag PUBLISHED — NO TAG -check-published-versions: 1 PUBLISHED VERSION(S) WITH NO TAG - v0.121.2 is downloadable at … but has no git tag. - git push origin v0.121.2 -EXIT=1 +/storage/felhom-pbs → {"Datastore.Allocate":1,"Datastore.AllocateSpace":1} +/storage/felhom-backup → {"Sys.Audit":1,"SDN.Use":1,"Datastore.Audit":1} ``` -**Red-proof (observed):** with the fixture still live, replacing the probe set with `[]` produced -`ALL RELEASED VERSIONS INSTALLABLE, AND NONE UNTAGGED`, **exit 0** — a green run over a published -orphan. Restored. +The ungranted path answers **neither empty nor 403**. It answers with the privileges **inherited** +from the box-wide `/` grant. A probe asking *"did the path come back?"* — or *"does it hold +`Datastore.Audit`?"* — would have reported the blinded storage **healthy**. This is exactly what §3 +required to be measured rather than assumed, and it changed the design: the probe tests +`Datastore.AllocateSpace` specifically, and a red-proof pins that choice. -**Teardown:** fixture deleted (HTTP 204), absence independently re-verified (`GET → 404`), gate green -again (exit 0). No scratch tag was ever pushed. +## 3. The probe -**F — a tag with no package must still FAIL.** Demonstrated against a local stand-in serving two tags -where only one has a package: +`Client.Permissions` reads `/access/permissions?path=/storage/` **as the agent's own token** +(asking as root answers a different question and always says yes). `storeGrantStatuses` emits one +`capability.Status` per configured tier. -``` - ok v0.120.0: binary downloadable + tag serves its configs - FAIL v0.199.0: - - binary NOT downloadable (HTTP 404 …) - - tag does not serve configs/felhom-agent.service (HTTP 404) — a box would 404 mid-install -EXIT=1 -``` +**Deviation from §5/§8.1, stated because a recommendation not followed gets a line:** the spec asked +for the sudo `Prober` to be minimally generalised. This repo already has the better-established +pattern for exactly this — `poolReadStatus`, composed **around** the prober, with the comment *"an API +read does not belong inside the sudo-policy probe"* (v0.62.0, audit A1). The probe follows that +precedent instead. `capability.Status` is untouched either way, which is the constraint that mattered. -**Why not a real pushed tag:** pushing one wakes CI and would have emailed the operator a **true** -alarm about a fixture — the same attention cost R-188 exists to remove. F is also already -demonstrated in the wild: CI runs **#13** and **#17** failed for exactly this reason yesterday. +**Decisions:** -## 5. R-189 — a proof that survives a restart reaches the hub (Part 1) +- **The probed set comes from the box's own `BackupTiers()`**, never a fixed list — a hardcoded probe + list is the defect reproduced inside the fix. +- **Critical** (§8.3): the hub alerts only on critical, so a non-critical entry would ride the report + and alert nobody — the same silence with extra steps. **Except** the `local` fallback target, which + host-install's own comment calls the DEGRADED configuration: still probed, still reported, but it + does not page. Turning an ordinary documented setup into an alert is how a signal becomes something + an operator archives unread. +- **It never consults content**, so it cannot alarm on a newborn tier by construction — a stronger + guarantee than gating on emptiness would be. +- **It never reports ok when it could not ask.** Unreachable PVE is degraded: a self-check that fails + open converts *"I do not know"* into *"fine"*. -`restore_tests[]` came only from the in-memory `backup.Store`, whose comment read *"lost on restart; -the cadence re-populates"*. True under a timer; false since R-86, because the agent refuses to re-test -an archive it has already proven — so a lost proof is not repeated for a whole archive generation. +## 4. The installer — the root cause was not where the row or the task expected -**What changed** +Both assumed `PVE_STORAGES` (the fixed grant list) was the culprit. **It is not.** +`configure_backup_target` has two arms: -- `RestoreTestState` stores the **tier** and what was **verified** beside the archive (v3 shape). - Both are recorded **at proof time from the run's own result** — deriving them later would need a - storage-type lookup at report-building time, a network call that can fail on the one path where - failing means mislabelling a proof. -- `ProvenRestoreTests` renders the stored proofs as report entries; `Collector.SetProvenRestoreTests` - merges them with the in-memory result. -- **Merge rule: one entry per tier, newest by `TestedAt` wins.** It falls out of what each source - means rather than from a preference: a fresh failure beats a stored success (the failure is the - news and lives nowhere else), a stored success beats a stale in-memory entry after a restart, and a - tier never appears twice — the hub would read that as two tests. An unparseable timestamp counts as - **older**, so a malformed entry cannot displace a good one. -- **It refuses to lie.** A record missing the archive **or** the tier produces no entry, and run - mechanics (scratch VMID, duration) are not re-invented: an absent duration is not a claim, a - fabricated one would be. -- **The asymmetry is now in the code** (§8.1): a success *suppresses* future work so it must be - durable; a failure *causes* future work and heals itself, and persisting one would make a healed - tier keep reporting a fault. -- **`Store`'s comment is corrected in place** — leaving it is how the next reader concludes this is - handled. +- **Case A** creates the storage and calls `felhom-backup-target-apply grant` in the same breath — a + box that builds its own target has always been correct. +- **The Scenario-F arm** — *"the target already exists, leave it exactly as it is"* — **returned + without granting**. -**Migration, and it is visible on the live box:** a pre-R-189 record has an archive but no tier, so it -is **not** reportable. Upgrading does not retroactively make an old proof visible; the tier's next -real proof fills it in. Confirmed immediately after the deploy — still `0 restore-tests`, with the -v2 record sitting on disk. +So a box whose `felhom-backup` pre-dated the install (created by the vzdump-target-move runbook, or +surviving a reinstall — which is both demo boxes) pointed `local_backup_target` at a storage its own +token could not read. The reuse arm now ensures the ACL through the same guarded wrapper. -## 6. Scenario A, live on demo-felhom — against the observation that filed R-189 +**Scenario F is unviolated:** the storage DEFINITION is still untouched. Granting the role the agent is +supposed to have on the target this same 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 it is +unchanged and a box whose token was rotated gets it back. -**The observation being replaced (2026-08-03, 15:25):** a real offsite restore-test PASSED, the agent -was restarted 2 m 43 s later, and the hub logged `0 restore-tests` on the next two host-reports. +**`$BACKUP_TARGET_ID` is deliberately still NOT in `PVE_STORAGES`,** and the comment now says why: that +list is granted in step 4/5, *before* `configure_backup_target` runs in step 6, and `--acl-storages` +entries are preflight-checked for existence. Adding it there would grant on a storage that may not yet +exist and would split ownership of the decision across two places. -**The same sequence, on v0.122.0:** +**A gate now asserts it:** every arm of `configure_backup_target` that resolves the target must also +grant on it — the check that would have caught this. -``` -16:44:06 restore-test tier is DUE target=felhom-pbs - archive=felhom-pbs:backup/ct/9201/2026-07-27T19:55:41Z - reason="newest settled archive … has not been proven (last proven archive was a different one)" -16:55:21 restore-test: scratch guest torn down vmid=990000 -16:55:21 backup: scheduled restore-test PASSED archive=felhom-pbs:…2026-07-27T19:55:41Z duration_s=675.1 -16:55:32 systemctl restart felhom-agent ← INSIDE the 15-minute reporting window -16:55:36 hub: host-report from demo-felhom-8363b5 (… 1 restore-tests …) ← was 0 -``` +## 5. Live validation, in order -**The proof on disk (v3 — the tier is what the old shape lacked):** +| # | evidence | +|---|---| +| 1 | Part 0's measurements above, taken **before** any change | +| 2 | **The signal that has never existed**, on the still-blind box: `capability DEGRADED … capability=pve:store-grant:felhom-backup … reason="the agent token lacks Datastore.AllocateSpace on /storage/felhom-backup (grant FelhomAgentStore there) — this tier's archives are INVISIBLE to the agent and it is never restore-tested" critical=true`, with `ok=69 total=70 degraded=1`. The hub: `Host capability: demo-felhom-8363b5 ok → degraded (agent_capability_degraded)` and **`Operator email sent`** | +| 3 | Grant applied (user **and** token — a privsep token's rights are the intersection); the token then lists **3 archives** where it listed none, and the permission answer becomes `{"Datastore.AllocateSpace":1,"Datastore.Allocate":1}` | +| 4 | `capabilities self-check ok=70 total=70 degraded=0`; the hub: `degraded → ok (agent_capability_recovered)` | +| 5 | **The host tier is a due-check candidate for the first time on that box**: `tier=felhom-backup due=true archive="…2026_08_02-04_42_14.tar.zst" proven=""` — and the settle rule applies to it exactly as to the others, selecting the **08-02** archive because the 08-03 one has not settled 24 h | +| 6 | The served installer over HTTPS: `SCRIPT_VERSION="1.24.0"`, and the served bytes carry the fix itself, not merely the version | -```json -{"felhom-pbs": {"archive": "felhom-pbs:backup/ct/9201/2026-07-27T19:55:41Z", - "tier": "pbs", "verified": "boot+running", "proven_at": "2026-08-03T14:55:21Z"}} -``` - -**What the HUB stored** — read from its own database (copied with its `-wal`, freshness confirmed by -the newest row's `received_at` = `2026-08-03 14:55:36` UTC, matching the ingest line): - -```json -{ "source_archive": "felhom-pbs:backup/ct/9201/2026-07-27T19:55:41Z", - "source_tier": "pbs", "pass": true, "verified": "boot+running", - "tested_at": "2026-08-03T14:55:21Z", "scratch_vmid": 0, "duration_seconds": 0 } -``` - -That report was built **after** the restart, when the in-memory store was empty — so the entry can -only have come from the persisted state. The archive, the tier, and the **original** test time -survived; the run mechanics are zero because they are deliberately not re-invented. - -**A 14.5 GB encrypted offsite archive**, restored, booted, verified and destroyed in **675 s** — and -this time the proof outlived the process that produced it. - -**Teardown, all three layers:** scratch guest absent from `pct list` (0), its volumes gone from `lvs` -(0), the validation drop-in removed and the daemon back on its defaults -(`eval_interval=6h0m0s settle=24h0m0s`). The hub-side `restore_tests[]` record is **retained -deliberately** — it is the proof the staleness check reads, so deleting it would delete the result. -No `restore_test_*` event was raised, because nothing failed and nothing is stale. +## 6. The other machines +- **demo-hp CARRIES THE SAME DRIFT — and was fixed.** `local_backup_target=felhom-backup`, ACL rows for + `local`, `local-lvm`, `felhom-pbs` only. §8.6 assumed a single affected box; the same one-line, + additive, path-scoped, idempotent grant applies to the other, and leaving a known-blind backup tier + on a Tier-0 box after finding it would be this row happening twice. Granted (user + token); its + token now lists **4 archives**. It still runs agent `0.120.0`, so it has no probe yet — that arrives + when you vouch. +- **The tester's box was NOT touched** (Tier 2, protected). **What is known without connecting to it:** + it very likely carries the same drift — the mechanism is the Scenario-F reuse arm, which fires on + any box whose target pre-dated its install, and its target was moved by the very runbook that + creates that condition. It is due for reinstall, and installer 1.24.0 fixes it on the way in. ## 7. Tests and red-proofs -Green gate: `go build ./... && go vet ./... && go test ./...` — **29 packages ok, rc=0**, plus -`python3 scripts/agent_gates.py` (reuse-refs + published-versions) all OK. The test run and the commit -were always separate commands. +Green gate: `go build ./... && go vet ./... && go test ./...` — rc=0, plus `agent_gates.py` and +`repo_gates.py` all OK. Test runs and commits were always separate commands. | # | Test | Asserts | Mutation | Observed | |---|---|---|---|---| -| A | `TestMerge_ProofSurvivesARestart` | an empty in-memory store + a persisted proof → the proof is reported, with its archive and its original time | the persisted merge deleted (the pre-R-189 body) | **FAIL** — `after a restart the persisted proof must be reported; got 0 entr(ies): []` — the live observation exactly | -| B | `TestMerge_NeverInventsAPassForAnUnprovenTier` | no proof → no entry; a tier-less record → no entry | — (its state-layer twin below carries the mutation) | pass | -| B′ | `TestProvenRestoreTests_RefusesToReportWhatItCannotDescribe` | v1 + v2 + v3 records side by side → only the describable one is reported | the `reportable()` filter dropped | **FAIL** — `got 3` entries, two with an empty `SourceTier`/`SourceArchive` | -| C | `TestMerge_NewerWinsAndNeverDuplicatesATier` | one entry per tier, newest wins, in both directions | de-duplication removed | **FAIL** — `one entry per tier; got 2 for "pbs" — the hub would read two tests` | -| D | `TestMerge_AFailureIsStillReported` | a fresh failure beats an older stored success | (same mutation) | **FAIL** — 2 entries, i.e. the failure no longer the single answer for that tier | -| — | `TestMerge_MalformedTimestampNeverWins` | unparseable ≠ newest | — | pass | -| — | `TestMerge_NilProvenSourceIsANoOp` | pre-R-189 behaviour unchanged when unwired | — | pass | -| — | `TestScheduler_ProofIsRecordedReportably` | a pass **through the scheduler** leaves a reportable proof | — | pass | -| — | `TestScheduler_AFailureLeavesNoPersistedProof` | §8.1's asymmetry, asserted not assumed | — | pass | -| G | the gate's converse assertion | a published version with no tag fails | probe set → `[]` | **FAIL** (green over a live orphan) | -| I | `TestMainWiresTheDurableRestoreTestProof` | **AST**: `SetProvenRestoreTests` is called **and fed `rtState`** | the call commented out | **FAIL** — `main.go never calls collector.SetProvenRestoreTests` (a `strings.Contains` check would have passed — the string is still there) | -| H | reproducibility | three identical sha256 | — (measurement, §2) | pass | +| A | `TestStoreGrant_ForbiddenStorageIsDegradedAndNamed` | degraded, critical, naming storage **and** role | probe removed from `probeAll` | **FAIL** — `main.go never calls storeGrantStatuses` (via the seam test); with the wrong-privilege mutation: `must be DEGRADED, not "ok"` | +| A′ | `TestStoreGrant_InheritedPrivilegesAreNotAGrant` | the measured trap: inherited ≠ granted | probe `Datastore.Audit` instead | **FAIL** — `checking for the wrong privilege reports a blinded storage healthy; got "ok"` | +| B | `TestStoreGrant_GrantedButEmptyIsHealthy` | a readable-but-empty tier is healthy | — (it never reads content, so emptiness cannot reach it) | pass | +| B′ | `TestStoreGrant_TheFallbackTargetIsNotCritical` | `local` is reported but does not page | gating removed (`return true`) | **FAIL** — `must not page the operator about an ordinary, documented configuration` | +| C | `TestStoreGrant_ForbiddenAndNewbornAreDistinguishable` | different status **and** different capability id | — | pass | +| — | `TestStoreGrant_UnreachablePVEIsDegradedNotOK` | unknown ≠ ok | — | pass | +| F | `hostinstall_gates.py` backup-target assertion | every resolving arm also grants | reuse arm reverted | **FAIL** — `resolves the backup target in 2 place(s) but grants in only 1` | +| H | `TestMainWiresTheStoreGrantProbe` | **AST** of `main.go` | call commented out | **FAIL** — a `strings.Contains` check would have passed | -**Jitter, per §10:** every timestamp fixture uses odd minutes and seconds (`13:25:14`, `19:55:41`, -`13:41:07`, `04:41:58`) — several taken from the real box — rather than round hours. Yesterday a test -was hollow because a perfectly regular series landed exactly on a threshold and survived its own -mutation. +**A hollow test caught and fixed before it shipped:** the first draft of `storegrant_test.go` +re-implemented the verdict branch inside the test. It passed, and would have kept passing while +production diverged. The decision was extracted into `storeGrantVerdict` and the tests now call it. -## 8. Files changed +**Scenario B's red-proof, honestly:** the spec asked for "degrade on an empty content listing" as the +mutation. That is not a mutation of this code — the probe never looks at content, which is a stronger +guarantee than gating on emptiness. The gating red-proof above (`storeGrantCritical`) is the one that +exercises the guard that does exist, and it fails as required. -`internal/backup/restoretest_state.go` (v3 record + `ProvenRestoreTests`), `internal/backup/store.go` -(the comment that had become false), `internal/backup/schedule.go` (record tier + verified), -`internal/hub/collect.go` (the seam + the merge), `cmd/felhom-agent/main.go` (wiring), -`scripts/release-agent.sh` (ordering, reproducible build, loud half-done release), -`scripts/publish-agent.sh` (identical build flags), `scripts/check-published-versions.py` (the -converse invariant), plus `REUSE.md`, `CHANGELOG.md`, `CONTEXT.md`, `CLAUDE.md` and three test files. +## 8. Files, commits, tag -**Commits** — `felhom-agent`: `7581f81` (v0.122.0). `felhom.eu`: see §10. +`internal/proxmox/query.go` (`Permissions`), `cmd/felhom-agent/main.go` (`storeGrantStatuses`, +`storeGrantVerdict`, `storeGrantCritical`, `storeGrantRequiredPriv`, wiring), +`cmd/felhom-agent/storegrant_test.go`, `CHANGELOG.md`, `CONTEXT.md`, `REUSE.md`, `REPORT.md`. +`felhom.eu`: `scripts/felhom-host-install.sh`, `scripts/hostinstall_gates.py`, `scripts/CHANGELOG.md`, +`manifests/webpage.yaml`, `CONTEXT.md`, `STATUS.md`, `documentation/architecture/00-capability-map.md`, +`documentation/backlog/OPEN-ITEMS.md`, `documentation/runbooks/RUNBOOK-vzdump-target-move-2026-07-29.md`. -## 9. The independent-verification command (Part 3, recorded in `CLAUDE.md`) +**Commits** — `felhom-agent`: `fe14bc6` (v0.123.0). `felhom.eu`: `688470c` (installer 1.24.0), the +manifest bump, `e3187c8` (docs). **Installer tag:** `installer-v1.24.0`. -```bash -V=0.122.0 -git checkout "v$V" && go build -trimpath -buildvcs=false -ldflags "-X main.version=$V" \ - -o /tmp/felhom-agent-check ./cmd/felhom-agent -sha256sum /tmp/felhom-agent-check -curl -fsSL "https://gitea.dooplex.hu/api/packages/admin/generic/felhom-agent/$V/felhom-agent" | sha256sum -``` +## 9. Deployment -Both print `d5f294e56c1ef59055e8e87fb9135aa477632dbbc56a5d4bff46bbd0466c1edf` (§2). +Agent released through `release-agent.sh` — tag `v0.123.0`, sha256 +`74910135ac4feb1b7f0ad4dbd1541d965cbc0fe70d4f47b62ebf7e4bfb962453`, round-trip verified. The +**published bytes** were downloaded and deployed: the running binary's sha matches the published one. +`felhom-agent --version` → **0.123.0**, `systemctl is-active` → active, prior kept as `.bak-0.122.0`. +**NOT VOUCHED** — that stays the operator's act. ## 10. Registers -- **R-189 → CLOSED** (shipped + proven live), **R-188 → CLOSED** (shipped), **R-186 → CLOSED** - (shipped + measured). -- **R-185 remains OPEN and untouched** — it is a missing `/storage/felhom-backup` ACL on demo-felhom, - a permission defect, not a reporting one. Nothing in this session changed it, and the priority list - says so explicitly. -- No new IDs minted. `ROADMAP.md` contains none of these three rows, so there was nothing to collapse. -- `00-capability-map.md`'s restore-proof row now records that the evidence path itself had a gap and - what closed it; `CONTEXT.md` gains **S-19** (the proof/failure asymmetry and the merge rule) and - **S-20** (the release ordering and what each step protects); `STATUS.md` rewritten for the operator - and trimmed to 83 lines. +- **R-185 → CLOSED** (shipped + proven live on both demo boxes), with the corrected root cause + recorded on the row. +- No new IDs minted; `ROADMAP.md` contains no R-185 row, so there was nothing to collapse. +- **The capability map's whole-guest row was OPTIMISTIC and now says so:** every live restore-test it + cited is on the OFFSITE tier, and the HOST tier was not merely unproven but *unprovable* on both + demo boxes. It now records that, the closure, and that it will carry a host-tier live proof when one + runs. +- The vzdump-target-move runbook's item 5 **predicted this** and is annotated, not rewritten: it + expected a 403 on backup, and the reason it did not surface that way is that `vzdump` writes through + a root path, so backups kept landing while the agent's *read* stayed blind. +- `CONTEXT.md`: agent-side entry, plus `felhom.eu` **S-21** (empty ≠ forbidden; the measured trap) and + **S-22** (the Scenario-F arm must finish the job). -## 11. Observations — noticed, recorded, NOT acted on +## 11. Teardown -- **The proof state holds ONE record per tier, so proving an OLDER archive re-arms a newer one — - CONFIRMED after the validation, not merely predicted.** With the defaults restored, the due-check - reads: `tier=felhom-pbs due=true archive="…2026-07-28T04:49:43Z" proven="…2026-07-27T19:55:41Z" — - newest settled archive has not been proven (last proven archive was a different one)`. - Surfaced by this session's own validation method: to get a fresh proof without waiting a week, the - settle lag was widened so the older, unproven offsite archive became the candidate. That overwrote - the record for the newer archive, so once the default 24 h settle returns, the newest settled - archive is no longer the recorded proof and the tier becomes due once more. **Consequence, stated - rather than left to surprise: demo-felhom will run one further unattended offsite restore-test - within 6 h, after which the newest archive is the recorded proof** — the correct steady state. In - normal operation this cannot arise, because the candidate only ever moves forward. -- **`RestoreTestState.Snapshot()` has no caller again.** The host report is now fed by - `ProvenRestoreTests`, which carries what a bare timestamp cannot. The method's doc comment says in - as many words that it should be deleted if it does not acquire one — deliberately not deleted in - this session, because removing an exported method is a change with no bearing on the three rows. -- **Ten files in this repo are not `gofmt`-clean and were already so on arrival** - (`internal/capability/probe.go`, `internal/escrow/consume.go`, `internal/mgmtplane/mgmtplane.go`, - `internal/reconcile/bringup.go`, `internal/signedjobs/runner.go`, `internal/storage/{candidates, - intent}.go` and three test files). Every file this session touched is clean; the others are - untouched, and no gate checks formatting. -- **The hub sweeps every 60 s and re-reads 14 days of host-reports per customer** for the restore-test - staleness check. Unchanged here and not a defect at this fleet size; it is the cost centre if the - fleet grows, and it is the reason the window read was deliberately left at 14 days yesterday. -- **`felhom.eu/CONTEXT.md` still carries duplicate standing-ruling IDs** (three `S-14`s, two `S-15`s) - from before yesterday. New rulings continue to be numbered above the collision (S-19, S-20) rather - than adding to it; renumbering the existing ones is a separate, purely editorial change. +**Nothing was provisioned.** No scratch storage, no fixture grant, no probe tag, no scratch package +version. The two ACL grants are the intended durable change; the only other mutation was the +installer label, which is reversible by moving the tag. + +## 12. Observations — noticed, recorded, NOT acted on + +- **Both demo boxes are now due for a host-tier restore-test**, which has never run on either. The + scheduler will pick it up within 6 h unattended (a ~6 GB local restore — fast, and cheaper than the + offsite ones). Expected, not a defect, and the first host-tier proof this fleet will have. +- **`--acl-storages` semantics are unchanged and the automatic grant does not consult it.** If an + operator passes `--acl-storages` deliberately excluding the backup target, the target is still + granted by the resolution path. That is the correct precedence — a box cannot function with an + unreadable backup target — but it is a place where an override is not absolute, and it is written + here rather than left to be discovered. +- **`storeGrantRequiredPriv` is a single privilege**, chosen from measurement. If PVE ever changes + which privilege gates content listing, the probe would report healthy while the tier is blind. The + test asserts the constant's value so a change forces a re-measurement, but nothing detects a change + on PVE's side. +- **Ten pre-existing `gofmt`-unclean files** remain in the agent repo (unchanged from yesterday's + observation); every file touched here is clean. diff --git a/REUSE.md b/REUSE.md index c6cc30e..804af99 100644 --- a/REUSE.md +++ b/REUSE.md @@ -148,6 +148,7 @@ | `localapi.DiskOps` / `StorageGate` / `GuestAttacher` / `GuestLister` | internal/localapi/disks.go | `*storage.SudoHostOps`; `storageGateAdapter` (cmd/felhom-agent/main.go); `*GuestBinder`; `*proxmox.Client` | `fakeDiskOps`/`fakeGate`/`fakeGuestAttacher`/`fakeGuestList` internal/localapi/disks_test.go | | `localapi.GuestAPI` / `BackupService` / `BackupStore` / `TokenAuthority` | internal/localapi/server.go | `*proxmox.Client`, `*backup.BackupRunner`, `*backup.Store`, `*TokenStore` | `fakeGuests`/`fakeBackups`/`fakeStore` internal/localapi/server_test.go | | `backup.InFlight` | internal/backup/inflight.go | `TryAcquire(what) (release, busy, ok)` / `Busy()` | THE host-wide "one heavy guest operation at a time" gate — shared by the local-API backup path and the restore-test scheduler (R-85) | A **LINK** guard, not a lock one: the scratch VMID never touches the live guest's vzdump lock, but an offsite restore PULLS multi-GB over the tunnel a backup PUSHES one. Callers **DEFER, never cancel** — a deferred restore-test costs coverage, a cancelled backup costs the backup. A nil gate is ungated (pre-R-85 callers). | +| `capability` store-grant probe (`storeGrantStatuses` / `storeGrantVerdict` / `Client.Permissions`) | cmd/felhom-agent/main.go, internal/proxmox/query.go | *"may the agent READ this backup tier?"*, one `capability.Status` per configured tier | R-185. **Never infer permission from an empty content listing** — `{"data":[]}` is what a FORBIDDEN tier and a NEWBORN tier both return, and that ambiguity hid an unreadable host tier on both demo boxes. Ask `/access/permissions` **as the agent's own token** (root always says yes). **The ungranted answer is not empty and not a 403** — it carries the privileges inherited from the box-wide `/` grant, so test for **`Datastore.AllocateSpace`** specifically; path-presence or `Datastore.Audit` reports a blinded storage healthy. Probed set comes from `BackupTiers()`, never a fixed list. Critical except the `local` fallback. Composes AROUND the sudo prober (the `poolReadStatus` precedent); `Status`'s wire shape is untouched so the hub alert is free. Unreachable PVE ⇒ degraded, never ok. | | `backup.RestoreTestState` | internal/backup/restoretest_state.go | `RecordSuccess(target,archive,tier,verified,t)` / `ProvenArchive(target)` / `ProvenRestoreTests(ctx)` / `LastSuccess(target)` / `OldestFirst(targets)` | Per-tier restore-test PROOF state, persisted (atomic tmp+rename) — **which archive** was proven, and when (R-86) | **Credit ONLY on success** — a permanently failing tier must keep sorting first, or it looks freshly proven and stops being retried. Ties break on target id: without it, two tiers proven in the same second rotate by Go's randomised map order. **This one NEEDS persistence unlike R-84** — R-84 had ground truth to consult (the archive is still on the storage); a restore-test destroys its scratch and leaves no artifact. **R-86: the ARCHIVE is the state, the time is metadata** — a time alone cannot answer "have we proven THIS archive", which is the due-check's whole question. A pre-R-86 file (bare RFC3339 per target) keeps its time and yields NO proven archive, so each tier is due once after the upgrade; reading a legacy time as proof of the current archive would invent a guarantee. **R-189: it is also the REPORTABLE half of the restore-test signal.** The in-memory `backup.Store` holds only this process's latest run, and under per-archive due-ness the agent will not re-test a proven archive — so a proof lost to a restart is not repeated for a whole archive generation (observed live: a passing 14.5 GB offsite restore reached no host-report). `ProvenRestoreTests` renders the stored proofs as `hub.RestoreTest` entries and the collector merges them; a record missing the archive or the tier is NOT emitted, because an unproven tier reading as proven is worse than the defect. **Only successes are stored, deliberately:** a success suppresses future work, a failure causes it. | | `hub.ProvenRestoreTestReporter` + `Collector.SetProvenRestoreTests` | internal/hub/collect.go | the DURABLE restore-test source, merged with the in-memory one | R-189. Merge rule: **one entry per tier, newest by `TestedAt` wins** — a fresh failure beats a stored success (the failure is the news, and it lives nowhere else), a stored success beats a stale in-memory entry after a restart, and a tier never appears twice (the hub would read two tests). An unparseable timestamp counts as OLDER, so a malformed entry cannot displace a good one. **The wiring is pinned by an AST test** — the method this replaced (`RestoreTestState.Snapshot`) carried a doc comment naming a host-report gauge and had no caller for weeks. | | `backup.SpecBuilder` / `backup.TierPicker` / `(*BackupRunner).PickSettledRestoreCandidateOn` | internal/backup/schedule.go, runner.go | `func(ctx,archive) RestoreTestSpec`; `func(ctx,target,notAfter) (archive,landed,error)` | The per-run restore-test spec + per-tier **settled** candidate lookup (R-85, widened by R-86) | The spec is built **PER RUN**, never frozen at construction — the pre-R-85 immediately-invoked value made the offsite tier unschedulable AND went stale on any config change. `SourceTier` comes from **the archive**, never the configured target (the v0.100.0 rule). A tier with no archive returns `("", zero, nil)` — **`""` is NOT an error**, or every fresh box looks broken for its first week. **R-86: `notAfter` is the settle cutoff** (zero = no cutoff, which is what keeps `PickRestoreCandidateOn` a one-line call into it), and the picker now skips entries failing `archivePlausiblyComplete` — under per-archive due-ness an incomplete phantom would be picked forever, fail forever, never earn proof, and make the tier due at EVERY evaluation. |