R-1251: sync the restore copy's folders before the marker names it (a cut left undo.json as .tmp on Tester 1); the interrupted-restore record says the put-back waits, not 'run it again'. Commit only.

2026-10-24 19:36:32 +02:00
parent bb2809dc68
commit 062f9b41d9
12 changed files with 141 additions and 16 deletions
@@ -22,6 +22,9 @@ type RestoreOpResult struct {
 	// PutBack marks an interrupted restore that the start-up put the data back for (D22, R-1195): the message then
 	// says the app's data is as it was before the restore, not „start it again".
 	PutBack bool `json:"put_back,omitempty"`
+	// PutBackWaiting (R-1251) marks an interrupted restore whose put-back waits for the app's drive: the message says the
+	// box puts the data back when the drive is there — not „start it again", which would restore the backup over it.
+	PutBackWaiting bool `json:"put_back_waiting,omitempty"`
 }

 // RestoreResultWindow bounds how long a finished restore still counts as "what just happened".
@@ -318,3 +318,44 @@ func TestR1251_ACompletedRestoreDropsTheWaitingPutBack(t *testing.T) {
 		t.Fatal("a completed restore must drop the waiting put-back hold")
 	}
 }
+
+// MEASURED on Tester 1 (2026-10-24, audits/restore-holes-2026-10-24/P1-tester1/): a power cut 2.1 s after the copy was
+// recorded left its journal as undo.json.tmp — the marker named a copy the start could not read. The copy's folders
+// must be synced BEFORE the marker names it. RED-PROOF: with the syncUndoDirs call removed from persistUndoForCrash this
+// fails on „no folder was synced before the marker named the copy".
+func TestR1251_TheCopyIsOnDiskBeforeTheMarkerNamesIt(t *testing.T) {
+	root := t.TempDir()
+	marker := filepath.Join(root, "appstop-state.json")
+	copyDir := filepath.Join(root, "drive", "backups", "restore-undo", "app", "20261024T171928Z")
+	if err := os.MkdirAll(copyDir, 0o700); err != nil {
+		t.Fatal(err)
+	}
+	g := NewAppStopGuard(marker, log.New(io.Discard, "", 0))
+	if err := g.Begin("unit-restore:app", ReasonUnitRestore, []string{"app"}); err != nil {
+		t.Fatal(err)
+	}
+	m := &Manager{logger: log.New(io.Discard, "", 0), appStop: g}
+	var synced []string
+	markerNamedCopyAtSync := false
+	orig := syncUndoDirs
+	t.Cleanup(func() { syncUndoDirs = orig })
+	syncUndoDirs = func(dir string) error {
+		b, _ := os.ReadFile(marker)
+		markerNamedCopyAtSync = strings.Contains(string(b), dir)
+		synced = append(synced, dir)
+		return orig(dir)
+	}
+	m.persistUndoForCrash("app", &liveVolumeCopy{dir: copyDir, copied: []string{"app_data"}})
+	if len(synced) != 1 || synced[0] != copyDir {
+		t.Fatalf("no folder was synced before the marker named the copy: %v", synced)
+	}
+	if markerNamedCopyAtSync {
+		t.Fatal("the marker already named the copy when its folders were synced — the order is the fix")
+	}
+	if b, _ := os.ReadFile(marker); !strings.Contains(string(b), copyDir) {
+		t.Fatal("the copy must still be recorded in the marker")
+	}
+	if _, err := os.Stat(filepath.Join(copyDir, "undo.json")); err != nil {
+		t.Fatal("the journal must be in place")
+	}
+}
@@ -73,6 +73,15 @@ func (m *Manager) persistUndoForCrash(stack string, c *liveVolumeCopy) {
 		m.logger.Printf("[WARN] [backup] %s: the restore copy's journal could not be written: %v — a power cut would restart the app without putting it back", stack, err)
 		return
 	}
