Gate findings: deploy TimedOut has null failureReason; quarantine logged as 'unknown' and shown as BadInternalError; rig PSK 'REVOKED' warning vs working dispatch #784

New Issue

2026-11-09T21:10:08-05:00

dohertj2 commented

2026-11-09 21:10:08 -05:00

Source: docs/reviews/2026-10-21/gates.md (master 7891b3cb), observations section — found running the #761 docker-dev gates on 1b9c36f3.

Deploy timeout gives no reason over the API. In the normal timeout branch (some acks landed), GET /api/deployments/{id} returns status: TimedOut with failureReason: null. Only the central log names the node that didn't answer (No ack from: site-b-1:4053). The no-expected-rows branch does set a reason. Set failureReason to the list of nodes that never acked.
Script quarantine is hard to see.
- The main-log mirror says [Script] unknown emitted Error: Script "…" quarantined. The guard's logger carries no ScriptName/ScriptId, so ScriptLogCompanionSink labels it unknown and rate-limits it under one shared key. scripts-*.log has the correct line.
- On OPC UA a quarantined tag reads only BadInternalError. Consider a quarantine-specific status, e.g. BadOutOfService or a dedicated substatus, and document it.
Rig mesh-PSK warning contradicts behaviour. The docker-dev centrals log Mesh PSK secret ot-mesh-psk-{main,site-a,site-b} … absent … key is REVOKED — commands to that cluster and acks from it are refused, yet every dispatch and ack succeeded. Either enforcement is off on the rig while the warning claims otherwise, or the warning is stale. Find out which, and make the log tell the truth. This one may matter for production: if a revoked key really doesn't refuse commands, that is a security bug.

Source: docs/reviews/2026-10-21/gates.md (master 7891b3cb), observations section — found running the #761 docker-dev gates on 1b9c36f3. 1. **Deploy timeout gives no reason over the API.** In the normal timeout branch (some acks landed), `GET /api/deployments/{id}` returns `status: TimedOut` with `failureReason: null`. Only the central log names the node that didn't answer (`No ack from: site-b-1:4053`). The no-expected-rows branch does set a reason. Set `failureReason` to the list of nodes that never acked. 2. **Script quarantine is hard to see.** - The main-log mirror says `[Script] unknown emitted Error: Script "…" quarantined`. The guard's logger carries no ScriptName/ScriptId, so `ScriptLogCompanionSink` labels it `unknown` and rate-limits it under one shared key. `scripts-*.log` has the correct line. - On OPC UA a quarantined tag reads only `BadInternalError`. Consider a quarantine-specific status, e.g. `BadOutOfService` or a dedicated substatus, and document it. 3. **Rig mesh-PSK warning contradicts behaviour.** The docker-dev centrals log `Mesh PSK secret ot-mesh-psk-{main,site-a,site-b} … absent … key is REVOKED — commands to that cluster and acks from it are refused`, yet every dispatch and ack succeeded. Either enforcement is off on the rig while the warning claims otherwise, or the warning is stale. Find out which, and make the log tell the truth. This one may matter for production: if a revoked key really doesn't refuse commands, that is a security bug.

dohertj2 added the type:bug priority:p2 labels 2026-11-09 21:10:08 -05:00

dohertj2 referenced this issue

2026-11-09 21:10:24 -05:00

Review remediation 2026-10-21: run the Wave-4 integration gates (lanes A–G, H1, drivers) #761

dohertj2 commented

2026-11-09 21:39:30 -05:00

