diff --git a/CHANGELOG.md b/CHANGELOG.md index aac0a1e..02da763 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,32 @@ All notable changes to **felhom-agent** are recorded here. Update on every code change that gets pushed. +## v0.33.0 — C1 net: pre-start self-heal hook + decommission mp-delete (2026-06-15) + +The transitional defense for the C1 brick (B3 critical bug) ahead of the intermediary-mount +re-architecture (which makes C1 structural). Two independent nets: + +- **Pre-start self-heal hook** (`internal/guesthook`): a PVE `pre-start` hookscript runs + `felhom-agent guest-hook ` which, for every BIND mountpoint whose source path is + missing, creates an empty **host-root-owned** placeholder dir so the bind succeeds and the guest + always boots — fail-closed (host uid 0 is unmapped in the unprivileged-LXC userns, so the guest + can't write to the placeholder; a returning drive shadows it). It CREATES rather than DELETEs + because `pct set --delete` in pre-start would take the config lock the start task already holds + (dead-times-out → still bricks); the heal logic is in unit-tested Go, the wrapper just delegates. + Installed + registered per-guest by the provision back-half (`InstallSnippet`/`Register`). +- **Decommission mp-delete** (`GuestBinder.DetachBind` + `handleDiskDecommission`): decommission now + runs `pct set --delete mpN` on the slot binding the drive (lock-safe on the running guest), + so its now-missing source can't brick the next reboot. The old handler unmounted but left the dead + `mpN` in config — the exact B3 C1 bug. Eject keeps its mp (temporary; the hook covers a + reboot-while-ejected). + +Tests (non-hollow, each with a companion that fails the pre-fix/trivial impl): +`internal/guesthook/heal_test.go` (selector ignores storage volumes + present binds, heals only the +absent one; "return nothing"/"return all" both fail) and `TestDecommission_DeletesGuestMount` +(asserts the correct slot is `--delete`d; pre-fix never calls DetachBind → fails). + +Sudoers: new `FELHOM_GUESTHOOK` alias (snippet install, `pct set --hookscript`, `pct set --delete mpN`). + ## v0.32.0 — self-serve decommission + intent-aware re-assert (B2a) (2026-06-14) Customer-self-serve storage decommission (no operator signature; non-destructive — never formats), diff --git a/cmd/felhom-agent/main.go b/cmd/felhom-agent/main.go index a8a1ec3..eed3529 100644 --- a/cmd/felhom-agent/main.go +++ b/cmd/felhom-agent/main.go @@ -29,6 +29,7 @@ import ( "gitea.dooplex.hu/admin/felhom-agent/internal/config" "gitea.dooplex.hu/admin/felhom-agent/internal/desired" "gitea.dooplex.hu/admin/felhom-agent/internal/escrow" + "gitea.dooplex.hu/admin/felhom-agent/internal/guesthook" "gitea.dooplex.hu/admin/felhom-agent/internal/hub" "gitea.dooplex.hu/admin/felhom-agent/internal/lanresolver" "gitea.dooplex.hu/admin/felhom-agent/internal/localapi" @@ -43,9 +44,38 @@ 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.32.0" +var version = "0.33.0" + +// runGuestHook is the PVE pre-start hook body (`felhom-agent guest-hook `). On the +// pre-start phase it creates placeholder dirs for any absent bind-mount source so the guest always boots +// (the C1 net). It ALWAYS returns cleanly (exit 0) — a hook must never block a guest start. Heal output +// is written to stderr (PVE captures hook output into the task log). +func runGuestHook(args []string) { + if len(args) < 2 { + return + } + vmid, phase := args[0], args[1] + if phase != guesthook.PhasePreStart { + return + } + created, err := guesthook.Heal("/etc/pve/lxc/" + vmid + ".conf") + if len(created) > 0 { + fmt.Fprintf(os.Stderr, "felhom-agent guest-hook: vmid %s pre-start — created %d placeholder(s) for absent drive(s): %v\n", vmid, len(created), created) + } + if err != nil { + fmt.Fprintf(os.Stderr, "felhom-agent guest-hook: vmid %s heal error (boot continues): %v\n", vmid, err) + } +} func main() { + // Pre-start self-heal hook entrypoint. PVE invokes the registered hookscript as + // ` guest-hook `. Handled BEFORE flag parsing — it takes positional args, must be + // fast, needs no config/daemon/network, and must NEVER exit nonzero (a hook that fails would block + // the guest start). See internal/guesthook (the C1 net). + if len(os.Args) >= 2 && os.Args[1] == "guest-hook" { + runGuestHook(os.Args[2:]) + return + } var ( cfgPath string selftest selftestFlag diff --git a/configs/felhom-agent.sudoers b/configs/felhom-agent.sudoers index 5838ed2..a4d2063 100644 --- a/configs/felhom-agent.sudoers +++ b/configs/felhom-agent.sudoers @@ -61,4 +61,14 @@ Cmnd_Alias FELHOM_DNSMASQ = \ /usr/sbin/pct exec [0-9]* -- ip -4 -o addr show dev eth0, \ /usr/sbin/pct exec [0-9]* -- docker exec felhom-controller cat /opt/docker/felhom-controller/controller.yaml -felhom-agent ALL=(root) NOPASSWD: FELHOM_MOUNT, FELHOM_DISK, FELHOM_PROVISION, FELHOM_FORMAT, FELHOM_DNSMASQ +# Guest mountpoint lifecycle (intermediary-mount re-architecture + C1 net). The pre-start self-heal hook +# wrapper is installed once into the PVE snippets dir (from an agent-written /tmp file) and registered +# per-guest; decommission/eject DELETE the dead mountpoint slot so a missing bind source can't brick the +# guest at next boot (the B3 C1 fix). The agent fine-validates the vmid (numeric) + slot (mp[0-9]+) and +# the snippet path is fixed — the wildcards are the coarse allowlist. +Cmnd_Alias FELHOM_GUESTHOOK = \ + /usr/bin/install -m 0755 -- /tmp/felhom-guest-hook.sh /var/lib/vz/snippets/felhom-guest-hook.sh, \ + /usr/sbin/pct set [0-9]* --hookscript local\:snippets/felhom-guest-hook.sh, \ + /usr/sbin/pct set [0-9]* --delete mp[0-9]* + +felhom-agent ALL=(root) NOPASSWD: FELHOM_MOUNT, FELHOM_DISK, FELHOM_PROVISION, FELHOM_FORMAT, FELHOM_DNSMASQ, FELHOM_GUESTHOOK diff --git a/internal/guesthook/heal.go b/internal/guesthook/heal.go new file mode 100644 index 0000000..63528b7 --- /dev/null +++ b/internal/guesthook/heal.go @@ -0,0 +1,135 @@ +// Package guesthook is the LXC guest pre-start self-heal (C1 net, transitional). +// +// THE BUG IT FIXES (C1, B3 audit): in the per-drive bind model an external data drive is bound into the +// guest as `pct set -mpN /felhom-data,mp=/mnt/`. When that drive is ABSENT at guest +// boot, the bind SOURCE `/felhom-data` does not exist, `pct start` fails the mount, and the guest +// BRICKS (pre-start exit 255 — ALL apps down). Today nothing recovers it. +// +// THE FIX: a PVE `pre-start` hookscript runs this code; for every BIND mountpoint whose source path is +// missing it CREATES an empty, host-root-owned placeholder directory so the mount succeeds and the guest +// boots. It is fail-closed: the placeholder is owned by host root (uid 0), which is UNMAPPED in the +// unprivileged-LXC user namespace, so the in-guest controller/apps (even as guest-root) cannot write to +// it — and a returning drive simply shadows it (the agent mounts over it). +// +// WHY CREATE, NOT DELETE: removing the dead mp would need `pct set --delete mpN`, which takes the +// per-guest config lock the start task ALREADY holds → it dead-times-out (~10s) and the guest still +// bricks. So in pre-start we NEUTRALISE (placeholder) rather than mutate config. Proper mp removal runs +// OUTSIDE the start lock — at decommission (handleDiskDecommission → DetachBind) and the startup +// reconcile. The intermediary-mount re-architecture later makes C1 STRUCTURAL (the only bind source is +// the permanent, always-present /mnt/felhom-drives parent), after which this hook is pure defense-in-depth. +package guesthook + +import ( + "fmt" + "os" + "path/filepath" + "sort" + "strings" +) + +// PhasePreStart is the PVE hook phase at which we self-heal (before the container mounts are set up). +const PhasePreStart = "pre-start" + +// placeholderMode is the mode for a created bind-source placeholder. Host-root-owned + this mode = +// fail-closed against the unprivileged guest (host uid 0 is unmapped in the guest userns). +const placeholderMode = 0o755 + +// ParseConfMounts parses an LXC config file body (/etc/pve/lxc/.conf) and returns each mount key +// (`mp0`..`mp255`, plus `rootfs`) mapped to its SOURCE — the first comma-field of the value, before any +// `mp=`/`size=`/`backup=` options. A BIND mount has an absolute-path source (`/mnt/...`); a storage +// volume has a `:` source (no leading slash). Lines that aren't a mountpoint/rootfs key +// are ignored. +func ParseConfMounts(conf string) map[string]string { + out := map[string]string{} + for _, line := range strings.Split(conf, "\n") { + line = strings.TrimSpace(line) + colon := strings.IndexByte(line, ':') + if colon <= 0 { + continue + } + key := line[:colon] + if key != "rootfs" && !(strings.HasPrefix(key, "mp") && isAllDigits(strings.TrimPrefix(key, "mp"))) { + continue + } + val := strings.TrimSpace(line[colon+1:]) + if val == "" { + continue + } + src := val + if c := strings.IndexByte(val, ','); c >= 0 { + src = val[:c] + } + out[key] = strings.TrimSpace(src) + } + return out +} + +// isBindSource reports whether an mp source is a host-path BIND (an absolute path) rather than a PVE +// storage volume (`:`, never absolute). On the Linux host a bind source is `/mnt/...` +// (leading slash); the filepath.IsAbs arm additionally recognises an OS-absolute path so the real-IO +// tests pass under a Windows temp dir too — on Linux both arms agree and a storage volid matches neither. +func isBindSource(src string) bool { + return strings.HasPrefix(src, "/") || filepath.IsAbs(src) +} + +func isAllDigits(s string) bool { + if s == "" { + return false + } + for _, c := range s { + if c < '0' || c > '9' { + return false + } + } + return true +} + +// MissingBindSources returns the BIND-mount source paths (absolute host paths) that do NOT exist, sorted +// and de-duplicated. Storage-volume sources (`:`, no leading '/') are NEVER returned — +// only a real host-path bind can have a vanished source we must heal; a storage volume that's missing is +// PVE's own concern, not ours to mkdir. `exists` reports whether a path is present (injected for tests). +func MissingBindSources(mounts map[string]string, exists func(string) bool) []string { + seen := map[string]bool{} + var miss []string + for _, src := range mounts { + if !isBindSource(src) { // storage volume (:), not a host-path bind — never touch + continue + } + if seen[src] || exists(src) { + continue + } + seen[src] = true + miss = append(miss, src) + } + sort.Strings(miss) + return miss +} + +// Heal reads the LXC config at confPath and creates a placeholder directory for every bind-mount source +// that is missing, returning the list of paths it created. It never returns a fatal error for an +// unreadable/empty config (a guest with no config simply has nothing to heal) — the hook must NEVER block +// a start. A mkdir failure on one path is collected into err but the others still proceed. +func Heal(confPath string) (created []string, err error) { + data, readErr := os.ReadFile(confPath) + if readErr != nil { + // No config = nothing to heal. Never block the start over a read error. + return nil, nil + } + mounts := ParseConfMounts(string(data)) + miss := MissingBindSources(mounts, func(p string) bool { + _, statErr := os.Stat(p) + return statErr == nil + }) + var errs []string + for _, p := range miss { + if mkErr := os.MkdirAll(p, placeholderMode); mkErr != nil { + errs = append(errs, fmt.Sprintf("%s: %v", p, mkErr)) + continue + } + created = append(created, p) + } + if len(errs) > 0 { + return created, fmt.Errorf("guesthook: placeholder creation failed for: %s", strings.Join(errs, "; ")) + } + return created, nil +} diff --git a/internal/guesthook/heal_test.go b/internal/guesthook/heal_test.go new file mode 100644 index 0000000..0cbcade --- /dev/null +++ b/internal/guesthook/heal_test.go @@ -0,0 +1,118 @@ +package guesthook + +import ( + "os" + "path/filepath" + "reflect" + "testing" +) + +const sampleConf = `arch: amd64 +cores: 2 +hostname: demo-felhom +memory: 12288 +mp0: local-lvm:vm-9201-disk-1,mp=/var/lib/docker,backup=1,size=256G +mp1: /mnt/felhom-usb/felhom-data,mp=/mnt/felhom-usb +mp2: /mnt/felhom-flash/felhom-data,mp=/mnt/felhom-flash +mp9: /var/lib/felhom-agent/guests/9201/bootstrap,mp=/etc/felhom-bootstrap,ro=1 +net0: name=eth0,bridge=vmbr0 +rootfs: local-lvm:vm-9201-disk-0,size=32G +swap: 4096 +unprivileged: 1 +` + +func TestParseConfMounts(t *testing.T) { + got := ParseConfMounts(sampleConf) + want := map[string]string{ + "mp0": "local-lvm:vm-9201-disk-1", + "mp1": "/mnt/felhom-usb/felhom-data", + "mp2": "/mnt/felhom-flash/felhom-data", + "mp9": "/var/lib/felhom-agent/guests/9201/bootstrap", + "rootfs": "local-lvm:vm-9201-disk-0", + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("ParseConfMounts mismatch:\n got=%v\nwant=%v", got, want) + } + // net0/arch/etc. (non-mount keys) must NOT leak in. + if _, bad := got["net0"]; bad { + t.Fatalf("net0 was parsed as a mount source") + } +} + +// TestMissingBindSources is the load-bearing selector test. The flash drive is absent (its felhom-data +// source missing); usb is present; mp0/rootfs are STORAGE volumes (must never be selected); mp9's +// bootstrap source is present. Only the flash source may be returned. +// +// COMPANION GUARD — this test FAILS on the two trivial impls the spec warns about: +// - "return nothing" (the pre-fix no-op hook) → flash not selected → guest still bricks → FAIL. +// - "return every source" (mkdir everything) → would include the present usb bind AND the +// local-lvm storage volumes (creating bogus dirs that shadow real data) → FAIL. +func TestMissingBindSources(t *testing.T) { + mounts := ParseConfMounts(sampleConf) + present := map[string]bool{ + "/mnt/felhom-usb/felhom-data": true, // usb attached + "/var/lib/felhom-agent/guests/9201/bootstrap": true, // bootstrap always present + // "/mnt/felhom-flash/felhom-data" is ABSENT (drive unplugged) + } + got := MissingBindSources(mounts, func(p string) bool { return present[p] }) + want := []string{"/mnt/felhom-flash/felhom-data"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("MissingBindSources mismatch:\n got=%v\nwant=%v", got, want) + } + + // Explicit companion assertions (pin both failure directions independently of want): + for _, p := range got { + if p == "/mnt/felhom-usb/felhom-data" { + t.Fatalf("selected a PRESENT bind source — over-eager (would shadow live data)") + } + if p == "local-lvm:vm-9201-disk-1" || p == "local-lvm:vm-9201-disk-0" { + t.Fatalf("selected a STORAGE VOLUME source — must only heal host-path binds") + } + } + if len(got) == 0 { + t.Fatalf("selected nothing — the absent flash bind would brick the guest (pre-fix no-op)") + } +} + +// TestHealCreatesOnlyMissingBind drives the real filesystem path in a temp dir: a present bind source is +// left untouched, an absent one is created (so the guest boots), a storage volume is never created. +func TestHealCreatesOnlyMissingBind(t *testing.T) { + root := t.TempDir() + presentSrc := filepath.Join(root, "usb", "felhom-data") + absentSrc := filepath.Join(root, "flash", "felhom-data") + if err := os.MkdirAll(presentSrc, 0o755); err != nil { + t.Fatal(err) + } + conf := "" + + "mp0: local-lvm:vm-9-disk-0,mp=/var/lib/docker,backup=1\n" + + "mp1: " + presentSrc + ",mp=/mnt/usb\n" + + "mp2: " + absentSrc + ",mp=/mnt/flash\n" + + "rootfs: local-lvm:vm-9-disk-1,size=32G\n" + confPath := filepath.Join(root, "9.conf") + if err := os.WriteFile(confPath, []byte(conf), 0o644); err != nil { + t.Fatal(err) + } + + created, err := Heal(confPath) + if err != nil { + t.Fatalf("Heal: %v", err) + } + if !reflect.DeepEqual(created, []string{absentSrc}) { + t.Fatalf("Heal created %v, want [%s]", created, absentSrc) + } + if _, err := os.Stat(absentSrc); err != nil { + t.Fatalf("absent bind source not created — guest would still brick: %v", err) + } + // A storage-volume mp must never produce a bogus host directory. + if _, err := os.Stat(filepath.Join(root, "local-lvm:vm-9-disk-0")); err == nil { + t.Fatalf("a storage volume source was materialised as a directory") + } +} + +// TestHealMissingConfNeverErrors — a hook must never block a start, even for an unreadable config. +func TestHealMissingConfNeverErrors(t *testing.T) { + created, err := Heal(filepath.Join(t.TempDir(), "does-not-exist.conf")) + if err != nil || created != nil { + t.Fatalf("Heal on missing conf: created=%v err=%v (want nil,nil)", created, err) + } +} diff --git a/internal/guesthook/install.go b/internal/guesthook/install.go new file mode 100644 index 0000000..4138067 --- /dev/null +++ b/internal/guesthook/install.go @@ -0,0 +1,59 @@ +package guesthook + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strconv" + + "gitea.dooplex.hu/admin/felhom-agent/internal/proxmox" +) + +// Install/registration of the pre-start hook. The wrapper lives in a PVE `snippets`-enabled storage dir +// (the `local` storage maps to /var/lib/vz/snippets) and is referenced per-guest by its volid. +const ( + // SnippetDir is the local-storage snippets directory PVE serves hookscripts from. + SnippetDir = "/var/lib/vz/snippets" + // SnippetName is the wrapper filename. + SnippetName = "felhom-guest-hook.sh" + // HookVolID is the volid form `pct set --hookscript` expects. + HookVolID = "local:snippets/" + SnippetName + // AgentBin is the installed agent binary the wrapper delegates to. + AgentBin = "/usr/local/bin/felhom-agent" +) + +// SnippetPath is the absolute path of the installed wrapper. +var SnippetPath = filepath.Join(SnippetDir, SnippetName) + +// snippetBody is the tiny wrapper PVE execs as `