diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ebf6b4..de21604 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,37 @@ ## Changelog +### v0.66.0 — userdata layout + shared-storage ownership convention (2026-06-14) + +Customer-facing `userdata/` tree (sibling of appdata/backups under each drive's felhom-data namespace) +with a shared-ownership convention so FileBrowser + content apps collaborate without permission +collisions. Spike: `felhom.eu/documentation/audits/SPIKE-userdata-layout-2026-06-14.md`. Pairs with the +app-catalog commit that repoints media mounts to `${USERDATA_PATH}`. + +- **Convention helper** (`internal/appbackup/userdata.go`): `EnsureUserdataDir`/`EnsureDirOwned` = + MkdirAll → explicit `Chmod(ModeSetgid|0775)` (MkdirAll's mode is umask-masked AND drops setgid) → + chown group to `SharedContentGID` (1000). `UserdataDir`, `UserdataSkeleton` (media/{movies,tv,music, + audiobooks,books,comics,photos}, downloads, import/{paperless,calibre}, roms, documents), + `EnsureUserdataSkeleton`. Linux chown via `chownGID`/`StatGID` (`userdata_linux.go`); no-op stub + off-Linux (`userdata_other.go`). +- **USERDATA_PATH injection** (`stackEnv`, manager.go): injects `USERDATA_PATH = /userdata` + (HDD_PATH is the namespace root) alongside HDD_PATH, so the catalog's `${USERDATA_PATH}/...` mounts + resolve. +- **Skeleton pre-create**: `registerStoragePath` + `syncFileBrowserMounts` ensure the full skeleton on + every storage path (system + additional drives) with the convention. +- **Deploy belt**: `composeExecCustomEnv` (gated on `up`) pre-creates every `${USERDATA_PATH}/...` + bind source the stack declares (`ParseComposeUserdataMounts` + `ensureUserdataMounts`) so Docker + never auto-creates a userdata dir as guest-root — covers apps not in the skeleton. +- **FileBrowser mount switch** (`syncFileBrowserMounts`): mounts `/userdata` (was `appdata`) → + `/srv/`. FileBrowser runs as uid 1000 → can now create folders + upload into the 2775 setgid + userdata (fixes the permission-denied); app internals (appdata/) are no longer browsable. +- **#8 migration fix** (`migrate.go`): the non-app merge walk now preserves the SOURCE dir's full mode + (incl. setgid via `preserveDirOwnership`) + group, and `copyFile` preserves the full file mode + (`fi.Mode()`, not `.Perm()`) + group — so the ownership convention survives a whole-drive `MigrateAll`. +- Non-hollow tests: `EnsureDirOwned` produces 02775+setgid+gid (Linux companion proves a plain MkdirAll + has NO setgid); skeleton structure; `ParseComposeUserdataMounts` selectivity; deploy belt creates the + declared dirs; **migration preserves setgid+group** (Linux; mutation-proven against the pre-fix + 0755/.Perm() path). + ### v0.65.0 — data migration + self-serve decommission (B1+B2) (2026-06-14) Customer-self-serve storage **migration** (move app data between drives) and **decommission** (retire diff --git a/controller/internal/appbackup/derivestackname_test.go b/controller/internal/appbackup/derivestackname_test.go index 0553092..c1c001f 100644 --- a/controller/internal/appbackup/derivestackname_test.go +++ b/controller/internal/appbackup/derivestackname_test.go @@ -14,8 +14,8 @@ func TestDeriveStackName_KnownCrossRef(t *testing.T) { note string }{ {"romm-postgres", "romm", "role suffix of a real stack → strip"}, - {"my-cache", "my-cache", "container name IS a real stack → do NOT strip to 'my'"}, // pre-fix: "my" - {"my-cache-postgres", "my-cache", "strip role, result is a known stack"}, // pre-fix: "my-cache" (ok) — but via prefix here + {"my-cache", "my-cache", "container name IS a real stack → do NOT strip to 'my'"}, // pre-fix: "my" + {"my-cache-postgres", "my-cache", "strip role, result is a known stack"}, // pre-fix: "my-cache" (ok) — but via prefix here {"paperless-ngx-postgres", "paperless-ngx", "multi-hyphen stack, role suffix"}, {"romm_postgres", "romm", "underscore-separated compose name → longest known prefix"}, {"romm-1", "romm", "compose numeric suffix → longest known prefix"}, diff --git a/controller/internal/appbackup/userdata.go b/controller/internal/appbackup/userdata.go new file mode 100644 index 0000000..ecd2668 --- /dev/null +++ b/controller/internal/appbackup/userdata.go @@ -0,0 +1,76 @@ +package appbackup + +import ( + "os" + "path/filepath" +) + +// Customer-facing userdata layout + the shared-storage ownership convention (v0.66.0). +// +// userdata/ is a sibling of appdata/ and backups/ under a drive's felhom-data namespace. It is the +// ONLY customer-browsable tree (FileBrowser mounts it). Apps that handle customer content write here. +// +// Ownership convention: every userdata dir is group-owned by SharedContentGID, mode 2775 (setgid + +// group-rwx). Setgid makes new files/dirs inherit the shared group regardless of which app (or +// FileBrowser) created them, so members collaborate without permission collisions. FileBrowser +// (uid/gid 1000) and the content apps (PUID/PGID 1000, or pinned user 1000:1000) are all members. + +// SharedContentGID is the group that owns the userdata tree. +const SharedContentGID = 1000 + +// userdataDirMode is the on-disk mode for every userdata dir: setgid + group-rwx. os.ModeSetgid (NOT +// the raw 0o2000) is how Go's Chmod requests S_ISGID. MkdirAll's mode is umask-masked AND drops the +// setgid bit, so an explicit Chmod is mandatory after MkdirAll. +const userdataDirMode = os.ModeSetgid | 0o775 + +// UserdataDir returns the customer-facing userdata root under a namespace root. +func UserdataDir(nsRoot string) string { + return filepath.Join(nsRoot, "userdata") +} + +// UserdataSkeleton is the standard subtree created on every storage path (relative to UserdataDir). +// ASCII, no spaces (flows through ${} interpolation, shell, and the rsync merge walk). +func UserdataSkeleton() []string { + return []string{ + "media", "media/movies", "media/tv", "media/music", "media/audiobooks", + "media/books", "media/comics", "media/photos", + "downloads", + "import", "import/paperless", "import/calibre", + "roms", + "documents", + } +} + +// EnsureDirOwned creates path (idempotent) and enforces the convention: mode 2775 via an explicit +// Chmod incl. setgid (MkdirAll cannot) + group = gid. Setting an arbitrary group needs CAP_CHOWN — +// the in-guest controller runs as root, so this succeeds in production. Returns the first hard error. +func EnsureDirOwned(path string, gid int) error { + if err := os.MkdirAll(path, 0o755); err != nil { + return err + } + if err := os.Chmod(path, userdataDirMode); err != nil { + return err + } + return chownGID(path, gid) +} + +// EnsureUserdataDir applies the convention with the shared content group (GID 1000). Idempotent. +func EnsureUserdataDir(path string) error { return EnsureDirOwned(path, SharedContentGID) } + +// EnsureUserdataSkeleton creates the full userdata tree under a namespace root with the convention. +// It creates ALL dirs even if one errors (so a single chown/chmod hiccup doesn't truncate the tree), +// returning the first error seen for the caller to log. +func EnsureUserdataSkeleton(nsRoot string) error { + base := UserdataDir(nsRoot) + var firstErr error + rec := func(e error) { + if e != nil && firstErr == nil { + firstErr = e + } + } + rec(EnsureUserdataDir(base)) + for _, sub := range UserdataSkeleton() { + rec(EnsureUserdataDir(filepath.Join(base, sub))) + } + return firstErr +} diff --git a/controller/internal/appbackup/userdata_linux.go b/controller/internal/appbackup/userdata_linux.go new file mode 100644 index 0000000..7eb1c68 --- /dev/null +++ b/controller/internal/appbackup/userdata_linux.go @@ -0,0 +1,20 @@ +//go:build linux + +package appbackup + +import ( + "os" + "syscall" +) + +// chownGID sets the GROUP of path (owner unchanged via -1). Setting an arbitrary group requires +// CAP_CHOWN; the in-guest controller runs as root, so this succeeds in production. +func chownGID(path string, gid int) error { return os.Chown(path, -1, gid) } + +// StatGID returns the owning GID of fi (Linux). ok=false when the underlying stat is unavailable. +func StatGID(fi os.FileInfo) (int, bool) { + if st, ok := fi.Sys().(*syscall.Stat_t); ok { + return int(st.Gid), true + } + return -1, false +} diff --git a/controller/internal/appbackup/userdata_other.go b/controller/internal/appbackup/userdata_other.go new file mode 100644 index 0000000..4c4a2ea --- /dev/null +++ b/controller/internal/appbackup/userdata_other.go @@ -0,0 +1,11 @@ +//go:build !linux + +package appbackup + +import "os" + +// chownGID is a no-op off Linux (the dev machine has no POSIX group ownership). Production is Linux. +func chownGID(path string, gid int) error { return nil } + +// StatGID is unavailable off Linux. +func StatGID(fi os.FileInfo) (int, bool) { return -1, false } diff --git a/controller/internal/appbackup/userdata_setgid_linux_test.go b/controller/internal/appbackup/userdata_setgid_linux_test.go new file mode 100644 index 0000000..545af87 --- /dev/null +++ b/controller/internal/appbackup/userdata_setgid_linux_test.go @@ -0,0 +1,43 @@ +//go:build linux + +package appbackup + +import ( + "os" + "path/filepath" + "testing" +) + +// TestEnsureDirOwned_Setgid is the load-bearing assertion: EnsureDirOwned produces a dir with the +// SETGID bit + group-rwx (mode 02775) and the requested group. Uses the test's own gid so the chown +// succeeds without root. Companion: a plain MkdirAll(0755) does NOT get setgid — proving the explicit +// Chmod is what sets it (the spike's collision fix). This test FAILS on a pre-fix MkdirAll-only impl. +func TestEnsureDirOwned_Setgid(t *testing.T) { + gid := os.Getgid() + dir := filepath.Join(t.TempDir(), "userdata", "media", "movies") + if err := EnsureDirOwned(dir, gid); err != nil { + t.Fatalf("EnsureDirOwned: %v", err) + } + fi, err := os.Stat(dir) + if err != nil { + t.Fatal(err) + } + if fi.Mode()&os.ModeSetgid == 0 { + t.Errorf("dir is missing the setgid bit: mode=%v", fi.Mode()) + } + if perm := fi.Mode().Perm(); perm != 0o775 { + t.Errorf("dir perm = %o, want 0775", perm) + } + if g, ok := StatGID(fi); !ok || g != gid { + t.Errorf("dir gid = %d (ok=%v), want %d", g, ok, gid) + } + + // Companion: the pre-fix behaviour (MkdirAll only, no explicit setgid Chmod) → NO setgid. + plain := filepath.Join(t.TempDir(), "plain") + if err := os.MkdirAll(plain, 0o755); err != nil { + t.Fatal(err) + } + if pfi, _ := os.Stat(plain); pfi.Mode()&os.ModeSetgid != 0 { + t.Errorf("plain MkdirAll unexpectedly has setgid — the explicit Chmod is not load-bearing") + } +} diff --git a/controller/internal/appbackup/userdata_test.go b/controller/internal/appbackup/userdata_test.go new file mode 100644 index 0000000..9ab5898 --- /dev/null +++ b/controller/internal/appbackup/userdata_test.go @@ -0,0 +1,52 @@ +package appbackup + +import ( + "os" + "path/filepath" + "testing" +) + +// TestSharedContentGID pins the shared content group to 1000 (FileBrowser's gid + the apps' PUID/PGID). +func TestSharedContentGID(t *testing.T) { + if SharedContentGID != 1000 { + t.Errorf("SharedContentGID = %d, want 1000", SharedContentGID) + } +} + +// TestUserdataSkeleton_List asserts the locked skeleton subdir set. +func TestUserdataSkeleton_List(t *testing.T) { + got := map[string]bool{} + for _, s := range UserdataSkeleton() { + got[s] = true + } + for _, want := range []string{ + "media/movies", "media/tv", "media/music", "media/audiobooks", "media/books", + "media/comics", "media/photos", "downloads", "import/paperless", "import/calibre", + "roms", "documents", + } { + if !got[want] { + t.Errorf("skeleton missing %q", want) + } + } +} + +// TestUserdataDir confirms the userdata root is a sibling under the namespace. +func TestUserdataDir(t *testing.T) { + if got := UserdataDir("/mnt/felhom-usb"); got != filepath.Clean("/mnt/felhom-usb/userdata") { + t.Errorf("UserdataDir = %q", got) + } +} + +// TestEnsureUserdataSkeleton_Structure: every skeleton dir is created (chown may fail off-root, which +// is ignored — dirs + setgid still land). Runs cross-platform. +func TestEnsureUserdataSkeleton_Structure(t *testing.T) { + ns := t.TempDir() + _ = EnsureUserdataSkeleton(ns) // ignore chown error on a non-root CI host + base := UserdataDir(ns) + for _, sub := range append([]string{""}, UserdataSkeleton()...) { + p := filepath.Join(base, sub) + if fi, err := os.Stat(p); err != nil || !fi.IsDir() { + t.Errorf("skeleton dir missing: %s (%v)", p, err) + } + } +} diff --git a/controller/internal/stacks/delete.go b/controller/internal/stacks/delete.go index 192fa16..24c28d5 100644 --- a/controller/internal/stacks/delete.go +++ b/controller/internal/stacks/delete.go @@ -62,10 +62,10 @@ func ProtectedHDDPaths(hddPath string) map[string]bool { return map[string]bool{ // Model A: the in-guest drive mount IS the felhom-data namespace root, so backups/ and // appdata/ sit directly under it (no felhom-data segment). - hddPath: true, - filepath.Join(hddPath, "appdata"): true, - filepath.Join(hddPath, "backups"): true, - filepath.Join(hddPath, "media"): true, + hddPath: true, + filepath.Join(hddPath, "appdata"): true, + filepath.Join(hddPath, "backups"): true, + filepath.Join(hddPath, "media"): true, filepath.Join(hddPath, "Dokumentumok"): true, // Legacy pre-Model-A double-nest location; kept protected so any leftover data there is // never wiped by a removal. @@ -505,6 +505,53 @@ func buildPathInfo(path string) HDDPath { return item } +// ParseComposeUserdataMounts reads a docker-compose.yml and extracts the host bind-source paths that +// reference ${USERDATA_PATH} (resolved to userdataPath) — the dirs the deploy belt must pre-create +// with the userdata convention. Same scanner shape as ParseComposeHDDMounts. +func ParseComposeUserdataMounts(composePath, userdataPath string) []string { + if userdataPath == "" { + return nil + } + data, err := os.ReadFile(composePath) + if err != nil { + return nil + } + var mounts []string + seen := make(map[string]bool) + scanner := bufio.NewScanner(strings.NewReader(string(data))) + inVolumes := false + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + if strings.HasPrefix(line, "volumes:") { + inVolumes = true + continue + } + if inVolumes && !strings.HasPrefix(line, "-") && !strings.HasPrefix(line, "#") && line != "" { + inVolumes = false + } + if !inVolumes || !strings.HasPrefix(line, "- ") { + continue + } + mountStr := strings.Trim(strings.TrimPrefix(line, "- "), "\"'") + parts := strings.SplitN(mountStr, ":", 3) + if len(parts) < 2 { + continue + } + hostPath := strings.ReplaceAll(parts[0], "${USERDATA_PATH}", userdataPath) + cleanPath := filepath.Clean(hostPath) + cleanUD := filepath.Clean(userdataPath) + // must be userdataPath itself or a subpath (clean before check — traversal-safe) + if cleanPath != cleanUD && !strings.HasPrefix(cleanPath, cleanUD+string(filepath.Separator)) { + continue + } + if !seen[cleanPath] { + seen[cleanPath] = true + mounts = append(mounts, cleanPath) + } + } + return mounts +} + // ParseComposeHDDMounts reads a docker-compose.yml and extracts host paths // that reference the HDD path from volume bind mounts. func ParseComposeHDDMounts(composePath, hddPath string) []string { diff --git a/controller/internal/stacks/deploy.go b/controller/internal/stacks/deploy.go index 08341ec..e8d12e4 100644 --- a/controller/internal/stacks/deploy.go +++ b/controller/internal/stacks/deploy.go @@ -919,4 +919,4 @@ func randomAlphanumeric(length int) (string, error) { result[i] = alphanumChars[n.Int64()] } return string(result), nil -} \ No newline at end of file +} diff --git a/controller/internal/stacks/deploy_crashsafety_regression_test.go b/controller/internal/stacks/deploy_crashsafety_regression_test.go index 384f30c..8964619 100644 --- a/controller/internal/stacks/deploy_crashsafety_regression_test.go +++ b/controller/internal/stacks/deploy_crashsafety_regression_test.go @@ -83,7 +83,7 @@ func TestTransitionalDeployStateReadsNotDeployed(t *testing.T) { } // This is exactly the expression ScanStacks uses: deployed := cfg != nil && cfg.Deployed if got.Deployed { - t.Fatalf("CTRL-T2-1: a deploy that did not complete reads as Deployed=true — ghost-deployed; "+ + t.Fatalf("CTRL-T2-1: a deploy that did not complete reads as Deployed=true — ghost-deployed; " + "DeployStack must persist Deployed:false until compose succeeds") } diff --git a/controller/internal/stacks/manager.go b/controller/internal/stacks/manager.go index ffcb8dc..8ae336e 100644 --- a/controller/internal/stacks/manager.go +++ b/controller/internal/stacks/manager.go @@ -13,6 +13,7 @@ import ( "sync" "time" + "gitea.dooplex.hu/admin/felhom-controller/internal/appbackup" "gitea.dooplex.hu/admin/felhom-controller/internal/config" "gitea.dooplex.hu/admin/felhom-controller/internal/crypto" "gitea.dooplex.hu/admin/felhom-controller/internal/settings" @@ -122,6 +123,33 @@ func NewManager(cfg *config.Config, logger *log.Logger) (*Manager, error) { }, nil } +// ensureUserdataMounts is the deploy belt: pre-create every ${USERDATA_PATH}/... bind source the +// stack declares with the userdata convention, so Docker never auto-creates one as guest-root. +func (m *Manager) ensureUserdataMounts(stackDir string, env []string) { + userdataPath := envLookup(env, "USERDATA_PATH") + if userdataPath == "" { + return + } + composePath := filepath.Join(stackDir, "docker-compose.yml") + for _, src := range ParseComposeUserdataMounts(composePath, userdataPath) { + if err := appbackup.EnsureUserdataDir(src); err != nil { + m.logger.Printf("[WARN] [stacks] userdata belt: ensure %s: %v", src, err) + } + } +} + +// envLookup returns the value of key in a "K=V" env slice (last occurrence wins, "" if absent). +func envLookup(env []string, key string) string { + prefix := key + "=" + val := "" + for _, e := range env { + if strings.HasPrefix(e, prefix) { + val = e[len(prefix):] + } + } + return val +} + // SetEncryptionKey sets the AES-256 key used to encrypt/decrypt sensitive values in app.yaml. func (m *Manager) SetEncryptionKey(key []byte) { m.mu.Lock() @@ -829,6 +857,12 @@ func (m *Manager) stackEnv(stackDir string) []string { for k, v := range appCfg.Env { env = append(env, fmt.Sprintf("%s=%s", k, v)) } + // Inject USERDATA_PATH = /userdata alongside HDD_PATH (v0.66.0). HDD_PATH IS + // the namespace root (the chosen StoragePath: a Model-A user drive's mount, or the SSD's + // felhom-data dir), so the catalog's ${USERDATA_PATH}/... mounts resolve under userdata/. + if hdd := appCfg.Env["HDD_PATH"]; hdd != "" { + env = append(env, fmt.Sprintf("USERDATA_PATH=%s", appbackup.UserdataDir(hdd))) + } } return env @@ -857,6 +891,13 @@ func (m *Manager) composeExecCustomEnv(dir string, env []string, args ...string) cmd.Env = env } + // Deploy belt (v0.66.0): before `up`, pre-create every ${USERDATA_PATH}/... bind source with the + // userdata convention (2775 setgid, gid 1000) so the Docker daemon never auto-creates one as + // guest-root — covers apps not in the skeleton too. Best-effort: a perms hiccup never blocks deploy. + if len(args) > 0 && args[0] == "up" { + m.ensureUserdataMounts(dir, env) + } + // Log env var keys at debug level if m.isDebug() { var appKeys []string diff --git a/controller/internal/stacks/metadata.go b/controller/internal/stacks/metadata.go index c5055c4..ea18472 100644 --- a/controller/internal/stacks/metadata.go +++ b/controller/internal/stacks/metadata.go @@ -21,7 +21,7 @@ type Metadata struct { AppInfo AppInfo `yaml:"app_info" json:"app_info"` OptionalConfig []OptionalConfigGroup `yaml:"optional_config" json:"optional_config"` HealthCheck *HealthCheckConfig `yaml:"healthcheck,omitempty" json:"healthcheck,omitempty"` - Integrations []IntegrationDef `yaml:"integrations,omitempty" json:"integrations,omitempty"` + Integrations []IntegrationDef `yaml:"integrations,omitempty" json:"integrations,omitempty"` } // AppInfo holds detailed app information for the info page. @@ -52,25 +52,25 @@ type OptionalConfigField struct { // ResourceHints describe what the app needs. type ResourceHints struct { - MemRequest string `yaml:"mem_request" json:"mem_request"` - MemLimit string `yaml:"mem_limit" json:"mem_limit"` - PiCompatible bool `yaml:"pi_compatible" json:"pi_compatible"` - NeedsHDD bool `yaml:"needs_hdd" json:"needs_hdd"` - HungarianUI bool `yaml:"hungarian_ui" json:"hungarian_ui"` + MemRequest string `yaml:"mem_request" json:"mem_request"` + MemLimit string `yaml:"mem_limit" json:"mem_limit"` + PiCompatible bool `yaml:"pi_compatible" json:"pi_compatible"` + NeedsHDD bool `yaml:"needs_hdd" json:"needs_hdd"` + HungarianUI bool `yaml:"hungarian_ui" json:"hungarian_ui"` } // DeployField defines one configuration field shown during first deployment. type DeployField struct { - EnvVar string `yaml:"env_var" json:"env_var"` - Label string `yaml:"label" json:"label"` - Type string `yaml:"type" json:"type"` // domain, subdomain, secret, password, path, text, select, boolean - Generate string `yaml:"generate" json:"generate"` // e.g., "password:24", "hex:32", "static:admin" - Default string `yaml:"default" json:"default"` - Required bool `yaml:"required" json:"required"` - Placeholder string `yaml:"placeholder" json:"placeholder"` - Description string `yaml:"description" json:"description"` + EnvVar string `yaml:"env_var" json:"env_var"` + Label string `yaml:"label" json:"label"` + Type string `yaml:"type" json:"type"` // domain, subdomain, secret, password, path, text, select, boolean + Generate string `yaml:"generate" json:"generate"` // e.g., "password:24", "hex:32", "static:admin" + Default string `yaml:"default" json:"default"` + Required bool `yaml:"required" json:"required"` + Placeholder string `yaml:"placeholder" json:"placeholder"` + Description string `yaml:"description" json:"description"` LockedAfterDeploy bool `yaml:"locked_after_deploy" json:"locked_after_deploy"` - Options []SelectOption `yaml:"options" json:"options,omitempty"` + Options []SelectOption `yaml:"options" json:"options,omitempty"` // DataKey marks a field as a DATA-ENCRYPTING key (e.g. AdventureLog's "Titkosítási kulcs"): // the app encrypts stored data with it, so regenerating it would render restored data // unreadable. It is a fail-closed annotation only — the recovery unit never stores secrets; @@ -113,7 +113,7 @@ type HealthCheckConfig struct { // HealthCheckItem defines a single health check probe. type HealthCheckItem struct { - Type string `yaml:"type" json:"type"` // "http", "api", "tcp" + Type string `yaml:"type" json:"type"` // "http", "api", "tcp" Port int `yaml:"port" json:"port"` Path string `yaml:"path" json:"path"` // for http/api; default "/" Method string `yaml:"method" json:"method"` // for api; default "GET" diff --git a/controller/internal/stacks/migrate.go b/controller/internal/stacks/migrate.go index 7caaf51..4d3a757 100644 --- a/controller/internal/stacks/migrate.go +++ b/controller/internal/stacks/migrate.go @@ -848,7 +848,13 @@ func walkMerge(lg *log.Logger, srcNS, dstNS string, skip map[string]bool, assert if assertOnly { return nil } - return os.MkdirAll(dst, 0o755) + if err := os.MkdirAll(dst, 0o755); err != nil { + return err + } + // #8 (v0.66.0): preserve the SOURCE dir's full mode (incl. setgid) + group, so the userdata + // ownership convention (2775 setgid, gid 1000) survives a whole-drive migration. MkdirAll's + // mode is umask-masked + drops setgid, so re-stamp explicitly from the source. + return preserveDirOwnership(dst, d) } // Symlink: recreate-if-absent (copy mode); ignored in assert mode. @@ -994,10 +1000,15 @@ func copyFile(src, dst string) (int64, error) { os.Remove(tmp) return 0, err } - if err := os.Chmod(tmp, fi.Mode().Perm()); err != nil { + // #8 (v0.66.0): preserve the SOURCE file's FULL mode (incl. setgid/setuid/sticky — not .Perm(), + // which masks them off) + group, so the userdata convention survives a whole-drive migration. + if err := os.Chmod(tmp, fi.Mode()); err != nil { os.Remove(tmp) return 0, err } + if gid, ok := appbackup.StatGID(fi); ok { + _ = os.Chown(tmp, -1, gid) // best-effort; needs root for an arbitrary group (the controller is) + } if err := os.Rename(tmp, dst); err != nil { os.Remove(tmp) return 0, err @@ -1005,6 +1016,22 @@ func copyFile(src, dst string) (int64, error) { return n, nil } +// preserveDirOwnership re-stamps a freshly-created target dir with the SOURCE dir's full mode (incl. +// setgid) and group — part of the #8 fix so the userdata convention survives a migration. +func preserveDirOwnership(dst string, d fs.DirEntry) error { + info, err := d.Info() + if err != nil { + return err + } + if err := os.Chmod(dst, info.Mode()); err != nil { + return err + } + if gid, ok := appbackup.StatGID(info); ok { + _ = os.Chown(dst, -1, gid) // best-effort; root sets an arbitrary group (the controller is root) + } + return nil +} + // fileSum returns the hex sha256 of a file (streaming). func fileSum(path string) (string, error) { f, err := os.Open(path) diff --git a/controller/internal/stacks/migrate_setgid_linux_test.go b/controller/internal/stacks/migrate_setgid_linux_test.go new file mode 100644 index 0000000..8280dfc --- /dev/null +++ b/controller/internal/stacks/migrate_setgid_linux_test.go @@ -0,0 +1,59 @@ +//go:build linux + +package stacks + +import ( + "io" + "log" + "os" + "path/filepath" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/appbackup" +) + +// TestWalkMerge_PreservesSetgid is the #8 fix proof: the non-app merge walk must preserve the SOURCE +// dir's setgid bit + group, so the userdata ownership convention (2775 setgid, gid 1000) survives a +// whole-drive migration. Companion: the pre-fix path (MkdirAll 0755 / copyFile .Perm()) drops setgid +// → this test FAILS on the old code (mutation-proven separately). +func TestWalkMerge_PreservesSetgid(t *testing.T) { + lg := log.New(io.Discard, "", 0) + src := t.TempDir() + dst := t.TempDir() + + // Source userdata-style tree with setgid dirs (the convention). + media := filepath.Join(src, "userdata", "media") + if err := os.MkdirAll(media, 0o755); err != nil { + t.Fatal(err) + } + for _, d := range []string{filepath.Join(src, "userdata"), media} { + if err := os.Chmod(d, os.ModeSetgid|0o775); err != nil { + t.Fatal(err) + } + } + if err := os.WriteFile(filepath.Join(media, "movie.txt"), []byte("VID"), 0o664); err != nil { + t.Fatal(err) + } + + if err := walkMerge(lg, src, dst, nil, false, nil); err != nil { + t.Fatalf("walkMerge: %v", err) + } + + // Target media dir must still carry setgid + the source group. + srcFI, _ := os.Stat(media) + dstFI, err := os.Stat(filepath.Join(dst, "userdata", "media")) + if err != nil { + t.Fatalf("target media dir missing: %v", err) + } + if dstFI.Mode()&os.ModeSetgid == 0 { + t.Errorf("migration DROPPED the setgid bit on the dir: mode=%v", dstFI.Mode()) + } + if dstFI.Mode().Perm() != 0o775 { + t.Errorf("target dir perm = %o, want 0775", dstFI.Mode().Perm()) + } + sg, _ := appbackup.StatGID(srcFI) + dg, _ := appbackup.StatGID(dstFI) + if sg != dg { + t.Errorf("target dir group = %d, want source group %d", dg, sg) + } +} diff --git a/controller/internal/stacks/userdata_belt_test.go b/controller/internal/stacks/userdata_belt_test.go new file mode 100644 index 0000000..5bb3043 --- /dev/null +++ b/controller/internal/stacks/userdata_belt_test.go @@ -0,0 +1,73 @@ +package stacks + +import ( + "os" + "path/filepath" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/appbackup" +) + +const beltCompose = `services: + app: + image: x + volumes: + - app_config:/config + - ${USERDATA_PATH}/media/movies:/media/movies + - ${USERDATA_PATH}/downloads:/downloads + - ${HDD_PATH}/appdata/app:/data + - /etc/passwd:/host:ro +volumes: + app_config: +` + +// TestParseComposeUserdataMounts: only ${USERDATA_PATH}/... bind sources are returned, resolved; HDD +// appdata mounts, named volumes, and unrelated host paths are ignored. +func TestParseComposeUserdataMounts(t *testing.T) { + dir := t.TempDir() + cp := filepath.Join(dir, "docker-compose.yml") + if err := os.WriteFile(cp, []byte(beltCompose), 0o644); err != nil { + t.Fatal(err) + } + ud := filepath.Clean("/mnt/felhom-usb/userdata") + got := map[string]bool{} + for _, m := range ParseComposeUserdataMounts(cp, ud) { + got[m] = true + } + for _, want := range []string{ + filepath.Join(ud, "media", "movies"), + filepath.Join(ud, "downloads"), + } { + if !got[want] { + t.Errorf("missing userdata mount %q (got %v)", want, got) + } + } + if len(got) != 2 { + t.Errorf("expected exactly 2 userdata mounts, got %d: %v", len(got), got) + } +} + +// TestEnsureUserdataMounts_CreatesBeltDirs: the deploy belt pre-creates every declared ${USERDATA_PATH} +// bind source before compose-up (so Docker never auto-creates one as root). Uses a real temp userdata +// root via env injection. +func TestEnsureUserdataMounts_CreatesBeltDirs(t *testing.T) { + m := newMigManager(t, "") // minimal Manager (cfg+logger+settings) + 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{"USERDATA_PATH=" + ud} + + // movies dir absent before + if _, err := os.Stat(filepath.Join(ud, "media", "movies")); err == nil { + t.Fatal("precondition: movies dir should not exist yet") + } + 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 did not create %s (%v)", p, err) + } + } + _ = appbackup.SharedContentGID // keep import referenced cross-platform +} diff --git a/controller/internal/web/handlers.go b/controller/internal/web/handlers.go index 6a1292a..aa96f4e 100644 --- a/controller/internal/web/handlers.go +++ b/controller/internal/web/handlers.go @@ -12,6 +12,7 @@ import ( "strings" "time" + "gitea.dooplex.hu/admin/felhom-controller/internal/appbackup" "gitea.dooplex.hu/admin/felhom-controller/internal/backup" "gitea.dooplex.hu/admin/felhom-controller/internal/crypto" "gitea.dooplex.hu/admin/felhom-controller/internal/infra" @@ -1456,18 +1457,19 @@ func (s *Server) syncFileBrowserMounts(resetDBOnChange bool) { return } - // Build volume mount lines. SCOPE to the drive's `appdata/` subtree only (Phase 4A): the customer - // browses their userdata, but the recovery units + Tier 2 copies under `backups/` are NOT mounted - // into FileBrowser at all — so the thing that restores them can't be browsed or (even read-only) - // surfaced. mkdir the appdata dir first so the bind source exists with sane ownership. + // Build volume mount lines. SCOPE to the drive's `userdata/` subtree (v0.66.0): the customer + // browses ONLY userdata — app internals (appdata/) and the recovery units + Tier 2 copies + // (backups/) are NOT mounted into FileBrowser. userdata is owned group 1000 mode 2775 (setgid), + // and FileBrowser runs as uid 1000 → it can create folders + upload files (the old appdata mount + // was guest-root 0755 → permission-denied). Pre-create the full skeleton with the convention. var storageMounts []string for _, sp := range paths { mountName := filepath.Base(sp.Path) // "/mnt/hdd_1" → "hdd_1" - appdataSrc := filepath.Join(sp.Path, "appdata") - if err := os.MkdirAll(appdataSrc, 0755); err != nil { - s.logger.Printf("[WARN] [web] FileBrowser: could not ensure appdata dir %s: %v", appdataSrc, err) + 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) } - line := fmt.Sprintf(" - %s:/srv/%s", appdataSrc, mountName) + userdataSrc := appbackup.UserdataDir(sp.Path) + line := fmt.Sprintf(" - %s:/srv/%s", userdataSrc, mountName) storageMounts = append(storageMounts, line) } diff --git a/controller/internal/web/storage_handlers.go b/controller/internal/web/storage_handlers.go index c3ce52d..f21fde9 100644 --- a/controller/internal/web/storage_handlers.go +++ b/controller/internal/web/storage_handlers.go @@ -219,6 +219,12 @@ func (s *Server) registerStoragePath(where, label string, setDefault bool) error if strings.TrimSpace(label) == "" { label = settings.InferStorageLabel(where) } + // v0.66.0: create the full userdata skeleton with the shared-storage convention (2775 setgid, + // gid 1000) the moment a drive is registered — system drive AND additional drives. Idempotent; + // best-effort (a perms hiccup shouldn't block registration). + if err := appbackup.EnsureUserdataSkeleton(where); err != nil { + s.logger.Printf("[WARN] [web] userdata skeleton on %s: %v", where, err) + } // Change 4: re-enrolling a previously-DECOMMISSIONED drive must un-retire it. AddStoragePath // dedups a re-register into a no-op, so without this the soft marker would persist forever and the // apps' "missing storage" indicator would never clear.