diff --git a/Directory.Packages.props b/Directory.Packages.props index 85d2eb74..5d2209d6 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -109,7 +109,6 @@ - diff --git a/docker/docker-compose.yml b/docker/docker-compose.yml index 270b947e..b80614dc 100644 --- a/docker/docker-compose.yml +++ b/docker/docker-compose.yml @@ -17,59 +17,53 @@ x-secrets-store-env: &secrets-store-env Secrets__SqlitePath: "/data/scadabridge-secrets.db" -# ── Clustered secret replication: the pull-only gRPC hub (scadaproj#3) ───────── +# ── Secrets:Store:Provider=SqlServer: central shares one secret store directly (Task 5) ── # -# Central hosts the hub on its EXISTING h2c control-plane listener (CentralGrpcPort -# 8083, alongside CentralControlService); each site node sweeps it on an interval and -# writes what it pulls into its OWN local SQLite store. Nothing but ciphertext crosses -# the wire, so every participating node must resolve the SAME KEK. +# Task 11 retired secrets replication entirely — both the pull-only gRPC hub (central +# hosted, sites swept it) and the SQL-Server ReplicatingSecretStore mode that preceded it. +# Sites now receive secrets exclusively via Deploy Artifacts; there is no hub and nothing +# for a site to sweep. What remains, and is unrelated to replication, is Secrets:Store: +# Provider=SqlServer: it points BOTH central nodes at ONE shared SQL-Server database +# directly — no hub, no follower sweep, just two nodes reading and writing the same rows — +# so a clustered central pair can never serve sites a drifted secret set. central-a/b set +# it explicitly below; site nodes always ignore the key and keep their local SQLite store. # -# ENABLED ON FOUR NODES ONLY: the central pair (hub) and the site-a pair (followers). -# site-b and site-c are deliberately left without it, so the default-OFF posture is -# proven side by side on one rig — exactly as site-a is the rig's only LocalDb-replicated -# pair. A node with no Secrets__* override keeps the shipped appsettings default -# (Replication:Enabled=false, Mode=SqlServer) and composes a plain local store. -# -# ALL VALUES HERE ARE DEV-ONLY and committed under the same exception as the mesh PSKs -# and the ApiKeyPepper above: a local docker rig needs a working credential in source -# control to boot. Production supplies the KEK out of band (ZB_SECRETS_MASTER_KEY, never -# committed) and the hub token from appsettings/env — NEVER as a ${secret:} reference, -# since resolving one is what the hub exists to make possible. -x-secrets-hub-env: &secrets-hub-env - # DEV-ONLY KEK — NOT a real key. Identical on all four participating nodes: only - # ciphertext replicates, so a node with a different KEK fails closed on resolve with a - # kek_id mismatch that reads like corruption but is a deployment error. - ZB_SECRETS_MASTER_KEY: "zZiBWuoaVMbJmGXToLk9Lakw0iJozXoL/7Gxac3GwJ4=" - Secrets__Replication__Enabled: "true" - Secrets__Replication__Mode: "Grpc" - # DEV-ONLY shared bearer token — NOT a real secret. Presented by every follower and - # verified by the hub's fail-closed SecretsHubAuthInterceptor. Must be IDENTICAL on the - # hub and every follower; an unset token is a startup failure on both halves. - Secrets__GrpcHub__BearerToken: "secrets-hub-docker-dev-token" - -# Central-only half: the SHARED SQL-Server secret store (scadaproj#4, Secrets 0.5.0). -# In Grpc mode BOTH central nodes read and write ONE copy of every row in this database, -# so the two hub instances serve identical manifests by construction — that is what makes -# the site-side FallbackEndpoints below safe. ZbSecretsHub is a dedicated database on the -# rig's existing scadabridge-mssql container (default zbsecrets schema, created by the -# boot-time SqlServerSecretsStoreMigrator; the database itself + the scadabridge_app grant -# were provisioned once via sqlcmd — see docs/plans/2026-08-07-secrets-central-shared-store-live-gate.md). -# Same DEV-ONLY credentials as the ScadaBridge__Database__* strings above. Must be a -# LITERAL value, never a ${secret:} reference — the pre-host expander needs this string to -# reach the store that would resolve it (bootstrap circularity; registration rejects it). -# Site nodes must NEVER carry this key: sites talk to central, not to central's database. -x-secrets-hub-central-env: &secrets-hub-central-env +# ZbSecretsHub is a dedicated database on the rig's existing scadabridge-mssql container +# (default zbsecrets schema, created by the boot-time SqlServerSecretsStoreMigrator; the +# database itself + the scadabridge_app grant were provisioned once via sqlcmd — see +# docs/plans/2026-08-07-secrets-central-shared-store-live-gate.md). Same DEV-ONLY +# credentials as the ScadaBridge__Database__* strings below. Must be a LITERAL value, +# never a ${secret:} reference — the pre-host expander needs this string to reach the +# store that would resolve it (bootstrap circularity; registration rejects it). Site nodes +# must NEVER carry this key: sites talk to central, not to central's database. +x-secrets-central-store-env: &secrets-central-store-env + Secrets__Store__Provider: "SqlServer" Secrets__SqlServer__ConnectionString: "Server=scadabridge-mssql,1433;Database=ZbSecretsHub;User Id=scadabridge_app;Password=ScadaBridge_Dev1#;TrustServerCertificate=true" -# Site half of the same section. The hub client dials Endpoint first and fails over, per -# call, to FallbackEndpoints in order (sticky on whichever answered last, Secrets 0.5.0). -# Listing central-b is safe ONLY because both centrals serve the one shared SQL store -# above — never list endpoints backed by independent stores: failing over to an emptier -# hub is a silent convergence stop, the exact defect (scadaproj#4) the shared store -# exists to prevent. -x-secrets-hub-site-env: &secrets-hub-site-env - Secrets__GrpcHub__Endpoint: "http://scadabridge-central-a:8083" - Secrets__GrpcHub__FallbackEndpoints__0: "http://scadabridge-central-b:8083" +# ── Deploy-artifact secret KEKs ────────────────────────────────────────────────────── +# +# Every node that HANDLES a deployed secret — central (it encrypts what ships in the +# artifact) and site-a (it decrypts what it receives) — must resolve the SAME +# ZB_SECRETS_MASTER_KEY: only ciphertext ever crosses the wire, so a node with a different +# KEK fails closed on resolve with a kek_id mismatch that reads like corruption but is a +# deployment error. site-b/site-c deliberately carry a DIFFERENT dev KEK (below) so that +# mismatch is a real, provable negative on this rig rather than an assumption — see +# Task 13. +# +# ALL VALUES HERE ARE DEV-ONLY and committed under the same exception as the mesh PSKs +# and the ApiKeyPepper below: a local docker rig needs a working credential in source +# control to boot. Production supplies the KEK out of band (ZB_SECRETS_MASTER_KEY, never +# committed). +x-secrets-shared-kek-env: &secrets-shared-kek-env + # DEV-ONLY KEK — NOT a real key. Identical on central-a/b and site-a-a/b. + ZB_SECRETS_MASTER_KEY: "zZiBWuoaVMbJmGXToLk9Lakw0iJozXoL/7Gxac3GwJ4=" + +# A DIFFERENT DEV-ONLY KEK — NOT a real key — deliberately mismatched from the one above. +# site-b and site-c never receive a deployed secret from this rig's exercised paths today, +# but carrying a mismatched key here (rather than none) means a future deploy targeting +# them fails closed with a named kek_id mismatch instead of an untested code path. +x-secrets-mismatched-kek-env: &secrets-mismatched-kek-env + ZB_SECRETS_MASTER_KEY: "esKrmqV2RBmMq/+6T/5zK2BcalRy5gzDXhoG9Qydg6Q=" services: central-a: @@ -87,9 +81,9 @@ services: stop_grace_period: 30s container_name: scadabridge-central-a environment: - # Hub half of the pull-only gRPC secrets hub + the central-only shared SQL store - # (anchors at the top of this file). - <<: [*secrets-store-env, *secrets-hub-env, *secrets-hub-central-env] + # Central-only shared SQL secret store (Secrets:Store:Provider=SqlServer, Task 5) + the + # deploy-artifact secret KEK (anchors at the top of this file). + <<: [*secrets-store-env, *secrets-shared-kek-env, *secrets-central-store-env] SCADABRIDGE_CONFIG: Central ASPNETCORE_ENVIRONMENT: Development ASPNETCORE_URLS: "http://+:5000" @@ -154,9 +148,9 @@ services: stop_grace_period: 30s container_name: scadabridge-central-b environment: - # Hub half of the pull-only gRPC secrets hub + the central-only shared SQL store - # (anchors at the top of this file). - <<: [*secrets-store-env, *secrets-hub-env, *secrets-hub-central-env] + # Central-only shared SQL secret store (Secrets:Store:Provider=SqlServer, Task 5) + the + # deploy-artifact secret KEK (anchors at the top of this file). + <<: [*secrets-store-env, *secrets-shared-kek-env, *secrets-central-store-env] SCADABRIDGE_CONFIG: Central ASPNETCORE_ENVIRONMENT: Development ASPNETCORE_URLS: "http://+:5000" @@ -221,9 +215,11 @@ services: stop_grace_period: 30s container_name: scadabridge-site-a-a environment: - # Follower half of the pull-only gRPC secrets hub (anchors at the top of this - # file). site-b and site-c deliberately carry neither. - <<: [*secrets-store-env, *secrets-hub-env, *secrets-hub-site-env] + # The shared deploy-artifact secret KEK (anchors at the top of this file) — this node + # must decrypt what central ships in a deploy artifact. site-b and site-c deliberately + # carry a DIFFERENT (mismatched) KEK instead, so that failure mode is provable on this + # rig rather than assumed (Task 13). + <<: [*secrets-store-env, *secrets-shared-kek-env] SCADABRIDGE_CONFIG: Site # Register row 63. The central containers have always set this; the site containers # never did, so the Development-only DI scope validation (register row 50, @@ -258,9 +254,11 @@ services: stop_grace_period: 30s container_name: scadabridge-site-a-b environment: - # Follower half of the pull-only gRPC secrets hub (anchors at the top of this - # file). site-b and site-c deliberately carry neither. - <<: [*secrets-store-env, *secrets-hub-env, *secrets-hub-site-env] + # The shared deploy-artifact secret KEK (anchors at the top of this file) — this node + # must decrypt what central ships in a deploy artifact. site-b and site-c deliberately + # carry a DIFFERENT (mismatched) KEK instead, so that failure mode is provable on this + # rig rather than assumed (Task 13). + <<: [*secrets-store-env, *secrets-shared-kek-env] SCADABRIDGE_CONFIG: Site # Register row 63. The central containers have always set this; the site containers # never did, so the Development-only DI scope validation (register row 50, @@ -295,7 +293,9 @@ services: stop_grace_period: 30s container_name: scadabridge-site-b-a environment: - <<: *secrets-store-env + # A DIFFERENT (mismatched) deploy-artifact secret KEK from central/site-a's — see the + # anchor comment at the top of this file (Task 13's mismatch negative). + <<: [*secrets-store-env, *secrets-mismatched-kek-env] SCADABRIDGE_CONFIG: Site # Register row 63. The central containers have always set this; the site containers # never did, so the Development-only DI scope validation (register row 50, @@ -329,7 +329,9 @@ services: stop_grace_period: 30s container_name: scadabridge-site-b-b environment: - <<: *secrets-store-env + # A DIFFERENT (mismatched) deploy-artifact secret KEK from central/site-a's — see the + # anchor comment at the top of this file (Task 13's mismatch negative). + <<: [*secrets-store-env, *secrets-mismatched-kek-env] SCADABRIDGE_CONFIG: Site # Register row 63. The central containers have always set this; the site containers # never did, so the Development-only DI scope validation (register row 50, @@ -363,7 +365,9 @@ services: stop_grace_period: 30s container_name: scadabridge-site-c-a environment: - <<: *secrets-store-env + # A DIFFERENT (mismatched) deploy-artifact secret KEK from central/site-a's — see the + # anchor comment at the top of this file (Task 13's mismatch negative). + <<: [*secrets-store-env, *secrets-mismatched-kek-env] SCADABRIDGE_CONFIG: Site # Register row 63. The central containers have always set this; the site containers # never did, so the Development-only DI scope validation (register row 50, @@ -397,7 +401,9 @@ services: stop_grace_period: 30s container_name: scadabridge-site-c-b environment: - <<: *secrets-store-env + # A DIFFERENT (mismatched) deploy-artifact secret KEK from central/site-a's — see the + # anchor comment at the top of this file (Task 13's mismatch negative). + <<: [*secrets-store-env, *secrets-mismatched-kek-env] SCADABRIDGE_CONFIG: Site # Register row 63. The central containers have always set this; the site containers # never did, so the Development-only DI scope validation (register row 50, diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs b/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs index 921b5069..2a415cb9 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs @@ -33,8 +33,6 @@ using ZB.MOM.WW.ScadaBridge.Transport; using ZB.MOM.WW.Secrets.Abstractions; using ZB.MOM.WW.Secrets.Configuration; using ZB.MOM.WW.Secrets.DependencyInjection; -using ZB.MOM.WW.Secrets.Replicator.SqlServer; -using ZB.MOM.WW.Secrets.Replicator.SqlServer.DependencyInjection; using ZB.MOM.WW.Secrets.Sqlite; using ZB.MOM.WW.Secrets.Ui; using ZB.MOM.WW.Telemetry; @@ -77,31 +75,11 @@ var configuration = new ConfigurationBuilder() // Expand ${secret:...} config references before any validator/binder sees them (Layer A). // Throwaway provider — disposed here, shares no singletons with the host container. // -// The expander must read the SAME store the running node will serve. On a central node in Grpc -// replication mode that store is the SHARED SQL-Server store (see SecretsRegistration, -// scadaproj#4) — an expander left on SQLite there would resolve pre-host ${secret:} references -// from a stale/empty local store, silently diverging from what the node's own hub serves. Every -// other case (sites, SqlServer mode, replication off) keeps the local SQLite path exactly as it -// always was. A blank or ${secret:}-valued connection string fails HERE, through the same -// 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. +// This always reads the node's LOCAL SQLite store: Task 11 retired the gRPC secrets hub, the only +// topology that ever put a central node's store somewhere else pre-host. (Secrets:Store:Provider +// =SqlServer, the still-supported shared-central-store feature, is a post-host DI registration — +// see SecretsRegistration — and does not change what the Layer-A expander itself reads.) // -// 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. (That exit -// is real only because of the UnhandledException handler at the top of this file — the -// runtime's own crash path wedges as container PID 1, ScadaBridge#34.) 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) - && SecretsRegistration.UsesGrpcHub(configuration); - // Secrets 0.6.2 validates Secrets:SqlitePath as absolute AND outside the content root, finding the // content root from IHostEnvironment — which this throwaway ServiceCollection does not have, and // which does not exist yet anyway (the host is not built until further down). Both the resolution @@ -109,32 +87,15 @@ var expanderUsesSharedSqlStore = // AddPreHostSqliteExpander for why the content-root argument is load-bearing and silent when // dropped, and PreHostSecretsContentRootTests for the regression guard. var expanderServices = new ServiceCollection(); -if (expanderUsesSharedSqlStore) -{ - SecretsRegistration.EnsureCentralSharedStoreConnectionString( - configuration, $"{SecretsRegistration.ReplicationModeKey}=Grpc"); - expanderServices.AddZbSecretsSqlServerStore(configuration, "Secrets"); -} -else -{ - expanderServices.AddPreHostSqliteExpander( - configuration, SecretsRegistration.ResolveHostContentRoot()); -} +expanderServices.AddPreHostSqliteExpander( + configuration, SecretsRegistration.ResolveHostContentRoot()); #pragma warning disable ASP0000 // deliberate throwaway container await using (var secretsProvider = expanderServices.BuildServiceProvider()) #pragma warning restore ASP0000 { - if (expanderUsesSharedSqlStore) - { - await secretsProvider.GetRequiredService() - .MigrateAsync(default); - } - else - { - await secretsProvider.GetRequiredService() - .MigrateAsync(default); - } + await secretsProvider.GetRequiredService() + .MigrateAsync(default); var resolver = secretsProvider.GetRequiredService(); await new SecretReferenceExpander(resolver) @@ -740,21 +701,9 @@ try // connection to the right pipeline by listener/protocol. app.MapGrpcService(); - // The pull-only secrets hub (scadaproj#3), on the SAME h2c listener and the same addressing - // convention — a site dials central's CentralGrpcPort, never central's database. It shares - // the listener with CentralControlService the way the site's LocalDb sync endpoint shares - // its one: two disjoint service prefixes, two independent fail-closed gates. The - // CentralControlAuthInterceptor registered on AddGrpc is prefix-scoped to - // CentralControlService and passes hub calls straight through; the hub's own - // SecretsHubAuthInterceptor — attached per-service by AddZbSecretsGrpcHub — is what gates - // it, on the shared Secrets:GrpcHub:BearerToken. - // - // A NO-OP unless this node registered the hub. That is not an optimisation: mapping the - // service without registering it would map a hub whose interceptor was never attached, - // i.e. an unauthenticated endpoint serving every secret central holds. Both this call and - // the registration above route through SecretsRegistration.UsesGrpcHub so they cannot - // drift apart. - app.MapScadaBridgeSecretsHub(app.Configuration); + // The pull-only secrets hub that used to be mapped here (scadaproj#3) is retired — Task 11. + // Sites receive secrets exclusively via Deploy Artifacts; there is no hub endpoint to map + // on central and nothing left for a site to sweep. See SecretsRegistration. app.MapStaticAssets(); app.MapCentralUI(); diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/SecretsRegistration.cs b/src/ZB.MOM.WW.ScadaBridge.Host/SecretsRegistration.cs index 5a144b39..4d23057a 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/SecretsRegistration.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/SecretsRegistration.cs @@ -1,45 +1,26 @@ using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.Hosting.WindowsServices; using ZB.MOM.WW.Secrets.DependencyInjection; -using ZB.MOM.WW.Secrets.Replicator.Grpc.DependencyInjection; using ZB.MOM.WW.Secrets.Replicator.SqlServer.DependencyInjection; namespace ZB.MOM.WW.ScadaBridge.Host; /// -/// Which half of a clustered secrets topology this node composes. Passed in by the caller rather -/// than read from configuration, because both composition roots already know statically which one -/// they are — Program.cs for central and for a site — -/// and a role read from a key could be got wrong in a way that silently turns a site into a hub. +/// Which half of a composition this node is. Passed in by the caller rather than read from +/// configuration, because both composition roots already know statically which one they are — +/// Program.cs for central and for a site — and a role +/// read from a key could be got wrong in a way that silently turns a site's secret handling into +/// central's. /// public enum SecretsNodeRole { - /// The central cluster. Hosts the gRPC secrets hub; never sweeps from one. + /// The central cluster. Central, - /// A site cluster. Sweeps from central's hub; never hosts one. + /// A site cluster. Site, } -/// -/// The clustered-replication transport, selected by Secrets:Replication:Mode. -/// -public enum SecretsReplicationMode -{ - /// - /// Local store bidirectionally synced with a shared SQL-Server hub. The historical mode and - /// therefore the default, so an existing configuration that sets only - /// Secrets:Replication:Enabled keeps behaving exactly as it did. - /// - SqlServer, - - /// - /// Local store on every node, converged by a pull-only gRPC hub that central hosts. The - /// production topology (scadaproj#3). - /// - Grpc, -} - /// /// Single composition-root entry point for the host container's secret store, shared by the /// central-role registrations in Program.cs and the site-role registrations in @@ -47,61 +28,43 @@ public enum SecretsReplicationMode /// /// /// -/// Clustered replication is opt-in and off by default. ScadaBridge is hub-and-spoke — one -/// central cluster plus N separate site clusters — and every supported topology gives each SITE -/// node a LOCAL store, so a site keeps resolving secrets straight through a WAN outage. What -/// differs between the modes is how convergence works, and — in gRPC mode — where central's own -/// store lives. +/// Task 11 retired ALL secrets replication in ScadaBridge — both the pull-only gRPC hub +/// (central hosts, sites sweep) and the shared-SQL-Server ReplicatingSecretStore path that +/// preceded it. ScadaBridge is hub-and-spoke — one central cluster plus N separate site clusters — +/// and every topology now gives each SITE node a permanently LOCAL, undecorated SQLite store, so a +/// site keeps resolving secrets straight through a WAN outage AND never republishes anything it +/// deletes. That second property is load-bearing, not incidental: +/// ZB.MOM.WW.ScadaBridge.SiteRuntime.Secrets.DeployedSecretsMirror deletes a name from the +/// local store the moment a deploy artifact stops referencing it, and a replication-decorated +/// store (the retired SQL-Server mode) would have published that delete to every other node +/// sharing the hub — turning a single site's unreferencing of a secret into a fleet-wide deletion. +/// Sites now receive secrets exclusively via Deploy Artifacts (Tasks 1–10); there is no sweep, no +/// hub, and nothing for a site to converge against but its own local file. /// /// -/// is the production topology (scadaproj#3): -/// central hosts a pull-only gRPC hub and each site node sweeps it. It exists because -/// requires every site node to hold a connection -/// string to central's database, which violates ScadaBridge's standing rule that sites talk to -/// central, not to central's DB. Pull-only is a property of the wire contract rather than a -/// setting: the proto carries no write RPC, so a site cannot push even if misconfigured, and -/// secrets therefore originate at central only. Central's own store in this mode is the SHARED -/// SQL-Server store (AddZbSecretsSqlServerStore), not local SQLite: both central nodes -/// read and write ONE copy of every row, so the two hub instances serve identical manifests by -/// construction. The alternative — an independent local store per central node — is the recorded -/// defect (scadaproj#4): one central answering authenticated followers with an empty manifest, -/// turning site-side hub failover into a silent convergence stop. The accepted trade is that -/// central secret resolution now depends on SQL Server availability — acceptable because central -/// is already SQL-coupled to its core, and a SQL outage makes the hub throw (sites keep -/// last-known-good), never serve empty. The pre-host ${secret:} expander in -/// Program.cs follows this store swap, so pre-host references resolve from the same store -/// the running node serves. +/// is a SEPARATE, still-supported feature and is unaffected by +/// the retirement above. A fleet that ships secrets to sites on every artifact deploy still +/// needs a clustered CENTRAL pair to see the same secret set, so Secrets:Store:Provider=SqlServer +/// points both central nodes at ONE shared SQL-Server database directly — no hub, no follower +/// sweep, just two nodes reading and writing the same rows. This is CENTRAL-only; a Site node +/// always ignores the key and keeps its local SQLite store. /// /// -/// The SQL-Server gate is not a style preference. AddZbSecretsSqlServerReplication +/// The SQL-Server gate is not a style preference. AddZbSecretsSqlServerStore /// validates its options EAGERLY at registration time, so calling it unconditionally would throw at /// startup on every node where Secrets:SqlServer:ConnectionString is unset — which is every -/// dev box, every docker node and every existing deployment. Both the explicit -/// Secrets:Replication:Enabled flag and a non-blank connection string are therefore required +/// dev box, every docker node and every existing deployment. Both +/// Secrets:Store:Provider=SqlServer and a non-blank connection string are therefore required /// before the SQL-Server package is touched at all; otherwise this is byte-for-byte the plain /// local-SQLite registration it has always been. /// /// -/// The gRPC mode deliberately does NOT degrade the same way. A missing connection string in -/// SQL-Server mode falls back to a local-only store with a loud warning, and that behaviour is -/// preserved untouched. gRPC mode instead fails at startup when its endpoint or bearer token is -/// missing — or, on central, when the shared-store connection string is — because the fallback is -/// the exact outcome the hub exists to prevent — a site serving secrets that silently never -/// converge with central — and because selecting the mode takes an explicit second key, so an -/// operator who typed it meant it. -/// -/// -/// Bootstrap constraint. A replication credential can never itself come from the replicated -/// set — a node cannot read the hub to learn how to reach the hub. In SQL-Server mode the hub -/// connection string must arrive from outside it: an environment variable, or a ${secret:} -/// reference seeded in that node's own LOCAL store (the pre-host expander in Program.cs -/// runs against the local SQLite store before the host container exists, so such a reference does -/// resolve). In gRPC mode the rule is stricter on central: Secrets:SqlServer:ConnectionString -/// can never be a ${secret:} reference at all, because on central the pre-host expander -/// itself runs against the shared SQL store (see Program.cs) and would need this very -/// string to reach the store that resolves it — registration rejects such a value outright. The -/// same holds for Secrets:GrpcHub:BearerToken, which is supplied from appsettings or the -/// environment exactly as the mesh pre-shared keys are. +/// Bootstrap constraint. The shared store's own connection string can never itself come from +/// that store — a node cannot read the database to learn how to reach the database. It must arrive +/// from outside it: an environment variable, or a literal value in appsettings. It can never be a +/// ${secret:} reference, because on central the pre-host expander that would resolve such a +/// reference runs against the very store this string points at (see Program.cs) — the +/// reference could never resolve, so registration rejects it outright. /// /// public static class SecretsRegistration @@ -109,94 +72,21 @@ public static class SecretsRegistration /// Configuration section holding the core secrets options. public const string SecretsSectionPath = "Secrets"; - /// Configuration section holding the SQL-Server hub options. + /// Configuration section holding the shared SQL-Server store options. public const string SqlServerSectionPath = "Secrets:SqlServer"; - /// - /// Configuration section holding the gRPC hub options. One section serves both halves: central - /// reads BearerToken + MaxNamesPerRequest from it, a site reads Endpoint, - /// BearerToken and the sweep timings. - /// - public const string GrpcHubSectionPath = "Secrets:GrpcHub"; - - /// Configuration key gating clustered replication. Absent or false = off. - public const string ReplicationEnabledKey = "Secrets:Replication:Enabled"; - - /// - /// Configuration key selecting the replication transport. Absent or blank = - /// , the historical behaviour. - /// - public const string ReplicationModeKey = "Secrets:Replication:Mode"; - - /// Configuration key holding the shared SQL-Server hub connection string. + /// Configuration key holding the shared SQL-Server store's connection string. public const string HubConnectionStringKey = "Secrets:SqlServer:ConnectionString"; /// - /// Configuration key selecting a CENTRAL node's own local store provider — independent of - /// / and the gRPC hub. - /// Absent or blank = Sqlite (per-node, the historical default); SqlServer points - /// both central nodes at ONE shared SQL-Server database directly, with no hub in between — - /// for a fleet that ships secrets to sites on every artifact deploy, a clustered central needs - /// both nodes to see the same secret set, and this is the lighter-weight way to get that - /// without standing up the gRPC hub. A Site node always ignores this key. + /// Configuration key selecting a CENTRAL node's own local store provider. Absent or blank = + /// Sqlite (per-node, the historical default); SqlServer points both central + /// nodes at ONE shared SQL-Server database directly, with no hub in between — for a fleet + /// that ships secrets to sites on every artifact deploy, a clustered central needs both nodes + /// to see the same secret set. A Site node always ignores this key. /// public const string StoreProviderKey = "Secrets:Store:Provider"; - /// - /// Reads . An absent or blank value is - /// ; anything unrecognised throws. - /// - /// - /// The value is validated whenever it is present, even with replication switched off. A typo - /// only surfaces at the boot that introduces it that way — deferring the check to whenever - /// somebody later flips would fail a node in an unrelated - /// change, pointing at a key nobody touched. - /// - /// Application configuration. - /// The selected mode. - /// The configured value is not a known mode. - public static SecretsReplicationMode ResolveReplicationMode(IConfiguration config) - { - ArgumentNullException.ThrowIfNull(config); - - var raw = config[ReplicationModeKey]; - if (string.IsNullOrWhiteSpace(raw)) - { - return SecretsReplicationMode.SqlServer; - } - - if (!Enum.TryParse(raw.Trim(), ignoreCase: true, out SecretsReplicationMode mode) - || !Enum.IsDefined(mode)) - { - throw new InvalidOperationException( - $"{ReplicationModeKey} is '{raw}', which is not a known secrets replication mode. " - + $"Valid values are '{nameof(SecretsReplicationMode.SqlServer)}' and " - + $"'{nameof(SecretsReplicationMode.Grpc)}'."); - } - - return mode; - } - - /// - /// Whether this node participates in the pull-only gRPC secrets hub. - /// - /// - /// The single predicate behind BOTH the service registration and - /// , so the two can never disagree. That matters more - /// than tidiness: mapping the hub endpoint without registering it would map a service whose - /// fail-closed bearer interceptor was never attached — an unauthenticated hub serving every - /// secret central holds. - /// - /// Application configuration. - /// when replication is enabled in gRPC mode. - public static bool UsesGrpcHub(IConfiguration config) - { - ArgumentNullException.ThrowIfNull(config); - - return config.GetValue(ReplicationEnabledKey) - && ResolveReplicationMode(config) == SecretsReplicationMode.Grpc; - } - /// /// Resolves the content root the HOST will use, from outside the host — the Layer-A /// ${secret:} expander in Program.cs runs before any builder exists, so @@ -258,20 +148,15 @@ public static class SecretsRegistration /// /// The one definition of "a Central node has a usable shared-store connection string", shared - /// by every caller that puts a Central node on the shared SQL-Server store — the Grpc-hub - /// branch of , the =SqlServer - /// branch, and the Layer-A expander in Program.cs — so none of them can diverge on what - /// "usable" means. The expander runs FIRST on a real boot — without this sharing, a bad value - /// would die inside SqlConnection with a generic "initialization string" format error - /// before the designed message ever fired. + /// by every caller that puts a Central node on the shared SQL-Server store — the + /// =SqlServer branch of — so + /// nothing can diverge on what "usable" means. /// /// Application configuration. /// /// Names the setting that put this Central node on the shared store, e.g. - /// "Secrets:Replication:Mode=Grpc" or "Secrets:Store:Provider=SqlServer" — folded - /// into the blank-connection-string message so a boot failure on either path names its own - /// real trigger instead of misdescribing the other one (a Store:Provider=SqlServer failure - /// must not claim Grpc mode is in effect, and vice versa). + /// "Secrets:Store:Provider=SqlServer" — folded into the blank-connection-string + /// message so a boot failure names its own real trigger. /// /// The validated connection string. /// @@ -287,35 +172,31 @@ public static class SecretsRegistration if (string.IsNullOrWhiteSpace(hubConnectionString)) { - // Fail closed, matching the mode's philosophy: this configuration REQUIRES the - // shared SQL store. An independent local store per central node is exactly the - // divergence scadaproj#4 recorded — one central hub answered authenticated followers - // with an EMPTY manifest, so a hub failover would "succeed" against nothing and stop - // convergence silently. + // Fail closed: this configuration REQUIRES the shared SQL store. An independent + // local store per central node would silently drift the moment anyone edits a + // secret on only one node, then ship an inconsistent secret set to sites on + // artifact deploy. throw new InvalidOperationException( $"{HubConnectionStringKey} is empty, but this node is Central with " + $"{triggerContext}. That configuration requires the SHARED " + "SQL-Server secret store — an independent local store per " - + "central node is the divergence scadaproj#4 recorded (a hub " - + "failover would 'succeed' against an empty manifest). Supply the " - + "connection string via appsettings or the environment " - + "(Secrets__SqlServer__ConnectionString)."); + + "central node would silently drift between the two nodes. " + + "Supply the connection string via appsettings or the " + + "environment (Secrets__SqlServer__ConnectionString)."); } if (hubConnectionString.Contains("${secret:", StringComparison.Ordinal)) { // The secrets store's own connection string can never be a secret reference: the // pre-host expander needs the store this string points at in order to resolve it — - // bootstrap circularity, the same rule the class remarks document for the hub bearer - // token. The message deliberately does not echo the configured value: a mistyped - // value here could be a pasted real credential. + // bootstrap circularity. The message deliberately does not echo the configured + // value: a mistyped value here could be a pasted real credential. throw new InvalidOperationException( $"{HubConnectionStringKey} contains a ${{secret:}} reference. The " + "secret store's own connection string can never be a secret " + "reference — the pre-host expander needs the store this string " - + "points at in order to resolve it (bootstrap circularity, the " - + "same rule as Secrets:GrpcHub:BearerToken). Supply a literal " - + "value via appsettings or the environment " + + "points at in order to resolve it (bootstrap circularity). " + + "Supply a literal value via appsettings or the environment " + "(Secrets__SqlServer__ConnectionString)."); } @@ -323,15 +204,21 @@ public static class SecretsRegistration } /// - /// Registers the host container's secret store: a plain local SQLite store by default, or a - /// replicating topology when is set — a shared SQL-Server - /// hub, or the pull-only gRPC hub whose half is chosen by . In gRPC - /// mode a central node's store is the SHARED SQL-Server store, and registration fails closed - /// when is blank or is a ${secret:} reference. + /// Registers the host container's secret store: a plain local SQLite store, or — on Central + /// only, with set to SqlServer — the shared SQL-Server + /// store both central nodes point at directly. Registration fails closed when + /// is blank or is a ${secret:} reference. /// + /// + /// A Site node ( = ) always resolves + /// to the plain local SQLite store — there is no replicating or decorated store left for a + /// site to be given. This is deliberate (Task 11): DeployedSecretsMirror deletes a name + /// from the local store directly, and a replication decorator would have published that + /// delete fleet-wide. + /// /// The service collection to register into. /// Application configuration for options binding. - /// Which half of a clustered topology this node composes. + /// Which half of the composition this node is. /// The same instance, for chaining. public static IServiceCollection AddScadaBridgeSecrets( this IServiceCollection services, @@ -341,135 +228,23 @@ public static class SecretsRegistration ArgumentNullException.ThrowIfNull(services); ArgumentNullException.ThrowIfNull(config); - // Secrets:Store:Provider=SqlServer on Central, BEFORE the replication branch below and - // gated OFF whenever clustered replication is enabled (that is a different, existing - // feature — the gRPC hub or SQL-Server replication already decide central's store in that - // case). A Site never reaches this: it always keeps its local SQLite store, straight - // through a WAN outage, same as every other topology. + // Secrets:Store:Provider=SqlServer on Central only. A Site never reaches this: it always + // keeps its local SQLite store, straight through a WAN outage, same as every other + // topology, and never decorated — see the class remarks. if (role == SecretsNodeRole.Central - && string.Equals(config[StoreProviderKey], "SqlServer", StringComparison.OrdinalIgnoreCase) - && !config.GetValue(ReplicationEnabledKey)) + && string.Equals(config[StoreProviderKey], "SqlServer", StringComparison.OrdinalIgnoreCase)) { - // Same fail-closed rules as the Grpc-mode central branch below: blank, or a - // ${secret:} reference (bootstrap circularity — the pre-host expander needs this - // same store to resolve it). Names ITS OWN trigger (Store:Provider=SqlServer, not - // Replication:Mode=Grpc) — this branch runs only when replication is off. + // Fail-closed rules: blank, or a ${secret:} reference (bootstrap circularity — the + // pre-host expander needs this same store to resolve it). EnsureCentralSharedStoreConnectionString(config, $"{StoreProviderKey}=SqlServer"); // Registers the shared SQL-Server store as ISecretStore directly — no hub, no // follower sweep. Both central nodes just point at the same database, so they serve - // identical manifests by construction, the same guarantee the Grpc-mode branch gets - // from the hub, without standing one up. + // identical manifests by construction. return services.AddZbSecretsSqlServerStore( config, SecretsSectionPath, SqlServerSectionPath); } - // UsesGrpcHub is the single predicate shared with MapScadaBridgeSecretsHub — the branch - // condition must never be re-derived here, or registration and endpoint mapping could - // disagree about whether the hub is on. - if (UsesGrpcHub(config)) - { - switch (role) - { - case SecretsNodeRole.Central: - // Runs BEFORE any registration so the boot failure names the real problem - // instead of surfacing later as a package validation message or a - // malformed-connection-string fault on first use. On a real boot the Layer-A - // expander in Program.cs has already run the same check even earlier. - EnsureCentralSharedStoreConnectionString(config, $"{ReplicationModeKey}=Grpc"); - - // Central's store is the SHARED SQL-Server store: registers - // SqlServerSecretStore as ISecretStore BEFORE calling AddZbSecrets internally, - // displacing SQLite — do NOT also call AddZbSecrets here. One copy of every - // row means both central hubs serve identical manifests by construction, which - // is what makes site-side hub failover between them safe. - services.AddZbSecretsSqlServerStore( - config, SecretsSectionPath, SqlServerSectionPath); - - // Throws when Secrets:GrpcHub:BearerToken is unset. Deliberately not caught: a - // hub that starts and refuses every follower looks like a network fault from - // the site end, and would be discovered as a stale secret rather than as a - // boot failure. - services.AddZbSecretsGrpcHub(config, GrpcHubSectionPath); - break; - case SecretsNodeRole.Site: - // A site keeps its LOCAL SQLite store — it resolves secrets straight through a - // WAN outage, and sites talk to central, never to central's database. Neither - // gRPC extension registers the local store — pull-only means local writes - // never leave the node, so there is nothing to decorate and the store stays - // exactly as configured. Register it first; the sweep resolves it. - services.AddZbSecrets(config, SecretsSectionPath); - - // Throws when Secrets:GrpcHub:Endpoint or :BearerToken is unset. Same reasoning. - services.AddZbSecretsGrpcHubClient(config, GrpcHubSectionPath); - break; - default: - throw new ArgumentOutOfRangeException(nameof(role), role, - "Unknown SecretsNodeRole — a new role must decide explicitly which half " + - "of the gRPC hub it composes rather than fall into either branch."); - } - - return services; - } - - // Reached with the mode already validated (UsesGrpcHub resolves it on every path), so a - // false answer here means SqlServer mode or replication off — never an unvetted mode value. - var enabled = config.GetValue(ReplicationEnabledKey); - var connectionString = config[HubConnectionStringKey]; - - if (enabled && !string.IsNullOrWhiteSpace(connectionString)) - { - // Registers the validated hub options, decorates ISecretStore with - // ReplicatingSecretStore, and adds the hub-schema migration + bidirectional sweep - // hosted services. Calls AddZbSecrets internally — do NOT also call it here. - return services.AddZbSecretsSqlServerReplication( - config, SecretsSectionPath, SqlServerSectionPath); - } - - if (enabled) - { - // Asked for replication but gave nothing to replicate against. Falling back to the - // local-only store keeps the node up, but it is silently NOT participating in the - // cluster, so say so loudly rather than letting it look healthy. - Serilog.Log.Warning( - "{Key} is true but {ConnKey} is empty — clustered secret replication is DISABLED " - + "and this node's secrets are local-only. The hub connection string cannot come " - + "from the hub itself; supply it via environment variable or seed it in this " - + "node's local store.", - ReplicationEnabledKey, - HubConnectionStringKey); - } - return services.AddZbSecrets(config, SecretsSectionPath); } - - /// - /// Maps the central-hosted secrets hub endpoint, but only on a node whose configuration - /// actually registered it. A no-op otherwise. - /// - /// - /// Central-only by construction: registers the hub service - /// and its bearer interceptor solely for , and a site node - /// never reaches this call. The endpoint lands on the shared endpoint routing, so it is served - /// on the dedicated h2c control-plane listener (ScadaBridge:Node:CentralGrpcPort, - /// default 8083) — the same listener and the same addressing convention sites already use to - /// reach CentralControlService. - /// - /// The endpoint route builder. - /// Application configuration. - /// The same instance, for chaining. - public static IEndpointRouteBuilder MapScadaBridgeSecretsHub( - this IEndpointRouteBuilder endpoints, - IConfiguration config) - { - ArgumentNullException.ThrowIfNull(endpoints); - ArgumentNullException.ThrowIfNull(config); - - if (!UsesGrpcHub(config)) - { - return endpoints; - } - - return endpoints.MapZbSecretsHub(); - } } diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/SiteServiceRegistration.cs b/src/ZB.MOM.WW.ScadaBridge.Host/SiteServiceRegistration.cs index a3b5e54f..267397c0 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/SiteServiceRegistration.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/SiteServiceRegistration.cs @@ -115,15 +115,16 @@ public static class SiteServiceRegistration services.AddZbLocalDbReplication(config); services.AddDataConnectionLayer(); - // Local SQLite store by default. With Secrets:Replication:Enabled true it becomes either a - // local store synced against a shared SQL-Server hub (Mode=SqlServer) or — the production - // topology, scadaproj#3 — the FOLLOWER half of the pull-only gRPC secrets hub (Mode=Grpc), - // sweeping central over the same gRPC path the control plane already uses instead of - // holding a connection string to central's database. A site never hosts the hub; passing - // the role explicitly is what makes that a property of this composition root. - // - // Either way the node keeps a FULL local store, so it resolves secrets straight through a - // WAN outage to central and simply converges late. See SecretsRegistration. + // A Site node's secret store is ALWAYS the local SQLite store — never a replication + // decorator. Task 11 retired both the pull-only gRPC secrets hub (central hosts, sites + // sweep) and the shared-SQL-Server replication mode that preceded it. This is deliberate, + // not merely simplification: ZB.MOM.WW.ScadaBridge.SiteRuntime.Secrets.DeployedSecretsMirror + // deletes a name from the local store directly the moment a deploy artifact stops + // referencing it, and a replication-decorated store would have published that delete to + // every other node sharing the hub — turning one site's unreferencing of a secret into a + // fleet-wide deletion. Sites now receive secrets exclusively via Deploy Artifacts, so the + // node keeps a FULL local store and simply picks up whatever the next deploy ships. See + // SecretsRegistration. services.AddScadaBridgeSecrets(config, SecretsNodeRole.Site); // Adapter that surfaces the site id to // StoreAndForwardService through DI WITHOUT introducing a diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/StartupValidator.cs b/src/ZB.MOM.WW.ScadaBridge.Host/StartupValidator.cs index ef65431e..9933c150 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/StartupValidator.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/StartupValidator.cs @@ -26,6 +26,13 @@ public static class StartupValidator var seedNodes = configuration.GetSection("ScadaBridge:Cluster:SeedNodes").Get>(); + // Secrets replication (the gRPC hub, and the SQL-Server ReplicatingSecretStore mode that + // preceded it) is retired in ScadaBridge (Task 11) — sites receive secrets exclusively via + // Deploy Artifacts, and a replicated site store would publish DeployedSecretsMirror's local + // deletes fleet-wide. Checked unconditionally (both roles): a Site used to be the gRPC + // hub's follower half, so the flag was never Central-only. + var secretsReplicationEnabled = configuration.GetValue("Secrets:Replication:Enabled"); + // GrpcPort: default 8083 when absent; only fails the range rule when the key is // present AND invalid. The out-param assignment mirrors the original so the // resolved grpcPort feeds the cross-field rules even on a parse failure. @@ -49,6 +56,12 @@ public static class StartupValidator .Require("ScadaBridge:Node:RemotingPort", _ => portValid, "must be 1-65535") + // Retired for BOTH roles (Task 11) — a Site could set this too, as the gRPC hub's + // follower half. Deliberately NOT folded into a role-scoped block below. + .Require("Secrets:Replication:Enabled", + _ => !secretsReplicationEnabled, + "Secrets replication (the gRPC hub) is retired in ScadaBridge — sites receive " + + "secrets with Deploy Artifacts; central uses Secrets:Store:Provider=SqlServer") // SiteId (Site only) — note: OUTSIDE the big Site block in the original, // so it must run before the unconditional SeedNodes-count rule. .When(role == "Site", p => p @@ -238,7 +251,6 @@ public static class StartupValidator var seedNodes = configuration.GetSection("ScadaBridge:Cluster:SeedNodes").Get>(); var provider = InboundApiKeyStoreOptions.ReadProvider(configuration); var secretsStoreProvider = configuration["Secrets:Store:Provider"]; - var secretsReplicationEnabled = configuration.GetValue("Secrets:Replication:Enabled"); var isClusteredCentral = role == "Central" && seedNodes is { Count: >= 2 }; var warnings = new List(); @@ -257,15 +269,13 @@ public static class StartupValidator // both central nodes to deploy the SAME secret set; a per-node store means each node has // silently drifted the moment anyone edits a secret on only one of them. // - // Gated off when Secrets:Replication:Enabled is true (review round 1, finding #1): that - // flag already puts this central node on a topology that shares state across the pair — - // the pre-existing gRPC hub (whose OWN central-side store is the shared SQL-Server store, - // not per-node SQLite — see SecretsRegistration's Grpc branch) or SQL-Server replication - // (ReplicatingSecretStore). Firing this warning there would be a false positive against - // the design's own named production topology (scadaproj#3), telling an operator to set a - // key that is already functionally moot. + // No Secrets:Replication:Enabled gate here any more (Task 11): that flag is now a hard + // Validate() failure on either role (the gRPC hub and SQL-Server secrets replication are + // both retired), so CollectWarnings never runs with it true on a config that also passed + // Validate — the old gate that suppressed this warning when replication was on is dead code + // and would otherwise mask the exact per-node-drift risk it exists to catch on any caller + // that invokes CollectWarnings directly. if (isClusteredCentral - && !secretsReplicationEnabled && !string.Equals(secretsStoreProvider, "SqlServer", StringComparison.OrdinalIgnoreCase)) { warnings.Add( diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/ZB.MOM.WW.ScadaBridge.Host.csproj b/src/ZB.MOM.WW.ScadaBridge.Host/ZB.MOM.WW.ScadaBridge.Host.csproj index 63245a35..416cccab 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/ZB.MOM.WW.ScadaBridge.Host.csproj +++ b/src/ZB.MOM.WW.ScadaBridge.Host/ZB.MOM.WW.ScadaBridge.Host.csproj @@ -41,11 +41,11 @@ + - - diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/appsettings.json b/src/ZB.MOM.WW.ScadaBridge.Host/appsettings.json index 13c3bc40..ccc14944 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/appsettings.json +++ b/src/ZB.MOM.WW.ScadaBridge.Host/appsettings.json @@ -15,33 +15,14 @@ "RunMigrationsOnStartup": true, "ResolveCacheTtl": "00:00:30", "Store": { - "_comment": "CENTRAL ONLY (a Site always ignores this and keeps its local SQLite store). Independent of Replication above and the gRPC hub - no hub is stood up either way. 'Sqlite' (default) is per-node: on a clustered central, each node's secrets can silently drift the moment anyone edits one on only a single node, which then ships an inconsistent secret set to sites on artifact deploy - StartupValidator warns on a >=2-seed central left on Sqlite. 'SqlServer' points BOTH central nodes at ONE shared SQL-Server database directly (Secrets:SqlServer:ConnectionString, same fail-closed blank/${secret:} rules as Grpc mode's shared store), so the pair can never diverge without standing up the gRPC hub.", + "_comment": "CENTRAL ONLY (a Site always ignores this and keeps its local SQLite store, permanently - Task 11 retired secrets replication, so there is no decorator left for a site to be given). 'Sqlite' (default) is per-node: on a clustered central, each node's secrets can silently drift the moment anyone edits one on only a single node, which then ships an inconsistent secret set to sites on artifact deploy - StartupValidator warns on a >=2-seed central left on Sqlite. 'SqlServer' points BOTH central nodes at ONE shared SQL-Server database directly (Secrets:SqlServer:ConnectionString, fail-closed on a blank or ${secret:} value), so the pair can never diverge.", "Provider": "Sqlite" }, - "Replication": { - "_comment": "Default-OFF pin: no deployment enables clustered secret replication by shipping this file. Mode selects the transport when Enabled is true - 'SqlServer' (the default, kept so an existing config that sets only Enabled behaves exactly as before) or 'Grpc' (the PRODUCTION topology, scadaproj#3: central hosts a pull-only hub under Secrets:GrpcHub and sites sweep it, so a site never needs a connection string to central's database). Every node in either topology must carry the SAME ZB_SECRETS_MASTER_KEY - only ciphertext crosses the wire, so a node with a different KEK fails closed on resolve with a kek_id mismatch that reads like corruption but is a deployment error.", - "Enabled": false, - "Mode": "SqlServer" - }, + "_replication": "Secrets replication (the gRPC hub, and the SQL-Server ReplicatingSecretStore mode that preceded it) is retired - Task 11. Secrets:Replication:Enabled=true is now a hard startup failure on either role; sites receive secrets exclusively via Deploy Artifacts, and Secrets:Store:Provider=SqlServer (above) is how a clustered central shares one secret set.", "SqlServer": { "ConnectionString": "", "SchemaName": "zbsecrets", - "SyncInterval": "00:00:30", - "SyncOnStartup": true, - "_comment": "Two modes read this section. Mode=SqlServer: the hub connstr must be seeded in the LOCAL store (or supplied via env) - it cannot come from the hub itself. Mode=Grpc: REQUIRED on CENTRAL, where it points BOTH central nodes at ONE shared SQL-Server secret store so the pair cannot diverge (scadaproj#4 - an independent store per central node let one hub serve an authenticated EMPTY manifest); it must be a LITERAL or environment value (Secrets__SqlServer__ConnectionString), never a ${secret:} reference - the pre-host expander needs this store to resolve references, so the reference could never resolve (bootstrap circularity; registration rejects it). SITES leave it empty in every mode - sites talk to central, never to central's database." - }, - "GrpcHub": { - "_comment": "Read ONLY when Secrets:Replication:Enabled is true AND Mode=Grpc; inert otherwise. ONE section, both halves: CENTRAL reads BearerToken + MaxNamesPerRequest and hosts the hub on its CentralGrpcPort h2c listener (default 8083, alongside CentralControlService); a SITE reads Endpoint + BearerToken + the sweep timings and pulls. Replication is pull-only by wire contract - the proto has no write RPC - so secrets originate at central and a site cannot push. FAIL-CLOSED: unlike SqlServer mode there is no local-only fallback; a missing BearerToken (either role) or Endpoint (site) is a startup failure naming the key.", - "_bearerToken": "Shared credential every follower presents. Supply it from appsettings or the environment (Secrets__GrpcHub__BearerToken), NOT as a ${secret:...} reference - resolving that reference is what the hub exists to make possible, so it cannot come from the hub. Same rationale and same handling as the mesh pre-shared keys. Never commit a real value here; the empty default below is fail-closed, not open.", - "BearerToken": "", - "_endpoint": "SITE ONLY. Absolute http/https URI of a CENTRAL node's gRPC (h2c) port - the same address family as ScadaBridge:Communication:CentralGrpcEndpoints, e.g. 'http://central-a-host:8083'. NOT via Traefik (HTTP/1 only). With FallbackEndpoints left empty the hub client dials this SINGLE endpoint only: a sweep against a downed central-a then stalls rather than failing over - survivable because the sweep is best-effort (one warning per interval, then retry) and the site keeps serving its full local last-known-good store, but secrets stop converging until that central node returns. Listing the second central under FallbackEndpoints removes that stall.", - "Endpoint": "", - "_fallbackEndpoints": "SITE ONLY. List the SECOND central node's hub endpoint here (same URI form as Endpoint, e.g. 'http://central-b-host:8083') so a sweep fails over across the central pair instead of stalling on a downed primary. Safe ONLY because both central nodes serve ONE shared SQL secret store, so either hub answers with the same manifest - NEVER list an endpoint backed by an independent store: failing over to an emptier hub is a silent convergence stop, the exact defect scadaproj#4 recorded. Empty by default; leaving it empty keeps the pre-0.5.0 single-endpoint behavior byte-identical.", - "FallbackEndpoints": [], - "SyncInterval": "00:00:30", - "SyncOnStartup": true, - "CallDeadline": "00:00:30", - "MaxNamesPerRequest": 1000 + "_comment": "Read ONLY when Secrets:Store:Provider=SqlServer: points BOTH central nodes at ONE shared SQL-Server secret store so the pair cannot diverge. Must be a LITERAL or environment value (Secrets__SqlServer__ConnectionString), never a ${secret:} reference - the pre-host expander needs this store to resolve references, so the reference could never resolve (bootstrap circularity; registration rejects it). SITES leave it empty - sites talk to central, never to central's database. Every node that handles deployed secrets must still carry the SAME ZB_SECRETS_MASTER_KEY - only ciphertext crosses the wire, so a node with a different KEK fails closed on resolve with a kek_id mismatch that reads like corruption but is a deployment error." } }, "Serilog": { diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/MappedGrpcAuthenticationCoverageTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/MappedGrpcAuthenticationCoverageTests.cs index dcb6ace1..3d3ac7bf 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/MappedGrpcAuthenticationCoverageTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/MappedGrpcAuthenticationCoverageTests.cs @@ -5,12 +5,8 @@ using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Routing; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; -using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Options; using ZB.MOM.WW.LocalDb.Replication; -using ZB.MOM.WW.Secrets.Replicator.Grpc; -using ZB.MOM.WW.Secrets.Replicator.Grpc.DependencyInjection; namespace ZB.MOM.WW.ScadaBridge.Host.Tests; @@ -21,14 +17,7 @@ public class MappedGrpcAuthenticationCoverageTests public async Task EveryMappedGrpcService_IsCoveredIncludingExternalMappings() { var builder = WebApplication.CreateBuilder(); - builder.Configuration.AddInMemoryCollection(new Dictionary - { - ["Secrets:Replication:Enabled"] = "true", - ["Secrets:Replication:Mode"] = "Grpc", - ["Secrets:GrpcHub:BearerToken"] = "coverage-test-token", - }); builder.Services.AddGrpc(); - builder.Services.AddZbSecretsGrpcHub(builder.Configuration, "Secrets:GrpcHub"); await using var app = builder.Build(); // Discover the production map calls, not a hand-maintained three-service type list. @@ -51,17 +40,19 @@ public class MappedGrpcAuthenticationCoverageTests map.MakeGenericMethod(type).Invoke(null, [app]); } - // Exercise the production extension mappings too; these packages hide their MapGrpcService calls. + // Exercise the production extension mappings too; this package hides its MapGrpcService call. Assert.Contains(hostSources.SelectMany(s => s.DescendantNodes().OfType()), n => n.Expression.ToString().EndsWith(".MapZbLocalDbSync", StringComparison.Ordinal)); app.MapZbLocalDbSync(); - app.MapScadaBridgeSecretsHub(builder.Configuration); var metadata = ((IEndpointRouteBuilder)app).DataSources.SelectMany(s => s.Endpoints) .Select(e => e.Metadata.GetMetadata()).Where(m => m is not null).ToList(); Assert.NotEmpty(metadata); var serviceTypes = metadata.Select(m => m!.ServiceType).Distinct().ToList(); - Assert.True(serviceTypes.Count >= typeNames.Count + 2, "External LocalDb and Secrets mappings must both be exercised."); + Assert.True(serviceTypes.Count >= typeNames.Count + 1, "The external LocalDb mapping must also be exercised."); + // The secrets hub — which used to install an all-method interceptor on its own service + // options and so needed a separate gate check here — is retired (Task 11): there is no + // longer a mapped gRPC service whose only gate is anything other than one of these prefixes. var prefixes = ControlPlaneAuthInterceptor.DefaultGatedPrefixes .Concat(CentralControlAuthInterceptor.DefaultGatedPrefixes) .Append((string)typeof(LocalDbSyncAuthInterceptor).GetField("ServicePrefix", BindingFlags.Static | BindingFlags.NonPublic)!.GetRawConstantValue()!) @@ -78,21 +69,14 @@ public class MappedGrpcAuthenticationCoverageTests Assert.NotEmpty(binder.Methods); Assert.Equal(metadata.Where(m => m!.ServiceType == type).Select(m => m!.Method.FullName).Order(), binder.Methods.Order()); - // Secrets installs an all-method interceptor on its own service options. Inspect actual DI options. - var optionsType = typeof(IOptions<>).MakeGenericType(typeof(GrpcServiceOptions<>).MakeGenericType(type)); - var options = app.Services.GetRequiredService(optionsType); - var serviceOptions = (GrpcServiceOptions)optionsType.GetProperty("Value")!.GetValue(options)!; - var hasSecretsGate = serviceOptions.Interceptors.Any(i => i.Type == typeof(SecretsHubAuthInterceptor)); Assert.All(binder.Methods, method => Assert.True( - hasSecretsGate || prefixes.Any(prefix => method.StartsWith(prefix, StringComparison.Ordinal)), + prefixes.Any(prefix => method.StartsWith(prefix, StringComparison.Ordinal)), $"Mapped service {type.FullName} method {method} has no authentication interceptor gate.")); - if (!hasSecretsGate) - { - // Negative control uses the real service's methods: removing its gate must be detected. - var removed = prefixes.First(p => binder.Methods.Any(m => m.StartsWith(p, StringComparison.Ordinal))); - var remaining = prefixes.Where(p => p != removed).ToArray(); - Assert.Contains(binder.Methods, method => !remaining.Any(p => method.StartsWith(p, StringComparison.Ordinal))); - } + + // Negative control uses the real service's methods: removing its gate must be detected. + var removed = prefixes.First(p => binder.Methods.Any(m => m.StartsWith(p, StringComparison.Ordinal))); + var remaining = prefixes.Where(p => p != removed).ToArray(); + Assert.Contains(binder.Methods, method => !remaining.Any(p => method.StartsWith(p, StringComparison.Ordinal))); } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SecretsReplicationWiringTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SecretsReplicationWiringTests.cs index 03d4e601..3c933d52 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SecretsReplicationWiringTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SecretsReplicationWiringTests.cs @@ -1,13 +1,6 @@ -using Grpc.Net.Client; -using Microsoft.AspNetCore.Builder; -using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Hosting; using ZB.MOM.WW.Secrets.Abstractions; -using ZB.MOM.WW.Secrets.Replication; -using ZB.MOM.WW.Secrets.Replicator.Grpc; -using ZB.MOM.WW.Secrets.Replicator.Grpc.DependencyInjection; using ZB.MOM.WW.Secrets.Replicator.SqlServer; using ZB.MOM.WW.Secrets.Sqlite; @@ -16,6 +9,14 @@ namespace ZB.MOM.WW.ScadaBridge.Host.Tests; /// /// DI-resolution tests for . /// +/// Task 11 retired every secrets REPLICATION mode in ScadaBridge — the pull-only gRPC hub (central +/// hosts, sites sweep) and the SQL-Server ReplicatingSecretStore mode that preceded it — on +/// BOTH roles. What is left: the plain local SQLite store, which is every node's default and now a +/// Site's ONLY possible shape, and the still-supported Task 5 +/// =SqlServer direct shared store (Central-only, +/// no hub, no follower sweep — just two nodes reading and writing one database). +/// +/// /// These deliberately BUILD a container and RESOLVE out of it rather than asserting over /// s. A decorator registration can look perfectly correct as a /// descriptor list and still throw on the first resolve because the undecorated concrete store it @@ -25,10 +26,8 @@ namespace ZB.MOM.WW.ScadaBridge.Host.Tests; /// /// Registration must not need a reachable SQL Server: the SQL-Server package validates its options /// eagerly, but validation only checks that the connection string is non-empty and the schema name -/// is a legal identifier. Nothing connects until a sweep or a write runs, and these tests do -/// neither — so the dummy connection string below never resolves to a real host. The same holds for -/// the gRPC hub client: GrpcChannel.ForAddress performs no I/O, so the dummy endpoint is -/// never dialled either. +/// is a legal identifier. Nothing connects until a write or read runs, and these tests do neither — +/// so the dummy connection string below never resolves to a real host. /// /// public class SecretsReplicationWiringTests @@ -37,33 +36,6 @@ public class SecretsReplicationWiringTests private const string DummyConnectionString = "Server=unused;Database=x;Integrated Security=true;"; - // Syntactically valid, deliberately unreachable. Never dialled by these tests. - private const string DummyHubEndpoint = "http://unused.invalid:8083"; - - // Syntactically valid, deliberately unreachable. Never dialled by these tests. - private const string DummyFallbackHubEndpoint = "http://unused-fallback.invalid:8083"; - - /// - /// Keyed service key of the dialing fallback endpoint 0 — the 0.5.0 - /// package contract: the primary channel keeps - /// unchanged and fallback - /// i gets :fallback:i appended. - /// - private const string FallbackChannelKey = - SecretsGrpcHubClientExtensions.ChannelServiceKey + ":fallback:0"; - - /// - /// The package-internal reader seam the follower sweep pulls through - /// (ISecretsHubReader). Internal to the package, so it is obtained by full name and - /// resolved by — visibility never gates DI resolution, only compile-time - /// references. - /// - private static readonly Type HubReaderInterface = typeof(SecretsHubAuthInterceptor).Assembly - .GetType("ZB.MOM.WW.Secrets.Replicator.Grpc.ISecretsHubReader", throwOnError: true)!; - - // Not a credential — a non-blank placeholder, which is all the fail-closed validators check. - private const string DummyBearerToken = "test-hub-token"; - private static IConfiguration BuildConfig(params (string Key, string Value)[] overrides) { var settings = new Dictionary @@ -93,116 +65,102 @@ public class SecretsReplicationWiringTests } /// - /// Any hosted service contributed by the SQL-Server replicator package. Asserted by assembly - /// rather than by type because SqlServerHubMigrationHostedService is internal to it. - /// - private static bool IsSqlServerReplicatorHostedService(ServiceDescriptor descriptor) => - descriptor.ServiceType == typeof(IHostedService) - && descriptor.ImplementationType?.Assembly.GetName().Name - == "ZB.MOM.WW.Secrets.Replicator.SqlServer"; - - /// - /// Any descriptor whose service OR implementation type lives in the SQL-Server replicator - /// assembly. Honest bound: the package also registers ISecretStore/ISecretsStoreMigrator + /// Any descriptor whose service OR implementation type lives in the SQL-Server package. + /// 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. + /// site-purity pin still holds because the same extension unconditionally registers 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) => + private static bool IsFromSqlServerPackage(ServiceDescriptor descriptor) => descriptor.ServiceType.Assembly.GetName().Name == "ZB.MOM.WW.Secrets.Replicator.SqlServer" || descriptor.ImplementationType?.Assembly.GetName().Name == "ZB.MOM.WW.Secrets.Replicator.SqlServer"; - /// - /// The gRPC hub package's one hosted service — the follower sweep. Asserted by assembly and - /// type NAME because GrpcSecretSyncService is internal to the package. The hub SERVER - /// half contributes no hosted service at all, so presence of one from this assembly is - /// specifically the follower half and nothing else. - /// - private static bool IsGrpcHubSweepService(IHostedService service) => - service.GetType().Assembly.GetName().Name == "ZB.MOM.WW.Secrets.Replicator.Grpc" - && service.GetType().Name == "GrpcSecretSyncService"; - - /// - /// The complete central shape for gRPC mode, connection string included: central's store in - /// this mode is the SHARED SQL-Server store (scadaproj#4), so a central config without a - /// connection string is not a valid variant — it is a registration failure, pinned separately. - /// - private static (string Key, string Value)[] GrpcCentralConfig() => - [ - ("Secrets:Replication:Enabled", "true"), - ("Secrets:Replication:Mode", "Grpc"), - ("Secrets:GrpcHub:BearerToken", DummyBearerToken), - ("Secrets:SqlServer:ConnectionString", DummyConnectionString), - ]; - - private static (string Key, string Value)[] GrpcSiteConfig() => - [ - ("Secrets:Replication:Enabled", "true"), - ("Secrets:Replication:Mode", "Grpc"), - ("Secrets:GrpcHub:Endpoint", DummyHubEndpoint), - ("Secrets:GrpcHub:BearerToken", DummyBearerToken), - ]; - // --------------------------------------------------------------------------------------- - // Replication off — the default, and the shape every existing deployment is in. + // Site: the ONLY shape left, on every configuration — plain local SQLite, never a + // replication decorator (Task 11, ruling #1). DeployedSecretsMirror deletes a name from + // this store directly the moment a deploy artifact stops referencing it; a decorator would + // publish that delete to every other node sharing whatever it decorates against, turning + // one site's unreferencing of a secret into a fleet-wide deletion. // --------------------------------------------------------------------------------------- [Fact] - public void ReplicationDisabledByDefault_ResolvesPlainSqliteStore() + public void Site_ResolvesPlainSqliteStore_NeverAReplicationDecorator() { - using ServiceProvider provider = BuildProvider(BuildConfig()); + using ServiceProvider provider = BuildProvider(BuildConfig(), SecretsNodeRole.Site); var store = provider.GetRequiredService(); + // Assert.IsType checks the EXACT runtime type, not merely "is a SqliteSecretStore or a + // decorator wrapping one" — nothing may sit between the site and its local file. Assert.IsType(store); - Assert.IsNotType(store); } [Fact] - public void ReplicationDisabledByDefault_RegistersNoReplicatorHostedServices() + public void Site_IgnoresStoreProvider_EvenSetToSqlServer() { - var services = new ServiceCollection(); - services.AddLogging(); - services.AddScadaBridgeSecrets(BuildConfig(), SecretsNodeRole.Central); + // A Site node ignores Secrets:Store:Provider entirely and always keeps its local SQLite + // store — even with the value set to SqlServer and no connection string configured at + // all, which would fail closed on Central. + IConfiguration config = BuildConfig(("Secrets:Store:Provider", "SqlServer")); - Assert.DoesNotContain(services, IsSqlServerReplicatorHostedService); - } - - /// - /// Flag off ⇒ NEITHER half of the gRPC hub, on EITHER role — even with the hub section fully - /// populated. The mode key selects a transport; it must never be what turns replication on. - /// - [Theory] - [InlineData(SecretsNodeRole.Central)] - [InlineData(SecretsNodeRole.Site)] - public void ReplicationDisabled_WithGrpcModeAndFullHubConfig_WiresNeitherHubHalf( - SecretsNodeRole role) - { - IConfiguration config = BuildConfig( - ("Secrets:Replication:Enabled", "false"), - ("Secrets:Replication:Mode", "Grpc"), - ("Secrets:GrpcHub:Endpoint", DummyHubEndpoint), - ("Secrets:GrpcHub:BearerToken", DummyBearerToken)); - - using ServiceProvider provider = BuildProvider(config, role); + using ServiceProvider provider = BuildProvider(config, SecretsNodeRole.Site); Assert.IsType(provider.GetRequiredService()); - Assert.Null(provider.GetService()); - Assert.DoesNotContain(provider.GetServices(), IsGrpcHubSweepService); - Assert.False(SecretsRegistration.UsesGrpcHub(config)); + } + + [Fact] + public void Site_HasNoSqlServerPackageTypesInTheContainer() + { + // The sites-don't-talk-to-SQL pin: a site node's container must contain NOTHING from the + // SQL-Server package — not just resolve a SQLite store, but hold no descriptor that could + // ever open a connection to central's database. + var services = new ServiceCollection(); + services.AddLogging(); + services.AddScadaBridgeSecrets(BuildConfig(), SecretsNodeRole.Site); + + Assert.DoesNotContain(services, IsFromSqlServerPackage); + + using ServiceProvider provider = services.BuildServiceProvider(validateScopes: true); + + Assert.IsType(provider.GetRequiredService()); + Assert.Null(provider.GetService()); + } + + // --------------------------------------------------------------------------------------- + // Central, default — plain local SQLite, the shape every existing deployment is in. + // --------------------------------------------------------------------------------------- + + [Fact] + public void Central_DefaultsToPlainSqliteStore() + { + using ServiceProvider provider = BuildProvider(BuildConfig()); + + Assert.IsType(provider.GetRequiredService()); + } + + [Fact] + public void Central_StoreProviderSqlite_IsIndistinguishableFromUnset() + { + using ServiceProvider explicitProvider = + BuildProvider(BuildConfig(("Secrets:Store:Provider", "Sqlite"))); + using ServiceProvider unsetProvider = BuildProvider(BuildConfig()); + + Assert.IsType(explicitProvider.GetRequiredService()); + Assert.IsType(unsetProvider.GetRequiredService()); } // --------------------------------------------------------------------------------------- // Secrets:Store:Provider=SqlServer — a clustered central sharing ONE SQL secret store - // directly, without standing up the gRPC hub (task 5). Independent of Replication/Mode. + // directly, without a hub (task 5). The one replication-adjacent feature Task 11 left + // standing, because it is a plain shared database, not a converging replica set. // --------------------------------------------------------------------------------------- [Fact] - public void Central_StoreProviderSqlServer_UsesSharedSqlStore_WithoutHub() + public void Central_StoreProviderSqlServer_UsesSharedSqlStore() { IConfiguration config = BuildConfig( ("Secrets:Store:Provider", "SqlServer"), @@ -211,12 +169,6 @@ public class SecretsReplicationWiringTests using ServiceProvider provider = BuildProvider(config, SecretsNodeRole.Central); Assert.IsType(provider.GetRequiredService()); - - // No hub: neither the fail-closed bearer interceptor nor the follower sweep is wired, - // and UsesGrpcHub — the single predicate MapScadaBridgeSecretsHub also checks — is false. - Assert.Null(provider.GetService()); - Assert.DoesNotContain(provider.GetServices(), IsGrpcHubSweepService); - Assert.False(SecretsRegistration.UsesGrpcHub(config)); } [Fact] @@ -230,17 +182,18 @@ public class SecretsReplicationWiringTests var ex = Assert.Throws( () => services.AddScadaBridgeSecrets(config, SecretsNodeRole.Central)); - // Same helper, same message style as the Grpc-mode central check — but this path's - // message must name ITS OWN trigger (review round 1, finding #2), never claim - // Secrets:Replication:Mode=Grpc, which is not in effect here. Assert.Contains( SecretsRegistration.HubConnectionStringKey, ex.Message, StringComparison.Ordinal); Assert.Contains( $"{SecretsRegistration.StoreProviderKey}=SqlServer", ex.Message, StringComparison.Ordinal); - Assert.DoesNotContain( - $"{SecretsRegistration.ReplicationModeKey}=Grpc", ex.Message, StringComparison.Ordinal); } + /// + /// The store's own connection string can never be a ${secret:} reference — the pre-host + /// expander needs the store this string points at in order to resolve it (bootstrap + /// circularity). Rejected by name rather than left to fail as a malformed connection string on + /// first use. + /// [Fact] public void Central_StoreProviderSqlServer_SecretReferenceConnectionString_Throws() { @@ -254,324 +207,6 @@ public class SecretsReplicationWiringTests var ex = Assert.Throws( () => services.AddScadaBridgeSecrets(config, SecretsNodeRole.Central)); - Assert.Contains( - SecretsRegistration.HubConnectionStringKey, ex.Message, StringComparison.Ordinal); - Assert.Contains("${secret:", ex.Message, StringComparison.Ordinal); - Assert.DoesNotContain("hub-connection-string", ex.Message, StringComparison.Ordinal); - } - - [Fact] - public void Site_IgnoresStoreProvider() - { - // A Site node ignores Secrets:Store:Provider entirely and always keeps its local SQLite - // store — even with the value set to SqlServer and no connection string configured at - // all, which would fail closed on Central. - IConfiguration config = BuildConfig(("Secrets:Store:Provider", "SqlServer")); - - using ServiceProvider provider = BuildProvider(config, SecretsNodeRole.Site); - - Assert.IsType(provider.GetRequiredService()); - } - - /// - /// Precedence pin (review round 1, finding #3): with BOTH triggers configured, the - /// pre-existing Secrets:Replication:Enabled=true Grpc-hub topology wins, and the new - /// Store:Provider=SqlServer branch defers to it rather than skipping hub registration. The - /// Store:Provider branch's own guard (!ReplicationEnabledKey) is what makes this so — - /// this test locks the ordering in against a future refactor of - /// that reorders the two branches. - /// - [Fact] - public void Central_ReplicationEnabledGrpcMode_TakesPrecedenceOverStoreProvider() - { - IConfiguration config = BuildConfig( - [ - .. GrpcCentralConfig(), - ("Secrets:Store:Provider", "SqlServer"), - ]); - - using ServiceProvider provider = BuildProvider(config, SecretsNodeRole.Central); - - // Same shared SQL-Server store either way, but the Grpc hub is ALSO wired here — the - // Store:Provider-only path never registers the interceptor or the follower sweep. - Assert.IsType(provider.GetRequiredService()); - Assert.NotNull(provider.GetService()); - Assert.True(SecretsRegistration.UsesGrpcHub(config)); - } - - // --------------------------------------------------------------------------------------- - // SQL-Server mode — regression pins. Behaviour must be byte-for-byte what it was before the - // mode key existed, both when the mode is left unset and when it is named explicitly. - // --------------------------------------------------------------------------------------- - - [Fact] - public void ReplicationEnabledWithoutConnectionString_StaysOnPlainSqliteStore() - { - // The gate requires BOTH the flag and a connection string. Enabling the flag alone must not - // reach the SQL-Server package, whose options validation would throw at registration and - // take the whole node down at startup. - var services = new ServiceCollection(); - services.AddLogging(); - - IConfiguration config = BuildConfig(("Secrets:Replication:Enabled", "true")); - - services.AddScadaBridgeSecrets(config, SecretsNodeRole.Central); - - using ServiceProvider provider = services.BuildServiceProvider(validateScopes: true); - - Assert.IsType(provider.GetRequiredService()); - Assert.DoesNotContain(services, IsSqlServerReplicatorHostedService); - } - - [Theory] - [InlineData(null)] - [InlineData("SqlServer")] - public void ReplicationEnabledWithConnectionString_ResolvesReplicatingStore(string? mode) - { - IConfiguration config = mode is null - ? BuildConfig( - ("Secrets:Replication:Enabled", "true"), - ("Secrets:SqlServer:ConnectionString", DummyConnectionString)) - : BuildConfig( - ("Secrets:Replication:Enabled", "true"), - ("Secrets:Replication:Mode", mode), - ("Secrets:SqlServer:ConnectionString", DummyConnectionString)); - - using ServiceProvider provider = BuildProvider(config); - - Assert.IsType(provider.GetRequiredService()); - } - - [Fact] - public void ReplicationEnabledWithConnectionString_UndecoratedLocalStoreAlsoResolves() - { - // ReplicatingSecretStore is constructed from the CONCRETE SqliteSecretStore, not from - // ISecretStore (which is the decorator itself — that would recurse). If the concrete - // registration were ever dropped, the decorator would fail on first resolve. This is the - // exact defect that shipped once, so it gets its own test. - IConfiguration config = BuildConfig( - ("Secrets:Replication:Enabled", "true"), - ("Secrets:SqlServer:ConnectionString", DummyConnectionString)); - - using ServiceProvider provider = BuildProvider(config); - - Assert.NotNull(provider.GetRequiredService()); - } - - [Fact] - public void ReplicationEnabledWithConnectionString_RegistersReplicatorHostedServices() - { - var services = new ServiceCollection(); - services.AddLogging(); - - IConfiguration config = BuildConfig( - ("Secrets:Replication:Enabled", "true"), - ("Secrets:SqlServer:ConnectionString", DummyConnectionString)); - - services.AddScadaBridgeSecrets(config, SecretsNodeRole.Central); - - // Both the hub-schema migration service and the bidirectional sweep. - Assert.Equal(2, services.Count(IsSqlServerReplicatorHostedService)); - } - - /// - /// SQL-Server mode is role-agnostic — both nodes of a hub topology sync bidirectionally - /// against the same shared database. The role parameter added for the gRPC hub must not have - /// quietly changed that. - /// - [Theory] - [InlineData(SecretsNodeRole.Central)] - [InlineData(SecretsNodeRole.Site)] - public void SqlServerMode_IsIdenticalOnBothRoles(SecretsNodeRole role) - { - IConfiguration config = BuildConfig( - ("Secrets:Replication:Enabled", "true"), - ("Secrets:Replication:Mode", "SqlServer"), - ("Secrets:SqlServer:ConnectionString", DummyConnectionString)); - - using ServiceProvider provider = BuildProvider(config, role); - - Assert.IsType(provider.GetRequiredService()); - Assert.Null(provider.GetService()); - Assert.DoesNotContain(provider.GetServices(), IsGrpcHubSweepService); - } - - // --------------------------------------------------------------------------------------- - // gRPC mode — central hosts, site follows, and neither does the other's job. - // --------------------------------------------------------------------------------------- - - [Fact] - public void GrpcMode_Central_RegistersHubAndItsAuthInterceptor() - { - IConfiguration config = BuildConfig(GrpcCentralConfig()); - - using ServiceProvider provider = BuildProvider(config, SecretsNodeRole.Central); - - // The fail-closed bearer gate must RESOLVE, not merely be listed: an interceptor that - // cannot be activated is one gRPC would fail to attach, which is an ungated hub. - Assert.NotNull(provider.GetService()); - Assert.True(SecretsRegistration.UsesGrpcHub(config)); - - // Central's store is the SHARED SQL-Server store — one copy of every row, so both - // central hubs serve identical manifests by construction (scadaproj#4). Undecorated: - // pull-only means nothing publishes, so there is still nothing to wrap it in. - Assert.IsType(provider.GetRequiredService()); - } - - [Fact] - public void GrpcMode_Central_DoesNotRegisterTheFollowerSweep() - { - // Central is the origin of every write. A sweep here would have it pull from itself, and - // on a misconfigured endpoint, from somebody else. - using ServiceProvider provider = - BuildProvider(BuildConfig(GrpcCentralConfig()), SecretsNodeRole.Central); - - Assert.DoesNotContain(provider.GetServices(), IsGrpcHubSweepService); - } - - [Fact] - public void GrpcMode_Site_RegistersTheFollowerSweep() - { - using ServiceProvider provider = - BuildProvider(BuildConfig(GrpcSiteConfig()), SecretsNodeRole.Site); - - // Resolving the hosted services builds the sweep's whole graph — reader, keyed channel, - // local store — which is where a missing AddZbSecrets would surface. - Assert.Contains(provider.GetServices(), IsGrpcHubSweepService); - Assert.IsType(provider.GetRequiredService()); - } - - [Fact] - public void GrpcMode_Site_DoesNotHostTheHub() - { - // A site hosting the hub would serve central's secrets to anything holding the shared - // token, from inside the site network. - using ServiceProvider provider = - BuildProvider(BuildConfig(GrpcSiteConfig()), SecretsNodeRole.Site); - - Assert.Null(provider.GetService()); - } - - /// - /// A configured fallback endpoint swaps the sweep's reader for the package's failover - /// composition and adds one keyed channel per fallback. Safe ONLY because both central nodes - /// serve one shared SQL store (scadaproj#4) — the type-name assertion is deliberate: the - /// failover reader is internal to the package, and its name is the observable contract here. - /// - [Fact] - public void GrpcMode_Site_WithFallbackEndpoint_ResolvesTheFailoverReader() - { - IConfiguration config = BuildConfig( - [ - .. GrpcSiteConfig(), - ("Secrets:GrpcHub:FallbackEndpoints:0", DummyFallbackHubEndpoint), - ]); - - using ServiceProvider provider = BuildProvider(config, SecretsNodeRole.Site); - - object reader = provider.GetRequiredService(HubReaderInterface); - Assert.Equal("FailoverSecretsHubReader", reader.GetType().Name); - Assert.NotNull(provider.GetKeyedService(FallbackChannelKey)); - } - - /// - /// Zero fallbacks pins the unchanged default: the reader stays the plain single-endpoint - /// client and no fallback channel enters the container — exactly the pre-0.5.0 shape every - /// existing deployment is in. - /// - [Fact] - public void GrpcMode_Site_WithoutFallbackEndpoints_KeepsThePlainHubClient() - { - using ServiceProvider provider = - BuildProvider(BuildConfig(GrpcSiteConfig()), SecretsNodeRole.Site); - - object reader = provider.GetRequiredService(HubReaderInterface); - Assert.Equal("GrpcSecretsHubClient", reader.GetType().Name); - Assert.Null(provider.GetKeyedService(FallbackChannelKey)); - } - - // --------------------------------------------------------------------------------------- - // gRPC mode fails closed. There is no local-only fallback here, unlike SQL-Server mode: - // a site quietly serving secrets that never converge is the outcome the hub exists to prevent. - // --------------------------------------------------------------------------------------- - - [Fact] - public void GrpcMode_Central_WithoutBearerToken_FailsAtRegistration() - { - var services = new ServiceCollection(); - services.AddLogging(); - - // Connection string present so the missing bearer token is the ONE defect in this config — - // the connection-string pre-check runs first and would otherwise mask it. - IConfiguration config = BuildConfig( - ("Secrets:Replication:Enabled", "true"), - ("Secrets:Replication:Mode", "Grpc"), - ("Secrets:SqlServer:ConnectionString", DummyConnectionString)); - - var ex = Assert.Throws( - () => services.AddScadaBridgeSecrets(config, SecretsNodeRole.Central)); - - // The package's own message, unwrapped, naming the exact key an operator has to set. - Assert.Contains("Secrets:GrpcHub:BearerToken", ex.Message, StringComparison.Ordinal); - } - - /// - /// Central + gRPC + no connection string is the scadaproj#4 defect asked for by name: each - /// central node on an independent local store, one of them answering authenticated followers - /// with an EMPTY manifest. It must be a boot failure naming the key, never a quiet fallback. - /// - [Theory] - [InlineData(null)] - [InlineData("")] - [InlineData(" ")] - public void GrpcMode_Central_WithBlankConnectionString_FailsAtRegistration(string? connectionString) - { - var services = new ServiceCollection(); - services.AddLogging(); - - (string, string)[] overrides = connectionString is null - ? - [ - ("Secrets:Replication:Enabled", "true"), - ("Secrets:Replication:Mode", "Grpc"), - ("Secrets:GrpcHub:BearerToken", DummyBearerToken), - ] - : - [ - ("Secrets:Replication:Enabled", "true"), - ("Secrets:Replication:Mode", "Grpc"), - ("Secrets:GrpcHub:BearerToken", DummyBearerToken), - ("Secrets:SqlServer:ConnectionString", connectionString), - ]; - - var ex = Assert.Throws( - () => services.AddScadaBridgeSecrets(BuildConfig(overrides), SecretsNodeRole.Central)); - - Assert.Contains( - SecretsRegistration.HubConnectionStringKey, ex.Message, StringComparison.Ordinal); - } - - /// - /// The store's own connection string can never be a ${secret:} reference — the pre-host - /// expander needs the store this string points at in order to resolve it (bootstrap - /// circularity, the same rule as the hub bearer token). Rejected by name rather than left to - /// fail as a malformed connection string on first use. - /// - [Fact] - public void GrpcMode_Central_WithSecretReferenceConnectionString_FailsAtRegistration() - { - var services = new ServiceCollection(); - services.AddLogging(); - - IConfiguration config = BuildConfig( - ("Secrets:Replication:Enabled", "true"), - ("Secrets:Replication:Mode", "Grpc"), - ("Secrets:GrpcHub:BearerToken", DummyBearerToken), - ("Secrets:SqlServer:ConnectionString", "${secret:hub-connection-string}")); - - var ex = Assert.Throws( - () => services.AddScadaBridgeSecrets(config, SecretsNodeRole.Central)); - Assert.Contains( SecretsRegistration.HubConnectionStringKey, ex.Message, StringComparison.Ordinal); Assert.Contains("${secret:", ex.Message, StringComparison.Ordinal); @@ -579,227 +214,4 @@ public class SecretsReplicationWiringTests // the configured value, because a mistyped value here could be a pasted real credential. Assert.DoesNotContain("hub-connection-string", ex.Message, StringComparison.Ordinal); } - - /// - /// The sites-don't-talk-to-SQL pin: a site node's container must contain NOTHING from the - /// SQL-Server replicator package — not just resolve a SQLite store, but hold no descriptor - /// that could ever open a connection to central's database. - /// - [Fact] - public void GrpcMode_Site_HasNoSqlServerPackageTypesInTheContainer() - { - var services = new ServiceCollection(); - services.AddLogging(); - services.AddScadaBridgeSecrets(BuildConfig(GrpcSiteConfig()), SecretsNodeRole.Site); - - Assert.DoesNotContain(services, IsFromSqlServerReplicatorPackage); - - using ServiceProvider provider = services.BuildServiceProvider(validateScopes: true); - - Assert.IsType(provider.GetRequiredService()); - Assert.Null(provider.GetService()); - } - - [Fact] - public void GrpcMode_Site_WithoutEndpoint_FailsAtRegistration() - { - var services = new ServiceCollection(); - services.AddLogging(); - - IConfiguration config = BuildConfig( - ("Secrets:Replication:Enabled", "true"), - ("Secrets:Replication:Mode", "Grpc"), - ("Secrets:GrpcHub:BearerToken", DummyBearerToken)); - - var ex = Assert.Throws( - () => services.AddScadaBridgeSecrets(config, SecretsNodeRole.Site)); - - Assert.Contains("Secrets:GrpcHub:Endpoint", ex.Message, StringComparison.Ordinal); - } - - [Fact] - public void GrpcMode_Site_WithoutBearerToken_FailsAtRegistration() - { - var services = new ServiceCollection(); - services.AddLogging(); - - IConfiguration config = BuildConfig( - ("Secrets:Replication:Enabled", "true"), - ("Secrets:Replication:Mode", "Grpc"), - ("Secrets:GrpcHub:Endpoint", DummyHubEndpoint)); - - var ex = Assert.Throws( - () => services.AddScadaBridgeSecrets(config, SecretsNodeRole.Site)); - - Assert.Contains("Secrets:GrpcHub:BearerToken", ex.Message, StringComparison.Ordinal); - } - - // --------------------------------------------------------------------------------------- - // Mode key parsing. - // --------------------------------------------------------------------------------------- - - [Theory] - [InlineData(null, SecretsReplicationMode.SqlServer)] - [InlineData("", SecretsReplicationMode.SqlServer)] - [InlineData("SqlServer", SecretsReplicationMode.SqlServer)] - [InlineData("sqlserver", SecretsReplicationMode.SqlServer)] - [InlineData("Grpc", SecretsReplicationMode.Grpc)] - [InlineData("grpc", SecretsReplicationMode.Grpc)] - [InlineData(" Grpc ", SecretsReplicationMode.Grpc)] - public void ResolveReplicationMode_ReadsTheKey(string? value, SecretsReplicationMode expected) - { - IConfiguration config = value is null - ? BuildConfig() - : BuildConfig(("Secrets:Replication:Mode", value)); - - Assert.Equal(expected, SecretsRegistration.ResolveReplicationMode(config)); - } - - [Fact] - public void ResolveReplicationMode_RejectsAnUnknownValue_EvenWithReplicationOff() - { - // Caught at the boot that introduces the typo rather than at whichever later boot flips - // Secrets:Replication:Enabled — otherwise an unrelated change fails on a key nobody touched. - IConfiguration config = BuildConfig(("Secrets:Replication:Mode", "GrpcHub")); - - var ex = Assert.Throws( - () => SecretsRegistration.ResolveReplicationMode(config)); - - Assert.Contains("Secrets:Replication:Mode", ex.Message, StringComparison.Ordinal); - } - - /// - /// A numeric value must not be accepted. Enum.TryParse happily parses any integer, - /// including ones outside the enum, so "2" would otherwise select a mode that does not exist - /// and fall through the switch to the SQL-Server branch. - /// - [Fact] - public void ResolveReplicationMode_RejectsAnOutOfRangeNumericValue() - { - IConfiguration config = BuildConfig(("Secrets:Replication:Mode", "7")); - - Assert.Throws( - () => SecretsRegistration.ResolveReplicationMode(config)); - } -} - -/// -/// Endpoint-mapping pins for the central-hosted secrets hub. -/// -/// Registration and mapping are gated by one shared predicate on purpose, and this asserts the -/// consequence: the hub endpoint exists exactly when the node that serves it registered the -/// fail-closed interceptor. A mapped-but-unregistered hub would be an anonymous endpoint serving -/// every secret central holds, and it would look completely healthy. -/// -/// -public class SecretsHubEndpointMappingTests -{ - private const string DummyBearerToken = "test-hub-token"; - - private static WebApplication BuildCentralApp(params (string Key, string Value)[] overrides) - { - var builder = WebApplication.CreateBuilder(); - builder.Configuration.Sources.Clear(); - - var settings = new Dictionary - { - ["Secrets:SqlitePath"] = Path.Combine( - Path.GetTempPath(), $"sb-secrets-map-test-{Guid.NewGuid():N}.db"), - ["Secrets:MasterKey:Source"] = "Environment", - ["Secrets:MasterKey:EnvVarName"] = "ZB_SECRETS_MASTER_KEY", - ["Secrets:RunMigrationsOnStartup"] = "false", - }; - - foreach ((string key, string value) in overrides) - { - settings[key] = value; - } - - builder.Configuration.AddInMemoryCollection(settings); - builder.Services.AddGrpc(); - builder.Services.AddScadaBridgeSecrets(builder.Configuration, SecretsNodeRole.Central); - - WebApplication app = builder.Build(); - app.MapScadaBridgeSecretsHub(app.Configuration); - return app; - } - - /// - /// The gRPC service's WIRE name, from package zb.mom.ww.secrets.hub.v1; service SecretsHub. - /// Matched on the route pattern rather than on the C# type, because the route is the thing a - /// follower actually addresses and the thing a rename would break. - /// - private const string HubRoutePrefix = "/zb.mom.ww.secrets.hub.v1.SecretsHub/"; - - private static IReadOnlyList HubRoutes(WebApplication app) => - ((IEndpointRouteBuilder)app).DataSources - .SelectMany(source => source.Endpoints) - .OfType() - .Select(endpoint => "/" + endpoint.RoutePattern.RawText?.TrimStart('/')) - .Where(route => route.StartsWith(HubRoutePrefix, StringComparison.Ordinal)) - // Grpc.AspNetCore always adds a "{unimplementedMethod}" catch-all per service, which - // answers Unimplemented. It is framework scaffolding, not an RPC the contract declares. - .Where(route => !route.Contains('{', StringComparison.Ordinal)) - .ToList(); - - private static bool HasHubEndpoint(WebApplication app) => HubRoutes(app).Count > 0; - - // Syntactically valid, deliberately unreachable — central + gRPC requires the shared - // SQL-Server store's connection string at registration; nothing here ever connects. - private const string DummyConnectionString = - "Server=unused;Database=x;Integrated Security=true;"; - - [Fact] - public void GrpcMode_Central_MapsTheHubEndpoint() - { - using WebApplication app = BuildCentralApp( - ("Secrets:Replication:Enabled", "true"), - ("Secrets:Replication:Mode", "Grpc"), - ("Secrets:GrpcHub:BearerToken", DummyBearerToken), - ("Secrets:SqlServer:ConnectionString", DummyConnectionString)); - - Assert.True(HasHubEndpoint(app)); - } - - /// - /// Pull-only is a property of the wire contract, and this is where it becomes observable to - /// ScadaBridge: central maps exactly the two READ methods and nothing that could accept a - /// pushed row. If a future package version adds a write RPC, this fails here rather than - /// silently opening a path for a site to overwrite central's secrets. - /// - [Fact] - public void GrpcMode_Central_MapsOnlyTheTwoReadMethods() - { - using WebApplication app = BuildCentralApp( - ("Secrets:Replication:Enabled", "true"), - ("Secrets:Replication:Mode", "Grpc"), - ("Secrets:GrpcHub:BearerToken", DummyBearerToken), - ("Secrets:SqlServer:ConnectionString", DummyConnectionString)); - - Assert.Equal( - [HubRoutePrefix + "GetManifest", HubRoutePrefix + "GetSecrets"], - HubRoutes(app).Order(StringComparer.Ordinal).ToArray()); - } - - [Fact] - public void ReplicationDisabled_MapsNoHubEndpoint() - { - using WebApplication app = BuildCentralApp( - ("Secrets:Replication:Enabled", "false"), - ("Secrets:Replication:Mode", "Grpc"), - ("Secrets:GrpcHub:BearerToken", DummyBearerToken)); - - Assert.False(HasHubEndpoint(app)); - } - - [Fact] - public void SqlServerMode_MapsNoHubEndpoint() - { - using WebApplication app = BuildCentralApp( - ("Secrets:Replication:Enabled", "true"), - ("Secrets:Replication:Mode", "SqlServer"), - ("Secrets:SqlServer:ConnectionString", "Server=unused;Database=x;Integrated Security=true;")); - - Assert.False(HasHubEndpoint(app)); - } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/StartupValidatorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/StartupValidatorTests.cs index 628fe73f..934b63f1 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/StartupValidatorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/StartupValidatorTests.cs @@ -771,23 +771,58 @@ public class StartupValidatorTests Assert.Empty(warnings); } - [Fact] - public void ClusteredCentral_ReplicationEnabled_NoSecretsStoreWarning() + /// + /// Task 11: secrets replication (the gRPC hub, and the SQL-Server ReplicatingSecretStore mode + /// that preceded it) is retired in ScadaBridge — sites receive secrets exclusively via Deploy + /// Artifacts, and a replicated site store would publish DeployedSecretsMirror's local deletes + /// fleet-wide. Checked on BOTH roles: a Site used to be the gRPC hub's follower half, so the + /// flag was never Central-only. Replaces the old warning-suppression test + /// (ClusteredCentral_ReplicationEnabled_NoSecretsStoreWarning) — that suppression is gone too, + /// because CollectWarnings never runs with this flag true on a config that also passed + /// Validate. + /// + [Theory] + [InlineData("true")] + [InlineData("True")] + public void ReplicationEnabled_IsRejected_OnCentral(string value) { - // Review round 1, finding #1: Secrets:Replication:Enabled=true already puts this central - // node on a topology that shares state across the pair — the pre-existing gRPC hub - // (whose central-side store is the shared SQL-Server store, not per-node SQLite) or - // SQL-Server replication. No Secrets:Store:Provider is set at all here — the exact case - // that used to warn before this fix — and it must stay silent because the node is not - // actually on a per-node SQLite store in this configuration. var values = ValidCentralConfig(); - values["ScadaBridge:InboundApi:ApiKeyStore:Provider"] = "SqlServer"; + values["Secrets:Replication:Enabled"] = value; + var config = BuildConfig(values); + + var ex = Assert.Throws(() => StartupValidator.Validate(config)); + + Assert.Contains( + "Secrets replication (the gRPC hub) is retired in ScadaBridge", ex.Message); + Assert.Contains("sites receive secrets with Deploy Artifacts", ex.Message); + Assert.Contains("central uses Secrets:Store:Provider=SqlServer", ex.Message); + } + + [Fact] + public void ReplicationEnabled_IsRejected_OnSite() + { + var values = ValidSiteConfig(); values["Secrets:Replication:Enabled"] = "true"; var config = BuildConfig(values); - var warnings = StartupValidator.CollectWarnings(config); + var ex = Assert.Throws(() => StartupValidator.Validate(config)); - Assert.Empty(warnings); + Assert.Contains( + "Secrets replication (the gRPC hub) is retired in ScadaBridge", ex.Message); + } + + [Fact] + public void ReplicationDisabled_PassesValidation() + { + // Explicit false must not be confused with "set" — the default posture every existing + // deployment ships stays valid. + var values = ValidCentralConfig(); + values["Secrets:Replication:Enabled"] = "false"; + var config = BuildConfig(values); + + var ex = Record.Exception(() => StartupValidator.Validate(config)); + + Assert.Null(ex); } [Fact]