From bbd503375a65ead1bd7517a4df4bf5cfaed8f789 Mon Sep 17 00:00:00 2001 From: Fredrik Ahlgren Date: Mon, 12 Oct 2026 08:19:53 +0200 Subject: [PATCH] fix(backup): bind extraction stages to their restore operation --- .changeset/restore-stage-phase.md | 5 + go/internal/backup/archive.go | 6 ++ go/internal/backup/archive_test.go | 50 ++++++++++ go/internal/backup/restore_crash_test.go | 10 ++ go/internal/backup/restore_point.go | 119 +++++++++++++++++++---- go/internal/backup/restore_point_test.go | 30 +++++- 6 files changed, 198 insertions(+), 22 deletions(-) create mode 100644 .changeset/restore-stage-phase.md diff --git a/.changeset/restore-stage-phase.md b/.changeset/restore-stage-phase.md new file mode 100644 index 00000000..ef6f58a3 --- /dev/null +++ b/.changeset/restore-stage-phase.md @@ -0,0 +1,5 @@ +--- +"ftw": patch +--- + +Bind each restore stage to its own phase so a later extraction-only retry can keep older complete safety points without accepting partial activation. diff --git a/go/internal/backup/archive.go b/go/internal/backup/archive.go index 7e59f3fe..458c580f 100644 --- a/go/internal/backup/archive.go +++ b/go/internal/backup/archive.go @@ -882,6 +882,9 @@ func RestoreContents(archivePath, dataDir string, now time.Time) (RestoreResult, return RestoreResult{}, err } defer os.RemoveAll(staging) + if err := writeRestoreStage(dataDir, staging, ""); err != nil { + return RestoreResult{}, fmt.Errorf("backup: record extraction phase: %w", err) + } if err := extractArchive(archivePath, staging); err != nil { return RestoreResult{}, err } @@ -909,6 +912,9 @@ func RestoreContents(archivePath, dataDir string, now time.Time) (RestoreResult, if err := writeRestorePoint(safetyDir, current, false); err != nil { return RestoreResult{}, fmt.Errorf("backup: record previous file list: %w", err) } + if err := writeRestoreStage(dataDir, staging, filepath.Base(safetyDir)); err != nil { + return RestoreResult{}, fmt.Errorf("backup: record preservation phase: %w", err) + } movedCurrent := make([]string, 0, len(current)) for _, name := range current { if err := moveRestoreEntry(filepath.Join(dataDir, name), filepath.Join(safetyDir, name)); err != nil { diff --git a/go/internal/backup/archive_test.go b/go/internal/backup/archive_test.go index 443b8f86..46b7dc57 100644 --- a/go/internal/backup/archive_test.go +++ b/go/internal/backup/archive_test.go @@ -615,3 +615,53 @@ func TestExtractionRetryRetainsOldStageAndAllowsCompleteRevert(t *testing.T) { t.Fatalf("old extraction changed: %q %v", got, err) } } + +func TestExtractionRetryWithOlderCompleteRestorePoint(t *testing.T) { + root := t.TempDir() + source := filepath.Join(root, "source") + if err := os.Mkdir(source, 0o700); err != nil { + t.Fatal(err) + } + st, err := state.Open(filepath.Join(source, "state.db")) + if err != nil { + t.Fatal(err) + } + defer st.Close() + if err := st.SaveConfig("generation", "backup"); err != nil { + t.Fatal(err) + } + info, err := Create(context.Background(), CreateOptions{State: st, StatePath: filepath.Join(source, "state.db"), DataDir: source, OutputDir: filepath.Join(root, "backup")}) + if err != nil { + t.Fatal(err) + } + if err := st.Close(); err != nil { + t.Fatal(err) + } + target := filepath.Join(root, "target") + writeTestFile(t, filepath.Join(target, "old"), "original data") + first, err := RestoreContents(info.Path, target, time.Now()) + if err != nil { + t.Fatal(err) + } + stage := filepath.Join(target, ".ftw-restore-stage-second") + writeTestFile(t, filepath.Join(stage, "partial"), "partial extraction") + writeTestFile(t, filepath.Join(stage, restoreStageFile), `{"schema":1,"phase":"extracting"}`) + second, err := RestoreContents(info.Path, target, time.Now().Add(time.Second)) + if err != nil { + t.Fatalf("older point blocked extraction-only retry: %v", err) + } + if err := verifyRestorePoint(first.SafetyDir); err != nil { + t.Fatalf("older point changed: %v", err) + } + back, err := RevertContents(target, second.SafetyDir) + if err != nil || !back.SafetyComplete { + t.Fatalf("completed retry could not revert: %+v %v", back, err) + } + if _, err := RevertContents(target, second.SafetyDir); err != nil { + t.Fatal(err) + } + got, err := os.ReadFile(filepath.Join(target, ".ftw-abandoned-stage-second", "partial")) + if err != nil || string(got) != "partial extraction" { + t.Fatalf("old extraction bytes changed: %q %v", got, err) + } +} diff --git a/go/internal/backup/restore_crash_test.go b/go/internal/backup/restore_crash_test.go index 48707c0a..876ca7af 100644 --- a/go/internal/backup/restore_crash_test.go +++ b/go/internal/backup/restore_crash_test.go @@ -141,6 +141,16 @@ func TestRestoreProcessCrashPreservesDataAndRefusesPartialPoints(t *testing.T) { t.Fatalf("missing retained point: %v %v", points, err) } point := points[0] + if phase != "reverting" { + stages, err := filepath.Glob(filepath.Join(data, ".ftw-restore-stage-*")) + if err != nil || len(stages) != 1 { + t.Fatalf("stage association missing: %v %v", stages, err) + } + stage, err := readRestoreStage(stages[0]) + if err != nil || stage.Phase != "preserving" || stage.Safety != filepath.Base(point) { + t.Fatalf("stage not bound to its point: %+v %v", stage, err) + } + } if phase == "reverting" { active, err := topLevelEntries(data) if err != nil || len(active) != 0 { diff --git a/go/internal/backup/restore_point.go b/go/internal/backup/restore_point.go index 560de5e0..bdebd505 100644 --- a/go/internal/backup/restore_point.go +++ b/go/internal/backup/restore_point.go @@ -12,6 +12,13 @@ import ( ) const restorePointFile = ".ftw-restore-point.json" +const restoreStageFile = ".ftw-restore-stage-record.json" + +type restoreStage struct { + Schema int `json:"schema"` + Phase string `json:"phase"` + Safety string `json:"safety,omitempty"` +} type restorePoint struct { Schema int `json:"schema"` @@ -22,11 +29,20 @@ type restorePoint struct { // Record the file list before moving anything. A failed or interrupted write // remains unconfirmed; revert must not treat it as a complete previous state. func writeRestorePoint(dir string, entries []string, complete bool) error { - file, err := os.OpenFile(filepath.Join(dir, restorePointFile), os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o600) + return writeRestoreRecord(dir, restorePointFile, restorePoint{Schema: 1, Complete: complete, Entries: entries}) +} + +func writeRestoreRecord(dir, name string, record any) error { + if info, err := os.Lstat(filepath.Join(dir, name)); err == nil && !info.Mode().IsRegular() { + return errors.New("restore record is not a regular file") + } else if err != nil && !errors.Is(err, os.ErrNotExist) { + return err + } + file, err := os.OpenFile(filepath.Join(dir, name), os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o600) if err != nil { return err } - writeErr := json.NewEncoder(file).Encode(restorePoint{Schema: 1, Complete: complete, Entries: entries}) + writeErr := json.NewEncoder(file).Encode(record) if writeErr == nil { writeErr = file.Sync() } @@ -85,45 +101,106 @@ func verifyRestorePoint(dir string) error { return nil } -func hasRestoreStaging(entries []os.DirEntry) bool { - for _, entry := range entries { - if strings.HasPrefix(entry.Name(), ".ftw-restore-stage-") { - return true - } +// Associate each stage with its own phase before moving active files. Older +// complete safety points do not imply that a later extraction moved data. +func writeRestoreStage(dataDir, dir, safety string) error { + phase := "extracting" + if safety != "" { + phase = "preserving" + } + if err := writeRestoreRecord(dir, restoreStageFile, restoreStage{Schema: 1, Phase: phase, Safety: safety}); err != nil { + return err } - return false + return syncDir(dataDir) } -// A retry must not certify a partially moved active directory as a new, -// complete previous state. Interrupted extraction alone has no safety point -// and can still be retried. Unrecorded older points require a separate target. +func readRestoreStage(dir string) (restoreStage, error) { + var stage restoreStage + name := filepath.Join(dir, restoreStageFile) + info, err := os.Lstat(name) + if err != nil { + return stage, err + } + if !info.Mode().IsRegular() || info.Size() > 4096 { + return stage, errors.New("invalid stage record") + } + f, err := os.Open(name) + if err != nil { + return stage, err + } + defer f.Close() + d := json.NewDecoder(io.LimitReader(f, 4096)) + d.DisallowUnknownFields() + if err := d.Decode(&stage); err != nil { + return stage, err + } + if err := d.Decode(new(any)); err != io.EOF { + return stage, errors.New("invalid trailing stage record") + } + if stage.Schema != 1 || (stage.Phase != "extracting" && stage.Phase != "preserving") { + return stage, errors.New("unknown stage phase") + } + if stage.Phase == "extracting" && stage.Safety != "" { + return stage, errors.New("extraction already names a safety point") + } + if stage.Phase == "preserving" && (!strings.HasPrefix(stage.Safety, ".ftw-pre-restore-") || strings.ContainsAny(stage.Safety, `/\`)) { + return stage, errors.New("invalid stage safety reference") + } + return stage, nil +} + +// A retry must not certify a partially moved active directory as complete. +// Legacy stages without records remain retryable only when no safety point +// could be associated with them. Unknown or changed records always stop. func checkRestorePointsBeforeRestore(dataDir string) error { entries, err := os.ReadDir(dataDir) if err != nil { return err } - staging := hasRestoreStaging(entries) + points := 0 + unresolved := func(path string, err error) error { + return fmt.Errorf("backup: unresolved restore operation %s; retain all directories and recover a complete point or a full backup on a separate target: %w", path, err) + } for _, entry := range entries { - if strings.HasPrefix(entry.Name(), ".ftw-rejected-restore-") || (staging && strings.HasPrefix(entry.Name(), ".ftw-pre-restore-")) { - return fmt.Errorf("backup: unresolved interrupted restore at %s; retain all directories and recover a complete point or a full backup on a separate target", filepath.Join(dataDir, entry.Name())) + path := filepath.Join(dataDir, entry.Name()) + if strings.HasPrefix(entry.Name(), ".ftw-rejected-restore-") { + return unresolved(path, errors.New("revert did not finish")) } - if !strings.HasPrefix(entry.Name(), ".ftw-pre-restore-") && !strings.HasPrefix(entry.Name(), ".ftw-rejected-restore-") { + if !strings.HasPrefix(entry.Name(), ".ftw-pre-restore-") { continue } - point := filepath.Join(dataDir, entry.Name()) + points++ if !entry.IsDir() { - return fmt.Errorf("backup: unresolved restore point %s; retain the data and restore a full backup into a separate directory", point) + return unresolved(path, errors.New("point is not a directory")) } - if err := verifyRestorePoint(point); err != nil { - return fmt.Errorf("backup: unresolved restore point %s; retain the data and restore a full backup into a separate directory: %w", point, err) + if err := verifyRestorePoint(path); err != nil { + return unresolved(path, err) + } + } + for _, entry := range entries { + if !strings.HasPrefix(entry.Name(), ".ftw-restore-stage-") { + continue + } + path := filepath.Join(dataDir, entry.Name()) + if !entry.IsDir() { + return unresolved(path, errors.New("stage is not a directory")) + } + stage, err := readRestoreStage(path) + if errors.Is(err, os.ErrNotExist) && points == 0 { + continue + } + if err != nil { + return unresolved(path, err) + } + if stage.Phase != "extracting" { + return unresolved(path, fmt.Errorf("preservation started for %s", stage.Safety)) } } return nil } func currentRestoreStateComplete(dataDir string) bool { - entries, err := os.ReadDir(dataDir) - return err == nil && !hasRestoreStaging(entries) && checkRestorePointsBeforeRestore(dataDir) == nil + return checkRestorePointsBeforeRestore(dataDir) == nil } func rollbackRestorePoint(pointDir, dataDir string, moved []string) error { diff --git a/go/internal/backup/restore_point_test.go b/go/internal/backup/restore_point_test.go index 2afdf276..74babf67 100644 --- a/go/internal/backup/restore_point_test.go +++ b/go/internal/backup/restore_point_test.go @@ -24,7 +24,7 @@ func TestRevertRejectsInterruptedPreservationWithoutMovingFiles(t *testing.T) { } // An older unrecorded point is also refused without staging: an // older error path may have cleaned staging after a failed rollback. - if _, err := RestoreContents("unused.ftwbak", data, time.Time{}); err == nil || !strings.Contains(err.Error(), "unresolved restore point") { + if _, err := RestoreContents("unused.ftwbak", data, time.Time{}); err == nil || !strings.Contains(err.Error(), "unresolved restore") { t.Fatalf("retry did not reject the incomplete root before import: %v", err) } for path, want := range map[string]string{ @@ -213,3 +213,31 @@ func TestRetryImportRejectsInterruptedActivationWithCompletePoint(t *testing.T) t.Fatalf("import certified partial activation: %v", err) } } + +func TestRestoreStageRejectsLinkedAndUnknownRecords(t *testing.T) { + for _, body := range []string{`{"schema":2,"phase":"extracting"}`, `{"schema":1,"phase":"unknown"}`, `{"schema":1,"phase":"extracting","safety":".ftw-pre-restore-old"}`, `{"schema":1,"phase":"preserving","safety":"../outside"}`, `{"schema":1,"phase":"extracting"} trailing`} { + t.Run(body, func(t *testing.T) { + dir := t.TempDir() + writeTestFile(t, filepath.Join(dir, restoreStageFile), body) + if _, err := readRestoreStage(dir); err == nil { + t.Fatal("invalid stage accepted") + } + }) + } + dir := t.TempDir() + external := filepath.Join(t.TempDir(), "unchanged") + writeTestFile(t, external, "retain these bytes") + if err := os.Symlink(external, filepath.Join(dir, restoreStageFile)); err != nil { + t.Skipf("symlink unavailable: %v", err) + } + if err := writeRestoreStage(dir, dir, ""); err == nil { + t.Fatal("stage write followed a link") + } + if _, err := readRestoreStage(dir); err == nil { + t.Fatal("stage read followed a link") + } + got, err := os.ReadFile(external) + if err != nil || string(got) != "retain these bytes" { + t.Fatalf("linked data changed: %q %v", got, err) + } +}