v0.244.0: the backup page stops promising what it does not hold (R-537/R-538/R-536)

R-537 — the contents label is now PER TIER. One string computed from the app's shape was rendered on all three tier rows; a Tier-1 unit has no file-copy step, so for the four class-A apps it was claiming „Adatok" for files it does not hold. R-538 — a unit restore REFUSES before anything is touched when the unit cannot return the app's drive-side files, and names the route that can. It runs before the stack is stopped because the measured harm included the app's own wastebasket going unreachable, which still held every byte. R-536 — „Alkalmazás telepítve" moved from the deploy's acceptance to its completion, with app_deploy_started and app_deploy_failed as the honest pair. Each fix red-proofed: seen failing with its own sentence, passing when restored. Requires hub v0.116.0 for the two new event types. MinAgent unchanged (0.131.0). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS
2026-09-16 16:55:55 +02:00
parent 383a30b3c0
commit 2f8ff2414c
16 changed files with 640 additions and 50 deletions
@@ -0,0 +1,56 @@
+package backup
+
+import (
+	"strings"
+
+	"gitea.dooplex.hu/admin/felhom-controller/internal/appbackup"
+)
+
+// DeclaredDriveFileLegs answers ONE question, for the label and for the restore guard alike: which
+// of this app's own files live on the customer's DATA DRIVE rather than inside a Docker volume?
+//
+// R-537 / R-538 (measured 2026-09-16 on a fresh box). A Tier-1 recovery unit captures compose +
+// app.yaml + the DB dumps and volume tars that already exist beside it — `CaptureRecoveryUnit` has
+// no file-copy step at all, and `RecoveryManifest` has no field to record one. The whole-guest tiers
+// do not carry them either (`mp8 /mnt/felhom-drives` is a bind mount and vzdump logs
+// "excluding bind mount point mp8 … (not a volume)"). So for the four class-A apps the drive-side
+// paths are carried by Tier 2 and Tier 3 ONLY — which is the design (07-backup-architecture §6.2),
+// and is exactly why a page that says „Adatok" over a Tier-1 unit, or a restore that replays a
+// database over files it does not have, is a lie rather than a design choice.
+//
+// It returns the DECLARED mandatory paths, resolved but deliberately NOT stat-filtered. The filter
+// belongs to a capture (a declared path that is missing on disk is a capture gap, and
+// `offboxCaptureSet` warns about it there). Here the question is what the app CLAIMS to keep on the
+// drive, and an empty folder the customer has not filled yet must still count — otherwise the label
+// tells the truth today and starts lying the moment they use the app.
+//
+// Empty for: no stack provider, a legacy app with no `backup:` block (nothing declares a namespace
+// path — all 45 class-B apps), or an app with no resolvable HDD_PATH (undeployed).
+func (m *Manager) DeclaredDriveFileLegs(stack string) []string {
+	if m.stackProvider == nil {
+		return nil
+	}
+	binds, has := m.stackProvider.GetStackClassifiedBinds(stack)
+	if !has {
+		return nil
+	}
+	hdd := strings.TrimSpace(m.stackProvider.GetStackHDDPath(stack))
+	if hdd == "" {
+		return nil
+	}
+	cs := appbackup.ComputeCaptureSet(binds, has, appbackup.TierOffsite, m.namespaceRoot(hdd), m.stackProvider.GetImportRoot())
+	out := make([]string, 0, len(cs.Paths))
+	for _, p := range cs.Paths {
+		out = append(out, p.Abs)
+	}
+	if len(out) == 0 {
+		return nil
+	}
+	return out
+}
+
+// HasDriveFileLegs is DeclaredDriveFileLegs as a predicate, for the surfaces that only need the
+// yes/no. Kept beside it so the two can never disagree.
+func (m *Manager) HasDriveFileLegs(stack string) bool {
+	return len(m.DeclaredDriveFileLegs(stack)) > 0
+}
@@ -0,0 +1,78 @@
+package backup
+
+import (
+	"errors"
+	"testing"
+)
+
+// R-538 — a unit restore must REFUSE when the unit holds no copy of the app's files.
+//
+// The defect this pins, measured live on 2026-09-16: five photos were put into Nextcloud, the
+// customer pressed „Visszaállítás indítása" on the Tier-1 unit, and the restore replayed three
+// volume tars and a database dump over an app whose files live on the data drive. It reported
+// „3 adatkötet és az adatbázis visszaállítva", and afterwards the folder listed all five photos and
+// none of them opened — the replayed database referenced files that were never captured, and it had
+// also stopped referencing the app's own wastebasket, which still held every byte.
+//
+// The assertion is the CONSEQUENCE, not the mechanism: the call returns the refusal and the app is
+// left alone. Red-proof: delete the guard in RestoreFromRecoveryUnitAtWith → this test fails at
+// "a restore that cannot return the files must refuse".
+func TestUnitRestore_RefusesWhenTheUnitCannotHoldTheFiles(t *testing.T) {
+	drive := t.TempDir()
+	m, _, prov := classifiedOffboxManager(t, drive)
+
+	// A class-A app: it declares a MANDATORY bind under the drive, which is where its files live and
+	// which a Tier-1 unit structurally cannot capture.
+	prov.hdd["nextcloud"] = drive
+	prov.binds["nextcloud"] = []ClassifiedBind{mandatoryHDD("appdata/nextcloud")}
+	prov.has["nextcloud"] = true
+	mkUnit(t, drive, "nextcloud")
+
+	_, err := m.RestoreFromRecoveryUnitAt("nextcloud", RecoveryUnitPath(drive, "nextcloud"))
+	var refusal *ErrUnitLacksFileLegs
+	if !errors.As(err, &refusal) {
+		t.Fatalf("a restore that cannot return the files must refuse; got err=%v", err)
+	}
+	if refusal.Stack != "nextcloud" || len(refusal.Paths) == 0 {
+		t.Fatalf("the refusal must name the app and the paths it cannot return: %+v", refusal)
+	}
+
+	// NEGATIVE CONTROL, and it is the half that keeps the guard from being over-broad: an app that
+	// declares no drive-side files (all 45 class-B templates, whose data IS in the volumes the unit
+	// captured) must NOT be refused. If this ever starts refusing, the guard has stopped asking about
+	// the unit and started asking about nothing in particular.
+	prov.hdd["privatebin"] = drive
+	prov.has["privatebin"] = false
+	mkUnit(t, drive, "privatebin")
+	_, err = m.RestoreFromRecoveryUnitAt("privatebin", RecoveryUnitPath(drive, "privatebin"))
+	if errors.As(err, &refusal) {
+		t.Fatalf("an app with no drive-side files must not be refused: %v", err)
+	}
+
+	// The explicit second step („csak az adatbázist és a beállításokat") passes the guard. It may
+	// still fail further down for unrelated fixture reasons — what is asserted is only that consent
+	// is what the guard consults.
+	_, err = m.RestoreFromRecoveryUnitAtWith("nextcloud", RecoveryUnitPath(drive, "nextcloud"), UnitRestoreOptions{AcceptMissingFiles: true})
+	if errors.As(err, &refusal) {
+		t.Fatalf("explicit consent must pass the guard, not be refused by it: %v", err)
+	}
+}
+
+// DeclaredDriveFileLegs is the ONE predicate the label (R-537) and the refusal (R-538) share. A
+// second copy of this question is how a page and a guard drift apart, so it is pinned here too.
+func TestDeclaredDriveFileLegs_IsAboutDeclarationNotExistence(t *testing.T) {
+	drive := t.TempDir()
+	m, _, prov := classifiedOffboxManager(t, drive)
+	prov.hdd["nextcloud"] = drive
+	prov.binds["nextcloud"] = []ClassifiedBind{mandatoryHDD("appdata/nextcloud")}
+	prov.has["nextcloud"] = true
+
+	// The folder does NOT exist on disk in this fixture. It must still count: a label that tells the
+	// truth only until the customer starts using the app is not telling the truth.
+	if !m.HasDriveFileLegs("nextcloud") {
+		t.Fatal("a declared mandatory drive path must count even before the customer has put anything in it")
+	}
+	if got := m.DeclaredDriveFileLegs("unknown-app"); got != nil {
+		t.Fatalf("an app the provider does not know has no declared legs; got %v", got)
+	}
+}
@@ -223,11 +223,61 @@ func (m *Manager) primaryUnitDirFor(stackName string) string {
 // start → replay → start, R-47), the secret reconciliation with unit-over-guest precedence and the
 // fail-closed data-key gate, and the no-unit fallback to RestoreApp with its CountsUnknown handling.
 func (m *Manager) RestoreFromRecoveryUnitAt(stackName, unitDir string) (UnitRestoreResult, error) {
+	return m.RestoreFromRecoveryUnitAtWith(stackName, unitDir, UnitRestoreOptions{})
+}
+
+// UnitRestoreOptions carries the caller's EXPLICIT consent for a restore this package would
+// otherwise refuse. It exists because of R-538, and it has exactly one member for now.
+type UnitRestoreOptions struct {
+	// AcceptMissingFiles lets a unit restore proceed for an app whose own files live on the data
+	// drive and are therefore NOT in the unit. The default — zero value, every existing caller — is
+	// to REFUSE, because the run that produced this option replayed a database over files that were
+	// never captured, reported „3 adatkötet és az adatbázis visszaállítva", and left Nextcloud
+	// listing five photos that returned `Sabre\DAV\Exception\NotFound`.
+	//
+	// It is a per-call argument and never a field on the Manager: a consent that outlives the act it
+	// was given for is not consent.
+	AcceptMissingFiles bool
+}
+
+// ErrUnitLacksFileLegs is the refusal R-538 asks for. It names the app and the paths that are NOT in
+// the unit, so the caller can build an honest sentence without re-deriving anything.
+type ErrUnitLacksFileLegs struct {
+	Stack string
+	Paths []string
+}
+
+func (e *ErrUnitLacksFileLegs) Error() string {
+	return fmt.Sprintf("%s: the recovery unit carries no copy of the app's files on the data drive (%s) — refusing to replay the database over them",
+		e.Stack, strings.Join(e.Paths, ", "))
+}
+
+// RestoreFromRecoveryUnitAtWith is RestoreFromRecoveryUnitAt with the caller's explicit consent
+// flags. See UnitRestoreOptions.
+func (m *Manager) RestoreFromRecoveryUnitAtWith(stackName, unitDir string, opt UnitRestoreOptions) (UnitRestoreResult, error) {
 	var res UnitRestoreResult
 	if m.stackProvider == nil {
 		return res, fmt.Errorf("stack provider not configured")
 	}

+	// R-538 — REFUSE BEFORE ANYTHING IS TOUCHED. This runs before the lock, before the stack is
+	// stopped and before a single volume is replaced, because the measured harm was not only the
+	// missing files: the replayed database also stopped referencing the app's OWN wastebasket, which
+	// still held every byte on the drive. A refusal that has already stopped the app has destroyed
+	// the customer's last route while declining to help them.
+	//
+	// The condition is about the UNIT, not the app class: a unit structurally cannot hold these
+	// paths (`CaptureRecoveryUnit` has no file-copy step, `RecoveryManifest` no field for one), and
+	// that is true of the Tier-2 mirror of a unit as well — Tier 2's file half is a separate action
+	// („Fájlok visszaállítása", RestoreTier2Files), which is exactly what the caller should offer.
+	if !opt.AcceptMissingFiles {
+		if legs := m.DeclaredDriveFileLegs(stackName); len(legs) > 0 {
+			m.logger.Printf("[WARN] [backup] unit restore REFUSED for %s: the unit carries no file leg; %d drive path(s) would be left as they are: %s",
+				stackName, len(legs), strings.Join(legs, ", "))
+			return res, &ErrUnitLacksFileLegs{Stack: stackName, Paths: legs}
+		}
+	}
+
 	m.mu.Lock()
 	if m.running {
 		m.mu.Unlock()