admin/felhom-agent
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

v0.114.0 — R-113: drive presence means the DEVICE, not the bind

Browse Source 
BoundUnderParent, the one field the controller's drive-absent gate keys on,
reported only "is this path a mount target in the guest's mountinfo". The
drive's raw mount at /mnt/<name> is a systemd mount unit bound to its device and
dies with it, but the agent's own bind of <raw>/felhom-data under the shared
parent is an ordinary bind: nothing ties it to the device, so its mountinfo
entry OUTLIVES the device as a stale shell. Presence read that survivor as true,
planDriveGates never produced a Stop action, and nothing fired on any channel --
not backup_target_absent, not the generic storage_disconnected. Measured live in
E-2d: detached at 10:58:37Z, silent for 4.5 minutes while the agent itself
logged "enrolled drive absent by UUID" every 20s (felhom.eu
audits/E2D-fresh-vm-2026-07-29.md §5.2).

The fix: BoundUnderParent becomes a CONJUNCTION -- bound under the parent AND
the drive's raw host mount still mounted (devicePresent, new deviceCheck seam
defaulting to isHostMountpoint). Applied at BOTH /disks construction sites. The
union path matters more, not less: it hardcodes State:"attached", so the
raw-mount check is the only device truth that row carries, and it is exactly the
shape E-2d detached.

Why a conjunction and not a replacement: half 2 alone would regress boot
ordering, where the raw drive mounts early and the bind lands ~18s later; the
gate depends on that window reading ABSENT. The conjunction leaves that
byte-identical and closes only the case the gate could never see.

Unknown is never absent: devicePresent("") returns TRUE. A false absent stops a
working customer's apps -- the failure mode of this fix, not of the bug.

Controller UNCHANGED, no MinAgent bump. BoundUnderParent has exactly one
functional consumer (planDriveGates, intermediary.go:226); every other mention
in both repos is a comment or a test, and boot convergence deliberately moved
off it to pollLiveBinds/driveBindLive. The alternative -- a new DevicePresent
bool the controller ANDs in -- was rejected as dangerous: a bool absent from an
older agent's JSON decodes to false, so every drive on a pre-0.114.0 agent would
have read ABSENT and stopped its apps.

Tests +6 in internal/localapi (208 -> 214): groups A-D plus a wire-contract test
asserting the ENCODED bound_under_parent, since that is what crosses to the
controller. Four red-proofs run and reverted (drop the conjunction on each path;
invert unknown; drop the bind half); disks.go verified byte-identical after.

