From aebdd56b525685eed82f60e45a7bf422a6f28481 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 7 Aug 2026 10:47:10 -0400 Subject: [PATCH] =?UTF-8?q?docs(secrets):=20review=20follow-ups=20?= =?UTF-8?q?=E2=80=94=20pre-Serilog=20window=20note,=20ThrowIfNull=20hygien?= =?UTF-8?q?e,=20honest=20site-purity=20scan=20bound?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The expander comment now records that a SQL outage at central boot exits pre-Serilog with a bare stderr trace (honest, restart-retryable, unenriched) and why the role-reading predicate deliberately stays in Program.cs rather than SecretsRegistration (that class refuses config-read roles by design). EnsureCentralSharedStoreConnectionString gains the file's standard ThrowIfNull. The site-purity scan's doc no longer overclaims: two factory-lambda descriptors evade it individually; the pin holds because three concrete-type registrations from the same call cannot. Claude-Session: https://claude.ai/code/session_014WNM4vjoVksyyBraTXSZE1 --- src/ZB.MOM.WW.ScadaBridge.Host/Program.cs | 9 +++++++++ src/ZB.MOM.WW.ScadaBridge.Host/SecretsRegistration.cs | 2 ++ .../SecretsReplicationWiringTests.cs | 11 ++++++++--- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs b/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs index 8eb7deff..e8ab8291 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs @@ -64,6 +64,15 @@ var configuration = new ConfigurationBuilder() // EnsureCentralSharedStoreConnectionString check AddScadaBridgeSecrets runs — this is the // earliest code that would otherwise hand the bad value to SqlConnection, whose generic // "initialization string" format error would bury the designed message. +// +// Two consequences worth knowing. This block runs BEFORE Serilog exists and outside the +// try/catch below, so a SQL Server outage at central boot exits with a bare stderr stack +// trace, no structured log — honest and container-restart-retryable, just unenriched; the +// SQLite path had the same window but its only failure mode was a local file. And the +// role check is deliberately NOT folded into SecretsRegistration: that class refuses to +// read the node role from configuration (a config-read role could silently turn a site +// into a hub — see SecretsNodeRole), while Program.cs must read it to branch the whole +// composition anyway, so the config read stays here at the composition root. var expanderUsesSharedSqlStore = string.Equals( configuration["ScadaBridge:Node:Role"], "Central", StringComparison.OrdinalIgnoreCase) diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/SecretsRegistration.cs b/src/ZB.MOM.WW.ScadaBridge.Host/SecretsRegistration.cs index 9ecb761a..cb18cec3 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/SecretsRegistration.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/SecretsRegistration.cs @@ -199,6 +199,8 @@ public static class SecretsRegistration /// internal static string EnsureCentralSharedStoreConnectionString(IConfiguration config) { + ArgumentNullException.ThrowIfNull(config); + var hubConnectionString = config[HubConnectionStringKey]; if (string.IsNullOrWhiteSpace(hubConnectionString)) diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SecretsReplicationWiringTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SecretsReplicationWiringTests.cs index 715e5f22..13592f99 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SecretsReplicationWiringTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SecretsReplicationWiringTests.cs @@ -79,9 +79,14 @@ public class SecretsReplicationWiringTests == "ZB.MOM.WW.Secrets.Replicator.SqlServer"; /// - /// Any descriptor contributed by the SQL-Server replicator package, whatever its shape. The - /// package's factory-lambda registrations (its store, migrator, connection factory) carry the - /// concrete type as the SERVICE type, so checking both sides catches every registration form. + /// Any descriptor whose service OR implementation type lives in the SQL-Server replicator + /// assembly. Honest bound: the package also registers ISecretStore/ISecretsStoreMigrator + /// through factory lambdas whose service type is the Abstractions interface and whose + /// ImplementationType is null — those two descriptors would evade this scan in isolation. The + /// site-purity pin still holds because the same extension unconditionally registers three + /// concrete types from the target assembly first, which this scan does catch — so the package + /// cannot enter the container without tripping it, even though not every individual descriptor + /// it adds is individually detectable. /// private static bool IsFromSqlServerReplicatorPackage(ServiceDescriptor descriptor) => descriptor.ServiceType.Assembly.GetName().Name == "ZB.MOM.WW.Secrets.Replicator.SqlServer"