From a1df7ef26c0490b4205e5d74063e79666a79a898 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sat, 18 Jul 2026 14:39:40 -0400 Subject: [PATCH] fix(secrets): root-cause + fix the Akka replicator's hosted-process DI deadlock (0.2.2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the defect in scadaproj#1. The hang was never Akka: the package's DI wiring closed a circular singleton dependency the container cannot see through factory lambdas — ISecretStore (ReplicatingSecretStore decorator) -> ISecretReplicator -> SecretReplicationActorProvider -> ISecretCacheInvalidator -> DefaultSecretResolver -> ISecretStore. Resolution recurses around the loop until MS.DI's StackGuard hops it onto a fresh thread-pool thread, which then blocks forever on a singleton call-site lock the first thread still holds: a silent permanent hang instead of a stack overflow. Managed stacks from dotnet-dump show the repeating cycle and both parked threads; both candidate causes in the issue (DistributedPubSub.Get vs the Lazy lock, missing Akka.Cluster.Tools HOCON) are disproven — the actor constructor was never reached, and the deadlock reproduces on a single non-clustered node. Fix: defer the one cycle-closing edge. The provider now gets a DeferredSecretCacheInvalidator that resolves the real invalidator on first eviction — which only happens when a replicated row is applied, strictly after graph resolution. Severing the edge instead is wrong: a null-invalidator experiment ran the live gate at 5/6, with deleted secrets still resolving on the peer. The SqlServer package never had the cycle (its replicator chain never touches the invalidator), which is why the hub gate always passed. Verified: live 2-node convergence gate now 6/6 (was: infinite hang), including the delete-visibility check that proves the deferred invalidator really evicts. New HostedProcessResolutionTests builds the graph as a host does (container- registered ActorSystem, hosted services, watchdogged resolves) and fails on 0.2.1; DeferredSecretCacheInvalidatorTests pins the wrapper contract. Full suite 180 passed / 0 failed / 15 skipped (env-gated live SQL). Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts --- CLAUDE.md | 2 +- ZB.MOM.WW.Secrets/Directory.Build.props | 2 +- .../AkkaSecretsServiceCollectionExtensions.cs | 9 +- .../DeferredSecretCacheInvalidator.cs | 39 +++++ .../AddZbSecretsAkkaReplicationTests.cs | 14 +- .../DeferredSecretCacheInvalidatorTests.cs | 75 +++++++++ .../HostedProcessResolutionTests.cs | 153 ++++++++++++++++++ components/secrets/GAPS.md | 50 +++--- 8 files changed, 317 insertions(+), 27 deletions(-) create mode 100644 ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet/DependencyInjection/DeferredSecretCacheInvalidator.cs create mode 100644 ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests/DependencyInjection/DeferredSecretCacheInvalidatorTests.cs create mode 100644 ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests/DependencyInjection/HostedProcessResolutionTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index 0e95818..ad59a73 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -148,7 +148,7 @@ each project's **code-verified current state**, and the **gaps** between. See | Config + validation (options / startup validation) | Adopted (lib `0.1.0`, on the feed; **all 4 apps**, pushed — the "local only / not yet pushed" caveat was stale) | Shared `ZB.MOM.WW.Configuration` lib | [`components/configuration/`](components/configuration/) | [`ZB.MOM.WW.Configuration/`](ZB.MOM.WW.Configuration/) | | Audit (event model + writer seam) | Adopted (lib `0.1.0`, on the feed; **all 4 apps**, pushed to origin) | Shared `ZB.MOM.WW.Audit` lib | [`components/audit/`](components/audit/) | [`ZB.MOM.WW.Audit/`](ZB.MOM.WW.Audit/) | | Galaxy Repository (object-hierarchy SQL browse + gRPC service) | **Adopted** (feed `0.1.0` + `0.2.0`; consumed at `0.2.0` by HistorianGateway **and mxaccessgw**, whose Server wires `AddZbGalaxyRepository` — verified 2026-07-18; the prior "mxaccessgw adoption is a follow-on" claim was stale) | Shared `ZB.MOM.WW.GalaxyRepository` lib | _(design in histsdk + design doc 2026-06-23)_ | [`ZB.MOM.WW.GalaxyRepository/`](ZB.MOM.WW.GalaxyRepository/) | -| Secrets (encrypted store + `${secret:}` resolution) | Built (libs **`0.1.3`**, **published to the Gitea feed**; **HistorianGateway adopted + live-proven** 2026-07-16; **mxaccessgw adopted G-4/G-5/G-6 + merged to `origin/main` @ `e088dfa`** 2026-07-16, box-verified fail-closed + encrypt-at-rest; **ScadaBridge adopted G-3/G-4/G-5/G-6 + merged to `origin/main` @ `128f1596`** 2026-07-16, **G-3 live-proven vs the real production MxGateway gateway** `wonder-app-vd03:5120` (secret-ref ApiKey → Connected + browsed real Galaxy); **OtOpcUa adopted G-2/G-4/G-5/G-6 + merged to `origin/master` @ `872cf7e3`** (lmxopcua) 2026-07-16 — the last app, so **all four now adopted**; Layer-B driver secrets (Galaxy API key + OpcUaClient `Password`/`UserCertificatePassword`) resolve fail-closed at driver session-open, `AddZbSecrets` registered unconditionally so driver-only nodes work; **G-2 live-proven vs the real production MxGateway** `wonder-app-vd03:5120` — a `secret:`-ref Galaxy ApiKey resolved through OtOpcUa's real `GalaxyDriverBrowser` → dummy key `MxGatewayAuthenticationException`, real key → browsed the real Galaxy root (10 nodes); temp key minted+revoked+deleted); **G-8 KEK-rotation BUILT (lib bumped `0.1.2`→`0.1.3`, 2026-07-17):** `Rewrap` DEK primitive + CAS-guarded `ApplyRewrapAsync` + `KekRotationService.RewrapAllAsync` + `secret rewrap-all` CLI + operator runbook (`ZB.MOM.WW.Secrets/docs/operations/kek-rotation.md`), full suite green + CLI smoke + adversarial crypto review PASS (TOCTOU closed via compare-and-swap). **G-7 clustered replication BUILT 2026-07-18 (lib `0.2.0`) — BOTH fork options, packaged as two new libs:** **`ZB.MOM.WW.Secrets.Replicator.SqlServer`** (shared SQL-Server `ISecretStore` = the plan's Option A, *plus* a local-store-with-hub mode) and **`ZB.MOM.WW.Secrets.Replicator.AkkaDotNet`** (peer-to-peer over distributed pub/sub = Option B, which the plan had deferred). Core gained `ISecretsStoreMigrator`, `SecretLastWriterWins` (one shared LWW tie-break so stores cannot diverge), `SecretReplicationReconciler`, and `ReplicatingSecretStore` — the last of which fixes a real gap: **nothing had ever called `ISecretReplicator.PublishAsync`**, so the seam was inert. Verified 170 offline / 184 with the live SQL suite / 1 skip / 0 warnings, including **15 live tests vs a real SQL Server 2022** (the SQLite suite ported case-for-case) and a **9-test in-process 2-node Akka cluster** (real remoting; write→peer, delete propagation, both anti-entropy directions, late-joiner catch-up). Hard constraints across all topologies: same KEK on every node, ciphertext-only across trust boundaries, and re-wraps deliberately do NOT replicate (`rewrap-all` runs once per independent store). Operator runbook `ZB.MOM.WW.Secrets/docs/operations/clustered-secrets.md`. **Corrected 2026-07-18:** G-8 + the G-7 plan **ARE committed and pushed** on `main` (`d82d345`) — the "not yet committed/pushed" note was stale. **`0.1.3` PUBLISHED to the feed 2026-07-18** (restore-verified), carrying a **transitive security pin** (`SQLitePCLRaw.lib.e_sqlite3` → `2.1.12`, advisory GHSA-2m69-gcr7-jv3q) that `0.1.2` lacked. **All 4 apps bumped to `0.2.1`** 2026-07-18 (local `feat/secrets-0.2.0` branches, NOT pushed). `0.2.1` fixes a critical defect: `0.2.0`'s Akka replicator was **silently inert** (TryAdd registration order — it published into a no-op sink and never spawned an actor). SQL-Server replication was never affected. Clustered topology is **wired but default-OFF and NOT live-validated** — OtOpcUa on Akka peer-to-peer, ScadaBridge on SQL-hub mode (its central and site clusters are separate Akka clusters, so pub/sub cannot cross them). Separately, a **live** SQLitePCLRaw vulnerability was found and fixed in ScadaBridge, where a NuGetAuditSuppress had been masking vulnerable 2.1.11 in 15 projects. **`0.2.0` PUBLISHED to the feed 2026-07-18** (5 packages, restore-verified from a scratch consumer; merged to `main` @ `15ef1f3`) — no app has adopted a clustered topology yet | Shared `ZB.MOM.WW.Secrets` lib (5 packages + CLI) | [`components/secrets/`](components/secrets/) | [`ZB.MOM.WW.Secrets/`](ZB.MOM.WW.Secrets/) | +| Secrets (encrypted store + `${secret:}` resolution) | Built (libs **`0.1.3`**, **published to the Gitea feed**; **HistorianGateway adopted + live-proven** 2026-07-16; **mxaccessgw adopted G-4/G-5/G-6 + merged to `origin/main` @ `e088dfa`** 2026-07-16, box-verified fail-closed + encrypt-at-rest; **ScadaBridge adopted G-3/G-4/G-5/G-6 + merged to `origin/main` @ `128f1596`** 2026-07-16, **G-3 live-proven vs the real production MxGateway gateway** `wonder-app-vd03:5120` (secret-ref ApiKey → Connected + browsed real Galaxy); **OtOpcUa adopted G-2/G-4/G-5/G-6 + merged to `origin/master` @ `872cf7e3`** (lmxopcua) 2026-07-16 — the last app, so **all four now adopted**; Layer-B driver secrets (Galaxy API key + OpcUaClient `Password`/`UserCertificatePassword`) resolve fail-closed at driver session-open, `AddZbSecrets` registered unconditionally so driver-only nodes work; **G-2 live-proven vs the real production MxGateway** `wonder-app-vd03:5120` — a `secret:`-ref Galaxy ApiKey resolved through OtOpcUa's real `GalaxyDriverBrowser` → dummy key `MxGatewayAuthenticationException`, real key → browsed the real Galaxy root (10 nodes); temp key minted+revoked+deleted); **G-8 KEK-rotation BUILT (lib bumped `0.1.2`→`0.1.3`, 2026-07-17):** `Rewrap` DEK primitive + CAS-guarded `ApplyRewrapAsync` + `KekRotationService.RewrapAllAsync` + `secret rewrap-all` CLI + operator runbook (`ZB.MOM.WW.Secrets/docs/operations/kek-rotation.md`), full suite green + CLI smoke + adversarial crypto review PASS (TOCTOU closed via compare-and-swap). **G-7 clustered replication BUILT 2026-07-18 (lib `0.2.0`) — BOTH fork options, packaged as two new libs:** **`ZB.MOM.WW.Secrets.Replicator.SqlServer`** (shared SQL-Server `ISecretStore` = the plan's Option A, *plus* a local-store-with-hub mode) and **`ZB.MOM.WW.Secrets.Replicator.AkkaDotNet`** (peer-to-peer over distributed pub/sub = Option B, which the plan had deferred). Core gained `ISecretsStoreMigrator`, `SecretLastWriterWins` (one shared LWW tie-break so stores cannot diverge), `SecretReplicationReconciler`, and `ReplicatingSecretStore` — the last of which fixes a real gap: **nothing had ever called `ISecretReplicator.PublishAsync`**, so the seam was inert. Verified 170 offline / 184 with the live SQL suite / 1 skip / 0 warnings, including **15 live tests vs a real SQL Server 2022** (the SQLite suite ported case-for-case) and a **9-test in-process 2-node Akka cluster** (real remoting; write→peer, delete propagation, both anti-entropy directions, late-joiner catch-up). Hard constraints across all topologies: same KEK on every node, ciphertext-only across trust boundaries, and re-wraps deliberately do NOT replicate (`rewrap-all` runs once per independent store). Operator runbook `ZB.MOM.WW.Secrets/docs/operations/clustered-secrets.md`. **Corrected 2026-07-18:** G-8 + the G-7 plan **ARE committed and pushed** on `main` (`d82d345`) — the "not yet committed/pushed" note was stale. **`0.1.3` PUBLISHED to the feed 2026-07-18** (restore-verified), carrying a **transitive security pin** (`SQLitePCLRaw.lib.e_sqlite3` → `2.1.12`, advisory GHSA-2m69-gcr7-jv3q) that `0.1.2` lacked. **All 4 apps bumped to `0.2.1`** 2026-07-18 (local `feat/secrets-0.2.0` branches, NOT pushed). `0.2.1` fixes a critical defect: `0.2.0`'s Akka replicator was **silently inert** (TryAdd registration order — it published into a no-op sink and never spawned an actor). SQL-Server replication was never affected. Clustered topology is **wired but default-OFF and NOT live-validated** — OtOpcUa on Akka peer-to-peer, ScadaBridge on SQL-hub mode (its central and site clusters are separate Akka clusters, so pub/sub cannot cross them). Separately, a **live** SQLitePCLRaw vulnerability was found and fixed in ScadaBridge, where a NuGetAuditSuppress had been masking vulnerable 2.1.11 in 15 projects. **`0.2.0` PUBLISHED to the feed 2026-07-18** (5 packages, restore-verified from a scratch consumer; merged to `main` @ `15ef1f3`) — no app has adopted a clustered topology yet. **`0.2.2` (2026-07-18, [scadaproj#1](https://gitea.dohertylan.com/dohertj2/scadaproj/issues/1) FIXED): the Akka replicator's hosted-process DI deadlock root-caused** — a circular singleton dependency hidden in factory lambdas (`ISecretStore` decorator → `ISecretReplicator` → actor provider → `ISecretCacheInvalidator` → `DefaultSecretResolver` → `ISecretStore`), which MS.DI's `StackGuard` turns into a silent cross-thread call-site-lock deadlock instead of a stack overflow; NOT `DistributedPubSub`/HOCON (both disproven — the actor was never constructed). Fixed by deferring the one cycle edge (`DeferredSecretCacheInvalidator`, resolves on first eviction); **Akka live gate now 6/6** (was: hang), regression coverage `HostedProcessResolutionTests` (container-built graph + hosted services; red on 0.2.1), suite 180/0/15. Fourth defect class only visible with the DI graph built in a real host. Not yet published to the feed | Shared `ZB.MOM.WW.Secrets` lib (5 packages + CLI) | [`components/secrets/`](components/secrets/) | [`ZB.MOM.WW.Secrets/`](ZB.MOM.WW.Secrets/) | | LocalDb (embedded cache + 2-node sync) | Built (3 pkgs `0.1.0`, **published to the Gitea feed** 2026-07-18, restore-verified from a scratch consumer; no app adoption yet) | Shared `ZB.MOM.WW.LocalDb` lib | [`docs/plans/2026-07-17-localdb-design.md`](docs/plans/2026-07-17-localdb-design.md) | [`ZB.MOM.WW.LocalDb/`](ZB.MOM.WW.LocalDb/) | The auth component is fully populated: a normalized [`spec`](components/auth/spec/SPEC.md), a diff --git a/ZB.MOM.WW.Secrets/Directory.Build.props b/ZB.MOM.WW.Secrets/Directory.Build.props index 0829136..6b68385 100644 --- a/ZB.MOM.WW.Secrets/Directory.Build.props +++ b/ZB.MOM.WW.Secrets/Directory.Build.props @@ -5,7 +5,7 @@ enable enable latest - 0.2.1 + 0.2.2 true README.md diff --git a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet/DependencyInjection/AkkaSecretsServiceCollectionExtensions.cs b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet/DependencyInjection/AkkaSecretsServiceCollectionExtensions.cs index 1fd0523..e3aa26f 100644 --- a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet/DependencyInjection/AkkaSecretsServiceCollectionExtensions.cs +++ b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet/DependencyInjection/AkkaSecretsServiceCollectionExtensions.cs @@ -69,10 +69,17 @@ public static class AkkaSecretsServiceCollectionExtensions // The actor is created lazily on first use rather than at registration: the ActorSystem is // typically registered by the application AFTER this call, and a cluster node may also not be // ready to join at the moment the container is built. + // + // The invalidator MUST stay deferred (scadaproj#1): it is the resolver, the resolver reads + // ISecretStore, and ISecretStore is the ReplicatingSecretStore decorator registered below — + // resolving it here closes a singleton cycle the container cannot detect through factory + // lambdas, and every hosted process then deadlocks at startup inside the container's + // resolution locks. DeferredSecretCacheInvalidator waits until the first eviction, which + // can only happen after the graph has finished resolving. services.TryAddSingleton(sp => new SecretReplicationActorProvider( sp.GetRequiredService(), sp.GetRequiredService(), - sp.GetService(), + new DeferredSecretCacheInvalidator(sp.GetService), sp.GetRequiredService>().Value)); // MUST be registered BEFORE AddZbSecrets. That call does diff --git a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet/DependencyInjection/DeferredSecretCacheInvalidator.cs b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet/DependencyInjection/DeferredSecretCacheInvalidator.cs new file mode 100644 index 0000000..40a717d --- /dev/null +++ b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet/DependencyInjection/DeferredSecretCacheInvalidator.cs @@ -0,0 +1,39 @@ +using ZB.MOM.WW.Secrets.Abstractions; + +namespace ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.DependencyInjection; + +/// +/// Resolves the real on first eviction instead of at +/// construction, so the replication graph can be wired without closing a dependency cycle. +/// +/// +/// +/// This deferral is load-bearing, not an optimization (scadaproj#1). The invalidator is the +/// resolver, the resolver reads through ISecretStore, and with replication registered +/// ISecretStore is the ReplicatingSecretStore decorator whose replicator owns this +/// node's actor — so resolving the invalidator eagerly inside the actor provider's factory closes +/// a singleton cycle the container cannot see through factory lambdas. Resolution then recurses +/// until MS.DI's StackGuard hops it onto another thread, which deadlocks on a call-site +/// lock the first thread holds: any host resolving the graph hangs at startup, silently and +/// permanently. +/// +/// +/// Deferring to first eviction is sufficient because evictions only happen when a replicated row +/// is applied — strictly after the graph has finished resolving, at which point every singleton on +/// the former cycle is already materialized and the lookup completes without re-entering the +/// container's resolution locks. +/// +/// +/// +/// Lookup for the real invalidator; returns null when the application registered none. +/// +internal sealed class DeferredSecretCacheInvalidator(Func resolve) + : ISecretCacheInvalidator +{ + private readonly Lazy _inner = new( + resolve ?? throw new ArgumentNullException(nameof(resolve)), + LazyThreadSafetyMode.ExecutionAndPublication); + + /// + public void Invalidate(SecretName name) => _inner.Value?.Invalidate(name); +} diff --git a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests/DependencyInjection/AddZbSecretsAkkaReplicationTests.cs b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests/DependencyInjection/AddZbSecretsAkkaReplicationTests.cs index 39151af..dbd812d 100644 --- a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests/DependencyInjection/AddZbSecretsAkkaReplicationTests.cs +++ b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests/DependencyInjection/AddZbSecretsAkkaReplicationTests.cs @@ -25,12 +25,14 @@ namespace ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests.DependencyInjection; /// the other, so the gap is closed here. /// /// -/// These assert against the rather than a built provider on -/// purpose. Resolving ISecretReplicator eagerly spawns the replication actor, whose -/// PreStart calls DistributedPubSub.Get(...); that requires a joined cluster, so a -/// provider-based test would need a real single-node cluster and would hang without one. The -/// defect was entirely about which descriptor wins, and the descriptor is where it is visible. -/// End-to-end actor behaviour is already covered by TwoNodeClusterReplicationTests. +/// These assert against the because the 0.2.0 defect was entirely +/// about which descriptor wins, and the descriptor is where it is visible. Resolution through a +/// BUILT provider — where the scadaproj#1 singleton-cycle deadlock lived — is covered by +/// , and end-to-end actor behaviour by +/// TwoNodeClusterReplicationTests. (An earlier revision of this comment claimed a +/// provider-based test was impossible because DistributedPubSub.Get needs a joined +/// cluster; the hang it was avoiding was actually that DI deadlock, and a single self-joined +/// node is all the cluster the actor needs.) /// /// public sealed class AddZbSecretsAkkaReplicationTests diff --git a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests/DependencyInjection/DeferredSecretCacheInvalidatorTests.cs b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests/DependencyInjection/DeferredSecretCacheInvalidatorTests.cs new file mode 100644 index 0000000..b30e70d --- /dev/null +++ b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests/DependencyInjection/DeferredSecretCacheInvalidatorTests.cs @@ -0,0 +1,75 @@ +using ZB.MOM.WW.Secrets.Abstractions; +using ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.DependencyInjection; + +namespace ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests.DependencyInjection; + +/// +/// The deferred invalidator exists to break the scadaproj#1 dependency cycle, which makes it a +/// pass-through that could silently pass nothing through — the exact failure mode (inert seam, +/// green suite) this library has shipped before. These tests pin the two halves of its contract: +/// nothing resolves before the first eviction, and evictions actually reach the real invalidator. +/// +public sealed class DeferredSecretCacheInvalidatorTests +{ + private sealed class RecordingInvalidator : ISecretCacheInvalidator + { + public List Evicted { get; } = []; + + public void Invalidate(SecretName name) => Evicted.Add(name); + } + + [Fact] + public void Construction_does_not_resolve_the_inner_invalidator() + { + // THE point of the type: resolving eagerly is what closed the singleton cycle and hung + // every hosted process at startup. + bool resolved = false; + + _ = new DeferredSecretCacheInvalidator(() => + { + resolved = true; + return null; + }); + + Assert.False(resolved); + } + + [Fact] + public void Invalidate_forwards_to_the_resolved_invalidator() + { + var inner = new RecordingInvalidator(); + var deferred = new DeferredSecretCacheInvalidator(() => inner); + var name = new SecretName("gate/alpha"); + + deferred.Invalidate(name); + + Assert.Equal([name], inner.Evicted); + } + + [Fact] + public void The_lookup_runs_once_and_the_instance_is_reused() + { + int lookups = 0; + var inner = new RecordingInvalidator(); + var deferred = new DeferredSecretCacheInvalidator(() => + { + lookups++; + return inner; + }); + + deferred.Invalidate(new SecretName("gate/alpha")); + deferred.Invalidate(new SecretName("gate/beta")); + + Assert.Equal(1, lookups); + Assert.Equal(2, inner.Evicted.Count); + } + + [Fact] + public void A_null_resolution_means_evictions_are_no_ops() + { + // Mirrors the reconciler's own contract: no invalidator registered, nothing to evict. + var deferred = new DeferredSecretCacheInvalidator(() => null); + + deferred.Invalidate(new SecretName("gate/alpha")); + } +} diff --git a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests/DependencyInjection/HostedProcessResolutionTests.cs b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests/DependencyInjection/HostedProcessResolutionTests.cs new file mode 100644 index 0000000..cd1b17b --- /dev/null +++ b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests/DependencyInjection/HostedProcessResolutionTests.cs @@ -0,0 +1,153 @@ +using Akka.Actor; +using Akka.Cluster; +using Akka.Configuration; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using ZB.MOM.WW.Secrets.Abstractions; +using ZB.MOM.WW.Secrets.Replication; +using ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.DependencyInjection; + +namespace ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests.DependencyInjection; + +/// +/// Builds the replication graph the way a real application host does — container-registered +/// , hosted services started, services resolved from the built provider — +/// and asserts resolution actually completes. +/// +/// +/// +/// This is the regression test for scadaproj#1. The 0.2.x graph had a circular singleton +/// dependency reachable only through the container: ISecretStore (the +/// ReplicatingSecretStore decorator) → ISecretReplicator → +/// SecretReplicationActorProvider → ISecretCacheInvalidator → +/// DefaultSecretResolver → ISecretStore. Every edge is an opaque factory lambda, so +/// the container cannot detect the cycle statically; at runtime the resolution recurses until +/// MS.DI's StackGuard moves it onto a fresh thread-pool thread, which then blocks forever on +/// a singleton call-site lock the first thread still holds. The symptom was a silent, permanent +/// hang — not a stack overflow, not an exception — which is why every direct-construction test +/// passed while any hosted process froze at startup. +/// +/// +/// The suite's other cluster tests construct the actor with system.ActorOf directly and can +/// never see this defect. Nothing here may resolve the affected services before the assertions do. +/// +/// +public sealed class HostedProcessResolutionTests : IAsyncLifetime +{ + // Generous next to the milliseconds a healthy resolve takes, tiny next to CI noise: on + // regression the resolve NEVER completes, so the only cost of margin is this test's runtime. + private static readonly TimeSpan ResolveBudget = TimeSpan.FromSeconds(20); + + private readonly string _dbPath = + Path.Combine(Path.GetTempPath(), $"zb-hosted-di-{Guid.NewGuid():N}.db"); + + private ActorSystem _system = null!; + private ServiceProvider _provider = null!; + + public async Task InitializeAsync() + { + // A single self-joined cluster node: `actor.provider = cluster` is what the replication + // actor's DistributedPubSub mediator needs, and joining makes the rig an honest miniature + // of an app node rather than a special case. + _system = ActorSystem.Create("zb-secrets-cluster", ConfigurationFactory.ParseString(""" + akka { + loglevel = WARNING + actor.provider = cluster + remote.dot-netty.tcp { + hostname = "127.0.0.1" + public-hostname = "127.0.0.1" + port = 0 + } + } + """).WithFallback(AkkaSecretsReplication.SerializationConfig)); + + Cluster cluster = Cluster.Get(_system); + cluster.Join(cluster.SelfAddress); + + DateTime deadline = DateTime.UtcNow + TimeSpan.FromSeconds(20); + while (cluster.State.Members.All(m => m.Status != MemberStatus.Up)) + { + if (DateTime.UtcNow > deadline) + { + throw new TimeoutException("Single-node cluster never reached Up."); + } + + await Task.Delay(100); + } + + IConfigurationRoot config = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["Secrets:SqlitePath"] = _dbPath, + ["Secrets:RunMigrationsOnStartup"] = "true", + ["Secrets:Replication:AnnounceInterval"] = "00:00:30", + }) + .Build(); + + var services = new ServiceCollection(); + services.AddLogging(); + services.AddSingleton(_system); + services.AddZbSecretsAkkaReplication(config, "Secrets", "Secrets:Replication"); + _provider = services.BuildServiceProvider(); + + // Started exactly as a host would, BEFORE anything resolves the replicated store — the + // order OtOpcUa's startup follows, and the order under which the deadlock was reproduced. + foreach (IHostedService hosted in _provider.GetServices()) + { + await hosted.StartAsync(CancellationToken.None); + } + } + + [Fact] + public async Task Resolving_the_replicated_graph_through_the_container_completes() + { + // One test covers all three entry points IN this order: the store is the edge OtOpcUa's + // startup hook actually resolves, the replicator is the narrowest reproduction of the + // hang, and the resolver closes the loop from the invalidator's side. Split into separate + // facts, each would rebuild the rig only to hit the same three singletons. + ISecretStore store = await ResolveWithinBudgetAsync( + sp => sp.GetRequiredService(), nameof(ISecretStore)); + ISecretReplicator replicator = await ResolveWithinBudgetAsync( + sp => sp.GetRequiredService(), nameof(ISecretReplicator)); + ISecretResolver resolver = await ResolveWithinBudgetAsync( + sp => sp.GetRequiredService(), nameof(ISecretResolver)); + + Assert.IsType(store); + Assert.IsType(replicator); + Assert.NotNull(resolver); + } + + private async Task ResolveWithinBudgetAsync( + Func resolve, string label) + { + // The failure mode under test is an infinite wait inside the container, so the resolve + // runs on the pool with a watchdog. On regression the worker thread is unrecoverable — + // acceptable in a test process that is about to exit, and vastly better than hanging CI. + Task resolution = Task.Run(() => resolve(_provider)); + + Task first = await Task.WhenAny(resolution, Task.Delay(ResolveBudget)); + + Assert.True( + first == resolution, + $"Resolving {label} did not complete within {ResolveBudget.TotalSeconds:0}s — " + + "the DI singleton cycle deadlock (scadaproj#1) has regressed."); + + return await resolution; + } + + public async Task DisposeAsync() + { + await _provider.DisposeAsync(); + await _system.Terminate(); + + Microsoft.Data.Sqlite.SqliteConnection.ClearAllPools(); + foreach (string path in new[] { _dbPath, _dbPath + "-wal", _dbPath + "-shm" }) + { + if (File.Exists(path)) + { + File.Delete(path); + } + } + } +} diff --git a/components/secrets/GAPS.md b/components/secrets/GAPS.md index f710e40..b4ef33a 100644 --- a/components/secrets/GAPS.md +++ b/components/secrets/GAPS.md @@ -105,28 +105,42 @@ The KEK control fired spontaneously first: a re-run against a hub still holding run (different KEK) threw `SecretDecryptionException` rather than returning garbage — unplanned but real evidence of fail-closed. -### Live gate: Akka peer-to-peer (OtOpcUa topology) — ❌ FAILED, blocking defect +### Live gate: Akka peer-to-peer (OtOpcUa topology) — ✅ PASSES 6/6 (root-caused + fixed in `0.2.2`, 2026-07-18) -**Resolving `ISecretReplicator` hangs indefinitely** in a real clustered process. Reproduced twice -on a healthy 2-node cluster (both members confirmed `Up` before the hang). Isolated by resolving -`ISecretReplicator` *directly* rather than via `ISecretStore`, so it is **not** nested-resolution -contention — the hang is in constructing `AkkaSecretReplicator` → `SecretReplicationActorProvider.ActorRef` -→ `system.ActorOf(SecretReplicationActor.Props(...))`. Native stacks show `Monitor_Wait`. +The `0.2.1` hang is **root-caused and fixed**. It was never Akka at all: the Akka package's DI +wiring had a **circular singleton dependency** the container cannot detect through factory lambdas — +`ISecretStore` (the `ReplicatingSecretStore` decorator) → `ISecretReplicator` → +`SecretReplicationActorProvider` → `ISecretCacheInvalidator` → `DefaultSecretResolver` → +`ISecretStore` again. Resolution recurses around that loop (same-thread `Monitor` re-entry keeps it +alive) until MS.DI's `StackGuard` moves the recursion onto a fresh thread-pool thread to avoid a +stack overflow — and that thread then blocks forever on a singleton call-site lock the first thread +still holds. Managed stacks from a `dotnet-dump` of the hung process show the full cycle repeating +and both threads parked, which is the `Monitor_Wait` the native samples saw. Both earlier candidate +causes (`DistributedPubSub.Get` vs the `Lazy` lock; missing `Akka.Cluster.Tools` HOCON) +are **disproven** — the actor's constructor was never even reached, and the deadlock reproduces on a +single-node rig with no clustering in play. -**Impact if enabled:** OtOpcUa would hang at startup, since the wiring resolves `ISecretStore` from a -startup hook. It is currently harmless only because `Secrets:Replication:Enabled` defaults false. +**Fix (`0.2.2`):** the one cycle-closing edge is deferred. `SecretReplicationActorProvider` now +receives a `DeferredSecretCacheInvalidator` that resolves the real invalidator on **first +eviction** — which can only happen when a replicated row is applied, strictly after the graph has +finished resolving. The SQL-Server package never had the cycle (its replicator chain never touches +the invalidator), which is exactly why the hub gate passed while Akka hung. -**Not explained by the library's own suite:** `TwoNodeClusterReplicationTests` creates the same actor -on a real 2-node cluster and passes (33 tests green). The difference is creation through DI inside a -real host process. Root cause NOT yet identified — candidates: `DistributedPubSub.Get(...)` in -`PreStart` interacting with the `Lazy` (`ExecutionAndPublication`) lock, or missing -`Akka.Cluster.Tools` reference config in the composed HOCON. +**Verified:** live convergence gate re-run — **6/6 checks** on a real 2-node cluster (write→peer, +delete→tombstone, tombstoned secret no longer resolves on the peer — the check that proves the +deferred invalidator actually evicts — reverse direction, wrong-KEK fail-closed). Full Akka test +project 38/38 green. -- ⬜ **Do NOT enable `Secrets:Replication:Enabled` in OtOpcUa.** The Akka topology is not adoptable - until this deadlock is root-caused and fixed, with a regression test that creates the actor - through DI in a hosted process rather than directly from a test. - **Tracked:** [`scadaproj#1`](https://gitea.dohertylan.com/dohertj2/scadaproj/issues/1) (the defect, - where the library code lives) and [`lmxopcua#482`](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/482) +- ✅ **Regression test now exists at the exact blind spot:** `HostedProcessResolutionTests` builds + the graph the way a host does (container-registered `ActorSystem`, hosted services started, then + resolves `ISecretStore`/`ISecretReplicator`/`ISecretResolver` under a 20 s watchdog). Verified to + discriminate: it deadlocks/fails on `0.2.1` and passes on `0.2.2`. `DeferredSecretCacheInvalidatorTests` + pins the wrapper's contract (no eager resolve; evictions forward; lookup runs once). This was the + **fourth** defect in this library visible only when the DI graph is built inside a real host. +- ⬜ OtOpcUa may now adopt the Akka topology: bump to `0.2.2` (once published), re-run its gate, + then consider enabling `Secrets:Replication:Enabled`. + **Tracked:** [`scadaproj#1`](https://gitea.dohertylan.com/dohertj2/scadaproj/issues/1) (the defect — + fixed) and [`lmxopcua#482`](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/482) (the consumer-side tracker: bump the package, re-run the gate, then consider enabling). - ✅ ScadaBridge's hub topology **is** live-validated and safe to enable (config + a shared KEK).