From b67d736ac9f40e2c6049edfc39ba70eb0724dce8 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 09:50:37 -0400 Subject: [PATCH] docs: reconcile stale cluster/security/notification docs; close register row 117, add rows 128-135 (review 2026-10-21 wave 2) --- README.md | 4 +- docs/components/Host.md | 6 +-- docs/components/NotificationService.md | 4 +- docs/components/Security.md | 39 ++++++++----------- docs/components/TraefikProxy.md | 2 +- docs/operations/failover-procedures.md | 14 +++---- .../2026-07-08-deferred-work-register.md | 12 +++++- .../Component-ClusterInfrastructure.md | 4 +- docs/requirements/Component-Host.md | 2 +- docs/requirements/HighLevelReqs.md | 2 +- 10 files changed, 45 insertions(+), 44 deletions(-) diff --git a/README.md b/README.md index 60ee4e38..0d87da77 100644 --- a/README.md +++ b/README.md @@ -18,7 +18,7 @@ This repository is the full **implementation** project for ScadaBridge — the C | Authentication | Direct LDAP/AD bind (LDAPS/StartTLS), JWT sessions | | Notifications | Delivered from the central cluster (Email via SMTP/OAuth2-M365 or on-prem Exchange EWS; SMS via Twilio REST); store-and-forwarded from sites | | Hosting | Windows Server, Windows Service | -| Cluster | Akka.NET Cluster (active/standby, keep-oldest SBR) | +| Cluster | Akka.NET Cluster (active/standby, `auto-down` downing) | | Logging | Serilog (structured) | ### Scale @@ -88,7 +88,7 @@ Both stacks share the infrastructure services in [`infra/`](infra/) (MS SQL, LDA | 10 | Security & Auth | [docs/requirements/Component-Security.md](docs/requirements/Component-Security.md) | Direct LDAP bind (LDAPS/StartTLS), JWT sessions (HMAC-SHA256, 15-min refresh, 30-min idle), role-based authorization (incl. the `Operator`/`Verifier` two-person secured-write roles + policies), site-scoped permissions. | | 11 | Health Monitoring | [docs/requirements/Component-HealthMonitoring.md](docs/requirements/Component-HealthMonitoring.md) | 30s report interval, 60s offline threshold, monotonic sequence numbers, raw error counts, tag resolution counts, dead letter monitoring. | | 12 | Site Event Logging | [docs/requirements/Component-SiteEventLogging.md](docs/requirements/Component-SiteEventLogging.md) | SQLite storage, 30-day retention + 1GB cap, daily purge, paginated remote queries with keyword search. | -| 13 | Cluster Infrastructure | [docs/requirements/Component-ClusterInfrastructure.md](docs/requirements/Component-ClusterInfrastructure.md) | Akka.NET cluster, keep-oldest SBR with down-if-alone, min-nr-of-members=1, 2s/10s/15s failure detection, CoordinatedShutdown, automatic dual-node recovery. The `ClusterInfrastructure` project owns the `ClusterOptions` config model; the Akka bootstrap/SBR/CoordinatedShutdown wiring lives in the Host. | +| 13 | Cluster Infrastructure | [docs/requirements/Component-ClusterInfrastructure.md](docs/requirements/Component-ClusterInfrastructure.md) | Akka.NET cluster, `auto-down` downing strategy (2026-07-21 availability-over-partition-safety decision; `keep-oldest` stays a supported option), min-nr-of-members=1, 2s/10s/15s failure detection, CoordinatedShutdown, automatic dual-node recovery. The `ClusterInfrastructure` project owns the `ClusterOptions` config model; the Akka bootstrap/downing/CoordinatedShutdown wiring lives in the Host. | | 14 | Inbound API | [docs/requirements/Component-InboundAPI.md](docs/requirements/Component-InboundAPI.md) | POST /api/{methodName}, X-API-Key header, flat JSON, extended type system (Object/List), script-based implementations, failures-only logging. | | 15 | Host | [docs/requirements/Component-Host.md](docs/requirements/Component-Host.md) | Single deployable binary, role-based component registration, per-component config binding (Options pattern), readiness gating, dead letter monitoring, Akka.NET bootstrap, ASP.NET Core hosting for central. | | 16 | Commons | [docs/requirements/Component-Commons.md](docs/requirements/Component-Commons.md) | Namespace/folder convention (Types/Interfaces/Entities/Messages), shared data types, POCOs, repository interfaces, message contracts with additive-only versioning, UTC timestamp convention, the unified read-only alarm condition model (`AlarmConditionState`/`AlarmKind`), and native alarm source entities + the `IAlarmSubscribableConnection` capability seam. | diff --git a/docs/components/Host.md b/docs/components/Host.md index 09f80935..47a1981b 100644 --- a/docs/components/Host.md +++ b/docs/components/Host.md @@ -177,12 +177,12 @@ Options are bound via the .NET Options pattern (`IOptions`). Each component o | Key | Default | Description | |-----|---------|-------------| | `SeedNodes` | — | List of Akka seed-node URIs (`akka.tcp://scadabridge@host:port`). At least 2 required. Must reference remoting ports, not gRPC ports. | -| `SplitBrainResolverStrategy` | `keep-oldest` | Active strategy name (e.g. `"keep-oldest"`). | -| `StableAfter` | `"00:00:15"` | Duration the cluster must be stable before the resolver acts. | +| `SplitBrainResolverStrategy` | `auto-down` | Downing strategy: `"auto-down"` (default) or `"keep-oldest"`. | +| `StableAfter` | `"00:00:15"` | Sustained unreachability before downing (`auto-down-unreachable-after` under `auto-down`). | | `HeartbeatInterval` | `"00:00:02"` | Akka failure-detector heartbeat cadence. | | `FailureDetectionThreshold` | `"00:00:10"` | Acceptable heartbeat pause before a node is considered unreachable. | | `MinNrOfMembers` | `1` | Minimum cluster members before the leader is elected. | -| `DownIfAlone` | `true` | When using `keep-oldest`, whether a lone surviving node downs itself. | +| `DownIfAlone` | `true` | `keep-oldest` only; inert under `auto-down`. | ### `ScadaBridge:Database` → `DatabaseOptions` diff --git a/docs/components/NotificationService.md b/docs/components/NotificationService.md index f5cfc754..873622dd 100644 --- a/docs/components/NotificationService.md +++ b/docs/components/NotificationService.md @@ -20,7 +20,7 @@ Before the current design, site nodes delivered notifications directly over SMTP This means: - Credential exposure is limited to the central cluster. - List membership is resolved at delivery time, so a list change takes effect for all future deliveries without redeploying to sites. -- The SMTP `MaxConcurrentConnections` value is configured at a single point, though it is not currently enforced (no connection gate or semaphore). +- The SMTP `MaxConcurrentConnections` value is configured at a single point and now clamps the Notification Outbox dispatcher's Email delivery parallelism (review 2026-10-21 06/IMPL-2); it is still unused for the EWS transport. ### `NotificationType` discriminator @@ -156,7 +156,7 @@ services.AddNotificationOutbox(); // registers EmailNotificationDeliveryAdapte | Section | Key | Default | Description | |---------|-----|---------|-------------| | `ScadaBridge:Notification` | `ConnectionTimeoutSeconds` | `30` | SMTP connection/operation timeout in seconds. Applied when `SmtpConfiguration.ConnectionTimeoutSeconds` is zero or negative. | -| `ScadaBridge:Notification` | `MaxConcurrentConnections` | `5` | Maximum concurrent SMTP connections. Used as the documented fallback default when the `SmtpConfiguration` row is unset; this limit is not currently enforced by a connection gate or semaphore. | +| `ScadaBridge:Notification` | `MaxConcurrentConnections` | `5` | Maximum concurrent SMTP connections. Used as the fallback default when the `SmtpConfiguration` row is unset; the outbox dispatcher clamps Email parallelism to it (unused under EWS). | SMTP retry settings (`MaxRetries`, `RetryDelay`) live on the `SmtpConfiguration` entity and are read by the Notification Outbox dispatcher — they are not part of `NotificationOptions`. diff --git a/docs/components/Security.md b/docs/components/Security.md index fc7f9693..f4fff3d1 100644 --- a/docs/components/Security.md +++ b/docs/components/Security.md @@ -1,6 +1,6 @@ # Security & Auth -The Security & Auth component handles user authentication against an LDAP/Active Directory server and enforces role-based authorization across all central cluster operations. It owns the cookie+JWT hybrid session model, the LDAP-group-to-role mapping pipeline, site-scoped deployment permissions, and the inbound API key management seam. +The Security & Auth component handles user authentication against an LDAP/Active Directory server and enforces role-based authorization across all central cluster operations. It owns the cookie session model (interactive UI) and the bearer-JWT model (CLI/API), the LDAP-group-to-role mapping pipeline, site-scoped deployment permissions, and the inbound API key management seam. ## Overview @@ -8,7 +8,7 @@ Security & Auth (#10) runs exclusively on the central cluster — sites have no The component registers: -- `JwtTokenService` — token generation, validation, idle-timeout enforcement, and sliding refresh logic. +- `JwtTokenService` — mints and validates the short-lived HS256 **bearer** JWT issued by `POST /auth/token` for CLI/API callers (it no longer participates in the interactive cookie session; the sliding-refresh/idle members were removed in the 2026-10-21 review remediation). - `RoleMapper` — DB-backed LDAP-group-to-role resolution with site-scope union semantics. - `ScadaBridgeGroupRoleMapper` — adapter exposing `RoleMapper` on the shared `IGroupRoleMapper` seam. - `HttpAuditActorAccessor` — resolves the authenticated username from the ambient HTTP context for audit `Actor` stamping. @@ -21,15 +21,15 @@ The component registers: Authentication uses a direct username/password bind against the LDAP/AD server via the shared `ILdapAuthService` (`ZB.MOM.WW.Auth.Ldap`). The flow is: service-account bind → search for the user entry by username → user-credential bind → group-membership query. The app never caches credentials locally. LDAPS (port 636) or StartTLS is required for production; the `AllowInsecure` flag in `LdapOptions` gates unencrypted use to explicitly opted-in deployments (local dev only). No Kerberos/NTLM path exists. -LDAP failure behavior is fail-closed at the login boundary and fail-open at the session boundary: a new login fails immediately if the directory is unreachable; an active session (valid cookie+JWT) continues with its current claims until the JWT expires. This avoids disrupting engineers mid-work during a brief directory outage. When the directory recovers, the next token refresh re-queries groups and issues a fresh token. +LDAP failure behavior is fail-closed at the login boundary and fail-open at the session boundary: a new login fails immediately if the directory is unreachable; an active cookie session continues with its current claims (its mid-session role refresh is DB-only and never calls LDAP), and a CLI holding a bearer token keeps working until the token expires. This avoids disrupting engineers mid-work during a brief directory outage. Group-membership changes are picked up at the next login. -### Cookie+JWT hybrid session +### Sessions: cookie (UI) and bearer JWT (CLI/API) -On successful login the server mints a JWT via `JwtTokenService.GenerateToken` and writes it into an HttpOnly/Secure authentication cookie. The cookie is the transport — not a bearer header — because Blazor Server's persistent SignalR circuits do not carry `Authorization` headers on reconnect. The browser sends the cookie on every HTTP and SignalR request automatically. +**Interactive UI.** `POST /auth/login` signs the user in with ASP.NET Core cookie authentication. The HttpOnly/Secure cookie carries a data-protected claims ticket (no embedded JWT) built by the shared `SessionClaimBuilder`: display name, username, roles, permitted site IDs (site-scoped Deployer), the raw LDAP groups and the idle / role-refresh anchors. The cookie is the transport because Blazor Server's SignalR circuits do not carry `Authorization` headers on reconnect. Both central nodes share the Data Protection key ring, so either node validates a cookie the other issued. The cookie's `ExpireTimeSpan` is `IdleTimeoutMinutes` (default 30) with sliding expiration; passive traffic does not renew it. Once the role-refresh anchor is older than `RoleRefreshThresholdMinutes` (default 15) the validation hook re-runs the DB-backed `RoleMapper` on the stored groups (no LDAP call). -The JWT is signed with HMAC-SHA256 using a shared symmetric key (`JwtSigningKey`). Both central nodes share the same key, so either node can issue and validate tokens without a shared session store; the load balancer needs no sticky-session configuration. `ClockSkew` is set to `TimeSpan.Zero` to close the standard five-minute tolerance window. +**CLI / API.** `POST /auth/token` performs one LDAP bind per invocation and returns a short-lived HS256 JWT (`JwtExpiryMinutes`, default 15) signed with `JwtSigningKey`, shared by both central nodes; the response advertises `management_bearer: true`. `ManagementAuthenticator` accepts `Authorization: Bearer` alongside HTTP Basic on `POST /management`, the audit REST endpoints and the debug-stream hub. Roles inside a valid token are authoritative until it expires, so a revocation reaches a bearer caller within `JwtExpiryMinutes`; rotating `JwtSigningKey` invalidates every token. Under `DisableLogin`, `/auth/token` answers 404. Full spec: `docs/requirements/Component-Security.md`. -Claims embedded in every token: +Claims in the bearer token: | Claim type | Constant | Value | |---|---|---| @@ -37,15 +37,8 @@ Claims embedded in every token: | `ZbClaimTypes.Username` | `JwtTokenService.UsernameClaimType` | Authenticated username | | `ClaimTypes.Role` (URI) | `JwtTokenService.RoleClaimType` | One claim per role | | `ZbClaimTypes.ScopeId` | `JwtTokenService.SiteIdClaimType` | One claim per permitted site (Deployer only) | -| `"LastActivity"` | `JwtTokenService.LastActivityClaimType` | ISO 8601 idle-timeout anchor | -`MapOutboundClaims` is cleared on the mint path (`GenerateToken`) so `JwtSecurityTokenHandler` writes claim type strings verbatim into the token. On the validate path (`ValidateToken`) only `MapInboundClaims = false` is set — the outbound map is not touched. Together these settings prevent the default claim-type rewrite maps from transforming the canonical role URI or `zb:` claim types, keeping the type strings byte-for-byte identical in the token and in every policy check. - -### Token lifecycle and idle timeout - -The JWT lifetime (`JwtExpiryMinutes`, default 15 minutes) and the cookie idle window (`IdleTimeoutMinutes`, default 30 minutes) are separate layers. ASP.NET cookie auth's `SlidingExpiration = true` with `ExpireTimeSpan = IdleTimeout` models the idle window: the middleware re-issues the cookie once the session passes its halfway mark, keeping active users signed in. The JWT within that cookie has its own 15-minute expiry. - -`JwtTokenService.ShouldRefresh` checks whether remaining JWT lifetime is below `JwtRefreshThresholdMinutes` (default 5 minutes). `RefreshToken` issues a fresh JWT while **preserving** the existing `LastActivity` anchor — a background refresh is not treated as user activity. `RecordActivity` advances the anchor to now. `IsIdleTimedOut` checks whether the elapsed time since `LastActivity` exceeds `IdleTimeoutMinutes`; `RefreshToken` enforces the idle check internally so an idle-expired session cannot be kept alive by background polling regardless of caller discipline (Security-014). +Claim-type maps are disabled on mint and validate (`MapInboundClaims = false`), so the canonical role URI and `zb:` claim types stay byte-identical in the token and in every policy check; validation allows a 30 s clock skew for node-to-node drift. ## Architecture @@ -143,10 +136,9 @@ Login flow (Central UI `/auth/login` and `/auth/token`): 1. Call `ILdapAuthService.AuthenticateAsync(username, password)` (registered by Host via `AddZbLdapAuth`). 2. On success, call `RoleMapper.MapGroupsToRolesAsync(ldapGroups)` to resolve roles and site scope. -3. Call `JwtTokenService.GenerateToken(displayName, username, roles, permittedSiteIds)` to mint a signed JWT. -4. Write the JWT into the HttpOnly cookie via the ASP.NET cookie auth `SignInAsync`. +3. `/auth/login`: build the claims via `SessionClaimBuilder` and call the ASP.NET cookie `SignInAsync`. `/auth/token`: call `JwtTokenService.GenerateToken(displayName, username, roles, permittedSiteIds)` and return the bearer JWT. -On each subsequent request, middleware reads the cookie, validates the embedded JWT with `JwtTokenService.ValidateToken`, and checks `IsIdleTimedOut`. If the token is near expiry (`ShouldRefresh`), fresh claims are re-queried from LDAP and `RefreshToken` issues a replacement. Genuine user interactions call `RecordActivity` to advance the last-activity anchor. +On each subsequent cookie request the validation hook enforces the idle timeout and, past the role-refresh threshold, re-maps the stored groups against the DB. Bearer requests validate the token with `JwtTokenService.ValidateToken` and perform no LDAP bind or role re-map. Authorization gates use the named policies: @@ -167,10 +159,11 @@ if (!AuthorizationPolicies.OperationalAuditRoles.Contains(userRole)) | Key | Default | Description | |---|---|---| -| `JwtSigningKey` | *(required)* | Symmetric HMAC-SHA256 signing key. Must be at least 32 bytes (256 bits); validated at `JwtTokenService` construction — startup fails if too short. | -| `JwtExpiryMinutes` | `15` | JWT lifetime in minutes before embedded token expires and must be refreshed. | -| `JwtRefreshThresholdMinutes` | `5` | Minutes before JWT expiry at which `ShouldRefresh` triggers a re-issue. | -| `IdleTimeoutMinutes` | `30` | Session idle timeout in minutes. Cookie `ExpireTimeSpan` is set to this value; `IsIdleTimedOut` enforces it from the `LastActivity` claim. | +| `JwtSigningKey` | *(required)* | Symmetric HMAC-SHA256 signing key. Must be at least 32 bytes (256 bits); `StartupValidator` rejects a shorter key on Central. | +| `JwtExpiryMinutes` | `15` | Lifetime of the bearer JWT issued by `/auth/token`. | +| `IdleTimeoutMinutes` | `30` | Cookie session idle timeout; set as the cookie `ExpireTimeSpan`. | +| `RoleRefreshThresholdMinutes` | `15` | Age of the role-refresh anchor after which the cookie's roles are re-mapped from the DB (LDAP-free). | +| `RoleMappingCacheSeconds` | `30` | Process-wide `RoleMappingCache` TTL (`0` disables); invalidated on the local node after mapping/scope-rule edits. | | `RequireHttpsCookie` | `true` | When `true`, the cookie is `Secure`-only (HTTPS required). Set `false` for HTTP-only dev deployments; a warning is logged at startup. | | `CookieName` | `ZB.MOM.WW.ScadaBridge.Auth` | Authentication cookie name. Override per deployment when two ScadaBridge stacks share a hostname — browsers scope cookies by host+path but not by port. | @@ -211,7 +204,7 @@ Maps from `LdapAuthFailure.GroupLookupFailed`. The user-credential bind succeede ### Session expires unexpectedly -Check `IdleTimeoutMinutes` and `JwtExpiryMinutes` in `SecurityOptions`. A background refresh that fires while the user is idle preserves the `LastActivity` anchor (`RefreshToken` does not advance it); `IsIdleTimedOut` enforces the window from the last genuine user interaction. If the idle timeout fires before the expected window, confirm that `RecordActivity` is being called on genuine user requests. +Check `IdleTimeoutMinutes` and `JwtExpiryMinutes` in `SecurityOptions`. Passive traffic (`/auth/ping`, Blazor transport validation) does not renew the cookie; only the antiforgery-protected `/auth/activity` endpoint does. If the idle timeout fires earlier than expected, confirm genuine user interaction reaches that endpoint. ### Two ScadaBridge environments on the same host clobber each other's session diff --git a/docs/components/TraefikProxy.md b/docs/components/TraefikProxy.md index ba78e45c..f178a0d7 100644 --- a/docs/components/TraefikProxy.md +++ b/docs/components/TraefikProxy.md @@ -38,7 +38,7 @@ The identical oldest-member check backs `ActiveNodeGate` — the `IActiveNodeGat ### Automatic failover -When the active central node goes down, the Akka cluster's keep-oldest split-brain resolver leaves the surviving node as the oldest Up member and the singletons migrate to it (roughly 25 seconds: 10-second heartbeat threshold plus a 15-second stable-after period). Once the surviving node's `OldestNodeActiveHealthCheck` starts returning 200, Traefik's next poll cycle — within the 5-second interval — removes the failed backend from the pool and routes all subsequent requests to the new active node. No config change or restart is required on the Traefik side. +When the active central node goes down, the Akka cluster's `auto-down` downing strategy (the leader among the reachable members downs the dead node) leaves the surviving node as the oldest Up member and the singletons migrate to it (roughly 25 seconds: 10-second heartbeat threshold plus a 15-second `auto-down-unreachable-after` window). Once the surviving node's `OldestNodeActiveHealthCheck` starts returning 200, Traefik's next poll cycle — within the 5-second interval — removes the failed backend from the pool and routes all subsequent requests to the new active node. No config change or restart is required on the Traefik side. ## Architecture diff --git a/docs/operations/failover-procedures.md b/docs/operations/failover-procedures.md index 5263915c..9801dbfa 100644 --- a/docs/operations/failover-procedures.md +++ b/docs/operations/failover-procedures.md @@ -8,16 +8,16 @@ 1. Active central node becomes unreachable (process crash, network failure, hardware failure). 2. Akka.NET failure detection triggers after ~10 seconds (2s heartbeat, 10s threshold). -3. Split-brain resolver (keep-oldest) evaluates cluster state for 15 seconds (stable-after). +3. `auto-down`: the leader among the reachable members downs the unreachable node after 15 seconds of sustained unreachability. 4. Standby node is promoted to active. Total time: ~25 seconds. 5. Cluster singletons migrate to the new active node. 6. Load balancer detects the failed node via `/health/ready` and routes traffic to the surviving node. -7. Active user sessions continue (JWT tokens are validated by the new node using the shared signing key). +7. Active user sessions continue (the cookie's data-protected ticket is validated by the new node via the shared Data Protection key ring; CLI bearer tokens via the shared signing key). 8. SignalR connections are dropped and Blazor clients automatically reconnect. **What is preserved:** - All configuration and deployment state (stored in SQL Server) -- Active JWT sessions (shared signing key) +- Active sessions (shared Data Protection keys for cookies, shared signing key for CLI bearer tokens) - Deployment status records (SQL Server with optimistic concurrency) **What is temporarily disrupted:** @@ -82,9 +82,9 @@ **Symptoms:** Both nodes believe they are the active node. Logs show "Cluster partition detected". **How the system handles it:** -- Keep-oldest resolver: The older node (first to join cluster) survives; the younger is downed. -- `down-if-alone = on`: If a node is alone (no peers), it downs itself. -- Stable-after (15s): The resolver waits 15 seconds for the partition to stabilize before acting. +- `auto-down` (decision 2026-07-21): each side downs the unreachable peer after 15 s and continues as a one-node cluster, so a real partition leaves BOTH nodes active until an operator restarts one side (accepted trade for ~25 s failover on either-node crash). +- `keep-oldest` (opt-in): the older node survives and the younger is downed; but a crash of the oldest is a total outage. +- Stable-after (15s): the window of sustained unreachability before the downing decision. **Manual intervention (if auto-resolution fails):** 1. Stop both nodes. @@ -96,7 +96,7 @@ **Symptoms:** Central UI returns errors. `/health/ready` returns 503. Logs show database connection failures. **Impact:** -- Active sessions with valid JWTs can still access cached UI state. +- Active sessions (valid cookie or bearer token) can still access cached UI state. - New logins fail (LDAP auth still works but role mapping requires DB). - Template changes and deployments fail. - Sites continue operating independently. diff --git a/docs/plans/2026-07-08-deferred-work-register.md b/docs/plans/2026-07-08-deferred-work-register.md index 096fb472..cec466e0 100644 --- a/docs/plans/2026-07-08-deferred-work-register.md +++ b/docs/plans/2026-07-08-deferred-work-register.md @@ -29,7 +29,7 @@ All 7 fix-now items landed via PLAN-04/05/06/07/08 (verified in review 08 round | 33 | ~~**`LocalDbOplogBacklog` has no threshold, KPI history, or dashboard treatment**~~ **RESOLVED 2026-08-15 (`8da60f57`).** Both halves shipped: sampled into KPI history via `SiteHealthKpiSampleSource` (conditionally — 12 samples unreplicated / 14 replicated, so a series gap truthfully means "not replicated") plus a backlog trend chart on the site health panel, and a SUSTAINED threshold badge (`LocalDbBacklogThreshold` 1000 held for `LocalDbBacklogSustainedFor` 15 min, anchored by `LocalDbBacklogAboveSince` in the aggregator) — a bare instantaneous threshold would fire on every replicated site every day from the documented purge burst. Null backlog is a deliberate third branch: it leaves the anchor unchanged (neither evidence of a drain nor of a backlog), so unreplicated sites can never anchor and can never be badged. NOTE the row's premise was false — the `SiteAuditBacklog` "configurable threshold badge" it told us to mirror was described twice in `Component-HealthMonitoring.md` but NEVER BUILT; both stale doc claims corrected, and the new badge was modeled on the real Metrics-stale badge instead. | Found by R7 while documenting the purge burst | Closed 2026-08-15. | — | | 34 | ~~**`site_events` purge burst is unbounded in the oplog and unmeasured at scale**~~ **RESOLVED 2026-08-16 (`d9a35846`) — measured, and paced.** Measured with the REAL `EventLogPurgeService` over a real `RegisterReplicated` LocalDb file + a loopback sync pair at the rig's `MaxBatchSize: 64`: 393 B/row incl. indexes ⇒ a 1 GB event log holds **~2.73 M rows**, and expiring them in one tick took 285 s and queued **2 731 947 oplog rows = 2.7x LocalDb's `MaxOplogRows` default (1 M)** — enough to make the maintenance tick prune the oplog and flag a FULL ten-table snapshot resync, bought by a routine retention purge. The decisive nuance: the *resident* backlog is produced−minus−drained, so a healthy peer blunts it (391 k peak on loopback, drained 6.0 min) — the old safety margin was a function of peer throughput and vanished with the peer down. Fix: `MaxPurgedRowsPerTick` (default **250 000**, `0` = unlimited escape hatch), one budget SHARED across retention + cap trim; a ceiling-limited tick reschedules after `PurgeCatchUpInterval` (**5 min**, validated `> 0` and `<= PurgeInterval`) so worst-case retention converges in ~55 min instead of 11 days; purge loop is now a delay loop anchored to the END of the previous purge; rate-limited Warning + counter `scadabridge.site.event_log.purge_ceiling_hits`. No purge-exemption path added. Row-33 interplay verified empirically (500 k drained in 46 s vs the 15-min dwell). Fail-before: `PurgeBudget` neutralised → 6 of the new tests fail, all 19 pre-existing pass. Measurement + decision record: `docs/plans/2026-08-15-site-events-purge-burst-measurement.md`. Side-finding recorded as row 81 (storage cap unenforceable on a replicated node). | Found by R7 | Closed 2026-08-16. | — | | 35 | ~~**`DeploymentRecords` index does not cover the deployment-list sort key**~~ **RESOLVED 2026-08-15 (`be29bf47`).** Migration `20260815165452_AddDeploymentRecordListSortIndex` + hand-written `docs/plans/sql/AddDeploymentRecordListSortIndex.sql`: composite `IX_DeploymentRecords_DeployedAt_Id (DeployedAt DESC, Id DESC)`; the old single-column index DROPPED as strictly redundant (`Id` is the clustered PK and was already in the old index's leaf as the row locator — same leaf width, now a declared ordered key; keeping it would cost a second index on every insert/status transition for no read benefit). Serves the three reads sharing the key (`QueryDeploymentListPageAsync`, `QueryDeploymentSummariesAsync`, `GetCurrentDeploymentStatusAsync`). `Id` in the key is also a CORRECTNESS matter: a bulk site deploy stamps many rows in one `DeployedAt` instant, so without the tie-break keyset pages can repeat or drop rows. Recorded caveat: the optimizer may already have served the ordering via a backward scan of the old index, so the "sorts per page" premise was possibly optimizer-dependent — the composite makes it deterministic and self-documenting; an actual plan capture against the rig would quantify the before/after. The ConfigurationDatabase suite exercises the descending composite as real DDL via SQLite `EnsureCreated()`, proving provider portability. | Found by R3 (`35ce1413`) | Closed 2026-08-15. | — | -| 36 | **Deployments free-text search is unindexable by construction** | Found by R3; re-verified 2026-08-17 (Low-tail sweep) | Four `LIKE '%term%'` predicates (instance name, deployment id, revision hash, user) — leading wildcards preclude index seeks, so a search is a scan of the join (`DeploymentManagerRepository.cs:100-104`; only searched column with any index is `DeploymentId`, unseekable under a leading wildcard). Acceptable at the purge-bounded row count. **2026-08-17 refresh: the row undercounted the cost by half** — `BuildListQuery` is shared with `GetDeploymentStatusCountsAsync`, so one search = TWO scans of the join (list + status tiles), mitigated by the UI's trailing-edge debounce (one pair per pause, not per keystroke). Anchoring the id/hash legs was evaluated and REJECTED as a micro-fix: `DeploymentListFilter.Search`'s doc comment promises substring matching and a 4-case `[Theory]` deliberately pins mid-string matches — changing it is a contract renegotiation, not an optimization. Fix if hot: renegotiate to prefix on id/hash legs, or full-text. *(Low)* | Search latency complaints on the Deployments page | +| 36 | **Deployments free-text search is unindexable by construction** | Found by R3; re-verified 2026-08-17 (Low-tail sweep) | Four `LIKE '%term%'` predicates (instance name, deployment id, revision hash, user) — leading wildcards preclude index seeks, so a search is a scan of the join (`DeploymentManagerRepository.cs:100-104`; only searched column with any index is `DeploymentId`, unseekable under a leading wildcard). Acceptable at the purge-bounded row count. **2026-08-17 refresh: the row undercounted the cost by half** — `BuildListQuery` is shared with `GetDeploymentStatusCountsAsync`, so one search = TWO scans of the join (list + status tiles), mitigated by the UI's trailing-edge debounce (one pair per pause, not per keystroke). Anchoring the id/hash legs was evaluated and REJECTED as a micro-fix: `DeploymentListFilter.Search`'s doc comment promises substring matching and a 4-case `[Theory]` deliberately pins mid-string matches — changing it is a contract renegotiation, not an optimization. Fix if hot: renegotiate to prefix on id/hash legs, or full-text. *(Low)* **2026-10-21 note:** the same leading-wildcard `LIKE` shape exists on Instances and `AuditLogEntries` free-text search. `ListInstances` is no longer one of them — it pages, site-scopes and **prefix**-searches in SQL as of the 2026-10-21 review remediation (`baab6ac7`; CLI `instance list --search` is now a prefix match) — but the config-audit viewer filters and the Deployments page remain scans. | Search latency complaints on the Deployments page or the config audit log | | 37 | ~~**Two paged reads of `DeploymentRecords` with different contracts**~~ **RESOLVED 2026-08-17 (merge `ad0100f2`, agent commit `9f67147e`).** `QueryDeploymentSummariesAsync` and `DeploymentRecordSummary` are DELETED; the CLI/HTTP `QueryDeployments` handler now rides `QueryDeploymentListPageAsync` + `DeploymentListFilter` — one row shape (`DeploymentListRow`), one scoping mechanism (site-id via the query join), total count available to both surfaces (`DeploymentListPage` now also owns `MaxPageSize` + `DefaultPageSize = 50`). Rows 86–88 were resolved with the consolidation, as this row anticipated. Row 36's 4-case substring-search Theory untouched and still passing. Remaining UI-vs-CLI asymmetry (`GetDeploymentStatusCountsAsync` status tiles are UI-only) is deliberate, not drift. **Live-probed on the rig:** CLI `deploy status --status Success --page-size 5` returns the consolidated page — rows now carry `siteId` + `instanceUniqueName`, and the response carries `totalCount` (11) the CLI never had. | Found by R3; consolidation shipped with rows 86–88 | Closed 2026-08-17. | — | | 38 | ~~**OtOpcUa: `RoslynVirtualTagEvaluatorTests` racing-clear test is load-fragile**~~ **RESOLVED 2026-08-18 in OtOpcUa (`34829ea0` + `837d8a17`, branch perf/followups-r3).** Rebuilt on the assert-on-retry-exhaustion option, renamed `…never_leaks_a_raw_disposed_failure`. Two findings beyond the brief: (1) the DOMINANT load-failure mode was the test's own 45 s `WaitAsync` budget — every clear forces a recompile, so the fixed 12×400 workload's wall-clock tracked spare CPU; reproduced RED 4/4 at 4-way self-concurrency, fixed by duration-bounding the loops (10 s window OR 400 iters), GREEN 4/4 and 8/8 after. (2) A second flake vector: `TimedScriptEvaluator`'s 5 s clock starts at enqueue, so pool starvation surfaces "script timed out" from healthy evaluations — now its own bounded bucket, not counted as a leak; failure-reason literals promoted to shared consts. Honest caveat recorded: the disposed race itself does not reproduce on the arm64 macOS box (zero ODEs even with the product retry disabled), so the raw-escape assertion is the load-independent guarantee. Commit stats cross-checked from this repo. | Found by R4; fixed cross-session by the OtOpcUa agent 2026-08-18 | Closed 2026-08-18. | — | | 39 | ~~**OtOpcUa: late-enabled replication cannot baseline pre-existing rows**~~ **RESOLVED 2026-08-18 in OtOpcUa (`3e62f161` + review round `df533916`, branch perf/followups-r3).** ScadaBridge's WP1.3/WP3.3 pattern ported: registration follows replication config — configured ⇒ `RegisterReplicated(..., baselineExistingRows: true)` (probed live on 0.2.1: seeds `__localdb_row_version` at HLC 0), unconfigured ⇒ boot-time `DeregisterReplicated` self-heal, pair-symmetric; review round added deregister-preserves-data + partial-flip-fails-loudly tests. Green: `LocalDbSetupTests` 15/15, Host.IntegrationTests 226 pass/0 fail. **Stayed pinned to LocalDb 0.2.1 by deliberate choice** — the 0.2.1 API already carries both needed methods, and the v2→v3 one-way bookkeeping upgrade makes a 0.3.x bump a both-nodes-together operation; recorded caveat in OtOpcUa's CLAUDE.md: on 0.2.1 the snapshot-resync flag is per-DATABASE, so a late enable snapshots the whole file rather than per-table (0.3.1 available; bump logged as a deliberate future change in OtOpcUa's own follow-up register). Commit stats cross-checked from this repo. | Found by R4; fixed cross-session by the OtOpcUa agent 2026-08-18 | Closed 2026-08-18. | — | @@ -109,7 +109,7 @@ All 7 fix-now items landed via PLAN-04/05/06/07/08 (verified in review 08 round | 114 | *Never issued.* The 2026-09-09 step-logs close-out minted two rows numbered **113** — this one and the "other actors' log lines" follow-up. The duplicates were renumbered **115** / **116** at final review (I2) and 114 was left unused rather than reshuffling every number after it; register numbers are permanent identifiers, cited from code comments and `CLAUDE.md`. | — | — | — | | 115 | **Other actors' log lines are not captured for an execution — only the script's own `Log.*` and the runtime lines inside a scoped helper** *(Low)*. The Flow Debugger's step logs (2026-09-09) capture what the script logs and what `ScriptRuntimeContext`'s own logger emits while a run is traced, because those two go through one seam. What the *Instance Actor*, the *DCL*, the alarm actors or the S&F engine log while working on the same execution never reaches the graph: there are **zero** `LogContext.PushProperty` calls in the repo, `Enrich.FromLogContext()` is not configured on the Serilog pipeline (`TraceContextEnricher` stamps W3C ids only), and `site_events` has no execution-id column. Capturing them is a separate enrichment initiative: push `ExecutionId` (and instance/script) onto the ambient `LogContext` at run start, enable `Enrich.FromLogContext()`, add an execution-id column + index to `site_events`, then either sink matching entries into `ScriptLog` rows or query the site event log by execution id. | Step-logs design `docs/plans/2026-09-09-flow-debugger-step-logs-design.md` §2 + §9 ("Out of scope") | Out of proportion to the ask: it changes the logging pipeline of every actor on a site node, adds a replicated-table column (`site_events` is one of LocalDb's ten `ReplicatedTables`, so the change is a both-nodes-together schema edit), and risks a volume increase on untraced runs unless the enrichment is itself gated. The script-authored lines cover the debugging case the Flow Debugger exists for. | First investigation where the answer sits in what the Instance Actor or the DCL logged during a traced run and the operator cannot correlate it by hand. | | 116 | **`CompileSurfaceParityTests.AccessorMirrorPairs` cannot guard the `(ScriptLogger, CompileLog)` pair — the theory has no per-member exclusion mechanism** *(Low)*. The arity theory asserts the compile mirror exposes at least as many overloads as the runtime accessor for **every** public instance method, and `ScriptLogger` has one the mirror must NOT have: `FlushDropped()`, the run-end closing-row flush, which is called by `ScriptRunLauncher`, not by a script. Adding the pair today would fail the theory for a member that is correctly absent, so the pair is left out and `Log`'s four methods are guarded only by the top-level name-superset check plus the hand-written surface tests. The fix is an exclusion seam on the theory — an optional `IReadOnlySet ignoredRuntimeMethods` per row (or an `[NotScriptReachable]` marker attribute on `FlushDropped`, which also documents the intent at the declaration) — after which the pair joins `AccessorMirrorPairs`. | Found while wiring Task 15 of `docs/plans/2026-09-09-flow-debugger-step-logs.md`; test `tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/CompileSurfaceParityTests.cs` | No drift is possible without a code change to `ScriptLogger`, and a missing `Log` method on the mirror surfaces immediately as a `CS1061` at the design-time gate for any script that calls it. The seam is a test-infrastructure change that touches every existing row's signature. | The next public method added to `ScriptLogger` (or to any runtime accessor whose mirror must deliberately omit a member) — at that point build the exclusion seam and add the pair. | -| 117 | **Flow Debugger site trace pull: `FlowDebugRunner.WriteAndPollAsync` fetches ONE bounded page with a fixed `since`** *(Low)*. The write-and-poll loop asks the site for `FlowDebugRunner.PullTraceEventsMax` rows per poll, always `since` the run start, and the site serves `ORDER BY OccurredAtUtc ASC LIMIT` — so the page is the OLDEST rows in the window, never a continuation of the previous one. Step logs made that reachable: one traced run can now emit up to 201 `ScriptLog` rows, so a handful of chatty runs inside the window can fill the page and push a later spawned run's `ScriptRun` row past the end of it, at which point the loop never sees the awaited run and reports a false `NO_TRIGGER_WITHIN_WINDOW` for a run that did happen. Final review I1 raised the cap 500 -> 2 000 (the site's own ceiling, `SiteStreamGrpcServer.MaxTraceEventsPerPull`), which makes it ~10x harder but does not remove it. The real fix is to PAGE — advance `since` to the last-seen `OccurredAtUtc` between polls (keeping the already-known ids so nothing is double-counted) — or to exclude `ScriptLog` from the awaited-run poll entirely and pull the log rows once, after the run has settled. | Final review of `docs/plans/2026-09-09-flow-debugger-step-logs.md` (I1); `src/ZB.MOM.WW.ScadaBridge.ManagementService/FlowDebug/FlowDebugRunner.cs` `WriteAndPollAsync` | 2 000 is the site's hard ceiling, so the headroom cannot be bought again with a bigger number; paging needs the poll loop to track a cursor and the awaited-run predicate to survive a partial view, which is more than the raise was worth at review time. | The first `NO_TRIGGER_WITHIN_WINDOW` reported for a run whose Replay of the same root DOES show the spawned `ScriptRun`. | +| 117 | ~~**Flow Debugger site trace pull: `FlowDebugRunner.WriteAndPollAsync` fetches ONE bounded page with a fixed `since`**~~ **RESOLVED 2026-09-09 (`9ee402bb`, arch-review 2026-09-09 MG-6; closed in this register 2026-10-21 — found stale by the 2026-10-21 system review).** `WriteAndPollAsync` now advances a `(SinceUtc, AfterEventId)` keyset cursor between polls and only treats a page as progress when the cursor moved, so a chatty run can no longer push a spawned run past the page end. Original description kept for traceability: *(Low)*. The write-and-poll loop asks the site for `FlowDebugRunner.PullTraceEventsMax` rows per poll, always `since` the run start, and the site serves `ORDER BY OccurredAtUtc ASC LIMIT` — so the page is the OLDEST rows in the window, never a continuation of the previous one. Step logs made that reachable: one traced run can now emit up to 201 `ScriptLog` rows, so a handful of chatty runs inside the window can fill the page and push a later spawned run's `ScriptRun` row past the end of it, at which point the loop never sees the awaited run and reports a false `NO_TRIGGER_WITHIN_WINDOW` for a run that did happen. Final review I1 raised the cap 500 -> 2 000 (the site's own ceiling, `SiteStreamGrpcServer.MaxTraceEventsPerPull`), which makes it ~10x harder but does not remove it. The real fix is to PAGE — advance `since` to the last-seen `OccurredAtUtc` between polls (keeping the already-known ids so nothing is double-counted) — or to exclude `ScriptLog` from the awaited-run poll entirely and pull the log rows once, after the run has settled. | Final review of `docs/plans/2026-09-09-flow-debugger-step-logs.md` (I1); `src/ZB.MOM.WW.ScadaBridge.ManagementService/FlowDebug/FlowDebugRunner.cs` `WriteAndPollAsync` | 2 000 is the site's hard ceiling, so the headroom cannot be bought again with a bigger number; paging needs the poll loop to track a cursor and the awaited-run predicate to survive a partial view, which is more than the raise was worth at review time. | The first `NO_TRIGGER_WITHIN_WINDOW` reported for a run whose Replay of the same root DOES show the spawned `ScriptRun`. | | 118 | **Pre-existing bUnit flake: `CentralUI.Tests.Pages.NotificationReportDetailModalTests.CloseButton_DismissesModal` (~10 %)** *(Low)*. Fails intermittently with `UnknownEventHandlerIdException` — the test resolves the close button once and clicks it after a re-render has already retired that element's handler id. Not caused by the step-logs work: the file is untouched since `40944a0d`. The fix is a one-liner — re-`Find` the button immediately before the click (the pattern the other modal tests already use) rather than reusing an element captured earlier in the test. | Observed during the step-logs full-suite runs, 2026-09-09 | It is a test-only race with a known cause and a known fix; it was out of scope for a remediation pass whose rule was to touch only the files under review. | Next time `CentralUI.Tests` is worked on, or the first time the flake blocks a gate run. | | 68 | ~~**`deploy.sh` run from a git worktree silently rehomes every node's bind-mount data dir**~~ **RESOLVED 2026-08-15 (guard added in the same close-out).** The compose files' bind mounts are relative (`./site-a-node-a/data`), so a deploy from a worktree checkout mounts the worktree's copies — the rig keeps running and looks healthy, but its SQLite stores split from the main checkout's and everything written there is orphaned by the next deploy from main. Bit the rows-46–53 close-out live: the seeding agent's `rig-probe-1` deploy persisted into worktree-homed dirs and "vanished" after the orchestrator's redeploy (no tombstone, no product bug — LWW/tombstone forensics ruled out deletion; the row sat in `.claude/worktrees/…/docker/site-a-node-a/data/site-localdb.db` the whole time). Both `docker/deploy.sh` and `docker-env2/deploy.sh` now refuse to run from a `.claude/worktrees/` path (override: `SCADABRIDGE_ALLOW_WORKTREE_DEPLOY=1`). | Found during the rows-46–53 close-out | Closed 2026-08-15. | — | | 119 | **Port the QA/prod MES receiver scripts onto the reset-aware `WriteBatchAndWaitAsync` overload (Gitea #36)** *(Medium)*. Template scripts CvdReactor MesMoveIn (13) / MesMoveOut (14) / MesSampleData (36) and base MESReceiver MoveIn/MoveOut (15, 16, 25, 26, 37, 38) still reset `CompleteFlag` with a plain write and rely on `Task.Delay(1000)`; QA carries a config-only `WaitAsync(false, 10 s)` patch on 13/14 (deployment record 110). Rewrite: `var hs = await rx.Attributes.WriteBatchAndWaitAsync(values, "Flag", true, "CompleteFlag", true, 25 s, "CompleteFlag", false, 10 s); if (!hs.Matched) return new { WasSuccessful=false, ErrorText = hs.ResetFailed ? "Timeout waiting for CompleteFlag to reset" : "Timeout waiting for to complete", BatchID=0 };` and delete the delay. Requires the #36 build on the target central + sites first (the overload does not exist on older sites → deploy-gate compile failure, by design). | Gitea #36, design `2026-09-10-handshake-reset-echo-design.md` §8 | Repo-only scope was the user's decision for the #36 session; the script bodies live only in the QA/prod databases. | Next QA deployment of a build ≥ this change (write a deployment record; then prod). | @@ -121,6 +121,14 @@ All 7 fix-now items landed via PLAN-04/05/06/07/08 (verified in review 08 round | 125 | **Pending-updates counts have no Health-dashboard tile or CLI surface** *(Low)*. The aggregate (`PendingUpdatesSnapshot.StaleInstanceCount` / `StaleArtifactSiteCount`) is consumed only by the Central UI Sites page and rail indicator; there is no Health dashboard KPI tile and no `deploy`/`site` CLI command surfacing it — `site list` shows only the raw `artifactRevisionHash`/`artifactsDeployedAt` fields on each site, not whether they are current. | `docs/plans/2026-09-14-pending-updates-design.md` §3.2 ("Not in scope") | Out of scope for the UI-focused ask; the CLI/Health surfaces are a separate consumer of the same `IPendingUpdatesCache`. | An operator wants the pending-updates counts without opening the Central UI (headless monitoring, CLI scripting). | | 126 | ~~**TODO — roll the two outstanding migrations to the QA pair vq03/vq04**~~ **DONE 2026-09-28** — QA vq03/vq04 upgraded `2b9a7c3c` → `b3021e7c`, 55 → 57, artifacts deployed (record `docs/deployment/deployment-records/2026-09-28-wonder-app-vq03-vq04-b3021e7c.md`). *(deployment, closed)*. **DEV vd03 DONE 2026-09-28** (`fe89363b`, 55 → 57, record `docs/deployment/deployment-records/2026-09-28-wonder-app-vd03-fe89363b.md` — the `sb-mig-55to57.sql` there is the same script QA needs). QA still runs `2b9a7c3c` with the DB at migration 55 (`20260909203046_EnforceGlobalInstanceNamesAndProtectProtocolConfiguration`); `main` (`ac833f5a`, pushed 2026-09-14) carries two more: **56** `20260911135501_AddAlarmMesReportable` (computed-alarm `IsMesReportable` flag) and **57** `20260914173147_AddSiteArtifactRevision` (`Sites.ArtifactRevisionHash`/`ArtifactsDeployedAt`, both nullable). Both are additive `AddColumn`s — no data upgrade, no coordinated step like the 2026-09-10 protocol-config encryption. Recipe (same as the 55 rollout): generate ONE from→to delta script `dotnet ef migrations script EnforceGlobalInstanceNamesAndProtectProtocolConfiguration AddSiteArtifactRevision --project src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase --startup-project src/ZB.MOM.WW.ScadaBridge.Host -o sb-mig-55to57.sql` (NOT idempotent, identical for vd03 and the QA pair), take the pre-upgrade DB backup, stop the pair TOGETHER, apply via `sqlcmd -C -b -I` on the SQL host, publish/swap the `ac833f5a` binaries, start the pair, confirm the Central boot log shows 0 pending migrations and no "Applying migration" (the script already applied them). After upgrade every site reads *Artifacts never deployed* until one *Deploy Artifacts to All Sites* — run it as the last step. Write the deployment record `docs/deployment/deployment-records/-wonder-app-{vd03,vq03-vq04}-ac833f5a.md`. | Pending-updates finish (2026-09-14) | ~~Blocked by the 2026-09-11 remote-access hold~~ — **unblocked 2026-09-28**: `winrm` is re-enabled for deployment activities (CLAUDE.md). vd03 done; QA done 2026-09-28. | Closed 2026-09-28. | | 127 | **TODO — roll out site secrets via artifact deploy to vd03 + QA (vq03/vq04)**. The gRPC secrets hub is retired in ScadaBridge as of `docs/plans/2026-09-28-site-secrets-artifact-deploy-design.md` (merged to `main` @ `23a139a4` 2026-09-28, plus the same-day follow-ups: per-site secret-store outage handling, removal audit rows, per-node mirror status on the health report, artifact-deploy CLI exit codes; rig-only so far); vd03 and QA still run the pre-feature build with `Secrets:Replication:Enabled=false`, so upgrading them will **not** fail the boot (the retirement's `StartupValidator` rejection only fires on `Enabled=true`). **Upgrade impact:** once upgraded, any site whose data connections carry `secret:` references fails its **whole** artifact deploy (scripts, external systems, DB connections too) and reads *Out of date* until every referenced secret exists in the central store — so before upgrading vd03/QA, check whether their connection configs use `secret:` references and whether their site pairs run LocalDb replication, and do the prerequisites in the same window. Prerequisites, in order: (1) provision the `zbsecrets` schema/store in the environment's configuration SQL Server (the same server `ConfigurationDb` already points at); (2) set `Secrets:Store:Provider=SqlServer` on **both** central nodes of the environment; (3) verify the KEK (`ZB_SECRETS_MASTER_KEY` / master-key file) is identical across **every** node — both centrals and every site node — since a mismatch is caught only at deploy time by `DeployedSecretsMirror`'s per-secret verify, not at boot (after the first deploy a mismatch on either node — the standby included — shows as a *Secret mirror failing* badge on that site's Health card); (4) confirm every site pair that uses secrets runs LocalDb replication — on an unreplicated pair only the node active at deploy time gets the secrets, so a failover leaves the new active node without them until the next *Deploy Artifacts*; the upgrade also adds two replicated site tables (`deployed_secrets`, `secret_mirror_status`), so stop and start each site pair together as usual; (5) recreate, at the new central shared store, any secret an operator previously hand-seeded directly into a site's local `secrets.db` (those never round-tripped through central and would otherwise be silently absent from the referenced-secrets scan). Then run *Deploy Artifacts* against every affected site as the rollout's last step. | `docs/plans/2026-09-28-site-secrets-artifact-deploy-design.md` §4 *Rollout*; `docs/deployment/topology-guide.md` → *Secrets: shared store, per-site deploy, and the KEK* | Deployment record, not a code change — vd03/QA config is user-held and their rollout schedule is independent of the branch merging to main. | User asks for the vd03/QA secrets window. | +| 128 | **No TLS production profile: Traefik, Akka remoting and the gRPC h2c listeners are all plaintext** *(Medium for a non-lab network; accepted for the lab)*. Only the gRPC command/control and stream boundaries carry per-site PSK auth; nothing is encrypted and Akka remoting is unauthenticated (see CLAUDE.md *Security*). A deployable TLS profile — Traefik entrypoint certs, Akka `enable-ssl` + secure cookie, gRPC over h2/TLS with the PSK retained — has never been designed end to end. | 2026-10-21 system review (`docs/plans/2026-10-21-system-review-remediation.md`) (carried from prior reviews) | Trusted-network lab/plant deployment is the stated assumption; TLS is purely additive to the PSK design and needs no protocol change, but needs cert distribution for 8+ nodes. | First non-lab deployment (any network not wholly inside the plant security boundary). | +| 129 | **`ExternalDefinitionLookupCache` is not invalidated on external-system / DB-connection edits** *(Low)*. The central short-TTL definition cache (review 05/P2) is never `InvalidateAll()`'d by the management edit paths because ManagementService has no reference to ExternalSystemGateway; the node that took the edit and its peer both see it only after `DefinitionCacheTtl` (default 10 s). | 2026-10-21 system review (`docs/plans/2026-10-21-system-review-remediation.md`) (lane M) | A 10 s staleness window on a rarely-edited definition is harmless; wiring needs a new project reference or a bus event. | A report of an edit visibly not taking effect for an inbound-method call, or any increase of `DefinitionCacheTtl`. | +| 130 | **`TemplateService` member add/update still loads the whole template analysis graph** *(Low)*. 04/P6 is only partly done: the read paths use lean projections, but member add/update still materialises the full analysis graph because no repository method loads just a template's inheritance chain + composition closure. | 2026-10-21 system review (`docs/plans/2026-10-21-system-review-remediation.md`) (lanes F/G, 04/P6 partial) | Designer-rate operation; the new chain-only query is a repository + service refactor with its own correctness risk. | Template-edit latency complaints on a large library, or the library growing past a few hundred templates. | +| 131 | **`SiteStreamGrpcClientFactory` never evicts idle `(siteId, endpoint)` clients** *(Low)*. Clients are keyed per site and endpoint and live until disposed with their session/factory; an endpoint that is removed or renamed in the site's address pair leaves its client (and channel) resident. | 2026-10-21 system review (`docs/plans/2026-10-21-system-review-remediation.md`) (lane E item 6) | Bounded by the number of address edits over a process lifetime; a rename is rare. | Sites whose gRPC addresses change often, or a channel/handle leak seen in a long-running central. | +| 132 | **Site compile-cache eviction counter is not exported as a Meter** *(Low)*. `SiteScriptCompileCache` counts evictions and logs a rate-limited Warning (review 01/SR-M2) but the counter is not a `System.Diagnostics.Metrics` instrument, and `ZbTelemetryOptions.Meters` is an allowlist (`SiteServiceRegistration.ObservedMeters`), so exporting it also needs the Host allowlist entry. | 2026-10-21 system review (`docs/plans/2026-10-21-system-review-remediation.md`) (lane B) | The Warning log already surfaces thrash; a Meter is observability polish. | Wanting to alert on compile-cache thrash from Prometheus. | +| 133 | **Dedicated health field for site audit rejected rows** *(Low)*. Permanent push rejections (`IngestAck.rejected_event_ids`) are counted only by the `scadabridge.site.audit.rejected_rows` counter and the `Rejected` `ForwardState`; there is no `SiteHealthReport` field/dashboard badge. | 2026-10-21 system review (`docs/plans/2026-10-21-system-review-remediation.md`) (lane H) | Rejections only come from row-intrinsic SQL errors and should be near-zero; the counter is sufficient for now. | The first non-zero `rejected_rows` seen in production, or an operator asking for it on the Health dashboard. | +| 134 | **An oversize `AuditLog` row (larger than the gRPC message cap) wedges both push and pull** *(Low, accepted)*. A single row bigger than the 4 MB gRPC cap cannot be shipped by either the telemetry push or the reconciliation pull, so the queue head stalls. Unreachable at the default caps (8 KB / 64 KB payload caps, `InboundMaxBytes` 1 MiB) — it needs an operator to raise caps past the transport limit. | 2026-10-21 system review (`docs/plans/2026-10-21-system-review-remediation.md`) (lane H) | Defaults keep every row far below the limit; guarding means a skip/park state for unshippable rows. | Any deployment that raises `InboundMaxBytes` or the payload caps toward 4 MB. | +| 135 | **Config-audit single-column indexes are possibly redundant** *(Low)*. `ReviewRemediationIndexes` added composite `AuditLogEntries` indexes `(EntityType, Timestamp DESC)`, `(User, Timestamp DESC)` and `(Timestamp DESC, Id DESC)`; the older single-column indexes on timestamp/user/entity type may now be dead weight on a write-heavy table. | 2026-10-21 system review (`docs/plans/2026-10-21-system-review-remediation.md`) (lane P / O) | Dropping needs a measured before/after on a populated table, not an assumption. | Measure index usage (`sys.dm_db_index_usage_stats`) after the migration has run in QA/prod for a few weeks, then drop the unused ones. | ## Resolved (verified against the code 2026-07-10) Rows removed from the Deferred table above once confirmed shipped. Kept here for traceability. diff --git a/docs/requirements/Component-ClusterInfrastructure.md b/docs/requirements/Component-ClusterInfrastructure.md index 7bbf201e..d47fad96 100644 --- a/docs/requirements/Component-ClusterInfrastructure.md +++ b/docs/requirements/Component-ClusterInfrastructure.md @@ -29,7 +29,7 @@ realised across two projects: timings, split-brain settings) bound from `appsettings.json` via the Options pattern. - **`src/ZB.MOM.WW.ScadaBridge.Host`** owns the cluster **bootstrap and runtime wiring**: it builds the Akka.NET HOCON from `ClusterOptions`, starts the `ActorSystem`, - configures the keep-oldest split-brain resolver (`down-if-alone = on`), wires + configures the downing strategy (`auto-down` by default; `keep-oldest` with `down-if-alone` is the opt-in alternative), wires `CoordinatedShutdown` into the service lifecycle, and provides active-node / cluster-membership health checks. See `Component-Host.md` (REQ-HOST-*) for detail. @@ -220,7 +220,7 @@ If both nodes in a cluster fail simultaneously (e.g., site power outage): 2. **State recovery** (each node has its own local copy of all required data): - **Site clusters**: The Deployment Manager singleton reads deployed configurations from local SQLite and re-creates the full Instance Actor hierarchy. Store-and-forward buffers are already persisted locally. Alarm states re-evaluate from incoming data values. - **Central cluster**: All state is in MS SQL (configuration database). The active node resumes normal operation. -3. The keep-oldest resolver handles the "both starting fresh" case naturally — there is no pre-existing cluster to conflict with. +3. Cold start does not involve the downing strategy: nothing is unreachable yet, so there is no pre-existing cluster to conflict with (the simultaneous-start residual is covered by the bootstrap guard above). ## Graceful Shutdown & Singleton Handover diff --git a/docs/requirements/Component-Host.md b/docs/requirements/Component-Host.md index 4ad0a194..7d2ae5bf 100644 --- a/docs/requirements/Component-Host.md +++ b/docs/requirements/Component-Host.md @@ -58,7 +58,7 @@ The Host must bind configuration sections from `appsettings.json` to strongly-ty | `ScadaBridge:HealthMonitoring` | `HealthMonitoringOptions` | Health Monitoring | ReportInterval, OfflineTimeout | | `ScadaBridge:SiteEventLog` | `SiteEventLogOptions` | Site Event Logging | RetentionDays, MaxStorageMb, PurgeScheduleCron | | `ScadaBridge:Communication` | `CommunicationOptions` | Communication | DeploymentTimeout, LifecycleTimeout, QueryTimeout, TransportHeartbeatInterval, TransportFailureThreshold | -| `ScadaBridge:Security` | `SecurityOptions` | Security & Auth | LdapServer, LdapPort, LdapUseTls, JwtSigningKey, JwtExpiryMinutes, IdleTimeoutMinutes | +| `ScadaBridge:Security` | `SecurityOptions` | Security & Auth | LdapServer, LdapPort, LdapUseTls, JwtSigningKey (HS256 key for the CLI/API bearer token from `/auth/token`; >= 32 bytes, validated on Central), JwtExpiryMinutes (bearer lifetime), IdleTimeoutMinutes (cookie idle window), RoleRefreshThresholdMinutes, RoleMappingCacheSeconds | | `ScadaBridge:InboundApi` | `InboundApiOptions` | Inbound API | DefaultMethodTimeout, MaxRequestBodyBytes, ApiMethodCacheTtl, StandbyForwardPeerEndpoints (HTTP base URLs of the other central node — a standby relays `POST /api/*` there instead of answering 503), StandbyForwardTimeout | | `ScadaBridge:Notification` | `NotificationOptions` | Notification Service | (SMTP config is stored in the central config DB, not in appsettings) | | `ScadaBridge:NotificationOutbox` | `NotificationOutboxOptions` | Notification Outbox | Dispatcher poll interval, stuck-age threshold, retention window (delivery retry settings reuse the central SMTP configuration) | diff --git a/docs/requirements/HighLevelReqs.md b/docs/requirements/HighLevelReqs.md index 72bd5b8b..f22331fa 100644 --- a/docs/requirements/HighLevelReqs.md +++ b/docs/requirements/HighLevelReqs.md @@ -437,7 +437,7 @@ Audit logging is implemented as part of the **Configuration Database** component ### 10.1 Storage - Audit logs are stored in the **configuration MS SQL database** alongside system config data, enabling direct querying. -- Entries are **append-only** — never modified or deleted. No retention policy — retained indefinitely. +- Entries are **append-only** — never modified or deleted. Retained indefinitely by default; an optional `ScadaBridge:ConfigAudit:RetentionDays` (default `0` = off) enables a daily batched purge of older entries. ### 10.2 Scope All system-modifying actions are logged, including: