docs(secrets): review follow-ups — pre-Serilog window note, ThrowIfNull hygiene, honest site-purity scan bound

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
This commit is contained in:
Joseph Doherty
2026-08-07 10:47:10 -04:00
parent a358244d9e
commit aebdd56b52
3 changed files with 19 additions and 3 deletions
@@ -64,6 +64,15 @@ var configuration = new ConfigurationBuilder()
// EnsureCentralSharedStoreConnectionString check AddScadaBridgeSecrets runs — this is the // EnsureCentralSharedStoreConnectionString check AddScadaBridgeSecrets runs — this is the
// earliest code that would otherwise hand the bad value to SqlConnection, whose generic // earliest code that would otherwise hand the bad value to SqlConnection, whose generic
// "initialization string" format error would bury the designed message. // "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 = var expanderUsesSharedSqlStore =
string.Equals( string.Equals(
configuration["ScadaBridge:Node:Role"], "Central", StringComparison.OrdinalIgnoreCase) configuration["ScadaBridge:Node:Role"], "Central", StringComparison.OrdinalIgnoreCase)
@@ -199,6 +199,8 @@ public static class SecretsRegistration
/// </exception> /// </exception>
internal static string EnsureCentralSharedStoreConnectionString(IConfiguration config) internal static string EnsureCentralSharedStoreConnectionString(IConfiguration config)
{ {
ArgumentNullException.ThrowIfNull(config);
var hubConnectionString = config[HubConnectionStringKey]; var hubConnectionString = config[HubConnectionStringKey];
if (string.IsNullOrWhiteSpace(hubConnectionString)) if (string.IsNullOrWhiteSpace(hubConnectionString))
@@ -79,9 +79,14 @@ public class SecretsReplicationWiringTests
== "ZB.MOM.WW.Secrets.Replicator.SqlServer"; == "ZB.MOM.WW.Secrets.Replicator.SqlServer";
/// <summary> /// <summary>
/// Any descriptor contributed by the SQL-Server replicator package, whatever its shape. The /// Any descriptor whose service OR implementation type lives in the SQL-Server replicator
/// package's factory-lambda registrations (its store, migrator, connection factory) carry the /// assembly. Honest bound: the package also registers <c>ISecretStore</c>/<c>ISecretsStoreMigrator</c>
/// concrete type as the SERVICE type, so checking both sides catches every registration form. /// 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.
/// </summary> /// </summary>
private static bool IsFromSqlServerReplicatorPackage(ServiceDescriptor descriptor) => private static bool IsFromSqlServerReplicatorPackage(ServiceDescriptor descriptor) =>
descriptor.ServiceType.Assembly.GetName().Name == "ZB.MOM.WW.Secrets.Replicator.SqlServer" descriptor.ServiceType.Assembly.GetName().Name == "ZB.MOM.WW.Secrets.Replicator.SqlServer"