From 09108018f19cb44da76f72082078240d5ed08c56 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sat, 15 Aug 2026 11:38:28 -0400 Subject: [PATCH 1/2] =?UTF-8?q?fix(management):=20audit-config=20query=20r?= =?UTF-8?q?eturned=20{}=20for=20every=20filter=20=E2=80=94=20project=20the?= =?UTF-8?q?=20repo=20ValueTuple=20into=20a=20named=20result=20record?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deferred-work register row 46. HandleQueryAuditLog returned ICentralUiRepository's (IReadOnlyList Entries, int TotalCount) ValueTuple straight to ManagementActor.SerializeResult. System.Text.Json writes properties only, and a ValueTuple exposes Item1/Item2 as public FIELDS, so the whole payload serialized to the literal "{}" — the CLI faithfully printed an empty object no matter how many AuditLogEntries rows matched, with no error anywhere in the stack. Fixed at the handler (the layer that leaks the tuple), not the CLI: new Commons record AuditLogQueryResult(Entries, TotalCount), mirroring the SecuredWriteListResult shape already used by the other paged management query. Also guards the whole class of bug: SerializeResult now throws on any ITuple result, so the next handler that returns a tuple fails loudly instead of silently emitting {}. Tests (fail-before verified — the query test asserted exactly "{}"): - QueryAuditLogCommand_ReturnsEntriesAndTotalCount - SerializeResult_TupleResult_ThrowsInsteadOfEmittingEmptyObject - SerializeResult_AuditLogQueryResult_EmitsEntriesAndTotalCount Docs: Component-CLI.md + CLI README document the response shape and why it is a record. --- docs/requirements/Component-CLI.md | 7 ++ src/ZB.MOM.WW.ScadaBridge.CLI/README.md | 22 ++++++ .../Messages/Management/AuditLogCommands.cs | 19 +++++ .../ManagementActor.cs | 43 +++++++++- .../ManagementActorTests.cs | 79 +++++++++++++++++++ 5 files changed, 167 insertions(+), 3 deletions(-) diff --git a/docs/requirements/Component-CLI.md b/docs/requirements/Component-CLI.md index 77cdf04f..1b209303 100644 --- a/docs/requirements/Component-CLI.md +++ b/docs/requirements/Component-CLI.md @@ -241,6 +241,13 @@ warning; migrate scripts to `audit-config`. scadabridge audit-config query [--user ] [--entity-type ] [--action ] [--from ] [--to ] [--page ] [--page-size ] ``` +The response is an `AuditLogQueryResult` — `{ "entries": [ … ], "totalCount": }`, where +`totalCount` is the unpaged match count. The result type is a named record specifically +because the handler must not return the repository's `(Entries, TotalCount)` ValueTuple: +`System.Text.Json` serializes properties only, so a tuple result renders as the literal +`{}` and every query looks empty regardless of how many rows matched. `ManagementActor.SerializeResult` +now rejects tuple results outright so the failure mode cannot recur silently. + The new centralized Audit Log component (#23) is exposed via the `scadabridge audit` group below. ### Centralized Audit Commands diff --git a/src/ZB.MOM.WW.ScadaBridge.CLI/README.md b/src/ZB.MOM.WW.ScadaBridge.CLI/README.md index c4d7f802..0f94170a 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CLI/README.md +++ b/src/ZB.MOM.WW.ScadaBridge.CLI/README.md @@ -1802,6 +1802,28 @@ scadabridge --url audit-config query [options] | `--page` | no | `1` | Page number | | `--page-size` | no | `50` | Results per page | +Output shape: + +```json +{ + "entries": [ + { + "id": 412, + "user": "multi-role", + "action": "Created", + "entityType": "Template", + "entityId": "7", + "entityName": "PumpTemplate", + "timestamp": "2026-08-15T12:00:00+00:00" + } + ], + "totalCount": 137 +} +``` + +`totalCount` is the unpaged number of rows matching the filters, so a client can page +without a second call. + --- ### `shared-script` — Manage shared scripts diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/AuditLogCommands.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/AuditLogCommands.cs index 402dbe25..2d00ca7c 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/AuditLogCommands.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/AuditLogCommands.cs @@ -1,6 +1,25 @@ +using ZB.MOM.WW.ScadaBridge.Commons.Entities.Audit; + namespace ZB.MOM.WW.ScadaBridge.Commons.Messages.Management; public record QueryAuditLogCommand( string? User = null, string? EntityType = null, string? Action = null, DateTimeOffset? From = null, DateTimeOffset? To = null, int Page = 1, int PageSize = 50); + +/// +/// Result wrapper for . +/// +/// +/// This record exists specifically so the handler does not return the repository's +/// (IReadOnlyList<AuditLogEntry> Entries, int TotalCount) ValueTuple directly. +/// A ValueTuple exposes Item1/Item2 as public fields, and +/// serializes only properties unless +/// JsonSerializerOptions.IncludeFields is set — so a tuple result serialized to +/// the literal string {} and every audit-config query looked empty no +/// matter how many rows the query matched (deferred-work register row 46). Mirrors the +/// SecuredWriteListResult shape used by the other paged management query. +/// +/// The page of configuration-change audit entries. +/// Total number of rows matching the filters (unpaged). +public record AuditLogQueryResult(IReadOnlyList Entries, int TotalCount); diff --git a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs index 5a6c8f66..8c3bc658 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs @@ -90,10 +90,35 @@ public class ManagementActor : ReceiveActor /// Serializes a command result to JSON using — the same /// serializer the HTTP endpoint uses — with cycle-safe settings. /// + /// + /// Rejects tuple results outright. writes only + /// properties by default, and a ValueTuple exposes its members as public + /// fields — so a handler that returns a repository's + /// (rows, totalCount) tuple serializes to the literal {}, silently + /// handing every caller an empty result with no error anywhere in the stack. That is + /// exactly how audit-config query shipped broken (deferred-work register row + /// 46). Failing loudly here converts the whole class of bug into an immediate, + /// obvious error the first time the handler is exercised; the fix is always to + /// project the tuple into a named result record before returning it. + /// /// The command result object to serialize, or null. /// The JSON string representation of , or "null" when null. - public static string SerializeResult(object? result) => - JsonSerializer.Serialize(result, ResultSerializerOptions); + /// + /// Thrown when is a tuple, which cannot round-trip through + /// the JSON contract. + /// + public static string SerializeResult(object? result) + { + if (result is System.Runtime.CompilerServices.ITuple) + { + throw new InvalidOperationException( + $"Management command results must be named types, not tuples — " + + $"System.Text.Json serializes '{result.GetType().Name}' to '{{}}' because tuple " + + "members are fields, not properties. Project the tuple into a result record."); + } + + return JsonSerializer.Serialize(result, ResultSerializerOptions); + } private void HandleEnvelope(ManagementEnvelope envelope) { @@ -2578,12 +2603,24 @@ public class ManagementActor : ReceiveActor // Audit Log handler // ======================================================================== + /// + /// Queries the configuration-change audit trail and projects the repository's + /// ValueTuple into . + /// + /// + /// The projection is load-bearing, not cosmetic: returning the repository tuple + /// directly made emit the literal {} (ValueTuple + /// exposes Item1/Item2 as fields, which System.Text.Json skips), so + /// audit-config query reported an empty result for every filter combination + /// regardless of how many rows matched — deferred-work register row 46. + /// private static async Task HandleQueryAuditLog(IServiceProvider sp, QueryAuditLogCommand cmd) { var repo = sp.GetRequiredService(); - return await repo.GetAuditLogEntriesAsync( + var (entries, totalCount) = await repo.GetAuditLogEntriesAsync( cmd.User, cmd.EntityType, cmd.Action, cmd.From, cmd.To, page: cmd.Page, pageSize: cmd.PageSize); + return new AuditLogQueryResult(entries, totalCount); } // ======================================================================== diff --git a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs index 84ff8003..4406edd3 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs @@ -238,6 +238,85 @@ public class ManagementActorTests : TestKit, IDisposable Assert.Equal(envelope.CorrelationId, response.CorrelationId); } + // ======================================================================== + // 2b. Configuration-change audit query -- result must not serialize to "{}" + // ======================================================================== + + /// + /// Deferred-work register row 46: audit-config query returned the literal + /// {} for every filter combination against the rig even though the + /// AuditLogEntries rows were present. The handler returned the repository's + /// (Entries, TotalCount) ValueTuple straight to SerializeResult, and + /// System.Text.Json writes properties only — a tuple's Item1/Item2 are + /// fields, so the whole payload vanished with no error anywhere. Pins that the rows + /// and the total actually reach the wire. + /// + [Fact] + public void QueryAuditLogCommand_ReturnsEntriesAndTotalCount() + { + var uiRepo = Substitute.For(); + var entries = new List + { + new("alice", "Created", "Template", "7", "PumpTemplate") + { + Id = 1, + Timestamp = new DateTimeOffset(2026, 8, 15, 12, 0, 0, TimeSpan.Zero) + } + }; + uiRepo.GetAuditLogEntriesAsync( + Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any(), Arg.Any(), + Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(((IReadOnlyList)entries, 137)); + _services.AddScoped(_ => uiRepo); + + var actor = CreateActor(); + var envelope = Envelope(new QueryAuditLogCommand(PageSize: 25), "Administrator"); + + actor.Tell(envelope); + + var response = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.NotEqual("{}", response.JsonData); + Assert.Contains("PumpTemplate", response.JsonData); + Assert.Contains("\"entries\"", response.JsonData); + Assert.Contains("\"totalCount\":137", response.JsonData); + } + + /// + /// Guard for the whole class of bug behind row 46: any handler that returns a tuple + /// silently serializes to {}. + /// now rejects tuples outright so the next occurrence is a loud failure rather than + /// an empty response. + /// + [Fact] + public void SerializeResult_TupleResult_ThrowsInsteadOfEmittingEmptyObject() + { + var ex = Assert.Throws( + () => ManagementActor.SerializeResult((new[] { "a" }, 1))); + + Assert.Contains("tuple", ex.Message, StringComparison.OrdinalIgnoreCase); + } + + /// + /// Pins the positive half of the guard: the named result record the handler now + /// returns serializes to a populated, camelCased object. + /// + [Fact] + public void SerializeResult_AuditLogQueryResult_EmitsEntriesAndTotalCount() + { + var json = ManagementActor.SerializeResult(new AuditLogQueryResult( + new List + { + new("bob", "Deleted", "Instance", "3", "Pump1") + }, + 42)); + + Assert.Contains("\"entries\"", json); + Assert.Contains("\"totalCount\":42", json); + Assert.Contains("Pump1", json); + } + // ======================================================================== // 3. Template list test -- verify data flows through // ======================================================================== From b5e6821caa4ba4f302a41c30e4539dbfccd33777 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sat, 15 Aug 2026 11:46:57 -0400 Subject: [PATCH 2/2] chore(docker): seed script for a deployable live-data probe instance on the rig MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deferred-work register row 48. The rig had no deployable instance — all seven soak-* rows fail pre-deployment validation (their bound data connections no longer exist on the sites) — so debug snapshot returned instanceNotFound, no gRPC site stream was ever opened, and the R2 batching histogram scadabridge.site.stream.batch_size had nothing to record. Any live probe of the data or streaming path was unverifiable. seed-live-probe.sh rebuilds the minimum working set via the CLI only (no DB edits): RigProbe template (Spike <- ns=3;s=SpikeData, RandomInt <- ns=3;s=RandomSignedInt32, both ~1 Hz; Heartbeat static attribute + a trust-clean 5s interval script) and a deployed rig-probe-1 on site-a against connection 1. Records the two traps that cost time here: - The custom tags in infra/opcua/nodes.json (Motor.*, Tank.*, Valve.*, TestChildObject.*) are DECLARED but not simulated by opc-plc — they read 0/false/ epoch forever, so binding to them yields a Connected, Good-quality connection with zero event traffic. Only the built-in nodes move. - scadabridge_site_connection_up counts open central->site gRPC SUBSCRIBER STREAMS, not OPC UA data connections. It reads 0 on a healthy site with no subscriber, which is how row 48 read it as a data-connection fault. Data-connection health lives in the health report's dataConnectionEndpoints / dataConnectionTagQuality. --- docker/README.md | 36 ++++++++++++ docker/seed-live-probe.sh | 120 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 156 insertions(+) create mode 100755 docker/seed-live-probe.sh diff --git a/docker/README.md b/docker/README.md index 0a6d89fd..c59d0d8f 100644 --- a/docker/README.md +++ b/docker/README.md @@ -87,6 +87,7 @@ docker/ ├── build.sh # Build Docker image ├── deploy.sh # Build + deploy all containers ├── seed-sites.sh # Create test sites with Akka + gRPC addresses +├── seed-live-probe.sh # Seed one deployable live-data instance on site-a (stream traffic) ├── teardown.sh # Stop and remove containers ├── central-node-a/ │ ├── appsettings.Central.json # Central node A configuration @@ -160,6 +161,41 @@ docker/deploy.sh # 3. Seed test sites (first-time only, after cluster is healthy) docker/seed-sites.sh + +# 4. Seed one deployable live-data instance on site-a, so the rig has real +# stream traffic for live probes (see "Live-data probe instance" below) +docker/seed-live-probe.sh +``` + +### Live-data probe instance + +`seed-live-probe.sh` creates the minimum working set the rig needs for any live probe +of the data or streaming path: a `RigProbe` template with two attributes bound to +**self-changing** opc-plc nodes (`ns=3;s=SpikeData`, `ns=3;s=RandomSignedInt32`, both +~1 Hz) plus a 5 s heartbeat script, and a deployed `rig-probe-1` instance on site-a. + +Two traps it exists to avoid: + +- **The custom tags in `infra/opcua/nodes.json` are declared but not simulated.** + `Motor.*`, `Tank.*`, `Valve.*`, `TestChildObject.*` read 0 / false / epoch forever. + Binding to them gives a `Connected` data connection with `Good` quality and *zero* + event traffic, which looks healthy and probes nothing. Use the built-in simulated + nodes (`SpikeData`, `RandomSignedInt32`, `StepUp`, `FastUInt1..5`, `SlowUInt1..5`). +- **`scadabridge_site_connection_up` does not mean what its name suggests.** It counts + open central→site gRPC *subscriber streams*, not OPC UA data connections, so it sits + at 0 on a perfectly healthy site with nobody watching. For data-connection health read + `dataConnectionEndpoints` / `dataConnectionTagQuality` off `scadabridge health site`. + +Verify (metrics are on container port 8084; the containers ship no curl/wget): + +```bash +scadabridge debug snapshot --id +scadabridge health site --identifier site-a +scadabridge debug stream --id # open a stream, then in another shell: + +docker exec scadabridge-site-a-a bash -c \ + 'exec 3<>/dev/tcp/localhost/8084 && printf "GET /metrics HTTP/1.0\r\nHost: localhost\r\n\r\n" >&3 && cat <&3' \ + | grep -E 'site_connection_up|stream_batch_size' ``` ### After Code Changes diff --git a/docker/seed-live-probe.sh b/docker/seed-live-probe.sh new file mode 100755 index 00000000..54e738a9 --- /dev/null +++ b/docker/seed-live-probe.sh @@ -0,0 +1,120 @@ +#!/bin/bash +set -euo pipefail + +# Seed ONE deployable, live-data instance on site-a so the rig always has real +# stream traffic for live probes. +# +# Why this exists: the rig repeatedly ends up with instances that cannot deploy +# (deferred-work register row 48 — the seven `soak-*` rows fail pre-deployment +# validation because their bound data connections no longer exist on the sites). +# With nothing deployed, `debug snapshot` returns `instanceNotFound`, no gRPC site +# stream is ever opened, and the R2 batching histogram +# `scadabridge.site.stream.batch_size` has nothing to record — so any live probe of +# the streaming path is unverifiable. This script rebuilds the minimum working set +# from scratch, using only the CLI (no DB edits). +# +# What it creates (all on site-a / site id 1, against data connection id 1 +# "OPC PLC Simulator" -> opc.tcp://scadabridge-opcua:50000): +# - Template `RigProbe` +# * Spike (Double) <- ns=3;s=SpikeData — opc-plc built-in, changes ~1/s +# * RandomInt (Int32) <- ns=3;s=RandomSignedInt32 — opc-plc built-in, changes ~1/s +# * Heartbeat (Int32) — static, bumped every 5s by the Heartbeat script +# * Heartbeat script — Interval 5000ms, trust-policy-clean +# - Instance `rig-probe-1`, bound + deployed +# +# IMPORTANT — pick self-changing tags. The custom tags in `infra/opcua/nodes.json` +# (Motor.*, Tank.*, Valve.*, TestChildObject.*) are declared but NOT simulated by +# opc-plc: they read 0 / false / epoch and never change on their own, so binding to +# them yields a healthy connection with zero event traffic. The built-in simulated +# nodes are the ones that move: SpikeData, RandomSignedInt32, StepUp, FastUInt1..5 +# (1s) and SlowUInt1..5 (10s). +# +# Prerequisites: +# - Infrastructure services running (infra/docker-compose.yml — needs scadabridge-opcua) +# - Application containers running (docker/deploy.sh) +# - Sites seeded (docker/seed-sites.sh) and central healthy +# curl -sf http://localhost:9000/health/ready +# +# Usage: +# docker/seed-live-probe.sh +# +# Idempotency: this script is NOT idempotent — template and instance names must be +# unique, so a second run fails on the create steps. To re-seed, delete the previous +# instance and template first: +# scadabridge instance delete --id && scadabridge template delete --id + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +PROJECT_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" +CLI="dotnet run --project $PROJECT_ROOT/src/ZB.MOM.WW.ScadaBridge.CLI --" +AUTH="--username multi-role --password password" +URL="--url http://localhost:9000" + +SITE_ID=1 # site-a +CONNECTION_ID=1 # "OPC PLC Simulator" on site-a +TEMPLATE_NAME="RigProbe" +INSTANCE_NAME="rig-probe-1" + +# Pulls the numeric "id" out of the CLI's JSON reply without needing jq. +json_id() { python3 -c 'import json,sys; print(json.load(sys.stdin)["id"])'; } + +echo "=== Seeding live-data probe instance on site-a ===" + +echo "" +echo "1/6 Creating template $TEMPLATE_NAME..." +TEMPLATE_ID=$($CLI $URL $AUTH template create \ + --name "$TEMPLATE_NAME" \ + --description "Minimal live-traffic probe: two OPC UA sim tags that change on their own + a 5s heartbeat script" \ + | json_id) +echo " template id = $TEMPLATE_ID" + +echo "" +echo "2/6 Adding data-bound attributes..." +$CLI $URL $AUTH template attribute add \ + --template-id "$TEMPLATE_ID" --name Spike --data-type Double \ + --data-source "ns=3;s=SpikeData" \ + --description "opc-plc SpikeData — Double, changes every ~1s on its own" > /dev/null +$CLI $URL $AUTH template attribute add \ + --template-id "$TEMPLATE_ID" --name RandomInt --data-type Int32 \ + --data-source "ns=3;s=RandomSignedInt32" \ + --description "opc-plc RandomSignedInt32 — Int32, changes every ~1s on its own" > /dev/null + +echo "" +echo "3/6 Adding the static heartbeat attribute + its interval script..." +$CLI $URL $AUTH template attribute add \ + --template-id "$TEMPLATE_ID" --name Heartbeat --data-type Int32 --value 0 \ + --description "Static attribute bumped every 5s by the Heartbeat script — script-sourced traffic" > /dev/null +$CLI $URL $AUTH template script add \ + --template-id "$TEMPLATE_ID" --name Heartbeat \ + --trigger-type Interval --trigger-config '{"intervalMs":5000}' \ + --code 'var current = await Instance.GetAttribute("Heartbeat"); var next = (int.TryParse(current?.ToString(), out var n) ? n : 0) + 1; await Instance.SetAttribute("Heartbeat", next.ToString());' > /dev/null + +echo "" +echo "4/6 Creating instance $INSTANCE_NAME..." +INSTANCE_ID=$($CLI $URL $AUTH instance create \ + --name "$INSTANCE_NAME" --template-id "$TEMPLATE_ID" --site-id "$SITE_ID" | json_id) +echo " instance id = $INSTANCE_ID" + +echo "" +echo "5/6 Binding the data-sourced attributes to connection $CONNECTION_ID..." +$CLI $URL $AUTH instance set-bindings --id "$INSTANCE_ID" \ + --bindings "[[\"Spike\",$CONNECTION_ID],[\"RandomInt\",$CONNECTION_ID]]" > /dev/null + +echo "" +echo "6/6 Deploying..." +$CLI $URL $AUTH instance deploy --id "$INSTANCE_ID" + +echo "" +echo "=== Done. Verify with: ===" +echo " scadabridge debug snapshot --id $INSTANCE_ID" +echo " scadabridge health site --identifier site-a # dataConnectionEndpoints / dataConnectionTagQuality" +echo " scadabridge debug stream --id $INSTANCE_ID # opens a gRPC site stream; ~2 events/s" +echo "" +echo "While a stream is open, the site node serving it reports both signals" +echo "(metrics are on container port 8084, and the containers have no curl/wget):" +echo " docker exec scadabridge-site-a-a bash -c 'exec 3<>/dev/tcp/localhost/8084 && printf \"GET /metrics HTTP/1.0\\r\\nHost: localhost\\r\\n\\r\\n\" >&3 && cat <&3' \\" +echo " | grep -E 'site_connection_up|stream_batch_size'" +echo "" +echo "NOTE: scadabridge_site_connection_up counts OPEN CENTRAL->SITE gRPC SUBSCRIBER" +echo "STREAMS, not OPC UA data connections. It reads 0 on a perfectly healthy site" +echo "with no subscriber attached — use the health report's dataConnectionEndpoints" +echo "for data-connection health."