diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Persistence/SiteStorageService.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Persistence/SiteStorageService.cs index 746e8547..8dd80e06 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Persistence/SiteStorageService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Persistence/SiteStorageService.cs @@ -156,9 +156,22 @@ public class SiteStorageService /// clause so the guard is atomic with no application-level read-modify-write. /// /// - /// This is the standby-node write path for replicated configs. The active-node - /// apply path () remains unguarded and always - /// overwrites, because the active node's write is always authoritative. + /// This is the reconciliation write path. SiteReconciliationActor runs a + /// per-node startup self-heal against central: it asks central what this node should be + /// running and fetches anything missing. That fetch races real deploys, so the + /// deployed_at guard is what stops a slow reconcile response from overwriting a + /// newer config that landed while it was in flight. The active-node apply path + /// () remains unguarded and always overwrites, + /// because a deploy is always authoritative. + /// + /// It was originally the standby write path as well, under notify-and-fetch: the + /// standby was told a deploy had happened and fetched the config itself. LocalDb Phase 2 + /// replaced that with change-data-capture — the config row simply replicates — so the + /// standby no longer writes here at all, and last-writer-wins on the primary key (not + /// this guard) is what orders concurrent writes between the two nodes. Reconciliation is + /// the reason the method survives; do not port the deployed_at guard onto the + /// replication path, where it would fight the HLC rather than help it. + /// /// /// is exposed for testing so that the exact /// deployed_at value can be controlled without sleeping between calls. diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentManagerActorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentManagerActorTests.cs index 8852d3fc..f898add5 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentManagerActorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentManagerActorTests.cs @@ -2,6 +2,7 @@ using Akka.Actor; using Akka.TestKit.Xunit2; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.Artifacts; using ZB.MOM.WW.ScadaBridge.Commons.Messages.Deployment; using ZB.MOM.WW.ScadaBridge.Commons.Messages.DebugView; using ZB.MOM.WW.ScadaBridge.Commons.Messages.InboundApi; @@ -883,6 +884,87 @@ public class DeploymentManagerActorTests : TestKit, IDisposable Assert.Equal("SenderPump", response.InstanceUniqueName); } + // ── LocalDb Phase 2 / Task 12: the active-node central-only purge ── + + [Fact] + public async Task ApplyingArtifacts_PurgesCentralOnlyNotificationConfig() + { + // Security cleanup. notification_lists and smtp_configurations can hold plaintext + // SMTP passwords written by a pre-2026-07-10 build, and the ACTIVE node's artifact + // apply is what clears them (DeploymentManagerActor.HandleDeployArtifacts). The + // standby's copy of this call lives in SiteReplicationActor and dies with it at + // Task 15, which makes this call site the ONLY remaining one. + // + // Nothing pins it today: ArtifactStorageTests covers the storage method, not the + // actor's call to it, so Task 16's edits to this actor could drop the call and every + // suite would stay green. That is precisely the kind of silent security regression + // this test exists to prevent — verified red-first by commenting out the call. + await SeedCentralOnlyRowsAsync(); + Assert.Equal(1, await RowCountAsync("notification_lists")); + Assert.Equal(1, await RowCountAsync("smtp_configurations")); + + var manager = CreateDeploymentManager(); + manager.Tell(new DeployArtifactsCommand( + DeploymentId: "dep-purge-1", + SharedScripts: null, + ExternalSystems: null, + DatabaseConnections: null, + NotificationLists: null, + DataConnections: null, + SmtpConfigurations: null, + Timestamp: DateTimeOffset.UtcNow)); + + // The apply runs on a Task.Run inside the actor, so poll rather than assert once. + await AwaitPurgedAsync(); + } + + private async Task SeedCentralOnlyRowsAsync() + { + // Seeded through the service's own (already-open) LocalDb connection — a raw + // SqliteConnection would lack the pragmas and the zb_hlc_next() UDF the site + // tables' capture triggers call. Raw SQL is the only way these rows can exist at + // all now that the site-side write paths are gone. + await using var connection = _storage.CreateConnection(); + await using var command = connection.CreateCommand(); + command.CommandText = """ + INSERT INTO notification_lists (name, recipient_emails, updated_at) + VALUES ('Ops Team', '["ops@example.com"]', @u); + INSERT INTO smtp_configurations + (name, server, port, auth_mode, from_address, username, password, oauth_config, updated_at) + VALUES ('smtp.example.com:587', 'smtp.example.com', 587, 'BasicAuth', + 'noreply@example.com', 'smtpuser', 'PLAINTEXT-SECRET', NULL, @u); + """; + command.Parameters.AddWithValue("@u", DateTimeOffset.UtcNow.ToString("O")); + await command.ExecuteNonQueryAsync(); + } + + private async Task RowCountAsync(string table) + { + await using var connection = _storage.CreateConnection(); + await using var command = connection.CreateCommand(); + command.CommandText = $"SELECT COUNT(*) FROM {table}"; + return (long)(await command.ExecuteScalarAsync())!; + } + + private async Task AwaitPurgedAsync() + { + var deadline = DateTime.UtcNow + TimeSpan.FromSeconds(10); + while (DateTime.UtcNow < deadline) + { + if (await RowCountAsync("notification_lists") == 0 && + await RowCountAsync("smtp_configurations") == 0) + { + return; + } + + await Task.Delay(50); + } + + Assert.Fail( + "Artifact apply did not purge the central-only notification/SMTP rows. " + + "The plaintext SMTP password is still on disk."); + } + /// /// In-test fake : returns a canned config JSON /// (notify-and-fetch success) or throws a canned exception (fetch failure), and records diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Persistence/SiteStorageServiceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Persistence/SiteStorageServiceTests.cs index 585e09c3..77dfcb78 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Persistence/SiteStorageServiceTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Persistence/SiteStorageServiceTests.cs @@ -226,7 +226,12 @@ public class SiteStorageServiceTests : IAsyncLifetime, IDisposable Assert.Empty(overrides); } - // ── Task 13: StoreDeployedConfigIfNewerAsync (guarded standby write) ── + // ── StoreDeployedConfigIfNewerAsync (the deployed_at-guarded write) ── + // + // Originally the standby's notify-and-fetch write path. LocalDb Phase 2 replaced that + // with change-data-capture, so the surviving caller is SiteReconciliationActor's + // startup self-heal against central, where the guard still stops a slow reconcile + // response from overwriting a newer config that landed while it was in flight. /// /// Seeds a deployed_configurations row with an explicit deployed_at timestamp using the same