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) <noreply@anthropic.com>
This commit is contained in:
@@ -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-`<drive>/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.**
|
||||
|
||||
@@ -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.
|
||||
@@ -120,6 +124,7 @@ func NewManager(cfg *config.Config, logger *log.Logger) (*Manager, error) {
|
||||
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 {
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user