test(management): freeze scrubber fragment coverage over DataConnection config types + document array-merge limit (plan R2-07 T12)

This commit is contained in:
Joseph Doherty
2026-07-13 11:05:19 -04:00
parent 1b88ca1296
commit 2751a6dba9
2 changed files with 53 additions and 1 deletions
@@ -18,7 +18,7 @@ internal static class ConfigSecretScrubber
private static readonly string[] SecretNameFragments = private static readonly string[] SecretNameFragments =
["password", "secret", "token", "apikey", "credential", "passphrase"]; ["password", "secret", "token", "apikey", "credential", "passphrase"];
private static bool IsSecretName(string propertyName) internal static bool IsSecretName(string propertyName)
{ {
foreach (var fragment in SecretNameFragments) foreach (var fragment in SecretNameFragments)
{ {
@@ -127,6 +127,15 @@ internal static class ConfigSecretScrubber
/// <param name="incoming">The client-submitted config JSON, possibly containing sentinel values.</param> /// <param name="incoming">The client-submitted config JSON, possibly containing sentinel values.</param>
/// <param name="stored">The previously stored config JSON to source real secret values from.</param> /// <param name="stored">The previously stored config JSON to source real secret values from.</param>
/// <returns>The incoming JSON with sentinel values replaced by the corresponding stored values.</returns> /// <returns>The incoming JSON with sentinel values replaced by the corresponding stored values.</returns>
/// <remarks>
/// <b>Array limitation (arch-review R2 N9):</b> sentinel restoration inside arrays
/// pairs incoming[i] with stored[i] BY INDEX. A client that reorders an array of
/// credentialed objects while round-tripping sentinels grafts the wrong stored
/// secret into each slot — a silent misconfiguration (never a disclosure). Clients
/// must not reorder arrays that carry sentinels (the CLI round-trip does not);
/// key-based matching is a possible future refinement if a config type ever nests
/// credentialed arrays.
/// </remarks>
public static string? MergeSentinels(string? incoming, string? stored) public static string? MergeSentinels(string? incoming, string? stored)
{ {
if (string.IsNullOrEmpty(incoming)) if (string.IsNullOrEmpty(incoming))
@@ -42,4 +42,47 @@ public class ConfigSecretScrubberTests
Assert.Contains("\"real\"", merged); Assert.Contains("\"real\"", merged);
Assert.DoesNotContain(ConfigSecretScrubber.Sentinel, merged); Assert.DoesNotContain(ConfigSecretScrubber.Sentinel, merged);
} }
// ── arch-review R2 N9: frozen scrubber fragment coverage ───────────────────
// Frozen non-secret allowlist (arch-review R2 N9), mirroring RequiredRoleMatrixTests:
// every string-typed property on the DataConnections config types must EITHER match
// the scrubber's secret fragments (it gets elided) OR be listed here after a
// deliberate "this is not a secret" decision. A new string property with neither
// fails EveryConfigStringProperty_IsScrubbedOrDeliberatelyNonSecret, so a future
// "SigningKey"/"Pin" cannot silently leak through List/Get/audit.
private static readonly Dictionary<Type, string[]> KnownNonSecretStringProperties = new()
{
[typeof(ZB.MOM.WW.ScadaBridge.Commons.Types.DataConnections.OpcUaEndpointConfig)] = ["EndpointUrl", "SubscriptionDisplayName"],
[typeof(ZB.MOM.WW.ScadaBridge.Commons.Types.DataConnections.OpcUaUserIdentityConfig)] = ["Username", "CertificatePath"],
[typeof(ZB.MOM.WW.ScadaBridge.Commons.Types.DataConnections.MxGatewayEndpointConfig)] = ["Endpoint", "ClientName", "CaFile", "ServerName"],
[typeof(ZB.MOM.WW.ScadaBridge.Commons.Types.DataConnections.OpcUaHeartbeatConfig)] = ["TagPath"],
[typeof(ZB.MOM.WW.ScadaBridge.Commons.Types.DataConnections.OpcUaDeadbandConfig)] = [],
};
[Fact]
public void EveryConfigStringProperty_IsScrubbedOrDeliberatelyNonSecret()
{
foreach (var (type, allowlist) in KnownNonSecretStringProperties)
{
var stringProps = type.GetProperties(System.Reflection.BindingFlags.Public | System.Reflection.BindingFlags.Instance)
.Where(p => p.PropertyType == typeof(string))
.Select(p => p.Name);
foreach (var name in stringProps)
{
var scrubbed = ConfigSecretScrubber.IsSecretName(name);
var allowlisted = allowlist.Contains(name);
Assert.True(scrubbed ^ allowlisted,
$"{type.Name}.{name}: must be EITHER fragment-scrubbed OR explicitly " +
"allowlisted as non-secret (and never both) — see arch-review R2 N9.");
}
}
}
[Theory]
[InlineData("Password")]
[InlineData("CertificatePassword")]
[InlineData("ApiKey")]
public void KnownSecretProperties_MatchTheFragmentList(string name)
=> Assert.True(ConfigSecretScrubber.IsSecretName(name));
} }