From 64efb49996dafb6a4334f4db5968fd1988046ac2 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 9 Nov 2026 15:12:01 -0500 Subject: [PATCH] fix(secrets): hub push sweep isolates each row (scadaproj#45) Secrets-008: SqlServerSecretSyncService.SweepAsync now catches a per-row push failure, logs it (EventId 4, name only) and continues, like the pull side. One row the hub refuses (a >450-char name, an over-long explicit KEK id) no longer stops every later name from reaching the hub. Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV --- .../SqlServerSecretSyncService.cs | 28 +++++++++++++++++-- .../Fakes/SqliteBackedHub.cs | 10 ++++++- .../SqlServerSecretSyncServiceTests.cs | 18 ++++++++++++ 3 files changed, 52 insertions(+), 4 deletions(-) diff --git a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.SqlServer/SqlServerSecretSyncService.cs b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.SqlServer/SqlServerSecretSyncService.cs index 2ce60dc..0cd1411 100644 --- a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.SqlServer/SqlServerSecretSyncService.cs +++ b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.SqlServer/SqlServerSecretSyncService.cs @@ -108,9 +108,25 @@ public sealed partial class SqlServerSecretSyncService : BackgroundService foreach (StoredSecret row in rows) { - // Verbatim, like the replicator — the hub records the originating node's revision. - await _hub.ApplyReplicatedAsync(row, ct).ConfigureAwait(false); - pushed++; + try + { + // Verbatim, like the replicator — the hub records the originating node's revision. + await _hub.ApplyReplicatedAsync(row, ct).ConfigureAwait(false); + pushed++; + } + catch (OperationCanceledException) when (ct.IsCancellationRequested) + { + throw; + } + catch (Exception ex) + { + // Isolated per row, like the pull side (SecretReplicationReconciler.ApplyAsync). + // The push set is recomputed in the same name order every interval, so one row the + // hub refuses (a name longer than its NVARCHAR(450) column, an over-long explicit + // KEK id) would otherwise stop every name sorting after it from ever reaching the + // hub. Log it and keep going; it is retried every sweep. + LogPushRowFailed(ex, row.Name.Value); + } } } @@ -155,4 +171,10 @@ public sealed partial class SqlServerSecretSyncService : BackgroundService Level = LogLevel.Warning, Message = "Discarding replicated secret row {SecretName} that could not be applied locally; the rest of the batch continues.")] private partial void LogRowDiscarded(Exception exception, string secretName); + + [LoggerMessage( + EventId = 4, + Level = LogLevel.Warning, + Message = "The hub refused secret row {SecretName} on push; the rest of the sweep continues and the row is retried next interval.")] + private partial void LogPushRowFailed(Exception exception, string secretName); } diff --git a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.SqlServer.Tests/Fakes/SqliteBackedHub.cs b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.SqlServer.Tests/Fakes/SqliteBackedHub.cs index 2f85574..0751b7c 100644 --- a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.SqlServer.Tests/Fakes/SqliteBackedHub.cs +++ b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.SqlServer.Tests/Fakes/SqliteBackedHub.cs @@ -45,7 +45,15 @@ public sealed class SqliteBackedHub(SqliteSecretStore store) : ISecretReplicatio return rows; } + /// + /// A row name the hub refuses on push, as SQL Server refuses a name longer than its + /// NVARCHAR(450) column ("String or binary data would be truncated"). + /// + public string? RejectPushOf { get; set; } + /// public Task ApplyReplicatedAsync(StoredSecret row, CancellationToken ct) => - store.ApplyReplicatedAsync(row, ct); + row.Name.Value == RejectPushOf + ? throw new InvalidOperationException("String or binary data would be truncated.") + : store.ApplyReplicatedAsync(row, ct); } diff --git a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.SqlServer.Tests/SqlServerSecretSyncServiceTests.cs b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.SqlServer.Tests/SqlServerSecretSyncServiceTests.cs index 7e31d4d..375de4e 100644 --- a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.SqlServer.Tests/SqlServerSecretSyncServiceTests.cs +++ b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.SqlServer.Tests/SqlServerSecretSyncServiceTests.cs @@ -88,6 +88,24 @@ public sealed class SqlServerSecretSyncServiceTests : IDisposable Assert.Equal(0xBB, hub!.Ciphertext[0]); } + [Fact] + public async Task Sweep_isolates_a_row_the_hub_rejects_so_later_names_still_push() + { + // Secrets-008: the push set is recomputed in the same name order every interval, so one + // poison row used to stop every name sorting after it, forever. + await _local.UpsertAsync(Row("a/first", 0x01), CancellationToken.None); + await _local.UpsertAsync(Row("m/poison", 0x02), CancellationToken.None); + await _local.UpsertAsync(Row("z/after", 0x03), CancellationToken.None); + _hub.RejectPushOf = "m/poison"; + + (_, int pushed) = await CreateService().SweepAsync(CancellationToken.None); + + Assert.Equal(2, pushed); + Assert.NotNull(await _hubStore.GetAsync(new SecretName("a/first"), CancellationToken.None)); + Assert.NotNull(await _hubStore.GetAsync(new SecretName("z/after"), CancellationToken.None)); + Assert.Null(await _hubStore.GetAsync(new SecretName("m/poison"), CancellationToken.None)); + } + [Fact] public async Task Sweep_preserves_the_originating_revision_when_pushing() {