Files
lmxopcua/docs/reviews/2026-10-21/03-host-controlplane-security-config.md
T

28 KiB
Raw Blame History

Review 03 — Host / ControlPlane / Security / Configuration

Repo: /Users/dohertj2/Desktop/OtOpcUa @ d51d35bd (master). Read-only audit of production code in src/Server/ZB.MOM.WW.OtOpcUa.Host, src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane, src/Server/ZB.MOM.WW.OtOpcUa.Security, src/Core/ZB.MOM.WW.OtOpcUa.Configuration. Every finding below was verified by reading the cited code. Line numbers are from the files as checked in.

Severity: Critical / High / Medium / Low. Confidence: High / Medium.


1. Memory / resource leaks

1.1 Runaway virtual-tag script leaks one spinning thread-pool thread per evaluation, with no breaker

  • Severity: High Confidence: High
  • Location: src/Server/ZB.MOM.WW.OtOpcUa.Host/Engines/RoslynVirtualTagEvaluator.cs:155-160; src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/TimedScriptEvaluator.cs:17-25; src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagHostActor.cs (no timeout bookkeeping — only line 224 mentions timeouts).
  • Evidence:
    var timed = new TimedScriptEvaluator<VirtualTagContext, object?>(evaluator, _runTimeout);
    var raw = timed.RunAsync(context).GetAwaiter().GetResult();   // RoslynVirtualTagEvaluator.cs:158-159
    
    TimedScriptEvaluator remarks: "when a script times out, the underlying ScriptRunner task continues running on a thread-pool thread … In the CPU-bound-infinite-loop case that's effectively 'leaked' … the orphan thread is rare."
  • Failure scenario: The "rare" claim only holds per evaluation. A change-triggered or timer-triggered virtual tag whose script spins (e.g. while(true), or a ctx.GetTag loop that never converges) is re-evaluated on every upstream change; each evaluation times out after 2 s and leaves a thread burning 100 % CPU forever. VirtualTagHostActor does nothing on a Failure("timed out…") result except publish Bad quality — no quarantine, no back-off, no consecutive-timeout cap. A 1 Hz input produces ~1 orphaned core per second; the node is CPU-starved (Akka dispatchers, OPC UA publish thread, SQL) within minutes and only a process restart recovers it. A Designer-role user can author this through the AdminUI.
  • Suggested fix: Track consecutive timeouts per virtualTagId in VirtualTagHostActor/evaluator and quarantine the tag (stop evaluating, publish Bad, log Error, surface on /uns) after N (e.g. 3) timeouts until the script is edited. Longer term: cooperative CancellationToken polling injected into compiled scripts, or an out-of-process runner.

1.2 Deployment / NodeDeploymentState / ConfigEdit / ConfigAuditLog grow without bound — every deploy stores a full artifact blob forever

  • Severity: Medium Confidence: High
  • Location: src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/AdminOperationsActor.cs:338-357; src/Core/ZB.MOM.WW.OtOpcUa.Configuration/OtOpcUaConfigDbContext.cs:720 (ArtifactBlob varbinary(max)); no Deployments.Remove/ExecuteDelete/retention anywhere in src (grep verified).
  • Evidence:
    db.Deployments.Add(new Deployment { …, ArtifactBlob = artifact.Blob });   // AdminOperationsActor.cs:338-345
    
  • Failure scenario: Each StartDeployment persists the gzip'd snapshot (tens of KB to MBs on a 50k-tag fleet) plus a ConfigEdit marker and one NodeDeploymentState row per node. Nothing prunes any of them. Over a year of routine edits the Deployment table alone is GBs of blobs; Deployments.razor projects the blob away (fine), but ClusterOverview.razor:135 and ConfigPublishCoordinator.PreStart (ConfigPublishCoordinator.cs:164-167) each materialise the full newest row including the blob. The IX_Deployment_Status index keeps the in-flight query cheap, so this is capacity/backup growth rather than an immediate hot-path cost.
  • Suggested fix: Retention job on the admin singleton (keep last N sealed + anything referenced by a NodeDeploymentState newer than X days; null out ArtifactBlob on older rows rather than deleting, so the audit row survives). Project ArtifactBlob away in the two full-row reads.