Fixed on master (3 commits, rebased onto #783):

Rig PSK warning — false alarm, no auth bypass. MeshPskProvider.TryGetPsk checks the config map MeshTransport:ClusterPsks first on every call, and CentralCommandAuthInterceptor / MeshGrpcCommandDialer both go through it with a constant-time Matches. docker-dev sets MeshTransport__ClusterPsks__{MAIN,SITE-A,SITE-B} on the centrals and seeds nothing in the store. I compared value hashes in the running containers: each central's override equals every member node's MeshTransport__GrpcPsk, so the keys were present and enforced. The warning came from RefreshAsync reading the store for override-keyed clusters too. It now skips them (Debug line: override in use), and 'REVOKED' is logged only for a cluster with no key anywhere. New test: the overridden cluster isn't read or warned about and a wrong token is rejected; an un-overridden cluster is still warned about and refused. Removing the skip fails it.
Deploy timeout reason. ConfigPublishCoordinator.HandleDeadline now saves 'Timed out waiting for acks (n/m landed). No ack from: …' (at most 20 named, then '(and K more)', ≤2048 chars), returned by GET /api/deployments/{id}. Two tests, both mutation-checked.
Quarantine observability. ScriptRunGuard.Quarantine attaches ScriptName, so the main-log mirror names the script and rate-limits per script (tested through the real sink). Kept BadInternalError on OPC UA: it's the documented script-fault code across virtual tags, Calculation and scripted alarms; BadOutOfService means a point someone deliberately disabled; and a new code would be a ScadaBridge-visible contract change that the once-per-Good→Bad dedup would rarely deliver anyway. Rationale in docs/VirtualTags.md 'Quarantine: what a client sees'.
Evidence: Release 0 warnings; agent run Core.Abstractions 549, Core.ScriptedAlarms 106, Core.Scripting 221, Calculation 79, ControlPlane 324, Runtime 982, Host 247, OpcUaServer 1026, IntegrationTests 132. After rebasing onto #783: ControlPlane 324, Configuration 219, IntegrationTests 132, Runtime 988 (one unnamed failure on the first pass did not reproduce on rerun; tracked under #769).

Fixed on master (3 commits, rebased onto #783): 1. **Rig PSK warning — false alarm, no auth bypass.** `MeshPskProvider.TryGetPsk` checks the config map `MeshTransport:ClusterPsks` first on every call, and `CentralCommandAuthInterceptor` / `MeshGrpcCommandDialer` both go through it with a constant-time `Matches`. docker-dev sets `MeshTransport__ClusterPsks__{MAIN,SITE-A,SITE-B}` on the centrals and seeds nothing in the store. I compared value hashes in the running containers: each central's override equals every member node's `MeshTransport__GrpcPsk`, so the keys were present and enforced. The warning came from `RefreshAsync` reading the store for override-keyed clusters too. It now skips them (Debug line: override in use), and 'REVOKED' is logged only for a cluster with no key anywhere. New test: the overridden cluster isn't read or warned about and a wrong token is rejected; an un-overridden cluster is still warned about and refused. Removing the skip fails it. 2. **Deploy timeout reason.** `ConfigPublishCoordinator.HandleDeadline` now saves 'Timed out waiting for acks (n/m landed). No ack from: <nodes>…' (at most 20 named, then '(and K more)', ≤2048 chars), returned by `GET /api/deployments/{id}`. Two tests, both mutation-checked. 3. **Quarantine observability.** `ScriptRunGuard.Quarantine` attaches `ScriptName`, so the main-log mirror names the script and rate-limits per script (tested through the real sink). **Kept `BadInternalError`** on OPC UA: it's the documented script-fault code across virtual tags, Calculation and scripted alarms; `BadOutOfService` means a point someone deliberately disabled; and a new code would be a ScadaBridge-visible contract change that the once-per-Good→Bad dedup would rarely deliver anyway. Rationale in docs/VirtualTags.md 'Quarantine: what a client sees'. Evidence: Release 0 warnings; agent run Core.Abstractions 549, Core.ScriptedAlarms 106, Core.Scripting 221, Calculation 79, ControlPlane 324, Runtime 982, Host 247, OpcUaServer 1026, IntegrationTests 132. After rebasing onto #783: ControlPlane 324, Configuration 219, IntegrationTests 132, Runtime 988 (one unnamed failure on the first pass did not reproduce on rerun; tracked under #769).

dohertj2 closed this issue

2026-11-09 21:39:31 -05:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: dohertj2/lmxopcua#784