fix(site): DeployedSecretsMirror — confirm-then-ledger LWW apply, per-row isolation, cache invalidation
Fix round 1 (review task-9-review.md), all rulings: 1. (Important 1) ApplyReplicatedAsync is no longer trusted blindly. Every apply is confirmed by a store.GetAsync read-back before ledgering: a tombstoned local row (routine remove-then-re-add) is safely re-stamped past the tombstone and re-applied (confirmed neither UpdatedUtc nor Revision is AEAD-bound in AesGcmEnvelopeCipher, so the bump cannot break decryption); a live local row with different ciphertext (a strictly-newer out-of-band secret) is left untouched, never ledgered, never verified against, and reported with a distinct Failed message so a later central tombstone can never delete it via this mirror. 2. (Important 2) Per-row try/catch (rethrow only on ct.IsCancellationRequested) so one poisoned row (e.g. an unrecognised ContentType from a newer central) no longer blocks every row after it. 3. (Important 3) Optional ISecretCacheInvalidator, invalidated after every successful apply/delete and before verify, so a rotated/removed secret's plaintext cannot be served stale for the remainder of the resolver's TTL. 4. (Minor 4) Loop and VerifyAsync OperationCanceledException handling now filters on ct/stoppingToken.IsCancellationRequested specifically, rather than swallowing every OCE. 5. (Minor 5) Class remarks note the mirror assumes a node-local ISecretStore; Task 11 will reject SqlServer secrets replication on the Site role. 6. (Minor 6/7) Added cancellation + concurrent-callers tests, dropped the redundant FailFor (tests now use ThrowFor/ReturnNullFor to exercise distinct decrypt-failure paths), and Dispose() now disposes the gate SemaphoreSlim. Verify: dotnet test tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests --filter FullyQualifiedName~DeployedSecretsMirrorTests (24/24); full SiteRuntime.Tests suite 876/876.
This commit is contained in:
@@ -11,11 +11,12 @@ namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Secrets;
|
||||
/// The outcome of one <see cref="DeployedSecretsMirror.ReconcileAsync"/> pass: the secret
|
||||
/// names applied (inserted or updated in this node's own secret store), the names removed
|
||||
/// (deleted because central tombstoned them and this node had previously mirrored them), and
|
||||
/// a human-readable failure message per live secret that did not decrypt on this node.
|
||||
/// a human-readable failure message per live secret that could not be applied and/or verified
|
||||
/// on this node.
|
||||
/// </summary>
|
||||
/// <param name="Applied">Names applied (inserted or updated) this pass.</param>
|
||||
/// <param name="Removed">Names deleted this pass because they were previously mirrored and are now tombstoned upstream.</param>
|
||||
/// <param name="Failed">One message per live secret that failed to decrypt on this node — never includes secret values.</param>
|
||||
/// <param name="Failed">One message per live secret that failed to apply and/or decrypt on this node — never includes secret values.</param>
|
||||
public sealed record MirrorResult(
|
||||
IReadOnlyList<string> Applied, IReadOnlyList<string> Removed, IReadOnlyList<string> Failed)
|
||||
{
|
||||
@@ -39,71 +40,132 @@ public sealed record MirrorResult(
|
||||
/// master key.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <b>Apply semantics (<see cref="ISecretStore.ApplyReplicatedAsync"/>).</b> The store applies
|
||||
/// last-writer-wins on (<c>UpdatedUtc</c>, <c>Revision</c>) — see
|
||||
/// <c>ZB.MOM.WW.Secrets.Abstractions.SecretLastWriterWins</c> — and a row that is not
|
||||
/// <em>strictly</em> newer than the local row is silently ignored. Consequence worth knowing:
|
||||
/// if an operator ever creates an out-of-band secret directly in a site node's local store
|
||||
/// under the SAME name as a deployed secret, and stamps it with a later <c>UpdatedUtc</c> (or
|
||||
/// an equal <c>UpdatedUtc</c> and a higher <c>Revision</c>) than the deployed row central last
|
||||
/// shipped, that out-of-band secret WINS locally — every subsequent reconcile pass's
|
||||
/// <c>ApplyReplicatedAsync</c> call for that name is a silent no-op until central ships a
|
||||
/// still-newer revision. This mirror does not attempt to detect or warn about that case (it
|
||||
/// has no way to distinguish "already applied, unchanged" from "out-of-band override in
|
||||
/// place" — both look like "nothing to do" from here); deliberately not adding that
|
||||
/// complexity per the design's scope.
|
||||
/// <b>Apply semantics and the confirm-then-ledger fix (fix round 1, review Important #1).</b>
|
||||
/// <see cref="ISecretStore.ApplyReplicatedAsync"/> applies last-writer-wins strictly on
|
||||
/// (<c>UpdatedUtc</c>, <c>Revision</c>) — see
|
||||
/// <see cref="ZB.MOM.WW.Secrets.Abstractions.SecretLastWriterWins"/> — and a row that is not
|
||||
/// strictly newer than the local row is silently ignored, so this mirror never trusts the call
|
||||
/// blindly: every apply is followed by an <see cref="ISecretStore.GetAsync"/> read-back, and
|
||||
/// <c>mirrored_secrets</c> is ledgered only when the local row is confirmed LIVE and its
|
||||
/// ciphertext (<c>Ciphertext</c> + <c>WrappedDek</c>) matches what was just shipped. Two
|
||||
/// non-ledgered outcomes are possible, both reported as a <see cref="MirrorResult.Failed"/>
|
||||
/// entry (never as the decrypt-failure message, which would misdiagnose either case) and never
|
||||
/// overwritten a second way:
|
||||
/// <list type="bullet">
|
||||
/// <item>
|
||||
/// <b>The local row is a tombstone</b> (this node previously deleted it — e.g. the site
|
||||
/// stopped referencing it, then referenced it again unchanged at central, so the shipped row's
|
||||
/// <c>UpdatedUtc</c> is older than the tombstone's). Re-stamping is safe here: neither
|
||||
/// <c>UpdatedUtc</c> nor <c>Revision</c> is bound into the AEAD associated data — confirmed
|
||||
/// against <c>ZB.MOM.WW.Secrets.Crypto.AesGcmEnvelopeCipher</c>, whose body layer binds only
|
||||
/// <c>SecretName.Value</c> and whose wrap layer binds only <c>KekId</c> — so the mirror
|
||||
/// re-applies the exact shipped ciphertext with <c>UpdatedUtc = max(now, tombstone.UpdatedUtc +
|
||||
/// 1 tick)</c> and <c>Revision = tombstone.Revision + 1</c>, which beats the tombstone under LWW
|
||||
/// without touching anything the cipher verifies. If that confirmed re-apply still does not
|
||||
/// read back live and matching (a concurrent conflict), the row is reported failed instead of
|
||||
/// silently left broken.
|
||||
/// </item>
|
||||
/// <item>
|
||||
/// <b>The local row is LIVE with different ciphertext</b> — an out-of-band secret created
|
||||
/// directly on this node under the same name, stamped strictly newer than what central shipped.
|
||||
/// It wins LWW legitimately. This mirror does NOT overwrite it, does NOT ledger it (so a later
|
||||
/// central tombstone of the name can never delete someone's out-of-band secret via this
|
||||
/// mirror), and does NOT run the decrypt-verify against it (verifying would trivially "succeed"
|
||||
/// against the operator's own value and mask that the deployed revision never took effect).
|
||||
/// </item>
|
||||
/// </list>
|
||||
/// Ledgering always uses the SHIPPED artifact's own revision (not any bumped local revision)
|
||||
/// so a later pass's "is this ledgered at the shipped revision" check stays correct.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <b>Per-row isolation (review Important #2).</b> Each row's apply/verify body runs in its own
|
||||
/// try/catch; an exception — an unrecognised <c>ContentType</c> from a newer central, a
|
||||
/// per-row store fault, a malformed name — is caught, reported as a <see cref="MirrorResult.Failed"/>
|
||||
/// entry naming the secret and the exception type/message (never a value), logged with the same
|
||||
/// de-spam rule, and does not stop the rest of the pass. Only an exception attributable to the
|
||||
/// caller's own cancellation (<c>ct.IsCancellationRequested</c>) propagates.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <b>Cache invalidation (review Important #3).</b> The concrete resolver
|
||||
/// (<c>DefaultSecretResolver</c>) caches decrypted plaintext for a TTL and also implements
|
||||
/// <see cref="ISecretCacheInvalidator"/>; every ZB.MOM.WW.Secrets write path invalidates after a
|
||||
/// write for exactly this reason. This mirror does the same: after every successful apply
|
||||
/// (including a tombstone re-materialization) or delete, and before the verify step, it calls
|
||||
/// <see cref="ISecretCacheInvalidator.Invalidate"/> (injected optionally — <c>null</c> if the
|
||||
/// host never registered one) so neither the verify nor a script running immediately afterward
|
||||
/// can observe a stale pre-rotation plaintext for up to the cache TTL.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <b>Node-local store assumption (review Minor #5).</b> This mirror assumes the site's
|
||||
/// <see cref="ISecretStore"/> is node-local (plain local SQLite, or the gRPC-hub follower half,
|
||||
/// both of which keep writes local). It is NOT safe to run against a site node whose store is
|
||||
/// decorated by SQL-Server secrets REPLICATION (<c>Secrets:Replication:Mode=SqlServer</c>) — its
|
||||
/// <c>DeleteAsync</c> publishes the tombstone to the shared hub, so a site merely un-referencing
|
||||
/// a secret would delete it fleet-wide. Task 11 rejects that configuration on the Site role;
|
||||
/// no code change is made here.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <b>Resolve semantics and the resolver cache.</b> The trial-resolve verify step goes through
|
||||
/// <see cref="ISecretResolver.GetAsync"/>, which the concrete resolver caches for a TTL
|
||||
/// (<c>ResolveCacheTtl</c>, 30 s in the shipped implementation). If this node's master key
|
||||
/// changes (e.g. a KEK rotation) between an earlier successful resolve and a reconcile pass
|
||||
/// run within the cache TTL, the verify can return the STALE cached plaintext rather than
|
||||
/// re-decrypting — that is still a successful decrypt of <em>a</em> value under the key that
|
||||
/// produced the cache entry, which is all this check is trying to prove (this node's master
|
||||
/// key can decrypt what central most recently shipped), so a cache hit is an acceptable false
|
||||
/// negative for the "did the key change" question and not a bug in this mirror.
|
||||
/// (<c>ResolveCacheTtl</c>, 30 s in the shipped implementation). Because every successful apply
|
||||
/// or delete now invalidates that cache first (see above), the remaining exposure is narrow: a
|
||||
/// KEK rotation on this node's OWN master key (not a secret rotation at central) landing inside
|
||||
/// the cache window of an unrelated resolve. That residual case is still an acceptable false
|
||||
/// negative for the "did the key change" question — the cache hit still proves this node once
|
||||
/// decrypted a value with a key that produced that cache entry — and is not a bug in this
|
||||
/// mirror.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <b>Error-log de-spam.</b> A secret that keeps failing to decrypt would otherwise log an
|
||||
/// Error line every reconcile pass forever. This mirror logs Error only the FIRST time a given
|
||||
/// (name, revision, kekId) tuple fails on this node; identical repeats log at Debug instead.
|
||||
/// The moment that same name resolves successfully (a fixed master key, or a newer revision
|
||||
/// central re-shipped), the remembered tuple is cleared, so a LATER failure — even for the
|
||||
/// exact same revision/kekId, e.g. a flapping key — logs at Error again. This state is
|
||||
/// in-memory only (per node process), which matches the ledger's own node-local scope.
|
||||
/// <b>Error-log de-spam.</b> A secret that keeps failing (to apply OR to decrypt) would
|
||||
/// otherwise log an Error line every reconcile pass forever. This mirror logs Error only the
|
||||
/// FIRST time a given failure fingerprint (kind + revision + relevant detail, e.g. KekId for a
|
||||
/// decrypt failure) occurs for a name on this node; identical repeats log at Debug instead. The
|
||||
/// moment that same name applies/resolves successfully, the remembered fingerprint is cleared,
|
||||
/// so a LATER failure — even one with an identical fingerprint — logs at Error again. This state
|
||||
/// is in-memory only (per node process), which matches the ledger's own node-local scope.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
public sealed class DeployedSecretsMirror : BackgroundService
|
||||
{
|
||||
private const string DeleteActor = "deployed-secrets-mirror";
|
||||
|
||||
private readonly SiteStorageService _storage;
|
||||
private readonly ISecretStore _store;
|
||||
private readonly ISecretResolver _resolver;
|
||||
private readonly ISecretCacheInvalidator? _invalidator;
|
||||
private readonly IOptions<SiteRuntimeOptions> _options;
|
||||
private readonly ILogger<DeployedSecretsMirror> _logger;
|
||||
private readonly TimeProvider _time;
|
||||
|
||||
private readonly SemaphoreSlim _gate = new(1, 1);
|
||||
private bool _disposed;
|
||||
|
||||
/// <summary>
|
||||
/// Per-secret-name record of the (Revision, KekId) tuple whose decrypt failure this node
|
||||
/// has already logged at Error. Cleared for a name the moment it resolves successfully, so
|
||||
/// a later failure — even an identical one — logs at Error again. In-memory only; not a
|
||||
/// durable ledger, and deliberately so (it exists purely to de-spam this process's logs).
|
||||
/// Per-secret-name record of the failure fingerprint this node has already logged at Error
|
||||
/// (an apply-conflict or a decrypt failure — see the class remarks). Cleared for a name the
|
||||
/// moment it applies/resolves successfully, so a later failure — even an identical one —
|
||||
/// logs at Error again. In-memory only; not a durable ledger, and deliberately so (it exists
|
||||
/// purely to de-spam this process's logs).
|
||||
/// </summary>
|
||||
private readonly Dictionary<string, (long Revision, string KekId)> _lastLoggedFailure = new(StringComparer.Ordinal);
|
||||
private readonly Dictionary<string, string> _lastLoggedFailure = new(StringComparer.Ordinal);
|
||||
|
||||
/// <summary>Initializes a new instance of <see cref="DeployedSecretsMirror"/>.</summary>
|
||||
/// <param name="storage">Site-local storage exposing <c>deployed_secrets</c> and the <c>mirrored_secrets</c> ledger.</param>
|
||||
/// <param name="store">This node's own secret store.</param>
|
||||
/// <param name="resolver">This node's own secret resolver, used to trial-decrypt each deployed secret.</param>
|
||||
/// <param name="invalidator">
|
||||
/// Optional cache invalidator for <paramref name="resolver"/> (the concrete
|
||||
/// <c>DefaultSecretResolver</c> implements this itself; <c>null</c> if the host never
|
||||
/// registered one, in which case no invalidation is attempted).
|
||||
/// </param>
|
||||
/// <param name="options">Supplies <see cref="SiteRuntimeOptions.DeployedSecretsMirrorInterval"/>.</param>
|
||||
/// <param name="logger">Logger instance. Never logs secret values — names, revisions, and KEK ids only.</param>
|
||||
/// <param name="time">Clock driving the reconcile loop's <see cref="PeriodicTimer"/>.</param>
|
||||
/// <param name="time">Clock driving the reconcile loop's <see cref="PeriodicTimer"/> and the tombstone-recovery stamp.</param>
|
||||
public DeployedSecretsMirror(
|
||||
SiteStorageService storage,
|
||||
ISecretStore store,
|
||||
ISecretResolver resolver,
|
||||
ISecretCacheInvalidator? invalidator,
|
||||
IOptions<SiteRuntimeOptions> options,
|
||||
ILogger<DeployedSecretsMirror> logger,
|
||||
TimeProvider time)
|
||||
@@ -111,6 +173,7 @@ public sealed class DeployedSecretsMirror : BackgroundService
|
||||
_storage = storage ?? throw new ArgumentNullException(nameof(storage));
|
||||
_store = store ?? throw new ArgumentNullException(nameof(store));
|
||||
_resolver = resolver ?? throw new ArgumentNullException(nameof(resolver));
|
||||
_invalidator = invalidator;
|
||||
_options = options ?? throw new ArgumentNullException(nameof(options));
|
||||
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
|
||||
_time = time ?? throw new ArgumentNullException(nameof(time));
|
||||
@@ -118,10 +181,11 @@ public sealed class DeployedSecretsMirror : BackgroundService
|
||||
|
||||
/// <summary>
|
||||
/// Runs one reconcile pass: applies every live <c>deployed_secrets</c> row into this
|
||||
/// node's own secret store, deletes rows this node previously mirrored that are now
|
||||
/// tombstoned upstream, and trial-decrypts every live row to verify it resolves under
|
||||
/// this node's own master key. Safe to call concurrently with itself (and with the
|
||||
/// hosted loop) — a single in-flight pass is enforced internally.
|
||||
/// node's own secret store (confirmed by read-back before ledgering), deletes rows this
|
||||
/// node previously mirrored that are now tombstoned upstream, and trial-decrypts every
|
||||
/// live, confirmed-applied row to verify it resolves under this node's own master key.
|
||||
/// Safe to call concurrently with itself (and with the hosted loop) — a single in-flight
|
||||
/// pass is enforced internally.
|
||||
/// </summary>
|
||||
/// <param name="ct">A token to cancel the operation.</param>
|
||||
/// <returns>The pass's <see cref="MirrorResult"/>.</returns>
|
||||
@@ -140,38 +204,25 @@ public sealed class DeployedSecretsMirror : BackgroundService
|
||||
|
||||
foreach (var row in rows)
|
||||
{
|
||||
if (row.IsRemoved)
|
||||
try
|
||||
{
|
||||
if (!mirrored.ContainsKey(row.Name))
|
||||
if (row.IsRemoved)
|
||||
{
|
||||
// Never mirrored by this node (or already un-mirrored) — an
|
||||
// out-of-band, same-named local secret is left alone.
|
||||
continue;
|
||||
await ProcessRemovedRowAsync(row, mirrored, removed, ct).ConfigureAwait(false);
|
||||
}
|
||||
else
|
||||
{
|
||||
await ProcessLiveRowAsync(row, mirrored, applied, failed, ct).ConfigureAwait(false);
|
||||
}
|
||||
|
||||
await _store.DeleteAsync(new SecretName(row.Name), "deployed-secrets-mirror", ct)
|
||||
.ConfigureAwait(false);
|
||||
await _storage.RemoveMirroredSecretAsync(row.Name).ConfigureAwait(false);
|
||||
removed.Add(row.Name);
|
||||
_lastLoggedFailure.Remove(row.Name);
|
||||
_logger.LogInformation(
|
||||
"Removed deployed secret {SecretName} (no longer referenced by this site)",
|
||||
row.Name);
|
||||
continue;
|
||||
}
|
||||
|
||||
if (!mirrored.TryGetValue(row.Name, out var mirroredRevision)
|
||||
|| mirroredRevision != row.Secret.Revision)
|
||||
catch (Exception ex) when (!ct.IsCancellationRequested)
|
||||
{
|
||||
await _store.ApplyReplicatedAsync(ToStored(row.Secret), ct).ConfigureAwait(false);
|
||||
await _storage.SetMirroredSecretAsync(row.Name, row.Secret.Revision).ConfigureAwait(false);
|
||||
applied.Add(row.Name);
|
||||
_logger.LogInformation(
|
||||
"Applied deployed secret {SecretName} r{Revision} (kek {KekId})",
|
||||
row.Name, row.Secret.Revision, row.Secret.KekId);
|
||||
ReportFailure(
|
||||
row.Name,
|
||||
$"error:{row.Secret.Revision}:{ex.GetType().Name}:{ex.Message}",
|
||||
$"Secret '{row.Name}' failed to process on this node ({ex.GetType().Name}: {ex.Message})",
|
||||
failed);
|
||||
}
|
||||
|
||||
await VerifyAsync(row, failed, ct).ConfigureAwait(false);
|
||||
}
|
||||
|
||||
var result = new MirrorResult(applied, removed, failed);
|
||||
@@ -185,7 +236,7 @@ public sealed class DeployedSecretsMirror : BackgroundService
|
||||
else
|
||||
{
|
||||
_logger.LogDebug(
|
||||
"Deployed secrets mirror in sync ({Count} secret(s), {FailedCount} failing to decrypt)",
|
||||
"Deployed secrets mirror in sync ({Count} secret(s), {FailedCount} failing)",
|
||||
rows.Count(r => !r.IsRemoved), failed.Count);
|
||||
}
|
||||
|
||||
@@ -197,12 +248,146 @@ public sealed class DeployedSecretsMirror : BackgroundService
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Handles a tombstoned <c>deployed_secrets</c> row: deletes it from this node's store, but
|
||||
/// only if this node had previously ledgered it as mirrored — an out-of-band, same-named
|
||||
/// local secret this mirror never applied is left alone.
|
||||
/// </summary>
|
||||
private async Task ProcessRemovedRowAsync(
|
||||
DeployedSecretRow row, Dictionary<string, long> mirrored, List<string> removed, CancellationToken ct)
|
||||
{
|
||||
if (!mirrored.ContainsKey(row.Name))
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
await _store.DeleteAsync(new SecretName(row.Name), DeleteActor, ct).ConfigureAwait(false);
|
||||
_invalidator?.Invalidate(new SecretName(row.Name));
|
||||
await _storage.RemoveMirroredSecretAsync(row.Name).ConfigureAwait(false);
|
||||
removed.Add(row.Name);
|
||||
_lastLoggedFailure.Remove(row.Name);
|
||||
_logger.LogInformation(
|
||||
"Removed deployed secret {SecretName} (no longer referenced by this site)",
|
||||
row.Name);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Handles a live <c>deployed_secrets</c> row: applies it (confirmed by read-back) when not
|
||||
/// already ledgered at the shipped revision, then verifies it decrypts — unless the apply
|
||||
/// outcome makes a verify meaningless (an out-of-band local secret won LWW), in which case a
|
||||
/// single, distinct failure is reported instead.
|
||||
/// </summary>
|
||||
private async Task ProcessLiveRowAsync(
|
||||
DeployedSecretRow row,
|
||||
Dictionary<string, long> mirrored,
|
||||
List<string> applied,
|
||||
List<string> failed,
|
||||
CancellationToken ct)
|
||||
{
|
||||
var name = new SecretName(row.Name);
|
||||
var alreadyLedgered =
|
||||
mirrored.TryGetValue(row.Name, out var mirroredRevision) && mirroredRevision == row.Secret.Revision;
|
||||
|
||||
if (!alreadyLedgered)
|
||||
{
|
||||
var outcome = await ApplyAndConfirmAsync(row, name, ct).ConfigureAwait(false);
|
||||
|
||||
switch (outcome)
|
||||
{
|
||||
case ApplyOutcome.Applied:
|
||||
await _storage.SetMirroredSecretAsync(row.Name, row.Secret.Revision).ConfigureAwait(false);
|
||||
_invalidator?.Invalidate(name);
|
||||
applied.Add(row.Name);
|
||||
_lastLoggedFailure.Remove(row.Name);
|
||||
_logger.LogInformation(
|
||||
"Applied deployed secret {SecretName} r{Revision} (kek {KekId})",
|
||||
row.Name, row.Secret.Revision, row.Secret.KekId);
|
||||
break;
|
||||
|
||||
case ApplyOutcome.LocalNewer:
|
||||
ReportFailure(
|
||||
row.Name,
|
||||
$"outofband:{row.Secret.Revision}",
|
||||
$"Secret '{row.Name}' exists on this node with a newer local value; " +
|
||||
"the deployed revision was not applied",
|
||||
failed);
|
||||
return; // Do not verify — that would decrypt the operator's own value.
|
||||
|
||||
case ApplyOutcome.Unconfirmed:
|
||||
ReportFailure(
|
||||
row.Name,
|
||||
$"unconfirmed:{row.Secret.Revision}",
|
||||
$"Secret '{row.Name}' could not be confirmed applied on this node " +
|
||||
"after a tombstone re-apply attempt",
|
||||
failed);
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
await VerifyAsync(row, failed, ct).ConfigureAwait(false);
|
||||
}
|
||||
|
||||
/// <summary>The confirmed outcome of applying one shipped row into this node's store. See the class remarks.</summary>
|
||||
private enum ApplyOutcome
|
||||
{
|
||||
/// <summary>The local row is now live and its ciphertext matches what was shipped.</summary>
|
||||
Applied,
|
||||
|
||||
/// <summary>The local row is live with DIFFERENT ciphertext — an out-of-band secret that legitimately won LWW.</summary>
|
||||
LocalNewer,
|
||||
|
||||
/// <summary>Neither of the above — a tombstone re-apply attempt did not read back as applied.</summary>
|
||||
Unconfirmed,
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Calls <see cref="ISecretStore.ApplyReplicatedAsync"/> and confirms the outcome by reading
|
||||
/// the row back — see the class remarks for why this mirror never trusts the call blindly.
|
||||
/// </summary>
|
||||
private async Task<ApplyOutcome> ApplyAndConfirmAsync(DeployedSecretRow row, SecretName name, CancellationToken ct)
|
||||
{
|
||||
var shipped = ToStored(row.Secret);
|
||||
await _store.ApplyReplicatedAsync(shipped, ct).ConfigureAwait(false);
|
||||
|
||||
var local = await _store.GetAsync(name, ct).ConfigureAwait(false);
|
||||
|
||||
if (local is not null && !local.IsDeleted && MatchesShippedCiphertext(local, shipped))
|
||||
{
|
||||
return ApplyOutcome.Applied;
|
||||
}
|
||||
|
||||
if (local is not null && local.IsDeleted)
|
||||
{
|
||||
// Neither UpdatedUtc nor Revision is bound into the AEAD associated data (confirmed
|
||||
// against AesGcmEnvelopeCipher — only SecretName and KekId are AAD), so re-stamping
|
||||
// strictly past the tombstone is safe: it changes nothing the cipher verifies.
|
||||
var nowUtc = _time.GetUtcNow();
|
||||
var tombstoneUtcPlusOneTick = local.UpdatedUtc.AddTicks(1);
|
||||
var bumpedUpdatedUtc = nowUtc > tombstoneUtcPlusOneTick ? nowUtc : tombstoneUtcPlusOneTick;
|
||||
var bumped = shipped with { UpdatedUtc = bumpedUpdatedUtc, Revision = local.Revision + 1 };
|
||||
|
||||
await _store.ApplyReplicatedAsync(bumped, ct).ConfigureAwait(false);
|
||||
var reconfirmed = await _store.GetAsync(name, ct).ConfigureAwait(false);
|
||||
|
||||
return reconfirmed is not null && !reconfirmed.IsDeleted && MatchesShippedCiphertext(reconfirmed, shipped)
|
||||
? ApplyOutcome.Applied
|
||||
: ApplyOutcome.Unconfirmed;
|
||||
}
|
||||
|
||||
// local is live but its ciphertext differs from what was shipped: an out-of-band value
|
||||
// with a strictly newer (UpdatedUtc, Revision) won LWW. (local is null only in a
|
||||
// pathological concurrent-delete race; Unconfirmed covers that too.)
|
||||
return local is null ? ApplyOutcome.Unconfirmed : ApplyOutcome.LocalNewer;
|
||||
}
|
||||
|
||||
private static bool MatchesShippedCiphertext(StoredSecret local, StoredSecret shipped) =>
|
||||
local.Ciphertext.AsSpan().SequenceEqual(shipped.Ciphertext)
|
||||
&& local.WrappedDek.AsSpan().SequenceEqual(shipped.WrappedDek);
|
||||
|
||||
/// <summary>
|
||||
/// Trial-resolves <paramref name="row"/>'s secret through <see cref="ISecretResolver"/> to
|
||||
/// verify it decrypts on this node. On failure, appends a message to
|
||||
/// <paramref name="failed"/> and logs — Error the first time this exact (name, revision,
|
||||
/// kekId) has failed since it last resolved successfully on this node, Debug on every
|
||||
/// repeat (see the class remarks on error-log de-spam).
|
||||
/// <paramref name="failed"/> and logs with the shared de-spam rule (see the class remarks).
|
||||
/// </summary>
|
||||
private async Task VerifyAsync(DeployedSecretRow row, List<string> failed, CancellationToken ct)
|
||||
{
|
||||
@@ -211,7 +396,7 @@ public sealed class DeployedSecretsMirror : BackgroundService
|
||||
{
|
||||
resolved = await _resolver.GetAsync(new SecretName(row.Name), ct).ConfigureAwait(false) is not null;
|
||||
}
|
||||
catch (OperationCanceledException)
|
||||
catch (OperationCanceledException) when (ct.IsCancellationRequested)
|
||||
{
|
||||
throw;
|
||||
}
|
||||
@@ -229,16 +414,25 @@ public sealed class DeployedSecretsMirror : BackgroundService
|
||||
var message =
|
||||
$"Secret '{row.Name}' (kek {row.Secret.KekId}) did not decrypt on this node — " +
|
||||
"the node's master key differs from central's";
|
||||
ReportFailure(row.Name, $"decrypt:{row.Secret.Revision}:{row.Secret.KekId}", message, failed);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Appends <paramref name="message"/> to <paramref name="failed"/> and logs it — Error the
|
||||
/// first time <paramref name="fingerprint"/> is seen for <paramref name="name"/> since it
|
||||
/// last succeeded, Debug on every identical repeat.
|
||||
/// </summary>
|
||||
private void ReportFailure(string name, string fingerprint, string message, List<string> failed)
|
||||
{
|
||||
failed.Add(message);
|
||||
|
||||
var current = (row.Secret.Revision, row.Secret.KekId);
|
||||
if (_lastLoggedFailure.TryGetValue(row.Name, out var previous) && previous == current)
|
||||
if (_lastLoggedFailure.TryGetValue(name, out var previous) && previous == fingerprint)
|
||||
{
|
||||
_logger.LogDebug("{Failure}", message);
|
||||
}
|
||||
else
|
||||
{
|
||||
_lastLoggedFailure[row.Name] = current;
|
||||
_lastLoggedFailure[name] = fingerprint;
|
||||
_logger.LogError("{Failure}", message);
|
||||
}
|
||||
}
|
||||
@@ -253,12 +447,15 @@ public sealed class DeployedSecretsMirror : BackgroundService
|
||||
{
|
||||
await ReconcileAsync(stoppingToken).ConfigureAwait(false);
|
||||
}
|
||||
catch (OperationCanceledException)
|
||||
catch (OperationCanceledException) when (stoppingToken.IsCancellationRequested)
|
||||
{
|
||||
break;
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
// Covers every other failure, including an OperationCanceledException NOT
|
||||
// attributable to this service's own shutdown token (e.g. a store/command
|
||||
// timeout) — that must not silently end the loop (review Minor #4).
|
||||
_logger.LogWarning(ex, "Deployed secrets mirror pass failed; retrying next interval.");
|
||||
}
|
||||
} while (await timer.WaitForNextTickAsync(stoppingToken).ConfigureAwait(false));
|
||||
@@ -281,4 +478,16 @@ public sealed class DeployedSecretsMirror : BackgroundService
|
||||
CreatedUtc = secret.CreatedUtc,
|
||||
UpdatedUtc = secret.UpdatedUtc,
|
||||
};
|
||||
|
||||
/// <inheritdoc />
|
||||
public override void Dispose()
|
||||
{
|
||||
if (!_disposed)
|
||||
{
|
||||
_disposed = true;
|
||||
_gate.Dispose();
|
||||
}
|
||||
|
||||
base.Dispose();
|
||||
}
|
||||
}
|
||||
|
||||
+343
-71
@@ -14,6 +14,13 @@ namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Secrets;
|
||||
/// Task 9, site-secrets artifact deploy: <see cref="DeployedSecretsMirror"/> — per-node apply
|
||||
/// + decrypt-verify of <c>deployed_secrets</c> into this node's own secret store.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Fix round 1 (review <c>task-9-review.md</c>): <see cref="FakeSecretStore"/> now honours the
|
||||
/// real <c>SqliteSecretStore</c> semantics — <c>ApplyReplicatedAsync</c> is strict
|
||||
/// last-writer-wins via <see cref="SecretLastWriterWins.IsNewer"/>, and <c>DeleteAsync</c>
|
||||
/// tombstones rather than hard-deleting — so tests exercise the same conflict shapes the real
|
||||
/// store can produce, not an idealized "always overwrite" double.
|
||||
/// </remarks>
|
||||
public class DeployedSecretsMirrorTests : IDisposable
|
||||
{
|
||||
private readonly TestLocalDb _localDb;
|
||||
@@ -35,8 +42,11 @@ public class DeployedSecretsMirrorTests : IDisposable
|
||||
[Fact]
|
||||
public async Task Reconcile_Applies_Verifies_AndOnlyDeletesWhatItMirrored()
|
||||
{
|
||||
var (mirror, storage, store, _) = await CreateAsync();
|
||||
store.Seed("out-of-band");
|
||||
var (mirror, storage, store, _, _) = await CreateAsync();
|
||||
// Seeded OLDER than what will be shipped, so the shipped row legitimately wins LWW
|
||||
// and is applied — the strictly-NEWER out-of-band case is covered separately by
|
||||
// Reconcile_OutOfBandNewerSecret_NotOverwritten_NotLedgered_AndSurvivesLaterRemoval.
|
||||
store.Seed("out-of-band", updatedUtc: new DateTimeOffset(2026, 9, 27, 0, 0, 0, TimeSpan.Zero));
|
||||
|
||||
await storage.UpsertDeployedSecretsAsync([Art("a", 1), Art("out-of-band", 1)], "d1");
|
||||
var r1 = await mirror.ReconcileAsync(CancellationToken.None);
|
||||
@@ -45,8 +55,9 @@ public class DeployedSecretsMirrorTests : IDisposable
|
||||
Assert.Empty(r1.Failed);
|
||||
|
||||
await storage.UpsertDeployedSecretsAsync([], "d2");
|
||||
// Simulate: OUT-OF-BAND was never actually mirrored by this node (an operator secret
|
||||
// that happens to share the name) by clearing its ledger row directly.
|
||||
// Simulate: "out-of-band" was never actually mirrored by this node (an operator secret
|
||||
// that happens to share the name) by clearing its ledger row directly — a defensive
|
||||
// check independent of the natural not-ledgered path exercised below.
|
||||
await storage.RemoveMirroredSecretAsync("out-of-band");
|
||||
|
||||
var r2 = await mirror.ReconcileAsync(CancellationToken.None);
|
||||
@@ -54,12 +65,86 @@ public class DeployedSecretsMirrorTests : IDisposable
|
||||
Assert.Equal(["a"], r2.Removed);
|
||||
Assert.True(store.Contains("out-of-band"));
|
||||
Assert.False(store.Contains("a"));
|
||||
Assert.True(store.IsTombstoned("a"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Reconcile_OutOfBandNewerSecret_NotOverwritten_NotLedgered_AndSurvivesLaterRemoval()
|
||||
{
|
||||
var (mirror, storage, store, _, invalidator) = await CreateAsync();
|
||||
var operatorCiphertext = new byte[] { 200, 201, 202 };
|
||||
// Strictly newer than Art()'s fixed UpdatedUtc (2026-09-28T01:00:00Z) — legitimately
|
||||
// wins LWW, exactly as the real store would order it.
|
||||
store.Seed("a",
|
||||
updatedUtc: new DateTimeOffset(2026, 9, 28, 2, 0, 0, TimeSpan.Zero),
|
||||
revision: 5,
|
||||
ciphertext: operatorCiphertext);
|
||||
|
||||
await storage.UpsertDeployedSecretsAsync([Art("a", 1)], "d1");
|
||||
var r1 = await mirror.ReconcileAsync(CancellationToken.None);
|
||||
|
||||
Assert.Empty(r1.Applied);
|
||||
var message = Assert.Single(r1.Failed);
|
||||
Assert.Contains("'a'", message);
|
||||
Assert.Contains("newer local value", message);
|
||||
Assert.DoesNotContain("did not decrypt", message); // distinct from the master-key message
|
||||
Assert.Equal(operatorCiphertext, store.Rows["a"].Ciphertext); // untouched
|
||||
Assert.Empty(invalidator.Invalidated); // never applied -> never invalidated
|
||||
|
||||
// Never ledgered, so a later central tombstone must NOT delete the operator's secret.
|
||||
await storage.UpsertDeployedSecretsAsync([], "d2");
|
||||
var r2 = await mirror.ReconcileAsync(CancellationToken.None);
|
||||
|
||||
Assert.Empty(r2.Removed);
|
||||
Assert.True(store.Contains("a"));
|
||||
Assert.Equal(operatorCiphertext, store.Rows["a"].Ciphertext);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Reconcile_RemoveThenReAdd_TombstoneIsRecovered_ReappliesAndVerifies()
|
||||
{
|
||||
var time = new FakeTimeProvider(new DateTimeOffset(2026, 9, 28, 6, 0, 0, TimeSpan.Zero));
|
||||
var (mirror, storage, store, _, invalidator) = await CreateAsync(time: time);
|
||||
await storage.UpsertDeployedSecretsAsync([Art("a", 1)], "d1");
|
||||
var firstApply = await mirror.ReconcileAsync(CancellationToken.None);
|
||||
Assert.Equal(["a"], firstApply.Applied);
|
||||
|
||||
// Site stops referencing it: central tombstones the row, this node deletes it. The
|
||||
// tombstone is stamped LATER than the originally-shipped UpdatedUtc (01:00).
|
||||
store.Now = new DateTimeOffset(2026, 9, 28, 12, 0, 0, TimeSpan.Zero);
|
||||
await storage.UpsertDeployedSecretsAsync([], "d2");
|
||||
var removePass = await mirror.ReconcileAsync(CancellationToken.None);
|
||||
Assert.Equal(["a"], removePass.Removed);
|
||||
Assert.True(store.IsTombstoned("a"));
|
||||
invalidator.Invalidated.Clear();
|
||||
|
||||
// Site references it again — central ships the SAME (unchanged, older-stamped)
|
||||
// secret. A naive ApplyReplicatedAsync call is a no-op against the newer tombstone;
|
||||
// the mirror must detect and recover this.
|
||||
time.Now = new DateTimeOffset(2026, 9, 28, 18, 0, 0, TimeSpan.Zero); // later than the tombstone
|
||||
await storage.UpsertDeployedSecretsAsync([Art("a", 1)], "d3");
|
||||
var reAddPass = await mirror.ReconcileAsync(CancellationToken.None);
|
||||
|
||||
Assert.Equal(["a"], reAddPass.Applied);
|
||||
Assert.Empty(reAddPass.Failed);
|
||||
Assert.True(store.Contains("a"));
|
||||
Assert.Equal(new byte[] { 1, 2, 3, 4, 5 }, store.Rows["a"].Ciphertext); // Art()'s ciphertext, re-materialized
|
||||
Assert.Contains("a", invalidator.Invalidated);
|
||||
|
||||
// Ledgering used the SHIPPED artifact's own revision, not the bumped local one, so a
|
||||
// later pass with the same shipped row is a true no-op.
|
||||
var thirdPass = await mirror.ReconcileAsync(CancellationToken.None);
|
||||
Assert.Empty(thirdPass.Applied);
|
||||
Assert.False(thirdPass.Changed);
|
||||
|
||||
var mirrored = await storage.GetMirroredSecretsAsync();
|
||||
Assert.Equal(1, mirrored.Single(m => m.Name == "a").Revision);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Reconcile_ReAppliesOnlyWhenRevisionChanges()
|
||||
{
|
||||
var (mirror, storage, store, _) = await CreateAsync();
|
||||
var (mirror, storage, store, _, _) = await CreateAsync();
|
||||
|
||||
await storage.UpsertDeployedSecretsAsync([Art("a", 1)], "d1");
|
||||
var r1 = await mirror.ReconcileAsync(CancellationToken.None);
|
||||
@@ -72,18 +157,18 @@ public class DeployedSecretsMirrorTests : IDisposable
|
||||
await storage.UpsertDeployedSecretsAsync([Art("a", 2)], "d2");
|
||||
var r3 = await mirror.ReconcileAsync(CancellationToken.None);
|
||||
Assert.Equal(["a"], r3.Applied);
|
||||
Assert.Equal(2, store.LastApplied["a"].Revision);
|
||||
Assert.Equal(2, store.Rows["a"].Revision);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Reconcile_AppliesThroughApplyReplicatedAsync_MappedFromSecretArtifact()
|
||||
{
|
||||
var (mirror, storage, store, _) = await CreateAsync();
|
||||
var (mirror, storage, store, _, _) = await CreateAsync();
|
||||
await storage.UpsertDeployedSecretsAsync([Art("a", 1, kek: "kek-central")], "d1");
|
||||
|
||||
await mirror.ReconcileAsync(CancellationToken.None);
|
||||
|
||||
var applied = store.LastApplied["a"];
|
||||
var applied = store.Rows["a"];
|
||||
Assert.Equal("kek-central", applied.KekId);
|
||||
Assert.Equal(1, applied.Revision);
|
||||
Assert.Equal(SecretContentType.Text, applied.ContentType);
|
||||
@@ -96,34 +181,119 @@ public class DeployedSecretsMirrorTests : IDisposable
|
||||
[InlineData(SecretContentType.BinaryBase64)]
|
||||
public async Task Reconcile_RoundTripsEveryContentType(SecretContentType contentType)
|
||||
{
|
||||
var (mirror, storage, store, _) = await CreateAsync();
|
||||
var (mirror, storage, store, _, _) = await CreateAsync();
|
||||
await storage.UpsertDeployedSecretsAsync(
|
||||
[Art("a", 1, contentType: contentType.ToString())], "d1");
|
||||
|
||||
await mirror.ReconcileAsync(CancellationToken.None);
|
||||
|
||||
Assert.Equal(contentType, store.LastApplied["a"].ContentType);
|
||||
Assert.Equal(contentType, store.Rows["a"].ContentType);
|
||||
}
|
||||
|
||||
// ── Per-row isolation (ruling 2) ──
|
||||
|
||||
[Fact]
|
||||
public async Task Reconcile_OneRowThrows_OthersStillApply()
|
||||
{
|
||||
var (mirror, storage, store, _, _) = await CreateAsync();
|
||||
// "bad" carries an unrecognised ContentType (as a newer central under version skew
|
||||
// might ship), so ToStored's Enum.Parse throws for that row only.
|
||||
await storage.UpsertDeployedSecretsAsync(
|
||||
[Art("bad", 1, contentType: "NotARealContentType"), Art("good", 1)], "d1");
|
||||
|
||||
var r = await mirror.ReconcileAsync(CancellationToken.None);
|
||||
|
||||
Assert.Equal(["good"], r.Applied);
|
||||
var message = Assert.Single(r.Failed);
|
||||
Assert.Contains("'bad'", message);
|
||||
Assert.True(store.Contains("good"));
|
||||
Assert.False(store.Contains("bad"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Reconcile_OneRowThrows_FailureIsDeSpammedLikeAnyOtherFailure()
|
||||
{
|
||||
var logger = new CapturingLogger<DeployedSecretsMirror>();
|
||||
var (mirror, storage, _, _, _) = await CreateAsync(logger);
|
||||
await storage.UpsertDeployedSecretsAsync([Art("bad", 1, contentType: "NotARealContentType")], "d1");
|
||||
|
||||
await mirror.ReconcileAsync(CancellationToken.None);
|
||||
await mirror.ReconcileAsync(CancellationToken.None);
|
||||
|
||||
var levels = logger.Entries
|
||||
.Where(e => e.Message.Contains("'bad'") && e.Message.Contains("failed to process"))
|
||||
.Select(e => e.Level)
|
||||
.ToList();
|
||||
Assert.Equal([LogLevel.Error, LogLevel.Debug], levels);
|
||||
}
|
||||
|
||||
// ── Cache invalidation (ruling 3) ──
|
||||
|
||||
[Fact]
|
||||
public async Task Reconcile_SuccessfulApply_InvalidatesCache_BeforeVerify()
|
||||
{
|
||||
var (mirror, storage, _, _, invalidator) = await CreateAsync();
|
||||
await storage.UpsertDeployedSecretsAsync([Art("a", 1)], "d1");
|
||||
|
||||
await mirror.ReconcileAsync(CancellationToken.None);
|
||||
|
||||
Assert.Contains("a", invalidator.Invalidated);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Reconcile_Removal_InvalidatesCache()
|
||||
{
|
||||
var (mirror, storage, _, _, invalidator) = await CreateAsync();
|
||||
await storage.UpsertDeployedSecretsAsync([Art("a", 1)], "d1");
|
||||
await mirror.ReconcileAsync(CancellationToken.None);
|
||||
invalidator.Invalidated.Clear();
|
||||
|
||||
await storage.UpsertDeployedSecretsAsync([], "d2");
|
||||
await mirror.ReconcileAsync(CancellationToken.None);
|
||||
|
||||
Assert.Contains("a", invalidator.Invalidated);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Reconcile_WithNoInvalidatorRegistered_StillApplies()
|
||||
{
|
||||
var storage = new SiteStorageService(_localDb.Db, NullLogger<SiteStorageService>.Instance);
|
||||
await storage.InitializeAsync();
|
||||
var mirror = new DeployedSecretsMirror(
|
||||
storage,
|
||||
new FakeSecretStore(),
|
||||
new FakeSecretResolver(),
|
||||
invalidator: null,
|
||||
Options.Create(new SiteRuntimeOptions { DeployedSecretsMirrorInterval = TimeSpan.FromSeconds(15) }),
|
||||
NullLogger<DeployedSecretsMirror>.Instance,
|
||||
TimeProvider.System);
|
||||
|
||||
await storage.UpsertDeployedSecretsAsync([Art("a", 1)], "d1");
|
||||
var r = await mirror.ReconcileAsync(CancellationToken.None);
|
||||
|
||||
Assert.Equal(["a"], r.Applied);
|
||||
}
|
||||
|
||||
// ── Decrypt verification ──
|
||||
|
||||
[Fact]
|
||||
public async Task Reconcile_DecryptFailure_IsReportedWithKekId()
|
||||
public async Task Reconcile_DecryptFailure_ExceptionFromResolver_IsReportedWithKekId()
|
||||
{
|
||||
var (mirror, storage, _, resolver) = await CreateAsync();
|
||||
resolver.FailFor("a");
|
||||
var (mirror, storage, _, resolver, _) = await CreateAsync();
|
||||
resolver.ThrowFor("a");
|
||||
|
||||
await storage.UpsertDeployedSecretsAsync([Art("a", 1, kek: "kek-central")], "d1");
|
||||
var r = await mirror.ReconcileAsync(CancellationToken.None);
|
||||
|
||||
Assert.Contains("kek-central", Assert.Single(r.Failed));
|
||||
Assert.Contains("'a'", Assert.Single(r.Failed));
|
||||
var message = Assert.Single(r.Failed);
|
||||
Assert.Contains("kek-central", message);
|
||||
Assert.Contains("'a'", message);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Reconcile_DecryptFailure_NullResolve_IsAlsoReported()
|
||||
{
|
||||
var (mirror, storage, _, resolver) = await CreateAsync();
|
||||
var (mirror, storage, _, resolver, _) = await CreateAsync();
|
||||
resolver.ReturnNullFor("a");
|
||||
|
||||
await storage.UpsertDeployedSecretsAsync([Art("a", 1)], "d1");
|
||||
@@ -132,22 +302,10 @@ public class DeployedSecretsMirrorTests : IDisposable
|
||||
Assert.Single(r.Failed);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Reconcile_DecryptFailure_ExceptionFromResolver_IsAlsoReported()
|
||||
{
|
||||
var (mirror, storage, _, resolver) = await CreateAsync();
|
||||
resolver.ThrowFor("a");
|
||||
|
||||
await storage.UpsertDeployedSecretsAsync([Art("a", 1)], "d1");
|
||||
var r = await mirror.ReconcileAsync(CancellationToken.None);
|
||||
|
||||
Assert.Single(r.Failed);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Reconcile_VerifiesEveryPass_EvenWhenAlreadyApplied()
|
||||
{
|
||||
var (mirror, storage, _, resolver) = await CreateAsync();
|
||||
var (mirror, storage, _, resolver, _) = await CreateAsync();
|
||||
await storage.UpsertDeployedSecretsAsync([Art("a", 1)], "d1");
|
||||
|
||||
var r1 = await mirror.ReconcileAsync(CancellationToken.None);
|
||||
@@ -156,20 +314,20 @@ public class DeployedSecretsMirrorTests : IDisposable
|
||||
// Master key "changes" (or a KEK rotation invalidates the local unwrap) between
|
||||
// passes — even though nothing about deployed_secrets changed, the next reconcile
|
||||
// must still catch the failure because it re-verifies unconditionally.
|
||||
resolver.FailFor("a");
|
||||
resolver.ThrowFor("a");
|
||||
var r2 = await mirror.ReconcileAsync(CancellationToken.None);
|
||||
Assert.Single(r2.Failed);
|
||||
Assert.Empty(r2.Applied); // unchanged revision — not re-applied, only re-verified
|
||||
}
|
||||
|
||||
// ── Error-log de-spam (ruling 1) ──
|
||||
// ── Error-log de-spam (ruling 1 of the original task, unaffected by fix round 1) ──
|
||||
|
||||
[Fact]
|
||||
public async Task Reconcile_DecryptFailure_LogsErrorOnlyFirstTime_ThenDebug_ThenErrorAgainAfterASuccess()
|
||||
{
|
||||
var logger = new CapturingLogger<DeployedSecretsMirror>();
|
||||
var (mirror, storage, _, resolver) = await CreateAsync(logger);
|
||||
resolver.FailFor("a");
|
||||
var (mirror, storage, _, resolver, _) = await CreateAsync(logger);
|
||||
resolver.ThrowFor("a");
|
||||
await storage.UpsertDeployedSecretsAsync([Art("a", 1, kek: "kek-1")], "d1");
|
||||
|
||||
await mirror.ReconcileAsync(CancellationToken.None); // 1st failure: Error
|
||||
@@ -190,7 +348,7 @@ public class DeployedSecretsMirrorTests : IDisposable
|
||||
Assert.Empty(recovered.Failed);
|
||||
|
||||
// Fails again — must log at Error again, not Debug, since it succeeded in between.
|
||||
resolver.FailFor("a");
|
||||
resolver.ThrowFor("a");
|
||||
await mirror.ReconcileAsync(CancellationToken.None);
|
||||
|
||||
var failureEntriesAfterRecovery = logger.Entries
|
||||
@@ -204,8 +362,8 @@ public class DeployedSecretsMirrorTests : IDisposable
|
||||
public async Task Reconcile_DecryptFailure_NewRevision_LogsErrorAgain_EvenWithoutASuccessInBetween()
|
||||
{
|
||||
var logger = new CapturingLogger<DeployedSecretsMirror>();
|
||||
var (mirror, storage, _, resolver) = await CreateAsync(logger);
|
||||
resolver.FailFor("a");
|
||||
var (mirror, storage, _, resolver, _) = await CreateAsync(logger);
|
||||
resolver.ThrowFor("a");
|
||||
|
||||
await storage.UpsertDeployedSecretsAsync([Art("a", 1, kek: "kek-1")], "d1");
|
||||
await mirror.ReconcileAsync(CancellationToken.None); // Error (rev 1)
|
||||
@@ -222,12 +380,63 @@ public class DeployedSecretsMirrorTests : IDisposable
|
||||
Assert.Equal([LogLevel.Error, LogLevel.Debug, LogLevel.Error], levels);
|
||||
}
|
||||
|
||||
// ── Cancellation + concurrency (ruling 6) ──
|
||||
|
||||
[Fact]
|
||||
public async Task ReconcileAsync_PreCancelledToken_ThrowsOperationCanceled()
|
||||
{
|
||||
var (mirror, _, _, _, _) = await CreateAsync();
|
||||
using var cts = new CancellationTokenSource();
|
||||
cts.Cancel();
|
||||
|
||||
await Assert.ThrowsAnyAsync<OperationCanceledException>(
|
||||
() => mirror.ReconcileAsync(cts.Token));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ReconcileAsync_ConcurrentCallers_BothCompleteWithoutDoubleApply()
|
||||
{
|
||||
var (mirror, storage, store, _, _) = await CreateAsync();
|
||||
await storage.UpsertDeployedSecretsAsync([Art("a", 1)], "d1");
|
||||
|
||||
var t1 = mirror.ReconcileAsync(CancellationToken.None);
|
||||
var t2 = mirror.ReconcileAsync(CancellationToken.None);
|
||||
var results = await Task.WhenAll(t1, t2);
|
||||
|
||||
// The gate serializes the two passes: whichever runs first applies it, the other
|
||||
// sees it already ledgered at the shipped revision and applies nothing.
|
||||
Assert.Equal(1, results.Sum(r => r.Applied.Count));
|
||||
Assert.True(store.Contains("a"));
|
||||
|
||||
var mirrored = await storage.GetMirroredSecretsAsync();
|
||||
Assert.Equal(1, mirrored.Single(m => m.Name == "a").Revision);
|
||||
}
|
||||
|
||||
// ── Dispose (ruling 6) ──
|
||||
|
||||
[Fact]
|
||||
public void Dispose_DoesNotThrow_AndIsIdempotent()
|
||||
{
|
||||
var storage = new SiteStorageService(_localDb.Db, NullLogger<SiteStorageService>.Instance);
|
||||
var mirror = new DeployedSecretsMirror(
|
||||
storage,
|
||||
new FakeSecretStore(),
|
||||
new FakeSecretResolver(),
|
||||
invalidator: null,
|
||||
Options.Create(new SiteRuntimeOptions()),
|
||||
NullLogger<DeployedSecretsMirror>.Instance,
|
||||
TimeProvider.System);
|
||||
|
||||
mirror.Dispose();
|
||||
mirror.Dispose();
|
||||
}
|
||||
|
||||
// ── Hosted loop ──
|
||||
|
||||
[Fact]
|
||||
public async Task ExecuteAsync_RunsOnceAtStart_AndAgainEveryInterval()
|
||||
{
|
||||
var (mirror, storage, store, resolver) = await CreateAsync(
|
||||
var (mirror, storage, store, resolver, _) = await CreateAsync(
|
||||
interval: TimeSpan.FromMilliseconds(30));
|
||||
await storage.UpsertDeployedSecretsAsync([Art("a", 1)], "d1");
|
||||
|
||||
@@ -250,10 +459,10 @@ public class DeployedSecretsMirrorTests : IDisposable
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ExecuteAsync_ReconcileExceptionIsLoggedAndLoopContinues()
|
||||
public async Task ExecuteAsync_PerRowFailure_IsIsolated_AndLoopRecoversOnceCleared()
|
||||
{
|
||||
var logger = new CapturingLogger<DeployedSecretsMirror>();
|
||||
var (mirror, storage, store, _) = await CreateAsync(
|
||||
var (mirror, storage, store, _, _) = await CreateAsync(
|
||||
logger, interval: TimeSpan.FromMilliseconds(30));
|
||||
await storage.UpsertDeployedSecretsAsync([Art("a", 1)], "d1");
|
||||
store.ThrowOnApply = true;
|
||||
@@ -261,8 +470,10 @@ public class DeployedSecretsMirrorTests : IDisposable
|
||||
await mirror.StartAsync(CancellationToken.None);
|
||||
try
|
||||
{
|
||||
// A per-row store fault is isolated (ruling 2): it never escapes ReconcileAsync,
|
||||
// so it shows up as a row-level Error log, not the pass-level Warning.
|
||||
await WaitUntilAsync(() => logger.Entries.Any(
|
||||
e => e.Level == LogLevel.Warning && e.Message.Contains("failed")));
|
||||
e => e.Level == LogLevel.Error && e.Message.Contains("failed to process")));
|
||||
|
||||
// The loop must still be alive after the failure — force success and confirm a
|
||||
// later pass applies the secret.
|
||||
@@ -278,15 +489,22 @@ public class DeployedSecretsMirrorTests : IDisposable
|
||||
|
||||
// ── Harness ──
|
||||
|
||||
private async Task<(DeployedSecretsMirror Mirror, SiteStorageService Storage, FakeSecretStore Store, FakeSecretResolver Resolver)> CreateAsync(
|
||||
private async Task<(
|
||||
DeployedSecretsMirror Mirror,
|
||||
SiteStorageService Storage,
|
||||
FakeSecretStore Store,
|
||||
FakeSecretResolver Resolver,
|
||||
FakeSecretCacheInvalidator Invalidator)> CreateAsync(
|
||||
CapturingLogger<DeployedSecretsMirror>? logger = null,
|
||||
TimeSpan? interval = null)
|
||||
TimeSpan? interval = null,
|
||||
TimeProvider? time = null)
|
||||
{
|
||||
var storage = new SiteStorageService(_localDb.Db, NullLogger<SiteStorageService>.Instance);
|
||||
await storage.InitializeAsync();
|
||||
|
||||
var store = new FakeSecretStore();
|
||||
var resolver = new FakeSecretResolver();
|
||||
var invalidator = new FakeSecretCacheInvalidator();
|
||||
var options = Options.Create(new SiteRuntimeOptions
|
||||
{
|
||||
DeployedSecretsMirrorInterval = interval ?? TimeSpan.FromSeconds(15),
|
||||
@@ -296,11 +514,12 @@ public class DeployedSecretsMirrorTests : IDisposable
|
||||
storage,
|
||||
store,
|
||||
resolver,
|
||||
invalidator,
|
||||
options,
|
||||
(ILogger<DeployedSecretsMirror>?)logger ?? NullLogger<DeployedSecretsMirror>.Instance,
|
||||
TimeProvider.System);
|
||||
time ?? TimeProvider.System);
|
||||
|
||||
return (mirror, storage, store, resolver);
|
||||
return (mirror, storage, store, resolver, invalidator);
|
||||
}
|
||||
|
||||
private static SecretArtifact Art(
|
||||
@@ -345,38 +564,68 @@ public class DeployedSecretsMirrorTests : IDisposable
|
||||
=> Entries.Enqueue((logLevel, formatter(state, exception)));
|
||||
}
|
||||
|
||||
/// <summary>Minimal controllable <see cref="TimeProvider"/> — mirrors the pattern used by <c>ApiMethodCacheTests</c>.</summary>
|
||||
private sealed class FakeTimeProvider : TimeProvider
|
||||
{
|
||||
public DateTimeOffset Now { get; set; }
|
||||
public FakeTimeProvider(DateTimeOffset start) => Now = start;
|
||||
public override DateTimeOffset GetUtcNow() => Now;
|
||||
}
|
||||
|
||||
/// <summary>Records every <see cref="ISecretCacheInvalidator.Invalidate"/> call by secret name.</summary>
|
||||
private sealed class FakeSecretCacheInvalidator : ISecretCacheInvalidator
|
||||
{
|
||||
public List<string> Invalidated { get; } = new();
|
||||
public void Invalidate(SecretName name) => Invalidated.Add(name.Value);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// In-memory <see cref="ISecretStore"/> recording <see cref="ApplyReplicatedAsync"/> /
|
||||
/// <see cref="DeleteAsync"/> calls, for asserting what the mirror applied/removed without
|
||||
/// a real ZB.MOM.WW.Secrets store.
|
||||
/// In-memory <see cref="ISecretStore"/> honouring the REAL <c>SqliteSecretStore</c>
|
||||
/// semantics (fix round 1, review Important #1): <see cref="ApplyReplicatedAsync"/> is
|
||||
/// strict last-writer-wins via <see cref="SecretLastWriterWins.IsNewer"/> and applies
|
||||
/// verbatim (no revision bump); <see cref="DeleteAsync"/> tombstones in place (bumps
|
||||
/// revision, stamps <see cref="Now"/>) rather than hard-deleting.
|
||||
/// </summary>
|
||||
private sealed class FakeSecretStore : ISecretStore
|
||||
{
|
||||
private readonly Dictionary<string, StoredSecret> _rows = new(StringComparer.Ordinal);
|
||||
|
||||
public IReadOnlyDictionary<string, StoredSecret> LastApplied => _rows;
|
||||
/// <summary>
|
||||
/// Controllable "now" used to stamp a <see cref="DeleteAsync"/> tombstone —
|
||||
/// deterministic, not wall-clock, so tests can control ordering against a shipped
|
||||
/// artifact's fixed <c>UpdatedUtc</c>.
|
||||
/// </summary>
|
||||
public DateTimeOffset Now { get; set; } = new DateTimeOffset(2026, 9, 28, 12, 0, 0, TimeSpan.Zero);
|
||||
|
||||
/// <summary>Set to make the next <see cref="ApplyReplicatedAsync"/> call throw, simulating a store fault.</summary>
|
||||
public bool ThrowOnApply { get; set; }
|
||||
|
||||
/// <summary>Seeds a row directly (bypassing ApplyReplicatedAsync) — an out-of-band secret this node never mirrored itself.</summary>
|
||||
public void Seed(string name) => _rows[name] = new StoredSecret
|
||||
{
|
||||
Name = new SecretName(name),
|
||||
ContentType = SecretContentType.Text,
|
||||
Ciphertext = [9],
|
||||
Nonce = [9],
|
||||
Tag = [9],
|
||||
WrappedDek = [9],
|
||||
WrapNonce = [9],
|
||||
WrapTag = [9],
|
||||
KekId = "kek-out-of-band",
|
||||
Revision = 1,
|
||||
CreatedUtc = DateTimeOffset.UtcNow,
|
||||
UpdatedUtc = DateTimeOffset.UtcNow,
|
||||
};
|
||||
/// <summary>Every row currently held, including tombstones, for direct inspection.</summary>
|
||||
public IReadOnlyDictionary<string, StoredSecret> Rows => _rows;
|
||||
|
||||
public bool Contains(string name) => _rows.ContainsKey(name);
|
||||
/// <summary>Seeds a LIVE row directly (bypassing ApplyReplicatedAsync/LWW) — an out-of-band secret.</summary>
|
||||
public void Seed(string name, DateTimeOffset updatedUtc, long revision = 1, byte[]? ciphertext = null) =>
|
||||
_rows[name] = new StoredSecret
|
||||
{
|
||||
Name = new SecretName(name),
|
||||
ContentType = SecretContentType.Text,
|
||||
Ciphertext = ciphertext ?? [9],
|
||||
Nonce = [9],
|
||||
Tag = [9],
|
||||
WrappedDek = [9],
|
||||
WrapNonce = [9],
|
||||
WrapTag = [9],
|
||||
KekId = "kek-out-of-band",
|
||||
Revision = revision,
|
||||
CreatedUtc = updatedUtc,
|
||||
UpdatedUtc = updatedUtc,
|
||||
};
|
||||
|
||||
/// <summary>True if the row is present and NOT tombstoned.</summary>
|
||||
public bool Contains(string name) => _rows.TryGetValue(name, out var row) && !row.IsDeleted;
|
||||
|
||||
/// <summary>True if the row is present as a tombstone.</summary>
|
||||
public bool IsTombstoned(string name) => _rows.TryGetValue(name, out var row) && row.IsDeleted;
|
||||
|
||||
public Task<StoredSecret?> GetAsync(SecretName name, CancellationToken ct) =>
|
||||
Task.FromResult(_rows.TryGetValue(name.Value, out var row) ? row : null);
|
||||
@@ -387,8 +636,24 @@ public class DeployedSecretsMirrorTests : IDisposable
|
||||
return Task.CompletedTask;
|
||||
}
|
||||
|
||||
public Task<bool> DeleteAsync(SecretName name, string? actor, CancellationToken ct) =>
|
||||
Task.FromResult(_rows.Remove(name.Value));
|
||||
/// <summary>Tombstones (does not hard-delete) — matches <c>SqliteSecretStore.DeleteAsync</c>.</summary>
|
||||
public Task<bool> DeleteAsync(SecretName name, string? actor, CancellationToken ct)
|
||||
{
|
||||
if (!_rows.TryGetValue(name.Value, out var existing) || existing.IsDeleted)
|
||||
{
|
||||
return Task.FromResult(false);
|
||||
}
|
||||
|
||||
_rows[name.Value] = existing with
|
||||
{
|
||||
IsDeleted = true,
|
||||
DeletedUtc = Now,
|
||||
UpdatedUtc = Now,
|
||||
Revision = existing.Revision + 1,
|
||||
UpdatedBy = actor,
|
||||
};
|
||||
return Task.FromResult(true);
|
||||
}
|
||||
|
||||
public Task<IReadOnlyList<SecretMetadata>> ListAsync(bool includeDeleted, CancellationToken ct) =>
|
||||
Task.FromResult<IReadOnlyList<SecretMetadata>>([]);
|
||||
@@ -396,6 +661,7 @@ public class DeployedSecretsMirrorTests : IDisposable
|
||||
public Task<IReadOnlyList<SecretManifestEntry>> GetManifestAsync(CancellationToken ct) =>
|
||||
Task.FromResult<IReadOnlyList<SecretManifestEntry>>([]);
|
||||
|
||||
/// <summary>Strict LWW on (UpdatedUtc, Revision) via <see cref="SecretLastWriterWins.IsNewer"/>; applied verbatim — matches <c>SqliteSecretStore</c>.</summary>
|
||||
public Task ApplyReplicatedAsync(StoredSecret row, CancellationToken ct)
|
||||
{
|
||||
if (ThrowOnApply)
|
||||
@@ -403,9 +669,12 @@ public class DeployedSecretsMirrorTests : IDisposable
|
||||
throw new InvalidOperationException("simulated store fault");
|
||||
}
|
||||
|
||||
// Test double only — real store applies last-writer-wins; here every apply
|
||||
// just overwrites, which is fine since these tests never seed a conflicting
|
||||
// higher local revision through this path.
|
||||
if (_rows.TryGetValue(row.Name.Value, out var existing)
|
||||
&& !SecretLastWriterWins.IsNewer(row.UpdatedUtc, row.Revision, existing.UpdatedUtc, existing.Revision))
|
||||
{
|
||||
return Task.CompletedTask; // no-op, matches real semantics
|
||||
}
|
||||
|
||||
_rows[row.Name.Value] = row;
|
||||
return Task.CompletedTask;
|
||||
}
|
||||
@@ -423,9 +692,12 @@ public class DeployedSecretsMirrorTests : IDisposable
|
||||
|
||||
public int CallCount;
|
||||
|
||||
public void FailFor(string name) => _nullReturning.Add(name);
|
||||
/// <summary>Row absent/tombstoned path — <c>DefaultSecretResolver.GetAsync</c> returns null.</summary>
|
||||
public void ReturnNullFor(string name) => _nullReturning.Add(name);
|
||||
|
||||
/// <summary>Decryption/integrity failure path — <c>DefaultSecretResolver.GetAsync</c> rethrows <c>SecretDecryptionException</c>.</summary>
|
||||
public void ThrowFor(string name) => _throwing.Add(name);
|
||||
|
||||
public void ClearFailures()
|
||||
{
|
||||
_nullReturning.Clear();
|
||||
|
||||
Reference in New Issue
Block a user