feat(secrets): Secrets:Store:Provider=SqlServer — central pair shares one SQL secret store without the hub

2026-09-28 17:44:14 -04:00
parent ebc55c5f72
commit 35235ecfed
5 changed files with 202 additions and 10 deletions
@@ -131,6 +131,17 @@ public static class SecretsRegistration
    /// <summary>Configuration key holding the shared SQL-Server hub connection string.</summary>
    public const string HubConnectionStringKey = "Secrets:SqlServer:ConnectionString";

+    /// <summary>
+    /// Configuration key selecting a CENTRAL node's own local store provider — independent of
+    /// <see cref="ReplicationEnabledKey"/>/<see cref="ReplicationModeKey"/> and the gRPC hub.
+    /// Absent or blank = <c>Sqlite</c> (per-node, the historical default); <c>SqlServer</c> 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.
+    /// </summary>
+    public const string StoreProviderKey = "Secrets:Store:Provider";
+
    /// <summary>
    /// Reads <see cref="ReplicationModeKey"/>. An absent or blank value is
    /// <see cref="SecretsReplicationMode.SqlServer"/>; anything unrecognised throws.
@@ -319,6 +330,28 @@ 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.
+        if (role == SecretsNodeRole.Central
+            && string.Equals(config[StoreProviderKey], "SqlServer", StringComparison.OrdinalIgnoreCase)
+            && !config.GetValue<bool>(ReplicationEnabledKey))
+        {
+            // 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).
+            EnsureCentralSharedStoreConnectionString(config);
+
+            // 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.
+            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.
@@ -97,6 +97,17 @@ public static class StartupValidator
                    value => string.IsNullOrWhiteSpace(value)
                        || (Enum.TryParse<InboundApiKeyStoreProvider>(value, ignoreCase: true, out var provider)
                            && Enum.IsDefined(provider)),
+                    "must be 'Sqlite' or 'SqlServer' when set")
+                // The central shared secret store provider (Secrets:Store:Provider — a fleet that
+                // ships secrets to sites on every artifact deploy needs a clustered central pair to
+                // see the same secret set). Not required — an unset value defaults to Sqlite,
+                // per-node, mirroring the inbound API-key store default above — but when it IS set,
+                // it must be one of the two legal values so a typo fails fast at boot instead of
+                // silently keeping the per-node SQLite store it looks like it opted out of.
+                .Require("Secrets:Store:Provider",
+                    value => string.IsNullOrWhiteSpace(value)
+                        || string.Equals(value, "Sqlite", StringComparison.OrdinalIgnoreCase)
+                        || string.Equals(value, "SqlServer", StringComparison.OrdinalIgnoreCase),
                    "must be 'Sqlite' or 'SqlServer' when set"))
            // SeedNodes count (unconditional, after SiteId).
            .Require("ScadaBridge:Cluster:SeedNodes",
@@ -226,20 +237,33 @@ public static class StartupValidator
        var role = configuration.GetSection("ScadaBridge:Node")["Role"];
        var seedNodes = configuration.GetSection("ScadaBridge:Cluster:SeedNodes").Get<List<string>>();
        var provider = InboundApiKeyStoreOptions.ReadProvider(configuration);
+        var secretsStoreProvider = configuration["Secrets:Store:Provider"];
+        var isClusteredCentral = role == "Central" && seedNodes is { Count: >= 2 };

-        if (role == "Central"
-            && seedNodes is { Count: >= 2 }
-            && provider == InboundApiKeyStoreProvider.Sqlite)
+        var warnings = new List<string>();
+
+        if (isClusteredCentral && provider == InboundApiKeyStoreProvider.Sqlite)
        {
-            return new[]
-            {
-                $"Inbound API keys use the per-node SQLite store on a {seedNodes.Count}-node central "
+            warnings.Add(
+                $"Inbound API keys use the per-node SQLite store on a {seedNodes!.Count}-node central "
                + "cluster; a failover will reject every inbound client until keys are re-issued. Set "
-                + "ScadaBridge:InboundApi:ApiKeyStore:Provider=SqlServer (design 2026-09-02)."
-            };
+                + "ScadaBridge:InboundApi:ApiKeyStore:Provider=SqlServer (design 2026-09-02).");
        }

-        return Array.Empty<string>();
+        // Secrets:Store:Provider defaults to Sqlite (per-node) when unset, mirroring the inbound
+        // API-key store default above — so this fires both when the key is absent and when it is
+        // explicitly "Sqlite". A fleet that ships secrets to sites on every artifact deploy needs
+        // 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.
+        if (isClusteredCentral
+            && !string.Equals(secretsStoreProvider, "SqlServer", StringComparison.OrdinalIgnoreCase))
+        {
+            warnings.Add(
+                "Secrets use a per-node SQLite store on a clustered central — each node would "
+                + "deploy its own secret set to sites; set Secrets:Store:Provider=SqlServer");
+        }
+
+        return warnings;
    }

    /// <summary>
@@ -14,6 +14,10 @@
    "MasterKey": { "Source": "Environment", "EnvVarName": "ZB_SECRETS_MASTER_KEY" },
    "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.",
+      "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,
@@ -196,6 +196,77 @@ public class SecretsReplicationWiringTests
        Assert.False(SecretsRegistration.UsesGrpcHub(config));
    }

