fix(dashboard): redact mirror tag values when ShowTagValues off; trim value-log doc (SEC-25, SEC-30)

SEC-25 (near-term hardening; full per-session EventsHub ACL stays deferred to
roadmap item 12): DashboardEventBroadcaster now redacts tag values from a deep
CLONE of the event before mirroring to SignalR when Dashboard:ShowTagValues is
false (the default). Clears MxEvent.value (covers OnDataChange/OnWriteComplete/
OperationComplete/OnBufferedDataChange — their bodies are empty discriminators,
values ride in the top-level field incl. buffered arrays) and the alarm body's
current_value/limit_value. Source event never mutated (shared with the gRPC
path + replay ring) - verified by a source-not-mutated test. Makes the formerly
dead ShowTagValues flag live for the mirror. EventsHub TODO(per-session-acl)
kept and tied to roadmap item 12. Tests: DashboardEventBroadcasterTests (3).

SEC-30: trim docs/Diagnostics.md to mark opt-in command-value logging as NOT
YET IMPLEMENTED (no LogCommandValues knob, RedactCommandValue has no call
sites, no values logged); wiring deferred pending secured-bulk redaction
coverage (SEC-13). No option/call sites added.

Server build clean (0 warnings); Dashboard tests 152/152.

Claude-Session: https://claude.ai/code/session_01DMXXvNuPekkkrTEyPNxEkW
This commit is contained in:
Joseph Doherty
2026-07-09 15:28:24 -04:00
parent 96702321b1
commit e15c3cb052
5 changed files with 230 additions and 13 deletions
+3 -1
View File
@@ -123,7 +123,9 @@ The split uses `count: 3` because the secret portion may itself contain undersco
### Command value redaction
`RedactCommandValue` enforces the "values are opt-in and redacted by default" rule:
> **Not yet implemented.** Command-value logging is *not* wired end-to-end. There is no `MxGateway:Diagnostics:LogCommandValues` (or equivalent) configuration knob, and `RedactCommandValue` / `IsCredentialBearingCommand` have no call sites in the gateway — no command values are logged anywhere today, which is the safest posture. The helpers below exist as the intended redaction seam for a future opt-in value-logging feature; that wiring is deferred until secured-bulk command variants are covered by the redactor's credential list (the `WriteSecuredBulk` / `WriteSecured2Bulk` gap), so enabling value logging cannot leak a secured-bulk payload. Until then, treat this section as describing the planned shape, not current behavior.
The intended `RedactCommandValue` would enforce the "values are opt-in and redacted by default" rule:
```csharp
public static object? RedactCommandValue(
+4 -3
View File
@@ -167,7 +167,7 @@ bearer). Each hub class is `[Authorize(Policy = HubClientsPolicy)]`.
|---|---|---|---|---|
| `DashboardSnapshotHub` | `/hubs/snapshot` | `DashboardSnapshotPublisher` (BackgroundService consuming `IDashboardSnapshotService.WatchSnapshotsAsync`) | `DashboardSnapshot` | Sent to all connected clients on every snapshot tick; new connections receive the current snapshot synchronously in `OnConnectedAsync`. |
| `AlarmsHub` | `/hubs/alarms` | `AlarmsHubPublisher` (BackgroundService consuming `IGatewayAlarmService.StreamAsync(filter: null)`) | `AlarmFeedMessage` (`active_alarm` / `snapshot_complete` / `transition`) | Connected clients auto-join `__alarms__`; all clients receive every message. Publisher auto-reconnects every 5s on stream faults. |
| `EventsHub` | `/hubs/events` | `DashboardEventBroadcaster` invoked by each session's internal dashboard-mirror subscriber on its `SessionEventDistributor` (registered when the session becomes Ready) | `MxEvent` | Clients call `SubscribeSession(sessionId)` to join `session:{id}`. The dashboard is a first-class distributor subscriber, so it receives the session's events whether or not a gRPC client is streaming. It sees RAW session events — not the per-gRPC-subscriber `AfterWorkerSequence` filtering that `EventStreamService` applies at its own boundary — because the dashboard is a separate LDAP-authenticated monitoring view meant to show the session's full event activity (per-session dashboard ACL is tracked separately). |
| `EventsHub` | `/hubs/events` | `DashboardEventBroadcaster` invoked by each session's internal dashboard-mirror subscriber on its `SessionEventDistributor` (registered when the session becomes Ready) | `MxEvent` | Clients call `SubscribeSession(sessionId)` to join `session:{id}`. The dashboard is a first-class distributor subscriber, so it receives the session's events whether or not a gRPC client is streaming. It sees RAW session events — not the per-gRPC-subscriber `AfterWorkerSequence` filtering that `EventStreamService` applies at its own boundary — because the dashboard is a separate LDAP-authenticated monitoring view meant to show the session's full event activity. Tag values are stripped from the mirrored `MxEvent` copy by `DashboardEventBroadcaster` when `Dashboard:ShowTagValues` is false (the default) — event metadata (tag reference, quality, status, timestamps) still renders, but the value fields are blanked, so no value leaks through this seam. The per-session hub ACL that would scope a Viewer to specific sessions is still outstanding (SEC-25 / remediation roadmap item 12); the value redaction is the near-term hardening that closes the value-leak seam independently of that ACL. |
`DashboardPageBase` opens a `DashboardSnapshotHub` connection via the connection
factory in `OnInitializedAsync`, seeds `Snapshot` synchronously from
@@ -311,8 +311,9 @@ Right-clicking an attribute (or double-clicking it) adds it to the subscription
panel. The panel shows each subscribed tag's live value, MXAccess data type,
quality and source timestamp, refreshed every two seconds. The subscription
panel is the explicit opt-in tag-value surface: it always shows values
regardless of `Dashboard:ShowTagValues`, which continues to govern only the
diagnostic session/worker views.
regardless of `Dashboard:ShowTagValues`, which governs the diagnostic
session/worker views and the per-session `EventsHub` mirror (values are
redacted from the mirrored events when the flag is false).
### Alarms page
@@ -1,5 +1,7 @@
using Microsoft.AspNetCore.SignalR;
using Microsoft.Extensions.Options;
using ZB.MOM.WW.MxGateway.Contracts.Proto;
using ZB.MOM.WW.MxGateway.Server.Configuration;
namespace ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs;
@@ -10,10 +12,22 @@ namespace ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs;
/// Errors are logged once and dropped — keeping the SignalR mirror best-effort
/// preserves the gRPC contract that exists today.
/// </summary>
/// <remarks>
/// When <c>MxGateway:Dashboard:ShowTagValues</c> is false (the default), tag
/// values are stripped from a redacted copy of the event before it reaches any
/// dashboard client. The source <see cref="MxEvent"/> is shared with the gRPC
/// event path and the reconnect replay ring, so it is never mutated in place —
/// the redaction is applied to a deep clone. This closes the value-leak seam at
/// the mirror independently of the still-outstanding per-session hub ACL
/// (see <see cref="EventsHub"/>).
/// </remarks>
public sealed class DashboardEventBroadcaster(
IHubContext<EventsHub> hubContext,
IOptions<GatewayOptions> options,
ILogger<DashboardEventBroadcaster> logger) : IDashboardEventBroadcaster
{
private readonly bool _showTagValues = options.Value.Dashboard.ShowTagValues;
/// <inheritdoc />
public void Publish(string sessionId, MxEvent mxEvent)
{
@@ -22,6 +36,8 @@ public sealed class DashboardEventBroadcaster(
return;
}
MxEvent outbound = _showTagValues ? mxEvent : RedactValues(mxEvent);
// Wrap the Task acquisition in a try/catch so a hypothetical synchronous throw
// from SendAsync (e.g. an implementation that throws before returning the Task)
// cannot escape Publish. The interface contract is never-throw; fire-and-forget.
@@ -30,7 +46,7 @@ public sealed class DashboardEventBroadcaster(
{
send = hubContext.Clients
.Group(EventsHub.GroupName(sessionId))
.SendAsync(EventsHub.EventMessage, mxEvent);
.SendAsync(EventsHub.EventMessage, outbound);
}
catch (Exception ex)
{
@@ -51,4 +67,27 @@ public sealed class DashboardEventBroadcaster(
TaskScheduler.Default);
}
}
/// <summary>
/// Produces a deep clone of <paramref name="source"/> with every tag-value
/// field cleared, leaving tag reference, quality, status, and timestamps
/// intact so the dashboard still renders the event without the value. The
/// source event is left untouched because it is shared downstream with the
/// gRPC stream and the replay ring.
/// </summary>
/// <param name="source">The source event to redact a copy of.</param>
/// <returns>A redacted deep clone of the event.</returns>
private static MxEvent RedactValues(MxEvent source)
{
MxEvent redacted = source.Clone();
redacted.Value = null;
if (redacted.BodyCase == MxEvent.BodyOneofCase.OnAlarmTransition)
{
redacted.OnAlarmTransition.CurrentValue = null;
redacted.OnAlarmTransition.LimitValue = null;
}
return redacted;
}
}
@@ -32,15 +32,19 @@ public sealed class EventsHub : Hub
/// the dashboard roles (Admin or Viewer); both roles may subscribe to
/// any session id they choose. This is acceptable today because (a) the
/// dashboard's per-session views show non-secret session metadata that
/// any authenticated dashboard user can already see, and (b) value
/// logging in the source gRPC stream is gated by the same redaction
/// policy that protects logs. The per-session ACL that gates the gRPC
/// any authenticated dashboard user can already see, and (b) tag values
/// are stripped from the mirrored events by
/// <see cref="DashboardEventBroadcaster"/> when
/// <c>MxGateway:Dashboard:ShowTagValues</c> is false (the default), so the
/// most sensitive payload cannot leak through this seam regardless of the
/// still-missing ACL. The per-session ACL that gates the gRPC
/// <c>StreamEvents</c> RPC is intentionally not yet mirrored here.
/// TODO(per-session-acl): once a role/scope is introduced that scopes a
/// Viewer to a specific session or tenant, add a session-access check
/// at this seam — either inline (consult the per-user allowed-session
/// set on <c>Context.User</c> claims / <c>Context.Items</c>) or via a
/// dedicated authorization policy applied to the hub method itself.
/// TODO(per-session-acl): tracked as remediation roadmap item 12
/// (SEC-25). Once a role/scope is introduced that scopes a Viewer to a
/// specific session or tenant, add a session-access check at this seam —
/// either inline (consult the per-user allowed-session set on
/// <c>Context.User</c> claims / <c>Context.Items</c>) or via a dedicated
/// authorization policy applied to the hub method itself.
/// </remarks>
/// <param name="sessionId">Session id to subscribe the caller to.</param>
/// <returns>A task representing the subscription operation.</returns>
@@ -0,0 +1,171 @@
using Microsoft.AspNetCore.SignalR;
using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.Extensions.Options;
using ZB.MOM.WW.MxGateway.Contracts.Proto;
using ZB.MOM.WW.MxGateway.Server.Configuration;
using ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs;
namespace ZB.MOM.WW.MxGateway.Tests.Gateway.Dashboard;
/// <summary>
/// Verifies that <see cref="DashboardEventBroadcaster"/> honours
/// <c>MxGateway:Dashboard:ShowTagValues</c> (SEC-25): tag values are stripped
/// from the mirrored copy when the flag is off, present when it is on, and the
/// shared source event is never mutated.
/// </summary>
public sealed class DashboardEventBroadcasterTests
{
/// <summary>Values are stripped from the mirror when ShowTagValues is off; metadata survives.</summary>
[Fact]
public void Publish_WhenShowTagValuesFalse_RedactsValuesButKeepsMetadata()
{
CapturingHubContext hubContext = new();
DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: false);
MxEvent source = BuildEventWithValue();
broadcaster.Publish("session-1", source);
MxEvent sent = Assert.IsType<MxEvent>(hubContext.LastArgument);
Assert.Null(sent.Value);
Assert.Null(sent.OnAlarmTransition.CurrentValue);
Assert.Null(sent.OnAlarmTransition.LimitValue);
// Metadata unrelated to the value survives redaction.
Assert.Equal("session-1", sent.SessionId);
Assert.Equal(7, sent.ServerHandle);
Assert.Equal(11, sent.ItemHandle);
Assert.Equal(192, sent.Quality);
Assert.Equal("Tank01.Level.HiHi", sent.OnAlarmTransition.AlarmFullReference);
}
/// <summary>Redaction applies to a clone, so the shared source event keeps its values.</summary>
[Fact]
public void Publish_WhenShowTagValuesFalse_DoesNotMutateSourceEvent()
{
CapturingHubContext hubContext = new();
DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: false);
MxEvent source = BuildEventWithValue();
broadcaster.Publish("session-1", source);
// The redaction must apply to a clone; the shared source keeps its values.
Assert.NotNull(source.Value);
Assert.Equal(42.5, source.Value.DoubleValue);
Assert.NotNull(source.OnAlarmTransition.CurrentValue);
Assert.NotNull(source.OnAlarmTransition.LimitValue);
Assert.NotSame(source, hubContext.LastArgument);
}
/// <summary>Values pass through unredacted when ShowTagValues is on.</summary>
[Fact]
public void Publish_WhenShowTagValuesTrue_KeepsValues()
{
CapturingHubContext hubContext = new();
DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: true);
MxEvent source = BuildEventWithValue();
broadcaster.Publish("session-1", source);
MxEvent sent = Assert.IsType<MxEvent>(hubContext.LastArgument);
Assert.NotNull(sent.Value);
Assert.Equal(42.5, sent.Value.DoubleValue);
Assert.NotNull(sent.OnAlarmTransition.CurrentValue);
Assert.NotNull(sent.OnAlarmTransition.LimitValue);
}
private static DashboardEventBroadcaster Create(CapturingHubContext hubContext, bool showTagValues)
{
GatewayOptions gatewayOptions = new()
{
Dashboard = new DashboardOptions { ShowTagValues = showTagValues },
};
return new DashboardEventBroadcaster(
hubContext,
Options.Create(gatewayOptions),
NullLogger<DashboardEventBroadcaster>.Instance);
}
private static MxEvent BuildEventWithValue()
{
return new MxEvent
{
Family = MxEventFamily.OnAlarmTransition,
SessionId = "session-1",
ServerHandle = 7,
ItemHandle = 11,
Quality = 192,
Value = new MxValue { DataType = MxDataType.Double, DoubleValue = 42.5 },
OnAlarmTransition = new OnAlarmTransitionEvent
{
AlarmFullReference = "Tank01.Level.HiHi",
CurrentValue = new MxValue { DataType = MxDataType.Double, DoubleValue = 88.0 },
LimitValue = new MxValue { DataType = MxDataType.Double, DoubleValue = 90.0 },
},
};
}
private sealed class CapturingHubContext : IHubContext<EventsHub>
{
private readonly CapturingHubClients _clients = new();
/// <summary>Gets the hub clients.</summary>
public IHubClients Clients => _clients;
/// <summary>Gets the group manager.</summary>
public IGroupManager Groups { get; } = new NoopGroupManager();
/// <summary>Gets the first argument of the most recent send call.</summary>
public object? LastArgument => _clients.GroupProxy.LastArgument;
}
private sealed class CapturingHubClients : IHubClients
{
/// <summary>Gets the capturing client proxy shared by this fake.</summary>
public CapturingClientProxy GroupProxy { get; } = new();
public IClientProxy All => GroupProxy;
public IClientProxy AllExcept(IReadOnlyList<string> excludedConnectionIds) => GroupProxy;
public IClientProxy Client(string connectionId) => GroupProxy;
public IClientProxy Clients(IReadOnlyList<string> connectionIds) => GroupProxy;
public IClientProxy Group(string groupName) => GroupProxy;
public IClientProxy GroupExcept(string groupName, IReadOnlyList<string> excludedConnectionIds) => GroupProxy;
public IClientProxy Groups(IReadOnlyList<string> groupNames) => GroupProxy;
public IClientProxy User(string userId) => GroupProxy;
public IClientProxy Users(IReadOnlyList<string> userIds) => GroupProxy;
}
private sealed class CapturingClientProxy : IClientProxy
{
/// <summary>Gets the first argument of the most recent send call.</summary>
public object? LastArgument { get; private set; }
/// <summary>Records the send call arguments and completes synchronously.</summary>
/// <param name="method">The SignalR method name.</param>
/// <param name="args">The method arguments.</param>
/// <param name="cancellationToken">Token to observe for cancellation.</param>
/// <returns>A completed task.</returns>
public Task SendCoreAsync(string method, object?[] args, CancellationToken cancellationToken = default)
{
LastArgument = args.Length > 0 ? args[0] : null;
return Task.CompletedTask;
}
}
private sealed class NoopGroupManager : IGroupManager
{
public Task AddToGroupAsync(string connectionId, string groupName, CancellationToken cancellationToken = default)
=> Task.CompletedTask;
public Task RemoveFromGroupAsync(string connectionId, string groupName, CancellationToken cancellationToken = default)
=> Task.CompletedTask;
}
}