From 6dab72a47963d4eda72db56fc698e850712c0cdc Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 09:42:49 -0400 Subject: [PATCH 01/20] refactor(host): extract MapOtOpcUaMeshGrpcServices (one authority for the five mesh maps) Behaviour-preserving: Program.cs:743-784 collapses to a single call, same gates, same order, every existing per-map rationale comment carried over. A new endpoint-metadata census (MeshGrpcAnonymousMetadataTests) drives the SAME production mapper across all three role shapes (admin-only / driver-only / fused), so the guard can never assert a shape production no longer has - archreview 03/S14, T1. --- .../Configuration/MeshGrpcEndpoints.cs | 97 +++++++++++ src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs | 48 +----- .../MeshGrpcAnonymousMetadataTests.cs | 150 ++++++++++++++++++ 3 files changed, 253 insertions(+), 42 deletions(-) create mode 100644 src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/MeshGrpcEndpoints.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshGrpcAnonymousMetadataTests.cs diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/MeshGrpcEndpoints.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/MeshGrpcEndpoints.cs new file mode 100644 index 00000000..c99c1194 --- /dev/null +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/MeshGrpcEndpoints.cs @@ -0,0 +1,97 @@ +using ZB.MOM.WW.LocalDb.Replication; +using ZB.MOM.WW.OtOpcUa.AdminUI.Grpc; +using ZB.MOM.WW.OtOpcUa.Host.Grpc; + +namespace ZB.MOM.WW.OtOpcUa.Host.Configuration; + +/// +/// Maps the mesh gRPC ingress endpoints (artifact serve, central ack, telemetry stream, node +/// command) and the LocalDb sync endpoint. +/// +/// +/// +/// Do not "tidy" this into Program.cs. A standing census guard +/// (MeshGrpcAnonymousMetadataTests) drives THIS method, so a map that moves back +/// inline leaves the guard asserting a shape production no longer has — which is exactly +/// how the AdminUI cookie FallbackPolicy silently blocked every mesh gRPC call on a +/// login-enabled admin node for seven phases of live gates (archreview 03/S14). +/// +/// +public static class MeshGrpcEndpoints +{ + /// + /// Maps the mesh gRPC ingress endpoints and the LocalDb sync endpoint, gated on the same + /// role + dedicated-port checks Program.cs used inline before this extraction. + /// + /// The built application to map endpoints on. + /// Whether this node carries the admin role. + /// Whether this node carries the driver role. + /// + /// The dedicated h2c port bound for the LocalDb passive sync endpoint, or 0 when it is + /// not bound. + /// + /// + /// The dedicated h2c port bound for the ConfigServe artifact + central command endpoints, or + /// 0 when it is not bound. + /// + /// + /// The dedicated h2c port bound for the telemetry stream + node command endpoints, or + /// 0 when it is not bound. + /// + public static void MapOtOpcUaMeshGrpcServices( + this WebApplication app, + bool hasAdmin, + bool hasDriver, + int syncListenPort, + int configServeGrpcPort, + int telemetryListenPort) + { + ArgumentNullException.ThrowIfNull(app); + + // Passive LocalDb sync endpoint. Gated on the same port check that bound the listener, so a + // default-OFF or admin-only graph carries no sync surface whatsoever. Mapping it would be + // safe even unauthenticated — LocalDbSyncAuthInterceptor fail-closes with no ApiKey + // configured — but not mapping it at all is the stronger default. + if (hasDriver && syncListenPort > 0) + { + app.MapZbLocalDbSync(); + } + + // Central config-serve artifact endpoint (per-cluster mesh Phase 3). Mapped only on + // admin-role nodes (which hold the central SQL connection) and only when a dedicated h2c + // port is bound. The ConfigServeAuthInterceptor is the ONLY inbound auth and fail-closes + // with no ConfigServe:ApiKey. + if (hasAdmin && configServeGrpcPort > 0) + { + app.MapGrpcService(); + + // Central command ingress (mesh gRPC command plane): the node→central deployment ack. It + // rides the SAME listener rather than claiming a port of its own — a second cleartext h2c + // port would buy nothing but another firewall rule, and both endpoints are admin-role and + // dialled by the same driver nodes. Deliberately NOT gated on MeshTransport:Mode: mapping + // it under Dps costs nothing (nothing dials it) whereas gating it would mean flipping the + // mode on a running fleet requires a central restart before any node can ack — the same + // reasoning that registers CentralCommunicationActor in both modes. + // CentralCommandAuthInterceptor (added at AddGrpc in Program.cs) is the ONLY inbound auth + // and fail-closes per cluster. + app.MapGrpcService(); + } + + // Driver-node live-telemetry stream (per-cluster mesh Phase 5). Central dials in; gated by + // TelemetryStreamAuthInterceptor (fail-closed on empty Telemetry:ApiKey). + // + // The node-side command endpoint (mesh gRPC command plane, Phase 1) rides the SAME + // driver-role h2c listener rather than claiming a fourth port: central already dials this + // node here, both directions are per-cluster-keyed, and a separate port would be one more + // thing to open on every site firewall for no isolation gain. Mapping it is what makes + // MeshCommandAuthInterceptor live — it is registered in Program.cs and is a pass-through + // until a call matches this service's method prefix. It fail-closes with no + // MeshTransport:GrpcPsk, so an operator who maps the service without setting a key gets a + // closed endpoint, not an open one. + if (hasDriver && telemetryListenPort > 0) + { + app.MapGrpcService(); + app.MapGrpcService(); + } + } +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs index 12eeee6b..40ae23ab 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs @@ -740,48 +740,12 @@ if (hasAdmin) app.MapOtOpcUaDeployApi(app.Configuration); } -// Passive LocalDb sync endpoint. Gated on the same port check that bound the listener, so a -// default-OFF or admin-only graph carries no sync surface whatsoever. Mapping it would be safe -// even unauthenticated — LocalDbSyncAuthInterceptor fail-closes with no ApiKey configured — but -// not mapping it at all is the stronger default. -if (hasDriver && syncListenPort > 0) -{ - app.MapZbLocalDbSync(); -} - -// Central config-serve artifact endpoint (per-cluster mesh Phase 3). Mapped only on admin-role -// nodes (which hold the central SQL connection) and only when a dedicated h2c port is bound. The -// ConfigServeAuthInterceptor is the ONLY inbound auth and fail-closes with no ConfigServe:ApiKey. -if (hasAdmin && configServeGrpcPort > 0) -{ - app.MapGrpcService(); - - // Central command ingress (mesh gRPC command plane): the node→central deployment ack. It - // rides the SAME listener rather than claiming a port of its own — a second cleartext h2c port - // would buy nothing but another firewall rule, and both endpoints are admin-role and dialled by - // the same driver nodes. Deliberately NOT gated on MeshTransport:Mode: mapping it under Dps - // costs nothing (nothing dials it) whereas gating it would mean flipping the mode on a running - // fleet requires a central restart before any node can ack — the same reasoning that registers - // CentralCommunicationActor in both modes. CentralCommandAuthInterceptor (added at AddGrpc - // above) is the ONLY inbound auth and fail-closes per cluster. - app.MapGrpcService(); -} - -// Driver-node live-telemetry stream (per-cluster mesh Phase 5). Central dials in; gated by -// TelemetryStreamAuthInterceptor (fail-closed on empty Telemetry:ApiKey). -// -// The node-side command endpoint (mesh gRPC command plane, Phase 1) rides the SAME driver-role -// h2c listener rather than claiming a fourth port: central already dials this node here, both -// directions are per-cluster-keyed, and a separate port would be one more thing to open on every -// site firewall for no isolation gain. Mapping it is what makes MeshCommandAuthInterceptor live — -// it is registered above and is a pass-through until a call matches this service's method prefix. -// It fail-closes with no MeshTransport:GrpcPsk, so an operator who maps the service without setting -// a key gets a closed endpoint, not an open one. -if (hasDriver && telemetryListenPort > 0) -{ - app.MapGrpcService(); - app.MapGrpcService(); -} +// Mesh gRPC ingress: the LocalDb passive sync endpoint, the ConfigServe artifact + central +// command endpoints, and the telemetry-stream + node-command endpoints. Extracted to +// MeshGrpcEndpoints so a standing census test drives the SAME production mapping (see that +// file's remarks — archreview 03/S14). +app.MapOtOpcUaMeshGrpcServices( + hasAdmin, hasDriver, syncListenPort, configServeGrpcPort, telemetryListenPort); app.MapOtOpcUaHealth(); app.MapOtOpcUaMetrics(); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshGrpcAnonymousMetadataTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshGrpcAnonymousMetadataTests.cs new file mode 100644 index 00000000..1c734779 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshGrpcAnonymousMetadataTests.cs @@ -0,0 +1,150 @@ +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.DependencyInjection; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Host.Configuration; + +namespace ZB.MOM.WW.OtOpcUa.Host.IntegrationTests; + +/// +/// R3-01 (T1-T3) — an endpoint-metadata census over MapOtOpcUaMeshGrpcServices, driven in +/// all three role shapes a node can actually carry (admin-only, driver-only, fused). +/// +/// +/// +/// Why this drives the PRODUCTION mapper, not a hand-copied shape. The Critical this +/// plan fixes (archreview 03/S14) survived seven phases of live gates precisely because every +/// prior test built its own pipeline by hand — MeshGrpcBoundaryTests, +/// ConfigServeListenerTests and LocalDbSyncListenerTests all reproduce the +/// author's mental model of the mapping rather than the host's actual composition. This +/// census calls — the SAME method +/// Program.cs calls — so it is structurally impossible for the guard to assert a shape +/// production no longer has. +/// +/// +/// Filters out grpc-dotnet's own catch-all infrastructure. Every MapGrpcService +/// call also registers a per-service "Unimplemented method for X" fallback, and the FIRST +/// such call in an app additionally registers one shared "Unimplemented service" catch-all — +/// neither carries a route pattern beginning with /, and neither ever reaches real +/// logic (they always answer Unimplemented regardless of caller identity), so their +/// authorization metadata carries no security meaning. +/// keeps only the five real, callable RPC methods. +/// +/// +/// Guard-of-the-guard. Every assertion below pins an EXACT expected count and the +/// exact set of route patterns, never just "not empty" — a census that silently found zero +/// endpoints (a typo'd role gate, a helper that never got called) would pass a bare +/// non-empty check trivially. It cannot pass an exact-count-and-set check by accident. +/// +/// +public sealed class MeshGrpcAnonymousMetadataTests +{ + private const string ArtifactFetch = "/deployment_artifact.v1.DeploymentArtifactService/Fetch"; + private const string CentralReportApply = "/mesh_transport.v1.CentralCommandService/ReportApply"; + private const string TelemetrySubscribe = "/telemetry.v1.TelemetryStreamService/Subscribe"; + private const string NodeExecute = "/mesh_transport.v1.NodeCommandService/Execute"; + private const string LocalDbSyncSync = "/localdb_sync.v1.LocalDbSync/Sync"; + + /// The five known mesh gRPC method prefixes — the ONLY endpoints this map may anonymise. + private static readonly string[] AllFivePrefixes = + [LocalDbSyncSync, ArtifactFetch, CentralReportApply, TelemetrySubscribe, NodeExecute]; + + [Fact] + public void AdminOnly_MapsExactlyTheConfigServeEndpoints() + { + // Admin-only: no driver role, so neither LocalDbSync nor the telemetry/node-command pair are + // mapped even though a port is configured for them below (0 here, but the shape is the point). + var endpoints = RealMethodEndpoints(BuildApp( + hasAdmin: true, hasDriver: false, + syncListenPort: 0, configServeGrpcPort: 4055, telemetryListenPort: 0)); + + endpoints.Count.ShouldBe(2); + endpoints.Select(RoutePatternOf).ShouldBe([ArtifactFetch, CentralReportApply], ignoreOrder: true); + } + + [Fact] + public void DriverOnly_WithNoSyncPort_MapsExactlyTheTelemetryAndNodeCommandEndpoints() + { + var endpoints = RealMethodEndpoints(BuildApp( + hasAdmin: false, hasDriver: true, + syncListenPort: 0, configServeGrpcPort: 0, telemetryListenPort: 4056)); + + endpoints.Count.ShouldBe(2); + endpoints.Select(RoutePatternOf).ShouldBe([TelemetrySubscribe, NodeExecute], ignoreOrder: true); + } + + [Fact] + public void DriverOnly_WithASyncPort_AlsoMapsTheLocalDbSyncEndpoint() + { + var endpoints = RealMethodEndpoints(BuildApp( + hasAdmin: false, hasDriver: true, + syncListenPort: 4054, configServeGrpcPort: 0, telemetryListenPort: 4056)); + + endpoints.Count.ShouldBe(3); + endpoints.Select(RoutePatternOf).ShouldBe( + [LocalDbSyncSync, TelemetrySubscribe, NodeExecute], ignoreOrder: true); + } + + [Fact] + public void Fused_WithAllThreePorts_MapsAllFiveEndpoints() + { + // The docker-dev central pair's shape (admin,driver,cluster-MAIN) once all three dedicated + // h2c ports are configured — the shape 03/S14 called out as breaking the deploy plane for + // the whole fleet, not only the four sites. + var endpoints = RealMethodEndpoints(BuildApp( + hasAdmin: true, hasDriver: true, + syncListenPort: 4054, configServeGrpcPort: 4055, telemetryListenPort: 4056)); + + endpoints.Count.ShouldBe(5); + endpoints.Select(RoutePatternOf).ShouldBe(AllFivePrefixes, ignoreOrder: true); + } + + [Fact] + public void NeitherRoleNorAnyPort_MapsNothing() + { + // The default-OFF shape: every dedicated port is 0. Nothing about the mesh gRPC surface + // should exist on a node that configured none of it. + var endpoints = RealMethodEndpoints(BuildApp( + hasAdmin: false, hasDriver: false, + syncListenPort: 0, configServeGrpcPort: 0, telemetryListenPort: 0)); + + endpoints.ShouldBeEmpty(); + } + + // --------------------------------------------------------------------------------------- + // harness + // --------------------------------------------------------------------------------------- + + private static WebApplication BuildApp( + bool hasAdmin, + bool hasDriver, + int syncListenPort, + int configServeGrpcPort, + int telemetryListenPort) + { + var builder = WebApplication.CreateBuilder(); + builder.Environment.ApplicationName = typeof(MeshGrpcAnonymousMetadataTests).Assembly.GetName().Name!; + builder.Services.AddGrpc(); + + var app = builder.Build(); + app.MapOtOpcUaMeshGrpcServices( + hasAdmin, hasDriver, syncListenPort, configServeGrpcPort, telemetryListenPort); + return app; + } + + /// + /// The real, callable RPC method endpoints — excludes grpc-dotnet's per-app "Unimplemented + /// service" catch-all and per-service "Unimplemented method for X" fallback (see class + /// remarks). Both carry a route pattern that never starts with /, which is what this + /// filters on. + /// + private static List RealMethodEndpoints(WebApplication app) => + ((IEndpointRouteBuilder)app).DataSources + .SelectMany(d => d.Endpoints) + .OfType() + .Where(e => e.RoutePattern.RawText?.StartsWith('/') == true) + .ToList(); + + private static string RoutePatternOf(RouteEndpoint e) => e.RoutePattern.RawText!; +} From ac9a71bcbde2fb7b75836fcba80bd9cee2d85ba0 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 09:43:06 -0400 Subject: [PATCH 02/20] docs(plans): mark R3-01 T1 done --- .../plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json b/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json index 35776954..91648629 100644 --- a/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json +++ b/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json @@ -5,7 +5,7 @@ { "id": "T1", "subject": "Extract MapOtOpcUaMeshGrpcServices from Program.cs:743-784 (behaviour-preserving) + endpoint census scaffold (03/S14)", - "status": "pending", + "status": "done", "blockedBy": [] }, { From b05ebd635353be1d3b2880170e70c6f65ed081fe Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 09:46:30 -0400 Subject: [PATCH 03/20] fix(host): mesh gRPC endpoints opt out of the AdminUI cookie FallbackPolicy (archreview 03/S14) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit .AllowAnonymous() on DeploymentArtifactGrpcService, CentralCommandGrpcService, TelemetryStreamGrpcService and NodeCommandGrpcService — the four MapGrpcService maps that are chainable. On a login-enabled admin node the global cookie FallbackPolicy (RequireAuthenticatedUser) applies to every endpoint without explicit authorization metadata, gRPC included; the PSK bearer interceptors run INSIDE the endpoint invocation, strictly after that middleware, so a correctly-keyed node call was challenged before the interceptor's verdict could matter. Safe because all four interceptors fail closed on an unset key. The census now asserts IAllowAnonymous on all four (red before this change, green after) plus a scope guard proving the opt-out never leaks onto an unrelated endpoint. MapZbLocalDbSync (T3) is not chainable the same way and is asserted absent here on purpose, so this test cannot start passing for the wrong reason once T3 lands. --- .../Configuration/MeshGrpcEndpoints.cs | 33 ++++++-- .../MeshGrpcAnonymousMetadataTests.cs | 76 +++++++++++++++++++ 2 files changed, 103 insertions(+), 6 deletions(-) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/MeshGrpcEndpoints.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/MeshGrpcEndpoints.cs index c99c1194..c12e77f3 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/MeshGrpcEndpoints.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/MeshGrpcEndpoints.cs @@ -10,11 +10,23 @@ namespace ZB.MOM.WW.OtOpcUa.Host.Configuration; /// /// /// +/// Why AllowAnonymous (archreview 03/S14). On an admin-role node +/// AddOtOpcUaAuth installs a global FallbackPolicy = RequireAuthenticatedUser +/// under the cookie scheme, which the authorization middleware applies to EVERY endpoint +/// without explicit authorization metadata — gRPC included. These endpoints authenticate +/// with a per-cluster PSK bearer inside a gRPC interceptor, which runs INSIDE the endpoint +/// invocation, i.e. after that middleware. Without this opt-out a correctly-keyed node call +/// is challenged by the cookie handler before the interceptor's verdict matters: artifact +/// fetches fail, deploy acks are dropped, deployments seal TimedOut. Anonymous is safe here +/// because all five interceptors fail CLOSED — an unset key rejects every call and the +/// presented token is compared with FixedTimeEquals. +/// +/// /// Do not "tidy" this into Program.cs. A standing census guard /// (MeshGrpcAnonymousMetadataTests) drives THIS method, so a map that moves back /// inline leaves the guard asserting a shape production no longer has — which is exactly /// how the AdminUI cookie FallbackPolicy silently blocked every mesh gRPC call on a -/// login-enabled admin node for seven phases of live gates (archreview 03/S14). +/// login-enabled admin node for seven phases of live gates. /// /// public static class MeshGrpcEndpoints @@ -63,7 +75,11 @@ public static class MeshGrpcEndpoints // with no ConfigServe:ApiKey. if (hasAdmin && configServeGrpcPort > 0) { - app.MapGrpcService(); + // AllowAnonymous (03/S14): this is every FetchAndCache node's artifact fetch. Without the + // opt-out the AdminUI cookie FallbackPolicy 401s/redirects it before + // ConfigServeAuthInterceptor's verdict matters — a fleet-wide deploy-plane outage on any + // login-enabled admin node. + app.MapGrpcService().AllowAnonymous(); // Central command ingress (mesh gRPC command plane): the node→central deployment ack. It // rides the SAME listener rather than claiming a port of its own — a second cleartext h2c @@ -73,8 +89,9 @@ public static class MeshGrpcEndpoints // mode on a running fleet requires a central restart before any node can ack — the same // reasoning that registers CentralCommunicationActor in both modes. // CentralCommandAuthInterceptor (added at AddGrpc in Program.cs) is the ONLY inbound auth - // and fail-closes per cluster. - app.MapGrpcService(); + // and fail-closes per cluster. AllowAnonymous (03/S14): this is every node's deploy ack, + // from every cluster — same reasoning as the artifact fetch above. + app.MapGrpcService().AllowAnonymous(); } // Driver-node live-telemetry stream (per-cluster mesh Phase 5). Central dials in; gated by @@ -90,8 +107,12 @@ public static class MeshGrpcEndpoints // closed endpoint, not an open one. if (hasDriver && telemetryListenPort > 0) { - app.MapGrpcService(); - app.MapGrpcService(); + // AllowAnonymous (03/S14) on both: on a fused admin+driver node (the docker-dev central + // pair, and any production fused central) these sit behind the SAME cookie FallbackPolicy + // as the ConfigServe endpoints above and would be 401/redirected identically without the + // opt-out — central's own telemetry dialer and node-command dispatch to its own pair. + app.MapGrpcService().AllowAnonymous(); + app.MapGrpcService().AllowAnonymous(); } } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshGrpcAnonymousMetadataTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshGrpcAnonymousMetadataTests.cs index 1c734779..bdaf71d7 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshGrpcAnonymousMetadataTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshGrpcAnonymousMetadataTests.cs @@ -1,3 +1,4 @@ +using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.DependencyInjection; @@ -112,6 +113,74 @@ public sealed class MeshGrpcAnonymousMetadataTests endpoints.ShouldBeEmpty(); } + // --------------------------------------------------------------------------------------- + // 03/S14 — every mesh endpoint carries IAllowAnonymous, so the AdminUI cookie FallbackPolicy + // (RequireAuthenticatedUser) never reaches it; the PSK interceptors are the ONLY gate. T3 + // (LocalDbSync's empty-prefix group) is asserted separately below. + // --------------------------------------------------------------------------------------- + + [Fact] + public void AdminOnly_BothConfigServeEndpoints_CarryAllowAnonymous() + { + var endpoints = RealMethodEndpoints(BuildApp( + hasAdmin: true, hasDriver: false, + syncListenPort: 0, configServeGrpcPort: 4055, telemetryListenPort: 0)); + + AnonymousPatternsOf(endpoints).ShouldBe([ArtifactFetch, CentralReportApply], ignoreOrder: true); + } + + [Fact] + public void DriverOnly_BothTelemetryAndNodeCommandEndpoints_CarryAllowAnonymous() + { + var endpoints = RealMethodEndpoints(BuildApp( + hasAdmin: false, hasDriver: true, + syncListenPort: 0, configServeGrpcPort: 0, telemetryListenPort: 4056)); + + AnonymousPatternsOf(endpoints).ShouldBe([TelemetrySubscribe, NodeExecute], ignoreOrder: true); + } + + [Fact] + public void Fused_AllFourGrpcServiceMapsCarryAllowAnonymous_LocalDbSyncIsNotYetOwnedHere() + { + // T3 owns the LocalDbSync map (MapZbLocalDbSync returns IEndpointRouteBuilder, not + // IEndpointConventionBuilder, so .AllowAnonymous() cannot be chained onto it the same way). + // Asserting its ABSENCE here — rather than skipping it — is what makes this test go red the + // moment T3's fix lands with the wrong scope (e.g. anonymising nothing, or the group leaking + // beyond LocalDbSync) instead of silently starting to pass for the wrong reason. + var endpoints = RealMethodEndpoints(BuildApp( + hasAdmin: true, hasDriver: true, + syncListenPort: 4054, configServeGrpcPort: 4055, telemetryListenPort: 4056)); + + AnonymousPatternsOf(endpoints).ShouldBe( + [ArtifactFetch, CentralReportApply, TelemetrySubscribe, NodeExecute], ignoreOrder: true); + } + + [Fact] + public void AllowAnonymous_NeverLeaksBeyondTheFiveKnownMeshEndpoints() + { + // Scope guard (theme 2): an endpoint mapped ALONGSIDE the mesh services, through no seam this + // extension touches, must never come out anonymous. If it did, some future edit widened the + // AllowAnonymous convention beyond the five known prefixes — the exact failure class the + // fallback-policy opt-out exists to bound. + var builder = WebApplication.CreateBuilder(); + builder.Environment.ApplicationName = typeof(MeshGrpcAnonymousMetadataTests).Assembly.GetName().Name!; + builder.Services.AddGrpc(); + var app = builder.Build(); + + app.MapOtOpcUaMeshGrpcServices( + hasAdmin: true, hasDriver: true, + syncListenPort: 4054, configServeGrpcPort: 4055, telemetryListenPort: 4056); + var sentinel = app.MapGet("/sentinel", () => "ok"); + + var sentinelEndpoint = ((IEndpointRouteBuilder)app).DataSources + .SelectMany(d => d.Endpoints) + .OfType() + .Single(e => e.RoutePattern.RawText == "/sentinel"); + + sentinelEndpoint.Metadata.GetMetadata().ShouldBeNull(); + _ = sentinel; // the builder handle itself is not the assertion target + } + // --------------------------------------------------------------------------------------- // harness // --------------------------------------------------------------------------------------- @@ -147,4 +216,11 @@ public sealed class MeshGrpcAnonymousMetadataTests .ToList(); private static string RoutePatternOf(RouteEndpoint e) => e.RoutePattern.RawText!; + + /// Route patterns of the endpoints in that carry . + private static List AnonymousPatternsOf(IEnumerable endpoints) => + endpoints + .Where(e => e.Metadata.GetMetadata() is not null) + .Select(RoutePatternOf) + .ToList(); } From 2f90156408c3f32c8831a8f0067e20478ca0f81c Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 09:46:40 -0400 Subject: [PATCH 04/20] docs(plans): mark R3-01 T2 done --- .../plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json b/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json index 91648629..7e7d1e5d 100644 --- a/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json +++ b/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json @@ -11,7 +11,7 @@ { "id": "T2", "subject": "Census asserts IAllowAnonymous + scope/count guard-of-the-guard; .AllowAnonymous() on the four MapGrpcService maps", - "status": "pending", + "status": "done", "blockedBy": [ "T1" ] From e3cf363703a2de2dd7ff94498535db3153bb7cf9 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 09:51:33 -0400 Subject: [PATCH 05/20] fix(host): LocalDb sync endpoint opts out of the cookie fallback via an empty-prefix anon group MapZbLocalDbSync returns IEndpointRouteBuilder, not an IEndpointConventionBuilder, so .AllowAnonymous() cannot be chained onto it the way the other four mesh maps are (T2). app.MapGroup(string.Empty).AllowAnonymous().MapZbLocalDbSync() carries the same convention while leaving the gRPC route pattern byte-identical - verified directly (LocalDbSync_AllowAnonymousGroup_LeavesTheRoutePatternByteIdentical). The census's RealMethodEndpoints filter had to move off a "route pattern starts with /" heuristic to an exact-route-pattern-set match: inside the empty-prefix group, grpc-dotnet's own "Unimplemented service"/"Unimplemented method" fallback endpoints also gain a leading "/", so the heuristic let them back in and over-counted (caught by DriverOnly_WithASyncPort_AlsoMapsTheLocalDbSyncEndpoint and LocalDbSync_AllowAnonymousGroup_LeavesTheRoutePatternByteIdentical going red against the naive filter before this fix). All five mesh gRPC endpoints now carry IAllowAnonymous (archreview 03/S14, T3). --- .../Configuration/MeshGrpcEndpoints.cs | 9 +++- .../MeshGrpcAnonymousMetadataTests.cs | 47 ++++++++++++------- 2 files changed, 39 insertions(+), 17 deletions(-) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/MeshGrpcEndpoints.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/MeshGrpcEndpoints.cs index c12e77f3..68a51ed8 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/MeshGrpcEndpoints.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/MeshGrpcEndpoints.cs @@ -64,9 +64,16 @@ public static class MeshGrpcEndpoints // default-OFF or admin-only graph carries no sync surface whatsoever. Mapping it would be // safe even unauthenticated — LocalDbSyncAuthInterceptor fail-closes with no ApiKey // configured — but not mapping it at all is the stronger default. + // + // AllowAnonymous (03/S14) via an empty-prefix MapGroup: MapZbLocalDbSync returns + // IEndpointRouteBuilder, NOT an IEndpointConventionBuilder, so .AllowAnonymous() cannot be + // chained onto it the way the other four maps below are. An empty-prefix group carries the + // AllowAnonymous convention to whatever it maps while leaving the gRPC route pattern + // byte-identical to the ungrouped map (verified by + // LocalDbSync_AllowAnonymousGroup_LeavesTheRoutePatternByteIdentical). if (hasDriver && syncListenPort > 0) { - app.MapZbLocalDbSync(); + app.MapGroup(string.Empty).AllowAnonymous().MapZbLocalDbSync(); } // Central config-serve artifact endpoint (per-cluster mesh Phase 3). Mapped only on diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshGrpcAnonymousMetadataTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshGrpcAnonymousMetadataTests.cs index bdaf71d7..3cc91692 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshGrpcAnonymousMetadataTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshGrpcAnonymousMetadataTests.cs @@ -26,11 +26,13 @@ namespace ZB.MOM.WW.OtOpcUa.Host.IntegrationTests; /// /// Filters out grpc-dotnet's own catch-all infrastructure. Every MapGrpcService /// call also registers a per-service "Unimplemented method for X" fallback, and the FIRST -/// such call in an app additionally registers one shared "Unimplemented service" catch-all — -/// neither carries a route pattern beginning with /, and neither ever reaches real -/// logic (they always answer Unimplemented regardless of caller identity), so their -/// authorization metadata carries no security meaning. -/// keeps only the five real, callable RPC methods. +/// such call in an app additionally registers one shared "Unimplemented service" catch-all. +/// Neither ever reaches real logic (they always answer Unimplemented regardless of +/// caller identity), so their authorization metadata carries no security meaning. +/// keeps only the five real, callable RPC methods, matched +/// against the exact known route-pattern set rather than a route-pattern-shape heuristic — +/// see that method's remarks for why a shape-based filter is NOT safe once LocalDbSync's +/// empty-prefix MapGroup (T3) is in play. /// /// /// Guard-of-the-guard. Every assertion below pins an EXACT expected count and the @@ -140,19 +142,30 @@ public sealed class MeshGrpcAnonymousMetadataTests } [Fact] - public void Fused_AllFourGrpcServiceMapsCarryAllowAnonymous_LocalDbSyncIsNotYetOwnedHere() + public void Fused_AllFiveMeshEndpoints_CarryAllowAnonymous() { - // T3 owns the LocalDbSync map (MapZbLocalDbSync returns IEndpointRouteBuilder, not - // IEndpointConventionBuilder, so .AllowAnonymous() cannot be chained onto it the same way). - // Asserting its ABSENCE here — rather than skipping it — is what makes this test go red the - // moment T3's fix lands with the wrong scope (e.g. anonymising nothing, or the group leaking - // beyond LocalDbSync) instead of silently starting to pass for the wrong reason. + // Includes LocalDbSync (T3): MapZbLocalDbSync returns IEndpointRouteBuilder, not an + // IEndpointConventionBuilder, so .AllowAnonymous() cannot be chained onto it the way the + // other four are — it needs the empty-prefix MapGroup wrapper instead. var endpoints = RealMethodEndpoints(BuildApp( hasAdmin: true, hasDriver: true, syncListenPort: 4054, configServeGrpcPort: 4055, telemetryListenPort: 4056)); - AnonymousPatternsOf(endpoints).ShouldBe( - [ArtifactFetch, CentralReportApply, TelemetrySubscribe, NodeExecute], ignoreOrder: true); + AnonymousPatternsOf(endpoints).ShouldBe(AllFivePrefixes, ignoreOrder: true); + } + + [Fact] + public void LocalDbSync_AllowAnonymousGroup_LeavesTheRoutePatternByteIdentical() + { + // The empty-prefix MapGroup wrapper (T3) must be pure convention-carrying — it must not + // change the gRPC route pattern LocalDbSyncAuthInterceptor and the sync client both key on. + var endpoints = RealMethodEndpoints(BuildApp( + hasAdmin: false, hasDriver: true, + syncListenPort: 4054, configServeGrpcPort: 0, telemetryListenPort: 0)); + + var sync = endpoints.ShouldHaveSingleItem(); + sync.RoutePattern.RawText.ShouldBe(LocalDbSyncSync); + sync.Metadata.GetMetadata().ShouldNotBeNull(); } [Fact] @@ -205,14 +218,16 @@ public sealed class MeshGrpcAnonymousMetadataTests /// /// The real, callable RPC method endpoints — excludes grpc-dotnet's per-app "Unimplemented /// service" catch-all and per-service "Unimplemented method for X" fallback (see class - /// remarks). Both carry a route pattern that never starts with /, which is what this - /// filters on. + /// remarks). Filtered against the exact set of known method route patterns rather than a + /// "starts with /" heuristic: inside the empty-prefix MapGroup the LocalDbSync map + /// (T3) uses, grpc-dotnet's own fallback patterns ALSO gain a leading /, so a + /// prefix-shaped filter would let them back in. /// private static List RealMethodEndpoints(WebApplication app) => ((IEndpointRouteBuilder)app).DataSources .SelectMany(d => d.Endpoints) .OfType() - .Where(e => e.RoutePattern.RawText?.StartsWith('/') == true) + .Where(e => AllFivePrefixes.Contains(e.RoutePattern.RawText)) .ToList(); private static string RoutePatternOf(RouteEndpoint e) => e.RoutePattern.RawText!; From 1759f6047613ed714a2279c6f20054e6ef055b7c Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 09:51:43 -0400 Subject: [PATCH 06/20] docs(plans): mark R3-01 T3 done --- .../plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json b/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json index 7e7d1e5d..95fe4701 100644 --- a/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json +++ b/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json @@ -19,7 +19,7 @@ { "id": "T3", "subject": "LocalDb sync endpoint anonymised via an empty-prefix MapGroup (MapZbLocalDbSync returns IEndpointRouteBuilder, not chainable)", - "status": "pending", + "status": "done", "blockedBy": [ "T2" ] From 458c84c34f0c8136f4d24a55aff5f80ed98c4d46 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 10:06:47 -0400 Subject: [PATCH 07/20] test(host): mesh gRPC boundary under the production auth stack (FallbackPolicy live) MeshGrpcAuthStackTests - the CI-shaped guard 03/S14 asked for. A real AddOtOpcUaAuth(config) + UseAuthentication/UseAuthorization pipeline (cookie scheme, DisableLogin=false, EnvironmentName=Production) with the production interceptor set, driving DeploymentArtifactService.Fetch, CentralCommandService.ReportApply and NodeCommandService.Execute through MapOtOpcUaMeshGrpcServices. Verified red-then-green by hand: with the T1 (pre-AllowAnonymous) mapper swapped in, all three positive calls fail with "HTTP/2 server reset the stream, PROTOCOL_ERROR" (RpcException/Internal) - the cookie challenge breaking prior-knowledge h2c, not a clean auth denial. With the current mapper (T2/T3) all three succeed. That asymmetry is the only thing distinguishing the two builds; every other assertion here (cookie handler identity, the falsifiability control) is unchanged by the fix and proves nothing on its own. Falsifiability control: LocalDbSyncService is mapped directly (bypassing MapOtOpcUaMeshGrpcServices) with no .AllowAnonymous(), and a correctly-shaped call to it is refused - proving the fixture's auth stack is genuinely live, not accidentally permissive. --- .../MeshGrpcAuthStackTests.cs | 306 ++++++++++++++++++ 1 file changed, 306 insertions(+) create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshGrpcAuthStackTests.cs diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshGrpcAuthStackTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshGrpcAuthStackTests.cs new file mode 100644 index 00000000..d55a27d3 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshGrpcAuthStackTests.cs @@ -0,0 +1,306 @@ +using System.Diagnostics.CodeAnalysis; +using System.Net; +using Akka.Actor; +using Akka.Hosting; +using Grpc.Core; +using Grpc.Net.Client; +using Microsoft.AspNetCore.Authentication.Cookies; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Server.Kestrel.Core; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Shouldly; +using Xunit; +using ZB.MOM.WW.LocalDb.Replication; +using ZB.MOM.WW.OtOpcUa.Cluster; +using ZB.MOM.WW.OtOpcUa.Commons.Messages.Admin; +using ZB.MOM.WW.OtOpcUa.Commons.Messages.Deploy; +using ZB.MOM.WW.OtOpcUa.Commons.Messages.Mesh; +using ZB.MOM.WW.OtOpcUa.Commons.Protos.DeploymentArtifact.V1; +using ZB.MOM.WW.OtOpcUa.Commons.Protos.MeshTransport.V1; +using ZB.MOM.WW.OtOpcUa.Commons.Types; +using ZB.MOM.WW.OtOpcUa.Configuration; +using ZB.MOM.WW.OtOpcUa.Configuration.Entities; +using ZB.MOM.WW.OtOpcUa.ControlPlane; +using ZB.MOM.WW.OtOpcUa.ControlPlane.Communication; +using ZB.MOM.WW.OtOpcUa.Host.Configuration; +using ZB.MOM.WW.OtOpcUa.Runtime; +using ZB.MOM.WW.OtOpcUa.Runtime.Telemetry; +using ZB.MOM.WW.OtOpcUa.Security; + +namespace ZB.MOM.WW.OtOpcUa.Host.IntegrationTests; + +/// +/// R3-01 T4 — the CI-shaped guard that would have caught 03/S14: mesh gRPC calls through the +/// REAL AddOtOpcUaAuth + UseAuthentication/UseAuthorization stack, not a +/// hand-built pipeline that never installs the middleware that broke production. +/// +/// +/// +/// The load-bearing assertion is the ALLOW. A blocked call and a refused call both +/// "fail"; only a correctly-keyed call SUCCEEDING through the real cookie +/// FallbackPolicy proves the .AllowAnonymous() opt-outs (T2/T3) actually work in +/// a pipeline that carries the policy at all. Every positive test here failed before T2/T3 — +/// confirmed by running this class against the T1 commit, where every one of them was +/// challenged by the cookie handler instead of reaching the interceptor. +/// +/// +/// Negative control (anti-vacuity). is mapped directly +/// here — via a bare MapGrpcService, deliberately NOT through +/// MapOtOpcUaMeshGrpcServices — with no .AllowAnonymous(). A correctly-shaped +/// call to it must be refused by the cookie challenge. If it were not, the fixture's auth +/// stack would be inert and every positive assertion above would be proving nothing. +/// +/// +public sealed class MeshGrpcAuthStackTests : IAsyncLifetime +{ + private const string ConfigServeKey = "test-configserve-key"; + private const string NodeCommandKey = "test-node-psk"; + private const string CentralCommandKey = "test-central-psk"; + private const string TelemetryKey = "test-telemetry-key"; + private const string ClusterId = "MAIN"; + + private WebApplication _app = null!; + private ActorSystem _system = null!; + private int _port; + private TaskCompletionSource _centralInbox = null!; + private TaskCompletionSource _nodeInbox = null!; + + /// + public async ValueTask InitializeAsync() + { + var builder = WebApplication.CreateBuilder(); + builder.Environment.ApplicationName = typeof(MeshGrpcAuthStackTests).Assembly.GetName().Name!; + // Production, not the WebApplicationBuilder default: forces ValidateOnBuild/ValidateScopes OFF + // (they engage only under Development), so services never resolved without a login attempt + // (ILdapAuthService's graph) don't need to be fully stood up for this fixture to build. + builder.Environment.EnvironmentName = Environments.Production; + builder.Logging.ClearProviders(); + + // DataProtection's PersistKeysToDbContext (inside AddOtOpcUaAuth) + // resolves a SCOPED OtOpcUaConfigDbContext via CreateScope() + GetRequiredService — it has no + // factory support. Mirrors AddOtOpcUaConfigDb's own factory+scoped-bridge shape, over InMemory. + var dbName = Guid.NewGuid().ToString("N"); + builder.Services.AddDbContextFactory(o => o.UseInMemoryDatabase(dbName)); + builder.Services.AddScoped(sp => + sp.GetRequiredService>().CreateDbContext()); + + builder.Configuration.AddInMemoryCollection(new Dictionary + { + // The whole point of this fixture: exercise the REAL cookie handler, not + // AutoLoginAuthenticationHandler. See CookieSchemeIsTheRealHandler_NotAutoLogin. + ["Security:Auth:DisableLogin"] = "false", + // This fixture serves cleartext h2c; a HTTPS-only cookie policy is irrelevant to what is + // under test (the FallbackPolicy applies identically either way) and would otherwise log + // a startup Warning for no reason. + ["Security:Cookie:RequireHttpsCookie"] = "false", + }); + + builder.Services.AddOtOpcUaAuth(builder.Configuration); + + // Interceptor options bound directly (NOT via AddOtOpcUaCluster, which also wires + // AkkaClusterOptions/ConfigSourceOptions/TelemetryDialOptions validators with ValidateOnStart — + // heavy machinery this fixture has no use for and no seed-node config to satisfy). + builder.Services.Configure(o => o.ApiKey = ConfigServeKey); + builder.Services.Configure(o => o.GrpcPsk = NodeCommandKey); + builder.Services.Configure(o => o.ApiKey = TelemetryKey); + builder.Services.AddSingleton(new FixedPskProvider(ClusterId, CentralCommandKey)); + builder.Services.AddSingleton(); + + _system = ActorSystem.Create("mesh-auth-stack-" + Guid.NewGuid().ToString("N")); + _centralInbox = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + _nodeInbox = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var registry = new ActorRegistry(); + registry.Register(_system.ActorOf(CaptureActor.Props(_centralInbox))); + registry.Register(_system.ActorOf(CaptureActor.Props(_nodeInbox))); + builder.Services.AddSingleton(registry); + + // The production interceptor set — the ONLY inbound auth on the gRPC side. Deliberately does + // NOT include LocalDbSyncAuthInterceptor: LocalDbSyncService is mapped below purely as the + // negative control and is never given a matching interceptor here. + builder.Services.AddGrpc(o => + { + o.Interceptors.Add(); + o.Interceptors.Add(); + o.Interceptors.Add(); + o.Interceptors.Add(); + }); + + _port = GetFreePort(); + builder.WebHost.ConfigureKestrel(k => k.ListenAnyIP(_port, o => o.Protocols = HttpProtocols.Http2)); + + _app = builder.Build(); + _app.UseAuthentication(); + _app.UseAuthorization(); + + // The production mapper — the four chainable maps carry AllowAnonymous (T2); LocalDbSync is + // NOT mapped here (syncListenPort: 0) so it stays free for the negative control below without + // a route collision. + _app.MapOtOpcUaMeshGrpcServices( + hasAdmin: true, hasDriver: true, + syncListenPort: 0, configServeGrpcPort: 1, telemetryListenPort: 1); + + // Negative control (T4): mapped directly, NOT through MapOtOpcUaMeshGrpcServices, and + // deliberately with no .AllowAnonymous(). + _app.MapGrpcService(); + + await _app.StartAsync(TestContext.Current.CancellationToken); + } + + /// + public async ValueTask DisposeAsync() + { + await _app.StopAsync(TimeSpan.FromSeconds(10)); + await _app.DisposeAsync(); + await _system.Terminate().WaitAsync(TimeSpan.FromSeconds(15)); + } + + [Fact] + public async Task CookieSchemeIsTheRealHandler_NotAutoLogin() + { + // If this resolves AutoLoginAuthenticationHandler, every assertion below is meaningless — the + // fallback policy would pass every request regardless of AllowAnonymous. + var provider = _app.Services.GetRequiredService(); + var scheme = await provider.GetSchemeAsync(CookieAuthenticationDefaults.AuthenticationScheme); + + scheme.ShouldNotBeNull(); + scheme.HandlerType.ShouldBe(typeof(CookieAuthenticationHandler)); + } + + [Fact] + public async Task CorrectlyKeyed_ArtifactFetch_SucceedsThroughTheRealAuthStack() + { + var deploymentId = Guid.NewGuid(); + var blob = "R3-01-T4-artifact-bytes"u8.ToArray(); + var factory = _app.Services.GetRequiredService>(); + await using (var db = await factory.CreateDbContextAsync(TestContext.Current.CancellationToken)) + { + db.Deployments.Add(new Deployment + { + DeploymentId = deploymentId, + RevisionHash = new string('a', 64), + CreatedBy = "r3-01-t4", + ArtifactBlob = blob, + }); + await db.SaveChangesAsync(TestContext.Current.CancellationToken); + } + + var client = new DeploymentArtifactService.DeploymentArtifactServiceClient(Channel()); + var headers = new Metadata { { "authorization", $"Bearer {ConfigServeKey}" } }; + + using var call = client.Fetch( + new FetchRequest { DeploymentId = deploymentId.ToString() }, + headers, + cancellationToken: TestContext.Current.CancellationToken); + + using var buffer = new MemoryStream(); + await foreach (var chunk in call.ResponseStream.ReadAllAsync(TestContext.Current.CancellationToken)) + chunk.Data.WriteTo(buffer); + + buffer.ToArray().ShouldBe(blob); + } + + [Fact] + public async Task CorrectlyKeyed_CentralCommandReportApply_SucceedsThroughTheRealAuthStack() + { + var ack = new ApplyAck( + new DeploymentId(Guid.NewGuid()), + new NodeId("node-a"), + ApplyAckOutcome.Applied, + FailureReason: null, + CorrelationId.NewId()); + + var client = new CentralCommandService.CentralCommandServiceClient(Channel()); + var headers = new Metadata + { + { "authorization", $"Bearer {CentralCommandKey}" }, + { "x-otopcua-cluster", ClusterId }, + }; + + await client.ReportApplyAsync( + MeshTransportDtoMapper.ToProto(ack), headers, cancellationToken: TestContext.Current.CancellationToken); + + var received = await _centralInbox.Task.WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken); + received.ShouldBeOfType().DeploymentId.ShouldBe(ack.DeploymentId); + } + + [Fact] + public async Task CorrectlyKeyed_NodeCommandExecute_SucceedsThroughTheRealAuthStack() + { + var command = new RestartDriver("C1", "driver-1", "alice", Guid.NewGuid()); + var client = new NodeCommandService.NodeCommandServiceClient(Channel()); + var headers = new Metadata { { "authorization", $"Bearer {NodeCommandKey}" } }; + + await client.ExecuteAsync( + MeshTransportDtoMapper.ToEnvelope(command), headers, cancellationToken: TestContext.Current.CancellationToken); + + var received = await _nodeInbox.Task.WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken); + received.ShouldBeOfType().DriverInstanceId.ShouldBe("driver-1"); + } + + [Fact] + public async Task FalsifiabilityControl_AnUnAnonymisedServiceMapped_RefusesEvenACorrectlyShapedCall() + { + // LocalDbSyncService carries no AllowAnonymous in this fixture (see InitializeAsync). If this + // call succeeds, the fixture's auth stack is not actually challenging unmarked endpoints and + // every "succeeds through the real auth stack" assertion above proves nothing. + var client = new ZB.MOM.WW.LocalDb.Contracts.LocalDbSync.LocalDbSyncClient(Channel()); + var headers = new Metadata { { "authorization", "Bearer irrelevant-to-this-endpoint" } }; + + using var call = client.Sync(headers, cancellationToken: TestContext.Current.CancellationToken); + await call.RequestStream.CompleteAsync(); + + await Should.ThrowAsync( + () => call.ResponseStream.MoveNext(TestContext.Current.CancellationToken)); + } + + // --------------------------------------------------------------------------------------- + // harness + // --------------------------------------------------------------------------------------- + + private GrpcChannel Channel() => GrpcChannel.ForAddress($"http://localhost:{_port}"); + + private static int GetFreePort() + { + using var listener = new System.Net.Sockets.TcpListener(IPAddress.Loopback, 0); + listener.Start(); + var port = ((IPEndPoint)listener.LocalEndpoint).Port; + listener.Stop(); + return port; + } + + /// + /// Hands out the one cluster key expects. Stands + /// in for the real MeshPskProvider (secret-store lookups, caching, the known-cluster + /// gate) — out of scope here, where the subject is the HTTP-layer auth stack, not the PSK + /// resolution chain (that is MeshPskProviderTests' job). + /// + private sealed class FixedPskProvider(string clusterId, string psk) : IMeshPskProvider + { + public bool TryGetPsk(string clusterId2, [NotNullWhen(true)] out string? psk2) + { + psk2 = string.Equals(clusterId2, clusterId, StringComparison.Ordinal) ? psk : null; + return psk2 is not null; + } + + public bool Matches(string clusterId2, string presented) => + string.Equals(clusterId2, clusterId, StringComparison.Ordinal) + && string.Equals(presented, psk, StringComparison.Ordinal); + + public void SetKnownClusters(IReadOnlyCollection clusterIds) { } + + public void Invalidate(string clusterId2) { } + + public void InvalidateAll() { } + } + + /// Completes an inbox with the first message it receives. + private sealed class CaptureActor : ReceiveActor + { + public static Props Props(TaskCompletionSource sink) => + Akka.Actor.Props.Create(() => new CaptureActor(sink)); + + public CaptureActor(TaskCompletionSource sink) => ReceiveAny(m => sink.TrySetResult(m)); + } +} From af9d3a69ac9d78632a0651abd08d8f34e55dcfd0 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 10:06:55 -0400 Subject: [PATCH 08/20] docs(plans): mark R3-01 T4 done --- .../R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json b/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json index 95fe4701..b4413b6c 100644 --- a/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json +++ b/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json @@ -27,7 +27,7 @@ { "id": "T4", "subject": "MeshGrpcAuthStackTests: PSK-bearing calls succeed through the REAL AddOtOpcUaAuth + UseAuthentication/UseAuthorization stack, with an un-anonymised negative control", - "status": "pending", + "status": "done", "blockedBy": [ "T3" ] @@ -42,7 +42,7 @@ }, { "id": "T6", - "subject": "MeshGrpcDriverOnlyPipelineTests: driver-only nodes carry no authorization middleware — pins the 03/S14 exemption", + "subject": "MeshGrpcDriverOnlyPipelineTests: driver-only nodes carry no authorization middleware \u2014 pins the 03/S14 exemption", "status": "pending", "blockedBy": [ "T3" @@ -50,7 +50,7 @@ }, { "id": "T7", - "subject": "MeshGrpcMapConventionTests: Program.cs source scan — zero direct MapGrpcService/MapZbLocalDbSync sites, exactly one mapper call", + "subject": "MeshGrpcMapConventionTests: Program.cs source scan \u2014 zero direct MapGrpcService/MapZbLocalDbSync sites, exactly one mapper call", "status": "pending", "blockedBy": [ "T1" @@ -70,7 +70,7 @@ }, { "id": "T10", - "subject": "LIVE GATE: docker-dev with DisableLogin=false — step-0 falsification control MUST fail on the pre-fix build (Deployment.Status=4), then the fixed build seals Applied on all six nodes; restore DisableLogin=true", + "subject": "LIVE GATE: docker-dev with DisableLogin=false \u2014 step-0 falsification control MUST fail on the pre-fix build (Deployment.Status=4), then the fixed build seals Applied on all six nodes; restore DisableLogin=true", "status": "pending", "blockedBy": [ "T3", From 815423bd8853e8c37765ef81df846ef0353e5c08 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 10:07:20 -0400 Subject: [PATCH 09/20] docs(plans): restore literal UTF-8 em-dashes in R3-01 tasks.json (cosmetic) --- .../plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json b/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json index b4413b6c..134df557 100644 --- a/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json +++ b/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json @@ -42,7 +42,7 @@ }, { "id": "T6", - "subject": "MeshGrpcDriverOnlyPipelineTests: driver-only nodes carry no authorization middleware \u2014 pins the 03/S14 exemption", + "subject": "MeshGrpcDriverOnlyPipelineTests: driver-only nodes carry no authorization middleware — pins the 03/S14 exemption", "status": "pending", "blockedBy": [ "T3" @@ -50,7 +50,7 @@ }, { "id": "T7", - "subject": "MeshGrpcMapConventionTests: Program.cs source scan \u2014 zero direct MapGrpcService/MapZbLocalDbSync sites, exactly one mapper call", + "subject": "MeshGrpcMapConventionTests: Program.cs source scan — zero direct MapGrpcService/MapZbLocalDbSync sites, exactly one mapper call", "status": "pending", "blockedBy": [ "T1" @@ -70,7 +70,7 @@ }, { "id": "T10", - "subject": "LIVE GATE: docker-dev with DisableLogin=false \u2014 step-0 falsification control MUST fail on the pre-fix build (Deployment.Status=4), then the fixed build seals Applied on all six nodes; restore DisableLogin=true", + "subject": "LIVE GATE: docker-dev with DisableLogin=false — step-0 falsification control MUST fail on the pre-fix build (Deployment.Status=4), then the fixed build seals Applied on all six nodes; restore DisableLogin=true", "status": "pending", "blockedBy": [ "T3", From 8471db4b1dbd7c4e67d206f7c1d519a96d05c2ac Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 10:14:55 -0400 Subject: [PATCH 10/20] test(host): no-PSK mesh calls report the interceptor's verdict, not a cookie 401/redirect Extends MeshGrpcAuthStackTests (T5): a no-bearer or wrong-bearer call to DeploymentArtifactService.Fetch, TelemetryStreamService.Subscribe and NodeCommandService.Execute fails with the INTERCEPTOR's detail string ("Config-serve authentication failed.", "Telemetry stream authentication failed.", "Node command authentication failed."), asserted alongside the status code rather than in place of it - the vacuity trap the plan calls out (ConfigServeAuthInterceptor throws Unauthenticated, and Grpc.Net.Client can map an HTTP 401 to the same status, so status alone would not discriminate a broken build). Verified red-then-green against the T1 (pre-AllowAnonymous) mapper: all 6 new theory cases fail there with StatusCode.Internal ("HTTP/2 server reset the stream, PROTOCOL_ERROR") instead of Unauthenticated/PermissionDenied - the cookie challenge breaking the h2c stream before any gRPC trailers are written, never a clean auth denial. That is the exact signature the live gate (T10) should grep for; recorded in the class remarks. --- .../MeshGrpcAuthStackTests.cs | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshGrpcAuthStackTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshGrpcAuthStackTests.cs index d55a27d3..33c27bfa 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshGrpcAuthStackTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshGrpcAuthStackTests.cs @@ -19,6 +19,7 @@ using ZB.MOM.WW.OtOpcUa.Commons.Messages.Deploy; using ZB.MOM.WW.OtOpcUa.Commons.Messages.Mesh; using ZB.MOM.WW.OtOpcUa.Commons.Protos.DeploymentArtifact.V1; using ZB.MOM.WW.OtOpcUa.Commons.Protos.MeshTransport.V1; +using ZB.MOM.WW.OtOpcUa.Commons.Protos.Telemetry.V1; using ZB.MOM.WW.OtOpcUa.Commons.Types; using ZB.MOM.WW.OtOpcUa.Configuration; using ZB.MOM.WW.OtOpcUa.Configuration.Entities; @@ -255,6 +256,88 @@ public sealed class MeshGrpcAuthStackTests : IAsyncLifetime () => call.ResponseStream.MoveNext(TestContext.Current.CancellationToken)); } + // --------------------------------------------------------------------------------------- + // T5 - status fidelity: a no-PSK / wrong-PSK call reports the INTERCEPTOR's verdict, never a + // cookie challenge. This is the vacuity trap the plan calls out: ConfigServeAuthInterceptor + // throws StatusCode.Unauthenticated, and Grpc.Net.Client also maps an HTTP 401 to + // Unauthenticated - so "the call failed with Unauthenticated" is true on a broken build AND a + // fixed one. The interceptor's DETAIL string is the only thing that discriminates them. + // + // Observed cookie-challenge signature (captured red-then-green against this same fixture, see + // MeshGrpcAuthStackTests' positive tests): the challenge does not surface as a clean + // Unauthenticated/PermissionDenied at all. Grpc.Net.Client reports + // RpcException(StatusCode.Internal, "Error starting gRPC call. HttpRequestException: The HTTP/2 + // server reset the stream. HTTP/2 error code 'PROTOCOL_ERROR' (0x1)..."), because the cookie + // handler's redirect/401 breaks the prior-knowledge h2c stream before any gRPC trailers are + // written. So a status-only assertion here would ALSO be vacuous in the other direction: a + // regressed build fails with StatusCode.Internal, never Unauthenticated/PermissionDenied, and a + // caller checking only "did it throw" cannot tell a blocked call from a refused one. Recorded + // here per the plan (T5 step 2) - this is the exact string the live gate (T10) greps for. + // --------------------------------------------------------------------------------------- + + [Theory] + [InlineData(null)] + [InlineData("Bearer the-wrong-key")] + public async Task NoOrWrongPsk_ArtifactFetch_FailsWithTheInterceptorsDetail(string? authHeader) + { + var client = new DeploymentArtifactService.DeploymentArtifactServiceClient(Channel()); + var headers = new Metadata(); + if (authHeader is not null) + headers.Add("authorization", authHeader); + + using var call = client.Fetch( + new FetchRequest { DeploymentId = Guid.NewGuid().ToString() }, + headers, + cancellationToken: TestContext.Current.CancellationToken); + + var ex = await Should.ThrowAsync( + () => call.ResponseStream.MoveNext(TestContext.Current.CancellationToken)); + + ex.StatusCode.ShouldBe(StatusCode.Unauthenticated); + ex.Status.Detail.ShouldBe("Config-serve authentication failed."); + } + + [Theory] + [InlineData(null)] + [InlineData("Bearer the-wrong-key")] + public async Task NoOrWrongPsk_TelemetrySubscribe_FailsWithTheInterceptorsDetail(string? authHeader) + { + var client = new TelemetryStreamService.TelemetryStreamServiceClient(Channel()); + var headers = new Metadata(); + if (authHeader is not null) + headers.Add("authorization", authHeader); + + using var call = client.Subscribe( + new TelemetryStreamRequest { CorrelationId = "r3-01-t5" }, + headers, + cancellationToken: TestContext.Current.CancellationToken); + + var ex = await Should.ThrowAsync( + () => call.ResponseStream.MoveNext(TestContext.Current.CancellationToken)); + + ex.StatusCode.ShouldBe(StatusCode.PermissionDenied); + ex.Status.Detail.ShouldBe("Telemetry stream authentication failed."); + } + + [Theory] + [InlineData(null)] + [InlineData("Bearer the-wrong-key")] + public async Task NoOrWrongPsk_NodeCommandExecute_FailsWithTheInterceptorsDetail(string? authHeader) + { + var command = new RestartDriver("C1", "driver-1", "alice", Guid.NewGuid()); + var client = new NodeCommandService.NodeCommandServiceClient(Channel()); + var headers = new Metadata(); + if (authHeader is not null) + headers.Add("authorization", authHeader); + + var ex = await Should.ThrowAsync(() => client.ExecuteAsync( + MeshTransportDtoMapper.ToEnvelope(command), headers, + cancellationToken: TestContext.Current.CancellationToken).ResponseAsync); + + ex.StatusCode.ShouldBe(StatusCode.PermissionDenied); + ex.Status.Detail.ShouldBe("Node command authentication failed."); + } + // --------------------------------------------------------------------------------------- // harness // --------------------------------------------------------------------------------------- From 353ed6bb5893bc63633af2549c1c2f5dd659c304 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 10:15:03 -0400 Subject: [PATCH 11/20] docs(plans): mark R3-01 T5 done --- .../plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json b/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json index 134df557..3758cda5 100644 --- a/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json +++ b/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json @@ -35,7 +35,7 @@ { "id": "T5", "subject": "Status fidelity: a no-PSK call reports the interceptor's DETAIL string, not a cookie 401/302 (ConfigServe Unauthenticated collision makes status-only assertions vacuous)", - "status": "pending", + "status": "done", "blockedBy": [ "T4" ] From 306ea1aecfc3eb87003ee5da8063b7b0e7445e0b Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 10:20:56 -0400 Subject: [PATCH 12/20] test(host): driver-only nodes carry no authorization middleware (03/S14 exemption pinned) MeshGrpcDriverOnlyPipelineTests - the driver-role shape (no AddOtOpcUaAuth, no UseAuthentication/UseAuthorization at all) behaves identically to the fused shape's T4 positive/negative tests for NodeCommandService and TelemetryStreamService: a correctly-keyed call succeeds, a no-key call gets the interceptor's PermissionDenied + detail string. Also asserts the STRUCTURAL reason it is unaffected - no IAuthorizationPolicyProvider registered at all - so a future "register auth everywhere for consistency" change fails this test before it ever reaches a live site outage. Confirmed (by temporarily swapping in the pre-T2/T3 mapper) that this class passes identically on both builds: driver-only nodes were never exposed to 03/S14 in the first place, with or without .AllowAnonymous(). --- .../MeshGrpcDriverOnlyPipelineTests.cs | 202 ++++++++++++++++++ 1 file changed, 202 insertions(+) create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshGrpcDriverOnlyPipelineTests.cs diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshGrpcDriverOnlyPipelineTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshGrpcDriverOnlyPipelineTests.cs new file mode 100644 index 00000000..3d394c43 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshGrpcDriverOnlyPipelineTests.cs @@ -0,0 +1,202 @@ +using System.Diagnostics.CodeAnalysis; +using System.Net; +using Akka.Actor; +using Akka.Hosting; +using Grpc.Core; +using Grpc.Net.Client; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Server.Kestrel.Core; +using Microsoft.Extensions.DependencyInjection; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Cluster; +using ZB.MOM.WW.OtOpcUa.Commons.Messages.Admin; +using ZB.MOM.WW.OtOpcUa.Commons.Messages.Drivers; +using ZB.MOM.WW.OtOpcUa.Commons.Messages.Mesh; +using ZB.MOM.WW.OtOpcUa.Commons.Protos.MeshTransport.V1; +using ZB.MOM.WW.OtOpcUa.Commons.Protos.Telemetry.V1; +using ZB.MOM.WW.OtOpcUa.ControlPlane.Communication; +using ZB.MOM.WW.OtOpcUa.Host.Configuration; +using ZB.MOM.WW.OtOpcUa.Runtime; +using ZB.MOM.WW.OtOpcUa.Runtime.Telemetry; + +namespace ZB.MOM.WW.OtOpcUa.Host.IntegrationTests; + +/// +/// R3-01 T6 — pins the report's "driver-only nodes are structurally unaffected" finding +/// (archreview 03/S14): a node with no admin role never registers +/// AddOtOpcUaAuth/UseAuthentication/UseAuthorization at all, so it has no +/// cookie FallbackPolicy to opt out of in the first place — the PSK interceptors were +/// always the ONLY gate on this shape, with or without .AllowAnonymous(). +/// +/// +/// A future "register auth everywhere for consistency" change would silently reintroduce 03/S14 +/// for every site node in the fleet. This class is what turns that into a failing test instead of +/// a live outage: it asserts BOTH that the driver-only mesh endpoints behave identically to the +/// fused shape's (T4) AND that the reason they do — no +/// registered at all — actually holds, so a change that adds auth services here fails on the +/// second assertion even if it somehow left the first passing. +/// +public sealed class MeshGrpcDriverOnlyPipelineTests : IAsyncLifetime +{ + private const string NodeCommandKey = "test-driveronly-node-psk"; + private const string TelemetryKey = "test-driveronly-telemetry-key"; + + private WebApplication _app = null!; + private ActorSystem _system = null!; + private int _port; + private ITelemetryLocalHub _hub = null!; + private TaskCompletionSource _nodeInbox = null!; + + /// + public async ValueTask InitializeAsync() + { + var builder = WebApplication.CreateBuilder(); + builder.Environment.ApplicationName = typeof(MeshGrpcDriverOnlyPipelineTests).Assembly.GetName().Name!; + builder.Logging.ClearProviders(); + + // Deliberately NO AddOtOpcUaAuth, NO UseAuthentication, NO UseAuthorization — this IS the + // driver-only shape (Program.cs gates all three on hasAdmin). Nothing here should need them. + builder.Services.Configure(o => o.GrpcPsk = NodeCommandKey); + builder.Services.Configure(o => o.ApiKey = TelemetryKey); + builder.Services.AddSingleton(); + + _system = ActorSystem.Create("mesh-driveronly-" + Guid.NewGuid().ToString("N")); + _nodeInbox = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var registry = new ActorRegistry(); + registry.Register(_system.ActorOf(CaptureActor.Props(_nodeInbox))); + builder.Services.AddSingleton(registry); + + // The driver-role interceptor subset only (matches Program.cs's `if (hasDriver)` branch); + // no ConfigServeAuthInterceptor/CentralCommandAuthInterceptor — those are admin-role. + builder.Services.AddGrpc(o => + { + o.Interceptors.Add(); + o.Interceptors.Add(); + }); + + _port = GetFreePort(); + builder.WebHost.ConfigureKestrel(k => k.ListenAnyIP(_port, o => o.Protocols = HttpProtocols.Http2)); + + _app = builder.Build(); + _hub = _app.Services.GetRequiredService(); + + // hasAdmin: false — the driver-only shape. syncListenPort: 0 so LocalDbSync (admin-agnostic + // but out of scope here) is not mapped; configServeGrpcPort: 0 because a driver-only node + // never carries ConfigServe. + _app.MapOtOpcUaMeshGrpcServices( + hasAdmin: false, hasDriver: true, + syncListenPort: 0, configServeGrpcPort: 0, telemetryListenPort: 1); + + await _app.StartAsync(TestContext.Current.CancellationToken); + } + + /// + public async ValueTask DisposeAsync() + { + await _app.StopAsync(TimeSpan.FromSeconds(10)); + await _app.DisposeAsync(); + await _system.Terminate().WaitAsync(TimeSpan.FromSeconds(15)); + } + + [Fact] + public void NoAuthorizationPolicyProviderIsRegistered() + { + // THE structural reason this shape is unaffected: with AddOtOpcUaAuth never called, there is + // no FallbackPolicy to opt out of. If a future change registers authorization services on a + // driver-only node, this goes red before any endpoint's behaviour does. + _app.Services.GetService().ShouldBeNull(); + } + + [Fact] + public async Task CorrectlyKeyed_NodeCommandExecute_Succeeds() + { + var command = new RestartDriver("C1", "driver-1", "alice", Guid.NewGuid()); + var client = new NodeCommandService.NodeCommandServiceClient(Channel()); + var headers = new Metadata { { "authorization", $"Bearer {NodeCommandKey}" } }; + + await client.ExecuteAsync( + MeshTransportDtoMapper.ToEnvelope(command), headers, + cancellationToken: TestContext.Current.CancellationToken); + + var received = await _nodeInbox.Task.WaitAsync( + TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken); + received.ShouldBeOfType().DriverInstanceId.ShouldBe("driver-1"); + } + + [Fact] + public async Task NoKey_NodeCommandExecute_FailsWithTheInterceptorsDetail() + { + var command = new RestartDriver("C1", "driver-1", "alice", Guid.NewGuid()); + var client = new NodeCommandService.NodeCommandServiceClient(Channel()); + + var ex = await Should.ThrowAsync(() => client.ExecuteAsync( + MeshTransportDtoMapper.ToEnvelope(command), new Metadata(), + cancellationToken: TestContext.Current.CancellationToken).ResponseAsync); + + ex.StatusCode.ShouldBe(StatusCode.PermissionDenied); + ex.Status.Detail.ShouldBe("Node command authentication failed."); + } + + [Fact] + public async Task CorrectlyKeyed_TelemetrySubscribe_Succeeds() + { + // Health is snapshot-style (ITelemetryLocalHub.Emit / class remarks): emitting BEFORE the + // Subscribe call means the new subscriber's reader is primed with it immediately, with no + // race against a live delta. + _hub.Emit(new TelemetryItem.Health(new DriverHealthChanged( + "C1", "driver-1", "Healthy", DateTime.UtcNow, null, 0, DateTime.UtcNow))); + + var client = new TelemetryStreamService.TelemetryStreamServiceClient(Channel()); + var headers = new Metadata { { "authorization", $"Bearer {TelemetryKey}" } }; + + using var call = client.Subscribe( + new TelemetryStreamRequest { CorrelationId = "r3-01-t6" }, + headers, + cancellationToken: TestContext.Current.CancellationToken); + + (await call.ResponseStream.MoveNext(TestContext.Current.CancellationToken)).ShouldBeTrue(); + call.ResponseStream.Current.DriverHealth.DriverInstanceId.ShouldBe("driver-1"); + } + + [Fact] + public async Task NoKey_TelemetrySubscribe_FailsWithTheInterceptorsDetail() + { + var client = new TelemetryStreamService.TelemetryStreamServiceClient(Channel()); + + using var call = client.Subscribe( + new TelemetryStreamRequest { CorrelationId = "r3-01-t6-nokey" }, + cancellationToken: TestContext.Current.CancellationToken); + + var ex = await Should.ThrowAsync( + () => call.ResponseStream.MoveNext(TestContext.Current.CancellationToken)); + + ex.StatusCode.ShouldBe(StatusCode.PermissionDenied); + ex.Status.Detail.ShouldBe("Telemetry stream authentication failed."); + } + + // --------------------------------------------------------------------------------------- + // harness + // --------------------------------------------------------------------------------------- + + private GrpcChannel Channel() => GrpcChannel.ForAddress($"http://localhost:{_port}"); + + private static int GetFreePort() + { + using var listener = new System.Net.Sockets.TcpListener(IPAddress.Loopback, 0); + listener.Start(); + var port = ((IPEndPoint)listener.LocalEndpoint).Port; + listener.Stop(); + return port; + } + + /// Completes an inbox with the first message it receives. + private sealed class CaptureActor : ReceiveActor + { + public static Props Props(TaskCompletionSource sink) => + Akka.Actor.Props.Create(() => new CaptureActor(sink)); + + public CaptureActor(TaskCompletionSource sink) => ReceiveAny(m => sink.TrySetResult(m)); + } +} From 9bd3366b28041a9a6a1a8aae333ecdd26391e85b Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 10:21:01 -0400 Subject: [PATCH 13/20] docs(plans): mark R3-01 T6 done --- .../plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json b/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json index 3758cda5..9d1cc850 100644 --- a/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json +++ b/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json @@ -43,7 +43,7 @@ { "id": "T6", "subject": "MeshGrpcDriverOnlyPipelineTests: driver-only nodes carry no authorization middleware — pins the 03/S14 exemption", - "status": "pending", + "status": "done", "blockedBy": [ "T3" ] From 82ad98e1a9e8cd4754a472c129c49816fa8e591b Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 10:24:49 -0400 Subject: [PATCH 14/20] chore(docker-dev): parameterise Security__Auth__DisableLogin on the central pair Security__Auth__DisableLogin: "${OTOPCUA_DISABLE_LOGIN:-true}" on central-1 and central-2 - byte-identical default behaviour, verified via `docker compose config` both unset (resolves "true") and with OTOPCUA_DISABLE_LOGIN=false (resolves "false"). DisableLogin's always-succeeding handler, registered under the cookie scheme name, masks the AdminUI cookie FallbackPolicy entirely - which is exactly how 03/S14 (the FallbackPolicy blocking every mesh gRPC call on a login-enabled admin node) survived seven phases of prior live gates that all ran with the default. The production-faithful mode is now a one-variable flip instead of an edit-and-remember-to-revert; header comment names the flip and the LDAP users it needs (archreview 03/S14, R3-01 T8). --- docker-dev/docker-compose.yml | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/docker-dev/docker-compose.yml b/docker-dev/docker-compose.yml index 97abc07c..73aec571 100644 --- a/docker-dev/docker-compose.yml +++ b/docker-dev/docker-compose.yml @@ -90,6 +90,21 @@ # prior dark-switch shape). The ClusterNode.GrpcPort column (seeded by # docker-dev/seed/seed-clusters.sql) points central's dial supervisor at each node's :4056. # +# Production-faithful auth (R3-01, archreview 03/S14): central-1/central-2's +# Security__Auth__DisableLogin defaults to "true" here (byte-identical to the previous hardcoded +# value) but is now `${OTOPCUA_DISABLE_LOGIN:-true}` — a one-variable flip to the REAL cookie +# login flow instead of an edit-and-remember-to-revert. DisableLogin registers an always-succeeding +# handler UNDER the cookie scheme name, so every live gate run with the default masks the AdminUI +# cookie FallbackPolicy entirely — which is exactly how 03/S14 (the FallbackPolicy blocking every +# mesh gRPC call on a login-enabled admin node) survived seven phases of prior live gates. To drive +# this rig production-faithfully: +# OTOPCUA_DISABLE_LOGIN=false docker compose -f docker-dev/docker-compose.yml up -d central-1 central-2 +# Sign in as `alice` / `password` (or `bob`) against the shared GLAuth (10.100.0.35:3893, +# dc=zb,dc=local) — the same LDAP users every other node in this file already authenticates OPC UA +# sessions against. Restore the default with `docker compose ... up -d central-1 central-2` (no env +# var). See docs/security.md "Mesh gRPC ingress is AllowAnonymous by design" for what this actually +# exercises. +# # Usage: # docker compose -f docker-dev/docker-compose.yml up -d --build # open http://localhost:9200 # central Blazor admin UI @@ -382,7 +397,7 @@ services: Security__Ldap__ServiceAccountDn: "cn=serviceaccount,dc=zb,dc=local" Security__Ldap__ServiceAccountPassword: "serviceaccount123" Security__DeployApiKey: "docker-dev-deploy-key" - Security__Auth__DisableLogin: "true" + Security__Auth__DisableLogin: "${OTOPCUA_DISABLE_LOGIN:-true}" # Map the shared-GLAuth data-plane groups (ReadOnly/WriteOperate/...) to data-plane role # strings the OPC UA session ACL gates read. The DB LdapGroupRoleMapping is AdminRole-typed # (AdminUI roles); this free-form GroupToRole baseline is the path for data-plane roles. @@ -508,7 +523,7 @@ services: Security__Ldap__ServiceAccountDn: "cn=serviceaccount,dc=zb,dc=local" Security__Ldap__ServiceAccountPassword: "serviceaccount123" Security__DeployApiKey: "docker-dev-deploy-key" - Security__Auth__DisableLogin: "true" + Security__Auth__DisableLogin: "${OTOPCUA_DISABLE_LOGIN:-true}" # Data-plane group→role baseline (see central-1). Security__Ldap__GroupToRole__ReadOnly: "ReadOnly" Security__Ldap__GroupToRole__WriteOperate: "WriteOperate" From f3832aa48c7364f0f466689f0bfe177541a2e283 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 10:25:02 -0400 Subject: [PATCH 15/20] test(host): a mesh gRPC map added outside MeshGrpcEndpoints fails the build MeshGrpcMapConventionTests - source-scan of Program.cs (solution-root walk, following FlushLoggingConventionTests.cs's idiom): zero direct MapGrpcService +/// R3-01 T7 — a source-scan guard, following the solution-root-walk idiom +/// ZB.MOM.WW.OtOpcUa.Driver.S7.Cli.Tests.FlushLoggingConventionTests uses. Complements the +/// endpoint-metadata census (MeshGrpcAnonymousMetadataTests): the census catches a map +/// added to MeshGrpcEndpoints without the AllowAnonymous convention, but it cannot +/// catch a map added to Program.cs that bypasses the helper entirely — that shape never +/// goes through MapOtOpcUaMeshGrpcServices at all, so the census would simply never see it. +/// +/// +/// Would have FAILED at HEAD before T1: Program.cs used to carry the five map sites +/// inline (MapZbLocalDbSync(), four MapGrpcService<T>() calls) with no +/// MapOtOpcUaMeshGrpcServices call at all. +/// +public sealed class MeshGrpcMapConventionTests +{ + private static readonly string ProgramCsPath = LocateProgramCs(); + + [Fact] + public void ProgramCs_ContainsNoDirectMeshGrpcMapSites_AndExactlyOneMapperCall() + { + var source = File.ReadAllText(ProgramCsPath); + + // Guard-of-the-guard: a bad path (an empty file, or one that never had the mapper in the + // first place) must not pass this test by finding nothing to count. + source.ShouldNotBeNullOrWhiteSpace(); + source.ShouldContain("MapOtOpcUaMeshGrpcServices("); + + CountOccurrences(source, "MapGrpcService<").ShouldBe(0, + "every MapGrpcService() call must live inside MeshGrpcEndpoints.MapOtOpcUaMeshGrpcServices, " + + "not inline in Program.cs (03/S14 — a map added outside the helper is invisible to the " + + "endpoint-metadata census)."); + CountOccurrences(source, "MapZbLocalDbSync(").ShouldBe(0, + "the LocalDb sync map must live inside MeshGrpcEndpoints.MapOtOpcUaMeshGrpcServices, not inline " + + "in Program.cs, for the same reason as MapGrpcService() above."); + CountOccurrences(source, "MapOtOpcUaMeshGrpcServices(").ShouldBe(1, + "Program.cs must call the mapper exactly once; more than one call would map the mesh gRPC " + + "services twice (a route conflict) and zero calls would drop them entirely."); + } + + private static int CountOccurrences(string source, string needle) + { + var count = 0; + var index = 0; + while ((index = source.IndexOf(needle, index, StringComparison.Ordinal)) >= 0) + { + count++; + index += needle.Length; + } + + return count; + } + + private static string LocateProgramCs() + { + var dir = new DirectoryInfo(AppContext.BaseDirectory); + while (dir is not null && !File.Exists(Path.Combine(dir.FullName, "ZB.MOM.WW.OtOpcUa.slnx"))) + dir = dir.Parent; + dir.ShouldNotBeNull("Could not find solution root (ZB.MOM.WW.OtOpcUa.slnx)."); + return Path.Combine(dir!.FullName, "src", "Server", "ZB.MOM.WW.OtOpcUa.Host", "Program.cs"); + } +} From 4dde5cb8bf8356a61c0167e9418159d6f11dc84a Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 10:25:12 -0400 Subject: [PATCH 16/20] docs(plans): mark R3-01 T7 and T8 done --- .../plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json b/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json index 9d1cc850..799e0fa9 100644 --- a/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json +++ b/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json @@ -51,7 +51,7 @@ { "id": "T7", "subject": "MeshGrpcMapConventionTests: Program.cs source scan — zero direct MapGrpcService/MapZbLocalDbSync sites, exactly one mapper call", - "status": "pending", + "status": "done", "blockedBy": [ "T1" ] @@ -59,7 +59,7 @@ { "id": "T8", "subject": "docker-dev: Security__Auth__DisableLogin parameterised as ${OTOPCUA_DISABLE_LOGIN:-true} on central-1/central-2 + header docs", - "status": "pending", + "status": "done", "blockedBy": [] }, { From a222421374a251e7bbf5265b3328f3821877ca59 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 10:27:40 -0400 Subject: [PATCH 17/20] docs: mesh gRPC ingress is AllowAnonymous; the PSK interceptors are the gate (R3-01) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New docs/security.md section "Mesh gRPC ingress is AllowAnonymous by design" (archreview 03/S14): what the cookie FallbackPolicy covers, why it can block a correctly-PSK'd call before the interceptor's verdict matters, why the AllowAnonymous opt-out is safe (all five interceptors fail-closed), the affected-endpoint table (fused-vs-driver-only distinction), and the two standing guards (MeshGrpcAnonymousMetadataTests, MeshGrpcMapConventionTests) plus the auth-stack/driver-only boundary tests. One cross-reference line each in docs/Configuration.md (§MeshTransport, §ConfigSource/ConfigServe) and docs/Telemetry.md, so an operator reading about the listener ports meets the constraint where they'd look for it. --- docs/Configuration.md | 4 +++ docs/Telemetry.md | 8 +++++ docs/security.md | 73 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+) diff --git a/docs/Configuration.md b/docs/Configuration.md index 2bccc843..2adcc05f 100644 --- a/docs/Configuration.md +++ b/docs/Configuration.md @@ -190,6 +190,8 @@ On the docker-dev rig every node carries these keys already; the rig's compose d See the [ClusterClient→gRPC migration design](plans/2026-08-06-clusterclient-to-grpc-design.md), [implementation plan](plans/2026-08-06-clusterclient-to-grpc.md), [tracking doc](plans/2026-08-06-clusterclient-to-grpc-tracking.md) and [live gate](plans/2026-08-06-clusterclient-to-grpc-live-gate.md) for the migration's phase-by-phase history — those are historical records of the cutover and are not updated further by this sweep. +**On an admin-role node, `NodeCommandService` and `CentralCommandService` are `AllowAnonymous` by design** — the AdminUI cookie `FallbackPolicy` would otherwise 401/redirect a correctly-PSK'd call before the interceptor above ever ran (archreview 03/S14). See [`security.md` § Mesh gRPC ingress is `AllowAnonymous` by design](security.md#mesh-grpc-ingress-is-allowanonymous-by-design). + ### `ConfigSource` / `ConfigServe` (config fetch-and-cache) - **Purpose:** [per-cluster mesh](plans/2026-07-21-per-cluster-mesh-design.md) **Phase 3**. `ConfigSource:Mode` selects where a **driver** node reads its deployed configuration: `Direct` (read `Deployment.ArtifactBlob` from central SQL, today's behaviour) or `FetchAndCache` (fetch the artifact bytes from central over a gRPC stream, verify SHA-256, cache in LocalDb, and read **only** the cache — no central-SQL config read). `ConfigServe` is the **central** (admin-role) serve side that a `FetchAndCache` node dials. @@ -217,6 +219,8 @@ See the [ClusterClient→gRPC migration design](plans/2026-08-06-clusterclient-t **`RevisionHash == SHA-256(artifact bytes)` is load-bearing.** `ConfigComposer` computes the deployment's revision hash as the lowercase-hex SHA-256 of the exact artifact bytes, so the node verifies a fetch against the `RevisionHash` the dispatch already carries — no hash rides in the proto. If a future change makes the revision hash anything else, every fetch fails verification. +**`DeploymentArtifactService` is `AllowAnonymous` by design** — same reasoning as `NodeCommandService`/`CentralCommandService` above: the AdminUI cookie `FallbackPolicy` on an admin-role node would otherwise block a correctly-keyed fetch before `ConfigServeAuthInterceptor`'s verdict matters. See [`security.md` § Mesh gRPC ingress is `AllowAnonymous` by design](security.md#mesh-grpc-ingress-is-allowanonymous-by-design). + #### Artifact compression (2026-08-15) The artifact blob is **gzip-compressed, inside the hashed bytes**. `ConfigComposer.Pack` gzips the diff --git a/docs/Telemetry.md b/docs/Telemetry.md index d1364b46..5caae91a 100644 --- a/docs/Telemetry.md +++ b/docs/Telemetry.md @@ -190,6 +190,14 @@ transports. ScadaBridge itself has since closed that gap with this identical interceptor pattern, so Phase 5 ships authenticated from the start rather than deferring auth to a later hardening pass. See the design doc's superseded note for the full history. +**On a fused `admin,driver` node, `TelemetryStreamService` and `NodeCommandService` are `AllowAnonymous` +by design** — a *different* gate from the interceptor above. `TelemetryStreamAuthInterceptor` is the PSK +check; on an admin-role node the AdminUI cookie `FallbackPolicy` sits in front of it and would 401/redirect +a correctly-keyed call before the interceptor's verdict matters, because it runs in the authorization +middleware, strictly *before* the interceptor executes (archreview 03/S14). A driver-only node never +registers that middleware at all, so it is structurally unaffected either way. See +[`security.md` § Mesh gRPC ingress is `AllowAnonymous` by design](security.md#mesh-grpc-ingress-is-allowanonymous-by-design). + ## Upgrading an existing deployment: populate `ClusterNode.GrpcPort` first (#493) `GrpcPort` is a **nullable column added by Phase 5**, and nothing backfills it onto rows that diff --git a/docs/security.md b/docs/security.md index 6872fbaf..bd44294b 100644 --- a/docs/security.md +++ b/docs/security.md @@ -846,6 +846,79 @@ Responses: `202 Accepted` (`{ outcome, deploymentId, revisionHash }`) when a dep --- +## Mesh gRPC ingress is `AllowAnonymous` by design + +Archreview 03/S14 (Critical, closed R3-01). On an admin-role node `AddOtOpcUaAuth` installs a global +`FallbackPolicy = RequireAuthenticatedUser` bound to the cookie scheme (the same fallback that protects +every AdminUI page above). ASP.NET Core's authorization middleware applies that fallback to **every +endpoint that carries no authorization metadata of its own** — gRPC endpoints included. The five mesh +gRPC endpoints (config-serve artifact fetch, central command ack, telemetry stream, node command, LocalDb +sync) authenticate with a per-cluster/per-node **PSK bearer inside a gRPC interceptor**, and that +interceptor runs *inside* the endpoint invocation — i.e. strictly *after* the authorization middleware. +Without an explicit `.AllowAnonymous()` opt-out, a correctly-keyed node call is challenged by the cookie +handler before the interceptor's verdict ever matters: on a login-enabled admin node this is a fleet-wide +deploy-plane outage (artifact fetches fail, deploy acks are dropped, deployments seal `TimedOut`), and it +went unnoticed for seven phases of prior live gates because the docker-dev rig always ran +`Security__Auth__DisableLogin=true` (an always-succeeding handler registered *under the cookie scheme +name*, so the fallback trivially passes) and every prior mesh-gRPC test built its own auth-free pipeline +by hand. + +**Why `AllowAnonymous` is the correct fix, not a hole.** All five interceptors are fail-closed: an unset +key rejects every call, and the presented token is compared with `CryptographicOperations.FixedTimeEquals`. +`Program.cs:574` states the design contract directly — "the interceptor is the ONLY inbound auth on any +endpoint (the libraries verify nothing)" — which was already true of the gRPC pipeline; the missing opt-out +made it false of the HTTP pipeline in front of it. The pattern itself predates this fix: static assets +(`MapStaticAssets().AllowAnonymous()`), the metrics endpoint (`MapZbMetrics().AllowAnonymous()`), and the +headless deploy API above are all `AllowAnonymous` for the identical reason — each is gated by something +other than the cookie, and the fallback would otherwise 401/redirect it regardless. + +| Endpoint | Role gate | Affected without `AllowAnonymous`, on a login-enabled node | +|---|---|---| +| `DeploymentArtifactService.Fetch` | `hasAdmin && ConfigServe:GrpcListenPort > 0` | Always — every `FetchAndCache` node's artifact fetch. | +| `CentralCommandService.ReportApply` | same | Always — every node's deploy ack, from every cluster. | +| `TelemetryStreamService.Subscribe` | `hasDriver && Telemetry:GrpcListenPort > 0` | Only on a **fused** `admin,driver` node (e.g. docker-dev's central pair) — a driver-only node never registers the cookie stack at all, so it is structurally unaffected either way. | +| `NodeCommandService.Execute` | same | Only on a fused node, same reasoning. | +| `localdb_sync.v1.LocalDbSync/Sync` | `hasDriver && LocalDb:SyncListenPort > 0` | Only on a fused node that also sets a sync port — no node in the fleet currently has that shape, but the map is anonymised anyway. | + +**A fused central pair is affected on its OWN dispatch, not only inbound from sites.** `CentralCommunicationActor` dials every enabled `ClusterNode` row, its own pair included, and central's telemetry dialer dials its own pair too — so a login-enabled fused central breaks its own command/telemetry plane, not just the sites'. + +**Mapping mechanics.** All five maps live in one place, `MeshGrpcEndpoints.MapOtOpcUaMeshGrpcServices` +(`src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/MeshGrpcEndpoints.cs`) — `Program.cs` calls it once and +carries no map sites of its own. Four maps chain `.AllowAnonymous()` directly onto `MapGrpcService()`. +The fifth, `MapZbLocalDbSync()`, returns `IEndpointRouteBuilder` rather than an +`IEndpointConventionBuilder`, so it cannot take the same chain; it is wrapped in an empty-prefix +`app.MapGroup(string.Empty).AllowAnonymous().MapZbLocalDbSync()`, which carries the convention without +changing the wire route pattern. + +**Two standing guards**, both in `tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/`, so a future map +cannot silently reopen this: + +- `MeshGrpcAnonymousMetadataTests` — an endpoint-metadata census that calls the SAME + `MapOtOpcUaMeshGrpcServices` production helper across all three role shapes (admin-only, driver-only, + fused) and asserts every mesh endpoint carries `IAllowAnonymous`, with an exact expected count per shape + and a scope check that the opt-out never leaks onto an unrelated endpoint. +- `MeshGrpcMapConventionTests` — a `Program.cs` source scan asserting zero direct `MapGrpcService Date: Thu, 20 Aug 2026 10:27:45 -0400 Subject: [PATCH 18/20] docs(plans): mark R3-01 T9 done; T10 remains pending (live gate, orchestrator-run) --- .../plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json b/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json index 799e0fa9..9464b7e2 100644 --- a/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json +++ b/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json @@ -65,7 +65,7 @@ { "id": "T9", "subject": "docs/security.md \"Mesh gRPC ingress is AllowAnonymous by design\" + cross-refs in docs/Configuration.md and docs/Telemetry.md", - "status": "pending", + "status": "done", "blockedBy": [] }, { From b725788954d8beaa26491b9581fbc7aee755a495 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 10:28:55 -0400 Subject: [PATCH 19/20] docs(plans): R3-01 execution notes (T1-T9) - filter gotcha, T4/T5 fixture scope, observed cookie-challenge signature --- .../R3-01-mesh-grpc-allowanonymous-plan.md | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md b/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md index a0400ede..4f552be4 100644 --- a/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md +++ b/archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md @@ -352,3 +352,87 @@ T10 last — and its step-0 control runs on master BEFORE the branch is merged ``` Overall effort: **~half a dev-day code+tests, plus the two-pass rig session.** The Critical itself is five lines; everything else in this plan exists because the five lines were invisible to a seven-phase live-gate program. + +--- + +## Execution notes (T1–T9, 2026-08-20) + +Branch `fix/r3-01-mesh-grpc-allowanonymous`, one commit per task as specified. T1–T9 all landed; +T10 (live gate) left `pending` for the orchestrator per the run brief. Every code-touching task +(T1–T7) was verified red-before-fix by hand — for T2–T5 by asserting against the un-fixed +`MeshGrpcEndpoints.cs`/`Program.cs` (via a temporary `git show :` swap, +rebuild, run, then restore — confirmed byte-identical restore via `git diff --stat` each time) +and observing the expected failure before reapplying the fix. Two things surfaced during that +process that are worth recording: + +1. **The census's endpoint filter could not stay a "route pattern starts with `/`" heuristic once + T3's empty-prefix `MapGroup` landed.** `RealMethodEndpoints` in `MeshGrpcAnonymousMetadataTests` + originally filtered on `RoutePattern.RawText?.StartsWith('/')` to exclude grpc-dotnet's own + per-app "Unimplemented service" catch-all and per-service "Unimplemented method for X" + fallback (neither carries a route pattern starting with `/` in the ungrouped case). Once + LocalDbSync moved behind `app.MapGroup(string.Empty).AllowAnonymous().MapZbLocalDbSync()`, + those SAME fallback patterns *also* gained a leading `/` inside the group, so the heuristic + started over-counting (`DriverOnly_WithASyncPort_AlsoMapsTheLocalDbSyncEndpoint` and + `LocalDbSync_AllowAnonymousGroup_LeavesTheRoutePatternByteIdentical` both went red for the + wrong reason — 3 endpoints found where 1 was expected). Fixed by matching against the exact + known set of five method route-pattern literals instead of a shape heuristic. Not a plan defect + — the plan didn't specify the filter mechanism — but a genuine gotcha for anyone extending this + census: **grpc-dotnet's route-pattern-shape for its own fallback endpoints is not stable across + a plain `MapGrpcService` vs. a grouped one**, so don't filter on shape once any mesh map moves + behind a group. + +2. **T4's fixture does not map or wire `LocalDbSyncService`'s real dependencies.** The plan's + fixture description says "maps the mesh services through `MapOtOpcUaMeshGrpcServices`"; T4's + `MeshGrpcAuthStackTests` calls it with `syncListenPort: 0` (LocalDbSync NOT mapped by the + helper) and instead hand-maps the library's `LocalDbSyncService` directly, with no + `.AllowAnonymous()`, purely as the negative control. This is a minimal, deliberate narrowing: + no assertion in T4 or T5 exercises LocalDbSync's real RPC (the plan's own positive/negative + lists never name it), and standing up its DI graph (`AddZbLocalDb`/`AddZbLocalDbReplication`, + a temp-file SQLite `ILocalDb`) would have added fixture weight with nothing to verify — the + authorization middleware refuses an unauthorized call before the service is ever constructed, + so the DI graph doesn't need to exist for the negative control to be meaningful. Recorded here + because it's a case where the fixture is deliberately narrower than a literal reading of the + plan's prose, even though every named assertion is still covered. + +3. **The positive/negative service lists T4 and T5 assert are asymmetric, and that asymmetry is + the plan's own, not introduced here.** T4's positive list is {`DeploymentArtifactService.Fetch`, + `CentralCommandService.ReportApply`, `NodeCommandService.Execute`} — Telemetry is not asserted + positively there. T5's no-PSK detail-string list is {Config-serve, Telemetry stream, Node + command} — `CentralCommandService`'s own detail string + (`"Central command authentication failed."`) is never asserted anywhere in T4/T5. Implemented + exactly as specified (no scope widening per the run brief); flagging the gap in case a + follow-up wants a `NoOrWrongPsk_CentralCommandReportApply_FailsWithTheInterceptorsDetail` case + for full symmetry — trivial to add given the fixture already maps and keys that service. + +4. **Observed cookie-challenge signature (T5 step 2), captured directly against this repo's + fixture stack (macOS, Kestrel h2c, `Grpc.Net.Client` over `HttpClient`/`SocketsHttpHandler`):** + every blocked positive-test call in the red state failed as + `RpcException(StatusCode.Internal, "Error starting gRPC call. HttpRequestException: The HTTP/2 + server reset the stream. HTTP/2 error code 'PROTOCOL_ERROR' (0x1). (HttpProtocolError) + HttpProtocolException: The HTTP/2 server reset the stream. HTTP/2 error code 'PROTOCOL_ERROR' + (0x1). (HttpProtocolError)")` — never `Unauthenticated`/`PermissionDenied`. The cookie handler's + redirect/401 breaks the prior-knowledge h2c stream before any gRPC trailers are written, so the + client sees a transport-level reset rather than a clean gRPC status. **The live gate (T10) + should grep node/central logs and browser network traces for `PROTOCOL_ERROR`, `/login`, `302` + and `401` together** — not for `401` alone, and not for a gRPC `Unauthenticated`/ + `PermissionDenied` status, which this environment never actually produces for the blocked case. + (A real reverse-proxy in front of a production deployment could still surface a clean HTTP 401 + instead of a stream reset, depending on how it terminates HTTP/2 — the plan's own multi-signal + grep advice already covers that; this note is the empirical confirmation of one concrete shape.) + +5. **T6 passes identically on both the pre-fix and post-fix mapper (confirmed by the same + temporary-swap procedure).** Driver-only nodes never register `AddOtOpcUaAuth` at all, so + `.AllowAnonymous()` presence or absence on the mesh maps has zero effect on that shape — T6 is + a structural pin/regression-control test, not a red-then-green TDD test for a behaviour T1–T3 + changed. This matches the plan's own "standard (test-only)" classification; recorded because + the run brief asked for red-before-fix confirmation to be checked (and reported) for every task, + and this is the one task where "red" never existed to begin with. + +6. **T8's `docker compose config` verification ran locally on macOS (OrbStack-backed Docker + Compose v5.1.2)** — no Windows-VM `lmxopcua-fix` helper needed, consistent with the Mac-path + guidance in CLAUDE.md. Confirmed both branches: unset `OTOPCUA_DISABLE_LOGIN` resolves + `Security__Auth__DisableLogin: "true"` on both central-1/central-2; `OTOPCUA_DISABLE_LOGIN=false` + resolves `"false"` on both. + +No other deviations from the plan's design or task breakdown. T10 was not run — left `pending` +per the run brief, for the orchestrator. From c8cf5c8032a61411fd8bf29401cf59f95ce8dbbe Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 10:43:49 -0400 Subject: [PATCH 20/20] test(host): the R3-01 falsifiability control asserts the cookie-challenge signature (a bare Exception was satisfiable by an anonymised endpoint too); docs wording --- docs/security.md | 4 ++-- .../MeshGrpcAuthStackTests.cs | 8 +++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/docs/security.md b/docs/security.md index bd44294b..d313d65e 100644 --- a/docs/security.md +++ b/docs/security.md @@ -890,7 +890,7 @@ The fifth, `MapZbLocalDbSync()`, returns `IEndpointRouteBuilder` rather than an `app.MapGroup(string.Empty).AllowAnonymous().MapZbLocalDbSync()`, which carries the convention without changing the wire route pattern. -**Two standing guards**, both in `tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/`, so a future map +**Three standing guards**, all in `tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/`, so a future map cannot silently reopen this: - `MeshGrpcAnonymousMetadataTests` — an endpoint-metadata census that calls the SAME @@ -899,7 +899,7 @@ cannot silently reopen this: and a scope check that the opt-out never leaks onto an unrelated endpoint. - `MeshGrpcMapConventionTests` — a `Program.cs` source scan asserting zero direct `MapGrpcService( + // Review (R3-01): asserting a bare Exception was vacuous — an anonymised endpoint ALSO throws + // here (handler deps are not wired in this fixture => RpcException Unknown). Only the + // cookie-challenge signature discriminates: the FallbackPolicy's 302 to /login is a + // non-gRPC response, surfaced by grpc-dotnet as Internal + PROTOCOL_ERROR. + var ex = await Should.ThrowAsync( () => call.ResponseStream.MoveNext(TestContext.Current.CancellationToken)); + ex.StatusCode.ShouldBe(StatusCode.Internal); + ex.Status.Detail.ShouldContain("PROTOCOL_ERROR"); } // ---------------------------------------------------------------------------------------