+    // ---------------------------------------------------------------------------------------
+    // 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.
+    // ---------------------------------------------------------------------------------------
+
+    [Fact]
+    public void Central_StoreProviderSqlServer_UsesSharedSqlStore_WithoutHub()
+    {
+        IConfiguration config = BuildConfig(
+            ("Secrets:Store:Provider", "SqlServer"),
+            ("Secrets:SqlServer:ConnectionString", DummyConnectionString));
+
+        using ServiceProvider provider = BuildProvider(config, SecretsNodeRole.Central);
+
+        Assert.IsType<SqlServerSecretStore>(provider.GetRequiredService<ISecretStore>());
+
+        // 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<SecretsHubAuthInterceptor>());
+        Assert.DoesNotContain(provider.GetServices<IHostedService>(), IsGrpcHubSweepService);
+        Assert.False(SecretsRegistration.UsesGrpcHub(config));
+    }
+
+    [Fact]
+    public void Central_StoreProviderSqlServer_BlankConnectionString_Throws()
+    {
+        var services = new ServiceCollection();
+        services.AddLogging();
+
+        IConfiguration config = BuildConfig(("Secrets:Store:Provider", "SqlServer"));
+
+        var ex = Assert.Throws<InvalidOperationException>(
+            () => services.AddScadaBridgeSecrets(config, SecretsNodeRole.Central));
+
+        // Same helper, same message style as the Grpc-mode central check.
+        Assert.Contains(
+            SecretsRegistration.HubConnectionStringKey, ex.Message, StringComparison.Ordinal);
+    }
+
+    [Fact]
+    public void Central_StoreProviderSqlServer_SecretReferenceConnectionString_Throws()
+    {
+        var services = new ServiceCollection();
+        services.AddLogging();
+
+        IConfiguration config = BuildConfig(
+            ("Secrets:Store:Provider", "SqlServer"),
+            ("Secrets:SqlServer:ConnectionString", "${secret:hub-connection-string}"));
+
+        var ex = Assert.Throws<InvalidOperationException>(
+            () => 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<SqliteSecretStore>(provider.GetRequiredService<ISecretStore>());
+    }
+
    // ---------------------------------------------------------------------------------------
    // 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.
@@ -699,7 +699,11 @@ public class StartupValidatorTests
    {
        // Design 2026-09-02: the inbound API key store is per-node SQLite by default, so a
        // 2+-node central cluster is one failover away from rejecting every inbound client.
-        var config = BuildConfig(ValidCentralConfig());
+        // Secrets:Store:Provider is set to SqlServer here to isolate this warning from the
+        // independent one added for task 5 (ClusteredCentral_SqliteSecretStore_Warns below).
+        var values = ValidCentralConfig();
+        values["Secrets:Store:Provider"] = "SqlServer";
+        var config = BuildConfig(values);

        var warnings = StartupValidator.CollectWarnings(config);

@@ -714,6 +718,7 @@ public class StartupValidatorTests
    {
        var values = ValidCentralConfig();
        values["ScadaBridge:InboundApi:ApiKeyStore:Provider"] = "sqlite";
+        values["Secrets:Store:Provider"] = "SqlServer";
        var config = BuildConfig(values);

        var warnings = StartupValidator.CollectWarnings(config);
@@ -726,6 +731,39 @@ public class StartupValidatorTests
    {
        var values = ValidCentralConfig();
        values["ScadaBridge:InboundApi:ApiKeyStore:Provider"] = "SqlServer";
+        values["Secrets:Store:Provider"] = "SqlServer";
+        var config = BuildConfig(values);
+
+        var warnings = StartupValidator.CollectWarnings(config);
+
+        Assert.Empty(warnings);
+    }
+
+    [Fact]
+    public void ClusteredCentral_SqliteSecretStore_Warns()
+    {
+        // Task 5: a clustered central left on the per-node SQLite secret store silently drifts
+        // the moment anyone edits a secret on only one node, then ships an inconsistent set to
+        // sites. ApiKeyStore:Provider is set to SqlServer here to isolate this warning from the
+        // pre-existing one above.
+        var values = ValidCentralConfig();
+        values["ScadaBridge:InboundApi:ApiKeyStore:Provider"] = "SqlServer";
+        var config = BuildConfig(values);
+
+        var warnings = StartupValidator.CollectWarnings(config);
+
+        var warning = Assert.Single(warnings);
+        Assert.Contains("clustered central", warning);
+        Assert.Contains("SQLite", warning);
+        Assert.Contains("Secrets:Store:Provider=SqlServer", warning);
+    }
+
+    [Fact]
+    public void ClusteredCentral_SqlServerSecretStore_Silent()
+    {
+        var values = ValidCentralConfig();
+        values["ScadaBridge:InboundApi:ApiKeyStore:Provider"] = "SqlServer";
+        values["Secrets:Store:Provider"] = "SqlServer";
        var config = BuildConfig(values);

        var warnings = StartupValidator.CollectWarnings(config);
@@ -779,6 +817,28 @@ public class StartupValidatorTests
        Assert.Null(ex);
    }

+    [Fact]
+    public void UnknownSecretStoreProvider_Fails()
+    {
+        var values = ValidCentralConfig();
+        values["Secrets:Store:Provider"] = "Postgres";
+        var config = BuildConfig(values);
+
+        var ex = Assert.Throws<InvalidOperationException>(() => StartupValidator.Validate(config));
+        Assert.Contains("Secrets:Store:Provider", ex.Message);
+    }
+
+    [Fact]
+    public void Validate_AcceptsSecretStoreProviderCaseInsensitively()
+    {
+        var values = ValidCentralConfig();
+        values["Secrets:Store:Provider"] = "sqlserver";
+        var config = BuildConfig(values);
+
+        var ex = Record.Exception(() => StartupValidator.Validate(config));
+        Assert.Null(ex);
+    }
+
    [Fact]
    public void MultipleErrors_AllReported()
    {