From 8df38085aaf716e05ecbb1167a93818a84ac71b4 Mon Sep 17 00:00:00 2001 From: Sev7eNup <79143581+Sev7eNup@users.noreply.github.com> Date: Sat, 24 Oct 2026 17:36:41 +0200 Subject: [PATCH] fix: export backups from a consistent database snapshot Capture all database backup sections and counts in one serializable transaction with execution-strategy retries. Count runtime settings from the captured document. Add a concurrent-writer regression that restores the generated archive and verifies references and counts. Fixes #608 --- ...001-system-configuration-backup-restore.md | 9 ++ .../Services/Backup/BackupService.cs | 42 +++++--- .../Controllers/BackupControllerTests.cs | 2 +- .../DatabaseRecoveryAuditServiceTests.cs | 21 ++-- .../Security/ProvisioningSeederTests.cs | 2 +- .../Services/Backup/BackupAlertingTests.cs | 2 +- .../Backup/BackupRestoreServiceTests.cs | 7 +- .../Backup/BackupServiceExportTests.cs | 26 ++++- .../Services/Backup/BackupSnapshotTests.cs | 100 ++++++++++++++++++ .../Backup/CustomActivityBackupTests.cs | 6 +- 10 files changed, 184 insertions(+), 33 deletions(-) create mode 100644 tests/NodePilot.Api.Tests/Services/Backup/BackupSnapshotTests.cs diff --git a/docs/adr/0001-system-configuration-backup-restore.md b/docs/adr/0001-system-configuration-backup-restore.md index a6852765..d6e457fb 100644 --- a/docs/adr/0001-system-configuration-backup-restore.md +++ b/docs/adr/0001-system-configuration-backup-restore.md @@ -228,6 +228,15 @@ operator to check the operation ID before repeating the restore. Request cancell skip this commit reconciliation. ### K9 — Export only the runtime overrides, as raw file content + +Export captures all database sections and their counts in one Serializable transaction, +inside the EF execution strategy. A retry recaptures every section. PostgreSQL uses its +transaction snapshot; SQL Server holds serializable read/range locks through capture. +Runtime overrides are captured once from the atomically replaced file; their count is +derived from that captured document. The file and database are separate snapshots, not +a distributed transaction. Encryption and archive serialization finish after the database +transaction is released. Any capture or transaction failure prevents returning an archive. + What is exported is **only** the **raw JSON content** of `appsettings.runtime.json` (the database/file overrides), **not** `IConfigurationRoot` — that one is already decrypted and merged with env, CLI and appsettings.json, which would drag host and environment secrets into the file. diff --git a/src/NodePilot.Api/Services/Backup/BackupService.cs b/src/NodePilot.Api/Services/Backup/BackupService.cs index a9f35044..6b215916 100644 --- a/src/NodePilot.Api/Services/Backup/BackupService.cs +++ b/src/NodePilot.Api/Services/Backup/BackupService.cs @@ -1,8 +1,11 @@ using System.Reflection; +using System.Data; using System.Text; using System.Text.Json; using System.Text.Json.Nodes; using NodePilot.Data.Security; +using NodePilot.Data; +using Microsoft.EntityFrameworkCore; namespace NodePilot.Api.Services.Backup; @@ -29,7 +32,7 @@ public sealed class BackupExportException(string message) : Exception(message); /// , assembles the current versioned system-backup envelope, and seals it /// and authenticates that complete payload with passphrase-derived AES-GCM. /// -public sealed class BackupService(IEnumerable parts) +public sealed class BackupService(IEnumerable parts, NodePilotDbContext db) { /// /// Minimum backup passphrase length. A backup is a portable secret-bearing artifact. @@ -87,21 +90,32 @@ public async Task ExportAsync( var salt = PassphraseSecretProtector.GenerateSalt(); var protector = PassphraseSecretProtector.Derive(passphrase, salt); - var ctx = new BackupExportContext { Protector = protector }; - - var sections = new JsonObject(); var includedOrdered = SectionOrder.Where(effective.Contains).ToList(); - var counts = new List(includedOrdered.Count); - foreach (var key in includedOrdered) + var strategy = db.Database.CreateExecutionStrategy(); + var (sections, counts) = await strategy.ExecuteAsync(async () => { - counts.Add(new BackupSectionCount(key, await byKey[key].CountAsync(ct))); - sections[key] = await byKey[key].ExportAsync(ctx, ct); - } - - if (ctx.Warnings.Count > 0) - throw new BackupExportException( - "Backup aborted because one or more selected values could not be exported completely: " - + string.Join(" | ", ctx.Warnings)); + var ctx = new BackupExportContext { Protector = protector }; + var captured = new JsonObject(); + var capturedCounts = new List(includedOrdered.Count); + // Serializable prevents cross-section phantoms on both PostgreSQL and SQL Server. + await using var snapshot = await db.Database.BeginTransactionAsync(IsolationLevel.Serializable, ct); + foreach (var key in includedOrdered) + { + captured[key] = await byKey[key].ExportAsync(ctx, ct); + // Runtime settings are an independent atomic file snapshot, outside DB isolation. + var count = key == BackupSections.Settings + ? (captured[key]?["runtimeJson"] as JsonObject)?.Count ?? 0 + : await byKey[key].CountAsync(ct); + capturedCounts.Add(new BackupSectionCount(key, count)); + } + + if (ctx.Warnings.Count > 0) + throw new BackupExportException( + "Backup aborted because one or more selected values could not be exported completely: " + + string.Join(" | ", ctx.Warnings)); + await snapshot.CommitAsync(ct); + return (captured, capturedCounts); + }); var payload = new JsonObject { diff --git a/tests/NodePilot.Api.Tests/Controllers/BackupControllerTests.cs b/tests/NodePilot.Api.Tests/Controllers/BackupControllerTests.cs index e2c2a19f..34553edd 100644 --- a/tests/NodePilot.Api.Tests/Controllers/BackupControllerTests.cs +++ b/tests/NodePilot.Api.Tests/Controllers/BackupControllerTests.cs @@ -54,7 +54,7 @@ public BackupControllerTests() new CustomActivityBackupPart(new CustomActivityDefinitionStore(_db)), new WorkflowBackupPart(_db), new SettingsBackupPart(overrides, _atRest), - ]); + ], _db); _restore = new BackupRestoreService( _db, _atRest, overrides, NullLogger.Instance, new NodePilot.Api.Services.WorkflowVersionDefinitionProtector( diff --git a/tests/NodePilot.Api.Tests/Hosting/DatabaseRecoveryAuditServiceTests.cs b/tests/NodePilot.Api.Tests/Hosting/DatabaseRecoveryAuditServiceTests.cs index f5c2fbbb..08378917 100644 --- a/tests/NodePilot.Api.Tests/Hosting/DatabaseRecoveryAuditServiceTests.cs +++ b/tests/NodePilot.Api.Tests/Hosting/DatabaseRecoveryAuditServiceTests.cs @@ -69,8 +69,7 @@ private void MaybeThrow(DbContextEventData eventData) [Fact] public async Task RealOutageRecovery_PersistsExactlyOneEpisodeAudit_AndNoTripAudit() { - using var connection = new SqliteConnection("DataSource=:memory:"); - connection.Open(); + using var connection = OpenDatabase(); await using (var schema = NewDb(connection)) await schema.Database.EnsureCreatedAsync(); @@ -122,8 +121,7 @@ public async Task RealOutageRecovery_PersistsExactlyOneEpisodeAudit_AndNoTripAud [Fact] public async Task TwoRealOutageRecoveries_PersistOneDistinctAuditPerEpisode() { - using var connection = new SqliteConnection("DataSource=:memory:"); - connection.Open(); + using var connection = OpenDatabase(); await using (var schema = NewDb(connection)) await schema.Database.EnsureCreatedAsync(); @@ -171,8 +169,7 @@ public async Task TwoRealOutageRecoveries_PersistOneDistinctAuditPerEpisode() public async Task RecoveryAuditWriteFailure_RetriesOnFreshContextWithoutDuplicatingUnknownCommit( bool commitBeforeAcknowledgementIsLost) { - using var connection = new SqliteConnection("DataSource=:memory:"); - connection.Open(); + using var connection = OpenDatabase(); await using (var schema = NewDb(connection)) await schema.Database.EnsureCreatedAsync(); @@ -189,7 +186,7 @@ public async Task RecoveryAuditWriteFailure_RetriesOnFreshContextWithoutDuplicat Interlocked.Increment(ref contextsCreated); return new NodePilotDbContext( new DbContextOptionsBuilder() - .UseSqlite(connection) + .UseSqlite(connection.ConnectionString) .AddInterceptors(failure) .Options); }); @@ -231,8 +228,16 @@ public async Task RecoveryAuditWriteFailure_RetriesOnFreshContextWithoutDuplicat } } + private static SqliteConnection OpenDatabase() + { + // Keep the shared in-memory database alive; each concurrent context owns its connection. + var connection = new SqliteConnection($"Data Source=recovery-audit-{Guid.NewGuid():N};Mode=Memory;Cache=Shared;Pooling=False"); + connection.Open(); + return connection; + } + private static NodePilotDbContext NewDb(SqliteConnection connection) - => new(new DbContextOptionsBuilder().UseSqlite(connection).Options); + => new(new DbContextOptionsBuilder().UseSqlite(connection.ConnectionString).Options); private static async Task> WaitForAuditRowsAsync( SqliteConnection connection, diff --git a/tests/NodePilot.Api.Tests/Security/ProvisioningSeederTests.cs b/tests/NodePilot.Api.Tests/Security/ProvisioningSeederTests.cs index 33d14759..a84ff338 100644 --- a/tests/NodePilot.Api.Tests/Security/ProvisioningSeederTests.cs +++ b/tests/NodePilot.Api.Tests/Security/ProvisioningSeederTests.cs @@ -60,7 +60,7 @@ private async Task BuildSeedAsync(string username = "seeded-admin") }); await source.SaveChangesAsync(); - var export = await new BackupService([new UserBackupPart(source)]) + var export = await new BackupService([new UserBackupPart(source)], source) .ExportAsync([BackupSections.Users], Passphrase, "admin", CancellationToken.None); return export.Content; } diff --git a/tests/NodePilot.Api.Tests/Services/Backup/BackupAlertingTests.cs b/tests/NodePilot.Api.Tests/Services/Backup/BackupAlertingTests.cs index 42b5c096..c1a5812f 100644 --- a/tests/NodePilot.Api.Tests/Services/Backup/BackupAlertingTests.cs +++ b/tests/NodePilot.Api.Tests/Services/Backup/BackupAlertingTests.cs @@ -54,7 +54,7 @@ private IBackupPart[] Parts(NodePilotDbContext db) => ]; private async Task ExportAsync(NodePilotDbContext db, List sections) - => (await new BackupService(Parts(db)).ExportAsync(sections, Passphrase, "admin", CancellationToken.None)).Content; + => (await new BackupService(Parts(db), db).ExportAsync(sections, Passphrase, "admin", CancellationToken.None)).Content; private BackupRestoreService Restore(NodePilotDbContext db) => new(db, _atRest, new RuntimeOverridesWriter(TempPath(), NullLogger.Instance), diff --git a/tests/NodePilot.Api.Tests/Services/Backup/BackupRestoreServiceTests.cs b/tests/NodePilot.Api.Tests/Services/Backup/BackupRestoreServiceTests.cs index f7ba2fa4..1e910ed0 100644 --- a/tests/NodePilot.Api.Tests/Services/Backup/BackupRestoreServiceTests.cs +++ b/tests/NodePilot.Api.Tests/Services/Backup/BackupRestoreServiceTests.cs @@ -74,7 +74,7 @@ private NodePilot.Api.Services.WorkflowVersionDefinitionProtector VersionProtect new(_atRest, NullLogger.Instance); private async Task ExportAsync(NodePilotDbContext db, List sections) - => (await new BackupService(Parts(db)).ExportAsync(sections, Passphrase, "admin", CancellationToken.None)).Content; + => (await new BackupService(Parts(db), db).ExportAsync(sections, Passphrase, "admin", CancellationToken.None)).Content; private static byte[] MutatePayload(byte[] backup, Action mutate) { @@ -208,7 +208,7 @@ public async Task Restore_CommitFailure_PreservesSettingsAndLiveScopesAccordingT sourceSettings.MutateAndWrite(root => root["Smtp"] = new JsonObject { ["Port"] = 2525 }); var backup = (await new BackupService(Parts(src) .Where(part => part is not SettingsBackupPart) - .Append(new SettingsBackupPart(sourceSettings, _atRest))) + .Append(new SettingsBackupPart(sourceSettings, _atRest)), src) .ExportAsync([BackupSections.Workflows, BackupSections.Settings], Passphrase, "admin", ct)).Content; using var dst = TestDbFactory.Create(); @@ -1324,7 +1324,8 @@ public async Task Restore_Settings_ReplacesOverrides_RemovingKeysNotInBackup() // Backup carries only a Smtp override. var srcWriter = new RuntimeOverridesWriter(TempPath(), NullLogger.Instance); srcWriter.MutateAndWrite(root => root["Smtp"] = new JsonObject { ["Port"] = 2525 }); - var backup = (await new BackupService([new SettingsBackupPart(srcWriter, _atRest)]) + using var src = TestDbFactory.Create(); + var backup = (await new BackupService([new SettingsBackupPart(srcWriter, _atRest)], src) .ExportAsync([BackupSections.Settings], Passphrase, "admin", CancellationToken.None)).Content; // Target has an extra override ("Foo") that is NOT in the backup. diff --git a/tests/NodePilot.Api.Tests/Services/Backup/BackupServiceExportTests.cs b/tests/NodePilot.Api.Tests/Services/Backup/BackupServiceExportTests.cs index ca6ce4e4..9c2f61aa 100644 --- a/tests/NodePilot.Api.Tests/Services/Backup/BackupServiceExportTests.cs +++ b/tests/NodePilot.Api.Tests/Services/Backup/BackupServiceExportTests.cs @@ -52,7 +52,7 @@ public BackupServiceExportTests() new WorkflowBackupPart(_db), new SettingsBackupPart(_overrides, _atRest), }; - _service = new BackupService(parts); + _service = new BackupService(parts, _db); } private static byte[] DeterministicKey() @@ -371,13 +371,35 @@ public async Task Export_ShortPassphrase_Throws() [Fact] public async Task Export_AnyPartWarning_AbortsInsteadOfProducingAnIncompleteArchive() { - var service = new BackupService([new WarningPart()]); + var service = new BackupService([new WarningPart()], _db); var act = () => service.ExportAsync( [BackupSections.Users], Passphrase, "admin", CancellationToken.None); await act.Should().ThrowAsync() .WithMessage("*could not be exported completely*"); + _db.Database.CurrentTransaction.Should().BeNull(); + } + + [Fact] + public async Task Export_Settings_CountMatchesTheCapturedFileWithoutASecondRead() + { + _overrides.MutateAndWrite(root => root["Smtp"] = new JsonObject { ["Port"] = 2525 }); + var part = new SettingsSnapshotPart(new SettingsBackupPart(_overrides, _atRest)); + var service = new BackupService([part], _db); + + var result = await service.ExportAsync([BackupSections.Settings], Passphrase, "admin", TestContext.Current.CancellationToken); + + result.Counts.Single().Count.Should().Be(1); + Parse(result.Content)["sections"]!["settings"]!["runtimeJson"]!["Smtp"]!["Port"]!.GetValue().Should().Be(2525); + } + + private sealed class SettingsSnapshotPart(IBackupPart inner) : IBackupPart + { + public string Key => inner.Key; + public IReadOnlyList DependsOn => inner.DependsOn; + public Task CountAsync(CancellationToken ct) => throw new InvalidOperationException("Export must count the captured settings document."); + public Task ExportAsync(BackupExportContext ctx, CancellationToken ct) => inner.ExportAsync(ctx, ct); } private sealed class WarningPart : IBackupPart diff --git a/tests/NodePilot.Api.Tests/Services/Backup/BackupSnapshotTests.cs b/tests/NodePilot.Api.Tests/Services/Backup/BackupSnapshotTests.cs new file mode 100644 index 00000000..f3f0b1ce --- /dev/null +++ b/tests/NodePilot.Api.Tests/Services/Backup/BackupSnapshotTests.cs @@ -0,0 +1,100 @@ +using System.Text.Json.Nodes; +using FluentAssertions; +using Microsoft.Data.Sqlite; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Logging.Abstractions; +using NodePilot.Api.Configuration; +using NodePilot.Api.Services; +using NodePilot.Api.Services.Backup; +using NodePilot.Api.Services.Backup.Parts; +using NodePilot.Core.Models; +using NodePilot.Data; +using NodePilot.Data.Security; +using NodePilot.TestCommons; +using Xunit; + +namespace NodePilot.Api.Tests.Services.Backup; + +public class BackupSnapshotTests +{ + [Fact] + public async Task Export_ConcurrentCredentialAndMachineCreation_RestoresIntoEmptyDestination() + { + const string passphrase = "snapshot-backup-passphrase"; + var databasePath = Path.Combine(Path.GetTempPath(), $"np-backup-snapshot-{Guid.NewGuid():N}.db"); + var runtimePath = Path.Combine(Path.GetTempPath(), $"np-backup-snapshot-{Guid.NewGuid():N}.json"); + var options = new DbContextOptionsBuilder().UseSqlite( + new SqliteConnectionStringBuilder { DataSource = databasePath, Pooling = false, DefaultTimeout = 1 }.ToString()).Options; + var atRest = new AesGcmSecretProtector(new byte[32]); + var ct = TestContext.Current.CancellationToken; + try + { + await using var source = new NodePilotDbContext(options); + await source.Database.EnsureCreatedAsync(ct); + var existing = new Credential { Name = "existing", Username = "existing", EncryptedPassword = atRest.Protect("existing-secret") }; + source.Credentials.Add(existing); + source.ManagedMachines.Add(new ManagedMachine { Name = "existing-host", Hostname = "existing.local", DefaultCredentialId = existing.Id }); + await source.SaveChangesAsync(ct); + + await using var writer = new NodePilotDbContext(options); + var concurrent = new Credential { Name = "concurrent", Username = "concurrent", EncryptedPassword = atRest.Protect("concurrent-secret") }; + writer.Credentials.Add(concurrent); + writer.ManagedMachines.Add(new ManagedMachine { Name = "concurrent-host", Hostname = "concurrent.local", DefaultCredentialId = concurrent.Id }); + var deferred = false; + var attempted = false; + var credentials = new AfterExportPart(new CredentialBackupPart(source, atRest), async () => + { + attempted = true; + try { await writer.SaveChangesAsync(ct); } + catch (SqliteException ex) when (ex.SqliteErrorCode is 5 or 6) + { + deferred = true; + } + catch (DbUpdateException ex) when (ex.InnerException is SqliteException { SqliteErrorCode: 5 or 6 }) + { + deferred = true; + } + }); + var export = new BackupService([credentials, new MachineBackupPart(source)], source); + + var backup = await export.ExportAsync([BackupSections.Machines], passphrase, "admin", ct); + attempted.Should().BeTrue(); + if (deferred) await writer.SaveChangesAsync(ct); + (await writer.ManagedMachines.CountAsync(ct)).Should().Be(2); + source.Database.CurrentTransaction.Should().BeNull(); + + using var destination = TestDbFactory.Create(); + var restore = new BackupRestoreService(destination, atRest, + new RuntimeOverridesWriter(runtimePath, NullLogger.Instance), + NullLogger.Instance, + new WorkflowVersionDefinitionProtector(atRest, NullLogger.Instance)); + await restore.RestoreAsync(backup.Content, passphrase, + new Dictionary(), null, ct); + + var machines = await destination.ManagedMachines.ToListAsync(ct); + var restoredCredentials = await destination.Credentials.ToListAsync(ct); + machines.Should().NotBeEmpty(); + machines.Should().OnlyContain(machine => restoredCredentials.Any(credential => credential.Id == machine.DefaultCredentialId)); + machines.Count.Should().Be(backup.Counts.Single(count => count.Section == BackupSections.Machines).Count); + restoredCredentials.Count.Should().Be(backup.Counts.Single(count => count.Section == BackupSections.Credentials).Count); + } + finally + { + File.Delete(databasePath); + File.Delete(runtimePath); + } + } + + private sealed class AfterExportPart(IBackupPart inner, Func afterExport) : IBackupPart + { + public string Key => inner.Key; + public IReadOnlyList DependsOn => inner.DependsOn; + public Task CountAsync(CancellationToken ct) => inner.CountAsync(ct); + public async Task ExportAsync(BackupExportContext ctx, CancellationToken ct) + { + var content = await inner.ExportAsync(ctx, ct); + await afterExport(); + return content; + } + } +} diff --git a/tests/NodePilot.Api.Tests/Services/Backup/CustomActivityBackupTests.cs b/tests/NodePilot.Api.Tests/Services/Backup/CustomActivityBackupTests.cs index f3a7b12f..4798d3d5 100644 --- a/tests/NodePilot.Api.Tests/Services/Backup/CustomActivityBackupTests.cs +++ b/tests/NodePilot.Api.Tests/Services/Backup/CustomActivityBackupTests.cs @@ -91,7 +91,7 @@ private BackupRestoreService Restore(NodePilotDbContext db) => _atRest, NullLogger.Instance)); private async Task ExportAsync(NodePilotDbContext db) => - (await new BackupService(Parts(db)).ExportAsync(Sections, Passphrase, "admin", CancellationToken.None)).Content; + (await new BackupService(Parts(db), db).ExportAsync(Sections, Passphrase, "admin", CancellationToken.None)).Content; private static async Task SeedAsync(NodePilotDbContext db, bool enabled) { @@ -196,7 +196,7 @@ public async Task WorkflowOnlyExport_AutoIncludesReferencedCustomActivityDefinit var definition = await SeedAsync(src, enabled: true); SeedWorkflow(src, definition.Id); - var result = await new BackupService(Parts(src)).ExportAsync( + var result = await new BackupService(Parts(src), src).ExportAsync( [BackupSections.Workflows], Passphrase, "admin", CancellationToken.None); result.AutoIncludedSections.Should().Contain(BackupSections.CustomActivities); @@ -228,7 +228,7 @@ public async Task Restore_LegacyV2Archive_IsRejected() { using var src = TestDbFactory.Create(); await SeedAsync(src, enabled: true); - var current = await new BackupService(Parts(src)).ExportAsync( + var current = await new BackupService(Parts(src), src).ExportAsync( [BackupSections.CustomActivities], Passphrase, "admin", CancellationToken.None); var envelope = (JsonObject)JsonNode.Parse(Encoding.UTF8.GetString(current.Content))!; envelope["schema"] = BackupSections.SchemaV2;