Review remediation: Lane C — Control plane / host / security (fable for C1–C2, opus for C3–C5; review fable) #733

New Issue

2026-10-22T06:05:18-04:00

dohertj2 commented

2026-10-22 06:05:18 -04:00

From the 2026-10-21 Fable system review. Findings R0x:ID refer to docs/reviews/2026-10-21/0x-*.md (location, evidence, failure scenario, fix recipe). Plan: docs/superpowers/plans/2026-10-22-review-remediation.md.

Lane C — Control plane / host / security (fable for C1–C2, opus for C3–C5; review fable)

Owns: src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/**, src/Server/ZB.MOM.WW.OtOpcUa.Host/** (except Host/Engines/**, which belongs to E), src/Server/ZB.MOM.WW.OtOpcUa.Security/**, src/Core/ZB.MOM.WW.OtOpcUa.Configuration/** (except Validation/DraftValidator.cs, which belongs to E).
Tests: ControlPlane.Tests, Host.Tests, Security.Tests, Configuration.Tests (central SQL)

C1 Deploy wedge + consistent snapshot (R03:2.2, 3.2, 3.5):
- In PreStart, a recovered deployment with an empty expected set is re-seeded and re-dispatched (or marked Failed + ResetForNext).
- HandleDeadline takes the race branch only when _expectedAcks.Count > 0.
- The Dispatching row and the expected set are created together in the coordinator.
- Validate + compose from ONE snapshot (extend DraftSnapshot with clusters/nodes/ApiKeys) inside a SNAPSHOT read transaction.
- Project ArtifactBlob away in full-row reads.
- Tests:
  - Inject a HandleDispatch throw; the next StartDeployment succeeds (positive control: a normal deploy seals).
  - An edit between validate and compose cannot ship unvalidated.
C2 Admin singleton mailbox (R03:3.1). Move TestDriverConnect to a per-request child actor (or Task.Run + PipeTo) and run deploy compose off-actor; only the in-flight check is serialized. Test: an alarm ack completes in < 1 s while a 60 s Test Connect is pending.
C3 OPC UA start failure is visible (R03:2.1, 1.4):
- Add an OpcUaEndpointHealthCheck (Ready + Active tags), Unhealthy while the SDK failed to start.
- A driver-only node rethrows (process exits non-zero).
- The "SDK not started" fact feeds the ServiceLevel calculator, so the peer takes Primary. If the calculator lives in a Lane B-owned file (OpcUaServer/**), C only exposes the fact (IOpcUaEndpointState) and files an F<N> follow-up for Wave 3 to consume it.
- StopAsync stops the SDK; DisposeAsync is idempotent.
- Tests: port-in-use ⇒ /health/ready 503, and the ServiceLevel published drops.
C4 Security:
- ApiKeyOptionsValidator + ValidateOnStart (non-blank pepper, minimum length, whenever driver or admin) (R03:2.3).
- Finish JWT (R03:2.4, user decision): AddJwtBearer using BuildValidationParameters(); JwtOptions ValidateOnStart when /auth/token is mapped; the /api/* machine endpoints accept a Bearer token as well as API keys.
- Wire IAuditActorAccessor through AuthenticationStateProvider, so audit rows carry the circuit principal (R03:2.5).
- Delete the unregistered CookieAuthenticationStateProvider.
- LDAP login exception logged at Warning (2.7).
- Admin ops return a fixed message + correlation id (4.1).
C5 Retention + throughput:
- DeploymentRetentionActor on the admin singleton: keep the last N=50 sealed rows per cluster plus anything referenced in the last 30 days; null ArtifactBlob on older rows; configurable Deployments:Retention:* (R03:1.2).
- AuditWriterActor handles duplicate keys per row (2601/2627 ⇒ detach and retry) (2.6).
- MeshGrpcCommandDialer evicts endpoints absent from ContactsLoaded (1.3).
- Async PSK refresh with an atomic swap instead of invalidate-on-refresh (3.4).
- Pre-auth API-key DoS: an in-memory key-id set from the artifact or IDataPlaneAuthoritySource, refreshed every StoreCacheSeconds; an unknown id is rejected without SQL; plus a per-endpoint token bucket (3.3).
- Document the 4.2 fail-open window in docs/security.md.

Lane gate: suites green. Wave 4 docker-dev gate: kill central-1 mid-dispatch, then the next deploy succeeds. Port 4840 taken ⇒ /health/ready 503.

From the 2026-10-21 Fable system review. Findings `R0x:ID` refer to `docs/reviews/2026-10-21/0x-*.md` (location, evidence, failure scenario, fix recipe). Plan: `docs/superpowers/plans/2026-10-22-review-remediation.md`. ### Lane C — Control plane / host / security (fable for C1–C2, opus for C3–C5; review fable) **Owns:** `src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/**`, `src/Server/ZB.MOM.WW.OtOpcUa.Host/**` (except `Host/Engines/**`, which belongs to E), `src/Server/ZB.MOM.WW.OtOpcUa.Security/**`, `src/Core/ZB.MOM.WW.OtOpcUa.Configuration/**` (except `Validation/DraftValidator.cs`, which belongs to E). **Tests:** `ControlPlane.Tests`, `Host.Tests`, `Security.Tests`, `Configuration.Tests` (central SQL) - **C1 Deploy wedge + consistent snapshot** (`R03:2.2`, `3.2`, `3.5`): - In PreStart, a recovered deployment with an empty expected set is re-seeded and re-dispatched (or marked `Failed` + `ResetForNext`). - `HandleDeadline` takes the race branch only when `_expectedAcks.Count > 0`. - The `Dispatching` row and the expected set are created together in the coordinator. - Validate + compose from ONE snapshot (extend `DraftSnapshot` with clusters/nodes/ApiKeys) inside a `SNAPSHOT` read transaction. - Project `ArtifactBlob` away in full-row reads. - Tests: - Inject a `HandleDispatch` throw; the next `StartDeployment` succeeds (positive control: a normal deploy seals). - An edit between validate and compose cannot ship unvalidated. - **C2 Admin singleton mailbox** (`R03:3.1`). Move `TestDriverConnect` to a per-request child actor (or `Task.Run` + `PipeTo`) and run deploy compose off-actor; only the in-flight check is serialized. Test: an alarm ack completes in < 1 s while a 60 s Test Connect is pending. - **C3 OPC UA start failure is visible** (`R03:2.1`, `1.4`): - Add an `OpcUaEndpointHealthCheck` (Ready + Active tags), Unhealthy while the SDK failed to start. - A driver-only node rethrows (process exits non-zero). - The "SDK not started" fact feeds the ServiceLevel calculator, so the peer takes Primary. If the calculator lives in a Lane B-owned file (`OpcUaServer/**`), C only exposes the fact (`IOpcUaEndpointState`) and files an `F<N>` follow-up for Wave 3 to consume it. - `StopAsync` stops the SDK; `DisposeAsync` is idempotent. - Tests: port-in-use ⇒ `/health/ready` 503, and the ServiceLevel published drops. - **C4 Security:** - `ApiKeyOptionsValidator` + `ValidateOnStart` (non-blank pepper, minimum length, whenever driver or admin) (`R03:2.3`). - **Finish JWT** (`R03:2.4`, user decision): `AddJwtBearer` using `BuildValidationParameters()`; `JwtOptions` `ValidateOnStart` when `/auth/token` is mapped; the `/api/*` machine endpoints accept a Bearer token as well as API keys. - Wire `IAuditActorAccessor` through `AuthenticationStateProvider`, so audit rows carry the circuit principal (`R03:2.5`). - Delete the unregistered `CookieAuthenticationStateProvider`. - LDAP login exception logged at Warning (`2.7`). - Admin ops return a fixed message + correlation id (`4.1`). - **C5 Retention + throughput:** - `DeploymentRetentionActor` on the admin singleton: keep the last N=50 sealed rows per cluster plus anything referenced in the last 30 days; null `ArtifactBlob` on older rows; configurable `Deployments:Retention:*` (`R03:1.2`). - `AuditWriterActor` handles duplicate keys per row (2601/2627 ⇒ detach and retry) (`2.6`). - `MeshGrpcCommandDialer` evicts endpoints absent from `ContactsLoaded` (`1.3`). - Async PSK refresh with an atomic swap instead of invalidate-on-refresh (`3.4`). - Pre-auth API-key DoS: an in-memory key-id set from the artifact or `IDataPlaneAuthoritySource`, refreshed every `StoreCacheSeconds`; an unknown id is rejected without SQL; plus a per-endpoint token bucket (`3.3`). - Document the `4.2` fail-open window in `docs/security.md`. **Lane gate:** suites green. Wave 4 docker-dev gate: kill central-1 mid-dispatch, then the next deploy succeeds. Port 4840 taken ⇒ `/health/ready` 503.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: dohertj2/lmxopcua#733