+	// MEASURED 2026-10-24 on Tester 1 (audits/restore-holes-2026-10-24/P1-tester1/): a power cut 2.1 s after this
+	// record left the copy's tar whole but its journal as undo.json.tmp — the rename lived only in the page cache, and
+	// the marker (another filesystem, already synced) named a copy no start could read. The copy's folders are new
+	// (copyLiveVolumes made them), so their own entries are not durable either. Every folder from the copy up to
+	// backups/ is synced BEFORE the marker names the copy; a sync that fails is logged and the copy is still recorded
+	// (an unreadable copy at start holds the app for support, R-1251 H2 — never a start on unknown data).
+	if err := syncUndoDirs(c.dir); err != nil {
+		m.logger.Printf("[WARN] [backup] %s: the restore copy's folders could not be synced to disk: %v — a power cut in the next seconds may leave a copy the start cannot read (the app is then held)", stack, err)
+	}
 	if err := m.appStop.SetUndo(stack, c.dir); err != nil {
 		m.logger.Printf("[WARN] [backup] %s: the restore copy could not be recorded for a power cut: %v", stack, err)
 		return
@@ -100,7 +109,38 @@ func copyDefinitionFiles(stackDir, dst string) error {
 	return nil
 }

-// atomicWriteFile writes data to path through a synced temp file and a rename (0600).
+// syncUndoDirs makes a restore copy's folder entries durable: its definition/ folder, the copy folder, and each parent
+// up to and including backups/ (restoreUndoDir = <ns>/backups/restore-undo/<app>/<stamp>). A seam for tests.
+var syncUndoDirs = func(copyDir string) error {
+	dirs := []string{filepath.Join(copyDir, undoDefinitionIn), copyDir}
+	for d, i := copyDir, 0; i < 3; i++ { // <app>, restore-undo, backups
+		d = filepath.Dir(d)
+		dirs = append(dirs, d)
+	}
+	var first error
+	for _, d := range dirs {
+		if err := syncDir(d); err != nil && !os.IsNotExist(err) && first == nil {
+			first = err
+		}
+	}
+	return first
+}
+
+// syncDir fsyncs one directory, so the entries created or renamed in it survive a power cut.
+func syncDir(dir string) error {
+	f, err := os.Open(dir)
+	if err != nil {
+		return err
+	}
+	if err := f.Sync(); err != nil {
+		f.Close()
+		return err
+	}
+	return f.Close()
+}
+
+// atomicWriteFile writes data to path through a synced temp file and a rename (0600), then syncs the folder so the
+// rename itself survives a power cut (R-1251: a journal left as .tmp was measured after a cut).
 func atomicWriteFile(path string, data []byte) error {
 	tmp := path + ".tmp"
 	f, err := os.OpenFile(tmp, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o600)
@@ -121,7 +161,10 @@ func atomicWriteFile(path string, data []byte) error {
 		os.Remove(tmp)
 		return err
 	}
-	return os.Rename(tmp, path)
+	if err := os.Rename(tmp, path); err != nil {
+		return err
+	}
+	return syncDir(filepath.Dir(path))
 }

 // CrashPutBack is the InterruptedRestorePutBacker the controller wires at start, before the backup manager exists.
@@ -33,6 +33,9 @@ const RestoreInterruptedKey = "note.restore.interrupted"
 // (D22, R-1195): the data is as it was before the restore.
 const RestoreInterruptedPutBackKey = "note.restore.interrupted_put_back"

+// RestoreInterruptedWaitingKey (R-1251): the interrupted restore's put-back waits for the app's drive.
+const RestoreInterruptedWaitingKey = "note.restore.interrupted_put_back_waiting"
+
 // restoreRecordFile is the on-disk shape.
 type restoreRecordFile struct {
 	Running     bool                       `json:"running"`
@@ -53,12 +56,12 @@ func (m *Manager) SetRestoreRecordPath(path string) {
 // LoadRestoreRecord restores the record at startup. It returns the restore that was interrupted by the
 // stop — non-nil exactly once per interruption — so the caller can log it and raise restore_interrupted.
 func (m *Manager) LoadRestoreRecord() *RestoreOpResult {
-	return m.LoadRestoreRecordAfterRecovery(nil)
+	return m.LoadRestoreRecordAfterRecovery(nil, nil)
 }

 // LoadRestoreRecordAfterRecovery is LoadRestoreRecord told which apps the start-up crash recovery put back
 // (AppStopRecovery.PutBack, D22 / R-1195): for those the interrupted restore reads „the data is as before".
-func (m *Manager) LoadRestoreRecordAfterRecovery(putBack []string) *RestoreOpResult {
+func (m *Manager) LoadRestoreRecordAfterRecovery(putBack, waiting []string) *RestoreOpResult {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 	if m.opRecordPath == "" {
@@ -95,6 +98,11 @@ func (m *Manager) LoadRestoreRecordAfterRecovery(putBack []string) *RestoreOpRes
 			res.Message, res.PutBack = m.note(RestoreInterruptedPutBackKey), true
 		}
 	}
+	for _, w := range waiting {
+		if w == rec.Stack {
+			res.Message, res.PutBackWaiting = m.note(RestoreInterruptedWaitingKey), true
+		}
+	}
 	m.opLast = &res
 	if m.opInterrupted == nil {
 		m.opInterrupted = map[string]RestoreOpResult{}
@@ -102,7 +102,7 @@ func TestRestoreRecord_NextRestoreOfThatAppClearsTheNotice(t *testing.T) {
 func TestR1195_TheRecordSaysTheDataIsAsBefore(t *testing.T) {
 	path := filepath.Join(t.TempDir(), "restore-status.json")
 	recordManager(path).BeginRestoreOp("offbox-restore", "bookstack")
-	got := recordManager(path).LoadRestoreRecordAfterRecovery([]string{"bookstack"})
+	got := recordManager(path).LoadRestoreRecordAfterRecovery([]string{"bookstack"}, nil)
 	if got == nil || !got.PutBack || !got.Interrupted {
 		t.Fatalf("want an interrupted, put-back record, got %+v", got)
 	}
@@ -112,8 +112,23 @@ func TestR1195_TheRecordSaysTheDataIsAsBefore(t *testing.T) {

 	path2 := filepath.Join(t.TempDir(), "restore-status.json")
 	recordManager(path2).BeginRestoreOp("restore", "gokapi")
-	other := recordManager(path2).LoadRestoreRecordAfterRecovery([]string{"bookstack"})
+	other := recordManager(path2).LoadRestoreRecordAfterRecovery([]string{"bookstack"}, nil)
 	if other == nil || other.PutBack || !strings.Contains(other.Message, "megszakadt") {
 		t.Fatalf("an app that was not put back keeps the old sentence, got %+v", other)
 	}
 }
+
+// R-1251: a restore whose put-back waits for the app's drive must not tell the household to run it again (that restores
+// the backup over the data the box is about to put back). MEASURED 2026-10-24 on Tester 1: the event said „indítsd el
+// újra". RED-PROOF: with the waiting loop removed from LoadRestoreRecordAfterRecovery this fails on the old sentence.
+func TestR1251_TheRecordSaysThePutBackWaits(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "restore-status.json")
+	recordManager(path).BeginRestoreOp("restore", "metube")
+	got := recordManager(path).LoadRestoreRecordAfterRecovery(nil, []string{"metube"})
+	if got == nil || !got.PutBackWaiting || got.PutBack || !got.Interrupted {
+		t.Fatalf("want an interrupted, put-back-waiting record, got %+v", got)
+	}
+	if !strings.Contains(got.Message, "meghajt") || strings.Contains(got.Message, "indítsd el újra") {
+		t.Fatalf("message %q must say the box puts the data back when the drive is there", got.Message)
+	}
+}
@@ -1872,6 +1872,7 @@
  "note.restore.full_failed": "The full restore failed: %s",
  "note.restore.full_unit_failed": "Full restore failed: %s",
  "note.restore.interrupted": "The restore was interrupted (the box restarted) — start it again.",
+  "note.restore.interrupted_put_back_waiting": "The restore was interrupted (a power cut). The app does not start until its drive is back: then the box puts its data from before the restore back and starts it. Do not run the restore again, because that puts the backup in place of your data.",
  "note.restore.interrupted_put_back": "The restore was interrupted (a power cut). The data of the app is as it was before the restore.",
  "note.restore.no_db_at_all": " This app has no database.",
  "note.restore.no_db_in_backup": " WARNING: this app HAS a database, but the backup held no database dump, so the database did NOT come back. The state from before the restore is backed up: %s",
@@ -1860,6 +1860,7 @@
  "note.restore.full_failed": "A teljes visszaállítás sikertelen: %s",
  "note.restore.full_unit_failed": "Teljes visszaállítás sikertelen: %s",
  "note.restore.interrupted": "A visszaállítás megszakadt (a doboz újraindult) — indítsd el újra.",
+  "note.restore.interrupted_put_back_waiting": "A visszaállítás félbeszakadt (áramszünet). Az alkalmazás nem indul el, amíg a meghajtója vissza nem tér: akkor a doboz visszateszi a visszaállítás előtti adatait, és elindítja. Ne indítsd újra a visszaállítást, mert az a mentést tölti vissza az adataid helyére.",
  "note.restore.interrupted_put_back": "A visszaállítás félbeszakadt (áramszünet). Az alkalmazás adatai a visszaállítás előtti állapotban vannak.",
  "note.restore.no_db_at_all": " Ennek az alkalmazásnak nincs adatbázisa.",
  "note.restore.no_db_in_backup": " FIGYELEM: ennek az alkalmazásnak VAN adatbázisa, de a mentés nem tartalmazott adatbázis-mentést, ezért az adatbázis NEM állt vissza. A visszaállítás előtti állapot mentése megvan: %s",