From a96c3d9473436b8bdae7af93ce53f9d334f2e157 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Tue, 4 Aug 2026 18:21:17 +0200 Subject: [PATCH] R-203: the export-mount resolver takes the namespace root too (its own commit) ExportDataMounts lives in delete.go, which reads as a destructive path. IT IS NOT: its single production caller is the .fab export adapter, and nothing deletes based on its result. The delete path's own guard, ProtectedHDDPaths, is layout-agnostic by construction -- it protects BOTH /... and /felhom-data/... -- so deletion was never affected by the namespace-root defect. That scope note is now in the function's doc comment, because the file placement will mislead the next reader exactly as it misled the spec for this change. Separated into its own commit anyway, so a change to a function whose filename says "delete" is reviewable on its own. An empty nsRoot falls back to hddPath -- the pre-R-203 shape -- so any caller not yet updated keeps working on enrolled drives. Tests cover both drive kinds and assert the NEGATIVE: no emitted path lies outside the app's own data roots. Red-proof: leaving the site bare fails the system-drive row, emitting /mnt/sys_drive/userdata where the canonical root is /mnt/sys_drive/felhom-data/userdata. --- controller/cmd/controller/main.go | 2 +- controller/internal/stacks/delete.go | 15 +++- .../internal/stacks/export_mounts_test.go | 77 +++++++++++++++++-- 3 files changed, 85 insertions(+), 9 deletions(-) diff --git a/controller/cmd/controller/main.go b/controller/cmd/controller/main.go index 2b15177..876e320 100644 --- a/controller/cmd/controller/main.go +++ b/controller/cmd/controller/main.go @@ -2229,7 +2229,7 @@ func (a *exportAdapter) GetStackHDDMounts(name string) []string { // F-S1; NOT the namespace wholesale), and derives that dir name from the compose binds // (F-S2, see backup.tier2AppDataName). Spec: // felhom.eu/documentation/audits/SPIKE-backup-classification-2026-07-14.md. - return stacks.ExportDataMounts(s.ComposePath, appCfg.Env["HDD_PATH"]) + return stacks.ExportDataMounts(s.ComposePath, appCfg.Env["HDD_PATH"], a.mgr.StackNamespaceRoot(name)) } return nil } diff --git a/controller/internal/stacks/delete.go b/controller/internal/stacks/delete.go index ea8655e..e146abf 100644 --- a/controller/internal/stacks/delete.go +++ b/controller/internal/stacks/delete.go @@ -559,12 +559,23 @@ func ParseComposeUserdataMounts(composePath, userdataPath string) []string { // already covers it (an app binding ${HDD_PATH} itself), and HDD mounts inside the userdata root // are dropped when the root is added (a literal ${HDD_PATH}/userdata/x bind would otherwise // double-tar and basename-collide with the root). -func ExportDataMounts(composePath, hddPath string) []string { +// R-203 — nsRoot is the app's felhom-data NAMESPACE ROOT, which is what UserdataDir takes. It is +// NOT hddPath: identical on an enrolled drive, one segment shorter on the system-data fallback. +// +// SCOPE NOTE, because this function lives in delete.go and that is misleading: it is EXPORT-only. +// Its single production caller is the .fab export adapter (cmd/controller/main.go, exportAdapter. +// GetStackDataMounts). Nothing deletes based on this result. The delete path's own guard, +// ProtectedHDDPaths above, is layout-agnostic by construction — it protects BOTH /... and +// /felhom-data/... — so it was never affected by the namespace-root defect. +func ExportDataMounts(composePath, hddPath, nsRoot string) []string { if hddPath == "" { return nil } hddMounts := ParseComposeHDDMounts(composePath, hddPath) - ud := appbackup.UserdataDir(filepath.Clean(hddPath)) + if nsRoot == "" { + nsRoot = hddPath // an enrolled drive, or a caller with nothing better — the pre-R-203 shape + } + ud := appbackup.UserdataDir(filepath.Clean(nsRoot)) if len(ParseComposeUserdataMounts(composePath, ud)) == 0 { return hddMounts } diff --git a/controller/internal/stacks/export_mounts_test.go b/controller/internal/stacks/export_mounts_test.go index 6a81e2f..d4ae30b 100644 --- a/controller/internal/stacks/export_mounts_test.go +++ b/controller/internal/stacks/export_mounts_test.go @@ -4,6 +4,7 @@ import ( "os" "path/filepath" "reflect" + "strings" "testing" ) @@ -47,7 +48,7 @@ func TestExportDataMounts_UserdataConvention(t *testing.T) { t.Fatalf("precondition drifted: ParseComposeHDDMounts found %v — the compose shape no longer reproduces C6B-F1", old) } - got := ExportDataMounts(compose, hdd) + got := ExportDataMounts(compose, hdd, hdd) want := []string{filepath.Join(hdd, "userdata")} if !reflect.DeepEqual(got, want) { t.Fatalf("ExportDataMounts = %v, want the userdata ROOT %v", got, want) @@ -69,7 +70,7 @@ func TestExportDataMounts_HDDDirectAppUnchanged(t *testing.T) { - ${HDD_PATH}/incoming:/incoming `) hdd := filepath.Join(string(filepath.Separator)+"mnt", "drive", "app") - got := ExportDataMounts(compose, hdd) + got := ExportDataMounts(compose, hdd, hdd) want := []string{filepath.Join(hdd, "data"), filepath.Join(hdd, "incoming")} if !reflect.DeepEqual(got, want) { t.Fatalf("ExportDataMounts = %v, want unchanged HDD mounts %v (regression: the one working app must keep working)", got, want) @@ -85,7 +86,7 @@ func TestExportDataMounts_MixedBindsUnion(t *testing.T) { - ${USERDATA_PATH}/media:/media `) hdd := filepath.Join(string(filepath.Separator)+"mnt", "drive", "app") - got := ExportDataMounts(compose, hdd) + got := ExportDataMounts(compose, hdd, hdd) want := []string{filepath.Join(hdd, "direct"), filepath.Join(hdd, "userdata")} if !reflect.DeepEqual(got, want) { t.Fatalf("ExportDataMounts = %v, want the union %v", got, want) @@ -103,7 +104,7 @@ func TestExportDataMounts_HDDRootCoversUserdata(t *testing.T) { - ${USERDATA_PATH}/media:/media `) hdd := filepath.Join(string(filepath.Separator)+"mnt", "drive", "app") - got := ExportDataMounts(compose, hdd) + got := ExportDataMounts(compose, hdd, hdd) want := []string{hdd} if !reflect.DeepEqual(got, want) { t.Fatalf("ExportDataMounts = %v, want just the covering HDD root %v", got, want) @@ -121,7 +122,7 @@ func TestExportDataMounts_LiteralUserdataBindDeduped(t *testing.T) { - ${USERDATA_PATH}/bar:/bar `) hdd := filepath.Join(string(filepath.Separator)+"mnt", "drive", "app") - got := ExportDataMounts(compose, hdd) + got := ExportDataMounts(compose, hdd, hdd) want := []string{filepath.Join(hdd, "userdata")} if !reflect.DeepEqual(got, want) { t.Fatalf("ExportDataMounts = %v, want only the userdata root %v", got, want) @@ -130,7 +131,71 @@ func TestExportDataMounts_LiteralUserdataBindDeduped(t *testing.T) { func TestExportDataMounts_EmptyHDDPath(t *testing.T) { compose := writeCompose(t, sonarrShapeCompose) - if got := ExportDataMounts(compose, ""); got != nil { + if got := ExportDataMounts(compose, "", ""); got != nil { t.Fatalf("ExportDataMounts with empty HDD_PATH = %v, want nil", got) } } + +// R-203 / Scenario E — the export-mount resolver on BOTH drive kinds, and what it does NOT return. +// +// This function lives in delete.go, which reads as a destructive path; it is not. Its only production +// caller is the .fab export adapter. The assertions below therefore prove the NEGATIVE that matters +// for an export: no path outside the app's own data roots is ever emitted. +func TestExportDataMounts_BothDriveKinds(t *testing.T) { + const sys = "/mnt/sys_drive" + compose := writeCompose(t, `services: + app: + volumes: + - ${USERDATA_PATH}/media/books:/books + - ${HDD_PATH}/appdata/app:/data +`) + cases := []struct { + name, hdd, nsRoot, wantUD string + }{ + {"enrolled drive", "/mnt/felhom-usb", "/mnt/felhom-usb", "/mnt/felhom-usb/userdata"}, + {"system drive", sys, sys + "/felhom-data", sys + "/felhom-data/userdata"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := ExportDataMounts(compose, tc.hdd, tc.nsRoot) + var sawUD bool + for _, m := range got { + if m == tc.wantUD { + sawUD = true + } + // THE NEGATIVE: nothing outside the app's own roots, on either kind. + if !strings.HasPrefix(m, tc.hdd) && !strings.HasPrefix(m, tc.nsRoot) { + t.Errorf("emitted %q, which is outside both %q and %q", m, tc.hdd, tc.nsRoot) + } + for _, forbidden := range []string{"/etc", "/opt/docker/stacks", "/var/lib/docker", "/root"} { + if strings.HasPrefix(m, forbidden) { + t.Errorf("emitted a path outside the app's data: %q", m) + } + } + } + if !sawUD { + t.Errorf("userdata root %q not emitted; got %v", tc.wantUD, got) + } + }) + } +} + +// An empty nsRoot falls back to hddPath — the pre-R-203 shape — so a caller that has not been +// updated keeps working on enrolled drives rather than emitting an empty-rooted path. +func TestExportDataMounts_EmptyNamespaceRootFallsBack(t *testing.T) { + compose := writeCompose(t, `services: + app: + volumes: + - ${USERDATA_PATH}/x:/x +`) + withNS := ExportDataMounts(compose, "/mnt/felhom-usb", "/mnt/felhom-usb") + without := ExportDataMounts(compose, "/mnt/felhom-usb", "") + if len(withNS) != len(without) { + t.Fatalf("fallback changed the result: %v vs %v", withNS, without) + } + for i := range withNS { + if withNS[i] != without[i] { + t.Fatalf("fallback changed %q to %q", withNS[i], without[i]) + } + } +}