From e3cf363703a2de2dd7ff94498535db3153bb7cf9 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 09:51:33 -0400 Subject: [PATCH] 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!;