NOT LIVE-VALIDATED. No drive was pulled. Leg awaiting Session C: device loss ->
gate Stop -> SetDisconnected -> backup_target_absent on the wire.
This commit is contained in:
authentik Default Admin
2026-07-29 17:20:16 +02:00
parent 58b598b697
commit b58d7bcf39
7 changed files with 364 additions and 213 deletions
Download Patch File Download Diff File Expand all files Collapse all files
REPORT.md
+76 -208
View File
@@ -1,230 +1,98 @@
# REPORT — F-REBOOT + F-LEAK (v0.107.0 → v0.110.0) (2026-07-28)
# REPORT — R-113: drive presence means the DEVICE, not the bind (agent v0.114.0, 2026-07-29)
**Overwritten** per the standing rule. Agent **v0.106.0 → v0.110.0** across four commits, **three of them
corrections to the first**. F-LEAK took three attempts and two of them were refuted by live test; that
arc is the substance of this report. Companions: `felhom-controller` v0.180.0 (F-OBS) and `felhom.eu`
host-install v1.21.0 (F-LEAK's actual fix).
**Overwritten** per the standing rule. R-113, Session A of three (R-113 → R-114+R-112 → one drill-VM
validating all three). **Repos touched:** `felhom-agent` only; **`felhom-controller` deliberately
UNCHANGED** — see the ruling. **Status: code + tests complete, NOT LIVE-VALIDATED.** No drive was pulled;
that is Session C.
## Baselines (reconfirmed, not copied)
Agent `0.106.0` live on both boxes at start; controller `0.179.0`; `felhom-agent af1c21a`,
`felhom.eu d0cec9d`, `felhom-controller 8f46495`, all clean.
## Phase 0 ruling — shape (a), the agent tells the truth
---
`BoundUnderParent` becomes `bound && devicePresent`. Controller unchanged, no `MinAgent` bump.
## F-REBOOT — a guest rebooted mid-backup never came back (v0.107.0, SHIPPED + PROVEN LIVE)
Fault 11: the backup **succeeded**, and the guest was found `stopped` with 0 containers, no lock, and
nothing retrying — 9m47s of total appliance outage with every alarm silent, because nothing was broken
except that the box was off.
**Why the existing recovery missed it.** `RecoverStaleLockedGuests` already does unlock → delete
dangling snapshot → **start iff onboot**, and it is correct. Two narrow gaps: its predicate acts only
on a guest holding a **stale vzdump lock** (fault 11's guest was unlocked), and it runs **once at agent
startup** on the load-bearing invariant that a lock present *then* is stale by definition — a guest
that goes down while the agent is already up is never re-examined.
`internal/localapi/guestpower.go` closes exactly those two and nothing more: periodic (60 s), acting on
"should be running, is not, is not locked".
### The trap, which is F-CRIT-1's shape
A deliberately stopped guest must not be auto-started; fighting the operator is worse than the outage.
The discriminator is `onboot`, and it is deliberately **not invented here** — it is already what
`stalelock.go` uses for this same decision (so the two paths cannot disagree), it is 0 on
scratch/golden guests, and it is what `pve-guests` itself consults at host boot, so the agent agrees
with the platform instead of keeping a second private definition of "should be running". The hub's
desired-state `Run` is stronger but hub-dependent; `onboot` still works on a box that has lost hub
contact, which is when an unattended appliance most needs to come back.
Guards, in order: running → forget history · `!onboot` → return (Scenario B) · `lock != ""` → leave to
the stale-lock path · `BackupRunning` true **or error** → do not start (a stop-mode backup stops the
guest on purpose) · bounded 3 attempts at 1m/2m/4m → then ERROR once (Scenario C).
### Live, on demo-hp
| | |
| Q | Answer |
|---|---|
| guest stopped out from under the agent | recovered **unattended in 120 s** |
| the finding's original incident | **587 s**, needing a human `pct start` |
| lock-deferral guard | **fired live** during the replay |
| Scenario B, same box | an `onboot:0` guest left stopped across the whole window |
| 1. the symbol | `ResolveStorageDevice` (`internal/storage/durable_device.go:82`, `EvalSymlinks` of `/dev/disk/by-uuid/<uuid>`) behind `hostops.go:298`; `mountByDurable` (`localapi/disks.go:960-969`, **raw mounts only** — binds skipped at `:962`) behind `disks.go:986` |
| 2. available at `/disks`? | Yes at both sites. Observe path has `t.MountPath`/`t.State`; union path has `d.MountPath`/`d.UUID` and already resolves that UUID eight lines later |
| 3. cost | `os.Stat` / `EvalSymlinks` — at most two syscalls per drive per poll. No exec, no shell-out |
| 4. `State == "attached"` | **Split verdict.** Observe path: truthful (`reachable()`, `storage/observe.go:321-330` = `exactMount && DeviceExists`). Union path: **hardcoded `"attached"`** (`disks.go:257`) — so `planDriveGates:229`'s legacy raw branch carries the same defect. **Filed as an observation, not fixed** |
---
**Why (a) and not (b):** `BoundUnderParent` has exactly **one** functional consumer, `planDriveGates`
(`intermediary.go:226`); everything else across both repos is a comment or a test, and boot convergence
deliberately moved off it (`intermediary.go:362`). Tightening it changes one decision — the broken one.
Shape (b) is actively dangerous: a new `bool` absent from a pre-0.114.0 agent's JSON decodes to `false`,
so ANDing it in would make **every drive on an older agent read ABSENT and stop its apps** — the exact
Scenario D failure the spec forbids. Defending it needs `*bool` + feature detection: more machinery, worse
failure mode.
## F-LEAK — and the first fix was WRONG, refuted by its own live replay
## The signal chosen, and why it is the right one
**What I shipped in v0.107.0 and then removed in v0.108.0.** A teardown fallback that, on the 403,
adopted the stranded scratch into the `felhom` pool and retried the destroy — reasoning that the token
holds `Pool.Allocate` on `/pool/felhom`. It fired exactly as designed and PVE refused it:
Not a UUID probe — the **raw mount**. E-2d measured the asymmetry directly: with the device pulled,
`/mnt/mentes2` was NOT mounted while `/mnt/felhom-drives/mentes2` still read `/dev/sdb[/felhom-data]`.
The raw mount is a device-bound systemd unit; the agent's bind is not. So "is the raw mount still
mounted" **is** device presence, it is the same check on both construction paths, and it rests on a live
measurement rather than inference.
```
WARN restore-test: scratch teardown failed — adopting the stranded scratch ... vmid=990000 pool=felhom
ERROR restore-test: pool adoption failed; left for Recover vmid=990000
err="proxmox: PUT /pools/felhom -> HTTP 500: permission denied at /vms/990000 (missing privilege ...)"
```
## Files modified
`PUT /pools/{pool}` **also** requires `VM.Allocate` on the VM being added. **Pool membership cannot
bootstrap its own authority.** The code and its guard were removed rather than left in place — a path
that provably cannot work is worse than no path, because it looks like a fix.
**The real cause is structural**, not a missing privilege in the role: `FelhomAgentGuest` is granted at
`/pool/felhom`, and a guest joins that pool only when its restore **completes**. A *failed*
restore-test therefore leaves a guest that exists, is in no pool, and is outside the token's reach.
### Attempt 2 — the band-scoped ACL. It works exactly ONCE per slot.
`felhom-host-install.sh` v1.21.0 grants the role at each `/vms/990000`…`/vms/990009`. The live A/B on
the same guest, minutes apart, proved it does close the defect:
| | `DELETE /nodes/<node>/lxc/990000` with the agent's own token |
| File | Change |
|---|---|
| **grant removed** (the original defect) | `403 Permission check failed (/vms/990000, VM.Allocate)` — guest still present |
| **grant restored** | `200 UPID:...:vzdestroy:990000:felhom-agent@pve!agent` — guest gone |
| `internal/localapi/server.go` | new `deviceCheck func(string) bool` seam (mirrors `boundCheck`) |
| `internal/localapi/disks.go` | new `devicePresent(rawMountPath)`; conjunction at BOTH construction sites (`:210` Observe, `:263` union); `BoundUnderParent`'s doc restated to name both halves |
| `internal/localapi/intermediary_test.go` | existing fixture pinned `deviceCheck=true` — its subject is the bind half; unchanged otherwise |
| `internal/localapi/disks_device_presence_test.go` | **new** — 6 tests, groups A–D + the wire contract |
| `REUSE.md` | `devicePresent`/`deviceCheck` registered, with the "use this, never the bind" rule |
**And then the post-run ACL count caught it:** `/vms/990000` had **0 grants** afterwards, while the
other nine slots had 2 each. Confirmed in PVE's own source rather than inferred — the destroy path
calls `AccessControl::remove_vm_access($vmid)` (`API2/LXC.pm:906`), which deletes
`acl_root->children->vms->children->{$vmid}`, i.e. **every ACL at `/vms/<vmid>`**
(`AccessControl.pm:1898`).
## Tests: 208 → 214 in `internal/localapi` (+6). Full agent suite green.
**The grant is consumed by the very operation it authorises.** Ten teardowns and the band is ungranted
and the defect is back. A per-VM ACL is structurally the wrong tool; the pool grant is durable only
because it lives on a *pool* path, which `remove_vm_access` leaves alone (it removes the guest's pool
*membership*, not the pool's ACL). I would not have found this without checking the box after the fix.
### Attempt 3 — the fourth root-fenced exception (v0.110.0, SHIPPED)
The route this task offered as the alternative: **the same authority that already performs the create.**
`Privileged.DestroyScratchLXC` joins the fence (previously exactly three: keyctl `pct create`, USB
mount/fstab, SMART/sensors) and is fenced harder than any of them — in three places, none consumed by
use:
| layer | enforcement |
|---|---|
| **sudoers** | `/usr/sbin/pct destroy 99000[0-9] --purge` — sudo matches the vmid **literally**; a compromised agent asking for `9201` is refused by sudo itself |
| **`DestroyScratchLXC`** | re-checks the band before exec; refuses an unconfigured or inverted band rather than defaulting to anything |
| **`teardownScratch`** | acts only on this journal entry's own scratch provenance |
The API destroy is still tried **first** and remains the normal path; the v1.21.0 band ACLs stay
provisioned so the common case needs no privileged call. This is the fallback that makes teardown
deterministic instead of once-per-slot. Ships **with a sudoers change**, deployed alongside the binary.
### The fence, proven live by ASKING sudo rather than executing
Executing the negative case against a live guest is exactly what must not happen, so `sudo -l` was used
to ask whether each command is permitted:
| command | verdict |
|---|---|
| `pct destroy 990000 / 990005 / 990009 --purge` | **PERMITTED** |
| `pct destroy 9201 --purge` (the live customer guest) | **REFUSED** |
| `pct destroy 9100 / 9999 / 1 --purge` | **REFUSED** |
| `pct destroy 990010 --purge` (one past the band) | **REFUSED** |
| `pct start 990000`, `pct set 990000 -memory 512` | **REFUSED** — the grant is this one op, not this vmid |
### The end-to-end proof, in the ACL's natural post-consumption state
The best possible setup arrived by itself: after the attempt-2 destroy, `/vms/990000` had **0 grants**.
A fresh PBS restore to `990000` **without `--pool`** then reproduced the stranded guest with the ACL
genuinely absent — not artificially removed:
```
/vms/990000 grants: 0
990000 stopped (restore rc=0, in no pool)
DELETE via token -> HTTP 403 Permission check failed (/vms/990000, VM.Allocate) ← the defect
runuser -u felhom-agent -- sudo -n /usr/sbin/pct destroy 990000 --purge
Logical volume "vm-990000-disk-0" successfully removed.
Logical volume "vm-990000-disk-1" successfully removed.
Logical volume "vm-990000-disk-2" successfully removed.
purging CT 990000 from related configurations.. rc=0
after: pct list -> 9201 only · 990000 LVs remaining: 0 · /etc/pve/lxc/990000.conf: absent
```
That command vector is byte-identical to what `Privileged.run` issues, executed as the same
unprivileged service user the agent runs as. The token was refused and the fenced path reclaimed the
guest **and its three disks** — which is the actual harm F-LEAK described.
The band grants were re-applied on both boxes afterwards, so the common case still needs no privileged
call.
### Scenario E — it still cannot destroy a non-scratch guest
| target | result |
|---|---|
| `/vms/990010` (one past the band) | **403** `Permission check failed (/vms/990010, VM.Allocate)` |
| `/vms/100` | **403** same |
`990010` does not exist, and PVE still answered **403** rather than "does not exist" — so PVE checks
**permission before existence** and these are genuine authorization refusals, not artifacts. Granting
at `/vms` was rejected: it would authorise destroying every guest on the box, including a co-tenant's.
**A careless step, recorded rather than buried.** That probe loop also issued a live `DELETE` against
running guest **9201**. It was refused with `500 container is running` — but the permission check
*passed* (9201 is a pool member by design), so had the guest been stopped I would have destroyed the
live demo guest. The scratch-band probes were the safe ones; 9201 had no business in that list.
---
## v0.109.0 — the guest-power watchdog gets the observable it shipped without
**Self-correction, caught by applying this session's own F-OBS check to my own new code.** The v0.107.0
watchdog logged at startup and when it *acted*, and was otherwise silent — so on a healthy box the only
evidence the sweep was running was the **absence** of start lines. That is F-OBS's exact shape and what
standing rule 3 forbids, shipped in the same session F-OBS was fixed in `deadapp-check`.
INFO summary every 10th sweep (10 min) carrying `sweeps_since_boot`, `guests_evaluated`,
`currently_stopped` — what it *saw*, not merely that it ran, because "alive, all up" and "alive, one
guest deliberately left down" are different operator facts. Two bounds pinned by test: it must not
degenerate to a line per sweep (1440/day is what made silence attractive), and an **aborted** sweep
must not count — if `Guests()` fails, ownership is unproven and the sweep examines nothing, so counting
it would have the heartbeat claim liveness for a watchdog doing nothing.
---
## Red-proofs — 10 total, all observed failing
| # | red-proof | observed failure |
| Group | Test | Pins |
|---|---|---|
| 1 | delete the `Start` call | `guest 9201 was NOT started — this is F-REBOOT` |
| 2 | remove the `!onboot` guard | `a deliberately stopped guest (onboot:0) was started` |
| 3 | remove the max-attempts branch | `start attempted N times, want at most 3 — infinite retry loop` |
| 4 | drop the backoff | `retried inside the backoff window — the bound is not being honoured` |
| 5 | make `BackupRunning` non-fatal | `started despite being unable to confirm no backup is running` |
| 6 | unfiltered guest list on error | `acted with unproven ownership` |
| 7 | remove the liveness observable | `no liveness observable after 10 sweeps — silence is indistinguishable from a dead watchdog` |
| 8 | count aborted sweeps as healthy | `an aborted sweep was counted as healthy (sweeps=20)` |
| 9 | remove the band check from `DestroyScratchLXC` | `REFUSAL FAILED: executed [pct destroy 9201 --purge] for out-of-band vmid 9201` |
| 10 | remove the unconfigured-band check | `an unconfigured band admitted vmid 0 and EXECUTED [pct destroy 0 --purge]` |
| A | `..._ObservePath_DeviceLossReadsAbsent` | the E-2d shape: bind survives, device gone ⇒ absent |
| A | `..._UnionPath_DeviceLossReadsAbsent` | same on the registry path — the shape E-2d actually detached |
| B | `..._HealthyReadsPresent` (observe+union) | a healthy bound drive reads PRESENT — no false absent |
| C | `..._BootWindowStillReadsAbsent` | raw mounted early, bind not yet ⇒ still absent (no regression) |
| D | `..._UnknownIsNotAbsent` | `devicePresent("")` is TRUE — unknown never means gone |
| wire | `..._WireFieldIsFalseOnDeviceLoss` | the ENCODED `bound_under_parent`, which is what crosses |
`go build`, `go vet ./...`, `go test ./...` — 29 packages, `rc=0`, run separately from every commit.
**Seam wiring:** every test drives `NewServer` → `GET /disks` through `srv.Handler()` → the real JSON.
Only the two lowest-level mount reads are injected (a unit test cannot create real mounts); nothing above
them is faked. The wire test asserts the encoded field rather than the struct, so a regression that
dropped the field from the wire would fail it.
**A process failure of mine, corrected mid-task.** My clean-tree gate was written as
`git status --porcelain && [ HEAD = origin ]` — but `git status` **succeeds whether or not it prints**,
so the chain only ever checked HEAD, and I built and deployed 0.109.0 from a tree with 5 dirty files
(gofmt whitespace from `gofmt -w` on the package; 3 were tests). Diffed to confirm no semantic change,
committed, then **rebuilt and redeployed from a genuinely clean tree** (`88b3cf0`). The gate is now
`if [ -n "$(git status --porcelain)" ]; then abort; fi`.
**Consumer contract already pinned:** `TestPlanDriveGates` (controller) asserts `BoundUnderParent:false`
⇒ exactly one `Stop`, a healthy drive ⇒ zero actions, with an exact action count. Verified green on the
untouched controller tree, so no redundant case was added.
---
## Red-proofs — all four went red with the expected text, all reverted
## Deployed & fleet state
Agent **0.110.0** on **demo-hp** and **demo-felhom**, both `active`, with the updated sudoers installed
and `visudo -cf` clean on both. Band ACLs at 20 rows (10 vmids x user+token) on **both** boxes. Scratch-band ACLs applied on
**both** boxes. No leftover scratch guests — `pct list` shows only `9201(running)` on demo-hp.
| # | Mutation | Failure text produced |
|---|---|---|
| 1 | drop `&& devicePresent` from the **Observe** path | `BoundUnderParent reports present — the bind outlived the device (R-113). The controller's gate would emit no Stop action, so no alarm can fire.` |
| 2 | drop it from the **union** path | `union-path drive reports present — the bind outlived the device (R-113)` **and** `wire bound_under_parent = true (want false)` |
| 3 | make unknown mean absent (`return false`) | `devicePresent("") = false — an unanswerable question was reported as ABSENT` |
| 4 | drop the **bind** half (device only) | `boot window reports present before the bind landed — this regresses the reboot convergence…` |
The only WARN on demo-hp after restart is **F-CRIT-2's fix working**: it rejected the 1-byte phantom
`2026-07-28T05:31:14Z` with `size 1 B is below the 1048576 B plausibility floor`.
After the last revert `disks.go` was verified **byte-identical** to the pre-mutation file (`diff -q`).
**Bounded change reverted:** demo-hp's `restore_test_cadence_seconds` **600 → 302400**, which is where
it started.
## NOT LIVE-VALIDATED — awaiting Session C
---
The leg not exercised: **device loss → gate `Stop` → `SetDisconnected` → `backup_target_absent` on the
wire.** Deployment proves the service starts, nothing more.
## NOT yet live-validated (carried forward)
- **Fault 4** — restic transport interruption; four injection approaches defeated by guest-bridged
networking. Still the most valuable follow-up: F-CRIT-2 answered the phantom question for PBS and it
remains open for restic.
- **R-100** (new, filed today) — a restic offsite tier failing nightly never goes stale on the hub;
`isStale` reads only `LastRun`, which the controller writes unconditionally on failure. Investigated,
deliberately not fixed.
- **R-99** — prune never removes phantom snapshots (no retention bug; they accumulate).
- Fault 12 (host reboot mid-backup); the three-way backup + restore-test + GC overlap; the agent's DR
bring-up path (`ModeDRGuestLoss`), still read-only-verified.
- `contentionAlarmAfter` (3h) — unit-tested with an injected clock, **not** live.
- **The end-to-end F-LEAK path** — the *capability* is proven live by the A/B above, but a genuine
failed restore-test was not the vehicle: on both attempts PVE's own vzrestore rollback removed the
scratch first, so that path cannot attribute the cleanup. The A/B at the seam was chosen precisely
because it can.
⚠️ **Session C blocker to settle first:** a throwaway box installs the agent from the hub's Day-0 artifact
manifest, which currently vouches **0.113.0**. Unless 0.114.0 is published (`scripts/publish-agent.sh`)
and vouched, Session C's fresh box runs the agent **without this fix** and validates nothing for R-113.
This is R-111's trap in the same shape.
## Observations — filed, not fixed
1. **The legacy raw branch carries the same defect.** `disks.go:257` hardcodes `State:"attached"` on the
union path, and `planDriveGates:229` uses `d.State == "attached"` as presence for legacy raw paths — so
a registry-only drive registered at its raw path can never read absent either. Phase 0 Q4; out of scope
per the spec.
2. **`intermediary.go:220-224` is now incomplete, not wrong.** Its claim (presence is the bind, not merely
`State==attached`) still holds; it no longer says the bind now also implies the device. That clause
belongs there — add it in Session B when the controller is legitimately open, rather than touching a
repo Phase 0 ruled out.
3. **R-114's second half is expected to resolve as a side effect.** `SetDisconnected` is called only from
the `Stop` branch, and `firstOfferableDrive` skips `sp.Disconnected` — so once the gate fires, the
vanished drive should stop being offered. **Not acted on, not verified**; Session B/C.