diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e40639..7fab85a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,63 @@ All notable changes to **felhom-agent** are recorded here. Update on every code change that gets pushed. +## v0.5.0 — slice 5 Phase B: the host-root surface (mounts + SMART + grow + destructive gate) (2026-06-09) + +The write surface — the agent's first step outside its Proxmox API token into OS-root. +Isolated behind a narrow, argument-validated, adversarially-tested seam, exactly like the +slice-4 gate. Completes slice 5 (Phase A = read-only observe/report/watchdog at v0.5.0-rc1). + +### Added +- **`HostOps` seam + `SudoHostOps`** (`internal/storage/hostops.go`) — the one privileged + host surface: persistent mounts via **systemd `.mount` units keyed by fs-UUID** (enabled to + survive reboot), detach (stop+disable), SMART, and thin-pool metadata. Shells out via the + fenced Runner (`sudo -n`, **fixed arg vectors, no shell**); a fake backs the tests (no real + root in the suite). `NoopHostOps` is the safe fallback when the surface is unavailable. +- **The argument validator** (`internal/storage/validate.go`) — the security boundary: + `ValidateUUID` (strict hex), `ValidateMountPath` (absolute, no traversal, no metacharacters), + `ValidateSMARTDevice` (raw-disk whitelist), `ValidateLVMName`, and an in-process + `systemdEscapePath` (no `systemd-escape` shell-out). **Every argument is validated BEFORE a + command is constructed.** Headline test (`validate_test.go`): an adversarial matrix of + shell metacharacters / `../` traversal / malformed inputs is rejected with **zero exec**. +- **SMART** (`internal/storage/smart.go`) — parses `smartctl -a -j` into `StorageTarget.smart`: + **SATA** (reallocated/pending/offline-uncorrectable, temp, power-on-hours) **and NVMe** + (critical_warning, media_errors, percentage_used, temp), degrading to `UNKNOWN` for devices + with no SMART (USB-SATA bridges). **`lvs`** fills the lvmthin thin-pool **metadata** fill + (the value Phase A left null). Wired into the Observer's enrichment (Observe only, not the + watchdog's fast Known path). +- **Watchdog re-mount response** (`internal/storage/watchdog.go`) — on a known mount-backed + target's device returning **unmounted** (a new `DevicePresent` liveness probe), the watchdog + **dispatches a benign by-UUID re-mount off the poll path** (a goroutine, never under the + lock), rate-limited per target to the debounce window. The mount is routed through the gate + as benign (`gateRemounter` in `main.go`, so `storage` stays decoupled from `reconcile`). +- **Disk-grow executor** (`internal/reconcile`) — `ActionResize` (benign `ClassResize`), planned + **grow-only** (desired DiskBytes > actual → `pct resize rootfs +M`; a shrink is refused, + never silently grown) + a defensive executor guard (size must start with `+`). New + `proxmox.Client.ResizeLXC` (API; `VM.Config.Disk`+`Datastore.AllocateSpace`; async→UPID). + Built + fixture-tested; **unfed** live (no hub spec until slice 10). +- **Destructive storage ops through the slice-4 gate** (`internal/reconcile/storage_ops.go`) — + `IntentForStorageMount` (benign) and `IntentForStorageDestructive` (`ClassStorageWipe`/ + `ClassDecommission`). Host/target-scoped: the op binds on the storage **target identity** + (carried in `target.guest_id`). Reuses the existing verifier/role-scoping/binding/audit — no + new gate, no new crypto. Storage cases added to the adversarial matrix (`storage_test.go`): + unsigned wipe → `pending_signature`; "wipe A" signature vs "wipe B" → `binding_mismatch`; + valid → accepted. **Inert** live. +- **`--selftest=storage` [`-watch `]** — the live USB-runbook harness: an observe pass + (full table incl. SMART + thin-pool data+metadata), and a bounded watchdog window with the + re-mount response live. Runs standalone on the Proxmox host (no hub). +- **`configs/felhom-agent.sudoers`** — the documented narrow allowlist (install unit / systemctl + manage / smartctl / lvs), with the agent-side fine validation noted. +- **Config**: `privileged.{unit_dir,stage_dir,systemctl,install,smartctl,lvs}` (paths must match + the sudoers entries). + +### Notes +- Daemon still runs cleanly with no removable storage / no signers / no hub manifest, and a + missing/declined sudoers entry degrades with a warning (SMART→UNKNOWN, mount→logged error), + not a crash. `go test -race` passes (the watchdog re-mount dispatches off the poll path). +- Slice-3/4 + Phase-A exported surfaces, goldens, and adversarial tests intact. `authz` + untouched. The destructive-storage executor + grow are built/tested but unfed live until + slice 10. + ## v0.5.0-rc1 — slice 5 Phase A: storage observe + report + watchdog (read-only, live) (2026-06-09) Phase A of the storage slice (doc 03 §7). Read-only and live: the agent now observes every diff --git a/CLAUDE.md b/CLAUDE.md index 1254cab..5d9db5b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -15,7 +15,7 @@ - Module `gitea.dooplex.hu/admin/felhom-agent`; binary `felhom-agent` (`cmd/felhom-agent/`). - **Pure Go stdlib + `golang.org/x/crypto` only** — no web frameworks. - `go.mod` directive **go 1.25.0**; dep `golang.org/x/crypto v0.52.0` (declares go 1.25, will NOT build on Go 1.24). The **build server (192.168.0.180) runs go1.26.0** (upstream Go on PATH, backward-compatible). Build/run the agent there for live tests (same LAN as the demo host). -- Version: `version` var in `cmd/felhom-agent/main.go`, overridable via `-ldflags "-X main.version="`; `--version` flag. **Current: v0.5.0-rc1** (slice 5 Phase A: storage observe/report + watchdog, read-only; Phase B = the host-root surface, pending). Bump on meaningful changes + add a CHANGELOG entry. +- Version: `version` var in `cmd/felhom-agent/main.go`, overridable via `-ldflags "-X main.version="`; `--version` flag. **Current: v0.5.0** (slice 5 complete: storage observe/report + watchdog + the host-root surface — mounts/SMART/grow/destructive gate). Bump on meaningful changes + add a CHANGELOG entry. ## Layout @@ -52,8 +52,9 @@ Built in slices, all on `main`: - **v0.3.2** — slice-4 pre-check: reversible `SetConfig` step added to `--selftest=task`; passed live on guest 9999. Findings: LXC `description` write is **synchronous** (empty UPID — dual-mode modeling confirmed); PVE appends a trailing `\n` to `description` on read (reconcile must normalize). First live `VM.Config.*` exercise. - **v0.4.0-rc1** — slice-4 **Phase A** (structural): `internal/reconcile` — engine, per-guest serializer (§10), desired-state model + `DesiredProvider` seam, normalization layer (`NormDescription` promoted out of main.go), plan/diff engine (benign Start/Stop/SetConfig set), durable op journal + idempotency store. Wired into `runDaemon` sharing the queue. Runs **live but unfed** (EmptyProvider → zero mutations until slice 10). - **v0.4.0** — slice-4 **Phase B** (security core): the benign/destructive **classifier** (provenance + data-bearing, not by verb; scratch/same-txn provenance is agent-internal, never hub-sourced), the **reversibility gate** (destructive → `pending_signature` unless a verified, role-scoped, action-bound operator signature), the **signed-op consuming layer** over `internal/authz` (role-scoping per doc 04 §4, op-to-action binding, idempotency-by-nonce, audit), and the **crash-recovery consumer** (`Recover` over `InFlight()`, resume-or-rollback). The gate fronts the queue's executor (every mutation passes it). **Inert this slice** — no destructive deltas served until slice 10; the destructive path is classified, gated, and adversarially tested but not wired to live execution. `authz` surface untouched. -- **v0.5.0-rc1** — slice-5 **Phase A** (read-only, live): `internal/storage` — the `StorageTarget` wire contract (filled the slice-3 stub), `durable_id` derivation per type, the `Observer` (builds `storage_targets` from `ListStorage`/`NodeStorage` + non-privileged host reads), and the **storage watchdog** (third daemon goroutine; fast-poll → debounced out-of-band report on a known target's attach/disconnect). No host-root writes. Hub ingest extended to accept/persist `storage_targets`; cross-repo golden kept byte-identical. **Phase B (pending)** = the host-root surface: systemd `.mount` units + sudoers allowlist, benign re-mount, SMART, disk-grow executor, destructive-storage gating. -- **Next: slice 5 Phase B (host-root surface), then slice 6 (backup/restore)** — the destructive executors the gate already guards. +- **v0.5.0-rc1** — slice-5 **Phase A** (read-only, live): `internal/storage` — the `StorageTarget` wire contract (filled the slice-3 stub), `durable_id` derivation per type, the `Observer`, and the **storage watchdog** (third daemon goroutine; fast-poll → debounced out-of-band report on a known target's attach/disconnect). Hub ingest accepts/persists `storage_targets`; cross-repo golden byte-identical. +- **v0.5.0** — slice-5 **Phase B** (the host-root surface): the `HostOps` seam + `SudoHostOps` (systemd `.mount` units by fs-UUID, detach, SMART, lvs) behind a **strict argument validator** (the adversarial matrix is the headline security test — hostile UUID/path/device refused with zero exec); SMART (SATA+NVMe) + thin-pool metadata enrichment; the watchdog's benign **re-mount response** (off the poll path); the **disk-grow executor** (`pct resize`, grow-only, benign) and **destructive storage ops** through the slice-4 gate (target-scoped; built + tested, inert live); `--selftest=storage [-watch]`; `configs/felhom-agent.sudoers`. +- **Next: slice 6 (backup/restore orchestration)** — vzdump/PBS + the self-restore-test; the restore-overwrite executor the gate already guards. ## Demo host (for live tests) diff --git a/README.md b/README.md index 74ca4a6..e516900 100644 --- a/README.md +++ b/README.md @@ -47,6 +47,41 @@ The reported `StorageTarget` shape is a cross-repo contract duplicated in `felho `internal/hub/testdata/host-report.golden.json` is byte-identical with the hub's copy and a bidirectional key-set test guards drift. +### The privileged `HostOps` surface (slice 5 Phase B) + +The write side — the one place the agent steps outside its Proxmox API token into OS-root — +is isolated behind the `HostOps` seam (`hostops.go`): production `SudoHostOps` shells out via +a narrow **sudoers allowlist** (`configs/felhom-agent.sudoers`) with **fixed argument vectors +and no shell**; tests use a fake (no real root in the suite). + +- **Persistent mounts** are **systemd `.mount` units keyed by fs-UUID** + (`What=/dev/disk/by-uuid/`, enabled so they survive reboot) — not raw fstab or a + transient `mount`. Benign re-mount is idempotent; **detach** (stop+disable) is destructive + and routes through the gate. +- **Every argument is validated before any command is constructed** (`validate.go`): UUIDs + against a strict hex regex, mount paths confined + traversal-checked, SMART devices + whitelisted to raw disks, LVM names charset-checked. The adversarial matrix in + `validate_test.go` proves a hostile UUID / path / device is refused with **zero** exec. +- **SMART** (`smart.go`) fills `StorageTarget.smart` via `smartctl -a -j` — SATA *and* NVMe + attribute sets, degrading to `UNKNOWN` for devices that expose no SMART (e.g. a USB bridge). + **`lvs`** fills the lvmthin thin-pool **metadata** fill (metadata exhaustion corrupts a pool + like data exhaustion). +- The **watchdog** gains a benign **re-mount response**: when a known mount-backed target's + device returns unmounted, it dispatches (off the poll path) a by-UUID re-mount, routed + through the gate as benign. The **disk-grow executor** (`pct resize`, **grow-only**) lands + in `internal/reconcile` as a benign action; **destructive storage ops** (detach/wipe/ + data-losing-resize) construct a `ClassStorageWipe`/`ClassDecommission` intent bound to the + storage **target identity** and go through the slice-4 gate (built + tested, inert live). + +### `--selftest=storage` (live storage harness) + +Runs standalone on the Proxmox host (no hub needed): +- bare: an **observe pass** printing the full `StorageTarget` table incl. the SMART summary + and thin-pool data+metadata fill. +- `-watch ` (e.g. `--selftest=storage -watch 3m`): runs the watchdog verbose for the + window with the **re-mount response live**, so an operator can physically cycle a drive and + watch detect → report → re-mount in the logs. + ## The `proxmox` package — model Two backends, one fixed routing policy (the fence is structural — `Client` never shells out, diff --git a/REPORT.md b/REPORT.md index f6294b2..fb9bb9c 100644 --- a/REPORT.md +++ b/REPORT.md @@ -1,81 +1,79 @@ -# REPORT — Slice 5 Phase A: storage observe + report + watchdog (v0.5.0-rc1) (2026-06-09) +# REPORT — Slice 5 Phase B: the host-root surface (v0.5.0) (2026-06-09) > Overwrite-latest report (most recent significant work only). Cumulative history lives in [CHANGELOG.md](CHANGELOG.md). ## Outcome -**Slice 5 Phase A is complete and pushed as `v0.5.0-rc1`** — the read-only, live half of the -storage slice (doc 03 §7). The agent now observes every host storage target, reports it into -the host-report (the slice-3 `storage_targets` stub is filled), and runs a fast-poll storage -watchdog that pushes a disconnect to the hub out-of-band in seconds. **No host-root writes** -this phase — mounts/SMART/disk-grow/destructive-gate are Phase B, which earns isolated review -of the new privileged surface. This is the hard checkpoint: Phase A awaits validation before -Phase B begins. +**Slice 5 is complete and pushed as `v0.5.0`.** Phase B adds the privileged **write surface** — +the agent's first step outside its Proxmox API token into OS-root — isolated behind a narrow, +argument-validated, adversarially-tested seam, the same discipline as the slice-4 reversibility +gate. Phase A (read-only observe/report/watchdog, `v0.5.0-rc1`) is reused unchanged. ## What landed -New package **`internal/storage`**: - -- **`StorageTarget` wire contract** (`internal/hub/report.go`) — the slice-3 `struct{}` stub - is now the full reported shape: identity (`name`/`type`/`durable_id`), `state` - (`attached`/`disconnected`/`decommissioned`) + `reachable`, usage (`total`/`used`/`avail`/ - `used_fraction`), `content`, `mount_path`/`backing_device`, a `class_hint` (a rotational - HINT — **never** authoritative; class is hub-owned), `role` (empty until slice 10), a - `thin_pool` sub-object (lvmthin DATA fill), and a `smart` sub-object (`UNKNOWN` until Phase B). -- **`durable_id` derivation** — deterministic per type, the DR-load-bearing re-attach key: - fs-UUID (usb/local-dir), `server:export` (nfs/cifs), `repo+fingerprint` (pbs), `vg/pool` - (lvmthin). Never empty — falls back to a stable store id so the hub's re-attach lookup - always has something. -- **`Observer`** — joins `ListStorage` (config) + `NodeStorage` (usage/active) with - non-privileged host reads (`/proc/mounts`, `/dev/disk/by-uuid`, `/sys/.../rotational` + - `removable`) behind a `HostReader` seam. Surfaces the lvmthin **thin-pool data fill** - prominently (a full pool corrupts every guest on it) and warns at ≥85%. -- **Storage watchdog** — the third daemon goroutine (alongside the hub loop + reconcile - engine). Fast-polls (default 8s) the *known* target set for `attached↔disconnected` - transitions and fires a **debounced** (default 30s) out-of-band host-report. Flags only a - *known* target's change (never a never-attached device), coalesces flaps (leading + - trailing edge). `CachingKnownTargets` rate-limits the Proxmox-derived known set; - `HostLiveness` does device/mount-presence (local) + a reachability dial (network). - -Wiring + supporting changes: - -- `proxmox.Storage` gained additive parse-only config fields (server/export/share/datastore/ - fingerprint/vgname/thinpool) — the durable_id sources. The API/root fence is untouched. -- The collector gained a `StorageObserver` seam (hub does **not** import storage); `Loop` - gained `SetTrigger` for the out-of-band report; the daemon runs the watchdog as a third - goroutine; `StorageConfig` exposes the watchdog knobs. -- **Hub** (`felhom.eu/hub`): `hostReportPayload` parses `storage_targets`, persists them via - `report_json`, counts/warns on disconnected targets, and carries its half of the - bidirectional golden key-set test. Both repos' goldens are byte-identical. +- **`HostOps` seam + `SudoHostOps`** — the one privileged surface: persistent mounts via + **systemd `.mount` units keyed by fs-UUID** (enabled, survive reboot), detach (stop+disable), + SMART, thin-pool metadata. Shells out via the fenced Runner (`sudo -n`, fixed arg vectors, + **no shell**). Tests use a fake — **no real root in the suite**. `NoopHostOps` is the + degrade-cleanly fallback. +- **The argument validator (the security boundary)** — `ValidateUUID` / `ValidateMountPath` + (absolute, no traversal, no metacharacters) / `ValidateSMARTDevice` (raw-disk whitelist) / + `ValidateLVMName`, plus an in-process `systemd-escape`. **Every argument is validated before a + command is constructed.** The headline test is an adversarial matrix (shell metacharacters, + `../`, malformed) that must be refused with **zero exec** — proven both at the validators and + at `SudoHostOps` (a recording runner asserts no command was built). +- **SMART** — `smartctl -a -j` parsed into `StorageTarget.smart` for **SATA and NVMe** attribute + sets, degrading to `UNKNOWN` on devices with no SMART. **`lvs`** fills the lvmthin thin-pool + **metadata** fill. Enrichment runs in `Observe` only (not the watchdog's fast `Known` path). +- **Watchdog re-mount response** — a new `DevicePresent` probe lets the watchdog detect a known + mount-backed target whose **device returned but is unmounted**, and **dispatch a benign + by-UUID re-mount off the poll path** (goroutine, never under the lock), rate-limited to the + debounce window. Routed through the gate as benign (`gateRemounter` in `main.go`, keeping + `storage` decoupled from `reconcile`). +- **Disk-grow executor** — `proxmox.ResizeLXC` (API, async) + a benign `ActionResize` planned + **grow-only**; a shrink is refused (never silently grown) at the planner and re-guarded at the + executor. Built + fixture-tested; unfed live. +- **Destructive storage ops through the slice-4 gate** — `IntentForStorageMount` (benign) / + `IntentForStorageDestructive` (`ClassStorageWipe`/`ClassDecommission`), host/target-scoped + (binds on the storage **target identity** in `target.guest_id`). Reuses the existing + verifier/role-scoping/binding/audit. Adversarial cases: unsigned → `pending_signature`, + wrong-target → `binding_mismatch`, valid → accepted. Inert live. +- **`--selftest=storage` [`-watch `]** — the live USB-runbook harness (observe pass + a + bounded watchdog window with re-mount live), standalone on the Proxmox host (no hub). +- **`configs/felhom-agent.sudoers`** — the documented narrow allowlist + the agent-side fine + validation. New `privileged.*` config knobs for the binary paths / dirs. ## Tests -`go test ./...` is green across both repos. New tests: observer build (incl. lvmthin -thin-pool fill, USB-unplugged→disconnected, Proxmox-error fatal, mount-read-failure -degrade), `durable_id` per-type table, watchdog transitions + debounce coalescing + -never-attached suppression + caching TTL + stale-on-error + HostLiveness mount/network, -collector seam (populate + error-degrade), loop out-of-band trigger, and the cross-repo -golden contract on both sides. +`go test ./...` green; **`go test -race ./...` green on the build server** (the watchdog +re-mount dispatch runs off the poll path concurrently). New tests: the **validator adversarial +matrix** (headline) + `SudoHostOps` "no-exec-on-hostile-arg"; HostOps mount/re-mount lifecycle +against a fake; SMART parsing SATA + NVMe + unsupported; thin-pool metadata parse; the grow +executor (grow applies, non-grow refused at the executor) + plan grow-only/shrink-refused; +the destructive-storage gate cases (unsigned / wrong-target / valid) reusing the slice-4 +real-verifier harness; and the watchdog → re-mount path (device-return triggers, rate-limited, +re-armed after a successful mount). -**`go test -race`** must be run on the build server (192.168.0.180, has cgo/gcc); the local -Windows toolchain lacks gcc. To run after pull: `CGO_ENABLED=1 go test -race ./...`. +## Design decisions (flagged for the reviewer) -## Out of scope (deferred) +1. **Unit-file write mechanism.** The agent stages the `.mount` unit to an agent-owned dir + (`privileged.stage_dir`, default `/var/lib/felhom-agent/units`) then `sudo install -o root + -g root -m 0644 -- /etc/systemd/system/`. This avoids a stdin-to-root channel + and keeps fixed arg vectors. The sudoers entry uses path-prefix wildcards; the agent's strict + validation (unit name derived from a validated mountpoint; no traversal) is the fine gate, so + the wildcard can't be abused. Swappable behind the seam if you prefer another mechanism. +2. **Storage-op gate scoping.** Host/target-scoped destructive ops carry the storage **target + name** (the operator handle / hub manifest key) in `target.guest_id`, with VMID 0. So "wipe A" + cannot authorize "wipe B" (binding_mismatch), exactly like the wrong-guest case. +3. **Shrink handling.** A desired disk < actual is refused by omission at the planner (no + action) and re-guarded at the executor (size must start with `+`); a deliberate shrink would + be a signed destructive op (slice 10), never a benign reconcile grow. -- **Phase B** (next, after this checkpoint validates): the host-root surface — systemd - `.mount` units + sudoers allowlist behind a `HostOps` seam, benign re-mount-by-UUID, - SMART (SATA + NVMe), the disk-grow executor (`pct resize`, grow-only), and routing - destructive storage ops (detach/wipe/resize-shrink) through the slice-4 gate. Built + - tested but inert live until slice 10 serves destructive deltas. -- Reconcile against the hub manifest (attach new targets, enforce class/role/policy, - `decommissioned`) → slice 10. Backup/restore orchestration → slice 6. +## Live validation (for project Claude / the runbook) -## Validation notes for the reviewer - -- The reachability heuristic for dir storages: a Felhom usb/local-dir target is realized as - its **own** mountpoint, so reachability = it is currently an exact mount + its device node - exists (we deliberately do not fall through to PVE's `active` flag, which reads stale- - attached because the mountpoint directory survives on the root fs after an unplug). Builtin - `local` and network/block targets use the `active` flag. Worth confirming against the demo - host's actual storage set during live validation. -- A live `--selftest=hub` against the demo host will print the populated `storage_targets`. +Not run from here (the observer reads the *local* host's `/proc/mounts` + privileged tools, so a +meaningful run must be **on the demo Proxmox host**, not the build server). The +`--selftest=storage -watch` harness is the intended live USB-cycle test: +`felhom-agent --selftest=storage -watch 3m` on `felhom-pve`, then physically unplug/replug a USB +target and watch detect → out-of-band report → by-UUID re-mount in the logs. SMART/lvs need the +sudoers drop-in installed (`configs/felhom-agent.sudoers`). diff --git a/cmd/felhom-agent/main.go b/cmd/felhom-agent/main.go index e3b0571..5c9b447 100644 --- a/cmd/felhom-agent/main.go +++ b/cmd/felhom-agent/main.go @@ -16,6 +16,7 @@ import ( "os" "os/signal" "path/filepath" + "strings" "syscall" "time" @@ -30,18 +31,20 @@ import ( // version is the agent version. Overridable at build time with // -ldflags "-X main.version="; defaults to the in-repo CHANGELOG version. -var version = "0.5.0-rc1" +var version = "0.5.0" func main() { var ( cfgPath string selftest selftestFlag vmid int + watch time.Duration showVersion bool ) flag.StringVar(&cfgPath, "config", envOr("FELHOM_AGENT_CONFIG", "/etc/felhom-agent/agent.json"), "path to the agent config file (JSON)") - flag.Var(&selftest, "selftest", "run a self-test and exit: bare/`read` = read-only queries; `task` = reversible mutating exercise (needs -vmid); `hub` = one collect+report to the hub") + flag.Var(&selftest, "selftest", "run a self-test and exit: bare/`read` = read-only queries; `task` = reversible mutating exercise (needs -vmid); `hub` = one collect+report to the hub; `storage` = observe storage targets (+ -watch for the live watchdog)") flag.IntVar(&vmid, "vmid", 0, "guest VMID for --selftest=task (the reversible snapshot/rollback exercise)") + flag.DurationVar(&watch, "watch", 0, "for --selftest=storage: run the watchdog verbose for this duration (e.g. 3m) with the re-mount response live; 0 = observe pass only") flag.BoolVar(&showVersion, "version", false, "print version and exit") flag.Parse() @@ -71,6 +74,8 @@ func main() { os.Exit(runSelftestTask(context.Background(), cfg, logger, vmid)) case "hub": os.Exit(runSelftestHub(context.Background(), cfg, logger)) + case "storage": + os.Exit(runSelftestStorage(context.Background(), cfg, logger, watch)) } } @@ -88,6 +93,62 @@ func newProxmoxClient(cfg config.Config) (*proxmox.Client, error) { }) } +// newHostOps builds the privileged storage surface (slice 5 Phase B) from config. It shells +// out via the same fenced Runner the proxmox layer uses (sudo -n, arg vectors, no shell); +// every argument is validated in internal/storage before any command is built. A +// missing/declined sudoers entry degrades per-op (SMART→UNKNOWN, mount→logged error), not a +// crash. +func newHostOps(cfg config.Config, logger *slog.Logger) storage.HostOps { + mode := proxmox.RunnerMode(cfg.Privileged.Mode) + if mode == "" { + mode = proxmox.RunnerSudo + } + runner := &proxmox.ExecRunner{Mode: mode, SudoPath: cfg.Privileged.SudoPath} + return storage.NewSudoHostOps(storage.SudoHostOpsConfig{ + Runner: runner, + Bins: storage.Binaries{ + Systemctl: cfg.Privileged.Systemctl, + Install: cfg.Privileged.Install, + Smartctl: cfg.Privileged.Smartctl, + Lvs: cfg.Privileged.Lvs, + }, + UnitDir: cfg.Privileged.UnitDir, + StageDir: cfg.Privileged.StageDir, + Logger: logger, + }) +} + +// gateRemounter is the watchdog's benign re-mount response: it routes the re-mount through +// the reversibility gate (classified benign) and, if allowed, calls HostOps.EnsureMount. It +// lives here (not in internal/storage) so storage stays decoupled from reconcile — main is +// the one place that holds both. +type gateRemounter struct { + gate *reconcile.Gate + ops storage.HostOps + hostID string + logger *slog.Logger +} + +// Remount authorizes (benign) and performs a by-UUID re-mount of a returned target. +func (r *gateRemounter) Remount(ctx context.Context, t storage.KnownTarget) { + dec := r.gate.Authorize(reconcile.IntentForStorageMount(r.hostID, t.Name), nil) + if !dec.Allowed { + r.logger.Warn("storage: re-mount refused by gate (unexpected for a benign mount)", + "target", t.Name, "reason", dec.Reason) + return + } + uuid := t.UUID + if uuid == "" { + uuid = strings.TrimPrefix(t.DurableID, "uuid:") // durable_id carries it for usb/local-dir + } + spec := storage.MountSpec{Name: t.Name, UUID: uuid, Where: t.MountPath} + if err := r.ops.EnsureMount(ctx, spec); err != nil { + r.logger.Error("storage: re-mount failed", "target", t.Name, "where", t.MountPath, "err", err) + return + } + r.logger.Info("storage: re-mounted returned target", "target", t.Name, "where", t.MountPath) +} + // runDaemon is the default mode: collect a host-report and POST it to the hub on a // loop. Requires both proxmox (to collect) and hub config. func runDaemon(cfg config.Config, logger *slog.Logger) int { @@ -110,34 +171,17 @@ func runDaemon(cfg config.Config, logger *slog.Logger) int { return 1 } hcfg := cfg.Hub.WithDefaults() - // Storage observer (slice 5): read-only, builds the report's storage_targets from - // Proxmox + non-privileged host reads. Wired into the collector via the StorageObserver - // seam (so hub does not import storage). + // Storage observer (slice 5): builds the report's storage_targets from Proxmox + + // non-privileged host reads, enriched with the privileged SMART/lvs reads via HostOps + // (Phase B). Wired into the collector via the StorageObserver seam (so hub does not + // import storage). hostReader := storage.NewProcHostReader() - observer := storage.NewObserver(px, hostReader, logger) + hostOps := newHostOps(cfg, logger) + observer := storage.NewObserver(px, hostReader, hostOps, logger) collector := hub.NewCollector(px, hub.SystemctlProber{}, observer, cfg.Hub.HostID, version, logger) loop := hub.NewLoop(collector, client, time.Duration(hcfg.PollSeconds)*time.Second, logger) interval := time.Duration(hcfg.PollSeconds) * time.Second - // Storage watchdog (slice 5): a third daemon goroutine fast-polling the known target - // set for attached↔disconnected transitions and triggering an immediate, debounced - // out-of-band report. With no removable/network storage it simply finds nothing to flag. - storageTrigger := make(chan struct{}, 1) - loop.SetTrigger(storageTrigger) - watchdog := storage.NewWatchdog(storage.WatchdogOptions{ - Targets: storage.NewCachingKnownTargets(observer, cfg.Storage.KnownRefresh()), - Liveness: storage.NewHostLiveness(hostReader, 0), - Trigger: func() { - select { - case storageTrigger <- struct{}{}: - default: - } - }, - Interval: cfg.Storage.WatchdogInterval(), - Debounce: cfg.Storage.WatchdogDebounce(), - Logger: logger, - }) - ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) defer stop() logger.Info("felhom-agent daemon starting", @@ -178,6 +222,29 @@ func runDaemon(cfg config.Config, logger *slog.Logger) int { } gate := reconcile.NewGate(verifier, cfg.Hub.HostID, reconcile.SlogAudit{Logger: logger}, logger) + // Storage watchdog (slice 5): the third daemon goroutine. Fast-polls the known target + // set for attached↔disconnected transitions → debounced out-of-band report; and, on a + // known mount-backed target's device returning unmounted, dispatches a benign re-mount + // (routed through the gate as benign, then HostOps). With no removable/network storage + // it finds nothing to flag. The re-mount dispatch is off the poll path (a goroutine). + storageTrigger := make(chan struct{}, 1) + loop.SetTrigger(storageTrigger) + remounter := &gateRemounter{gate: gate, ops: hostOps, hostID: cfg.Hub.HostID, logger: logger} + watchdog := storage.NewWatchdog(storage.WatchdogOptions{ + Targets: storage.NewCachingKnownTargets(observer, cfg.Storage.KnownRefresh()), + Liveness: storage.NewHostLiveness(hostReader, 0), + Remounter: remounter, + Trigger: func() { + select { + case storageTrigger <- struct{}{}: + default: + } + }, + Interval: cfg.Storage.WatchdogInterval(), + Debounce: cfg.Storage.WatchdogDebounce(), + Logger: logger, + }) + engine := reconcile.NewEngine(reconcile.EngineOptions{ API: px, Queue: queue, @@ -270,7 +337,7 @@ func runSelftestHub(ctx context.Context, cfg config.Config, logger *slog.Logger) fmt.Fprintln(os.Stderr, "selftest: hub client:", err) return 1 } - observer := storage.NewObserver(px, storage.NewProcHostReader(), logger) + observer := storage.NewObserver(px, storage.NewProcHostReader(), newHostOps(cfg, logger), logger) collector := hub.NewCollector(px, hub.SystemctlProber{}, observer, cfg.Hub.HostID, version, logger) ctx, cancel := context.WithTimeout(ctx, 60*time.Second) @@ -299,6 +366,100 @@ func runSelftestHub(ctx context.Context, cfg config.Config, logger *slog.Logger) return 0 } +// runSelftestStorage is the live storage harness (slice 5 Phase B, for the USB runbook). +// It needs Proxmox config only (NO hub) so it runs standalone on the Proxmox host: +// - observe pass: print the full StorageTarget table incl. the privileged SMART summary +// and thin-pool data+metadata fill. +// - -watch D: run the watchdog verbose for D with the re-mount response LIVE, so the +// operator can physically cycle a drive and watch detect → report → re-mount. +func runSelftestStorage(ctx context.Context, cfg config.Config, logger *slog.Logger, watch time.Duration) int { + if err := cfg.Validate(); err != nil { + fmt.Fprintln(os.Stderr, "selftest: proxmox not configured:", err) + return 1 + } + px, err := newProxmoxClient(cfg) + if err != nil { + fmt.Fprintln(os.Stderr, "selftest: proxmox client:", err) + return 1 + } + hostReader := storage.NewProcHostReader() + hostOps := newHostOps(cfg, logger) + observer := storage.NewObserver(px, hostReader, hostOps, logger) + + octx, cancel := context.WithTimeout(ctx, 60*time.Second) + fmt.Printf("=== felhom-agent %s selftest=storage ===\n", version) + targets, err := observer.Observe(octx) + cancel() + if err != nil { + fmt.Fprintln(os.Stderr, " [FAIL] observe:", err) + return 1 + } + fmt.Printf(" observed %d storage target(s):\n", len(targets)) + for _, t := range targets { + fmt.Printf(" - %-12s type=%-9s state=%-12s reach=%-5v class=%-4s durable=%s\n", + t.Name, t.Type, t.State, t.Reachable, t.ClassHint, t.DurableID) + fmt.Printf(" usage %s/%s (%.0f%%) mount=%q dev=%q\n", + gib(t.UsedBytes), gib(t.TotalBytes), t.UsedFraction*100, t.MountPath, t.BackingDevice) + fmt.Printf(" smart: health=%s%s\n", t.Smart.Health, smartCounters(t.Smart)) + if t.ThinPool != nil { + meta := "n/a" + if t.ThinPool.MetadataUsedFraction != nil { + meta = fmt.Sprintf("%.1f%%", *t.ThinPool.MetadataUsedFraction*100) + } + fmt.Printf(" thin-pool: data=%.1f%% metadata=%s\n", t.ThinPool.DataUsedFraction*100, meta) + } + } + + if watch <= 0 { + fmt.Println("=== selftest=storage OK (observe pass; pass -watch D for the live watchdog) ===") + return 0 + } + + // Live watchdog window: re-mount response active. Cycle a drive and watch the logs. + fmt.Printf(" --- watching for %s (cycle a drive now; detect → report → re-mount) ---\n", watch) + wctx, stop := signal.NotifyContext(ctx, os.Interrupt, syscall.SIGTERM) + defer stop() + wctx, cancel2 := context.WithTimeout(wctx, watch) + defer cancel2() + + gate := reconcile.NewGate(nil, cfg.Hub.HostID, reconcile.SlogAudit{Logger: logger}, logger) + remounter := &gateRemounter{gate: gate, ops: hostOps, hostID: cfg.Hub.HostID, logger: logger} + wd := storage.NewWatchdog(storage.WatchdogOptions{ + Targets: storage.NewCachingKnownTargets(observer, cfg.Storage.KnownRefresh()), + Liveness: storage.NewHostLiveness(hostReader, 0), + Remounter: remounter, + Trigger: func() { logger.Info("storage: (selftest) would send out-of-band host-report now") }, + Interval: cfg.Storage.WatchdogInterval(), + Debounce: cfg.Storage.WatchdogDebounce(), + Logger: logger, + }) + _ = wd.Run(wctx) + fmt.Println("=== selftest=storage watch window ended ===") + return 0 +} + +// smartCounters renders the non-nil SMART counters compactly for the selftest table. +func smartCounters(s hub.SmartSummary) string { + var parts []string + add := func(name string, v *int) { + if v != nil { + parts = append(parts, fmt.Sprintf("%s=%d", name, *v)) + } + } + add("temp", s.TemperatureC) + add("poh", s.PowerOnHours) + add("realloc", s.ReallocatedSectors) + add("pending", s.PendingSectors) + add("offline_unc", s.OfflineUncorrectable) + add("crit_warn", s.CriticalWarning) + add("media_err", s.MediaErrors) + add("pct_used", s.PercentageUsed) + if len(parts) == 0 { + return "" + } + return " " + strings.Join(parts, " ") +} + // runSelftestRead loads config, builds the API client, and runs the read-only // queries against the live host, printing a short health report. It mutates // nothing. Missing/invalid config is reported cleanly (no panic). @@ -585,8 +746,10 @@ func (f *selftestFlag) Set(v string) error { f.mode = "task" case "hub": f.mode = "hub" + case "storage": + f.mode = "storage" default: - return fmt.Errorf("invalid --selftest value %q (want read|task|hub)", v) + return fmt.Errorf("invalid --selftest value %q (want read|task|hub|storage)", v) } return nil } diff --git a/configs/felhom-agent.sudoers b/configs/felhom-agent.sudoers new file mode 100644 index 0000000..e329f13 --- /dev/null +++ b/configs/felhom-agent.sudoers @@ -0,0 +1,29 @@ +# felhom-agent sudoers allowlist — the NARROW host-root surface (slice 5 Phase B, doc 03 §3/§7). +# +# Install as a drop-in: /etc/sudoers.d/felhom-agent (mode 0440, root:root), validated with +# `visudo -cf`. The agent runs as the non-root `felhom-agent` service user and shells out via +# `sudo -n` with FIXED argument vectors (no shell). The fine-grained validation is done IN +# the agent BEFORE exec (internal/storage/validate.go): UUIDs against a strict hex regex, +# mount paths confined+traversal-checked, SMART devices whitelisted to raw disks, LVM names +# charset-checked. These sudoers wildcards are the COARSE allowlist; the agent is the fine +# gate, so a wildcard can never be abused by a value the agent didn't already validate. +# +# Binary paths MUST match the agent config (privileged.systemctl/install/smartctl/lvs). Adjust +# for your distro (Debian/PVE shown). A missing/declined entry degrades the agent with a +# warning (SMART→UNKNOWN, mount→logged error), it does not crash. + +Cmnd_Alias FELHOM_MOUNT = \ + /usr/bin/install -o root -g root -m 0644 -- /var/lib/felhom-agent/units/* /etc/systemd/system/*.mount, \ + /usr/bin/systemctl daemon-reload, \ + /usr/bin/systemctl enable --now -- *.mount, \ + /usr/bin/systemctl disable -- *.mount, \ + /usr/bin/systemctl stop -- *.mount + +Cmnd_Alias FELHOM_DISK = \ + /usr/sbin/smartctl -a -j /dev/sd[a-z]*, \ + /usr/sbin/smartctl -a -j /dev/nvme[0-9]*n[0-9]*, \ + /usr/sbin/smartctl -a -j /dev/vd[a-z]*, \ + /usr/sbin/smartctl -a -j /dev/hd[a-z]*, \ + /usr/sbin/lvs --reportformat json --units b -o lv_name,data_percent,metadata_percent -- * + +felhom-agent ALL=(root) NOPASSWD: FELHOM_MOUNT, FELHOM_DISK diff --git a/internal/config/config.go b/internal/config/config.go index 8a88dbb..5b9bf2c 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -116,12 +116,22 @@ type TLSTrust struct { InsecureSkipVerify bool `json:"insecure_skip_verify"` // off by default; selftest-only } -// PrivilegedConfig configures the fenced root-CLI runner. +// PrivilegedConfig configures the fenced root-CLI runner and the slice-5 HostOps surface +// (systemd mount units + smartctl + lvs). The binary paths must match the sudoers allowlist +// exactly (see configs/felhom-agent.sudoers). type PrivilegedConfig struct { // Mode: "sudo" (default — non-root agent + narrow sudoers) or "direct". Mode string `json:"mode"` // SudoPath overrides the sudo binary (default "sudo"). SudoPath string `json:"sudo_path"` + + // HostOps (slice 5 Phase B) — the privileged storage write/read surface. + UnitDir string `json:"unit_dir"` // where enabled .mount units live (default /etc/systemd/system) + StageDir string `json:"stage_dir"` // agent-owned staging dir for unit files (default /var/lib/felhom-agent/units) + Systemctl string `json:"systemctl"` // default /usr/bin/systemctl + Install string `json:"install"` // default /usr/bin/install + Smartctl string `json:"smartctl"` // default /usr/sbin/smartctl + Lvs string `json:"lvs"` // default /usr/sbin/lvs } // Default returns a Config pre-populated with sane defaults. diff --git a/internal/proxmox/mutate.go b/internal/proxmox/mutate.go index c40c441..d0ca8c1 100644 --- a/internal/proxmox/mutate.go +++ b/internal/proxmox/mutate.go @@ -135,6 +135,25 @@ func (c *Client) SetConfig(ctx context.Context, vmid int, params map[string]stri return c.dataString(ctx, http.MethodPut, path, v) } +// ResizeLXC grows a guest volume via PUT /nodes/{node}/lxc/{vmid}/resize +// (token-covered: VM.Config.Disk + Datastore.AllocateSpace). Returns the UPID. +// +// disk is the volume key (e.g. "rootfs", "mp0"); size is a Proxmox size string. A +// LEADING '+' means GROW BY that amount (e.g. "+5G"); an absolute value can only ever +// grow (Proxmox rejects a shrink for a mounted/most volumes, but the agent must NOT rely +// on that — the reconcile layer is responsible for refusing a shrink before it reaches +// here, since a data-losing shrink is a destructive op, not a benign resize). +func (c *Client) ResizeLXC(ctx context.Context, vmid int, disk, size string) (string, error) { + if vmid == 0 || disk == "" || size == "" { + return "", fmt.Errorf("proxmox: ResizeLXC needs vmid, disk and size") + } + v := url.Values{} + v.Set("disk", disk) + v.Set("size", size) + path := fmt.Sprintf("/nodes/%s/lxc/%d/resize", c.node, vmid) + return c.dataString(ctx, http.MethodPut, path, v) +} + // Start starts a guest via POST /nodes/{node}/lxc/{vmid}/status/start (VM.PowerMgmt). func (c *Client) Start(ctx context.Context, vmid int) (string, error) { path := fmt.Sprintf("/nodes/%s/lxc/%d/status/start", c.node, vmid) diff --git a/internal/reconcile/classify.go b/internal/reconcile/classify.go index 8ff9f36..f639d30 100644 --- a/internal/reconcile/classify.go +++ b/internal/reconcile/classify.go @@ -17,6 +17,11 @@ const ( ClassStart OpClass = "start" ClassStop OpClass = "stop" ClassSetConfig OpClass = "set_config" // benign sizing/description changes only + ClassResize OpClass = "resize" // GROW-only rootfs/volume resize (slice 5 Phase B) + + // Benign storage attach — re-mount-by-UUID of a known target whose device returned + // (slice 5 Phase B). Additive (no data loss), so benign by construction. + ClassStorageMount OpClass = "storage_mount" // Benign by construction — classified now, executors land in later slices. ClassCreate OpClass = "create" // provision a NEW guest (restore-to-new, slice 7) @@ -76,7 +81,7 @@ func (p Provenance) internalEvidence() bool { // - an UNKNOWN class fails safe → Destructive (require a signature). func Classify(class OpClass, prov Provenance) Disposition { switch class { - case ClassStart, ClassStop, ClassSetConfig, ClassCreate, ClassRestart: + case ClassStart, ClassStop, ClassSetConfig, ClassResize, ClassStorageMount, ClassCreate, ClassRestart: return Benign case ClassGuestDestroy, ClassStorageWipe, ClassRestoreOverwrite, ClassDecommission: if prov.internalEvidence() { @@ -100,6 +105,8 @@ func classOfAction(k ActionKind) OpClass { return ClassStop case ActionSetConfig: return ClassSetConfig + case ActionResize: + return ClassResize default: return OpClass(k) } diff --git a/internal/reconcile/engine.go b/internal/reconcile/engine.go index b2e09c9..ed95436 100644 --- a/internal/reconcile/engine.go +++ b/internal/reconcile/engine.go @@ -5,6 +5,7 @@ import ( "fmt" "log/slog" "strconv" + "strings" "sync/atomic" "time" @@ -164,6 +165,15 @@ func (e *Engine) execute(ctx context.Context, act Action) error { upid, err = e.api.Stop(ctx, act.VMID) case ActionSetConfig: upid, err = e.api.SetConfig(ctx, act.VMID, act.Params) + case ActionResize: + // Defensive grow-only guard at the executor: a resize size MUST be a "+" grow. + // The planner only ever emits grows, but never let a shrink reach Proxmox here. + disk, size := act.Params["disk"], act.Params["size"] + if !strings.HasPrefix(size, "+") { + err = fmt.Errorf("reconcile: refusing non-grow resize size %q (data-losing shrink is a signed op)", size) + } else { + upid, err = e.api.ResizeLXC(ctx, act.VMID, disk, size) + } default: err = fmt.Errorf("reconcile: unknown action kind %q", act.Kind) } @@ -208,7 +218,9 @@ func (e *Engine) readActual(ctx context.Context) (ActualState, error) { } guests := make(map[int]ActualGuest, len(lxc)) for _, g := range lxc { - a := ActualGuest{VMID: g.VMID, Run: normRun(g.Status)} + // MaxDisk (bytes) comes from the list entry and is reliable independent of the + // per-guest config read — it is the actual side of the grow comparison. + a := ActualGuest{VMID: g.VMID, Run: normRun(g.Status), DiskBytes: g.MaxDisk} cfg, err := e.api.GuestConfig(ctx, g.VMID) if err != nil { e.logger.Warn("reconcile: GuestConfig failed; spec unknown (run-state kept)", diff --git a/internal/reconcile/engine_test.go b/internal/reconcile/engine_test.go index f028ffa..b3250dd 100644 --- a/internal/reconcile/engine_test.go +++ b/internal/reconcile/engine_test.go @@ -18,8 +18,8 @@ type fakeAPI struct { lxc []proxmox.Guest cfg map[int]proxmox.GuestConfig - startUPID, stopUPID, setUPID string - startErr, stopErr, setErr error + startUPID, stopUPID, setUPID, resizeUPID string + startErr, stopErr, setErr, resizeErr error // waitFunc maps a UPID to a (status, err); default = OK. Mirrors the real client, // which errors on a non-OK exitstatus. waitFunc func(upid string) (proxmox.TaskStatus, error) @@ -29,10 +29,16 @@ type fakeAPI struct { starts []int stops []int sets []setCall + resizes []resizeCall waits []string listErr error } +type resizeCall struct { + vmid int + disk, size string +} + func (f *fakeAPI) TaskStatusOnce(_ context.Context, upid string) (proxmox.TaskStatus, error) { if f.statusFunc != nil { return f.statusFunc(upid) @@ -81,6 +87,13 @@ func (f *fakeAPI) SetConfig(_ context.Context, vmid int, params map[string]strin return f.setUPID, f.setErr } +func (f *fakeAPI) ResizeLXC(_ context.Context, vmid int, disk, size string) (string, error) { + f.mu.Lock() + f.resizes = append(f.resizes, resizeCall{vmid, disk, size}) + f.mu.Unlock() + return f.resizeUPID, f.resizeErr +} + func (f *fakeAPI) WaitTask(_ context.Context, upid string, _ proxmox.WaitOptions) (proxmox.TaskStatus, error) { f.mu.Lock() f.waits = append(f.waits, upid) diff --git a/internal/reconcile/plan.go b/internal/reconcile/plan.go index aaa151a..2bce053 100644 --- a/internal/reconcile/plan.go +++ b/internal/reconcile/plan.go @@ -20,8 +20,18 @@ const ( // ActionSetConfig applies benign config changes (cores/memory/description) in one // PUT (proxmox VM.Config.*). May return synchronously (empty UPID) — slice-4 proven. ActionSetConfig ActionKind = "set_config" + // ActionResize GROWS the rootfs (proxmox `pct resize`, async). Grow-only — the planner + // emits it only when desired DiskBytes > actual; a shrink is data-losing and is refused + // (never silently applied as a grow). Slice 5 Phase B; unfed live until slice 10. + ActionResize ActionKind = "resize" ) +// growRoundMiB rounds a positive byte delta UP to whole MiB for the Proxmox `+M` grow +// size (Proxmox resizes in whole units; rounding up never under-provisions the desired size). +func growRoundMiB(deltaBytes int64) int64 { + return (deltaBytes + bytesPerMiB - 1) / bytesPerMiB +} + // Action is one minimal mutation the engine will dispatch onto the per-guest queue. // In Phase A every Action is benign by construction (only the benign kinds exist). // Phase B's classifier/gate sits in front of the executor and may tag an action @@ -99,8 +109,27 @@ func Plan(desired DesiredState, actual ActualState, norm FieldNormalizers) []Act params["memory"] = strconv.FormatInt(want, 10) reasons = append(reasons, fmt.Sprintf("memory %dMiB->%dMiB", a.MemoryMiB, want)) } - // DiskBytes is intentionally NOT reconciled here (rootfs grow is - // `pct resize`, grow-only and separate — a later slice). + } + // Rootfs GROW (slice 5 Phase B) — a separate async op from the config PUT, so + // its own Action. GROW-ONLY: emit a resize only when desired > actual. A shrink + // (desired < actual) is data-losing and is REFUSED here — we never silently + // clamp it to a grow; it is simply not planned (a deliberate shrink would have + // to come as a signed destructive op, slice 10). DiskBytes==0 means unmanaged. + if d.Spec != nil && d.Spec.DiskBytes > 0 && a.DiskBytes > 0 { + switch { + case d.Spec.DiskBytes > a.DiskBytes: + deltaMiB := growRoundMiB(d.Spec.DiskBytes - a.DiskBytes) + actions = append(actions, Action{ + VMID: vmid, + Kind: ActionResize, + Params: map[string]string{"disk": "rootfs", "size": fmt.Sprintf("+%dM", deltaMiB)}, + Reason: fmt.Sprintf("disk grow %dB->%dB (+%dMiB)", a.DiskBytes, d.Spec.DiskBytes, deltaMiB), + }) + case d.Spec.DiskBytes < a.DiskBytes: + // Shrink refused by omission: emit NO action (a data-losing shrink is a + // signed destructive op, slice 10 — never a benign reconcile grow). The + // executor also guards (size must start with '+'). See the resize note above. + } } if d.Description != nil && !norm.Equal("description", *d.Description, a.Description) { params["description"] = *d.Description diff --git a/internal/reconcile/state.go b/internal/reconcile/state.go index a0b0f22..3f9a128 100644 --- a/internal/reconcile/state.go +++ b/internal/reconcile/state.go @@ -69,6 +69,7 @@ type ActualGuest struct { SpecKnown bool Cores int MemoryMiB int64 // proxmox LXC `memory` is MiB + DiskBytes int64 // rootfs size in bytes (from the LXC list MaxDisk; for grow planning) Description string // raw (may carry PVE's trailing newline; compared via normalizers) } @@ -111,6 +112,8 @@ type GuestAPI interface { Start(ctx context.Context, vmid int) (string, error) Stop(ctx context.Context, vmid int) (string, error) SetConfig(ctx context.Context, vmid int, params map[string]string) (string, error) + // ResizeLXC grows a volume (grow-only; the planner never emits a shrink). Async → UPID. + ResizeLXC(ctx context.Context, vmid int, disk, size string) (string, error) WaitTask(ctx context.Context, upid string, opts proxmox.WaitOptions) (proxmox.TaskStatus, error) // TaskStatusOnce is a single non-blocking task-status read — used by crash // recovery to learn the outcome of an op that was in flight when the agent died. diff --git a/internal/reconcile/storage_ops.go b/internal/reconcile/storage_ops.go new file mode 100644 index 0000000..ad36ef9 --- /dev/null +++ b/internal/reconcile/storage_ops.go @@ -0,0 +1,49 @@ +package reconcile + +import "encoding/json" + +// Storage operations (slice 5 Phase B) flow through the SAME reversibility gate as guest +// ops — no new gate, no new crypto. They are HOST/TARGET-scoped (no guest), so the op binds +// on the STORAGE TARGET IDENTITY rather than a vmid. +// +// Scoping decision (documented): the scoped resource id is carried in the op's +// target.guest_id (and the Intent.GuestID) as the storage target's NAME — the operator- +// facing handle and the hub manifest key. VMID is 0 (host-scoped; no queue routing by +// guest). So a signature for "wipe target A" (guest_id="A") cannot authorize "wipe target +// B" (guest_id="B") — the gate's op-to-action binding rejects it (binding_mismatch), +// exactly as it does for the wrong guest on a guest op. +// +// Benign storage ops (re-mount, slice 5) use IntentForStorageMount and pass the gate +// unsigned. Destructive storage ops (detach/wipe/decommission, inert until slice 10) use +// IntentForStorageDestructive and require a verified, role-scoped, target-bound operator +// signature — else pending_signature. + +// IntentForStorageMount builds the benign re-mount intent for a known target (additive, no +// data loss → benign by classification). targetID is the storage target name. +func IntentForStorageMount(hostID, targetID string) Intent { + return Intent{ + Class: ClassStorageMount, + HostID: hostID, + GuestID: targetID, // storage target identity (host-scoped op) + VMID: 0, + Provenance: Provenance{}, // never hub-sourced + Source: SourceDesiredDelta, + } +} + +// IntentForStorageDestructive builds a destructive storage intent (detach/wipe via +// ClassStorageWipe, or ClassDecommission). It carries the target identity in GuestID and the +// canonical params for op-to-action binding. Provenance is the zero value — a destructive +// storage op is NOT made benign by hub-supplied evidence (only agent-internal provenance +// could, and storage detach/wipe carries none here). +func IntentForStorageDestructive(class OpClass, hostID, targetID string, params json.RawMessage, source SourceKind) Intent { + return Intent{ + Class: class, + HostID: hostID, + GuestID: targetID, + VMID: 0, + ParamsJSON: params, + Provenance: Provenance{}, + Source: source, + } +} diff --git a/internal/reconcile/storage_test.go b/internal/reconcile/storage_test.go new file mode 100644 index 0000000..4d68fe6 --- /dev/null +++ b/internal/reconcile/storage_test.go @@ -0,0 +1,170 @@ +package reconcile + +import ( + "context" + "encoding/json" + "path/filepath" + "testing" + + "gitea.dooplex.hu/admin/felhom-agent/internal/authz" + "gitea.dooplex.hu/admin/felhom-agent/internal/hub" +) + +// --- The disk-grow executor (deferred from slice 4): grow applies, shrink refused. --- + +func TestPlan_DiskGrowOnly(t *testing.T) { + norm := DefaultNormalizers() + + t.Run("grow emits a resize action", func(t *testing.T) { + desired := DesiredState{Guests: map[int]DesiredGuest{ + 100: {VMID: 100, Spec: &hub.GuestSpec{DiskBytes: 20 << 30}}, // want 20 GiB + }} + actual := ActualState{Guests: map[int]ActualGuest{ + 100: {VMID: 100, SpecKnown: true, DiskBytes: 8 << 30}, // have 8 GiB + }} + var resize *Action + for _, a := range Plan(desired, actual, norm) { + if a.Kind == ActionResize { + a := a + resize = &a + } + } + if resize == nil { + t.Fatal("expected a resize action for a grow") + } + if resize.Params["disk"] != "rootfs" || resize.Params["size"] != "+12288M" { + t.Errorf("resize params = %v, want disk=rootfs size=+12288M", resize.Params) + } + }) + + t.Run("shrink is refused (no action)", func(t *testing.T) { + desired := DesiredState{Guests: map[int]DesiredGuest{100: {VMID: 100, Spec: &hub.GuestSpec{DiskBytes: 4 << 30}}}} + actual := ActualState{Guests: map[int]ActualGuest{100: {VMID: 100, SpecKnown: true, DiskBytes: 8 << 30}}} + for _, a := range Plan(desired, actual, norm) { + if a.Kind == ActionResize { + t.Fatalf("a data-losing shrink must NOT be planned as a resize: %+v", a) + } + } + }) + + t.Run("equal size is a no-op", func(t *testing.T) { + desired := DesiredState{Guests: map[int]DesiredGuest{100: {VMID: 100, Spec: &hub.GuestSpec{DiskBytes: 8 << 30}}}} + actual := ActualState{Guests: map[int]ActualGuest{100: {VMID: 100, SpecKnown: true, DiskBytes: 8 << 30}}} + for _, a := range Plan(desired, actual, norm) { + if a.Kind == ActionResize { + t.Fatalf("equal disk size must not resize: %+v", a) + } + } + }) +} + +func TestEngine_GrowExecutes_NonGrowRefusedAtExecutor(t *testing.T) { + a := &fakeAPI{resizeUPID: "UPID:resize:1"} + e, _, q := newEngine(t, a, EmptyProvider{}) + defer q.Close() + + // A grow applies (ResizeLXC called with the grow size). + if err := e.execute(context.Background(), Action{VMID: 100, Kind: ActionResize, + Params: map[string]string{"disk": "rootfs", "size": "+12288M"}}); err != nil { + t.Fatalf("grow execute: %v", err) + } + if len(a.resizes) != 1 || a.resizes[0].size != "+12288M" || a.resizes[0].disk != "rootfs" { + t.Fatalf("ResizeLXC not called correctly: %+v", a.resizes) + } + + // A non-grow ("absolute"/shrink) size is refused at the executor and never hits the API. + a.resizes = nil + if err := e.execute(context.Background(), Action{VMID: 100, Kind: ActionResize, + Params: map[string]string{"disk": "rootfs", "size": "4G"}}); err == nil { + t.Fatal("a non-grow resize size must be refused at the executor") + } + if len(a.resizes) != 0 { + t.Fatalf("refused resize must not call the API: %+v", a.resizes) + } +} + +// --- Destructive storage ops through the slice-4 gate (real verifier). --- + +func wipeIntent(targetID string) Intent { + return IntentForStorageDestructive(ClassStorageWipe, testHost, targetID, + json.RawMessage(`{"wipe":true}`), SourceOneShotJob) +} + +func TestGate_StorageWipeUnsignedPendingSignature(t *testing.T) { + op := newTestSigner(t) + v, _ := realVerifierAt(t, filepath.Join(t.TempDir(), "n.log"), testHost, op.allowed(t, "op1", authz.RoleOperational)) + aud := &captureAudit{} + g := NewGate(v, testHost, aud, nil) + + d := g.Authorize(wipeIntent("usb-backup"), nil) + if d.Allowed || d.Reason != ReasonPendingSignature { + t.Fatalf("unsigned storage wipe: got allowed=%v reason=%s, want pending_signature", d.Allowed, d.Reason) + } + if len(aud.recs) != 1 || aud.recs[0].Allowed { + t.Errorf("refused wipe must be audited: %+v", aud.recs) + } +} + +func TestGate_StorageWipeWrongTargetBindingMismatch(t *testing.T) { + // A valid signature authorizing "wipe target A" must NOT authorize "wipe target B" — + // the op-to-action binding rejects it (the storage analog of the wrong-guest case). + op := newTestSigner(t) + v, _ := realVerifierAt(t, filepath.Join(t.TempDir(), "n.log"), testHost, op.allowed(t, "op1", authz.RoleOperational)) + g := NewGate(v, testHost, nil, nil) + + issued, expires := freshWindow() + signed := op.mint("storage_wipe", testHost, "usb-backup", "op1", nonce(), `{"wipe":true}`, issued, expires) + d := g.Authorize(wipeIntent("nfs-arch"), signed) // action targets a DIFFERENT store + if d.Allowed || d.Reason != ReasonBindingMismatch { + t.Fatalf("wrong-target wipe: got allowed=%v reason=%s, want binding_mismatch", d.Allowed, d.Reason) + } +} + +func TestGate_StorageWipeValidAccepted(t *testing.T) { + op := newTestSigner(t) + v, _ := realVerifierAt(t, filepath.Join(t.TempDir(), "n.log"), testHost, op.allowed(t, "op1", authz.RoleOperational)) + g := NewGate(v, testHost, nil, nil) + + issued, expires := freshWindow() + signed := op.mint("storage_wipe", testHost, "usb-backup", "op1", nonce(), `{"wipe":true}`, issued, expires) + d := g.Authorize(wipeIntent("usb-backup"), signed) + if !d.Allowed || d.Reason != ReasonSigned { + t.Fatalf("valid storage wipe: got allowed=%v reason=%s err=%v, want accepted/signed", d.Allowed, d.Reason, d.Err) + } +} + +func TestGate_StorageMountBenign(t *testing.T) { + // A re-mount is benign by classification → allowed unsigned (no verifier needed). + g := NewGate(nil, testHost, nil, nil) + d := g.Authorize(IntentForStorageMount(testHost, "usb-backup"), nil) + if !d.Allowed || d.Reason != ReasonBenign { + t.Fatalf("storage mount: got allowed=%v reason=%s, want benign", d.Allowed, d.Reason) + } +} + +// TestRunSignedJob_StorageDestructiveExecutes proves the authorized destructive-storage path +// reaches its executor (inert live — wired in main.go later; here a fake exec records it). +func TestRunSignedJob_StorageDestructiveExecutes(t *testing.T) { + op := newTestSigner(t) + v, _ := realVerifierAt(t, filepath.Join(t.TempDir(), "n.log"), testHost, op.allowed(t, "op1", authz.RoleOperational)) + a := &fakeAPI{} + q := NewQueue() + t.Cleanup(q.Close) + e := NewEngine(EngineOptions{API: a, Queue: q, Gate: NewGate(v, testHost, nil, nil), HostID: testHost}) + + issued, expires := freshWindow() + signed := op.mint("storage_wipe", testHost, "usb-backup", "op1", nonce(), `{"wipe":true}`, issued, expires) + + var ran bool + exec := func(_ context.Context, intent Intent, _ *authz.VerifiedOp) (string, error) { + ran = true + if intent.GuestID != "usb-backup" { + t.Errorf("executor got wrong target %q", intent.GuestID) + } + return "", nil + } + res := e.RunSignedJob(context.Background(), wipeIntent("usb-backup"), signed, exec) + if !res.Executed || !ran || res.Err != nil { + t.Fatalf("authorized storage wipe should execute cleanly: %+v ran=%v", res, ran) + } +} diff --git a/internal/storage/hostops.go b/internal/storage/hostops.go new file mode 100644 index 0000000..7c2c24c --- /dev/null +++ b/internal/storage/hostops.go @@ -0,0 +1,259 @@ +package storage + +import ( + "context" + "fmt" + "log/slog" + "os" + "path/filepath" + "strings" + + "gitea.dooplex.hu/admin/felhom-agent/internal/hub" + "gitea.dooplex.hu/admin/felhom-agent/internal/proxmox" +) + +// HostOps is the narrow privileged host surface (slice 5 Phase B) — the ONE place the +// agent steps outside its Proxmox API token into OS-root. Production is *SudoHostOps +// (shells out via a sudoers allowlist, arg vectors, no shell); tests use a fake, so NO +// real root runs in the test suite. +// +// Every method validates its arguments (validate.go) before constructing a command. The +// surface is deliberately tiny: persistent mounts (systemd .mount units keyed by fs-UUID), +// detach (stop+disable the unit), SMART, and thin-pool metadata. +type HostOps interface { + // EnsureMount writes + enables a systemd .mount unit for spec (idempotent: re-applying + // an existing target is a no-op). Benign — additive, no signature. + EnsureMount(ctx context.Context, spec MountSpec) error + // Unmount stops + disables the .mount unit for a mountpoint (detach from service). This + // is DESTRUCTIVE (deliberately removing a target) — the caller MUST have routed it + // through the gate first; HostOps only performs an already-authorized op. + Unmount(ctx context.Context, where string) error + // SMART returns a parsed health summary for a raw block device, degrading to + // {Health: UNKNOWN} when the device exposes no SMART (e.g. a USB-SATA bridge). + SMART(ctx context.Context, device string) (hub.SmartSummary, error) + // ThinPoolMetadata returns the lvmthin pool's metadata-used fraction (0..1) via lvs. + // ok=false when it cannot be read (the field stays null in the report). + ThinPoolMetadata(ctx context.Context, vg, pool string) (fraction float64, ok bool) +} + +// MountSpec describes a persistent by-UUID mount. +type MountSpec struct { + Name string // storage name (unit Description only) + UUID string // filesystem UUID — validated; becomes What=/dev/disk/by-uuid/ + Where string // mountpoint — validated; the unit name is derived from it + FSType string // optional Type= + Options string // optional Options= +} + +// Binaries holds the absolute paths of the allow-listed binaries (overridable from config +// so the sudoers entries and the agent agree on exact paths). +type Binaries struct { + Systemctl string + Install string + Smartctl string + Lvs string +} + +func (b Binaries) withDefaults() Binaries { + if b.Systemctl == "" { + b.Systemctl = "/usr/bin/systemctl" + } + if b.Install == "" { + b.Install = "/usr/bin/install" + } + if b.Smartctl == "" { + b.Smartctl = "/usr/sbin/smartctl" + } + if b.Lvs == "" { + b.Lvs = "/usr/sbin/lvs" + } + return b +} + +// SudoHostOps is the production HostOps: it stages a unit file the agent owns, then uses +// the sudoers allowlist (`install` it into the unit dir, `systemctl` to manage it, +// `smartctl`/`lvs` to read). The Runner execs with an arg vector (no shell) — see +// proxmox.ExecRunner in RunnerSudo mode. +type SudoHostOps struct { + runner proxmox.Runner + bins Binaries + unitDir string // where enabled units live (e.g. /etc/systemd/system) + stageDir string // agent-owned staging dir for unit files before install + logger *slog.Logger +} + +// SudoHostOpsConfig configures a SudoHostOps. +type SudoHostOpsConfig struct { + Runner proxmox.Runner + Bins Binaries + UnitDir string // default /etc/systemd/system + StageDir string // default /units; must be agent-writable + Logger *slog.Logger +} + +// NewSudoHostOps builds the production privileged surface. +func NewSudoHostOps(cfg SudoHostOpsConfig) *SudoHostOps { + unitDir := cfg.UnitDir + if unitDir == "" { + unitDir = "/etc/systemd/system" + } + stageDir := cfg.StageDir + if stageDir == "" { + stageDir = "/var/lib/felhom-agent/units" + } + logger := cfg.Logger + if logger == nil { + logger = slog.Default() + } + return &SudoHostOps{ + runner: cfg.Runner, + bins: cfg.Bins.withDefaults(), + unitDir: unitDir, + stageDir: stageDir, + logger: logger, + } +} + +// EnsureMount validates, renders, stages, installs and enables the .mount unit. +func (h *SudoHostOps) EnsureMount(ctx context.Context, spec MountSpec) error { + // VALIDATE FIRST — refuse before constructing any command. + if err := ValidateUUID(spec.UUID); err != nil { + return err + } + if err := ValidateMountPath(spec.Where); err != nil { + return err + } + if err := validateUnitOpt(spec.FSType); err != nil { + return fmt.Errorf("storage: fstype: %w", err) + } + if err := validateUnitOpt(spec.Options); err != nil { + return fmt.Errorf("storage: mount options: %w", err) + } + unitName, err := UnitNameForMount(spec.Where) + if err != nil { + return err + } + content := renderMountUnit(spec) // uses the validated fields only + + // Stage the unit file (agent-owned dir; no root needed for this write). + if err := os.MkdirAll(h.stageDir, 0o700); err != nil { + return fmt.Errorf("storage: staging dir: %w", err) + } + stagePath := filepath.Join(h.stageDir, unitName) + if err := os.WriteFile(stagePath, []byte(content), 0o644); err != nil { + return fmt.Errorf("storage: staging unit: %w", err) + } + + dest := filepath.Join(h.unitDir, unitName) + // install as root (atomic copy with fixed mode/owner) — fixed arg vector. + if err := h.run(ctx, h.bins.Install, "-o", "root", "-g", "root", "-m", "0644", "--", stagePath, dest); err != nil { + return fmt.Errorf("storage: installing unit %s: %w", unitName, err) + } + if err := h.run(ctx, h.bins.Systemctl, "daemon-reload"); err != nil { + return fmt.Errorf("storage: daemon-reload: %w", err) + } + // enable --now both mounts now and persists across reboot. Idempotent. + if err := h.run(ctx, h.bins.Systemctl, "enable", "--now", "--", unitName); err != nil { + return fmt.Errorf("storage: enabling mount %s: %w", unitName, err) + } + h.logger.Info("storage: ensured mount", "name", spec.Name, "where", spec.Where, "unit", unitName) + return nil +} + +// Unmount stops + disables the unit (detach). The caller is responsible for authorization. +func (h *SudoHostOps) Unmount(ctx context.Context, where string) error { + unitName, err := UnitNameForMount(where) + if err != nil { + return err + } + if err := h.run(ctx, h.bins.Systemctl, "stop", "--", unitName); err != nil { + return fmt.Errorf("storage: stopping mount %s: %w", unitName, err) + } + if err := h.run(ctx, h.bins.Systemctl, "disable", "--", unitName); err != nil { + return fmt.Errorf("storage: disabling mount %s: %w", unitName, err) + } + h.logger.Info("storage: unmounted (detached)", "where", where, "unit", unitName) + return nil +} + +// SMART runs `smartctl -a -j ` and parses the JSON. +func (h *SudoHostOps) SMART(ctx context.Context, device string) (hub.SmartSummary, error) { + if err := ValidateSMARTDevice(device); err != nil { + return hub.SmartSummary{Health: hub.SmartUnknown}, err + } + out, stderr, err := h.runner.Run(ctx, h.bins.Smartctl, "-a", "-j", device) + if err != nil && len(out) == 0 { + // smartctl uses a nonzero exit bitmask even on success; only treat empty output + // as a hard failure. A device with no SMART → degrade to UNKNOWN, not an error. + return hub.SmartSummary{Health: hub.SmartUnknown}, fmt.Errorf("storage: smartctl %s: %w: %s", device, err, trim(stderr)) + } + return parseSMART(out), nil +} + +// ThinPoolMetadata runs `lvs` for the pool and returns its metadata-used fraction. +func (h *SudoHostOps) ThinPoolMetadata(ctx context.Context, vg, pool string) (float64, bool) { + if err := ValidateLVMName(vg); err != nil { + h.logger.Warn("storage: refusing lvs on invalid vg", "vg", vg, "err", err) + return 0, false + } + if err := ValidateLVMName(pool); err != nil { + h.logger.Warn("storage: refusing lvs on invalid pool", "pool", pool, "err", err) + return 0, false + } + // --reportformat json, metadata_percent for the specific LV. lv path "vg/pool". + out, stderr, err := h.runner.Run(ctx, h.bins.Lvs, "--reportformat", "json", "--units", "b", + "-o", "lv_name,data_percent,metadata_percent", "--", vg+"/"+pool) + if err != nil && len(out) == 0 { + h.logger.Warn("storage: lvs failed", "vg", vg, "pool", pool, "err", err, "stderr", trim(stderr)) + return 0, false + } + return parseThinPoolMetadata(out) +} + +// run execs an allow-listed command with a fixed arg vector and wraps a nonzero exit. +func (h *SudoHostOps) run(ctx context.Context, name string, args ...string) error { + _, stderr, err := h.runner.Run(ctx, name, args...) + if err != nil { + return fmt.Errorf("%s %s: %w: %s", name, strings.Join(args, " "), err, trim(stderr)) + } + return nil +} + +// validateUnitOpt rejects metacharacters / newlines in an optional unit value (FSType / +// Options) so a crafted value can't inject extra directives into the unit file. Empty is OK. +func validateUnitOpt(v string) error { + if v == "" { + return nil + } + if strings.ContainsAny(v, "\n\r\x00[]=") { + return fmt.Errorf("storage: value %q contains forbidden characters", v) + } + return nil +} + +func trim(b []byte) string { + s := strings.TrimSpace(string(b)) + if len(s) > 300 { + return s[:300] + "…" + } + return s +} + +// NoopHostOps is the safe fallback when the privileged surface is unavailable or declined +// (a missing sudoers entry must degrade with a clear warning, not crash — slice notes). It +// reports SMART as UNKNOWN, no thin-pool metadata, and errors on any write (so a benign +// re-mount logs a clear failure rather than silently "succeeding"). +type NoopHostOps struct{ Logger *slog.Logger } + +func (n NoopHostOps) EnsureMount(context.Context, MountSpec) error { + return fmt.Errorf("storage: privileged HostOps not configured; cannot mount") +} +func (n NoopHostOps) Unmount(context.Context, string) error { + return fmt.Errorf("storage: privileged HostOps not configured; cannot unmount") +} +func (n NoopHostOps) SMART(context.Context, string) (hub.SmartSummary, error) { + return hub.SmartSummary{Health: hub.SmartUnknown}, nil +} +func (n NoopHostOps) ThinPoolMetadata(context.Context, string, string) (float64, bool) { + return 0, false +} diff --git a/internal/storage/hostops_test.go b/internal/storage/hostops_test.go new file mode 100644 index 0000000..b88b76e --- /dev/null +++ b/internal/storage/hostops_test.go @@ -0,0 +1,194 @@ +package storage + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-agent/internal/hub" +) + +// scriptRunner returns fixed stdout per binary name (for SMART/lvs parsing tests) and +// records calls. +type scriptRunner struct { + out map[string][]byte // binary name -> stdout + calls [][]string + err error +} + +func (s *scriptRunner) Run(_ context.Context, name string, args ...string) ([]byte, []byte, error) { + s.calls = append(s.calls, append([]string{name}, args...)) + return s.out[name], nil, s.err +} + +func testStageDir() string { return filepath.Join(os.TempDir(), "felhom-test-units") } + +func TestHostOps_MountLifecycle(t *testing.T) { + ctx := context.Background() + stage := t.TempDir() + unitDir := t.TempDir() + rr := &recordingRunner{} + ops := NewSudoHostOps(SudoHostOpsConfig{ + Runner: rr, + Bins: Binaries{Systemctl: "/usr/bin/systemctl", Install: "/usr/bin/install"}, + UnitDir: unitDir, + StageDir: stage, + Logger: quietLogger(), + }) + + // A hyphen-free mountpoint so the systemd-escaped unit filename has no backslash — the + // backslash escaping is covered by TestSystemdEscapePath; here we just need a filename + // that stages on the test OS (Windows treats '\' as a path separator). Production is Linux. + spec := MountSpec{Name: "usb-backup", UUID: "0fc63daf-8483-4772-8e79-3d69d8477de4", Where: "/srv/felhom/bulk", FSType: "ext4"} + if err := ops.EnsureMount(ctx, spec); err != nil { + t.Fatalf("EnsureMount: %v", err) + } + + // Expect: install (stage→unitDir), daemon-reload, enable --now -- . + if len(rr.calls) != 3 { + t.Fatalf("expected 3 commands, got %d: %v", len(rr.calls), rr.calls) + } + if rr.calls[0][0] != "/usr/bin/install" || !contains(rr.calls[0], "0644") { + t.Errorf("call[0] not the install: %v", rr.calls[0]) + } + if !contains(rr.calls[1], "daemon-reload") { + t.Errorf("call[1] not daemon-reload: %v", rr.calls[1]) + } + if !contains(rr.calls[2], "enable") || !contains(rr.calls[2], "--now") { + t.Errorf("call[2] not enable --now: %v", rr.calls[2]) + } + + // The staged unit file is keyed by UUID and uses the validated mountpoint. + unitName, _ := UnitNameForMount(spec.Where) + content, err := os.ReadFile(filepath.Join(stage, unitName)) + if err != nil { + t.Fatalf("staged unit not written: %v", err) + } + cs := string(content) + if !strings.Contains(cs, "What=/dev/disk/by-uuid/"+spec.UUID) { + t.Errorf("unit missing by-uuid What=: %s", cs) + } + if !strings.Contains(cs, "Where=/srv/felhom/bulk") || !strings.Contains(cs, "Type=ext4") { + t.Errorf("unit missing Where/Type: %s", cs) + } + if !strings.Contains(cs, "WantedBy=multi-user.target") { + t.Errorf("unit not enabled-persistent: %s", cs) + } + + // Unmount (detach) = stop + disable. + rr.calls = nil + if err := ops.Unmount(ctx, spec.Where); err != nil { + t.Fatalf("Unmount: %v", err) + } + if len(rr.calls) != 2 || !contains(rr.calls[0], "stop") || !contains(rr.calls[1], "disable") { + t.Fatalf("Unmount should stop+disable: %v", rr.calls) + } +} + +func TestHostOps_SMART_SATA(t *testing.T) { + sata := []byte(`{ + "smart_status": {"passed": true}, + "temperature": {"current": 38}, + "power_on_time": {"hours": 12345}, + "ata_smart_attributes": {"table": [ + {"id": 5, "name": "Reallocated_Sector_Ct", "raw": {"value": 0}}, + {"id": 197, "name": "Current_Pending_Sector", "raw": {"value": 2}}, + {"id": 198, "name": "Offline_Uncorrectable", "raw": {"value": 1}} + ]} + }`) + ops := &SudoHostOps{runner: &scriptRunner{out: map[string][]byte{"/usr/sbin/smartctl": sata}}, bins: Binaries{}.withDefaults(), logger: quietLogger()} + s, err := ops.SMART(context.Background(), "/dev/sda") + if err != nil { + t.Fatal(err) + } + if s.Health != hub.SmartPassed { + t.Errorf("health = %q, want PASSED", s.Health) + } + if got := deref(s.TemperatureC); got != 38 { + t.Errorf("temp = %d", got) + } + if deref(s.ReallocatedSectors) != 0 || deref(s.PendingSectors) != 2 || deref(s.OfflineUncorrectable) != 1 { + t.Errorf("SATA counters wrong: %+v", s) + } + if s.MediaErrors != nil || s.PercentageUsed != nil { + t.Errorf("NVMe counters must be nil for a SATA disk") + } +} + +func TestHostOps_SMART_NVMe(t *testing.T) { + nvme := []byte(`{ + "smart_status": {"passed": true}, + "nvme_smart_health_information_log": { + "critical_warning": 0, + "media_errors": 5, + "percentage_used": 7, + "temperature": 41 + } + }`) + ops := &SudoHostOps{runner: &scriptRunner{out: map[string][]byte{"/usr/sbin/smartctl": nvme}}, bins: Binaries{}.withDefaults(), logger: quietLogger()} + s, _ := ops.SMART(context.Background(), "/dev/nvme0n1") + if s.Health != hub.SmartPassed { + t.Errorf("health = %q", s.Health) + } + if deref(s.CriticalWarning) != 0 || deref(s.MediaErrors) != 5 || deref(s.PercentageUsed) != 7 { + t.Errorf("NVMe counters wrong: %+v", s) + } + if deref(s.TemperatureC) != 41 { + t.Errorf("nvme temp = %v", s.TemperatureC) + } + if s.ReallocatedSectors != nil { + t.Errorf("SATA counters must be nil for an NVMe disk") + } +} + +func TestHostOps_SMART_Unsupported(t *testing.T) { + // A USB-SATA bridge that exposes no SMART: smartctl returns minimal JSON (no + // smart_status) and a nonzero exit. We degrade to UNKNOWN, not an error. + ops := &SudoHostOps{ + runner: &scriptRunner{out: map[string][]byte{"/usr/sbin/smartctl": []byte(`{"device":{"name":"/dev/sdc"}}`)}, err: errExit(2)}, + bins: Binaries{}.withDefaults(), logger: quietLogger(), + } + s, err := ops.SMART(context.Background(), "/dev/sdc") + if err != nil { + t.Fatalf("unsupported SMART must degrade, not error: %v", err) + } + if s.Health != hub.SmartUnknown { + t.Errorf("health = %q, want UNKNOWN", s.Health) + } +} + +func TestHostOps_ThinPoolMetadata(t *testing.T) { + lvs := []byte(`{"report":[{"lv":[{"lv_name":"data","data_percent":"42.00","metadata_percent":"10.50"}]}]}`) + ops := &SudoHostOps{runner: &scriptRunner{out: map[string][]byte{"/usr/sbin/lvs": lvs}}, bins: Binaries{}.withDefaults(), logger: quietLogger()} + frac, ok := ops.ThinPoolMetadata(context.Background(), "pve", "data") + if !ok { + t.Fatal("expected metadata fraction") + } + if frac < 0.104 || frac > 0.106 { + t.Errorf("metadata fraction = %v, want ~0.105", frac) + } +} + +func contains(ss []string, want string) bool { + for _, s := range ss { + if s == want { + return true + } + } + return false +} + +func deref(p *int) int { + if p == nil { + return -1 + } + return *p +} + +// errExit is a stand-in for a nonzero exit error from the runner. +type errExitT int + +func (e errExitT) Error() string { return "exit status nonzero" } +func errExit(code int) error { return errExitT(code) } diff --git a/internal/storage/mountunit.go b/internal/storage/mountunit.go new file mode 100644 index 0000000..91490e2 --- /dev/null +++ b/internal/storage/mountunit.go @@ -0,0 +1,44 @@ +package storage + +import ( + "fmt" + "strings" +) + +// renderMountUnit builds the systemd .mount unit content for a (already-validated) spec. +// Keyed by fs-UUID via What=/dev/disk/by-uuid/ so it survives /dev/sdX renumbering; +// WantedBy=multi-user.target so `enable` makes it persist across reboot. +// +// All interpolated values are pre-validated by the caller (ValidateUUID / ValidateMountPath +// / validateUnitOpt), so no value here can carry a newline or inject an extra directive. +func renderMountUnit(spec MountSpec) string { + what := byUUIDDir + "/" + spec.UUID + var b strings.Builder + b.WriteString("# Managed by felhom-agent — do not edit by hand.\n") + b.WriteString("[Unit]\n") + fmt.Fprintf(&b, "Description=Felhom storage mount %s\n", sanitizeDesc(spec.Name)) + b.WriteString("After=local-fs-pre.target\n") + b.WriteString("\n[Mount]\n") + fmt.Fprintf(&b, "What=%s\n", what) + fmt.Fprintf(&b, "Where=%s\n", spec.Where) + if spec.FSType != "" { + fmt.Fprintf(&b, "Type=%s\n", spec.FSType) + } + if spec.Options != "" { + fmt.Fprintf(&b, "Options=%s\n", spec.Options) + } + b.WriteString("\n[Install]\n") + b.WriteString("WantedBy=multi-user.target\n") + return b.String() +} + +// sanitizeDesc keeps the Description line single-line and harmless (it is cosmetic; the +// name is already a Proxmox storage id, but be defensive against any newline). +func sanitizeDesc(name string) string { + name = strings.ReplaceAll(name, "\n", " ") + name = strings.ReplaceAll(name, "\r", " ") + if name == "" { + return "(unnamed)" + } + return name +} diff --git a/internal/storage/observe.go b/internal/storage/observe.go index 6a945e7..3349544 100644 --- a/internal/storage/observe.go +++ b/internal/storage/observe.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "log/slog" + "regexp" "strings" "gitea.dooplex.hu/admin/felhom-agent/internal/hub" @@ -26,29 +27,37 @@ type StorageAPI interface { } // Observer builds the observed storage view from Proxmox + non-privileged host reads. +// In Phase B it also (optionally) enriches the reported view with the privileged reads — +// SMART + thin-pool metadata — via HostOps; a nil ops keeps the Phase-A behaviour +// (SMART UNKNOWN, metadata null). type Observer struct { api StorageAPI host HostReader + ops HostOps logger *slog.Logger } // NewObserver builds an Observer. host defaults to a ProcHostReader; logger to the -// default. A nil api makes Observe/Known return an error (misconfiguration), never panic. -func NewObserver(api StorageAPI, host HostReader, logger *slog.Logger) *Observer { +// default. ops is the privileged surface for SMART/lvs — nil disables those (Phase-A +// behaviour). A nil api makes Observe/Known return an error (misconfiguration), never panic. +func NewObserver(api StorageAPI, host HostReader, ops HostOps, logger *slog.Logger) *Observer { if host == nil { host = NewProcHostReader() } if logger == nil { logger = slog.Default() } - return &Observer{api: api, host: host, logger: logger} + return &Observer{api: api, host: host, ops: ops, logger: logger} } // observed is the rich internal view of one target, from which both the reported -// hub.StorageTarget and the watchdog's KnownTarget are projected. +// hub.StorageTarget and the watchdog's KnownTarget are projected. src/cat are kept for +// Observe-time privileged enrichment (NOT used by the watchdog's Known path). type observed struct { target hub.StorageTarget known KnownTarget + src proxmox.Storage + cat storageCategory } // Observe builds the reported []hub.StorageTarget. A non-nil error means the Proxmox read @@ -61,11 +70,43 @@ func (o *Observer) Observe(ctx context.Context) ([]hub.StorageTarget, error) { } out := make([]hub.StorageTarget, 0, len(snap)) for _, s := range snap { - out = append(out, s.target) + out = append(out, o.enrich(ctx, s)) } return out, nil } +// enrich adds the PRIVILEGED reads (SMART, thin-pool metadata) on top of the base target. +// Only Observe calls this (the watchdog's Known path skips it — these are the slow, +// root-shelling reads). A nil ops or a per-target failure degrades gracefully: SMART stays +// UNKNOWN, metadata stays null. +func (o *Observer) enrich(ctx context.Context, ob observed) hub.StorageTarget { + t := ob.target + if o.ops == nil { + return t + } + // SMART: only for dir-backed targets with a resolvable whole-disk device. + if ob.cat == catDir && t.BackingDevice != "" { + if dev, ok := smartDeviceFor(t.BackingDevice); ok { + if sm, err := o.ops.SMART(ctx, dev); err != nil { + o.logger.Warn("storage: SMART read failed; health UNKNOWN", "device", dev, "err", err) + } else { + t.Smart = sm + } + } + } + // Thin-pool metadata fill (the value Phase A left null): lvs on the vg/pool. + if t.Type == hub.StorageTypeLVMThin && t.ThinPool != nil && ob.src.VGName != "" && ob.src.ThinPool != "" { + if frac, ok := o.ops.ThinPoolMetadata(ctx, ob.src.VGName, ob.src.ThinPool); ok { + t.ThinPool.MetadataUsedFraction = &frac + if frac >= thinPoolWarnFraction { + o.logger.Warn("storage: lvmthin pool METADATA fill is high (exhaustion corrupts the pool like data exhaustion)", + "storage", t.Name, "metadata_used_fraction", frac) + } + } + } + return t +} + // Known projects the snapshot to the watchdog's lightweight KnownTarget set. Same Proxmox // + host reads as Observe — callers that poll it fast should wrap it in a cache (the // watchdog uses CachingKnownTargets). @@ -194,10 +235,13 @@ func (o *Observer) build(s proxmox.Storage, mounts []Mount) observed { return observed{ target: tgt, + src: s, + cat: category, known: KnownTarget{ Name: s.Storage, Type: typ, DurableID: durableID, + UUID: uuid, Network: category == catNetwork, MountBacked: typ == hub.StorageTypeUSB || typ == hub.StorageTypeLocalDir, BackingDevice: backingDevice, @@ -207,6 +251,27 @@ func (o *Observer) build(s proxmox.Storage, mounts []Mount) observed { } } +// smartDeviceFor maps a backing device (possibly a partition) to its whole-disk path for +// smartctl (which targets the disk, not the partition). Returns ok=false when the result +// isn't a recognized raw disk (e.g. device-mapper / LVM), so SMART is simply skipped. +func smartDeviceFor(device string) (string, bool) { + dev := device + if m := reNVMePart.FindStringSubmatch(device); m != nil { + dev = m[1] // /dev/nvme0n1p2 -> /dev/nvme0n1 + } else if m := reSDPart.FindStringSubmatch(device); m != nil { + dev = m[1] // /dev/sdb1 -> /dev/sdb + } + if ValidateSMARTDevice(dev) != nil { + return "", false + } + return dev, true +} + +var ( + reNVMePart = regexp.MustCompile(`^(/dev/nvme[0-9]+n[0-9]+)p[0-9]+$`) + reSDPart = regexp.MustCompile(`^(/dev/(?:sd|hd|vd)[a-z]+)[0-9]+$`) +) + // reachable decides whether the target is currently usable. // - usb / local-dir: a Felhom extra/removable dir storage is realized as its OWN // mountpoint, so reachable = it is currently an exact mount AND its device node exists. diff --git a/internal/storage/observe_test.go b/internal/storage/observe_test.go index 6964a98..4288c97 100644 --- a/internal/storage/observe_test.go +++ b/internal/storage/observe_test.go @@ -92,7 +92,7 @@ func TestObserve_BuildsTargetsFromProxmoxAndHostReads(t *testing.T) { removable: map[string]bool{"/dev/sdb1": true, "/dev/mapper/pve-root": false}, } - got, err := NewObserver(api, host, quietLogger()).Observe(context.Background()) + got, err := NewObserver(api, host, nil, quietLogger()).Observe(context.Background()) if err != nil { t.Fatalf("Observe: %v", err) } @@ -156,6 +156,72 @@ func TestObserve_BuildsTargetsFromProxmoxAndHostReads(t *testing.T) { } } +// fakeHostOps fills SMART + thin-pool metadata for the enrichment test. +type fakeHostOps struct { + smartByDevice map[string]hub.SmartSummary + metaByPool map[string]float64 // "vg/pool" -> fraction + smartDevices []string // records which devices SMART was called on +} + +func (f *fakeHostOps) EnsureMount(context.Context, MountSpec) error { return nil } +func (f *fakeHostOps) Unmount(context.Context, string) error { return nil } +func (f *fakeHostOps) SMART(_ context.Context, device string) (hub.SmartSummary, error) { + f.smartDevices = append(f.smartDevices, device) + if s, ok := f.smartByDevice[device]; ok { + return s, nil + } + return hub.SmartSummary{Health: hub.SmartUnknown}, nil +} +func (f *fakeHostOps) ThinPoolMetadata(_ context.Context, vg, pool string) (float64, bool) { + v, ok := f.metaByPool[vg+"/"+pool] + return v, ok +} + +func TestObserve_EnrichesSMARTAndThinPoolMetadata(t *testing.T) { + api := &fakeStorageAPI{ + node: "demo-felhom", + cluster: []proxmox.Storage{ + {Storage: "local-lvm", Type: "lvmthin", VGName: "pve", ThinPool: "data"}, + {Storage: "usb-backup", Type: "dir", Path: "/mnt/usb-backup"}, + }, + nodeSt: []proxmox.Storage{ + {Storage: "local-lvm", Type: "lvmthin", Active: 1, UsedFraction: 0.4}, + {Storage: "usb-backup", Type: "dir", Path: "/mnt/usb-backup", Active: 1}, + }, + } + host := &fakeHostReader{ + mounts: []Mount{{Device: "/dev/sdb1", MountPoint: "/mnt/usb-backup", FSType: "ext4"}}, + uuids: map[string]string{"/dev/sdb1": "1111-2222"}, + exists: map[string]bool{"/dev/sdb1": true}, + removable: map[string]bool{"/dev/sdb1": true}, + } + ops := &fakeHostOps{ + smartByDevice: map[string]hub.SmartSummary{"/dev/sdb": {Health: hub.SmartPassed}}, + metaByPool: map[string]float64{"pve/data": 0.12}, + } + got, err := NewObserver(api, host, ops, quietLogger()).Observe(context.Background()) + if err != nil { + t.Fatal(err) + } + m := byName(got) + + // SMART runs on the WHOLE disk (/dev/sdb), not the partition (/dev/sdb1). + if len(ops.smartDevices) != 1 || ops.smartDevices[0] != "/dev/sdb" { + t.Errorf("SMART should target the whole disk /dev/sdb, got %v", ops.smartDevices) + } + if m["usb-backup"].Smart.Health != hub.SmartPassed { + t.Errorf("usb SMART not enriched: %+v", m["usb-backup"].Smart) + } + // lvmthin metadata fill (Phase B) is now populated. + lvm := m["local-lvm"] + if lvm.ThinPool == nil || lvm.ThinPool.MetadataUsedFraction == nil { + t.Fatalf("lvmthin metadata fill not enriched: %+v", lvm.ThinPool) + } + if *lvm.ThinPool.MetadataUsedFraction != 0.12 { + t.Errorf("metadata fraction = %v, want 0.12", *lvm.ThinPool.MetadataUsedFraction) + } +} + func TestObserve_USBUnpluggedIsDisconnected(t *testing.T) { api := &fakeStorageAPI{ node: "demo-felhom", @@ -170,7 +236,7 @@ func TestObserve_USBUnpluggedIsDisconnected(t *testing.T) { host := &fakeHostReader{ mounts: []Mount{{Device: "/dev/mapper/pve-root", MountPoint: "/", FSType: "ext4"}}, // no /mnt/usb-backup } - got, err := NewObserver(api, host, quietLogger()).Observe(context.Background()) + got, err := NewObserver(api, host, nil, quietLogger()).Observe(context.Background()) if err != nil { t.Fatal(err) } @@ -187,7 +253,7 @@ func TestObserve_USBUnpluggedIsDisconnected(t *testing.T) { func TestObserve_ProxmoxErrorIsFatalForStorage(t *testing.T) { api := &fakeStorageAPI{node: "n", listErr: context.DeadlineExceeded} - if _, err := NewObserver(api, &fakeHostReader{}, quietLogger()).Observe(context.Background()); err == nil { + if _, err := NewObserver(api, &fakeHostReader{}, nil, quietLogger()).Observe(context.Background()); err == nil { t.Fatal("a Proxmox read error must surface (the collector then omits storage this cycle)") } } @@ -199,7 +265,7 @@ func TestObserve_MountReadFailureDegradesNotFatal(t *testing.T) { nodeSt: []proxmox.Storage{{Storage: "local-lvm", Type: "lvmthin", Active: 1, UsedFraction: 0.1}}, } host := &fakeHostReader{mountsErr: io.ErrUnexpectedEOF} - got, err := NewObserver(api, host, quietLogger()).Observe(context.Background()) + got, err := NewObserver(api, host, nil, quietLogger()).Observe(context.Background()) if err != nil { t.Fatalf("a host mount-read failure must degrade, not fail: %v", err) } diff --git a/internal/storage/smart.go b/internal/storage/smart.go new file mode 100644 index 0000000..24c6aa0 --- /dev/null +++ b/internal/storage/smart.go @@ -0,0 +1,140 @@ +package storage + +import ( + "encoding/json" + "strconv" + + "gitea.dooplex.hu/admin/felhom-agent/internal/hub" +) + +// smartctlJSON is the lenient subset of `smartctl -a -j` output we read. Pointers detect +// presence so an absent section (e.g. NVMe fields on a SATA disk, or no SMART at all on a +// USB bridge) decodes cleanly to nil and we degrade to UNKNOWN. +type smartctlJSON struct { + SmartStatus *struct { + Passed bool `json:"passed"` + } `json:"smart_status"` + Temperature *struct { + Current *int `json:"current"` + } `json:"temperature"` + PowerOnTime *struct { + Hours *int `json:"hours"` + } `json:"power_on_time"` + // SATA/ATA attribute table. + ATA *struct { + Table []struct { + ID int `json:"id"` + Raw struct { + Value int64 `json:"value"` + } `json:"raw"` + } `json:"table"` + } `json:"ata_smart_attributes"` + // NVMe health log. + NVMe *struct { + CriticalWarning *int `json:"critical_warning"` + MediaErrors *int64 `json:"media_errors"` + PercentageUsed *int `json:"percentage_used"` + Temperature *int `json:"temperature"` + } `json:"nvme_smart_health_information_log"` +} + +// SATA attribute IDs we surface. +const ( + ataReallocatedSectorCt = 5 + ataCurrentPending = 197 + ataOfflineUncorrect = 198 +) + +// parseSMART maps smartctl JSON to a hub.SmartSummary, handling SATA + NVMe and degrading +// to UNKNOWN when health is not reported. A device populates only its own attribute set. +func parseSMART(raw []byte) hub.SmartSummary { + s := hub.SmartSummary{Health: hub.SmartUnknown} + if len(raw) == 0 { + return s + } + var j smartctlJSON + if err := json.Unmarshal(raw, &j); err != nil { + return s // unparseable → UNKNOWN (never an error to the report) + } + + if j.SmartStatus != nil { + if j.SmartStatus.Passed { + s.Health = hub.SmartPassed + } else { + s.Health = hub.SmartFailing + } + } + if j.Temperature != nil && j.Temperature.Current != nil { + s.TemperatureC = j.Temperature.Current + } + if j.PowerOnTime != nil && j.PowerOnTime.Hours != nil { + s.PowerOnHours = j.PowerOnTime.Hours + } + + // SATA attributes. + if j.ATA != nil { + for _, a := range j.ATA.Table { + switch a.ID { + case ataReallocatedSectorCt: + s.ReallocatedSectors = intPtr(int(a.Raw.Value)) + case ataCurrentPending: + s.PendingSectors = intPtr(int(a.Raw.Value)) + case ataOfflineUncorrect: + s.OfflineUncorrectable = intPtr(int(a.Raw.Value)) + } + } + } + + // NVMe attributes. + if j.NVMe != nil { + s.CriticalWarning = j.NVMe.CriticalWarning + if j.NVMe.MediaErrors != nil { + s.MediaErrors = intPtr(int(*j.NVMe.MediaErrors)) + } + s.PercentageUsed = j.NVMe.PercentageUsed + // NVMe reports temperature in its own log when the top-level block is absent. + if s.TemperatureC == nil && j.NVMe.Temperature != nil { + s.TemperatureC = j.NVMe.Temperature + } + } + return s +} + +// lvsReport is the lenient subset of `lvs --reportformat json` output. +type lvsReport struct { + Report []struct { + LV []struct { + LVName string `json:"lv_name"` + DataPercent string `json:"data_percent"` + MetadataPercent string `json:"metadata_percent"` + } `json:"lv"` + } `json:"report"` +} + +// parseThinPoolMetadata extracts the metadata-used fraction (0..1) from lvs JSON. lvs +// reports percentages as decimal strings (e.g. "10.50"); an empty string means "not a thin +// pool / not applicable" → ok=false. +func parseThinPoolMetadata(raw []byte) (float64, bool) { + if len(raw) == 0 { + return 0, false + } + var r lvsReport + if err := json.Unmarshal(raw, &r); err != nil { + return 0, false + } + for _, rep := range r.Report { + for _, lv := range rep.LV { + if lv.MetadataPercent == "" { + continue + } + pct, err := strconv.ParseFloat(lv.MetadataPercent, 64) + if err != nil { + continue + } + return pct / 100, true + } + } + return 0, false +} + +func intPtr(v int) *int { return &v } diff --git a/internal/storage/validate.go b/internal/storage/validate.go new file mode 100644 index 0000000..7ad509f --- /dev/null +++ b/internal/storage/validate.go @@ -0,0 +1,178 @@ +package storage + +import ( + "fmt" + "regexp" + "strings" +) + +// This file is the security boundary for the privileged host surface (slice 5 Phase B). +// EVERY argument that will reach a root shell-out is validated HERE, before any command +// is constructed — the SudoHostOps methods refuse on a validation error and never build an +// arg vector, let alone exec. The adversarial matrix in validate_test.go is the proof that +// the "aggressive write side" is not a loose one: shell metacharacters, path traversal, and +// malformed inputs are rejected up front. Combined with arg-vector exec (never a shell +// string), a validated input cannot inject. + +var ( + // fs-UUIDs: ext/xfs are 8-4-4-4-12 lowercase hex; FAT/vFAT are "XXXX-XXXX" (upper + // hex); others vary. Accept hex groups joined by single hyphens, length-bounded. + // This rejects '/', '.', whitespace, and every shell metacharacter by construction. + reUUID = regexp.MustCompile(`^[A-Fa-f0-9]{4,}(-[A-Fa-f0-9]+){0,4}$`) + + // SMART device: a strict whitelist of real block-disk patterns under /dev. No + // /dev/disk/by-* symlinks, no device-mapper, no traversal — just the raw disks + // smartctl is run against. Anything else is refused. + reSMARTDevice = regexp.MustCompile(`^/dev/(sd[a-z]+|nvme[0-9]+n[0-9]+|hd[a-z]+|vd[a-z]+)$`) + + // LVM VG / pool names: LVM permits [A-Za-z0-9._+-]; we forbid leading '-' (would look + // like a flag) and cap the length. + reLVMName = regexp.MustCompile(`^[A-Za-z0-9_+.][A-Za-z0-9_+.-]*$`) + + // A single safe path segment (for mountpoint validation). No metacharacters; "." and + // ".." are rejected separately as traversal. + rePathSegment = regexp.MustCompile(`^[A-Za-z0-9._-]+$`) +) + +const ( + maxUUIDLen = 40 + maxPathLen = 255 + maxLVMLen = 128 + byUUIDDir = "/dev/disk/by-uuid" + maxMountSeg = 32 // a sane cap on mountpoint depth +) + +// ValidateUUID accepts a filesystem UUID for use in a by-uuid device path. It is the +// load-bearing check (the UUID is the DR re-attach key AND a shell-out argument). +func ValidateUUID(uuid string) error { + if uuid == "" { + return fmt.Errorf("storage: empty UUID") + } + if len(uuid) > maxUUIDLen { + return fmt.Errorf("storage: UUID too long (%d > %d)", len(uuid), maxUUIDLen) + } + if !reUUID.MatchString(uuid) { + return fmt.Errorf("storage: invalid UUID %q (want hex groups, no metacharacters)", uuid) + } + return nil +} + +// ByUUIDDevicePath returns the validated /dev/disk/by-uuid/ path for a mount unit's +// What=. Device paths for mounting are ALWAYS confined to this directory — we never accept +// an arbitrary device path from any source. +func ByUUIDDevicePath(uuid string) (string, error) { + if err := ValidateUUID(uuid); err != nil { + return "", err + } + return byUUIDDir + "/" + uuid, nil +} + +// ValidateMountPath accepts an absolute mountpoint with no traversal and no metacharacters. +// Each segment must be a safe token; "." / ".." segments are rejected; the bare root "/" +// is rejected (we never manage a mount at root). +func ValidateMountPath(path string) error { + if path == "" || path[0] != '/' { + return fmt.Errorf("storage: mount path must be absolute, got %q", path) + } + if len(path) > maxPathLen { + return fmt.Errorf("storage: mount path too long (%d > %d)", len(path), maxPathLen) + } + if strings.ContainsAny(path, "\x00\n\r\t") { + return fmt.Errorf("storage: mount path contains control characters") + } + segs := nonEmptySegments(path) + if len(segs) == 0 { + return fmt.Errorf("storage: refusing to manage a mount at %q", path) + } + if len(segs) > maxMountSeg { + return fmt.Errorf("storage: mount path too deep") + } + for _, s := range segs { + if s == "." || s == ".." { + return fmt.Errorf("storage: mount path traversal segment %q in %q", s, path) + } + if !rePathSegment.MatchString(s) { + return fmt.Errorf("storage: invalid mount path segment %q in %q", s, path) + } + } + return nil +} + +// ValidateSMARTDevice accepts only a raw block-disk path (sdX/nvmeXnY/hdX/vdX) under /dev. +func ValidateSMARTDevice(device string) error { + if !reSMARTDevice.MatchString(device) { + return fmt.Errorf("storage: refusing smartctl on non-whitelisted device %q", device) + } + return nil +} + +// ValidateLVMName accepts an LVM VG or LV (pool) name. +func ValidateLVMName(name string) error { + if name == "" { + return fmt.Errorf("storage: empty LVM name") + } + if len(name) > maxLVMLen { + return fmt.Errorf("storage: LVM name too long") + } + if !reLVMName.MatchString(name) { + return fmt.Errorf("storage: invalid LVM name %q", name) + } + return nil +} + +// UnitNameForMount returns the systemd .mount unit name for a (validated) mountpoint. A +// .mount unit's name MUST be the systemd-escaped mountpoint — this is computed +// deterministically from the already-validated path, so the result is inherently safe to +// pass in an arg vector (no shell). +func UnitNameForMount(where string) (string, error) { + if err := ValidateMountPath(where); err != nil { + return "", err + } + return systemdEscapePath(where) + ".mount", nil +} + +// nonEmptySegments splits a path on '/', dropping empties (so "//a///b/" → [a b]). +func nonEmptySegments(path string) []string { + parts := strings.Split(path, "/") + out := parts[:0] + for _, p := range parts { + if p != "" { + out = append(out, p) + } + } + return out +} + +// systemdEscapePath replicates `systemd-escape --path`: strip leading/trailing slashes and +// collapse internal repeats, then escape each char — '/' → '-', alnum/'_' kept, '.' kept +// (except a leading '.'), everything else (including a literal '-') → '\xNN'. The empty +// path / "/" escapes to "-". Computed in-process so no `systemd-escape` shell-out / sudoers +// entry is needed. +func systemdEscapePath(path string) string { + segs := nonEmptySegments(path) + if len(segs) == 0 { + return "-" + } + joined := strings.Join(segs, "/") + var b strings.Builder + for i := 0; i < len(joined); i++ { + c := joined[i] + switch { + case c == '/': + b.WriteByte('-') + case i == 0 && c == '.': + b.WriteString(`\x2e`) + case isAlnum(c) || c == '_': + b.WriteByte(c) + case c == '.': + b.WriteByte('.') + default: + fmt.Fprintf(&b, `\x%02x`, c) + } + } + return b.String() +} + +func isAlnum(c byte) bool { + return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') +} diff --git a/internal/storage/validate_test.go b/internal/storage/validate_test.go new file mode 100644 index 0000000..25396b2 --- /dev/null +++ b/internal/storage/validate_test.go @@ -0,0 +1,209 @@ +package storage + +import ( + "context" + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-agent/internal/proxmox" +) + +// recordingRunner records every command it is asked to run (and never execs anything). The +// adversarial matrix asserts that a rejected argument means ZERO commands were constructed — +// the validator is the wall, not the exec. +type recordingRunner struct { + calls [][]string + err error +} + +func (r *recordingRunner) Run(_ context.Context, name string, args ...string) ([]byte, []byte, error) { + r.calls = append(r.calls, append([]string{name}, args...)) + return nil, nil, r.err +} + +// --- The headline: the arg-validator adversarial matrix. --- + +func TestValidateUUID_AdversarialMatrix(t *testing.T) { + good := []string{ + "0fc63daf-8483-4772-8e79-3d69d8477de4", // ext4 + "1234-ABCD", // FAT + "deadbeefdeadbeef", // NTFS-ish 16 hex + } + for _, u := range good { + if err := ValidateUUID(u); err != nil { + t.Errorf("ValidateUUID(%q) rejected a valid UUID: %v", u, err) + } + } + bad := []string{ + "", // empty + "../../etc/shadow", // traversal + "abcd; rm -rf /", // shell metacharacters + "abcd$(reboot)", // command substitution + "abcd`reboot`", // backticks + "abcd&whoami", // & + "abcd|cat", // pipe + "abcd\nreboot", // newline + "abcd /dev/sda", // space + extra arg + "g00dlooking-but-z-not-hex", // non-hex + "/dev/disk/by-uuid/abcd", // a path, not a uuid + strings.Repeat("a", maxUUIDLen+1), // too long + "abcd\x00", // NUL + } + for _, u := range bad { + if err := ValidateUUID(u); err == nil { + t.Errorf("ValidateUUID(%q) ACCEPTED a hostile UUID", u) + } + } +} + +func TestValidateMountPath_AdversarialMatrix(t *testing.T) { + good := []string{"/mnt/usb-backup", "/srv/felhom/bulk", "/mnt/data_1"} + for _, p := range good { + if err := ValidateMountPath(p); err != nil { + t.Errorf("ValidateMountPath(%q) rejected a valid path: %v", p, err) + } + } + bad := []string{ + "", // empty + "relative/path", // not absolute + "/", // bare root + "/mnt/../etc", // traversal + "/mnt/./x", // dot segment + "/mnt/usb backup", // space + "/mnt/usb;reboot", // metacharacter + "/mnt/$(reboot)", // command substitution + "/mnt/x\nWhat=/dev/sda", // newline → unit-file injection attempt + "/mnt/x\x00", // NUL + "/mnt/x`reboot`", // backticks + } + for _, p := range bad { + if err := ValidateMountPath(p); err == nil { + t.Errorf("ValidateMountPath(%q) ACCEPTED a hostile path", p) + } + } +} + +func TestValidateSMARTDevice_AdversarialMatrix(t *testing.T) { + good := []string{"/dev/sda", "/dev/sdb", "/dev/nvme0n1", "/dev/vda"} + for _, d := range good { + if err := ValidateSMARTDevice(d); err != nil { + t.Errorf("ValidateSMARTDevice(%q) rejected a valid device: %v", d, err) + } + } + bad := []string{ + "/dev/sda1", // a partition, not the whole disk (smartctl targets the disk) + "/dev/../etc/shadow", // traversal + "/dev/sda;reboot", // metacharacter + "/dev/sda /dev/sdb", // extra arg + "/etc/passwd", // not /dev + "sda", // no /dev prefix + "/dev/mapper/pve-root", // device-mapper not whitelisted + "", // empty + } + for _, d := range bad { + if err := ValidateSMARTDevice(d); err == nil { + t.Errorf("ValidateSMARTDevice(%q) ACCEPTED a hostile device", d) + } + } +} + +func TestValidateLVMName_AdversarialMatrix(t *testing.T) { + for _, n := range []string{"pve", "data", "vg0", "vg.thin_pool"} { + if err := ValidateLVMName(n); err != nil { + t.Errorf("ValidateLVMName(%q) rejected a valid name: %v", n, err) + } + } + for _, n := range []string{"", "-rf", "vg;reboot", "vg/pool extra", "vg\nx", "vg$(x)"} { + if err := ValidateLVMName(n); err == nil { + t.Errorf("ValidateLVMName(%q) ACCEPTED a hostile name", n) + } + } +} + +// TestHostOps_RejectsHostileArgsBeforeExec is the proof that validation happens BEFORE any +// command is constructed: a hostile UUID / mount path / device → error AND zero runner calls. +func TestHostOps_RejectsHostileArgsBeforeExec(t *testing.T) { + ctx := context.Background() + + t.Run("EnsureMount hostile UUID", func(t *testing.T) { + rr := &recordingRunner{} + ops := newTestHostOps(rr) + err := ops.EnsureMount(ctx, MountSpec{Name: "x", UUID: "abcd; rm -rf /", Where: "/mnt/x"}) + if err == nil { + t.Fatal("expected rejection") + } + if len(rr.calls) != 0 { + t.Fatalf("a hostile UUID must be refused before any exec; got calls %v", rr.calls) + } + }) + + t.Run("EnsureMount traversal mountpoint", func(t *testing.T) { + rr := &recordingRunner{} + ops := newTestHostOps(rr) + err := ops.EnsureMount(ctx, MountSpec{Name: "x", UUID: "1234-ABCD", Where: "/mnt/../etc"}) + if err == nil || len(rr.calls) != 0 { + t.Fatalf("traversal mountpoint must be refused before exec; err=%v calls=%v", err, rr.calls) + } + }) + + t.Run("EnsureMount injection via mount options", func(t *testing.T) { + rr := &recordingRunner{} + ops := newTestHostOps(rr) + err := ops.EnsureMount(ctx, MountSpec{Name: "x", UUID: "1234-ABCD", Where: "/mnt/x", Options: "ro\nWhat=/dev/sda"}) + if err == nil || len(rr.calls) != 0 { + t.Fatalf("newline-injecting options must be refused before exec; err=%v calls=%v", err, rr.calls) + } + }) + + t.Run("SMART hostile device", func(t *testing.T) { + rr := &recordingRunner{} + ops := newTestHostOps(rr) + _, err := ops.SMART(ctx, "/dev/sda;reboot") + if err == nil || len(rr.calls) != 0 { + t.Fatalf("hostile smart device must be refused before exec; err=%v calls=%v", err, rr.calls) + } + }) + + t.Run("ThinPoolMetadata hostile vg", func(t *testing.T) { + rr := &recordingRunner{} + ops := newTestHostOps(rr) + if _, ok := ops.ThinPoolMetadata(ctx, "vg;reboot", "data"); ok { + t.Fatal("hostile vg must return ok=false") + } + if len(rr.calls) != 0 { + t.Fatalf("hostile vg must be refused before exec; calls=%v", rr.calls) + } + }) +} + +// newTestHostOps builds a SudoHostOps over a recording runner with a temp stage dir (so the +// EnsureMount staging write — which happens AFTER validation — has somewhere to go in the +// rare valid-path test; hostile-path tests never reach it). +func newTestHostOps(rr proxmox.Runner) *SudoHostOps { + return NewSudoHostOps(SudoHostOpsConfig{ + Runner: rr, + UnitDir: "/tmp/felhom-test-units", + StageDir: testStageDir(), + Logger: quietLogger(), + }) +} + +func TestSystemdEscapePath(t *testing.T) { + cases := map[string]string{ + "/mnt/usb-backup": "mnt-usb\\x2dbackup", + "/var/lib/vz": "var-lib-vz", + "/srv/data": "srv-data", + "/": "-", + "/etc/foo.conf": "etc-foo.conf", + } + for in, want := range cases { + if got := systemdEscapePath(in); got != want { + t.Errorf("systemdEscapePath(%q) = %q, want %q", in, got, want) + } + } + // The unit name is derived deterministically and ends in .mount. + name, err := UnitNameForMount("/mnt/usb-backup") + if err != nil || !strings.HasSuffix(name, ".mount") { + t.Errorf("UnitNameForMount = %q, %v", name, err) + } +} diff --git a/internal/storage/watchdog.go b/internal/storage/watchdog.go index 4f6fd08..b5265ec 100644 --- a/internal/storage/watchdog.go +++ b/internal/storage/watchdog.go @@ -23,6 +23,7 @@ type KnownTarget struct { Name string Type string DurableID string + UUID string // fs-UUID (mount-backed targets) — the by-UUID re-mount key Network bool // nfs/cifs/pbs — liveness is a reachability dial, not a device check MountBacked bool // usb/local-dir — a drop = its mountpoint disappears BackingDevice string // resolved block device (local targets) @@ -36,11 +37,23 @@ type KnownTargets interface { Known(ctx context.Context) ([]KnownTarget, error) } -// TargetLiveness reports whether one known target is presently up. Production is -// HostLiveness (device/mount presence + a reachability dial, all non-privileged); tests -// inject a fake. +// TargetLiveness reports a known target's liveness. Production is HostLiveness (device/mount +// presence + a reachability dial, all non-privileged); tests inject a fake. type TargetLiveness interface { + // Present is the "in service" signal: mounted + reachable. Present(ctx context.Context, t KnownTarget) bool + // DevicePresent is the "backing device is physically back" signal, independent of + // whether it is mounted — the trigger for a benign re-mount of a returned drive. + DevicePresent(ctx context.Context, t KnownTarget) bool +} + +// Remounter performs the benign re-mount response when a known mount-backed target's device +// returns but its mountpoint is missing. The watchdog dispatches to it OFF its poll path +// (a goroutine), never synchronously under the lock. Production routes through the gate +// (benign) then HostOps.EnsureMount; wired in main.go so storage stays decoupled from +// reconcile. A nil Remounter disables the response (observe-only, Phase-A behaviour). +type Remounter interface { + Remount(ctx context.Context, t KnownTarget) } // Transition is one observed state change for a known target (for logging/diagnostics). @@ -57,30 +70,34 @@ type Transition struct { // It NEVER mutates anything (Phase A is read-only) — the benign re-mount-by-UUID response // to a return lands in Phase B. Here it only observes and signals. type Watchdog struct { - targets KnownTargets - liveness TargetLiveness - interval time.Duration - debounce time.Duration - trigger func() // request an out-of-band report (debounced by the watchdog) - logger *slog.Logger - now func() time.Time + targets KnownTargets + liveness TargetLiveness + remounter Remounter // may be nil (observe-only) + interval time.Duration + debounce time.Duration + trigger func() // request an out-of-band report (debounced by the watchdog) + logger *slog.Logger + now func() time.Time + spawn func(func()) // spawn a background task (overridable in tests; default `go f()`) - mu sync.Mutex - last map[string]bool // name -> last observed present (only for seen targets) - lastFire time.Time - fired bool // lastFire is valid - pending bool // a transition is awaiting the debounce window + mu sync.Mutex + last map[string]bool // name -> last observed present (only for seen targets) + lastFire time.Time + fired bool // lastFire is valid + pending bool // a transition is awaiting the debounce window + lastRemount map[string]time.Time // name -> last re-mount dispatch (rate-limit) } // WatchdogOptions configures a Watchdog. Targets, Liveness and Trigger are required; the -// rest default. +// rest default. Remounter is optional (nil = observe-only). type WatchdogOptions struct { - Targets KnownTargets - Liveness TargetLiveness - Trigger func() - Interval time.Duration - Debounce time.Duration - Logger *slog.Logger + Targets KnownTargets + Liveness TargetLiveness + Remounter Remounter + Trigger func() + Interval time.Duration + Debounce time.Duration + Logger *slog.Logger } // NewWatchdog builds a Watchdog. A nil Trigger is tolerated (the watchdog still tracks @@ -103,14 +120,17 @@ func NewWatchdog(opts WatchdogOptions) *Watchdog { trigger = func() {} } return &Watchdog{ - targets: opts.Targets, - liveness: opts.Liveness, - interval: interval, - debounce: debounce, - trigger: trigger, - logger: logger, - now: func() time.Time { return time.Now().UTC() }, - last: map[string]bool{}, + targets: opts.Targets, + liveness: opts.Liveness, + remounter: opts.Remounter, + interval: interval, + debounce: debounce, + trigger: trigger, + logger: logger, + now: func() time.Time { return time.Now().UTC() }, + spawn: func(f func()) { go f() }, + last: map[string]bool{}, + lastRemount: map[string]time.Time{}, } } @@ -137,9 +157,11 @@ func (w *Watchdog) Run(ctx context.Context) error { } } -// tick performs one poll: read the known set, probe each target's liveness, diff against -// the last-seen state, and fire a debounced trigger on any transition for a SEEN target. -// It is deterministic given w.now — tests drive it directly with a fake clock. +// tick performs one poll. Structure: probe liveness OUTSIDE the lock (the probes do IO — +// mount reads, dials), then take the lock only for the state diff + debounce decision, then +// perform side-effects (report trigger, re-mount dispatch) AFTER unlocking. The re-mount is +// handed to a background task — never run synchronously under the lock or on the poll path. +// Deterministic given w.now — tests drive it directly with a fake clock. func (w *Watchdog) tick(ctx context.Context) { known, err := w.targets.Known(ctx) if err != nil { @@ -147,52 +169,78 @@ func (w *Watchdog) tick(ctx context.Context) { return } - w.mu.Lock() - defer w.mu.Unlock() - - var transitions []Transition - current := make(map[string]bool, len(known)) - for _, k := range known { - present := w.liveness.Present(ctx, k) - current[k.Name] = present - prev, seen := w.last[k.Name] - if !seen { - continue // first observation → baseline only (never flag a never-attached drop) - } - if prev != present { - transitions = append(transitions, Transition{Name: k.Name, From: stateStr(prev), To: stateStr(present)}) - } + // Probe outside the lock. + type probe struct { + t KnownTarget + present bool + devicePresent bool + } + probes := make([]probe, 0, len(known)) + for _, k := range known { + p := probe{t: k, present: w.liveness.Present(ctx, k)} + if k.MountBacked && !p.present { + p.devicePresent = w.liveness.DevicePresent(ctx, k) + } + probes = append(probes, p) } - // Replace the baseline with the current snapshot (targets no longer known drop out). - w.last = current now := w.now() - if len(transitions) > 0 { - for _, tr := range transitions { - w.logger.Warn("storage: watchdog detected target state change", - "target", tr.Name, "from", tr.From, "to", tr.To) + + w.mu.Lock() + var transitions []Transition + var remounts []KnownTarget + current := make(map[string]bool, len(probes)) + for _, p := range probes { + current[p.t.Name] = p.present + if prev, seen := w.last[p.t.Name]; seen && prev != p.present { + transitions = append(transitions, Transition{Name: p.t.Name, From: stateStr(prev), To: stateStr(p.present)}) } + // Re-mount candidate: a mount-backed target that is NOT mounted but whose backing + // device is physically present (a disconnected→device-back state). Rate-limited per + // target to the debounce window so a persistent mount failure can't storm HostOps. + if w.remounter != nil && p.t.MountBacked && !p.present && p.devicePresent { + if last, ok := w.lastRemount[p.t.Name]; !ok || now.Sub(last) >= w.debounce { + w.lastRemount[p.t.Name] = now + remounts = append(remounts, p.t) + } + } + // Once a target is present again, clear its re-mount rate-limit so a future cycle + // re-mounts promptly. + if p.present { + delete(w.lastRemount, p.t.Name) + } + } + w.last = current // targets no longer known drop out + + doFire := false + if len(transitions) > 0 { if !w.fired || now.Sub(w.lastFire) >= w.debounce { - w.fire(now, len(transitions)) + doFire = true + w.lastFire, w.fired, w.pending = now, true, false } else { w.pending = true - w.logger.Debug("storage: watchdog debouncing transition", "pending_until", w.lastFire.Add(w.debounce)) } - return + } else if w.pending && now.Sub(w.lastFire) >= w.debounce { + doFire = true + w.lastFire, w.pending = now, false } - // No new transition, but a debounced one is pending and the window has elapsed → fire. - if w.pending && now.Sub(w.lastFire) >= w.debounce { - w.fire(now, 0) - } -} + w.mu.Unlock() -// fire requests the out-of-band report and resets the debounce window. Called under w.mu. -func (w *Watchdog) fire(now time.Time, n int) { - w.lastFire = now - w.fired = true - w.pending = false - w.logger.Info("storage: watchdog triggering out-of-band host-report", "transitions", n) - w.trigger() + // Side-effects, off the lock. + for _, tr := range transitions { + w.logger.Warn("storage: watchdog detected target state change", + "target", tr.Name, "from", tr.From, "to", tr.To) + } + if doFire { + w.logger.Info("storage: watchdog triggering out-of-band host-report", "transitions", len(transitions)) + w.trigger() + } + for _, t := range remounts { + t := t + w.logger.Info("storage: watchdog dispatching benign re-mount (device returned)", + "target", t.Name, "where", t.MountPath) + w.spawn(func() { w.remounter.Remount(ctx, t) }) + } } func stateStr(present bool) string { @@ -250,6 +298,21 @@ func (h *HostLiveness) Present(ctx context.Context, t KnownTarget) bool { return true } +// DevicePresent reports whether the backing device is physically present (regardless of +// mount state) — the re-mount trigger. Checks /dev/disk/by-uuid/ first (the by-UUID +// link appears when the drive is plugged), then any known backing-device node. +func (h *HostLiveness) DevicePresent(ctx context.Context, t KnownTarget) bool { + if !t.MountBacked { + return false + } + if t.UUID != "" { + if dev, err := ByUUIDDevicePath(t.UUID); err == nil && h.host.DeviceExists(dev) { + return true + } + } + return t.BackingDevice != "" && h.host.DeviceExists(t.BackingDevice) +} + func (h *HostLiveness) mounted(path string) bool { if path == "" { return false diff --git a/internal/storage/watchdog_test.go b/internal/storage/watchdog_test.go index 68f9f91..a56554f 100644 --- a/internal/storage/watchdog_test.go +++ b/internal/storage/watchdog_test.go @@ -24,10 +24,11 @@ func (s *staticKnown) Known(context.Context) ([]KnownTarget, error) { return s.targets, s.err } -// mapLiveness is a settable per-target presence fake. +// mapLiveness is a settable per-target presence + device-presence fake. type mapLiveness struct { mu sync.Mutex present map[string]bool + device map[string]bool // backing-device presence (re-mount trigger) } func (m *mapLiveness) set(name string, p bool) { @@ -35,11 +36,24 @@ func (m *mapLiveness) set(name string, p bool) { defer m.mu.Unlock() m.present[name] = p } +func (m *mapLiveness) setDevice(name string, p bool) { + m.mu.Lock() + defer m.mu.Unlock() + if m.device == nil { + m.device = map[string]bool{} + } + m.device[name] = p +} func (m *mapLiveness) Present(_ context.Context, t KnownTarget) bool { m.mu.Lock() defer m.mu.Unlock() return m.present[t.Name] } +func (m *mapLiveness) DevicePresent(_ context.Context, t KnownTarget) bool { + m.mu.Lock() + defer m.mu.Unlock() + return m.device[t.Name] +} // newTestWatchdog builds a watchdog with a manual clock and a trigger counter. func newTestWatchdog(known KnownTargets, live TargetLiveness, debounce time.Duration) (*Watchdog, *int, *time.Time) { @@ -129,6 +143,71 @@ func TestWatchdog_DebounceCoalescesFlaps(t *testing.T) { } } +// fakeRemounter records re-mount dispatches. +type fakeRemounter struct { + mu sync.Mutex + calls []string +} + +func (r *fakeRemounter) Remount(_ context.Context, t KnownTarget) { + r.mu.Lock() + defer r.mu.Unlock() + r.calls = append(r.calls, t.Name) +} +func (r *fakeRemounter) count() int { + r.mu.Lock() + defer r.mu.Unlock() + return len(r.calls) +} + +func TestWatchdog_ReMountOnDeviceReturn(t *testing.T) { + known := &staticKnown{targets: []KnownTarget{{Name: "usb", MountBacked: true, UUID: "1234-ABCD", MountPath: "/mnt/usb"}}} + live := &mapLiveness{present: map[string]bool{"usb": true}, device: map[string]bool{"usb": true}} + rem := &fakeRemounter{} + w, _, clock := newTestWatchdog(known, live, 30*time.Second) + w.remounter = rem + w.spawn = func(f func()) { f() } // run the dispatch synchronously for deterministic assertion + ctx := context.Background() + + w.tick(ctx) // baseline: present + if rem.count() != 0 { + t.Fatalf("no re-mount at baseline, got %d", rem.count()) + } + + // Drop: device gone, unmounted. No re-mount (nothing to mount). + live.set("usb", false) + live.setDevice("usb", false) + w.tick(ctx) + if rem.count() != 0 { + t.Fatalf("no re-mount while device absent, got %d", rem.count()) + } + + // Device returns but still unmounted → re-mount dispatched. + live.setDevice("usb", true) + w.tick(ctx) + if rem.count() != 1 { + t.Fatalf("re-mount expected when device returns unmounted, got %d", rem.count()) + } + + // Still device-present-unmounted within the debounce window → rate-limited (no storm). + *clock = clock.Add(5 * time.Second) + w.tick(ctx) + if rem.count() != 1 { + t.Fatalf("re-mount must be rate-limited within debounce, got %d", rem.count()) + } + + // Successful mount (present=true) clears the rate-limit; a later cycle re-mounts again. + live.set("usb", true) + *clock = clock.Add(5 * time.Second) + w.tick(ctx) // present → clears lastRemount + live.set("usb", false) // drop again, device still present + *clock = clock.Add(5 * time.Second) + w.tick(ctx) + if rem.count() != 2 { + t.Fatalf("a fresh device cycle should re-mount again, got %d", rem.count()) + } +} + func TestWatchdog_ReadErrorSkipsTick(t *testing.T) { known := &staticKnown{err: errors.New("proxmox blip")} live := &mapLiveness{present: map[string]bool{}}