diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftValidator.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftValidator.cs
index 5b9bc7c7..fde97dbd 100644
--- a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftValidator.cs
+++ b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftValidator.cs
@@ -60,6 +60,11 @@ public static class DraftValidator
/// credential in the ConfigDb — persisted, replicated to every node's artifact cache, and readable by
/// anyone with config access — while the runtime silently ignored it and the driver failed to connect.
/// Discarding a secret on read is not the same as refusing to store it.
+ /// Two of the three surfaces are checked here. The third — a node's
+ /// — is not reachable from
+ /// and is gated at its save instead (#499); see
+ /// for why a deploy gate is the wrong instrument for a value that
+ /// never enters the artifact.
/// Checked on both config surfaces a Sql driver reads: the instance's
/// and the of every device
/// beneath it, because the two are merged before the DTO sees them — a credential pasted into the
@@ -74,7 +79,7 @@ public static class DraftValidator
///
private static void ValidateSqlConnectionStringNotPersisted(DraftSnapshot draft, List errors)
{
- const string ForbiddenKey = "connectionString";
+ const string ForbiddenKey = SqlCredentialGuard.ForbiddenKey;
var sqlInstanceIds = draft.DriverInstances
.Where(d => string.Equals(d.DriverType, Core.Abstractions.DriverTypeNames.Sql, StringComparison.Ordinal))
@@ -85,7 +90,7 @@ public static class DraftValidator
foreach (var d in draft.DriverInstances)
{
if (!sqlInstanceIds.Contains(d.DriverInstanceId)) continue;
- if (!HasTopLevelKey(d.DriverConfig, ForbiddenKey)) continue;
+ if (!SqlCredentialGuard.CarriesLiteralConnectionString(d.DriverConfig)) continue;
errors.Add(new("SqlConnectionStringPersisted",
$"Sql driver instance '{d.DriverInstanceId}' has a '{ForbiddenKey}' key in its DriverConfig. " +
"A Sql driver must name its credentials indirectly via 'connectionStringRef', which resolves " +
@@ -98,7 +103,7 @@ public static class DraftValidator
foreach (var dev in draft.Devices)
{
if (!sqlInstanceIds.Contains(dev.DriverInstanceId)) continue;
- if (!HasTopLevelKey(dev.DeviceConfig, ForbiddenKey)) continue;
+ if (!SqlCredentialGuard.CarriesLiteralConnectionString(dev.DeviceConfig)) continue;
errors.Add(new("SqlConnectionStringPersisted",
$"Device '{dev.DeviceId}' on Sql driver instance '{dev.DriverInstanceId}' has a " +
$"'{ForbiddenKey}' key in its DeviceConfig. DeviceConfig is merged onto DriverConfig before " +
@@ -107,34 +112,6 @@ public static class DraftValidator
}
}
- ///
- /// True when is a JSON object carrying at its top level,
- /// matched case-insensitively. Never throws — a blank, malformed or non-object blob simply has no keys,
- /// and shaping the config JSON is another rule's job.
- ///
- /// The config blob to inspect.
- /// The property name to look for.
- /// when the key is present at the top level.
- private static bool HasTopLevelKey(string? json, string key)
- {
- if (string.IsNullOrWhiteSpace(json)) return false;
- try
- {
- using var doc = System.Text.Json.JsonDocument.Parse(json);
- if (doc.RootElement.ValueKind != System.Text.Json.JsonValueKind.Object) return false;
- foreach (var property in doc.RootElement.EnumerateObject())
- {
- if (string.Equals(property.Name, key, StringComparison.OrdinalIgnoreCase)) return true;
- }
-
- return false;
- }
- catch (System.Text.Json.JsonException)
- {
- return false;
- }
- }
-
/// WP7 Calculation-driver deploy gates. For every tag bound to a Calculation driver:
///
/// - scriptId existence — the tag's TagConfig.scriptId must be present and resolve to
diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/SqlCredentialGuard.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/SqlCredentialGuard.cs
new file mode 100644
index 00000000..11ed396d
--- /dev/null
+++ b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/SqlCredentialGuard.cs
@@ -0,0 +1,139 @@
+using System.Text.Json;
+
+namespace ZB.MOM.WW.OtOpcUa.Configuration.Validation;
+
+///
+/// The one place that decides whether a persisted config blob carries a literal Sql
+/// connectionString (Gitea #498 / #499).
+///
+///
+///
+/// A Sql driver names its credentials indirectly — connectionStringRef resolves from
+/// the environment / secret store at Initialize — so nothing persisted should ever contain a
+/// database password. The typed SqlDriverConfigDto already drops a connectionString
+/// key on read, but discarding a secret on read is not the same as refusing to store it:
+/// config blobs are schemaless JSON columns authored through raw-JSON textareas, so the key can
+/// be written even though the runtime ignores it.
+///
+///
+/// There are three surfaces a Sql driver's config is persisted on, and they need different
+/// enforcement points:
+///
+/// - DriverInstance.DriverConfig and Device.DeviceConfig — both in the
+/// deployed artifact, both visible to DraftSnapshot, both gated by
+/// DraftValidator.
+/// - ClusterNode.DriverConfigOverridesJson — the per-node override map. It is
+/// not in DraftSnapshot, and deliberately does not go there: adding a
+/// ClusterNode collection would widen the snapshot (and every builder of one) for a
+/// single rule about a value that is not part of the artifact at all. More to the point, a
+/// deploy gate is the wrong instrument here — the artifact never carries node overrides, so
+/// blocking a deploy would not stop the credential being stored. It is already in the
+/// database by then. This surface is therefore gated at the save, which is the only
+/// point where "refuse to store it" is literally true.
+///
+///
+///
+/// Deliberately narrow. This checks the Sql driver's connectionString key
+/// only — not a general "credential-shaped key" sweep over every driver type. A broader rule
+/// would start refusing configs that are legitimate today for drivers that never made this
+/// guarantee, turning defence-in-depth into a regression. Widen it per driver, as each driver
+/// gains an indirect-credential contract of its own.
+///
+///
+public static class SqlCredentialGuard
+{
+ /// The config key a Sql driver must never persist. Matched case-insensitively.
+ public const string ForbiddenKey = "connectionString";
+
+ ///
+ /// Whether is a JSON object carrying at
+ /// its top level.
+ ///
+ ///
+ /// Matched case-insensitively: System.Text.Json binds ConnectionString to a
+ /// connectionString property by default, so a case variant is the same key, not a
+ /// different one. Only the top level is scanned — the DTO is flat, so a nested occurrence cannot
+ /// bind and is not the credential-shaped mistake this exists to catch.
+ ///
+ /// The config blob to inspect.
+ /// when the key is present at the top level.
+ public static bool CarriesLiteralConnectionString(string? configJson)
+ {
+ if (string.IsNullOrWhiteSpace(configJson)) return false;
+ try
+ {
+ using var doc = JsonDocument.Parse(configJson);
+ return doc.RootElement.ValueKind == JsonValueKind.Object
+ && HasTopLevelKey(doc.RootElement, ForbiddenKey);
+ }
+ catch (JsonException)
+ {
+ // A malformed blob simply has no keys. Shaping the config JSON is another rule's job, and
+ // throwing here would turn a formatting mistake into a credential-guard failure.
+ return false;
+ }
+ }
+
+ ///
+ /// The DriverInstanceIds in a ClusterNode.DriverConfigOverridesJson map whose
+ /// override object carries a literal connection string, restricted to instances that are actually
+ /// Sql drivers.
+ ///
+ ///
+ /// The override map is shaped { "<DriverInstanceId>": { …driver config keys… } } and is
+ /// merged onto the cluster-level DriverConfig, so a credential pasted into one lands in
+ /// exactly the place already refuses on the driver
+ /// itself.
+ ///
+ /// The node's override map. Null / blank / malformed yields no
+ /// violations.
+ /// The ids of driver instances whose DriverType is
+ /// Sql. Keys outside this set are ignored — a non-Sql driver never made the indirect-credential
+ /// guarantee, so flagging it would be a regression, not defence in depth.
+ /// The offending driver-instance ids, in document order; empty when there are none.
+ public static IReadOnlyList FindNodeOverrideViolations(
+ string? overridesJson, IReadOnlyCollection sqlDriverInstanceIds)
+ {
+ if (string.IsNullOrWhiteSpace(overridesJson) || sqlDriverInstanceIds.Count == 0)
+ {
+ return [];
+ }
+
+ var sqlIds = sqlDriverInstanceIds as IReadOnlySet
+ ?? sqlDriverInstanceIds.ToHashSet(StringComparer.Ordinal);
+
+ try
+ {
+ using var doc = JsonDocument.Parse(overridesJson);
+ if (doc.RootElement.ValueKind != JsonValueKind.Object) return [];
+
+ List? offenders = null;
+ foreach (var entry in doc.RootElement.EnumerateObject())
+ {
+ if (!sqlIds.Contains(entry.Name)) continue;
+ if (entry.Value.ValueKind != JsonValueKind.Object) continue;
+ if (!HasTopLevelKey(entry.Value, ForbiddenKey)) continue;
+
+ (offenders ??= []).Add(entry.Name);
+ }
+
+ return (IReadOnlyList?)offenders ?? [];
+ }
+ catch (JsonException)
+ {
+ return [];
+ }
+ }
+
+ /// Whether carries at its top level,
+ /// case-insensitively.
+ private static bool HasTopLevelKey(JsonElement obj, string key)
+ {
+ foreach (var property in obj.EnumerateObject())
+ {
+ if (string.Equals(property.Name, key, StringComparison.OrdinalIgnoreCase)) return true;
+ }
+
+ return false;
+ }
+}
diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Clusters/NodeEdit.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Clusters/NodeEdit.razor
index 2c1fef76..8f663bb5 100644
--- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Clusters/NodeEdit.razor
+++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Clusters/NodeEdit.razor
@@ -9,6 +9,8 @@
@using System.ComponentModel.DataAnnotations
@using ZB.MOM.WW.OtOpcUa.Configuration
@using ZB.MOM.WW.OtOpcUa.Configuration.Entities
+@using ZB.MOM.WW.OtOpcUa.Configuration.Validation
+@using ZB.MOM.WW.OtOpcUa.Core.Abstractions
@inject IDbContextFactory DbFactory
@inject NavigationManager Nav
@inject AuthenticationStateProvider AuthState
@@ -178,6 +180,33 @@ else
}
await using var db = await DbFactory.CreateDbContextAsync();
+
+ // #499 — the third Sql-credential surface. DriverConfigOverridesJson is merged onto the
+ // cluster-level DriverConfig, so a literal connectionString pasted here leaks exactly as one
+ // pasted into the driver itself. The deploy gate cannot see it (DraftSnapshot carries no
+ // ClusterNode rows) and would be the wrong place anyway: node overrides never enter the
+ // artifact, so by the time a deploy ran the credential would already be stored. Refuse the
+ // save instead. The message names the driver instance but NEVER the value.
+ if (!string.IsNullOrWhiteSpace(_form.DriverConfigOverridesJson))
+ {
+ var sqlDriverIds = await db.DriverInstances
+ .Where(d => d.ClusterId == ClusterId && d.DriverType == DriverTypeNames.Sql)
+ .Select(d => d.DriverInstanceId)
+ .ToListAsync();
+
+ var offenders = SqlCredentialGuard.FindNodeOverrideViolations(
+ _form.DriverConfigOverridesJson, sqlDriverIds);
+ if (offenders.Count > 0)
+ {
+ _error =
+ $"Driver config overrides carry a '{SqlCredentialGuard.ForbiddenKey}' for Sql driver " +
+ $"instance(s) {string.Join(", ", offenders)}. A Sql driver must name its credentials " +
+ "indirectly via 'connectionStringRef', which resolves from the environment / secret " +
+ "store at Initialize; a literal connection string here would be stored in the config " +
+ "database, and the runtime ignores it anyway. Remove the key.";
+ return;
+ }
+ }
if (IsNew)
{
if (await db.ClusterNodes.AnyAsync(n => n.NodeId == _form.NodeId))
diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/SqlCredentialGuardTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/SqlCredentialGuardTests.cs
new file mode 100644
index 00000000..6cc1e749
--- /dev/null
+++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/SqlCredentialGuardTests.cs
@@ -0,0 +1,132 @@
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.Configuration.Validation;
+
+namespace ZB.MOM.WW.OtOpcUa.Configuration.Tests;
+
+///
+/// — the shared "a Sql driver never persists a literal connection
+/// string" check behind both the #498 deploy gate and the #499 node-override save gate.
+///
+///
+/// The node-override half is the one #499 is about: ClusterNode.DriverConfigOverridesJson is a
+/// map keyed by DriverInstanceId that is merged onto the cluster-level DriverConfig, and
+/// it is invisible to DraftSnapshot — so nothing gated it at all before.
+///
+[Trait("Category", "Unit")]
+public sealed class SqlCredentialGuardTests
+{
+ /// The literal an operator would paste into a raw-JSON textarea.
+ private const string Leaked =
+ """{"provider":"SqlServer","connectionString":"Server=sql,1433;Database=Mes;User ID=sa;Password=hunter2"}""";
+
+ private const string Clean = """{"provider":"SqlServer","connectionStringRef":"Sql:Line3"}""";
+
+ // ---- CarriesLiteralConnectionString --------------------------------------------------------
+
+ /// The key is caught at the top level of a config blob.
+ [Fact]
+ public void A_literal_connectionString_is_detected()
+ => SqlCredentialGuard.CarriesLiteralConnectionString(Leaked).ShouldBeTrue();
+
+ /// The supported indirect form is not a violation — otherwise the rule would block the fix
+ /// it tells operators to apply.
+ [Fact]
+ public void The_indirect_connectionStringRef_form_is_clean()
+ => SqlCredentialGuard.CarriesLiteralConnectionString(Clean).ShouldBeFalse();
+
+ /// System.Text.Json binds ConnectionString to a connectionString property by
+ /// default, so a case variant is the same key — not a bypass.
+ [Theory]
+ [InlineData("""{"ConnectionString":"Server=x;Password=p"}""")]
+ [InlineData("""{"CONNECTIONSTRING":"Server=x;Password=p"}""")]
+ [InlineData("""{"connectionstring":"Server=x;Password=p"}""")]
+ public void Case_variants_are_the_same_key(string json)
+ => SqlCredentialGuard.CarriesLiteralConnectionString(json).ShouldBeTrue();
+
+ /// Blank, malformed and non-object blobs simply have no keys. Shaping the config JSON is
+ /// another rule's job, and throwing here would turn a formatting mistake into a credential failure.
+ ///
+ [Theory]
+ [InlineData(null)]
+ [InlineData("")]
+ [InlineData(" ")]
+ [InlineData("{ not json")]
+ [InlineData("[1,2,3]")]
+ [InlineData("\"a string\"")]
+ public void Blank_malformed_and_non_object_blobs_are_not_violations(string? json)
+ => SqlCredentialGuard.CarriesLiteralConnectionString(json).ShouldBeFalse();
+
+ /// Only the top level is scanned. The DTO is flat, so a nested occurrence cannot bind and is
+ /// not the credential-shaped mistake this rule exists to catch.
+ [Fact]
+ public void A_nested_occurrence_is_not_flagged()
+ => SqlCredentialGuard
+ .CarriesLiteralConnectionString("""{"nested":{"connectionString":"Server=x"}}""")
+ .ShouldBeFalse();
+
+ // ---- FindNodeOverrideViolations (#499) -----------------------------------------------------
+
+ /// The #499 leak: a credential pasted into a node's per-driver override map.
+ [Fact]
+ public void A_credential_in_a_node_override_for_a_Sql_driver_is_a_violation()
+ {
+ var overrides = $$"""{"di-sql": {{Leaked}} }""";
+
+ SqlCredentialGuard.FindNodeOverrideViolations(overrides, ["di-sql"])
+ .ShouldBe(["di-sql"]);
+ }
+
+ /// Every offending instance is reported, not just the first — an operator fixing one at a
+ /// time would otherwise need as many save attempts as there are leaks.
+ [Fact]
+ public void Every_offending_instance_is_reported()
+ {
+ var overrides = $$"""{"di-a": {{Leaked}}, "di-clean": {{Clean}}, "di-b": {{Leaked}} }""";
+
+ SqlCredentialGuard.FindNodeOverrideViolations(overrides, ["di-a", "di-b", "di-clean"])
+ .ShouldBe(["di-a", "di-b"]);
+ }
+
+ /// Keys outside the Sql set are ignored. A non-Sql driver never made the
+ /// indirect-credential guarantee, so flagging it would break a config that is legitimate today —
+ /// a regression, not defence in depth.
+ [Fact]
+ public void An_override_for_a_non_Sql_driver_is_ignored()
+ {
+ var overrides = $$"""{"di-modbus": {{Leaked}} }""";
+
+ SqlCredentialGuard.FindNodeOverrideViolations(overrides, ["di-sql"]).ShouldBeEmpty();
+ }
+
+ /// The realistic override — a node-specific endpoint — must keep saving.
+ [Fact]
+ public void A_normal_override_is_clean()
+ {
+ var overrides = """{"di-sql": {"commandTimeoutSeconds": 45}}""";
+
+ SqlCredentialGuard.FindNodeOverrideViolations(overrides, ["di-sql"]).ShouldBeEmpty();
+ }
+
+ /// Nothing to check when the cluster has no Sql drivers, or the node has no overrides.
+ [Theory]
+ [InlineData(null)]
+ [InlineData("")]
+ [InlineData(" ")]
+ [InlineData("{ not json")]
+ [InlineData("[1,2,3]")]
+ public void Blank_and_malformed_override_maps_yield_no_violations(string? overrides)
+ => SqlCredentialGuard.FindNodeOverrideViolations(overrides, ["di-sql"]).ShouldBeEmpty();
+
+ /// An empty Sql-driver set short-circuits — there is no instance the key could belong to.
+ [Fact]
+ public void No_Sql_drivers_means_no_violations()
+ => SqlCredentialGuard.FindNodeOverrideViolations($$"""{"di-sql": {{Leaked}} }""", [])
+ .ShouldBeEmpty();
+
+ /// A non-object override entry cannot carry config keys, and must not throw.
+ [Fact]
+ public void A_non_object_override_entry_is_skipped()
+ => SqlCredentialGuard.FindNodeOverrideViolations("""{"di-sql": "connectionString"}""", ["di-sql"])
+ .ShouldBeEmpty();
+}