From 5f81e65684324059a3f05caa384791201837261c Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 7 Aug 2026 06:52:26 -0400 Subject: [PATCH] fix(secrets): route the unset-token harness through the real hub composition (review drift-risk) An internal validateEagerly seam on AddZbSecretsGrpcHub replaces the harness's hand-copied registration fragment, so the deny-all second-layer tests exercise the production wiring and cannot drift from it. Claude-Session: https://claude.ai/code/session_014WNM4vjoVksyyBraTXSZE1 --- .../SecretsGrpcHubServerExtensions.cs | 26 +++++++++++++---- .../SecretsHubTestHarness.cs | 29 ++++++++++--------- 2 files changed, 36 insertions(+), 19 deletions(-) diff --git a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.Grpc/DependencyInjection/SecretsGrpcHubServerExtensions.cs b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.Grpc/DependencyInjection/SecretsGrpcHubServerExtensions.cs index 8026aad..d799cf6 100644 --- a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.Grpc/DependencyInjection/SecretsGrpcHubServerExtensions.cs +++ b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.Grpc/DependencyInjection/SecretsGrpcHubServerExtensions.cs @@ -43,7 +43,20 @@ public static class SecretsGrpcHubServerExtensions public static IServiceCollection AddZbSecretsGrpcHub( this IServiceCollection services, IConfiguration configuration, - string sectionPath = "Secrets:GrpcHub") + string sectionPath = "Secrets:GrpcHub") => + AddZbSecretsGrpcHubCore(services, configuration, sectionPath, validateEagerly: true); + + /// + /// The whole composition, with the eager validation optionally skipped. Internal so the test + /// harness can exercise the interceptor's deny-all second layer over the wire through the REAL + /// registration path — a hand-copied fragment of this method in the harness would silently + /// drift when this method changes. Production callers always validate eagerly. + /// + internal static IServiceCollection AddZbSecretsGrpcHubCore( + IServiceCollection services, + IConfiguration configuration, + string sectionPath, + bool validateEagerly) { ArgumentNullException.ThrowIfNull(services); ArgumentNullException.ThrowIfNull(configuration); @@ -74,10 +87,13 @@ public static class SecretsGrpcHubServerExtensions // startup failure and the runtime warning both name the key this host actually reads. services.PostConfigure(options => options.SectionPath = sectionPath); - var eager = new SecretsGrpcHubServerOptions { SectionPath = sectionPath }; - section.Bind(eager); - eager.SectionPath = sectionPath; - eager.Validate(); + if (validateEagerly) + { + var eager = new SecretsGrpcHubServerOptions { SectionPath = sectionPath }; + section.Bind(eager); + eager.SectionPath = sectionPath; + eager.Validate(); + } // Singleton so the "unconfigured" warning is emitted once per process rather than once per // interceptor instance; the gRPC interceptor activator prefers a registered instance. diff --git a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.Grpc.Tests/SecretsHubTestHarness.cs b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.Grpc.Tests/SecretsHubTestHarness.cs index e87b64f..2517165 100644 --- a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.Grpc.Tests/SecretsHubTestHarness.cs +++ b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.Grpc.Tests/SecretsHubTestHarness.cs @@ -216,23 +216,24 @@ internal sealed class SecretsHubTestServer : IAsyncDisposable /// /// /// AddZbSecretsGrpcHub refuses this configuration at registration — which is the intended - /// first line of defence and is pinned elsewhere — so the pieces are wired here the way that - /// extension wires them, minus the eager validation. That is precisely the case the interceptor's - /// deny-all exists for: a host that built the options some other way. Without this path the - /// second layer could only ever be tested against a hand-constructed interceptor, never over the - /// wire. + /// first line of defence and is pinned elsewhere — so this path goes through the REAL composition + /// via its internal validateEagerly: false seam, changing nothing else. That is precisely + /// the case the interceptor's deny-all exists for: a host that built the options some other way. + /// Without this path the second layer could only ever be tested against a hand-constructed + /// interceptor, never over the wire. /// /// The store the hub would serve. /// The running hub. - public static Task StartWithNoTokenConfiguredAsync(ISecretStore store) => - StartAsync(store, services => - { - services.Configure(options => options.BearerToken = string.Empty); - services.AddSingleton(); - services.AddGrpc() - .AddServiceOptions( - options => options.Interceptors.Add()); - }); + public static Task StartWithNoTokenConfiguredAsync(ISecretStore store) + { + IConfiguration configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary()) + .Build(); + + return StartAsync(store, services => + SecretsGrpcHubServerExtensions.AddZbSecretsGrpcHubCore( + services, configuration, "Secrets:GrpcHub", validateEagerly: false)); + } private static async Task StartAsync( ISecretStore store, Action registerHub)