diff --git a/internal/api/configapi/config_import_metadata_test.go b/internal/api/configapi/config_import_metadata_test.go new file mode 100644 index 0000000000..c72130ab32 --- /dev/null +++ b/internal/api/configapi/config_import_metadata_test.go @@ -0,0 +1,119 @@ +package configapi + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "reflect" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/config" +) + +func TestHandleImportConfigMetadataFailureDoesNotApplyOrSucceed(t *testing.T) { + for _, failure := range []string{"metadata_directory", "closed_store"} { + t.Run(failure, func(t *testing.T) { + const passphrase = "synthetic-import-validation-passphrase" + cfg := &config.Config{DataPath: t.TempDir(), PublicURL: "https://keep.invalid", AuthUser: "keep-admin", AuthPass: "synthetic-auth-hash"} + p := config.NewConfigPersistence(cfg.DataPath) + settings := config.DefaultSystemSettings() + settings.PublicURL = cfg.PublicURL + if err := p.SaveSystemSettings(*settings); err != nil { + t.Fatal(err) + } + if err := p.SaveAPITokens([]config.APITokenRecord{{ID: "keep-token", Hash: "synthetic-token-hash", OrgID: "default"}}); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(cfg.DataPath, ".env"), []byte("AUTO_UPDATE_ENABLED=true\nPOLLING_INTERVAL=77\n"), 0600); err != nil { + t.Fatal(err) + } + store := p.GetGuestMetadataStore() + if err := store.Set("keep:101", &config.GuestMetadata{Description: "Retained operator metadata"}); err != nil { + t.Fatal(err) + } + metadataBefore := store.GetAll() + if failure == "closed_store" { + if err := store.Close(time.Second); err != nil { + t.Fatal(err) + } + } else { + metadataPath := filepath.Join(cfg.DataPath, "guest_metadata.json") + if err := os.Remove(metadataPath); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(metadataPath, 0700); err != nil { + t.Fatal(err) + } + } + before := metadataImportFiles(t, cfg.DataPath) + beforeConfig := cfg.DeepCopy() + reloads, audits := 0, 0 + h := &ConfigHandlers{defaultConfig: cfg, defaultPersistence: p, reloadFunc: func() error { reloads++; return nil }} + h.SetRuntimeDependencies(RuntimeDependencies{AuditEvent: func(_, event, _, _, _ string, success bool, _ string) { + audits++ + if event != "config_imported" || success { + t.Error("failed metadata import produced a success audit") + } + }}) + plaintext, err := json.Marshal(map[string]any{ + "version": "4.4", "nodes": map[string]any{}, "alerts": map[string]any{}, + "email": map[string]any{}, "webhooks": nil, "system": map[string]any{}, + "guestMetadata": map[string]any{"imported:202": map[string]string{"description": failure}}, + }) + if err != nil { + t.Fatal(err) + } + request := ImportConfigRequest{Data: encryptedImportValidationFixture(t, plaintext, passphrase), Passphrase: passphrase} + body, err := json.Marshal(request) + if err != nil { + t.Fatal(err) + } + rec := httptest.NewRecorder() + h.HandleImportConfig(rec, httptest.NewRequest(http.MethodPost, "/api/config/import", bytes.NewReader(body))) + if rec.Code != http.StatusBadRequest || rec.Body.String() != "Failed to import configuration. Verify the backup file and passphrase are correct.\n" { + t.Fatalf("failed metadata import response = %d %q", rec.Code, rec.Body.String()) + } + if reloads != 0 || audits != 1 || !reflect.DeepEqual(beforeConfig, cfg.DeepCopy()) { + t.Error("failed import changed runtime or triggered apply") + } + if !reflect.DeepEqual(before, metadataImportFiles(t, cfg.DataPath)) || !reflect.DeepEqual(metadataBefore, store.GetAll()) { + t.Error("failed import changed persisted or live state") + } + readback, err := json.Marshal(map[string]any{"request": request, "status": rec.Code, "contentType": rec.Header().Get("Content-Type"), "body": rec.Body.String()}) + if err != nil { + t.Fatal(err) + } + t.Logf("SYNTHETIC_METADATA_IMPORT_REJECTION_READBACK %s", readback) + }) + } +} + +func metadataImportFiles(t *testing.T, dir string) map[string]string { + t.Helper() + files := make(map[string]string) + if err := filepath.WalkDir(dir, func(path string, entry os.DirEntry, err error) error { + if err != nil { + return err + } + info, err := entry.Info() + if err != nil { + return err + } + files[path] = info.Mode().String() + if !entry.IsDir() { + data, err := os.ReadFile(path) + if err != nil { + return err + } + files[path] += string(data) + } + return nil + }); err != nil { + t.Fatal(err) + } + return files +} diff --git a/internal/config/export.go b/internal/config/export.go index 26da1b366d..cd90fd08c5 100644 --- a/internal/config/export.go +++ b/internal/config/export.go @@ -258,15 +258,13 @@ func (c *ConfigPersistence) ImportConfig(encryptedData string, passphrase string } } - if err := tx.Commit(); err != nil { + if err := c.GetGuestMetadataStore().commitImport(tx, exportData.GuestMetadata); err != nil { return fmt.Errorf("failed to commit import transaction: %w", err) } committed = true - - // Import guest metadata into the active persistence scope. - if err := c.GetGuestMetadataStore().ReplaceAll(exportData.GuestMetadata); err != nil { - log.Warn().Err(err).Msg("Failed to import guest metadata") - } + c.mu.Lock() + c.cleanupOldBackups(c.nodesFile + ".backup-*") + c.mu.Unlock() return nil } diff --git a/internal/config/export_metadata_transaction_test.go b/internal/config/export_metadata_transaction_test.go new file mode 100644 index 0000000000..d917749ddd --- /dev/null +++ b/internal/config/export_metadata_transaction_test.go @@ -0,0 +1,178 @@ +package config + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "reflect" + "strings" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/notifications" +) + +func metadataImportArchive(t *testing.T, metadata map[string]*GuestMetadata) string { + t.Helper() + settings := DefaultSystemSettings() + settings.PublicURL = "https://imported.invalid" + data, err := json.Marshal(ExportData{ + Version: "4.4", System: *settings, + Email: notifications.EmailConfig{SMTPHost: "imported.invalid", Password: "synthetic-imported-secret"}, + APITokens: []APITokenRecord{{ID: "imported-token", Hash: "synthetic-imported-hash", OrgID: "default"}}, + GuestMetadata: metadata, + }) + if err != nil { + t.Fatal(err) + } + return encryptImportValidationData(t, data) +} + +func TestImportConfigMetadataFailurePreservesState(t *testing.T) { + for _, failure := range []string{"metadata_directory", "closed_store", "later_destination", "later_destination_without_metadata_file"} { + t.Run(failure, func(t *testing.T) { + cp := seedImportValidationState(t) + seedMetadataImportRecoveryArtifacts(t, cp) + store := cp.GetGuestMetadataStore() + metadataBefore := store.GetAll() + metadataFile := filepath.Join(cp.configDir, "guest_metadata.json") + var blocker string + switch failure { + case "metadata_directory": + blocker = metadataFile + case "closed_store": + if err := store.Close(time.Second); err != nil { + t.Fatal(err) + } + case "later_destination_without_metadata_file": + if err := os.Remove(metadataFile); err != nil { + t.Fatal(err) + } + blocker = cp.systemFile + case "later_destination": + blocker = cp.systemFile + } + if blocker != "" { + if err := os.Remove(blocker); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(blocker, 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(blocker, "keep"), []byte("retained blocker"), 0600); err != nil { + t.Fatal(err) + } + } + before := importValidationFiles(t, cp.configDir) + err := cp.ImportConfig(metadataImportArchive(t, map[string]*GuestMetadata{ + "imported:202": {Description: "Imported metadata", Tags: []string{"imported"}}, + }), "synthetic-import-validation-passphrase") + if err == nil { + t.Error("failed metadata recovery was reported as successful") + } + if !reflect.DeepEqual(before, importValidationFiles(t, cp.configDir)) { + t.Error("failed import changed files, credentials or transaction artifacts") + } + if !reflect.DeepEqual(metadataBefore, store.GetAll()) { + t.Error("failed import replaced live metadata") + } + }) + } +} + +func TestImportConfigCommitsMetadataWithConfiguration(t *testing.T) { + for _, inventory := range []string{"nil", "empty", "populated"} { + t.Run(inventory, func(t *testing.T) { + cp := seedImportValidationState(t) + seedMetadataImportRecoveryArtifacts(t, cp) + nodesBefore, err := os.ReadFile(cp.nodesFile) + if err != nil { + t.Fatal(err) + } + tokensBefore, err := os.ReadFile(cp.apiTokensFile) + if err != nil { + t.Fatal(err) + } + var metadata map[string]*GuestMetadata + expected := map[string]*GuestMetadata{} + if inventory == "empty" { + metadata = map[string]*GuestMetadata{} + } + if inventory == "populated" { + metadata = map[string]*GuestMetadata{ + "imported:202": {ID: "wrong-id", Description: "Imported description", Notes: []string{"retained note"}}, + "null-entry": nil, + } + expected["imported:202"] = &GuestMetadata{ID: "imported:202", Description: "Imported description", Notes: []string{"retained note"}, Tags: []string{}} + } + if err := cp.ImportConfig(metadataImportArchive(t, metadata), "synthetic-import-validation-passphrase"); err != nil { + t.Fatal(err) + } + for path, want := range map[string][]byte{cp.nodesFile + ".backup": nodesBefore, cp.apiTokensFile + ".backup": tokensBefore} { + if got, err := os.ReadFile(path); err != nil || string(got) != string(want) { + t.Error("successful import did not retain its previous configuration backup") + } + } + backups, err := filepath.Glob(cp.nodesFile + ".backup-*") + if err != nil || len(backups) != 10 { + t.Fatal("successful import did not apply the existing ten-backup retention") + } + env, err := os.ReadFile(filepath.Join(cp.configDir, ".env")) + if err != nil || strings.Contains(string(env), "POLLING_INTERVAL=") || !strings.Contains(string(env), "AUTO_UPDATE_ENABLED=false") || !strings.Contains(string(env), "KEEP_SETTING=retained") { + t.Fatal("successful import did not commit the existing legacy settings-file update") + } + if !reflect.DeepEqual(cp.GetGuestMetadataStore().GetAll(), expected) { + t.Error("successful import did not publish normalized metadata") + } + reloaded := NewGuestMetadataStore(cp.configDir, nil) + if !reflect.DeepEqual(reloaded.GetAll(), expected) { + t.Error("fresh metadata reader does not match the imported live state") + } + if err := cp.GetGuestMetadataStore().Load(); err != nil { + t.Fatalf("committed metadata cannot be reloaded: %v", err) + } + settings, err := cp.LoadSystemSettings() + if err != nil || settings.PublicURL != "https://imported.invalid" { + t.Fatal("successful import did not persist settings") + } + tokens, err := cp.LoadAPITokens() + if err != nil || len(tokens) != 1 || tokens[0].ID != "imported-token" { + t.Fatal("successful import did not persist credentials") + } + info, err := os.Stat(filepath.Join(cp.configDir, "guest_metadata.json")) + if err != nil || info.Mode().Perm() != 0600 { + t.Fatal("imported metadata is not a private file") + } + for path := range importValidationFiles(t, cp.configDir) { + if strings.HasPrefix(filepath.Base(path), ".import-") || strings.Contains(filepath.Base(path), ".import-backup-") { + t.Error("successful import retained transaction staging") + } + } + }) + } +} + +func seedMetadataImportRecoveryArtifacts(t *testing.T, cp *ConfigPersistence) { + t.Helper() + for path, data := range map[string]string{ + filepath.Join(cp.configDir, ".env"): "UPDATE_CHANNEL=stable\nAUTO_UPDATE_ENABLED=true\nPOLLING_INTERVAL=77\nKEEP_SETTING=retained\n", + cp.nodesFile + ".backup": "prior node backup", + cp.apiTokensFile + ".backup": "prior token backup", + } { + if err := os.WriteFile(path, []byte(data), 0600); err != nil { + t.Fatal(err) + } + } + // More than the normal retention bound makes premature pruning visible. + for i := 0; i < 11; i++ { + path := fmt.Sprintf("%s.backup-20260101-%06d", cp.nodesFile, i) + if err := os.WriteFile(path, []byte(fmt.Sprintf("retained backup %d", i)), 0600); err != nil { + t.Fatal(err) + } + stamp := time.Date(2000, 1, 1, 0, i, 0, 0, time.UTC) + if err := os.Chtimes(path, stamp, stamp); err != nil { + t.Fatal(err) + } + } +} diff --git a/internal/config/guest_metadata_import.go b/internal/config/guest_metadata_import.go new file mode 100644 index 0000000000..afd4b8bd6d --- /dev/null +++ b/internal/config/guest_metadata_import.go @@ -0,0 +1,52 @@ +package config + +import ( + "encoding/json" + "fmt" + "path/filepath" +) + +// commitImport includes guest metadata in the same file transaction as settings +// and credentials. Keep the store's writer and mutation locks through commit so +// an older background snapshot cannot overwrite the imported file, and readers +// cannot observe imported metadata before its configuration has committed. +func (s *GuestMetadataStore) commitImport(tx *importTransaction, metadata map[string]*GuestMetadata) error { + s.writeMu.Lock() + defer s.writeMu.Unlock() + s.mu.Lock() + defer s.mu.Unlock() + if s.closed.Load() { + return ErrGuestMetadataStoreClosed + } + + working := make(map[string]*GuestMetadata, len(metadata)) + for id, meta := range metadata { + if meta == nil { + continue + } + clone := cloneGuestMetadata(meta) + clone.ID = id + if clone.Tags == nil { + clone.Tags = []string{} + } + working[id] = clone + } + data, err := json.Marshal(working) + if err != nil { + return fmt.Errorf("serialize imported guest metadata: %w", err) + } + if err := tx.StageFile(filepath.Join(s.dataPath, "guest_metadata.json"), data, 0600); err != nil { + return fmt.Errorf("stage imported guest metadata: %w", err) + } + if err := tx.Commit(); err != nil { + return err + } + + // Publish only after every file committed. A pending writer will now see + // that this revision is durable instead of replaying a pre-import snapshot. + s.metadata = working + s.revision++ + s.attemptedRevision = s.revision + s.lastWriteErr = nil + return nil +} diff --git a/internal/config/guest_metadata_import_test.go b/internal/config/guest_metadata_import_test.go new file mode 100644 index 0000000000..433b51ba88 --- /dev/null +++ b/internal/config/guest_metadata_import_test.go @@ -0,0 +1,134 @@ +package config + +import ( + "encoding/json" + "os" + "path/filepath" + "reflect" + "sync" + "testing" + "time" +) + +type importBlockedMetadataFS struct { + FileSystem + once sync.Once + entered chan struct{} + release chan struct{} +} + +func (f *importBlockedMetadataFS) WriteFile(path string, data []byte, perm os.FileMode) error { + if filepath.Base(path) == "guest_metadata.json.tmp" { + f.once.Do(func() { close(f.entered); <-f.release }) + } + return f.FileSystem.WriteFile(path, data, perm) +} + +func TestGuestMetadataImportOrdersPendingSnapshotsAndCopiesInput(t *testing.T) { + dir := t.TempDir() + fs := &importBlockedMetadataFS{FileSystem: defaultFileSystem{}, entered: make(chan struct{}), release: make(chan struct{})} + store := NewGuestMetadataStore(dir, fs) + var releaseOnce sync.Once + release := func() { releaseOnce.Do(func() { close(fs.release) }) } + t.Cleanup(func() { + release() + if err := store.Close(time.Second); err != nil { + t.Error(err) + } + }) + if err := store.SetAsync("old:101", &GuestMetadata{Description: "Old snapshot"}); err != nil { + t.Fatal(err) + } + select { + case <-fs.entered: + case <-time.After(time.Second): + t.Fatal("pending metadata writer did not enter") + } + if err := store.RememberIdentity("old:101", "Old guest", "qemu"); err != nil { + t.Fatal(err) + } + tx, err := newImportTransaction(dir) + if err != nil { + t.Fatal(err) + } + defer tx.Cleanup() + settings := filepath.Join(dir, "system.json") + if err := os.WriteFile(settings, []byte("old settings"), 0600); err != nil { + t.Fatal(err) + } + if err := tx.StageFile(settings, []byte("imported settings"), 0600); err != nil { + t.Fatal(err) + } + metadata := map[string]*GuestMetadata{"new:202": {ID: "wrong", Description: "Imported snapshot", Tags: []string{"imported"}, Notes: []string{"note"}}} + done := make(chan error, 1) + go func() { done <- store.commitImport(tx, metadata) }() + select { + case err := <-done: + t.Fatalf("import overtook an in-flight metadata snapshot: %v", err) + case <-time.After(50 * time.Millisecond): + // A bounded negative observation, followed by the positive joined proof. + } + if data, err := os.ReadFile(settings); err != nil || string(data) != "old settings" { + t.Fatal("configuration committed while the old metadata write was still in flight") + } + release() + select { + case err := <-done: + if err != nil { + t.Fatal(err) + } + case <-time.After(5 * time.Second): + t.Fatal("import did not complete after the prior snapshot") + } + if !store.WaitForPendingWrites(time.Second) { + t.Fatal("pending writer did not drain") + } + expected := map[string]*GuestMetadata{"new:202": {ID: "new:202", Description: "Imported snapshot", Tags: []string{"imported"}, Notes: []string{"note"}}} + metadata["new:202"].Tags[0] = "caller mutation" + metadata["new:202"].Notes[0] = "caller mutation" + if !reflect.DeepEqual(store.GetAll(), expected) { + t.Fatal("import retained old records or caller-owned slices") + } + data, err := os.ReadFile(filepath.Join(dir, "guest_metadata.json")) + if err != nil { + t.Fatal(err) + } + var persisted map[string]*GuestMetadata + if err := json.Unmarshal(data, &persisted); err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(persisted, expected) { + t.Fatal("pending snapshot overwrote the imported metadata file") + } + if data, err := os.ReadFile(settings); err != nil || string(data) != "imported settings" { + t.Fatal("metadata import did not commit its settings") + } + if err := store.Load(); err != nil { + t.Fatalf("committed revision was left pending: %v", err) + } +} + +func TestGuestMetadataImportStageFailurePreservesMemory(t *testing.T) { + cp := seedImportValidationState(t) + store := cp.GetGuestMetadataStore() + before := store.GetAll() + tx, err := newImportTransaction(cp.configDir) + if err != nil { + t.Fatal(err) + } + // An occupied staging pathname is an actual filesystem failure before + // commit, not a simulated successful persistence result. + if err := os.Remove(tx.stagingDir); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(tx.stagingDir, []byte("block staging"), 0600); err != nil { + t.Fatal(err) + } + defer tx.Cleanup() + if err := store.commitImport(tx, map[string]*GuestMetadata{"new:202": {Description: "Do not publish"}}); err == nil { + t.Fatal("metadata staging failure was accepted") + } + if !reflect.DeepEqual(before, store.GetAll()) || tx.committed { + t.Fatal("staging failure changed live metadata or committed configuration") + } +} diff --git a/internal/config/import_transaction.go b/internal/config/import_transaction.go index 81949b9e0e..d8d85fe2c8 100644 --- a/internal/config/import_transaction.go +++ b/internal/config/import_transaction.go @@ -170,6 +170,12 @@ func (tx *importTransaction) Commit() error { continue } tx.backups[target] = "" + } else { + // This target did not exist before the import. Restoring its + // absence matters just as much as restoring an overwritten file. + if err := os.Remove(target); err != nil && !os.IsNotExist(err) { + joinRestoreErr(fmt.Errorf("remove new target %s during restore: %w", target, err)) + } } } return restoreErr @@ -203,11 +209,12 @@ func (tx *importTransaction) Commit() error { return failWithRestore(fmt.Errorf("stat destination %s: %w", target, err)) } + // Track the target before replacement: a failed rename after moving + // the original into a backup must restore that original too. + applied = append(applied, target) if err := os.Rename(stagedPath, target); err != nil { return failWithRestore(fmt.Errorf("apply staged file to %s: %w", target, err)) } - - applied = append(applied, target) } // Successful commit: remove backups. Keep success behavior, but surface cleanup failures in logs. diff --git a/internal/config/import_transaction_restore_test.go b/internal/config/import_transaction_restore_test.go new file mode 100644 index 0000000000..b978a6033a --- /dev/null +++ b/internal/config/import_transaction_restore_test.go @@ -0,0 +1,68 @@ +package config + +import ( + "os" + "path/filepath" + "testing" +) + +// Failed replacement must restore both overwritten files and the absence of +// files first created by this transaction, before returning to its caller. +func TestImportTransactionFailedCommitRestoresEveryTarget(t *testing.T) { + for _, failure := range []string{"later_directory", "missing_staged_existing", "missing_staged_new"} { + t.Run(failure, func(t *testing.T) { + dir := t.TempDir() + existing := filepath.Join(dir, "a-existing") + created := filepath.Join(dir, "b-new") + blocked := filepath.Join(dir, "z-directory") + if err := os.WriteFile(existing, []byte("original"), 0640); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(blocked, 0700); err != nil { + t.Fatal(err) + } + tx, err := newImportTransaction(dir) + if err != nil { + t.Fatal(err) + } + defer tx.Cleanup() + for _, target := range []string{existing, created, blocked} { + if err := tx.StageFile(target, []byte("replacement"), 0600); err != nil { + t.Fatal(err) + } + } + if failure != "later_directory" { + target := existing + if failure == "missing_staged_new" { + target = created + } + if err := os.Remove(tx.staged[target]); err != nil { + t.Fatal(err) + } + } + if err := tx.Commit(); err == nil { + t.Fatal("expected failed commit") + } + assertRestored := func() { + t.Helper() + data, err := os.ReadFile(existing) + if err != nil || string(data) != "original" { + t.Error("failed commit did not restore the original file") + } + info, err := os.Stat(existing) + if err != nil || info.Mode().Perm() != 0640 { + t.Error("failed commit did not restore original permissions") + } + if _, err := os.Stat(created); !os.IsNotExist(err) { + t.Error("failed commit left a newly created configuration file") + } + if info, err := os.Stat(blocked); err != nil || !info.IsDir() { + t.Error("failed commit changed the blocking directory") + } + } + assertRestored() + tx.Rollback() + assertRestored() + }) + } +} diff --git a/internal/config/persistence.go b/internal/config/persistence.go index 354698ae54..0963246f98 100644 --- a/internal/config/persistence.go +++ b/internal/config/persistence.go @@ -473,6 +473,16 @@ func (c *ConfigPersistence) writeConfigFileLocked(path string, data []byte, perm return nil } +// writeConfigBackupLocked preserves ordinary backup writes, but makes them +// part of an active import instead of changing recovery files before commit. +// Caller must hold c.mu. +func (c *ConfigPersistence) writeConfigBackupLocked(path string, data []byte, perm os.FileMode) error { + if c.tx != nil { + return c.tx.StageFile(path, data, perm) + } + return c.fs.WriteFile(path, data, perm) +} + // preserveCorruptConfigFileLocked moves a config file that failed to parse or // decrypt aside to .corrupt-. Loaders return empty data with // a nil error for such files, so read-modify-write savers would otherwise @@ -782,7 +792,7 @@ func (c *ConfigPersistence) persistAPITokensLocked(tokens []APITokenRecord) erro // Backup previous state (best effort). if existing, err := c.fs.ReadFile(c.apiTokensFile); err == nil && len(existing) > 0 { - if err := c.fs.WriteFile(c.apiTokensFile+".backup", existing, 0600); err != nil { + if err := c.writeConfigBackupLocked(c.apiTokensFile+".backup", existing, 0600); err != nil { log.Warn().Err(err).Msg("Failed to create API token backup file") } } @@ -1634,7 +1644,7 @@ func (c *ConfigPersistence) saveNodesConfig(pveInstances []PVEInstance, pbsInsta // Create timestamped backup timestampedBackup := fmt.Sprintf("%s.backup-%s", c.nodesFile, time.Now().Format("20060102-150405")) if backupData, err := c.fs.ReadFile(c.nodesFile); err == nil { - if err := c.fs.WriteFile(timestampedBackup, backupData, 0600); err != nil { + if err := c.writeConfigBackupLocked(timestampedBackup, backupData, 0600); err != nil { log.Warn().Err(err).Msg("Failed to create timestamped backup of nodes config") } else { log.Info().Str("backup", timestampedBackup).Msg("Created timestamped backup of nodes config") @@ -1644,13 +1654,16 @@ func (c *ConfigPersistence) saveNodesConfig(pveInstances []PVEInstance, pbsInsta // Also maintain a "latest" backup for quick recovery latestBackup := c.nodesFile + ".backup" if backupData, err := c.fs.ReadFile(c.nodesFile); err == nil { - if err := c.fs.WriteFile(latestBackup, backupData, 0600); err != nil { + if err := c.writeConfigBackupLocked(latestBackup, backupData, 0600); err != nil { log.Warn().Err(err).Msg("Failed to create latest backup of nodes config") } } - // Clean up old timestamped backups (keep last 10) - c.cleanupOldBackups(c.nodesFile + ".backup-*") + // Imports prune only after the entire archive commits. A failed + // recovery must not delete an older, still usable backup. + if c.tx == nil { + c.cleanupOldBackups(c.nodesFile + ".backup-*") + } } // Encrypt if crypto manager is available @@ -2037,6 +2050,9 @@ func (c *ConfigPersistence) saveSystemSettingsLocked(settings SystemSettings) er // Also update the .env file if it exists envFile := filepath.Join(c.configDir, ".env") if err := c.updateEnvFile(envFile, settings); err != nil { + if c.tx != nil { + return fmt.Errorf("stage imported environment settings: %w", err) + } log.Warn().Err(err).Msg("Failed to update .env file") // Don't fail the operation if .env update fails } @@ -3877,6 +3893,9 @@ func (c *ConfigPersistence) updateEnvFile(envFile string, settings SystemSetting perm = 0600 } } + if c.tx != nil { + return c.writeConfigFileLocked(envFile, []byte(content), perm) + } if err := c.fs.WriteFile(tempFile, []byte(content), perm); err != nil { return fmt.Errorf("write temp env file %s: %w", tempFile, err) }