diff --git a/CHANGELOG.md b/CHANGELOG.md index f96393b..883b541 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,40 @@ +## v0.87.0 — SystemDisks device-mapper walk: legacy-boot hosts get a working drive wizard (IA finding 2, MEDIUM) (2026-07-13) + +On a legacy-boot PVE (LVM root, no mounted ESP) `SystemDisks` resolved NOTHING — `wholeDiskOf` +stops at `/dev/mapper/pve-root` — so the all-system fail-safe classified EVERY disk system and +the drive wizard could never offer a candidate (the IA validation's hot-added 5 GB disk stayed +invisible on the drill box). Operator ruling 2026-07-13: walk the root's backing device through +`/sys/block//slaves` recursively down to physical disks (dm AND md; topology, never VG +names); those + any mounted-ESP holder are system; the all-system fail-safe returns to being the +WALK-FAILURE error case only. SAFETY DIRECTION: the outcome made impossible is a root-backing +disk classified candidate — per-branch conservatism means ANY unresolvable slave fails the whole +walk (`ok=false` → all-system, the unchanged code path). + +- **`physicalDisksOf` + `walkSlaves`** (`internal/storage/role.go`): symlink-canonicalize → + fast-path `wholeDiskOf` (raw disks/partitions, unchanged) → recursive sysfs slaves walk for + virtual devices, with cycle/depth guard; non-`/dev` sources (ZFS datasets, NFS, overlay) stay + unwalkable → fail-safe. Partition slaves resolve via the existing regexes (`sda3 → /dev/sda`); + exotic partition names the regexes don't cover (e.g. `md0p1`) fail the walk → fail-safe. +- **`HostReader.BlockSlaves(name)`** — the ONE new seam method (root-free: sysfs is + world-readable): slaves list + hasDir; production reads `/sys/block//slaves`; every test + fake mirrors it. Live-probed (§3): physical disks carry an EMPTY slaves dir — hasDir alone is + not resolution; a virtual device with an empty/unlistable slaves dir fails the walk. +- **Behavior deltas:** legacy-boot LVM hosts now resolve (`{root's physical parents}`, ok=true) + — the wizard lives (still behind the untouched data-bearing/claim guards); EFI(+LVM) hosts are + byte-identical (ESP and walk agree on the same disk — §3 felhom-pve transcript + fixture); + md-raid roots mark BOTH members system. NEW protective delta: a host whose ROOT topology + cannot be fully grounded is now all-system even if an ESP resolved (pre-fix it silently + trusted the ESP alone; ruling's per-disk conservatism). +- **Tests** (`role_walk_test.go`, fake-sysfs fixtures mirroring the §3 transcripts): the + SIGNATURE table (root-backing disk(s) ALWAYS in the system set across legacy-LVM / md-raid / + EFI+raw / EFI+LVM / nested dm-on-md — never weaken), dead-wizard-lives (A), dangling-slave + fail-safe through the REAL sysKnown=false path (D), cycle guard, empty-slaves. Red-proofs + recorded: A pre-fix resolver → dead wizard reproduced; B walk-returns-dm-node → SIGNATURE + VIOLATION naming the missing disk; D conservatism removed → "scratch classified candidate + while walk incomplete". +- Format/mkfs paths, data-bearing guards, wizard UI: UNTOUCHED. MinAgent/controller coupling: + none (agent-internal classification). + ## v0.86.0 — DR-tier-by-default: capability `inactive` state + F-3 provision-parent ownership (2026-07-12) Agent half of the DR-tier-by-default batch (DRILL-day0-vm-2026-07-12; operator decisions: DR diff --git a/CONTEXT.md b/CONTEXT.md index 9246151..70fd02e 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -5,6 +5,20 @@ ## Current +- **v0.87.0** (2026-07-13) — **SystemDisks device-mapper walk (IA finding 2, MEDIUM): + legacy-boot hosts get a working drive wizard.** Operator ruling (approved 2026-07-13, + verbatim): *resolve device-mapper/raid parents — for the root filesystem's backing block + device, walk `/sys/block//slaves` recursively down to physical disks; those, plus any + ESP holder when present, are system. Disks outside that set become wizard candidates (still + subject to the existing data-bearing guards). The all-system fail-safe remains ONLY for walk + failure — it returns to being the error case, not the legacy-boot common case.* Implemented as + `physicalDisksOf`/`walkSlaves` + `HostReader.BlockSlaves` (one seam method); per-branch + conservatism (any unresolvable slave → ok=false → unchanged all-system path); signature test + `TestSystemDisks_WalkTopologies` (root-backing disk ALWAYS system — never weaken). §3 spike + transcripts: drill (legacy) dm-1→sda3→sda; felhom-pve (EFI+LVM) ESP+walk agree on sda → + byte-identical regression. OPEN at ship time: §13.2 wizard live leg (hot-added scratch disk, + Viktor session + GO before format) + Day-0 manifest bump to 0.87.0 (operator UI). + - **v0.83.0** (2026-07-11, LIVE on felhom-pve; NOT published — Peti stays 0.81.0) — **observability pass** (pairs with controller v0.116.1 + hub v0.46.0). `applog.New` → `(logger, *Ring)`: slog fan-out, journald at the configured level, ~1000-entry ring FIXED at DEBUG. `GET /debug/logs` diff --git a/REPORT.md b/REPORT.md index dcf34ab..ccf011f 100644 --- a/REPORT.md +++ b/REPORT.md @@ -1,48 +1,87 @@ -# REPORT — agent v0.86.0: DR-tier-by-default (capability `inactive` + F-3 parent ownership) +# REPORT — v0.87.0: SystemDisks device-mapper walk (IA finding 2, MEDIUM) -**Date:** 2026-07-12 · **Baselines:** agent v0.85.0 (`bcb8dad`) → **v0.86.0**. -**Source of truth:** the DR-tier-by-default batch spec + `felhom.eu/documentation/audits/DRILL-day0-vm-2026-07-12.md`. -**Scope:** agent half of the batch — the capability `inactive` state (disabled ≠ degraded) and the -F-3 provision parent-dir ownership fix. No escrow/ceremony changes (decision 4: identity-only -escrow is PARKED by policy). Green gate: `go build ./... && go vet ./... && go test ./...` — all -26 packages ok. +**Date:** 2026-07-13 · **Version:** felhom-agent v0.87.0 (from v0.86.0, baseline `c20814e` confirmed) · Agent-only classification fix; MinAgent/controller coupling: none. **Safety direction:** the outcome made impossible is a root-backing disk classified candidate; the fixed failure is the opposite over-protection (legacy-boot → everything system → dead wizard). -## What shipped +## §3 spike (live, BEFORE code — both transcripts) -1. **Capability `inactive` state** (`internal/capability/probe.go`, `manifest.go`): - - `StatusInactive = "inactive"`, `ReasonInactive = "disabled by configuration"`. - - `Capability.GatedBy` + `Prober.GateActive(gate)` — a HEALTHY probe of a gated capability - whose gate is off is downgraded ok→inactive. Degraded (binary missing / sudo denied) is - NEVER downgraded: an un-migrated pre-v1.15.0 box must not masquerade as deliberately off. - - The 3 `pbsdr-*` entries are gated via the stable name prefix in `Manifest()` (a future - pbsdr-* op is gated automatically; nothing else ever is — pinned by test). - - `Summarize` counts only real degraded; `logCapabilities` logs an inactive count at INFO and - now runs AFTER the pbsdr wiring so the startup snapshot equals the first report's. -2. **`pbsdr.Manager.DRConfigured()`** (`internal/pbsdr/manager.go`): the gate's answer — live - descriptor state (anything but `disabled`), falling back to the persisted converged marker - before the first desired-state fetch (an applied box never flaps inactive across a restart). -3. **F-3** (`internal/provision/backhalf.go`): a root-run provision chowns `guests/` + - `guests//` to the state-dir's owner (`chown --reference`, non-recursive; bootstrap leaf - stays guest-root). Non-root (daemon) runs skip it via the `geteuid` seam. +- **Drill host** (root@192.168.0.152, the exact IA-failure shape): legacy boot — `/` from + `/dev/mapper/pve-root` → `readlink -f` → **/dev/dm-1** → `/sys/block/dm-1/slaves` = **sda3** → + `/sys/class/block/sda3/partition` exists → parent **sda**. NO /boot/efi mount (sda2 vfat + exists, unmounted — classic legacy shape). Bonus fact baked into the design: a PHYSICAL disk's + `slaves/` dir EXISTS and is EMPTY (`ls /sys/block/sda/slaves | wc -l` = 0) — so `hasDir` alone + is not resolution; the walk requires non-empty slaves to ground a virtual device. +- **felhom-pve** (regression baseline): `/` → dm-1 → slaves sda3 → sda AND `/boot/efi` → + /dev/sda2 → sda — ESP and walk agree on the same disk ⇒ post-fix output must be identical. + Pre-upgrade API captures saved (scratchpad `pve-disks-pre-0870.json`, `pve-cands-pre-0870.json`, + `drill-pre-0870.txt`): felhom-pve roles (felhom-usb/felhom-flash/raw-sdb user-data, local/lvm + system, pbs/offsite backup), candidates EMPTY on both hosts pre-fix. -## Tests + red-proofs (all three mutations proven red, then restored) +No surprises → proceeded. -| Test | Red-proof mutation | Result | -|---|---|---| -| `TestProbe_GateOffHealthyIsInactive` (+ BinaryMissingStaysDegraded, GateOnOrNilIsOK, ExactlyPBSDRGated) | gate branch in `Probe` disabled (`if false`) | inactive case FAILED; degraded case stayed green ✓ | -| `TestDRConfigured_Lifecycle` (unconfigured→applied→restart-marker→disabled) | `DRConfigured` ignores the `disabled` state | disabled case FAILED ✓ | -| `TestProvision_RootRunOwnsGuestsParents` (root issues, non-root doesn't, never `-R`) | F-3 chown block removed | root case FAILED ✓ | +## Implementation (commit on main) -## Live state touched +- `internal/storage/role.go`: `SystemDisks` now grounds every system mountpoint via + **`physicalDisksOf`** — symlink canonicalization → `wholeDiskOf` fast path (raw disks/ + partitions, unchanged) → **`walkSlaves`**: recursive `/sys/block//slaves` resolution for + virtual devices (dm-*, md*), partitions resolved by the existing regexes (`sda3 → /dev/sda`), + cycle/depth guard (visited set, cap 32). Per-branch conservatism (operator ruling): ANY + unresolvable slave — dangling entry, unrecognizable name (e.g. `md0p1`), virtual with empty + slaves — fails the WHOLE walk → `ok=false` → the UNCHANGED all-system fail-safe. Non-`/dev` + sources (ZFS dataset, NFS, overlay) are unwalkable → fail-safe (same as pre-fix). +- `internal/storage/hostread.go`: **`HostReader.BlockSlaves(name) ([]string, bool)`** — the one + new seam method (root-free; sysfs world-readable); `ProcHostReader` reads + `//slaves`; all four test fakes mirror it (storage `fakeHostReader` gains a + `slaves` map; localapi `fakeHostReader`/`uuidHostReader`/`f2HostReader` return nil,false). +- **Caller audit:** `handleDisks`, `roleForMountPath`, `deviceRole` (localapi/disks.go) and + `gatherClaimFacts` (storage/claim.go) — all consume `SystemDisks` through + `isSystemBacked`/`classifyClaim` fail-safes; NONE relies on all-system as a feature (finding: + none needed changes). Format/mkfs paths, data-bearing guards, wizard UI untouched. +- **Behavior deltas:** legacy-boot LVM → resolves, wizard lives; EFI(+LVM) → byte-identical; + md-raid root → BOTH members system. New PROTECTIVE delta: an ESP-resolving host whose root + topology cannot be grounded is now all-system (pre-fix trusted the ESP alone) — per the + ruling's conservatism; direction-safe. -- **felhom-pve latent F-3 state FIXED live** (pre-existing `guests{,/9201}` root:root 0700 → - chowned to felhom-agent, bootstrap subtree untouched — the drill's proven fix). -- **Peti's host: unreachable** — his F-3 check + agent update stay deferred (publish-train D/E/G). +## Tests + red-proofs (fake sysfs via the HostReader seam) -## Shipping notes +`role_walk_test.go` — fixtures mirror the §3 transcripts: legacy-LVM (dm→part→disk), md-raid +(md→2 disks, BOTH asserted system), EFI+raw, EFI+LVM (felhom-pve), nested dm-on-md; +dangling-slave (with a load-bearing RESOLVABLE sibling branch so a skip-mutation yields a +plausible partial set); cycle; virtual-with-empty-slaves. **Signature test +`TestSystemDisks_WalkTopologies`** asserts the root-backing physical disk(s) are ALWAYS in the +system set + the scratch disk never is + role-level consequences — never weaken. -- `configs/felhom-pbs-apply` was already in this repo; host-install **v1.15.0** (felhom.eu, - same-day) now ships it (drill F-7) + `age` (F-10) + `wg_tunnel.enabled: true` (F-9/decision 5). -- Hub-side rendering of the `inactive` chip + the per-customer DR flag = hub v0.51.0 (felhom.eu). -- Deploy: build+push 0.86.0 on 180, deploy to felhom-pve, publish via `scripts/publish-agent.sh`, - hub Day-0 manifest vouch as operator/browser follow-up. +Red-proofs (mutate → fail → revert, verbatim): +- **A (pre-fix resolver, no walk):** `TestSystemDisks_LegacyBoot_WizardLives` FAILS — + "legacy-boot topology must resolve (the all-system fail-safe is the ERROR case…)" — the exact + dead-wizard live shape; the signature table fails on legacy/md/nested too. +- **B (walk returns the dm node):** `SIGNATURE VIOLATION: root-backing disk /dev/sda missing + from system set map[/dev/mapper/pve-root:true]` (+ md0 / nested variants naming both disks). +- **D (per-branch conservatism removed):** `TestSystemDisks_DanglingSlave_FailSafe` FAILS — + scratch classified candidate while the walk is incomplete (asserted through the REAL + `sysKnown=false` → `isSystemBacked`/`RoleForRawDevice` path, not a lookalike). + +Full suite: `go test ./...` green (all packages; recovery-code flake did not fire). + +## §13 deploy + regression + +- (Filled at deploy — build 0.87.0 on 180 → drill host first with `.bak-0.86.0` backup + + restart + clean journal, then felhom-pve; 13.3 = post-upgrade `/disks` + `/disks/candidates` + diffed against the §3 pre-captures.) + +## STOPPED for Viktor (per the task's STOP) + +1. **13.2 wizard live leg** — hot-add the 5 GB scsi scratch disk to the drill PVE VM (qm 300 on + felhom-pve), then through YOUR logged-in dashboard session (no gate-lift): wizard OFFERS the + disk → enroll + format (GO required before format) → unenroll/detach, leave as found. +2. **Day-0 manifest bump** to agent 0.87.0 (hub operator UI, password-gated) + the IA audit doc + finding-2 disposition line (felhom.eu). + +## Observations + +- No `SystemDisks` caller relied on the all-system behavior as a feature (audited: disks.go ×3, + claim.go ×1) — the fix is purely widening resolution. +- md-raid fixture lesson: partitions of an md array (`md0p1`) are NOT covered by the partition + regexes — such a slave fails the walk → all-system (documented in code; acceptable, exotic). +- A physical disk's `slaves/` dir exists and is empty (live-probed) — any future sysfs-walking + code must treat "dir present" ≠ "virtual device". +- Pre-existing `gofmt -l` hits (capability/probe.go, escrow/*.go) — not touched, not mine. diff --git a/REUSE.md b/REUSE.md index 65246a0..2632cf5 100644 --- a/REUSE.md +++ b/REUSE.md @@ -132,7 +132,7 @@ |---|---|---|---| | `proxmox.Runner` | internal/proxmox/privileged.go | `*ExecRunner` (direct/sudo) | `mockRunner` internal/proxmox/mock_test.go; runner fakes in storage tests | | `storage.HostOps` | internal/storage/hostops.go | `*SudoHostOps` (prod), `NoopHostOps` (degraded) | fakes in internal/storage/observe_test.go, watchdog_test.go | -| `storage.HostReader` | internal/storage/hostread.go | `*ProcHostReader` | `fakeHostReader` internal/localapi/disks_test.go; internal/storage/role_test.go | +| `storage.HostReader` | internal/storage/hostread.go | `*ProcHostReader` | `fakeHostReader` internal/localapi/disks_test.go; internal/storage/role_test.go. v0.87.0: `BlockSlaves(name)` lists `/sys/block//slaves` (root-free) — backs the `SystemDisks` dm/md walk (`physicalDisksOf`/`walkSlaves`, role.go); per-branch conservatism: an unresolvable slave fails the WHOLE walk → all-system fail-safe. NEVER weaken the signature test `TestSystemDisks_WalkTopologies` (root-backing disk always in the system set). | | `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 | | `localapi.StaleLockController` | internal/localapi/stalelock.go | `*staleLockController` (Client + Runner + pool) | `fakeStaleLock` (Server-level) stalelock_test.go; `fakeStaleLockAPI` (controller-level, tests the A1 pool intersect) stalelock_pool_test.go | diff --git a/internal/localapi/disks_test.go b/internal/localapi/disks_test.go index b33c5a8..15f4728 100644 --- a/internal/localapi/disks_test.go +++ b/internal/localapi/disks_test.go @@ -90,11 +90,12 @@ func (f fakeGuestList) ListLXC(context.Context) ([]proxmox.Guest, error) { retur // a different whole-disk (e.g. /dev/sdb1) classifies as user-data without touching the real host. type fakeHostReader struct{ mounts []storage.Mount } -func (f fakeHostReader) Mounts() ([]storage.Mount, error) { return f.mounts, nil } -func (f fakeHostReader) ResolveUUID(string) (string, bool) { return "", false } -func (f fakeHostReader) DeviceExists(string) bool { return true } -func (f fakeHostReader) Rotational(string) (bool, bool) { return false, false } -func (f fakeHostReader) Removable(string) (bool, bool) { return false, false } +func (f fakeHostReader) Mounts() ([]storage.Mount, error) { return f.mounts, nil } +func (f fakeHostReader) ResolveUUID(string) (string, bool) { return "", false } +func (f fakeHostReader) DeviceExists(string) bool { return true } +func (f fakeHostReader) Rotational(string) (bool, bool) { return false, false } +func (f fakeHostReader) Removable(string) (bool, bool) { return false, false } +func (f fakeHostReader) BlockSlaves(string) ([]string, bool) { return nil, false } // sysOnSDA is the default system-disk fixture (root on /dev/sda) used by the disk-server test helpers. func sysOnSDA() fakeHostReader { diff --git a/internal/localapi/durableid_rawmount_test.go b/internal/localapi/durableid_rawmount_test.go index 34d35b1..c18ecbc 100644 --- a/internal/localapi/durableid_rawmount_test.go +++ b/internal/localapi/durableid_rawmount_test.go @@ -18,9 +18,10 @@ func (u uuidHostReader) ResolveUUID(dev string) (string, bool) { v, ok := u.uuids[dev] return v, ok } -func (u uuidHostReader) DeviceExists(string) bool { return true } -func (u uuidHostReader) Rotational(string) (bool, bool) { return false, false } -func (u uuidHostReader) Removable(string) (bool, bool) { return false, false } +func (u uuidHostReader) DeviceExists(string) bool { return true } +func (u uuidHostReader) Rotational(string) (bool, bool) { return false, false } +func (u uuidHostReader) Removable(string) (bool, bool) { return false, false } +func (u uuidHostReader) BlockSlaves(string) ([]string, bool) { return nil, false } // Impl-2b: a RAW enrolled drive is not a PVE storage (absent from Observe), so its durable-id must be // resolved from the mount table's device fs-UUID. RED-PROOF: with the Observe-only resolution (before diff --git a/internal/localapi/f2_role_fallback_test.go b/internal/localapi/f2_role_fallback_test.go index 0c85685..85b61e1 100644 --- a/internal/localapi/f2_role_fallback_test.go +++ b/internal/localapi/f2_role_fallback_test.go @@ -33,6 +33,7 @@ func (f *f2HostReader) ResolveUUID(dev string) (string, bool) { u, ok := f.uuids func (f *f2HostReader) DeviceExists(string) bool { return true } func (f *f2HostReader) Rotational(string) (bool, bool) { return false, false } func (f *f2HostReader) Removable(string) (bool, bool) { return false, false } +func (f *f2HostReader) BlockSlaves(string) ([]string, bool) { return nil, false } func (f *f2HostReader) mountsCalls() int { f.mu.Lock(); defer f.mu.Unlock(); return f.calls } // errStorage is a StorageView whose Observe always fails — for the Observe-error fail-safe (edge C3). @@ -67,8 +68,8 @@ func TestRoleForMountPath_F2_BindMountedUserData_Ejectable(t *testing.T) { }} host := &f2HostReader{ mounts: []storage.Mount{ - {Device: "/dev/sda1", MountPoint: "/"}, // system disk - {Device: "/dev/sdb", MountPoint: "/mnt/teszt_enroll"}, // the raw enrolled user-data drive + {Device: "/dev/sda1", MountPoint: "/"}, // system disk + {Device: "/dev/sdb", MountPoint: "/mnt/teszt_enroll"}, // the raw enrolled user-data drive }, uuids: map[string]string{"/dev/sdb": "f2236136-ced7"}, } diff --git a/internal/storage/hostread.go b/internal/storage/hostread.go index fd840c3..7f212cf 100644 --- a/internal/storage/hostread.go +++ b/internal/storage/hostread.go @@ -29,6 +29,11 @@ type HostReader interface { // Removable reads the backing disk's removable flag (true => a USB/hot-plug device). // ok=false when it cannot be determined. Removable(device string) (removable bool, ok bool) + // BlockSlaves lists the component (slave) device names beneath /sys/block//slaves — + // non-empty for VIRTUAL block devices (device-mapper dm-*, md-raid md*), empty for a + // physical disk (the dir exists but has no entries). hasDir=false when has no + // /sys/block entry at all (partitions, unknown names). Root-free (sysfs is world-readable). + BlockSlaves(name string) (slaves []string, hasDir bool) } // Mount is one active-mount-table entry. @@ -156,6 +161,25 @@ func (r *ProcHostReader) Removable(device string) (bool, bool) { return false, false } +// BlockSlaves lists /sys/block//slaves. Every whole device in /sys/block carries the +// slaves/ directory (empty on physical disks); partitions have no /sys/block entry at all — +// they resolve via the partition regexes in role.go, never through here. +func (r *ProcHostReader) BlockSlaves(name string) ([]string, bool) { + name = filepath.Base(strings.TrimSpace(name)) + if name == "" || name == "." || name == "/" { + return nil, false + } + entries, err := os.ReadDir(filepath.Join(r.sysBlockDir(), name, "slaves")) + if err != nil { + return nil, false + } + out := make([]string, 0, len(entries)) + for _, e := range entries { + out = append(out, e.Name()) + } + return out, true +} + // parentDisk maps a device path (possibly a partition like /dev/sdb1 or /dev/nvme0n1p2) // to its parent disk's sysfs name (sdb / nvme0n1). It uses /sys/class/block/, whose // real path ends in ...// for a partition and .../ for a whole disk. diff --git a/internal/storage/observe_test.go b/internal/storage/observe_test.go index 3a093bc..6addb8e 100644 --- a/internal/storage/observe_test.go +++ b/internal/storage/observe_test.go @@ -36,10 +36,11 @@ func (f *fakeStorageAPI) NodeStorage(context.Context) ([]proxmox.Storage, error) type fakeHostReader struct { mounts []Mount mountsErr error - uuids map[string]string // device -> uuid - exists map[string]bool // device -> present - rotational map[string]bool // device -> rotational (presence => known) - removable map[string]bool // device -> removable (presence => known) + uuids map[string]string // device -> uuid + exists map[string]bool // device -> present + rotational map[string]bool // device -> rotational (presence => known) + removable map[string]bool // device -> removable (presence => known) + slaves map[string][]string // block NAME -> sysfs slaves (presence => /sys/block dir exists) } func (h *fakeHostReader) Mounts() ([]Mount, error) { return h.mounts, h.mountsErr } @@ -56,6 +57,10 @@ func (h *fakeHostReader) Removable(device string) (bool, bool) { v, ok := h.removable[device] return v, ok } +func (h *fakeHostReader) BlockSlaves(name string) ([]string, bool) { + s, ok := h.slaves[name] + return s, ok +} // byName indexes observed targets for assertions. func byName(targets []hub.StorageTarget) map[string]hub.StorageTarget { diff --git a/internal/storage/role.go b/internal/storage/role.go index 6ab2044..1194041 100644 --- a/internal/storage/role.go +++ b/internal/storage/role.go @@ -3,6 +3,7 @@ package storage import ( "path/filepath" "regexp" + "strings" "gitea.dooplex.hu/admin/felhom-agent/internal/hub" ) @@ -17,7 +18,7 @@ import ( // - system : the appliance's OS/boot/EFI/guest-rootfs storage. Operator-signature ONLY. // - backup : the backup safety-net (PBS). Operator-signature ONLY (wiping it destroys the net). // - user-data : a customer external data drive — already within the controller's blast radius -// (it bind-mounts /mnt), so a customer informed-confirmation may authorize a wipe. +// (it bind-mounts /mnt), so a customer informed-confirmation may authorize a wipe. // // On ANY ambiguity the agent defaults to the MOST-PROTECTED role (system) — consistent with the // destructive-on-ambiguity invariant: an unrecognized device is treated as protected, never silently @@ -34,15 +35,21 @@ const ( var reWholeDisk = regexp.MustCompile(`^/dev/(?:sd|hd|vd)[a-z]+$|^/dev/nvme[0-9]+n[0-9]+$`) // systemMountPoints are the host mountpoints whose backing whole-disk is, by definition, the OS / -// system disk. /boot and /boot/efi are the load-bearing ones: on a typical Proxmox/Debian install -// the ESP is a raw partition directly on the OS disk, so it pins the OS whole-disk even when / is on -// LVM/device-mapper (which we cannot trace back to a raw disk without privileged LVM introspection). +// system disk. A mounted ESP (/boot/efi) pins the OS disk directly; an LVM/device-mapper root is +// traced to its physical parents by the root-free sysfs slaves/ walk (v0.87.0 — before that, a +// legacy-boot host with no mounted ESP resolved NOTHING and the all-system fail-safe killed the +// drive wizard permanently, IA finding 2). var systemMountPoints = map[string]bool{"/": true, "/boot": true, "/boot/efi": true} // SystemDisks resolves the set of whole-disk device paths that host the OS (the disks backing /, -// /boot and /boot/efi). ok=false when NONE could be resolved (no system mountpoint mapped to a raw -// disk) — callers then treat every candidate as system (most protected). Root-free: it parses the -// mount table + world-readable /dev symlinks only (the root-CLI fence is untouched). +// /boot and /boot/efi). Virtual backing devices (device-mapper/LVM, md-raid — the legacy-boot +// common case where / sits on /dev/mapper/pve-root and no ESP is mounted) are walked recursively +// through the sysfs slaves/ chain down to their physical parent disks (operator ruling +// 2026-07-13: walk topology, never VG names). ok=false ONLY when the topology could not be fully +// grounded — no system mountpoint found, or ANY system mount whose backing device the walk could +// not resolve to physical disks — and callers then treat every candidate as system (most +// protected). That all-system fail-safe is back to being the ERROR case, not the legacy-boot +// common case. Root-free: mount table + /dev symlinks + world-readable sysfs only. func SystemDisks(host HostReader) (set map[string]bool, ok bool) { if host == nil { return nil, false @@ -56,13 +63,73 @@ func SystemDisks(host HostReader) (set map[string]bool, ok bool) { if !systemMountPoints[cleanMountPath(m.MountPoint)] { continue } - if wd, wok := wholeDiskOf(m.Device); wok { - set[wd] = true + disks, dok := physicalDisksOf(host, m.Device) + if !dok { + // A system mount we cannot ground in physical disks — the WHOLE resolution is + // undeterminable. Never return a partial set as ok: a root-backing disk missing + // from the set is exactly the catastrophic direction (a system disk offered as + // a wizard candidate). + return nil, false + } + for _, d := range disks { + set[d] = true } } return set, len(set) > 0 } +// physicalDisksOf resolves a mounted device to the PHYSICAL whole disks backing it. Plain +// disks/partitions resolve directly (wholeDiskOf); a virtual device (dm-*, md*) is walked via +// its sysfs slaves. ok=false when the device cannot be grounded (network/dataset sources, +// unknown names, or any unresolvable slave branch). +func physicalDisksOf(host HostReader, device string) ([]string, bool) { + if device == "" { + return nil, false + } + dev := device + if resolved, err := filepath.EvalSymlinks(device); err == nil { + dev = resolved // canonicalize /dev/mapper/pve-root → /dev/dm-1, by-uuid links, … + } + if wd, wok := wholeDiskOf(dev); wok { + return []string{wd}, true // already a raw disk or a recognizable partition + } + if !strings.HasPrefix(dev, "/dev/") { + return nil, false // ZFS dataset, NFS, overlay, … — not a block topology we can walk + } + return walkSlaves(host, filepath.Base(dev), map[string]bool{}) +} + +// walkSlaves recursively resolves a VIRTUAL block device name (dm-*, md*) to physical whole +// disks via /sys/block//slaves. Per-branch conservatism (operator ruling): ANY slave that +// cannot be resolved — a dangling entry, an unrecognizable name, a virtual device with no +// listable slaves — fails the WHOLE walk. The candidate/protected verdict must never rest on a +// partially-understood topology. visited doubles as the cycle/degenerate-depth guard. +func walkSlaves(host HostReader, name string, visited map[string]bool) ([]string, bool) { + if name == "" || visited[name] || len(visited) > 32 { + return nil, false + } + visited[name] = true + slaves, hasDir := host.BlockSlaves(name) + if !hasDir || len(slaves) == 0 { + // No /sys/block entry (not a whole device) or nothing beneath a supposed virtual + // device — either way this branch cannot be grounded. + return nil, false + } + var out []string + for _, s := range slaves { + if wd, wok := wholeDiskOf("/dev/" + s); wok { + out = append(out, wd) // a physical disk or a partition of one (sda3 → /dev/sda) + continue + } + sub, sok := walkSlaves(host, s, visited) + if !sok { + return nil, false + } + out = append(out, sub...) + } + return out, true +} + // wholeDiskOf maps a device path (a partition, a whole disk, or a /dev/disk/by-* symlink) to its // whole-disk /dev path. ok=false when the result is not a recognizable raw disk (device-mapper / LVM // / network) — the caller then treats the topology as undeterminable (→ most-protected). diff --git a/internal/storage/role_walk_test.go b/internal/storage/role_walk_test.go new file mode 100644 index 0000000..5687e02 --- /dev/null +++ b/internal/storage/role_walk_test.go @@ -0,0 +1,198 @@ +package storage + +import "testing" + +// v0.87.0 — SystemDisks device-mapper/md walk (IA finding 2, operator ruling 2026-07-13). +// The direction that must be IMPOSSIBLE is a root-backing disk classified as a candidate; +// the direction being fixed is the legacy-boot all-system over-protection (dead wizard). +// +// Fixtures mirror the §3 live transcripts: on the drill host / → /dev/mapper/pve-root → +// dm-1 → slaves sda3 → parent sda; physical disks carry an EMPTY slaves dir (hasDir=true). +// COMPANION red-proofs (run → fail → revert, recorded in REPORT): +// B: walk returns only the dm node (pre-fix shape) → TestSystemDisks_WalkTopologies legacy +// fixture fails its system-set assertion. +// A: pre-fix resolver (no walk) → TestSystemDisks_LegacyBoot_WizardLives fails +// (reproduces the dead-wizard live shape). +// D: per-branch conservatism removed (skip unresolvable slaves) → the dangling fixture +// fails on "scratch classified candidate while walk incomplete". + +// walkTopology is one fixture: the host's mounts + sysfs slaves tree, with the disks that +// MUST be system and a scratch disk that must stay outside the set. +type walkTopology struct { + name string + mounts []Mount + slaves map[string][]string + wantSystem []string // every entry MUST be in the resolved set (signature assertion B) + scratch string // must NOT be in the set (wizard-eligible) +} + +func walkTopologies() []walkTopology { + return []walkTopology{ + { + // The drill host's exact shape (§3 transcript): legacy boot, LVM root, no ESP mount. + name: "legacy LVM root (dm -> partition -> disk)", + mounts: []Mount{{Device: "/dev/mapper/pve-root", MountPoint: "/", FSType: "ext4"}}, + slaves: map[string][]string{ + "pve-root": {"sda3"}, + "sda": {}, // physical disk: slaves dir exists, empty (live-probed) + }, + wantSystem: []string{"/dev/sda"}, + scratch: "/dev/sdd", + }, + { + // md-raid root: BOTH member disks are system. + name: "md-raid root (md -> 2 disks)", + mounts: []Mount{{Device: "/dev/md0", MountPoint: "/", FSType: "ext4"}}, + slaves: map[string][]string{ + "md0": {"sda1", "sdb1"}, + }, + wantSystem: []string{"/dev/sda", "/dev/sdb"}, + scratch: "/dev/sdd", + }, + { + // Plain EFI + raw partitions (the pre-walk demo shape) — no virtual layer at all. + name: "EFI + raw partitions", + mounts: []Mount{ + {Device: "/dev/sda2", MountPoint: "/", FSType: "ext4"}, + {Device: "/dev/sda1", MountPoint: "/boot/efi", FSType: "vfat"}, + }, + slaves: map[string][]string{}, + wantSystem: []string{"/dev/sda"}, + scratch: "/dev/sdd", + }, + { + // felhom-pve's exact shape (§3 baseline): EFI mount AND LVM root — the walk and the + // ESP must agree on the same disk (scenario C: output identical pre/post). + name: "EFI + LVM root (felhom-pve)", + mounts: []Mount{ + {Device: "/dev/mapper/pve-root", MountPoint: "/", FSType: "ext4"}, + {Device: "/dev/sda2", MountPoint: "/boot/efi", FSType: "vfat"}, + }, + slaves: map[string][]string{ + "pve-root": {"sda3"}, + }, + wantSystem: []string{"/dev/sda"}, + scratch: "/dev/sdd", + }, + { + // Nested virtual layers: dm on md on partitions of two disks. + name: "nested dm-on-md", + mounts: []Mount{{Device: "/dev/mapper/pve-root", MountPoint: "/", FSType: "ext4"}}, + slaves: map[string][]string{ + "pve-root": {"md0"}, + "md0": {"sda2", "sdb2"}, + }, + wantSystem: []string{"/dev/sda", "/dev/sdb"}, + scratch: "/dev/sdd", + }, + } +} + +// TestSystemDisks_WalkTopologies is the SIGNATURE test (scenario B): in every topology the +// walk resolves, the root-backing physical disk(s) are ALWAYS in the system set. This +// assertion may never be weakened. +func TestSystemDisks_WalkTopologies(t *testing.T) { + for _, tc := range walkTopologies() { + t.Run(tc.name, func(t *testing.T) { + host := &fakeHostReader{mounts: tc.mounts, slaves: tc.slaves} + set, ok := SystemDisks(host) + if !ok { + t.Fatalf("SystemDisks must resolve this topology, got ok=false (set=%v)", set) + } + for _, d := range tc.wantSystem { + if !set[d] { + t.Errorf("SIGNATURE VIOLATION: root-backing disk %s missing from system set %v", d, set) + } + } + if set[tc.scratch] { + t.Errorf("scratch disk %s wrongly in the system set %v", tc.scratch, set) + } + // And the classification consequences: the system disk is never user-data, the + // scratch disk is never system. + if RoleForRawDevice(tc.wantSystem[0], set, ok) != RoleSystem { + t.Errorf("root-backing disk %s must classify system", tc.wantSystem[0]) + } + if RoleForRawDevice(tc.scratch, set, ok) != RoleUserData { + t.Errorf("scratch disk %s must classify user-data (wizard-eligible)", tc.scratch) + } + }) + } +} + +// TestSystemDisks_LegacyBoot_WizardLives (scenario A): the drill-box shape resolves — the +// scratch disk is wizard-eligible instead of the pre-fix all-system dead end. +// RED-PROOF companion: with the pre-fix resolver (wholeDiskOf only, no walk) this fails on +// ok=false — the exact dead-wizard live shape from the IA report. +func TestSystemDisks_LegacyBoot_WizardLives(t *testing.T) { + host := &fakeHostReader{ + mounts: []Mount{{Device: "/dev/mapper/pve-root", MountPoint: "/", FSType: "ext4"}}, + slaves: map[string][]string{"pve-root": {"sda3"}}, + } + set, ok := SystemDisks(host) + if !ok { + t.Fatal("legacy-boot topology must resolve (the all-system fail-safe is the ERROR case, not the legacy-boot case)") + } + if !set["/dev/sda"] { + t.Fatalf("root parent disk missing: %v", set) + } + if isSystemBacked("/dev/sdd", set, ok) { + t.Fatal("hot-added scratch disk still classified system — the wizard stays dead") + } +} + +// TestSystemDisks_DanglingSlave_FailSafe (scenario D): an unresolvable slave fails the WHOLE +// walk and callers see the all-system behavior through the SAME code path as today +// (sysKnown=false → isSystemBacked true for everything). +// RED-PROOF companion: treating an unresolved slave as skippable makes ok=true here → the +// scratch disk classifies candidate while the walk is incomplete → this test fails. +func TestSystemDisks_DanglingSlave_FailSafe(t *testing.T) { + host := &fakeHostReader{ + mounts: []Mount{{Device: "/dev/mapper/pve-root", MountPoint: "/", FSType: "ext4"}}, + slaves: map[string][]string{ + // One branch resolves (sda3 → sda), one is DANGLING (dm-9 has no /sys/block entry). + // The resolvable branch is load-bearing for the red-proof: a "skip the unresolved + // slave" mutation would yield a plausible non-empty set — exactly the partial + // topology the conservatism rule forbids. + "pve-root": {"sda3", "dm-9"}, + }, + } + set, ok := SystemDisks(host) + if ok { + t.Fatalf("a dangling slave must fail the whole walk, got ok=true set=%v", set) + } + // The fail-safe path itself (not a lookalike): sysKnown=false forces system for EVERY + // device, exactly as the pre-walk legacy behavior did. + if !isSystemBacked("/dev/sdd", set, ok) { + t.Fatal("fail-safe violated: scratch disk classified candidate while the walk is incomplete") + } + if RoleForRawDevice("/dev/sdd", set, ok) != RoleSystem { + t.Fatal("fail-safe violated at role level") + } +} + +// TestSystemDisks_CycleGuard: a slaves cycle (corrupt sysfs / hostile fixture) terminates and +// fails safe instead of recursing forever. +func TestSystemDisks_CycleGuard(t *testing.T) { + host := &fakeHostReader{ + mounts: []Mount{{Device: "/dev/mapper/pve-root", MountPoint: "/", FSType: "ext4"}}, + slaves: map[string][]string{ + "pve-root": {"dm-1"}, + "dm-1": {"pve-root"}, + }, + } + if _, ok := SystemDisks(host); ok { + t.Fatal("a slaves cycle must fail safe (ok=false)") + } +} + +// TestSystemDisks_VirtualWithEmptySlaves: a virtual root whose slaves dir is EMPTY (nothing to +// ground on) fails safe — hasDir alone is not resolution. +func TestSystemDisks_VirtualWithEmptySlaves(t *testing.T) { + host := &fakeHostReader{ + mounts: []Mount{{Device: "/dev/mapper/pve-root", MountPoint: "/", FSType: "ext4"}}, + slaves: map[string][]string{"pve-root": {}}, + } + if _, ok := SystemDisks(host); ok { + t.Fatal("virtual device with no slaves must fail safe") + } +}