From 09cd5792203155cb45aee1b82d4691bd7c780449 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 22 May 2026 08:23:45 -0400 Subject: [PATCH] fix(core): resolve Medium code-review finding (Core-003) Add FolderSegment member to NodeAclScopeKind; update WalkSystemPlatform to report NodeAclScopeKind.FolderSegment (not Equipment) for each visited Galaxy folder level, so MatchedGrant.Scope in AuthorizationDecision.Provenance correctly distinguishes Galaxy folder grants from UNS Equipment grants in the audit trail and Admin UI diagnostics. Three regression tests added to PermissionTrieTests. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Enums/NodeAclScopeKind.cs | 6 +++ .../Authorization/PermissionTrie.cs | 11 ++--- .../Authorization/PermissionTrieTests.cs | 49 +++++++++++++++++++ 3 files changed, 60 insertions(+), 6 deletions(-) diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Enums/NodeAclScopeKind.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Enums/NodeAclScopeKind.cs index b6ad45c..c30262c 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Enums/NodeAclScopeKind.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Enums/NodeAclScopeKind.cs @@ -8,5 +8,11 @@ public enum NodeAclScopeKind UnsArea, UnsLine, Equipment, + /// + /// A Galaxy (SystemPlatform-kind) folder segment anchored below a namespace. + /// Distinguishes folder grants from UNS grants in the + /// AuthorizationDecision.Provenance audit trail and Admin UI diagnostics. + /// + FolderSegment, Tag, } diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/PermissionTrie.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/PermissionTrie.cs index 225dc38..7de8644 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/PermissionTrie.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/PermissionTrie.cs @@ -79,16 +79,15 @@ public sealed class PermissionTrie private static void WalkSystemPlatform(PermissionTrieNode ns, NodeScope scope, HashSet groups, List matches) { - // FolderSegments are nested under the namespace; each is its own trie level. Reuse the - // UnsArea scope kind for the flags — NodeAcl rows for Galaxy tags carry ScopeKind.Tag - // for leaf grants and ScopeKind.Namespace for folder-root grants; deeper folder grants - // are modeled as Equipment-level rows today since NodeAclScopeKind doesn't enumerate - // a dedicated FolderSegment kind. Future-proof TODO tracked in Stream B follow-up. + // FolderSegments are nested under the namespace; each is its own trie level. Use the + // dedicated FolderSegment scope kind so Galaxy folder grants report their true scope in + // AuthorizationDecision.Provenance — distinguishing them from UNS Equipment grants in + // the audit trail and Admin UI "Probe this permission" diagnostic. var current = ns; foreach (var segment in scope.FolderSegments) { if (!current.Children.TryGetValue(segment, out var child)) return; - CollectAtLevel(child, NodeAclScopeKind.Equipment, groups, matches); + CollectAtLevel(child, NodeAclScopeKind.FolderSegment, groups, matches); current = child; } diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/PermissionTrieTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/PermissionTrieTests.cs index 574d0b5..d11e116 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/PermissionTrieTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/PermissionTrieTests.cs @@ -125,6 +125,55 @@ public sealed class PermissionTrieTests matchB.ShouldBeEmpty(); } + /// + /// Core-003 regression: grants matched during the SystemPlatform (Galaxy) folder walk must + /// report in , + /// not . This distinguishes Galaxy folder grants + /// from UNS Equipment grants in the audit trail and Admin UI "Probe this permission" panel. + /// + [Fact] + public void Galaxy_FolderSegment_Grant_Reports_FolderSegment_Scope_Not_Equipment() + { + var paths = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + ["section-X"] = new(new[] { "ns-gal", "section-X" }), + }; + var rows = new[] { Row("cn=ops", NodeAclScopeKind.Equipment, "section-X", NodePermissions.Read) }; + var trie = PermissionTrieBuilder.Build("c1", 1, rows, paths); + + var matches = trie.CollectMatches(GalaxyTag("c1", "ns-gal", ["section-X"], "tag1"), ["cn=ops"]); + + matches.Count.ShouldBe(1); + matches[0].Scope.ShouldBe(NodeAclScopeKind.FolderSegment, + "the trie walk reports the structural level where the grant was found — FolderSegment for Galaxy, not Equipment"); + } + + [Fact] + public void Galaxy_DeepFolderPath_AllSegments_Report_FolderSegment_Scope() + { + // A three-level folder path — each visited level that carries a grant must report FolderSegment. + var paths = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + ["area"] = new(new[] { "ns-gal", "area" }), + ["area/line"] = new(new[] { "ns-gal", "area", "line" }), + ["area/line/cell"] = new(new[] { "ns-gal", "area", "line", "cell" }), + }; + var rows = new[] + { + Row("cn=ops", NodeAclScopeKind.Equipment, "area", NodePermissions.Read), + Row("cn=ops", NodeAclScopeKind.Equipment, "area/line", NodePermissions.Read), + Row("cn=ops", NodeAclScopeKind.Equipment, "area/line/cell", NodePermissions.Read), + }; + var trie = PermissionTrieBuilder.Build("c1", 1, rows, paths); + + var matches = trie.CollectMatches( + GalaxyTag("c1", "ns-gal", ["area", "line", "cell"], "tag1"), ["cn=ops"]); + + matches.Count.ShouldBe(3, "one match per folder level visited"); + matches.ShouldAllBe(m => m.Scope == NodeAclScopeKind.FolderSegment, + "every matched folder level must report FolderSegment, never Equipment"); + } + [Fact] public void CrossCluster_Grant_DoesNotLeak() {