From 474b858c0be0cd7f615a28ef379b71b3040d65ce Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Sat, 11 Jul 2026 20:46:59 +0200 Subject: [PATCH] =?UTF-8?q?v0.84.0:=20ReassertNetworkMounts=20=E2=80=94=20?= =?UTF-8?q?NAS=20automount=20survives=20guest=20reboots=20(RCA=20fix=201)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Storage §8 decision table (stop + enable --now on idle triggers; active mounts untouched), daemon leg at startup with per-running-guest visibility verify, guest-hook post-start leg (root, direct systemctl, non-fatal). Red-proofs: always-rearm table FAIL; unwired hook FAIL. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PSK5g6qYLknKj8u3QAFEr6 --- CHANGELOG.md | 32 +++++ REUSE.md | 1 + cmd/felhom-agent/guesthook_wiring_test.go | 33 +++++ cmd/felhom-agent/main.go | 42 ++++-- internal/guesthook/heal.go | 4 + internal/guesthook/netreassert.go | 81 +++++++++++ internal/guesthook/netreassert_test.go | 56 ++++++++ internal/localapi/netreassert.go | 59 ++++++++ internal/localapi/netreassert_test.go | 103 ++++++++++++++ internal/storage/netmount.go | 54 ++++++-- internal/storage/netreassert.go | 117 ++++++++++++++++ internal/storage/netreassert_test.go | 160 ++++++++++++++++++++++ 12 files changed, 715 insertions(+), 27 deletions(-) create mode 100644 cmd/felhom-agent/guesthook_wiring_test.go create mode 100644 internal/guesthook/netreassert.go create mode 100644 internal/guesthook/netreassert_test.go create mode 100644 internal/localapi/netreassert.go create mode 100644 internal/localapi/netreassert_test.go create mode 100644 internal/storage/netreassert.go create mode 100644 internal/storage/netreassert_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index cb424fb..9a8a6a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,35 @@ +## v0.84.0 — ReassertNetworkMounts: NAS automount survives guest reboots (RCA fix 1) (2026-07-11) + +Agent half of the RCA fix pair (controller v0.117.0). Source: +`felhom.eu/documentation/audits/AUDIT-nas-cwa-rca-2026-07-11.md` — a fresh guest namespace inherits +REAL submounts (ext4/nfs4) but NOT an idle autofs trigger, so after any guest reboot an idle NAS +share silently degrades to a local stub inside the guest. The heal (re-create the automount → the +fresh trigger-mount event propagates live into running guests) was live-proven in the RCA +remediation; this release makes it automatic. + +- **`internal/storage/netreassert.go`** `SudoHostOps.ReassertNetworkAutomounts`: per configured + network mount, the §8 decision table — real nfs/nfs4/cifs mounted → skip (inherited); `autofs` + trigger at the path → **stop + enable --now the `.automount`** (existing FELHOM_NETMOUNT sudoers + verbs; there is NO restart grant); neither → skip (removed/orphan states owned by add/remove). + Idempotent; per-share errors never stop the pass. Unit enumeration factored into + `networkUnitEntries()` (shared with `ListNetworkMounts`, behavior unchanged). +- **`internal/localapi/netreassert.go`** `Server.ReassertNetworkMounts`: the daemon leg — runs the + host-global pass once, then best-effort verifies each RUNNING guest actually sees each share path + (`GuestSeesMount`; the RCA's masking lesson). Type-asserted capability (the lean + `NetworkStorageOps` interface and its fakes stay unchanged — the main.go + `ReassertEnrolledMounts` pattern). Wired at agent startup after `ReassertGuestBinds`; + deliberately NOT in the 20 s ticker (an idle trigger is healthy and must not be churned). +- **`internal/guesthook/netreassert.go`** + `PhasePostStart`: the hook leg — PVE runs the hookscript + as root, so `guest-hook post-start` re-arms triggers with DIRECT systemctl and verifies + via `GuestSeesPath` (hook-process mirror of GuestSeesMount). Non-fatal by contract (stderr → PVE + task log; exit 0 always); 30 s bound. The installed wrapper snippet already forwards all phases — + no snippet re-install needed. +- Tests + red-proofs: §8 table (red-proof: always-rearm shape → FAIL "nfs → rearmed, want + skip-active" — the live-mount churn the table prevents); rearm emits EXACTLY stop+enable-now on + the right unit; active mount → ZERO systemctl calls; idempotent double-pass; hook wiring + (red-proof: PhasePostStart case removed → FAIL "got []"); daemon leg verifies running guests + only; invisible-share verify is WARN-only (non-fatal). + ## v0.83.0 — observability pass: always-DEBUG capture ring + GET /debug/logs + heartbeat log-pull + gap-fill sweep (2026-07-11) Agent half of the cross-repo observability task (controller v0.116.0 + hub v0.46.0). Motivating diff --git a/REUSE.md b/REUSE.md index ee53b6c..f26cdf6 100644 --- a/REUSE.md +++ b/REUSE.md @@ -50,6 +50,7 @@ | `GuestBinder.GuestSeesMount` / `GuestBootID` | internal/localapi/intermediary.go | `GuestSeesMount(ctx, vmid, path) bool` | guest-visible (usable) signal; reboot detection | Host bind present ≠ guest sees it (non-recursive parent bind) | | `SudoHostOps.EnsureNetworkMount` / `RemoveNetworkMount` / `ListNetworkMounts` | internal/storage/netmount.go | `EnsureNetworkMount(ctx, spec) error` | NAS automount pair | rm glob confined to `mnt-felhom*` units; NAS ≠ drive (no durable-id/SMART/wipe); RemoveNetworkMount doubles as the verify-fail rollback (idempotent) | | `NetworkMountedAt` / `NetworkEndpointReachable` | internal/storage/netmount.go | `NetworkMountedAt(where) bool` | verify mount-truth + the 2 s add pre-probe | /proc/mounts is the ONLY mount-success judge (autofs trigger ≠ mounted; readability ≠ mounted — SPIKE-nas-verify §8) | +| `SudoHostOps.ReassertNetworkAutomounts` + `Server.ReassertNetworkMounts` + `guesthook.PostStartNetworkReassert` | internal/storage/netreassert.go, internal/localapi/netreassert.go, internal/guesthook/netreassert.go | `ReassertNetworkAutomounts(ctx) []NetReassertResult` | NAS guest-reboot heal (RCA fix 1): re-arm idle automount triggers (stop + enable --now) so the fresh mount event propagates into running guests | NEVER call from periodic health paths (an idle trigger is HEALTHY); active real mounts are never touched; hook leg runs as root (direct systemctl), daemon leg via sudo | | `ClassifyNetVerifyFailure` | internal/storage/netverify.go | `ClassifyNetVerifyFailure(journalTail, tcpReachable) (code, hint)` | NAS verify failure categories | String-based BY DESIGN (every mount failure is rc=32); substrings verbatim from SPIKE-nas-verify Q4; `nfs_export` merges not-found/not-permitted (NFSv4 identical) | ### Durable stores (atomic state) diff --git a/cmd/felhom-agent/guesthook_wiring_test.go b/cmd/felhom-agent/guesthook_wiring_test.go new file mode 100644 index 0000000..1f6b891 --- /dev/null +++ b/cmd/felhom-agent/guesthook_wiring_test.go @@ -0,0 +1,33 @@ +package main + +import ( + "context" + "testing" +) + +// The hook WIRING red-proof target: `guest-hook post-start` must invoke the network +// reassert with the vmid; pre-start and unknown phases must NOT. (Companion red-proof: remove the +// PhasePostStart case from runGuestHook → the invoked assertion fails.) +func TestRunGuestHook_PostStartInvokesNetworkReassert(t *testing.T) { + orig := postStartNetworkReassertFn + t.Cleanup(func() { postStartNetworkReassertFn = orig }) + + var gotVMIDs []string + postStartNetworkReassertFn = func(_ context.Context, vmid string) { + gotVMIDs = append(gotVMIDs, vmid) + } + + runGuestHook([]string{"9201", "post-start"}) + if len(gotVMIDs) != 1 || gotVMIDs[0] != "9201" { + t.Fatalf("post-start must invoke the network reassert with vmid 9201, got %v", gotVMIDs) + } + + // pre-start must not touch the network reassert (it is the placeholder-heal phase; the heal + // no-ops on a nonexistent config path and never blocks). + runGuestHook([]string{"9201", "pre-start"}) + // unknown phases are ignored entirely. + runGuestHook([]string{"9201", "pre-stop"}) + if len(gotVMIDs) != 1 { + t.Fatalf("only post-start may invoke the network reassert, got %v", gotVMIDs) + } +} diff --git a/cmd/felhom-agent/main.go b/cmd/felhom-agent/main.go index 20696c2..b70ea2f 100644 --- a/cmd/felhom-agent/main.go +++ b/cmd/felhom-agent/main.go @@ -54,27 +54,38 @@ import ( // -ldflags "-X main.version="; defaults to the in-repo CHANGELOG version. var version = "0.63.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). +// runGuestHook is the PVE hook body (`felhom-agent guest-hook `). On pre-start it +// creates placeholder dirs for any absent bind-mount source so the guest always boots (the C1 net); +// on post-start it re-arms idle NAS automount triggers so the fresh guest namespace sees network +// shares again (RCA fix 1 — a new namespace inherits real mounts but not idle autofs triggers). +// It ALWAYS returns cleanly (exit 0) — a hook must never block a guest start. 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) + switch phase { + case guesthook.PhasePreStart: + 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) + } + case guesthook.PhasePostStart: + // Bounded: a hook must be fast; a wedged systemd call must not hold the start task. + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + postStartNetworkReassertFn(ctx, vmid) } } +// postStartNetworkReassertFn is the post-start hook action (seam — tests assert the wiring without +// touching real systemctl). +var postStartNetworkReassertFn = guesthook.PostStartNetworkReassert + 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 @@ -784,6 +795,11 @@ func runDaemon(cfg config.Config, logger *slog.Logger, logRing *applog.Ring) int // bind that a re-provision dropped — before serving, so the drive is back in the guest config // (activates on the guest's next reboot). On-durable-id-match; absent/swapped drives are skipped. localSrv.ReassertGuestBinds(ctx) + // RCA fix 1 (AUDIT-nas-cwa-rca-2026-07-11): re-arm idle NAS automount triggers ONCE at startup — + // covers the host-boot ordering where guests autostarted before the agent (their fresh namespaces + // missed the triggers). Deliberately NOT in the 20 s ticker: an idle trigger is healthy and must + // not be churned; guest starts are covered by the guest-hook post-start leg. + localSrv.ReassertNetworkMounts(ctx) // Intermediary-mount GUEST-REBOOT self-heal: re-run the reconcile periodically. A guest reboot // (without an agent restart) leaves enrolled drives bound on the HOST but INVISIBLE in the fresh // guest namespace (a non-recursive parent bind doesn't carry pre-existing submounts); the periodic diff --git a/internal/guesthook/heal.go b/internal/guesthook/heal.go index 63528b7..f80bccb 100644 --- a/internal/guesthook/heal.go +++ b/internal/guesthook/heal.go @@ -30,6 +30,10 @@ import ( // PhasePreStart is the PVE hook phase at which we self-heal (before the container mounts are set up). const PhasePreStart = "pre-start" +// PhasePostStart is the PVE hook phase after the container started — the NAS automount reassert +// point (the fresh guest namespace has no idle autofs triggers; see netreassert.go). +const PhasePostStart = "post-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 diff --git a/internal/guesthook/netreassert.go b/internal/guesthook/netreassert.go new file mode 100644 index 0000000..069ca2c --- /dev/null +++ b/internal/guesthook/netreassert.go @@ -0,0 +1,81 @@ +package guesthook + +import ( + "context" + "fmt" + "log/slog" + "os" + "strings" + + "gitea.dooplex.hu/admin/felhom-agent/internal/proxmox" + "gitea.dooplex.hu/admin/felhom-agent/internal/storage" +) + +// post-start network-storage reassert (RCA AUDIT-nas-cwa-rca-2026-07-11 fix 1, hook leg). +// +// A freshly started guest's namespace does NOT inherit an idle NAS autofs trigger (only real +// mounts), so its /mnt/felhom-drives/ path is a silent local stub until the trigger is +// re-created host-side. PVE runs the hookscript as root in the start task, so this leg calls +// systemctl DIRECTLY (no sudo) — the daemon leg (localapi.ReassertNetworkMounts) is the sudo path. +// Like the pre-start heal, this must NEVER fail the hook: all errors go to stderr (the PVE task +// log) and the guest start proceeds regardless. + +// netReasserter is the reassert capability (satisfied by *storage.SudoHostOps; faked in tests). +type netReasserter interface { + ReassertNetworkAutomounts(ctx context.Context) []storage.NetReassertResult +} + +// PostStartNetworkReassert re-arms idle NAS automount triggers after vmid started, then verifies +// the (now running) guest actually sees each share path. Best-effort throughout. +func PostStartNetworkReassert(ctx context.Context, vmid string) { + runner := &proxmox.ExecRunner{Mode: proxmox.RunnerDirect} + ops := storage.NewSudoHostOps(storage.SudoHostOpsConfig{ + Runner: runner, + Logger: slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelInfo})), + }) + postStartNetworkReassert(ctx, vmid, ops, func(ctx context.Context, vmid, path string) bool { + return GuestSeesPath(ctx, runner, vmid, path) + }) +} + +// postStartNetworkReassert is the seam-injected core (unit-tested; the wrapper above binds the +// real host surface). +func postStartNetworkReassert(ctx context.Context, vmid string, ops netReasserter, sees func(ctx context.Context, vmid, path string) bool) { + for _, res := range ops.ReassertNetworkAutomounts(ctx) { + if res.Err != nil || res.Action == storage.NetReassertSkipNone { + continue // already reported by the ops logger / nothing expected in the guest + } + if sees(ctx, vmid, res.Where) { + fmt.Fprintf(os.Stderr, "felhom-agent guest-hook: vmid %s post-start — network share %s visible in guest (%s)\n", + vmid, res.Name, res.Action) + } else { + fmt.Fprintf(os.Stderr, "felhom-agent guest-hook: vmid %s post-start — WARNING: network share %s NOT visible in guest after reassert (%s)\n", + vmid, res.Name, res.Action) + } + } +} + +// GuestSeesPath reports whether vmid's guest has `path` as a mount target in its own namespace — +// the hook-process mirror of localapi's GuestBinder.GuestSeesMount (which is method-bound to the +// daemon's binder and unavailable here). Resolution/read errors → false. +func GuestSeesPath(ctx context.Context, runner proxmox.Runner, vmid, path string) bool { + out, _, err := runner.Run(ctx, "lxc-info", "-n", vmid, "-p", "-H") + if err != nil { + return false + } + pid := strings.TrimSpace(string(out)) + if pid == "" { + return false + } + data, err := os.ReadFile("/proc/" + pid + "/mountinfo") + if err != nil { + return false + } + for _, line := range strings.Split(string(data), "\n") { + f := strings.Fields(line) + if len(f) >= 5 && f[4] == path { + return true + } + } + return false +} diff --git a/internal/guesthook/netreassert_test.go b/internal/guesthook/netreassert_test.go new file mode 100644 index 0000000..6291cbc --- /dev/null +++ b/internal/guesthook/netreassert_test.go @@ -0,0 +1,56 @@ +package guesthook + +import ( + "context" + "testing" + + "gitea.dooplex.hu/admin/felhom-agent/internal/storage" +) + +type fakeReasserter struct { + invoked int + results []storage.NetReassertResult +} + +func (f *fakeReasserter) ReassertNetworkAutomounts(context.Context) []storage.NetReassertResult { + f.invoked++ + return f.results +} + +// The post-start core must run the reassert pass and verify guest visibility for every share the +// pass acted on (or found actively mounted) — and never for skip-none/errored rows. +func TestPostStartNetworkReassert_Core(t *testing.T) { + ops := &fakeReasserter{results: []storage.NetReassertResult{ + {Name: "media", Where: "/mnt/felhom-drives/media", Action: storage.NetReassertRearmed}, + {Name: "active", Where: "/mnt/felhom-drives/active", Action: storage.NetReassertSkipActive}, + {Name: "gone", Where: "/mnt/felhom-drives/gone", Action: storage.NetReassertSkipNone}, + }} + var verified []string + postStartNetworkReassert(context.Background(), "9201", ops, func(_ context.Context, vmid, path string) bool { + if vmid != "9201" { + t.Errorf("verify called with vmid %q, want 9201", vmid) + } + verified = append(verified, path) + return true + }) + if ops.invoked != 1 { + t.Fatalf("reassert pass invoked %d times, want 1", ops.invoked) + } + if len(verified) != 2 || verified[0] != "/mnt/felhom-drives/media" || verified[1] != "/mnt/felhom-drives/active" { + t.Fatalf("verify must cover rearmed + skip-active only, got %v", verified) + } +} + +// A failed verify must be non-fatal: the core returns normally (the hook exits 0 regardless). +func TestPostStartNetworkReassert_VerifyFailureNonFatal(t *testing.T) { + ops := &fakeReasserter{results: []storage.NetReassertResult{ + {Name: "media", Where: "/mnt/felhom-drives/media", Action: storage.NetReassertRearmed}, + }} + // Must not panic or abort; the WARNING goes to stderr (PVE task log). + postStartNetworkReassert(context.Background(), "9201", ops, func(context.Context, string, string) bool { + return false + }) + if ops.invoked != 1 { + t.Fatalf("reassert pass invoked %d times, want 1", ops.invoked) + } +} diff --git a/internal/localapi/netreassert.go b/internal/localapi/netreassert.go new file mode 100644 index 0000000..9f49223 --- /dev/null +++ b/internal/localapi/netreassert.go @@ -0,0 +1,59 @@ +package localapi + +import ( + "context" + + "gitea.dooplex.hu/admin/felhom-agent/internal/storage" +) + +// networkReasserter is the optional concrete capability of the netStorage surface (satisfied by +// *storage.SudoHostOps). Type-asserted like main.go's ReassertEnrolledMounts so the lean +// NetworkStorageOps interface (and its test fakes) stay unchanged. +type networkReasserter interface { + ReassertNetworkAutomounts(ctx context.Context) []storage.NetReassertResult +} + +// ReassertNetworkMounts is the NAS counterpart of ReassertGuestBinds (RCA +// AUDIT-nas-cwa-rca-2026-07-11 fix 1): re-arm every idle network-share automount trigger host-side +// (a fresh trigger-mount event propagates live into running guests' slave binds), then best-effort +// verify each running guest actually sees each share's path. Runs at agent startup (after the +// guest-bind reconcile — covers guests that autostarted before the agent) and from the guest-hook +// post-start phase (its own process; this method is the daemon path). NEVER call this from a +// periodic health path — an idle trigger is healthy and must not be churned. +func (s *Server) ReassertNetworkMounts(ctx context.Context) { + if s.netStorage == nil { + return + } + r, ok := s.netStorage.(networkReasserter) + if !ok { + s.logger.Debug("netreassert: storage surface has no reassert capability — skipping") + return + } + results := r.ReassertNetworkAutomounts(ctx) + if len(results) == 0 { + return + } + // Best-effort guest-visibility verify (the RCA's masking lesson: host-side health said nothing + // about what the guests see). Only running guests; a failed verify is a WARN, never an error — + // the next guest start re-runs the hook path anyway. + if s.guestBinds == nil || s.guestAttach == nil { + return + } + for vmid := range s.guestBinds.Guests() { + if s.guestAttach.GuestBootID(ctx, vmid) == "" { + s.logger.Debug("netreassert: guest not running — visibility verify skipped", "vmid", vmid) + continue + } + for _, res := range results { + if res.Action == storage.NetReassertSkipNone || res.Err != nil { + continue // nothing expected in the guest for these rows + } + if s.guestAttach.GuestSeesMount(ctx, vmid, res.Where) { + s.logger.Debug("netreassert: guest sees network share", "vmid", vmid, "name", res.Name, "where", res.Where) + } else { + s.logger.Warn("netreassert: guest does NOT see network share after reassert", + "vmid", vmid, "name", res.Name, "where", res.Where, "action", res.Action) + } + } + } +} diff --git a/internal/localapi/netreassert_test.go b/internal/localapi/netreassert_test.go new file mode 100644 index 0000000..3b4d7a8 --- /dev/null +++ b/internal/localapi/netreassert_test.go @@ -0,0 +1,103 @@ +package localapi + +import ( + "context" + "testing" + + "gitea.dooplex.hu/admin/felhom-agent/internal/storage" +) + +// fakeNetOpsReassert is fakeNetOps plus the type-asserted reassert capability. +type fakeNetOpsReassert struct { + fakeNetOps + reassertN int + results []storage.NetReassertResult +} + +func (f *fakeNetOpsReassert) ReassertNetworkAutomounts(context.Context) []storage.NetReassertResult { + f.reassertN++ + return f.results +} + +// seeingAttacher records GuestSeesMount calls and controls per-vmid running state. +type seeingAttacher struct { + fakeGuestAttacher + running map[int]bool + seen []struct { + vmid int + path string + } + sees bool +} + +func (s *seeingAttacher) GuestBootID(_ context.Context, vmid int) string { + if s.running[vmid] { + return "boot-1" + } + return "" +} + +func (s *seeingAttacher) GuestSeesMount(_ context.Context, vmid int, path string) bool { + s.seen = append(s.seen, struct { + vmid int + path string + }{vmid, path}) + return s.sees +} + +// The daemon leg: reassert runs once, then visibility is verified per RUNNING guest per acted +// share; stopped guests are skipped entirely. +func TestReassertNetworkMounts_VerifiesRunningGuestsOnly(t *testing.T) { + n := &fakeNetOpsReassert{results: []storage.NetReassertResult{ + {Name: "media", Where: "/mnt/felhom-drives/media", Action: storage.NetReassertRearmed}, + }} + srv := newNetServer(t, n, t.TempDir()) + ga := &seeingAttacher{running: map[int]bool{8200: true, 9300: false}, sees: true} + srv.guestAttach = ga + gb := tempBindStore(t) + if err := gb.Record(8200, "uuid:aaa"); err != nil { + t.Fatal(err) + } + if err := gb.Record(9300, "uuid:bbb"); err != nil { + t.Fatal(err) + } + srv.guestBinds = gb + + srv.ReassertNetworkMounts(context.Background()) + + if n.reassertN != 1 { + t.Fatalf("host-side reassert must run exactly once (host-global), ran %d", n.reassertN) + } + if len(ga.seen) != 1 || ga.seen[0].vmid != 8200 || ga.seen[0].path != "/mnt/felhom-drives/media" { + t.Fatalf("visibility verify must cover the RUNNING guest only, got %+v", ga.seen) + } +} + +// A guest that does NOT see the share after reassert is a WARN, never an error — the method +// returns normally (non-fatal proof) and the pass still ran. +func TestReassertNetworkMounts_InvisibleShareNonFatal(t *testing.T) { + n := &fakeNetOpsReassert{results: []storage.NetReassertResult{ + {Name: "media", Where: "/mnt/felhom-drives/media", Action: storage.NetReassertRearmed}, + }} + srv := newNetServer(t, n, t.TempDir()) + ga := &seeingAttacher{running: map[int]bool{8200: true}, sees: false} + srv.guestAttach = ga + gb := tempBindStore(t) + if err := gb.Record(8200, "uuid:aaa"); err != nil { + t.Fatal(err) + } + srv.guestBinds = gb + + srv.ReassertNetworkMounts(context.Background()) // must not panic / abort + + if n.reassertN != 1 || len(ga.seen) != 1 { + t.Fatalf("pass must complete despite invisible share: reassert=%d seen=%+v", n.reassertN, ga.seen) + } +} + +// A netStorage surface WITHOUT the reassert capability (the lean interface, e.g. plain fakes) is a +// clean no-op — the type-assert gate. +func TestReassertNetworkMounts_NoCapabilityNoOp(t *testing.T) { + srv := newNetServer(t, &fakeNetOps{}, t.TempDir()) + srv.ReassertNetworkMounts(context.Background()) // must not panic +} diff --git a/internal/storage/netmount.go b/internal/storage/netmount.go index 7b4c7b2..93d456d 100644 --- a/internal/storage/netmount.go +++ b/internal/storage/netmount.go @@ -450,12 +450,43 @@ func (h *SudoHostOps) RemoveNetworkMount(ctx context.Context, name string) error // timeout — it NEVER stat()s the (possibly EIO/D-state) mountpoint, so a black-holed NAS cannot wedge // this call. Best-effort: an unreadable unit dir yields an empty list. func (h *SudoHostOps) ListNetworkMounts(ctx context.Context) ([]NetworkMountStatus, error) { + entries, err := h.networkUnitEntries() + if err != nil { + return nil, err + } + mounts := h.mountedFSTypes() + var out []NetworkMountStatus + for _, e := range entries { + st := NetworkMountStatus{ + Name: e.name, + Protocol: e.proto, + Server: e.server, + Export: e.export, + Where: e.where, + Configured: true, + Mounted: isNetworkMounted(mounts[e.where]), + Reachable: endpointReachable(netEndpoint(e.proto, e.server)), + } + st.Health = networkHealth(st.Reachable, st.Mounted) + out = append(out, st) + } + return out, nil +} + +// networkUnitEntry is one installed network-storage unit pair, parsed from its .mount file. +type networkUnitEntry struct { + name, proto, server, export, where string +} + +// networkUnitEntries enumerates the installed network-storage unit pairs from the (world-readable) +// unit dir — the shared core of ListNetworkMounts and ReassertNetworkAutomounts. No probing, no +// mountpoint access. Best-effort per file; an unreadable unit dir is the only error. +func (h *SudoHostOps) networkUnitEntries() ([]networkUnitEntry, error) { entries, err := os.ReadDir(h.unitDir) if err != nil { return nil, fmt.Errorf("netmount: reading unit dir: %w", err) } - mounts := h.mountedFSTypes() - var out []NetworkMountStatus + var out []networkUnitEntry for _, e := range entries { if !strings.HasSuffix(e.Name(), ".mount") { continue // the .mount unit carries the What/Type; the .automount mirrors Where only @@ -468,18 +499,13 @@ func (h *SudoHostOps) ListNetworkMounts(ctx context.Context) ([]NetworkMountStat if !ok { continue // not one of ours (or a drive by-uuid mount) } - st := NetworkMountStatus{ - Name: strings.TrimPrefix(where, NetworkMountRoot+"/"), - Protocol: proto, - Server: server, - Export: export, - Where: where, - Configured: true, - Mounted: isNetworkMounted(mounts[where]), - Reachable: endpointReachable(netEndpoint(proto, server)), - } - st.Health = networkHealth(st.Reachable, st.Mounted) - out = append(out, st) + out = append(out, networkUnitEntry{ + name: strings.TrimPrefix(where, NetworkMountRoot+"/"), + proto: proto, + server: server, + export: export, + where: where, + }) } return out, nil } diff --git a/internal/storage/netreassert.go b/internal/storage/netreassert.go new file mode 100644 index 0000000..496bf59 --- /dev/null +++ b/internal/storage/netreassert.go @@ -0,0 +1,117 @@ +package storage + +import ( + "context" + "fmt" + "strings" +) + +// Network-mount guest-reboot reassert (RCA AUDIT-nas-cwa-rca-2026-07-11 fix 1). +// +// THE BUG IT FIXES: a fresh guest namespace inherits REAL submounts of the shared parent (ext4, or +// an actively-mounted nfs4) but NOT an idle autofs trigger — so after any guest reboot an idle NAS +// share silently degrades to a local stub directory inside the guest. The heal is host-side and +// host-global: re-creating the .automount unit emits a FRESH trigger-mount event, which propagates +// live into every running guest's slave bind (live-proven in the RCA remediation, 2026-07-11). +// +// The action uses the sudoers-granted verbs only (`systemctl stop -- *.automount` + +// `systemctl enable --now -- *.automount`; there is NO restart grant). Idempotent: re-arming an +// already-armed trigger just recreates it — same end state, and the fresh mount event is harmless. +// An ACTIVE real mount is never touched (stopping the automount of a live mount would churn it). + +// Reassert actions (the §8 decision table, encoded). +const ( + // NetReassertRearmed: the trigger was re-created (stop + enable --now) — the propagation heal. + NetReassertRearmed = "rearmed" + // NetReassertSkipActive: a real network fs is mounted at the path — inherited by fresh + // namespaces, nothing to do (verify only). + NetReassertSkipActive = "skip-active" + // NetReassertSkipNone: neither a real mount nor an armed trigger at the path — a removed or + // orphan state owned by the add/remove flows, not this reconcile. + NetReassertSkipNone = "skip-none" +) + +// NetReassertResult is one share's outcome in a reassert pass. +type NetReassertResult struct { + Name string // share name (mount dir basename) + Where string // guest-visible mountpoint (/mnt/felhom-drives/) + Action string // NetReassert* constant + Err error // set when the rearm action failed (skip rows never error) +} + +// netReassertAction is the pure §8 decision: the /proc/mounts fstype at the share's mountpoint +// ("" = nothing mounted there) → the action to take. +func netReassertAction(fstype string) string { + switch { + case isNetworkMounted(fstype): + return NetReassertSkipActive + case fstype == "autofs": + return NetReassertRearmed + default: + return NetReassertSkipNone + } +} + +// ReassertNetworkAutomounts runs the reassert pass over every configured network mount: for each +// installed pair, decide per netReassertAction and re-arm idle triggers. Returns one result per +// share so callers (agent startup / guest-hook post-start) can verify guest visibility. Errors on +// one share never stop the pass. Callers MUST NOT invoke this from periodic health paths — an idle +// trigger is healthy, and the pass is only needed after a guest (re)start or at agent startup. +func (h *SudoHostOps) ReassertNetworkAutomounts(ctx context.Context) []NetReassertResult { + entries, err := h.networkUnitEntries() + if err != nil { + h.logger.Warn("netreassert: unit enumeration failed", "err", err) + return nil + } + fstypes := map[string]string{} + if h.host != nil { + if mounts, merr := h.host.Mounts(); merr == nil { + for _, m := range mounts { + fstypes[m.MountPoint] = m.FSType + } + } else { + h.logger.Warn("netreassert: mount-table read failed — treating all shares as unmounted", "err", merr) + } + } + var out []NetReassertResult + for _, e := range entries { + res := NetReassertResult{Name: e.name, Where: e.where, Action: netReassertAction(fstypes[e.where])} + switch res.Action { + case NetReassertSkipActive: + h.logger.Debug("netreassert: share actively mounted — skip (fresh namespaces inherit real mounts)", + "name", e.name, "where", e.where) + case NetReassertSkipNone: + h.logger.Debug("netreassert: no mount and no armed trigger — skip (removed/orphan state owned elsewhere)", + "name", e.name, "where", e.where) + case NetReassertRearmed: + if err := h.rearmNetworkAutomount(ctx, e.where); err != nil { + res.Err = err + h.logger.Warn("netreassert: trigger re-arm failed", "name", e.name, "where", e.where, "err", err) + } else { + h.logger.Info("netreassert: automount trigger re-armed (fresh mount event propagates into running guests)", + "name", e.name, "where", e.where) + } + } + out = append(out, res) + } + return out +} + +// rearmNetworkAutomount stops then re-enables+starts the .automount for a mountpoint. The stop is +// tolerated failing (unit not loaded); the enable --now is the action that must succeed. Both verbs +// are the existing FELHOM_NETMOUNT sudoers grants. +func (h *SudoHostOps) rearmNetworkAutomount(ctx context.Context, where string) error { + mountUnit, err := UnitNameForMount(where) + if err != nil { + return fmt.Errorf("netreassert: unit name for %s: %w", where, err) + } + automountUnit := strings.TrimSuffix(mountUnit, ".mount") + ".automount" + if err := h.run(ctx, h.bins.Systemctl, "stop", "--", automountUnit); err != nil { + // Tolerated: a not-loaded unit still enables cleanly below. + h.logger.Debug("netreassert: automount stop tolerated failure", "unit", automountUnit, "err", err) + } + if err := h.run(ctx, h.bins.Systemctl, "enable", "--now", "--", automountUnit); err != nil { + return fmt.Errorf("netreassert: enabling automount %s: %w", automountUnit, err) + } + return nil +} diff --git a/internal/storage/netreassert_test.go b/internal/storage/netreassert_test.go new file mode 100644 index 0000000..6a0cc2e --- /dev/null +++ b/internal/storage/netreassert_test.go @@ -0,0 +1,160 @@ +package storage + +import ( + "context" + "os" + "path/filepath" + "runtime" + "strings" + "testing" +) + +// The §8 decision table, encoded exactly (RCA AUDIT-nas-cwa-rca-2026-07-11 fix 1). +func TestNetReassertAction_Table(t *testing.T) { + cases := []struct { + fstype string + want string + }{ + {"nfs4", NetReassertSkipActive}, // real mount — inherited by fresh namespaces + {"nfs", NetReassertSkipActive}, // real mount + {"cifs", NetReassertSkipActive}, // real mount + {"autofs", NetReassertRearmed}, // idle trigger — NOT inherited, re-arm to propagate + {"", NetReassertSkipNone}, // nothing at the path — removed/orphan, owned elsewhere + {"ext4", NetReassertSkipNone}, // a local fs at the path is not a network state we own + {"tmpfs", NetReassertSkipNone}, // + } + for _, c := range cases { + if got := netReassertAction(c.fstype); got != c.want { + t.Errorf("netReassertAction(%q) = %q, want %q", c.fstype, got, c.want) + } + } +} + +// installNetUnitFile writes a rendered network .mount unit straight into unitDir (bypassing the +// runner-mediated install — the reassert only READS unit files). +func installNetUnitFile(t *testing.T, unitDir string, spec NetworkMountSpec) (where, automountUnit string) { + t.Helper() + where = spec.Where() + mountUnit, err := UnitNameForMount(where) + if err != nil { + t.Fatalf("unit name: %v", err) + } + if err := os.WriteFile(filepath.Join(unitDir, mountUnit), []byte(renderNetworkMountUnit(spec)), 0o644); err != nil { + t.Fatalf("write unit: %v", err) + } + return where, strings.TrimSuffix(mountUnit, ".mount") + ".automount" +} + +func netReassertOps(t *testing.T, unitDir string, mounts []Mount) (*SudoHostOps, *recordingRunner) { + t.Helper() + rr := &recordingRunner{} + ops := NewSudoHostOps(SudoHostOpsConfig{ + Runner: rr, Bins: Binaries{}.withDefaults(), UnitDir: unitDir, StageDir: t.TempDir(), + Host: &fakeHostReader{mounts: mounts}, Logger: quietLogger(), + }) + return ops, rr +} + +// An idle trigger (autofs at the mountpoint) must be re-armed with EXACTLY the granted verbs: +// `systemctl stop -- .automount` then `systemctl enable --now -- .automount`. +func TestReassertNetworkAutomounts_RearmsIdleTrigger(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("systemd-escaped unit filename contains a backslash; exercised on the Linux build server") + } + unitDir := t.TempDir() + spec := NetworkMountSpec{Name: "media", Protocol: ProtocolNFS, Server: "10.0.0.5", Export: "/srv/media", MappedUID: 1000, MappedGID: 1000} + where, autoUnit := installNetUnitFile(t, unitDir, spec) + ops, rr := netReassertOps(t, unitDir, []Mount{{MountPoint: where, FSType: "autofs"}}) + + results := ops.ReassertNetworkAutomounts(context.Background()) + if len(results) != 1 || results[0].Action != NetReassertRearmed || results[0].Err != nil { + t.Fatalf("want one rearmed result, got %+v", results) + } + if results[0].Where != where || results[0].Name != "media" { + t.Fatalf("result identity wrong: %+v", results[0]) + } + if len(rr.calls) != 2 { + t.Fatalf("want exactly stop + enable --now, got %d calls: %v", len(rr.calls), rr.calls) + } + stop, enable := strings.Join(rr.calls[0], " "), strings.Join(rr.calls[1], " ") + if !strings.Contains(stop, "systemctl stop -- "+autoUnit) { + t.Errorf("first call must be `systemctl stop -- %s`, got: %s", autoUnit, stop) + } + if !strings.Contains(enable, "systemctl enable --now -- "+autoUnit) { + t.Errorf("second call must be `systemctl enable --now -- %s`, got: %s", autoUnit, enable) + } +} + +// An ACTIVE real mount must not be touched — stopping the automount of a live mount would churn it. +// (Red-proof companion: a naive always-rearm implementation fails this with 2 recorded calls.) +func TestReassertNetworkAutomounts_ActiveMountUntouched(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("systemd-escaped unit filename contains a backslash; exercised on the Linux build server") + } + unitDir := t.TempDir() + spec := NetworkMountSpec{Name: "media", Protocol: ProtocolNFS, Server: "10.0.0.5", Export: "/srv/media", MappedUID: 1000, MappedGID: 1000} + where, _ := installNetUnitFile(t, unitDir, spec) + ops, rr := netReassertOps(t, unitDir, []Mount{{MountPoint: where, FSType: "nfs4"}}) + + results := ops.ReassertNetworkAutomounts(context.Background()) + if len(results) != 1 || results[0].Action != NetReassertSkipActive { + t.Fatalf("want one skip-active result, got %+v", results) + } + if len(rr.calls) != 0 { + t.Fatalf("an actively-mounted share must trigger ZERO systemctl calls, got: %v", rr.calls) + } +} + +// Neither a mount nor an armed trigger → skip (removed/orphan state, owned by add/remove flows). +func TestReassertNetworkAutomounts_NoTriggerSkips(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("systemd-escaped unit filename contains a backslash; exercised on the Linux build server") + } + unitDir := t.TempDir() + spec := NetworkMountSpec{Name: "media", Protocol: ProtocolNFS, Server: "10.0.0.5", Export: "/srv/media", MappedUID: 1000, MappedGID: 1000} + installNetUnitFile(t, unitDir, spec) + ops, rr := netReassertOps(t, unitDir, nil) // nothing at the mountpoint + + results := ops.ReassertNetworkAutomounts(context.Background()) + if len(results) != 1 || results[0].Action != NetReassertSkipNone { + t.Fatalf("want one skip-none result, got %+v", results) + } + if len(rr.calls) != 0 { + t.Fatalf("a unit with no trigger must not be acted on, got: %v", rr.calls) + } +} + +// Idempotency: two consecutive passes over an idle trigger both succeed with the same action and no +// error (re-arming a fresh trigger is harmless — same end state). +func TestReassertNetworkAutomounts_Idempotent(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("systemd-escaped unit filename contains a backslash; exercised on the Linux build server") + } + unitDir := t.TempDir() + spec := NetworkMountSpec{Name: "media", Protocol: ProtocolNFS, Server: "10.0.0.5", Export: "/srv/media", MappedUID: 1000, MappedGID: 1000} + where, _ := installNetUnitFile(t, unitDir, spec) + ops, rr := netReassertOps(t, unitDir, []Mount{{MountPoint: where, FSType: "autofs"}}) + + first := ops.ReassertNetworkAutomounts(context.Background()) + second := ops.ReassertNetworkAutomounts(context.Background()) + if first[0].Action != NetReassertRearmed || second[0].Action != NetReassertRearmed { + t.Fatalf("both passes must re-arm: first=%+v second=%+v", first, second) + } + if first[0].Err != nil || second[0].Err != nil { + t.Fatalf("idempotent passes must not error: first=%v second=%v", first[0].Err, second[0].Err) + } + if len(rr.calls) != 4 { + t.Fatalf("two passes = 2×(stop+enable), got %d: %v", len(rr.calls), rr.calls) + } +} + +// Zero configured network units → empty pass, zero commands. +func TestReassertNetworkAutomounts_NoUnitsNoOp(t *testing.T) { + ops, rr := netReassertOps(t, t.TempDir(), nil) + if results := ops.ReassertNetworkAutomounts(context.Background()); len(results) != 0 { + t.Fatalf("no units must yield no results, got %+v", results) + } + if len(rr.calls) != 0 { + t.Fatalf("no units must construct zero commands, got: %v", rr.calls) + } +}