Files
lmxopcua/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/SqlCredentialGuard.cs
T
Joseph Doherty 2193d9f2e4 fix(config): refuse to store a Sql credential in a node config override (#499)
`ClusterNode.DriverConfigOverridesJson` is the third surface a Sql driver's config
is persisted on, and the only one nothing gated. It is a map keyed by
`DriverInstanceId` merged onto the cluster-level `DriverConfig`, so a literal
`connectionString` pasted there leaks exactly as one pasted into the driver — the
leak #498 closed for `DriverInstance.DriverConfig` and `Device.DeviceConfig`.

Gated at the SAVE, not at the deploy — a deliberate departure from both options the
issue offered:

- `DraftSnapshot` carries no `ClusterNode` rows, and adding them would widen the
  snapshot and every builder of one for a single rule about a value that never
  enters the artifact.
- More to the point, a deploy gate is the wrong instrument here. Node overrides are
  not in the artifact, so blocking a deploy would not stop the credential being
  stored — it is already in the database and replicated by then. Refusing the save
  is the only point where "refuse to store it" is literally true, which is #498's
  own framing: discarding a secret on read is not the same as refusing to store it.

The check moves into a shared `SqlCredentialGuard` so the two enforcement points
cannot drift; `DraftValidator` now calls it instead of its own private helper.

Kept deliberately narrow — the `Sql` driver's `connectionString` only, and only for
instance ids that ARE Sql drivers. The issue asked whether to generalise to
credential-shaped keys across every driver type; not doing that, for the same
reason #498 did not: a broader sweep would start refusing configs that are
legitimate today for drivers which never made an indirect-credential guarantee,
which is a regression rather than defence in depth. Widen per driver, as each gains
its own contract.

The error names the offending driver instance(s) and NEVER the value — it reaches
the AdminUI and the audit trail.

14 new cases cover both halves, including the case variants (System.Text.Json binds
`ConnectionString` to `connectionString`, so a case variant is the same key, not a
bypass), non-Sql ids being ignored, and every blank/malformed/non-object shape.
2026-07-26 10:30:04 -04:00

140 lines
6.9 KiB
C#

using System.Text.Json;
namespace ZB.MOM.WW.OtOpcUa.Configuration.Validation;
/// <summary>
/// The one place that decides whether a persisted config blob carries a literal Sql
/// <c>connectionString</c> (Gitea #498 / #499).
/// </summary>
/// <remarks>
/// <para>
/// A <c>Sql</c> driver names its credentials indirectly — <c>connectionStringRef</c> resolves from
/// the environment / secret store at Initialize — so nothing persisted should ever contain a
/// database password. The typed <c>SqlDriverConfigDto</c> already drops a <c>connectionString</c>
/// key on <b>read</b>, 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.
/// </para>
/// <para>
/// There are <b>three</b> surfaces a Sql driver's config is persisted on, and they need different
/// enforcement points:
/// <list type="bullet">
/// <item><c>DriverInstance.DriverConfig</c> and <c>Device.DeviceConfig</c> — both in the
/// deployed artifact, both visible to <c>DraftSnapshot</c>, both gated by
/// <c>DraftValidator</c>.</item>
/// <item><c>ClusterNode.DriverConfigOverridesJson</c> — the per-node override map. It is
/// <b>not</b> in <c>DraftSnapshot</c>, and deliberately does not go there: adding a
/// <c>ClusterNode</c> 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 <b>save</b>, which is the only
/// point where "refuse to store it" is literally true.</item>
/// </list>
/// </para>
/// <para>
/// <b>Deliberately narrow.</b> This checks the <c>Sql</c> driver's <c>connectionString</c> 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.
/// </para>
/// </remarks>
public static class SqlCredentialGuard
{
/// <summary>The config key a Sql driver must never persist. Matched case-insensitively.</summary>
public const string ForbiddenKey = "connectionString";
/// <summary>
/// Whether <paramref name="configJson"/> is a JSON object carrying <see cref="ForbiddenKey"/> at
/// its top level.
/// </summary>
/// <remarks>
/// Matched <b>case-insensitively</b>: <c>System.Text.Json</c> binds <c>ConnectionString</c> to a
/// <c>connectionString</c> 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.
/// </remarks>
/// <param name="configJson">The config blob to inspect.</param>
/// <returns><see langword="true"/> when the key is present at the top level.</returns>
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;
}
}
/// <summary>
/// The <c>DriverInstanceId</c>s in a <c>ClusterNode.DriverConfigOverridesJson</c> map whose
/// override object carries a literal connection string, restricted to instances that are actually
/// Sql drivers.
/// </summary>
/// <remarks>
/// The override map is shaped <c>{ "&lt;DriverInstanceId&gt;": { …driver config keys… } }</c> and is
/// merged onto the cluster-level <c>DriverConfig</c>, so a credential pasted into one lands in
/// exactly the place <see cref="CarriesLiteralConnectionString"/> already refuses on the driver
/// itself.
/// </remarks>
/// <param name="overridesJson">The node's override map. Null / blank / malformed yields no
/// violations.</param>
/// <param name="sqlDriverInstanceIds">The ids of driver instances whose <c>DriverType</c> is
/// <c>Sql</c>. 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.</param>
/// <returns>The offending driver-instance ids, in document order; empty when there are none.</returns>
public static IReadOnlyList<string> FindNodeOverrideViolations(
string? overridesJson, IReadOnlyCollection<string> sqlDriverInstanceIds)
{
if (string.IsNullOrWhiteSpace(overridesJson) || sqlDriverInstanceIds.Count == 0)
{
return [];
}
var sqlIds = sqlDriverInstanceIds as IReadOnlySet<string>
?? sqlDriverInstanceIds.ToHashSet(StringComparer.Ordinal);
try
{
using var doc = JsonDocument.Parse(overridesJson);
if (doc.RootElement.ValueKind != JsonValueKind.Object) return [];
List<string>? 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<string>?)offenders ?? [];
}
catch (JsonException)
{
return [];
}
}
/// <summary>Whether <paramref name="obj"/> carries <paramref name="key"/> at its top level,
/// case-insensitively.</summary>
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;
}
}