test(cluster): guard that role reads live only in the authority (archreview theme 4)

archreview/plans/R3-04-one-role-authority-plan.md.tasks.json

@@ -59,7 +59,7 @@
     {
       "id": "T8",
       "subject": "RoleAuthorityGuardTests: source scan proving no production file outside the authority reads OTOPCUA_ROLES, Cluster:Roles, or a bare role literal",
-      "status": "pending",
+      "status": "completed",
       "blockedBy": [
         "T3",
         "T4",

src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ServiceCollectionExtensions.cs

@@ -21,7 +21,9 @@ namespace ZB.MOM.WW.OtOpcUa.ControlPlane;
 
 public static class ServiceCollectionExtensions
 {
-    public const string AdminRole = "admin";
+    // R3-04 (theme 4 guard): alias RoleParser.Admin rather than duplicating the literal — the same
+    // pattern Runtime/ServiceCollectionExtensions.DriverRole already follows for RoleParser.Driver.
+    public const string AdminRole = RoleParser.Admin;
 
     public const string ConfigPublishSingletonName = "config-publish";
     public const string AdminOperationsSingletonName = "admin-operations";

src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs

@@ -1,5 +1,6 @@
 using Akka.Actor;
 using Akka.Event;
+using ZB.MOM.WW.OtOpcUa.Cluster;
 using ZB.MOM.WW.OtOpcUa.Commons.Observability;
 using ZB.MOM.WW.OtOpcUa.Commons.OpcUa;
 using ZB.MOM.WW.OtOpcUa.Commons.Types;
@@ -310,7 +311,9 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers
     {
         var isWindowsOnly = driverType is "Galaxy";
         if (!OperatingSystem.IsWindows() && isWindowsOnly) return true;
-        if (roles.Contains("dev") && isWindowsOnly) return true;
+        // R3-04 (theme 4 guard): GossipedRolesHave, not a bare "dev" literal Contains — the same
+        // case-insensitivity the rest of the role authority gives every other consumer.
+        if (RoleParser.GossipedRolesHave(roles, RoleParser.Dev) && isWindowsOnly) return true;
         return false;
     }
 

tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/RoleAuthorityGuardTests.cs

@@ -0,0 +1,250 @@
+using System.Text.RegularExpressions;
+using Shouldly;
+using Xunit;
+
+namespace ZB.MOM.WW.OtOpcUa.Cluster.Tests;
+
+/// <summary>
+///     R3-04 (theme 4, "one rule stated twice diverges"): the role-reading rule may live in exactly
+///     one place. Three source-scan rules, modelled on
+///     <c>CommandDisposalConventionsTests</c>'s root-walk idiom, over every <c>src/**/*.cs</c> +
+///     <c>src/**/*.razor</c> file:
+///     <list type="number">
+///         <item><description>No file except <c>EffectiveRoles.cs</c>/<c>RoleParser.cs</c> reads
+///         <c>OTOPCUA_ROLES</c>.</description></item>
+///         <item><description>No file except <c>EffectiveRoles.cs</c> reads the <c>Cluster:Roles</c>
+///         key as code (an operator-facing message string that merely NAMES the key is fine — it
+///         reads as a sentence, i.e. its string literal also contains a space).</description></item>
+///         <item><description>No file except <c>RoleParser.cs</c> declares a bare
+///         <c>"admin"</c>/<c>"driver"</c>/<c>"dev"</c> role literal.</description></item>
+///     </list>
+///     A textual scan, not reflection, because the thing being guarded is a textual read
+///     (<c>Environment.GetEnvironmentVariable("OTOPCUA_ROLES")</c>, <c>GetSection("Cluster:Roles")</c>,
+///     a bare <c>"driver"</c>) that reflection cannot see.
+/// </summary>
+/// <remarks>
+///     <b>Falsification control:</b> <see cref="The_scan_actually_detects_the_pre_R3_04_offenders"/>
+///     feeds the scan the EXACT pre-fix <c>Program.cs</c>/<c>Fleet.razor</c> snippets (captured
+///     verbatim before T3/T5 touched them) and asserts it finds them — proof this guard can fail, not
+///     just that the current tree happens to pass. Run against the real pre-T3 tree at commit
+///     <c>7dfd6dcc</c> (Program.cs) / pre-T5 at <c>bcec88ef</c> (Fleet.razor), the live scan reported:
+///     <c>Host/Program.cs:61</c>, <c>:62</c>, <c>:63</c> (OTOPCUA_ROLES + two bare role literals) and
+///     <c>AdminUI/Components/Pages/Fleet.razor:149</c>, <c>:159</c> (two bare <c>"driver"</c> literals)
+///     — recorded here per the plan's "paste the list into the commit body" instruction.
+/// </remarks>
+public sealed class RoleAuthorityGuardTests
+{
+    private static readonly string[] OtopcuaRolesAllowedFiles = ["EffectiveRoles.cs", "RoleParser.cs"];
+    private static readonly string[] ClusterRolesKeyAllowedFiles = ["EffectiveRoles.cs"];
+    private static readonly string[] RoleLiteralAllowedFiles = ["RoleParser.cs"];
+
+    /// <summary>
+    ///     Files whose bare <c>"admin"</c>/<c>"driver"</c>/<c>"dev"</c> literal names something OTHER
+    ///     than an Akka <c>Cluster:Roles</c> role — each entry carries the reason, per the plan's
+    ///     "add the file to the small, named allow-list WITH A REASON" instruction.
+    /// </summary>
+    private static readonly IReadOnlyDictionary<string, string> RoleLiteralAllowListWithReasons =
+        new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
+        {
+            ["RawTreeService.cs"] =
+                "raw-tree node-KIND discriminator (Folder/Driver/Device/TagGroup/Tag), not an Akka role",
+            ["AclScopeService.cs"] =
+                "human-readable entity-type name passed to ResolveAsync for an error message, not a role",
+            ["SqlTagDefinitionFactory.cs"] =
+                "the Sql driver's own TagConfig JSON blob field ({\"driver\":\"Sql\",...}), not a role",
+            ["SqlEquipmentTagParser.cs"] =
+                "the Sql driver's own TagConfig JSON blob field ({\"driver\":\"Sql\",...}), not a role",
+            ["SqlAddressPickerBody.razor"] =
+                "the Sql driver's own TagConfig JSON blob field ({\"driver\":\"Sql\",...}), not a role",
+            ["OtOpcUaLdapAuthService.cs"] =
+                "an LDAP-derived ADMINUI RBAC role (the DevStubMode bypass group), not Cluster:Roles",
+        };
+
+    private static string SolutionRoot()
+    {
+        var dir = new DirectoryInfo(AppContext.BaseDirectory);
+        while (dir is not null && !File.Exists(Path.Combine(dir.FullName, "ZB.MOM.WW.OtOpcUa.slnx")))
+            dir = dir.Parent;
+        dir.ShouldNotBeNull("Could not find solution root (ZB.MOM.WW.OtOpcUa.slnx).");
+        return dir!.FullName;
+    }
+
+    private static IEnumerable<string> SourceFiles()
+    {
+        var srcDir = Path.Combine(SolutionRoot(), "src");
+        var sep = Path.DirectorySeparatorChar;
+        return Directory.EnumerateFiles(srcDir, "*.cs", SearchOption.AllDirectories)
+            .Concat(Directory.EnumerateFiles(srcDir, "*.razor", SearchOption.AllDirectories))
+            .Where(f => !f.Contains($"{sep}obj{sep}", StringComparison.Ordinal)
+                     && !f.Contains($"{sep}bin{sep}", StringComparison.Ordinal));
+    }
+
+    /// <summary>Every line of <paramref name="text"/> that is not a full-line <c>//</c>/<c>///</c>
+    /// comment, 1-based line numbers.</summary>
+    private static IEnumerable<(int Line, string Text)> NonCommentLines(string text)
+    {
+        var lines = text.Split('\n');
+        for (var i = 0; i < lines.Length; i++)
+        {
+            if (lines[i].TrimStart().StartsWith("//", StringComparison.Ordinal)) continue;
+            yield return (i + 1, lines[i]);
+        }
+    }
+
+    private static readonly Regex BareRoleLiteral = new("\"(admin|driver|dev)\"", RegexOptions.Compiled);
+
+    /// <summary>True when the occurrence of a <paramref name="matchLength"/>-character key at
+    /// <paramref name="matchIndex"/> sits inside a string literal that ALSO contains a space — the
+    /// shape an operator-facing message reads as (naming the key for guidance is not a CODE READ of
+    /// it), per the plan's carve-out for <c>Cluster:Roles</c> — applied identically to
+    /// <c>OTOPCUA_ROLES</c> for the same reason (<c>GrpcApplyAckSender</c>'s warning names BOTH keys
+    /// on one line; treating the two inconsistently would flag one and not the other on that exact
+    /// sentence).</summary>
+    private static bool IsAllowedMessageOccurrence(string line, int matchIndex, int matchLength)
+    {
+        var openQuote = line.LastIndexOf('"', matchIndex);
+        if (openQuote < 0) return false; // not inside any string literal at all -> a bare code read
+
+        var searchFrom = matchIndex + matchLength;
+        if (searchFrom >= line.Length) return false;
+        var closeQuote = line.IndexOf('"', searchFrom);
+        if (closeQuote < 0) return false;
+
+        return line[openQuote..(closeQuote + 1)].Contains(' ');
+    }
+
+    private static List<string> FindOtopcuaRolesOffenders(string text) =>
+        NonCommentLines(text)
+            .Where(l =>
+            {
+                var idx = l.Text.IndexOf("OTOPCUA_ROLES", StringComparison.Ordinal);
+                return idx >= 0 && !IsAllowedMessageOccurrence(l.Text, idx, "OTOPCUA_ROLES".Length);
+            })
+            .Select(l => l.Line.ToString())
+            .ToList();
+
+    private static List<string> FindClusterRolesKeyOffenders(string text) =>
+        NonCommentLines(text)
+            .Where(l =>
+            {
+                var idx = l.Text.IndexOf("Cluster:Roles", StringComparison.Ordinal);
+                return idx >= 0 && !IsAllowedMessageOccurrence(l.Text, idx, "Cluster:Roles".Length);
+            })
+            .Select(l => l.Line.ToString())
+            .ToList();
+
+    private static List<string> FindBareRoleLiteralOffenders(string text) =>
+        NonCommentLines(text)
+            .Where(l => BareRoleLiteral.IsMatch(l.Text))
+            .Select(l => l.Line.ToString())
+            .ToList();
+
+    [Fact]
+    public void No_file_outside_the_authority_reads_OTOPCUA_ROLES()
+    {
+        var offenders = new List<string>();
+        foreach (var file in SourceFiles())
+        {
+            if (OtopcuaRolesAllowedFiles.Contains(Path.GetFileName(file))) continue;
+
+            foreach (var line in FindOtopcuaRolesOffenders(File.ReadAllText(file)))
+                offenders.Add($"{RelativePath(file)}:{line}");
+        }
+
+        offenders.ShouldBeEmpty(
+            "OTOPCUA_ROLES must be read only by EffectiveRoles.cs/RoleParser.cs. Offenders:\n"
+            + string.Join("\n", offenders));
+    }
+
+    [Fact]
+    public void No_file_outside_EffectiveRoles_reads_the_ClusterRoles_key_as_code()
+    {
+        var offenders = new List<string>();
+        foreach (var file in SourceFiles())
+        {
+            if (ClusterRolesKeyAllowedFiles.Contains(Path.GetFileName(file))) continue;
+
+            foreach (var line in FindClusterRolesKeyOffenders(File.ReadAllText(file)))
+                offenders.Add($"{RelativePath(file)}:{line}");
+        }
+
+        offenders.ShouldBeEmpty(
+            "Cluster:Roles must be read as code only by EffectiveRoles.cs (naming the key inside an "
+            + "operator-facing message string is fine). Offenders:\n" + string.Join("\n", offenders));
+    }
+
+    [Fact]
+    public void No_file_outside_RoleParser_declares_a_bare_role_literal()
+    {
+        var offenders = new List<string>();
+        foreach (var file in SourceFiles())
+        {
+            var fileName = Path.GetFileName(file);
+            if (RoleLiteralAllowedFiles.Contains(fileName)) continue;
+            if (RoleLiteralAllowListWithReasons.ContainsKey(fileName)) continue;
+
+            foreach (var line in FindBareRoleLiteralOffenders(File.ReadAllText(file)))
+                offenders.Add($"{RelativePath(file)}:{line}");
+        }
+
+        offenders.ShouldBeEmpty(
+            "A bare \"admin\"/\"driver\"/\"dev\" role literal must live only in RoleParser.cs (use "
+            + "RoleParser.Admin/.Driver/.Dev, or add the file to RoleLiteralAllowListWithReasons WITH "
+            + "A REASON if it genuinely names something else). Offenders:\n" + string.Join("\n", offenders));
+    }
+
+    /// <summary>The allow-list itself must not silently grow stale — every reason must be non-blank,
+    /// and the file it names must actually exist and actually contain the literal it is excused for
+    /// (an allow-list entry that no longer applies is dead and hides nothing).</summary>
+    [Fact]
+    public void Every_allow_list_entry_has_a_reason_and_still_applies()
+    {
+        foreach (var (fileName, reason) in RoleLiteralAllowListWithReasons)
+        {
+            reason.ShouldNotBeNullOrWhiteSpace($"{fileName}'s allow-list entry has no reason.");
+
+            var matches = SourceFiles().Where(f => Path.GetFileName(f) == fileName).ToList();
+            matches.ShouldNotBeEmpty($"{fileName} is allow-listed but no such file exists under src/.");
+
+            var stillOffends = matches.Any(f => FindBareRoleLiteralOffenders(File.ReadAllText(f)).Count > 0);
+            stillOffends.ShouldBeTrue(
+                $"{fileName} is allow-listed for a bare role literal but no longer contains one — "
+                + "remove the stale entry.");
+        }
+    }
+
+    /// <summary>
+    ///     Falsification control (per the plan: "Run on the pre-T3 tree → FAIL... paste that list
+    ///     into the commit body"). These are the EXACT pre-fix lines — copied verbatim, not
+    ///     paraphrased — from <c>Host/Program.cs</c> (before T3, commit <c>7dfd6dcc</c>) and
+    ///     <c>AdminUI/Components/Pages/Fleet.razor</c> (before T5, commit <c>bcec88ef</c>). If this
+    ///     test ever goes green for the wrong reason (the scan silently stops matching), it is no
+    ///     longer proof the guard can fail.
+    /// </summary>
+    [Fact]
+    public void The_scan_actually_detects_the_pre_R3_04_offenders()
+    {
+        const string preT3ProgramCs = """
+            // Roles drive the entire conditional wiring below — see ZB.MOM.WW.OtOpcUa.Cluster.RoleParser.
+            var roles = RoleParser.Parse(Environment.GetEnvironmentVariable("OTOPCUA_ROLES"));
+            var hasAdmin = roles.Contains("admin");
+            var hasDriver = roles.Contains("driver");
+            """;
+
+        const string preT5FleetRazor = """
+            var akkaDrivers = Cluster.MembersWithRole("driver")
+                .Select(n => n.Value).ToHashSet(StringComparer.OrdinalIgnoreCase);
+            """;
+
+        FindOtopcuaRolesOffenders(preT3ProgramCs).ShouldNotBeEmpty(
+            "the falsification control failed to reproduce the pre-T3 OTOPCUA_ROLES violation");
+        FindBareRoleLiteralOffenders(preT3ProgramCs).Count.ShouldBeGreaterThanOrEqualTo(2,
+            "the falsification control failed to reproduce BOTH pre-T3 bare-literal violations "
+            + "('admin' and 'driver')");
+        FindBareRoleLiteralOffenders(preT5FleetRazor).ShouldNotBeEmpty(
+            "the falsification control failed to reproduce the pre-T5 Fleet.razor bare-literal violation");
+    }
+
+    private static string RelativePath(string fullPath) =>
+        Path.GetRelativePath(SolutionRoot(), fullPath).Replace('\\', '/');
+}