From 2751a6dba901d57809f6df8e1136301f1b624534 Mon Sep 17 00:00:00 2001
From: Joseph Doherty <dohejw01@gmail.com>
Date: Mon, 13 Jul 2026 11:05:19 -0400
Subject: [PATCH] test(management): freeze scrubber fragment coverage over
 DataConnection config types + document array-merge limit (plan R2-07 T12)

---
 .../ConfigSecretScrubber.cs                   | 11 ++++-
 .../ConfigSecretScrubberTests.cs              | 43 +++++++++++++++++++
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ConfigSecretScrubber.cs b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ConfigSecretScrubber.cs
index 4cc28c94..6ecdbe2d 100644
--- a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ConfigSecretScrubber.cs
+++ b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ConfigSecretScrubber.cs
@@ -18,7 +18,7 @@ internal static class ConfigSecretScrubber
     private static readonly string[] SecretNameFragments =
         ["password", "secret", "token", "apikey", "credential", "passphrase"];
 
-    private static bool IsSecretName(string propertyName)
+    internal static bool IsSecretName(string propertyName)
     {
         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="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>
+    /// <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)
     {
         if (string.IsNullOrEmpty(incoming))
diff --git a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ConfigSecretScrubberTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ConfigSecretScrubberTests.cs
index 75e65e75..e5157a3c 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ConfigSecretScrubberTests.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ConfigSecretScrubberTests.cs
@@ -42,4 +42,47 @@ public class ConfigSecretScrubberTests
         Assert.Contains("\"real\"", 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));
 }