From 0b2605c5a8e718484fd068583bcee11112d3fdf4 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Mon, 22 Jun 2026 20:47:08 +0200 Subject: [PATCH] v0.75.0: gate userdata MkdirAll on a live mountpoint (no writes into an absent drive) Belt (ensureUserdataMounts) + FileBrowser sync skip ensure/mount when an external drive root is not a live mountpoint -> no 'mkdir userdata: permission denied' + no rootfs-shadow during a drive-absent window. System/local path never gated. Reuses system.IsMountPoint; matches planDriveGates external-only rule. T1-T4 + red-proofs. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 21 ++++++ controller/internal/stacks/manager.go | 22 ++++-- .../stacks/userdata_belt_gate_test.go | 69 +++++++++++++++++++ .../internal/web/filebrowser_gate_test.go | 25 +++++++ controller/internal/web/handlers.go | 17 +++++ 5 files changed, 150 insertions(+), 4 deletions(-) create mode 100644 controller/internal/stacks/userdata_belt_gate_test.go create mode 100644 controller/internal/web/filebrowser_gate_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index d53f547..7b3e92c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,26 @@ ## Changelog +### v0.75.0 — gate userdata MkdirAll on a live mountpoint (no writes into an absent drive) (2026-06-22) + +**Bugfix — two `MkdirAll`-into-`/userdata` sites fired without checking the drive was mounted**, +producing `mkdir …/userdata: permission denied` + transient `Created` flapping during a drive-absent +window (campaign-#2 findings #2/#3). Worse than noise: writing into an unmounted mountpoint lands app +data on the guest **rootfs**, shadowed when the drive returns (data-integrity + rootfs-fill hazard). + +- `internal/stacks/manager.go` — `ensureUserdataMounts` (the deploy belt) now skips when the + `HDD_PATH` drive root is an **external** path (not `sysDataPath`) that is **not a live mountpoint**; + the app is held by `planDriveGates` instead. New injectable `Manager.isMountPoint` seam (defaults to + `system.IsMountPoint`) for testability. The system/local path is never gated (it's legitimately not a + mountpoint). +- `internal/web/handlers.go` — the FileBrowser sync loop skips (and does not mount) a registered path + under `StableParentDir` that isn't a live mountpoint, via a new pure `skipFileBrowserPath` helper. + Matches `planDriveGates`' external-only rule. +- `EnsureUserdataDir`/`EnsureUserdataSkeleton`/`planDriveGates` unchanged (gated the callers). +- Tests: `TestEnsureUserdataMounts_{SkipsAbsentExternalDrive,EnsuresWhenMounted,SystemPathNeverSkipped}` + + `TestSkipFileBrowserPath` (both red-proofed against the pre-fix code). +- **Boot-time** occurrence (docker boot-restore starting drive-backed apps before the agent mounts the + drives) is a separate cause — documented as a design note (CONTEXT.md), not changed here. + ### v0.74.0 — fix the controller→agent connection leak (per-call agentapi client) (2026-06-22) **Bugfix — agent local-API socket leak that took down the whole agent-backed feature set after ~5 days.** diff --git a/controller/internal/stacks/manager.go b/controller/internal/stacks/manager.go index efb4d2c..b5303c4 100644 --- a/controller/internal/stacks/manager.go +++ b/controller/internal/stacks/manager.go @@ -17,6 +17,7 @@ import ( "gitea.dooplex.hu/admin/felhom-controller/internal/config" "gitea.dooplex.hu/admin/felhom-controller/internal/crypto" "gitea.dooplex.hu/admin/felhom-controller/internal/settings" + "gitea.dooplex.hu/admin/felhom-controller/internal/system" ) // ContainerState represents the current state of a container. @@ -97,6 +98,9 @@ type Manager struct { backupRunning func() bool // mutual exclusion with the backup orchestrator (Change 3) migDoneHook func(*MigrationJob) // fired on successful completion (decommission policy lives in caller) testSeams *migSeams // nil in production; tests inject fakes + // isMountPoint reports whether a path is a live mountpoint; defaults to system.IsMountPoint. + // Injectable so the userdata-belt drive-absent gate is testable (a t.TempDir is never a real mount). + isMountPoint func(string) bool } // NewManager creates a new stack manager. @@ -116,10 +120,11 @@ func NewManager(cfg *config.Config, logger *log.Logger) (*Manager, error) { } return &Manager{ - cfg: cfg, - logger: logger, - composeCmd: composeCmd, - stacks: make(map[string]*Stack), + cfg: cfg, + logger: logger, + composeCmd: composeCmd, + stacks: make(map[string]*Stack), + isMountPoint: system.IsMountPoint, }, nil } @@ -130,6 +135,15 @@ func (m *Manager) ensureUserdataMounts(stackDir string, env []string) { if userdataPath == "" { return } + // Drive-absent gate: an external drive root (not the system/local path) that isn't currently a live + // mountpoint means the drive is detached. Creating ${USERDATA_PATH}/... now would write app data onto + // the guest ROOTFS, shadowed when the drive returns (data-integrity + rootfs-fill hazard). Skip — the + // app is held by the drive gate (planDriveGates). The system/local path is legitimately not a + // mountpoint, so it is never gated. + if hdd := envLookup(env, "HDD_PATH"); hdd != "" && hdd != m.sysDataPath && !m.isMountPoint(hdd) { + m.logger.Printf("[INFO] [stacks] userdata belt: drive %s not mounted — skipping ensure (held by drive gate)", hdd) + return + } composePath := filepath.Join(stackDir, "docker-compose.yml") for _, src := range ParseComposeUserdataMounts(composePath, userdataPath) { if err := appbackup.EnsureUserdataDir(src); err != nil { diff --git a/controller/internal/stacks/userdata_belt_gate_test.go b/controller/internal/stacks/userdata_belt_gate_test.go new file mode 100644 index 0000000..06abf47 --- /dev/null +++ b/controller/internal/stacks/userdata_belt_gate_test.go @@ -0,0 +1,69 @@ +package stacks + +import ( + "os" + "path/filepath" + "testing" +) + +// T1: the deploy belt SKIPS ensure when an EXTERNAL drive root is not a live mountpoint (drive absent). +// Creating ${USERDATA_PATH}/... then would land app data on the rootfs, shadowed when the drive returns. +// Companion red-proof: removing the gate in ensureUserdataMounts makes the dir get created → this fails. +func TestEnsureUserdataMounts_SkipsAbsentExternalDrive(t *testing.T) { + m := newMigManager(t, "") // sysDataPath = /mnt/sys_drive + m.isMountPoint = func(string) bool { return false } // external drive is NOT mounted + stackDir := t.TempDir() + if err := os.WriteFile(filepath.Join(stackDir, "docker-compose.yml"), []byte(beltCompose), 0o644); err != nil { + t.Fatal(err) + } + ud := filepath.Join(t.TempDir(), "userdata") + env := []string{"HDD_PATH=/mnt/felhom-drives/flash", "USERDATA_PATH=" + ud} // external, != sysDataPath + + m.ensureUserdataMounts(stackDir, env) + + if _, err := os.Stat(filepath.Join(ud, "media", "movies")); err == nil { + t.Fatal("belt MUST NOT create userdata dirs when the external drive is absent (rootfs-shadow hazard)") + } + if _, err := os.Stat(filepath.Join(ud, "downloads")); err == nil { + t.Fatal("belt MUST NOT create userdata dirs when the external drive is absent") + } +} + +// T2: the belt ENSURES the bind-source dirs when the external drive IS a live mountpoint. +func TestEnsureUserdataMounts_EnsuresWhenMounted(t *testing.T) { + m := newMigManager(t, "") + m.isMountPoint = func(string) bool { return true } // drive mounted + stackDir := t.TempDir() + if err := os.WriteFile(filepath.Join(stackDir, "docker-compose.yml"), []byte(beltCompose), 0o644); err != nil { + t.Fatal(err) + } + ud := filepath.Join(t.TempDir(), "userdata") + env := []string{"HDD_PATH=/mnt/felhom-drives/flash", "USERDATA_PATH=" + ud} + + m.ensureUserdataMounts(stackDir, env) + + for _, p := range []string{filepath.Join(ud, "media", "movies"), filepath.Join(ud, "downloads")} { + if fi, err := os.Stat(p); err != nil || !fi.IsDir() { + t.Errorf("belt should create %s when the drive is mounted (%v)", p, err) + } + } +} + +// T3: the SYSTEM/local path (HDD_PATH == sysDataPath) is NEVER gated — the SSD path is legitimately not +// a mountpoint, so ensure must always run there (must-not-over-gate). +func TestEnsureUserdataMounts_SystemPathNeverSkipped(t *testing.T) { + m := newMigManager(t, "") // sysDataPath = /mnt/sys_drive + m.isMountPoint = func(string) bool { return false } // SSD path is not a mountpoint — but must not gate + stackDir := t.TempDir() + if err := os.WriteFile(filepath.Join(stackDir, "docker-compose.yml"), []byte(beltCompose), 0o644); err != nil { + t.Fatal(err) + } + ud := filepath.Join(t.TempDir(), "userdata") + env := []string{"HDD_PATH=" + m.sysDataPath, "USERDATA_PATH=" + ud} // system path + + m.ensureUserdataMounts(stackDir, env) + + if fi, err := os.Stat(filepath.Join(ud, "media", "movies")); err != nil || !fi.IsDir() { + t.Errorf("belt must ALWAYS ensure on the system/local path (not a mountpoint, but never gated): %v", err) + } +} diff --git a/controller/internal/web/filebrowser_gate_test.go b/controller/internal/web/filebrowser_gate_test.go new file mode 100644 index 0000000..aa31ce6 --- /dev/null +++ b/controller/internal/web/filebrowser_gate_test.go @@ -0,0 +1,25 @@ +package web + +import "testing" + +// T4: skipFileBrowserPath skips ONLY an external drive path (under StableParentDir) that is not a live +// mountpoint. A mounted external path and any system/local path are never skipped. +// Companion red-proof: dropping the gate (always-false) makes the absent-usb case fail. +func TestSkipFileBrowserPath(t *testing.T) { + // flash is mounted; usb is not. + isMount := func(p string) bool { return p == StableParentDir+"/flash" } + + cases := []struct { + path string + want bool + }{ + {StableParentDir + "/flash", false}, // external + mounted → keep + {StableParentDir + "/usb", true}, // external + NOT mounted → skip + {"/mnt/sys_drive/felhom-data", false}, // system path (not under StableParentDir) → never skip + } + for _, c := range cases { + if got := skipFileBrowserPath(c.path, isMount); got != c.want { + t.Errorf("skipFileBrowserPath(%q) = %v, want %v", c.path, got, c.want) + } + } +} diff --git a/controller/internal/web/handlers.go b/controller/internal/web/handlers.go index aa96f4e..192f50a 100644 --- a/controller/internal/web/handlers.go +++ b/controller/internal/web/handlers.go @@ -1433,6 +1433,15 @@ func (s *Server) SyncFileBrowserMountsReset() { s.syncFileBrowserMounts(true) } +// skipFileBrowserPath reports whether a registered storage path should be skipped this FileBrowser +// sync pass: an EXTERNAL drive path (under StableParentDir) that is not currently a live mountpoint is +// detached, so its userdata skeleton must not be created (would land on the rootfs) and it must not be +// mounted into FileBrowser until it returns. System/local paths (not under StableParentDir) are never +// skipped. Pure + isMount-injected for testability. +func skipFileBrowserPath(path string, isMount func(string) bool) bool { + return strings.HasPrefix(path, StableParentDir+"/") && !isMount(path) +} + func (s *Server) syncFileBrowserMounts(resetDBOnChange bool) { // Prevent concurrent syncs — multiple callers can race on the same files (H5 fix). s.fileBrowserMu.Lock() @@ -1465,6 +1474,14 @@ func (s *Server) syncFileBrowserMounts(resetDBOnChange bool) { var storageMounts []string for _, sp := range paths { mountName := filepath.Base(sp.Path) // "/mnt/hdd_1" → "hdd_1" + // Drive-absent gate: an external drive path that isn't currently a live mountpoint is detached — + // don't create its userdata skeleton (would write onto the rootfs) and don't mount it into + // FileBrowser this pass. It returns on the next sync after reconnect. Matches planDriveGates' + // external-only rule (system paths, not under StableParentDir, are never skipped). + if skipFileBrowserPath(sp.Path, system.IsMountPoint) { + s.logger.Printf("[INFO] [web] FileBrowser: drive %s not mounted — skipping userdata skeleton", sp.Path) + continue + } if err := appbackup.EnsureUserdataSkeleton(sp.Path); err != nil { s.logger.Printf("[WARN] [web] FileBrowser: could not ensure userdata skeleton on %s: %v", sp.Path, err) }