1.3 MeshGrpcCommandDialer channel cache never evicts a stale endpoint

  • Severity: Low Confidence: High
  • Location: src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Communication/MeshGrpcCommandDialer.cs:93, 194-199.
  • Evidence: _channels.GetOrAdd(endpoint, GrpcChannel.ForAddress) keyed by the http://{Host}:{GrpcPort} string; only Dispose() removes entries.
  • Failure scenario: Every ClusterNode.Host/GrpcPort edit (host rename, port move, IP→DNS migration) leaves the old GrpcChannel — and its idle HTTP/2 connection/socket — alive for the life of central. Bounded by the number of distinct endpoints ever configured, so it is a slow leak, not a flood.
  • Suggested fix: On ContactsLoaded, dispose channels whose endpoint is no longer in the target set (the actor already knows the set), or use SocketsHttpHandler.PooledConnectionIdleTimeout.

1.4 OtOpcUaServerHostedService.StopAsync detaches every sink but leaves the SDK listening until host dispose

  • Severity: Low Confidence: High
  • Location: src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/OtOpcUaServerHostedService.cs:448-476 (StopAsync) vs 482-490 (DisposeAsync → _appHost.DisposeAsync() → ApplicationInstance.StopAsync()).
  • Evidence: StopAsync nulls the write gateway, historian, authority and authorizer and returns Task.CompletedTask; the only ApplicationInstance.StopAsync() is in DisposeAsync.
  • Failure scenario: Between IHostedService.StopAsync and host disposal (Akka coordinated shutdown, other hosted services' StopAsync) the OPC UA endpoint still accepts and activates sessions and serves the last-written address-space values with the authority source set to empty — reads/subscribes succeed on stale data, writes get "unavailable". Clients see a server that is up but inert instead of a clean BadServerHalted/connection refusal that would trigger their redundancy failover sooner.
  • Suggested fix: Call _appHost.DisposeAsync() (or an explicit StopAsync) from StopAsync after the detaches, making DisposeAsync idempotent.

1.5 CookieAuthenticationStateProvider — dead type with a fire-and-forget ping loop

  • Severity: Low Confidence: High
  • Location: src/Server/ZB.MOM.WW.OtOpcUa.Security/Blazor/CookieAuthenticationStateProvider.cs:40.
  • Evidence: _pingLoop ??= Task.Run(() => PingLoopAsync(_cts.Token)); — and no registration or reference to the type anywhere else in src (grep: only its own file).
  • Failure scenario: Not reachable today, so no live leak. If someone registers it as the circuit AuthenticationStateProvider, the injected HttpClient has no BaseAddress server-side (the repo's own memory note "Blazor Server self-HubConnection" documents this trap), GetAsync("/auth/ping") throws, the loop logs once and dies, and the circuit never learns its cookie expired.
  • Suggested fix: Delete it (see 2.5), or wire it properly with a BaseAddress and register it.

2. Unimplemented / stubbed / dead features

2.1 A failed OPC UA SDK start is swallowed; no health check reflects it; the node stays "Ready", stays Primary-eligible, and acks deploys as Applied

  • Severity: High Confidence: High (swallow + no health probe) / Medium (clean-ack claim)
  • Location: src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/OtOpcUaServerHostedService.cs:163-182; src/Server/ZB.MOM.WW.OtOpcUa.Host/Health/HealthEndpoints.cs:34-96 (checks: configdb, akka, cluster-primary, localdb-replication — nothing for the OPC UA endpoint; the only custom IHealthCheck in Host is LocalDbReplicationHealthCheck); src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs:5-8 (falls back to NullOpcUaAddressSpaceSink).
  • Evidence:
    catch (Exception ex)
    {
        _logger.LogError(ex, "OtOpcUaServerHostedService: SDK start failed; OpcUaPublishActor writes will continue to no-op");
        // Don't rethrow — the rest of the host (admin UI, driver actors, etc.) can still boot.
        return;
    }
    
  • Failure scenario: Port 4840 already bound, PKI directory unwritable, cert validation failure, or a bad OpcUa:* override on a driver-only site node: the process logs one Error and then reports /health/ready 200, joins the Akka mesh, is elected driver Primary by age, publishes ServiceLevel through the still-Null publisher (so the peer is not promoted), and every deploy applies through the Null sink and acks Applied (the Null sink's surgical methods return false → SafeRebuild() → Null RebuildAddressSpace() no-op — the ack path was not traced further, hence Medium on that clause). Clients get connection-refused from the node the fleet says is primary and healthy. The comment's rationale ("admin UI can still boot") does not apply to a driver-only node, which has no AdminUI.
  • Suggested fix: Add an IHealthCheck (tags Ready+Active) that reports Unhealthy while _server?.NodeManager is null after StartAsync ran; on a driver-only node (no admin role) rethrow so Program.cs's fatal handler exits non-zero and the supervisor restarts; feed the "SDK not started" fact into the ServiceLevel calculator so the peer takes Primary.

2.2 Deploy pipeline can wedge permanently: a Deployment stuck in Dispatching with no NodeDeploymentState rows blocks every future deploy

  • Severity: High Confidence: High
  • Location: src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Coordinators/ConfigPublishCoordinator.cs:163-200 (PreStart recovery), 202-248 (HandleDispatch), 355-382 (HandleDeadline); src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/AdminOperationsActor.cs:193-206, 338-359.
  • Evidence:
    // PreStart: recovers Dispatching OR AwaitingApplyAcks rows
    var inflight = db.Deployments.Where(d => d.Status == DeploymentStatus.Dispatching || d.Status == DeploymentStatus.AwaitingApplyAcks)…
    _expectedAcks = nodeStates.Select(…).ToHashSet(NodeIdComparer);   // EMPTY when HandleDispatch never ran
    …
    // HandleDeadline:
    if (_acks.Count == _expectedAcks.Count) { /* Race … Already sealed/failed elsewhere. */ return; }   // 0 == 0
    
    AdminOperationsActor writes Status = Dispatching and SaveChangesAsync() before _coordinator.Tell(...) (lines 338-359) and refuses any new deploy while a Dispatching/AwaitingApplyAcks row exists (193-206).
  • Failure scenario: Any of: (a) HandleDispatch throws before its SaveChanges (transient SQL fault seeding NodeDeploymentState) → actor restarts, message dropped; (b) the process dies between the AdminOps SaveChangesAsync and the coordinator handling the Tell; (c) the singleton proxy's buffered DispatchDeployment is lost during a singleton hand-over. In all three the row stays Dispatching with zero NodeDeploymentState rows. PreStart recovers it with an empty expected set, the deadline fires, HandleDeadline hits the 0 == 0 "race" branch and returns without updating the row or clearing _current. From then on every StartDeployment is rejected with AnotherDeploymentInFlight until someone hand-edits SQL. No log line names the condition.
  • Suggested fix: In PreStart, treat a recovered deployment with an empty expected set as a failed dispatch: re-run DiscoverDriverNodes + seed rows + re-dispatch, or mark it TimedOut/Failed and ResetForNext(). In HandleDeadline, only take the "race" branch when _expectedAcks.Count > 0. Consider moving the Dispatching row write into the coordinator so the row and the expected set are always created together.

2.3 Security:ApiKeys:Pepper is never validated at startup — an unset pepper silently rejects every key

  • Severity: Medium Confidence: High
  • Location: src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs:164-168 (plain Configure<ApiKeyOptions>, no validator); src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/ApiKeyOpcUaUserAuthenticator.cs:114-127; src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/ApiKeyOptions.cs (Pepper = string.Empty default).
  • Evidence: Program.cs comment: "getting this wrong on either side is a silent authentication failure rather than a startup error" — acknowledged but not closed. The authenticator discovers it only when ApiKeySecret.Verify throws ArgumentException on the first activation, and then only logs.
  • Failure scenario: A new site node deployed without Security__ApiKeys__Pepper (or with a different pepper from central) boots green, passes health, applies deployments, and denies every machine client with the same opaque "Invalid API key." until someone reads the log. On a fused central node a wrong pepper also makes the AdminUI mint keys that can never verify anywhere.
  • Suggested fix: AddValidatedOptions<ApiKeyOptions, ApiKeyOptionsValidator> requiring a non-blank pepper (minimum length) whenever hasDriver || hasAdmin; every other security section in this file already uses ValidateOnStart.

2.4 JWT issuance is a dead end: /auth/token mints tokens nothing validates, and the signing-key check fires only on first use

  • Severity: Low Confidence: High
  • Location: src/Server/ZB.MOM.WW.OtOpcUa.Security/Endpoints/AuthEndpoints.cs:39 (/auth/token); src/Server/ZB.MOM.WW.OtOpcUa.Security/Jwt/JwtTokenService.cs:66 (ctor throws), 106, 139 (TryValidate/BuildValidationParameters — no callers outside the file; no AddJwtBearer anywhere).
  • Evidence: grep over src for TryValidate|BuildValidationParameters|JwtTokenService finds only the registration, IssueToken, and the type itself.
  • Failure scenario: The endpoint is an authenticated-only surface that hands out HS256 bearer tokens no endpoint in the product accepts. With Security:Jwt:SigningKey absent (the Host appsettings*.json carry none; only docker-dev sets it) the singleton's constructor throws InvalidOperationException on the first /auth/token call → HTTP 500 instead of a startup failure.
  • Suggested fix: Either delete /auth/token + JwtTokenService, or finish the feature (AddJwtBearer using BuildValidationParameters()) and validate JwtOptions with ValidateOnStart.

2.5 Registered-but-never-resolved: IAuditActorAccessor; never-registered: CookieAuthenticationStateProvider

  • Severity: Low Confidence: High
  • Location: src/Server/ZB.MOM.WW.OtOpcUa.Security/ServiceCollectionExtensions.cs:56 (TryAddScoped<IAuditActorAccessor, HttpAuditActorAccessor>()); src/Server/ZB.MOM.WW.OtOpcUa.Security/Audit/AuditActor.cs; src/Server/ZB.MOM.WW.OtOpcUa.Security/Blazor/CookieAuthenticationStateProvider.cs.
  • Evidence: grep over src (cs + razor) for IAuditActorAccessor|AuditActor.Resolve → only the registration and the two Security files; CookieAuthenticationStateProvider → only its own file.
  • Failure scenario: Dead surface that reads as if audit rows are attributed to the HTTP principal. Whoever eventually wires HttpAuditActorAccessor into a Blazor circuit will get HttpContext == null after the first render and every audit row stamped "system".
  • Suggested fix: Delete both, or resolve the actor from AuthenticationStateProvider in the AdminUI and actually consume IAuditActorAccessor where AuditEvent.Actor is built.

2.6 AuditWriterActor documents per-row dedup on the DB unique index, but one constraint hit drops the whole 500-event batch

  • Severity: Medium Confidence: High
  • Location: src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Audit/AuditWriterActor.cs:17-21 (claim) vs 86-117 (behaviour).
  • Evidence:
    // "a retry of an already-flushed batch hits the constraint and we drop the duplicate insert
    //  without losing the rest of the batch"            (lines 19-21)
    …
    using var db = _dbFactory.CreateDbContext();
    foreach (var evt in snapshot) db.ConfigAuditLogs.Add(…);
    db.SaveChanges();                                   // one statement batch
    …
    catch (Exception ex) { _log.Error(ex, "AuditWriter flush failed; {Count} events dropped", snapshot.Count); }
    
  • Failure scenario: A single duplicate EventId (any at-least-once producer — the audit seam is Tell-based through a singleton proxy that re-delivers across hand-over) makes SaveChanges throw DbUpdateException for the whole batch; up to 499 unrelated audit events are discarded and the Error line blames "flush failed". Audit is best-effort by contract, but the contract the comment states is not the one implemented.
  • Suggested fix: Catch DbUpdateException with SQL error 2601/2627, remove the offending entry from the change tracker and retry once; or pre-filter with WHERE EventId IN (...); or fix the comment.

2.7 AuthEndpoints.LoginAsync swallows the LDAP exception with no log

  • Severity: Low Confidence: High
  • Location: src/Server/ZB.MOM.WW.OtOpcUa.Security/Endpoints/AuthEndpoints.cs:72-79.
  • Evidence: catch (Exception) { return Results.StatusCode(503); } — no logger call.
  • Failure scenario: During a directory outage every login returns 503 and the server log contains nothing from this path; the operator has to infer the cause from the GLAuth/AD side.
  • Suggested fix: Log at Warning with the exception (the role-mapper branch a few lines down already does).

3. Performance limiters

3.1 The admin singleton's mailbox is suspended for the whole of every ReceiveAsync — a 60 s Test-Connect or a large deploy blocks AdminUI alarm acks, restarts and the next deploy

  • Severity: Medium Confidence: High
  • Location: src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/AdminOperationsActor.cs:77-82, 387-432.
  • Evidence:
    ReceiveAsync<StartDeployment>(HandleStartDeploymentAsync);
    ReceiveAsync<TestDriverConnect>(HandleTestDriverConnectAsync);   // awaits probe.ProbeAsync up to Math.Clamp(timeout,1,60) s
    Receive<AcknowledgeAlarmCommand>(HandleAcknowledgeAlarm);
    
    Akka.NET ReceiveAsync suspends the mailbox until the returned Task completes.
  • Failure scenario: One operator pressing Test Connect against an unreachable PLC (TCP connect black-hole, default 60 s clamp) stalls every other admin operation fleet-wide — including AcknowledgeAlarmCommand/ShelveAlarmCommand from the AdminUI alarm page — for up to a minute. HandleStartDeploymentAsync on a 50k-tag DB (two full snapshots + validation + gzip) has the same effect for seconds.
  • Suggested fix: Run the probe on a child/Task.Run + PipeTo (capture Sender), or move TestDriverConnect out of the singleton entirely (it needs no DB and no coordination). Same for the deploy compose: do the DB work off-actor and only serialize the "is one in flight" check.

3.2 StartDeployment reads the entire config DB twice with no transaction — validation and the composed artifact can diverge

  • Severity: Medium Confidence: High
  • Location: AdminOperationsActor.cs:218 (DraftSnapshotFactory.FromConfigDbAsync — 15 tables), :231-239 (clusters + nodes again), :313 (scripts again), :334 (ConfigComposer.SnapshotAndFlattenAsync — 17 tables); src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/ConfigComposer.cs:39-64; src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftSnapshotFactory.cs:7-37.
  • Evidence: Both loaders are sequential AsNoTracking().ToListAsync() per table on the same context with no BeginTransaction(IsolationLevel.Snapshot); the artifact is composed from a second read taken after validation returned.
  • Failure scenario: (a) Cost: ~32 sequential round trips and double materialisation of every row per deploy — on the admin singleton's thread (see 3.1). (b) Correctness: a /uns or /raw save that lands between the validator read and the composer read ships an artifact the DraftValidator never saw (e.g. an effective-name collision or a cross-cluster ACL that the gate exists to stop).
  • Suggested fix: Load once (DraftSnapshot already holds nearly every table the composer needs — add ServerClusters/ClusterNodes/ApiKeys to it and compose from the snapshot), inside a SNAPSHOT/SERIALIZABLE read transaction. Project Script.SourceCode once instead of three times.

3.3 Pre-authentication SQL round trip per distinct unknown key id, serialized under the SDK SessionManager lock

  • Severity: Medium Confidence: High
  • Location: src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/BoundedApiKeyStore.cs:46-50, 104-122, 180-184; src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/ConfigDbApiKeyStore.cs:101-119; src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OpcUaApplicationHost.cs:184-190 (sync bridge under the SDK lock).
  • Evidence: The negative cache is keyed by apiKeyId; its own remark says the ceiling makes "a flood of unknown ids … not become a SQL flood", but Store only clears wholesale at 1024 entries and a flood of distinct ids misses every time. Each miss = one SingleOrDefaultAsync bounded by StoreTimeoutMs (2 s) while the SDK holds its SessionManager-wide lock.
  • Failure scenario: Any client that can reach 4840 on an admin/fused node — no credential needed, only the apikey username and a well-formed otua_<random>_<x> password — can hold the global activation lock for one indexed query per attempt, ~1-2 ms normally and up to 2 s during SQL slowness, starving every legitimate activation (anonymous and X.509 included). The ACL path on a driver-only node (ArtifactApiKeyStore) is immune.
  • Suggested fix: Rate-limit/fail-fast per remote endpoint before the store call, or move the ConfigDb key set into memory (the artifact snapshot already carries it on every node — the fused node could read IDataPlaneAuthoritySource first and only fall through to SQL on a miss that the snapshot cannot answer), or prefix-check keyId against an in-memory bloom/set refreshed on StoreCacheSeconds.

3.4 Central's inbound-ack gate blocks a gRPC thread on a synchronous secret-store read after every contact refresh

  • Severity: Low Confidence: High
  • Location: src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Communication/IMeshPskProvider.cs:316-353 (GetAwaiter().GetResult() under a Lazy ExecutionAndPublication lock, 3 s bound); src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Communication/CentralCommunicationActor.cs:782-783 (InvalidateAll() on every refresh, default every 60 s).
  • Evidence: The provider's remarks call the block acceptable because it "happens at most once per cluster per process"; the actor's remarks say the cache is dropped "on every contact refresh". Both are true; together they mean once per cluster per minute, and the first ack/dial per cluster after each refresh pays it on a gRPC request thread (or, for dials, a thread-pool thread).
  • Failure scenario: With a slow secrets store (SQLite on a busy disk, or a future networked KEK) every deploy's first ack per cluster waits up to 3 s and, past the bound, is denied PermissionDenied — a transient, confusing authentication failure on an otherwise correct key.
  • Suggested fix: Refresh the per-cluster keys asynchronously from the actor's ContactsLoaded handler and swap the cache atomically, instead of invalidating and re-reading on the request path.

3.5 ConfigPublishCoordinator.PreStart materialises the full newest Deployment row including ArtifactBlob

  • Severity: Low Confidence: High
  • Location: ConfigPublishCoordinator.cs:164-167 — .OrderByDescending(d => d.CreatedAtUtc).FirstOrDefault() with no Select.
  • Failure scenario: One blob read on every singleton start/hand-over; harmless today, grows with 1.2.
  • Suggested fix: .Select(d => new { d.DeploymentId, d.CreatedAtUtc }).

4. Other hazards

4.1 Admin operations echo raw exception text to the UI

  • Severity: Low Confidence: High
  • Location: AdminOperationsActor.cs:375-384, 423-431, 459-462, 489-492 (Message: ex.Message).
  • Failure scenario: A SqlException/UriFormatException message (connection string fragments, host names) reaches any ConfigEditor-role browser session. Low because the audience is already authenticated admin staff.
  • Suggested fix: Return a fixed message + correlation id; keep the detail in the log.

4.2 Fail-open window on the mesh known-cluster gate at central boot (documented, bounded)

  • Severity: Low Confidence: High
  • Location: CentralCommunicationActor.cs:216-230, 286-293, 755-793; IMeshPskProvider.cs:63-69, 199-201.
  • Evidence: Until the first successful ClusterNode load, _knownClusters is null and TryGetPsk consults the secret store for any x-otopcua-cluster value. A 15 × 2 s bootstrap retry and the 60 s periodic refresh heal it once SQL answers.
  • Failure scenario: During a central boot with ConfigDb down, an unauthenticated prober can drive one bounded secret-store read + warning log per request. Keys are still required; this is only the cost/oracle the gate exists to remove. Acceptable, but worth knowing it is "fail-open until SQL".

4.3 ActiveNodeHealthCheck/Primary election and the OPC UA endpoint are decoupled (see 2.1)

Covered under 2.1 — listed here because the consequence is an availability hazard rather than a stub.


5. Checked and fine

  • DbContext lifetimes. Every consumer in the slice takes IDbContextFactory<OtOpcUaConfigDbContext> and creates a short-lived (pooled) context per call; read paths use AsNoTracking() (47 sites); the only scoped OtOpcUaConfigDbContext registration (Configuration/ServiceCollectionExtensions.cs:41) exists for DataProtection's key repository and returns the context to the pool at scope end.
  • BoundedApiKeyStore deadline race. Abandoned lookups are observed (ObserveAbandoned), never cached, CTSs disposed on both arms; faults/timeouts never cached. The negative-cache amplification (3.3) is the only gap.
  • ApiKeyOpcUaUserAuthenticator. Uniform deny message, anonymous-principal key refused, catch-all deny; one HMAC per activation — no per-request crypto beyond that.
  • gRPC auth interceptors (5). All fail closed on an unset key, use FixedTimeEquals, scope by method prefix, single public ctor; CentralCommandAuthInterceptor returns one status text for all denials.
  • MeshPskProvider.TryGetPsk Lazy-exception handling. A throwing logger cannot poison the cache for the process lifetime (entry evicted, fail-closed).
  • CentralCommunicationActor lifecycle. _lifecycle CTS cancelled + disposed in PostStop, token reads guarded by ObjectDisposedException; per-dial results piped as messages; nothing captured off-thread.
  • AclRevalidationTimer — created last in StartAsync, disposed first in StopAsync and in DisposeAsync, dispose-before-overwrite on restart, tick never throws.
  • ActorSystemTerminationWatchdog, DriverResilienceStatusPublisherService, SecretReplicationStarter, ClusterNodeAddressReconcilerActor, RedundancyStateActor — timers/subscriptions are cleaned up in PostStop/StopAsync; periodic loops catch and continue; nothing resolves the ActorSystem at construction.
  • TelemetryStreamGrpcService — static _activeStreams cap is paired on every path; stream lifetime bounded at 4 h; subscription disposed by using.
  • TelemetryStreamClient / MeshGrpcCommandDialer — owned GrpcChannels disposed; keep-alive pings set.
  • Program.cs — IHistorianBackendProvider registered via factory (so the container disposes it); Kestrel re-bind computed exactly once; HTTPS surface refuses takeover rather than silently dropping TLS; startup failures exit non-zero; HostAbortedException passes through.
  • DraftValidator — purely in-memory over the snapshot; GroupBy/First guards against duplicate ids throwing mid-pass; ACL scope rules mirror the picker (NodeAclScopeIdUnexpected etc.).
  • LdapGroupRoleMappingService, ClusterAuditQuery — context per call, AsNoTracking, parameterised Contains.
  • AlarmSfLegacyMigrator — read-only legacy open, single transaction, rename only after commit, INSERT OR IGNORE with payload-derived ids makes a re-run idempotent.
  • OtOpcUaLdapAuthService — singleton over a stateless library client; Enabled wins over DevStubMode; plaintext transport refused unless AllowInsecure.
  • AuthEndpoints — open-redirect guard on returnUrl, role-mapper fault fails closed to zero roles (documented and tested).