Files
felhom-agent/REPORT.md
T
admin 03b58cec0a
gates / gates (push) Successful in 7s
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.
2026-08-03 19:04:15 +02:00

206 lines
14 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# REPORT — R-185: a tier the box cannot READ must say so
**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.
---
## 1. Baselines, re-read on arrival
| Repo | `main` @ commit | Version | Matched §1? |
|---|---|---|---|
| `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 **R-189**; R-190+ confirmed free by grep, and none was needed.
## 2. Part 0 — the measurements, before anything was designed against them
**The row's three-way observation, reproduced unchanged:**
| 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 |
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`.
**The permission query, asked by the token itself — and the obvious reading is wrong:**
```
/storage/felhom-pbs → {"Datastore.Allocate":1,"Datastore.AllocateSpace":1}
/storage/felhom-backup → {"Sys.Audit":1,"SDN.Use":1,"Datastore.Audit":1}
```
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.
## 3. The probe
`Client.Permissions` reads `/access/permissions?path=/storage/<target>` **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.
**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.
**Decisions:**
- **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"*.
## 4. The installer — the root cause was not where the row or the task expected
Both assumed `PVE_STORAGES` (the fixed grant list) was the culprit. **It is not.**
`configure_backup_target` has two arms:
- **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**.
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.
**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.
**`$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.
**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.
## 5. Live validation, in order
| # | 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 |
## 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 ./...` — 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 | `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 |
**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.
**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.
## 8. Files, commits, tag
`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`.
**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`.
## 9. Deployment
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-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. Teardown
**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.