From 7e15a374b2b7405c0a3653c29fd8225c643cbe9c Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Mon, 26 Oct 2026 20:13:44 +0100 Subject: [PATCH] =?UTF-8?q?fix(R-1220):=20an=20unreadable=20app.yaml=20cou?= =?UTF-8?q?nts=20as=20installed=20=E2=80=94=20its=20folders=20are=20never?= =?UTF-8?q?=20kept=20data?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit liveDriveBinds(drives): an app whose app.yaml exists but does not parse binds every folder its compose names on every drive (fail closed); WARN per app. Red-proved (the live folder was listed). Commit only. --- CHANGELOG.md | 5 +++ controller/internal/stacks/kept.go | 34 ++++++++++++++++-- .../stacks/r1220_unreadable_appyaml_test.go | 36 +++++++++++++++++++ 3 files changed, 73 insertions(+), 2 deletions(-) create mode 100644 controller/internal/stacks/r1220_unreadable_appyaml_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 10ba3432..f5a67c74 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,11 @@ hub answers 400 to both and nothing else changes. as `restore_failed` — `since`, `why`), `app_hold_cleared` when a hold is gone; every hold is sent again once a day and at each start (`repeat: true`). The last-reported set is kept in `settings.reported_holds`, so a hold cleared while the controller was down is still reported cleared. Both severity info (the hub mails nothing on receipt). +- **R-1220 (P2, delete path):** an app whose `app.yaml` EXISTS but cannot be read counts as INSTALLED (fail closed): + `stacks.liveDriveBinds(drives)` adds every folder its compose binds on every given drive, so „Megőrzött adatok" never + lists — and never offers to delete — that app's live folder; one WARN names the app. Before: a parse error gave a nil + `AppConfig`, the app read as not installed, its folder was a „leftover". Test `TestR1220_AnUnreadableAppYamlKeepsTheFolderLive` + (with a real leftover as the control), red-proved against the old code. - `settings.RestoreHold.Why()` / `WireReason()`: the hold sentence moved out of `cmd/controller` (`holdWhy` calls it), so `--restore-holds` and the hub's list say the same thing. - Tests `internal/holdreport/holdreport_test.go` (8): the report field, the daily repeat, cleared once, cleared while diff --git a/controller/internal/stacks/kept.go b/controller/internal/stacks/kept.go index 5959bb33..a932d9f7 100644 --- a/controller/internal/stacks/kept.go +++ b/controller/internal/stacks/kept.go @@ -217,10 +217,31 @@ func readKeptMarker(dir string) *KeptMarker { } // liveDriveBinds is every drive folder an INSTALLED app binds — never kept data. -func (m *Manager) liveDriveBinds() []string { +// +// R-1220 — FAIL CLOSED ON AN UNREADABLE app.yaml. A parse error gives a nil AppConfig, so ScanStacks reads the app as +// „not installed"; before this, its live folder was then listed as kept data and offered for deletion. An app whose +// app.yaml EXISTS but cannot be read counts as installed: its HDD_PATH is unknown, so every folder its compose would bind +// on ANY of the given drives is live. One WARN per app per listing names it. Pinned by +// TestR1220_AnUnreadableAppYamlKeepsTheFolderLive. +func (m *Manager) liveDriveBinds(drives []string) []string { var out []string for _, st := range m.GetStacks() { + if st.ComposePath == "" { + continue + } if !st.Deployed || st.AppConfig == nil { + if !st.Deployed && !appYAMLUnreadable(filepath.Dir(st.ComposePath)) { + continue // not installed: no app.yaml, or one that reads as not deployed + } + m.logger.Printf("[WARN] [stacks] kept: %s has an app.yaml that cannot be read — it counts as INSTALLED, so its drive folders are never listed as kept data (R-1220)", st.Name) + for _, d := range drives { + if d = strings.TrimSpace(d); d == "" { + continue + } + for _, p := range ParseComposeHDDMounts(st.ComposePath, filepath.Clean(d)) { + out = append(out, filepath.Clean(p)) + } + } continue } hdd := strings.TrimSpace(st.AppConfig.Env["HDD_PATH"]) @@ -234,6 +255,15 @@ func (m *Manager) liveDriveBinds() []string { return out } +// appYAMLUnreadable is true when the stack folder holds an app.yaml that LoadAppConfig cannot turn into a config — the +// R-1220 case. A missing app.yaml is false (a template that was never installed, or a removed app). +func appYAMLUnreadable(stackDir string) bool { + if _, err := os.Stat(filepath.Join(stackDir, "app.yaml")); err != nil { + return false + } + return LoadAppConfig(stackDir) == nil +} + func overlaps(a, b string) bool { sep := string(filepath.Separator) return a == b || strings.HasPrefix(a, b+sep) || strings.HasPrefix(b, a+sep) @@ -284,7 +314,7 @@ var sizeFn = getDirSizeBytes // ListKept lists every kept item on the given drive roots: the dated folders under /kept//, // and each /appdata/ that holds something and that no installed app binds. Sorted newest first. func (m *Manager) ListKept(drives []string) []KeptItem { - live := m.liveDriveBinds() + live := m.liveDriveBinds(drives) var out []KeptItem seen := map[string]bool{} for _, d := range drives { diff --git a/controller/internal/stacks/r1220_unreadable_appyaml_test.go b/controller/internal/stacks/r1220_unreadable_appyaml_test.go new file mode 100644 index 00000000..f1c66a40 --- /dev/null +++ b/controller/internal/stacks/r1220_unreadable_appyaml_test.go @@ -0,0 +1,36 @@ +package stacks + +import ( + "os" + "path/filepath" + "testing" +) + +// R-1220: an installed app whose app.yaml cannot be read must count as INSTALLED (fail closed). Before the fix a parse +// error gave a nil AppConfig, so the app looked „not installed", its live drive folder was listed as kept data — and +// the household's Delete was offered on it. Red-proved against the code before the fix (the folder was listed). +func TestR1220_AnUnreadableAppYamlKeepsTheFolderLive(t *testing.T) { + m, drive := keptManager(t) + app := filepath.Join(filepath.Dir(m.stacks["cloudapp"].ComposePath)) + // An app.yaml that exists but does not parse (a half-written file, a disk error). + must(t, os.WriteFile(filepath.Join(app, "app.yaml"), []byte("deployed: true\nenv: [this is: not: yaml\n"), 0o600)) + must(t, m.ScanStacks()) + items := m.ListKept([]string{drive}) + for _, it := range items { + if it.App == "cloudapp" || it.Path == filepath.Join(drive, "appdata", "cloudapp") { + t.Fatalf("an app with an unreadable app.yaml had its live folder listed as kept: %+v", it) + } + } + if _, err := m.DeleteKept([]string{drive}, filepath.Join(drive, "appdata", "cloudapp")); err == nil { + t.Fatal("Delete was allowed on the folder of an app whose app.yaml cannot be read") + } + if _, err := os.Stat(filepath.Join(drive, "appdata/cloudapp/user1/file.txt")); err != nil { + t.Fatalf("the live folder's file is gone: %v", err) + } + // The control: with NO app.yaml at all (a real leftover), the folder IS listed. + must(t, os.Remove(filepath.Join(app, "app.yaml"))) + must(t, m.ScanStacks()) + if items := m.ListKept([]string{drive}); len(items) != 1 || items[0].App != "cloudapp" { + t.Fatalf("a real leftover (no app.yaml) is not listed: %+v", items) + } +}