From 489664d105f5bbb25be15953f410883e2c518a12 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 11 Nov 2026 02:25:24 -0500 Subject: [PATCH] fix(acl): folder event verdict invalidation reaches orphaned items, decides on the live instance, skips ACL-off; alarm-bearing reference map (#826 review) 1. Orphaned subscriptions (session closed without deleting subs) have Session == null, so the public MonitoredNode2.InvalidatePermissionCacheForSession never reached their items: a cached allow survived a deploy, the moved alarm's events were queued and delivered on TransferSubscriptions. Each notifier's cache is now cleared whole through the SDK's private MonitoredNode2.InvalidatePermissionCaches (reflection, guard-tested; fallback = per-session drop + Remove/Add of session-less items). 2. The verdict is decided on the condition LIVE under the event's NodeId, not the event's own instance: the SDK key has no instance identity, so a replaced instance's late event cached a deny its successor inherited until the next invalidation. 3. Invalidation is skipped while enforcement is off (forced across an on/off authorizer transition) and is one O(cache) clear per node instead of a whole-cache scan per item. 4. TagReferencingEquipment carries alarm-bearing raw tags only (TagConfigIntent.Alarm); the snapshot adopts an OrdinalIgnoreCase set instead of copying it again. Tests (each fails with its fix reverted): FolderEventVerdictInvalidationTests (orphan + deploy + transfer; replaced instance's late event; ACL-off skip; SDK private-method guard), DeploymentArtifactScopePathTests (alarm-bearing projection). --- .claude/rules/data-plane-auth.md | 13 +- docs/security.md | 7 +- .../Authorization/DataPlaneAuthority.cs | 13 +- .../OtOpcUaNodeManager.cs | 88 ++++++-- .../Drivers/DeploymentArtifact.cs | 22 +- .../FolderEventVerdictInvalidationTests.cs | 188 ++++++++++++++++++ .../DeploymentArtifactScopePathTests.cs | 61 ++++-- 7 files changed, 355 insertions(+), 37 deletions(-) create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/FolderEventVerdictInvalidationTests.cs diff --git a/.claude/rules/data-plane-auth.md b/.claude/rules/data-plane-auth.md index 221add473..ae4301f88 100644 --- a/.claude/rules/data-plane-auth.md +++ b/.claude/rules/data-plane-auth.md @@ -189,8 +189,17 @@ Two things to know before relying on any of this: SourceNode)`** until the monitored node's permission generation moves — `InvalidateFolderEventVerdicts` (one `MonitoredNode2.InvalidatePermissionCacheForSession` per notifier×session) must run whenever the inputs change: the authority/authorizer setters, `RevalidateMonitoredItems` (deploy `Changed` + timer tick) and - `CompleteAddressSpaceApply`. Without it a cached allow survives the deploy (caught by the native wire test's - restore leg / `t2`). A rebuild tombstones the monitored nodes, so their caches go with them. ⚠️ A hand-built + `CompleteAddressSpaceApply` — and is SKIPPED while enforcement is off (the authorizer setter forces it across an + on/off transition). Without it a cached allow survives the deploy (caught by the native wire test's restore leg / + `t2`). A rebuild tombstones the monitored nodes, so their caches go with them. ⚠️ (review) It clears each node's + cache WHOLE through the SDK's private `MonitoredNode2.InvalidatePermissionCaches` (reflection, pinned by + `FolderEventVerdictInvalidationTests.The_sdks_node_wide…`): the public `InvalidatePermissionCacheForSession` + misses ORPHANED subscriptions (session closed without deleting subs ⇒ `Session` null), which then queued a moved + alarm's events and delivered them on `TransferSubscriptions`, and rescanned the cache per item. Fallback if the SDK + drops it: per-session drop + `Remove`/`Add` of each session-less item. ⚠️ The verdict is decided on the LIVE + instance holding the event's condition NodeId, never the event's own instance — the SDK key has no instance + identity, so denying a replaced instance's late event cached a deny the new instance inherited. + `TagReferencingEquipment` holds ALARM-BEARING tags only (`TagConfigIntent.Alarm`). ⚠️ A hand-built authority (rigs) must pass `tagReferencingEquipment` or native alarms stop reaching equipment-folder items under enforcement (`ServerEventRig.TagReferences()`); null ⇒ none (fail closed). Tests: `UnsFolderScopeWireTests.A_moved_alarm_stops_reaching_its_old_folders_subscribers…`, diff --git a/docs/security.md b/docs/security.md index 00b3c2b77..30fe71689 100644 --- a/docs/security.md +++ b/docs/security.md @@ -729,8 +729,11 @@ Behaviours worth knowing before enabling it: dropped no longer admits the native alarm there either. **`ConditionRefresh`** on a folder item is filtered the same way. The hook is `INodeManager3.ValidateEventRolePermissions`, which the node manager re-implements (the base method is not virtual); the SDK memoizes that verdict per `(item, event type, SourceNode)`, so the node - manager invalidates it on every new generation or authorizer, on each `AclRevalidationTimer` tick, and at the - end of every apply. Withheld events are not replayed and the client is not told; restoring the reference or + manager clears each notifier's cache whole (sessions and orphaned, session-less subscriptions alike — an orphaned + subscription must not queue a moved alarm's events for a later `TransferSubscriptions`) on every new generation or + authorizer, on each `AclRevalidationTimer` tick, and at the end of every apply, and not at all while enforcement is + off. A verdict is decided on the condition currently live under the event's NodeId, so a replaced instance's late + event cannot leave a deny behind for its successor. Only alarm-bearing raw tags are carried in the reference map. Withheld events are not replayed and the client is not told; restoring the reference or grant resumes the feed on the next event. ⚠️ A native alarm reaches an equipment folder's items under enforcement only while the installed generation carries that reference — a hand-built authority (tests) must supply `tagReferencingEquipment`. Inert with the flag off. diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/DataPlaneAuthority.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/DataPlaneAuthority.cs index 57a7f20fc..dc4a03bd2 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/DataPlaneAuthority.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/DataPlaneAuthority.cs @@ -163,7 +163,7 @@ public sealed class DataPlaneAuthoritySnapshot public IReadOnlyDictionary UnsFolderScopes { get; } /// - /// RawPath → the EquipmentIds whose UnsTagReference projects that raw tag, as THIS generation deploys + /// RawPath → the EquipmentIds whose UnsTagReference projects that ALARM-BEARING raw tag, as THIS generation deploys /// them (Gitea #826). Keyed Ordinal (a RawPath is a NodeId identifier); the id sets are /// OrdinalIgnoreCase (identifier rule). /// @@ -395,9 +395,14 @@ public sealed class DataPlaneAuthoritySnapshot foreach (var (rawPath, equipmentIds) in tagReferencingEquipment) { if (string.IsNullOrEmpty(rawPath) || equipmentIds is null) continue; - // Re-keyed under the identifier comparer, as the maps above are. - referencing[rawPath] = new HashSet( - equipmentIds.Where(id => !string.IsNullOrWhiteSpace(id)), StringComparer.OrdinalIgnoreCase); + // Re-keyed under the identifier comparer, as the maps above are — but a set already built under it + // (what DeploymentArtifact produces) is adopted rather than copied a second time. + referencing[rawPath] = equipmentIds is HashSet set + && ReferenceEquals(set.Comparer, StringComparer.OrdinalIgnoreCase) + && !set.Any(string.IsNullOrWhiteSpace) + ? set + : new HashSet( + equipmentIds.Where(id => !string.IsNullOrWhiteSpace(id)), StringComparer.OrdinalIgnoreCase); } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index 525d06976..62489d97c 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -654,8 +654,10 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3 // there is no second wiring that could silently not run (an unattached resolver splits opaque ids, // which only ever DENIES narrow grants, so a miss would read as working enforcement). if (value is not null) value.UnsSubjectResolver = ResolveUnsAclSubject; + var wasEnforced = AclEnforced; _authorizer = value; - InvalidateFolderEventVerdicts(); + // Across an on/off transition the cached verdicts were decided under the other mode. + InvalidateFolderEventVerdicts(force: wasEnforced || AclEnforced); } } @@ -5557,8 +5559,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3 /// when the item's session may receive it through that notifier. /// /// - /// Three conditions, all against the CURRENT state: the event's condition is the LIVE instance (a torn-down - /// one denies); the notifier is still a legitimate notifier of it — the condition itself, a node it hangs + /// Three conditions, all against the CURRENT state: a live condition holds the event's condition NodeId; + /// the notifier is still a legitimate notifier of THAT live instance — the condition itself, a node it hangs /// beneath, or a wired folder the current generation still projects its raw tag into; and the condition is /// visible to the session under exactly the Server-object rule ( over /// ), so a moved scripted alarm is withheld from its old folder just @@ -5569,6 +5571,15 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3 /// and the condition's own method audits reach these notifiers. /// /// + /// ⚠️ Decided on the live instance, never on the event's own (review of #826). The SDK caches the + /// verdict per (item, event type, SourceNode) — no instance identity — so a verdict must be a + /// function of the condition's NodeId and the current state alone. Denying an event because it came from a + /// REPLACED instance (a surgical remove + re-materialise under the same NodeId, the old instance's last + /// event still in the SDK's queue) cached a deny the new instance then inherited until the next + /// invalidation. A late event from a replaced instance is therefore delivered exactly when the live one's + /// would be. + /// + /// /// Runs on the SDK's per-node delivery task, where the inherited role-permission check already takes /// Lock (via GetNodeMetadata); the scope resolution here takes it the same way. /// @@ -5577,8 +5588,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3 { if (item.ManagerHandle is not NodeHandle { Node: { } notifier }) return false; if (EventConditionOf(e) is not { } condition) return false; - if (!_alarmConditions.TryGetValue(MapKey(condition.NodeId), out var live) || !ReferenceEquals(live, condition)) - return false; + if (!_alarmConditions.TryGetValue(MapKey(condition.NodeId), out var live)) return false; if (!IsLiveNotifierOf(live, notifier)) return false; return IsConditionVisibleTo(item, ConditionVisibilityScopes(live)); } @@ -5662,31 +5672,85 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3 /// Drops every per-event verdict the SDK has memoized for event items on this manager's notifiers (Gitea #826), /// so the next event on each is decided afresh by . /// + /// Run even while enforcement is off — the authorizer setter's on/off transition, where the + /// cached verdicts were decided under the other mode. /// - /// MonitoredNode2.InvalidatePermissionCacheForSession is the SDK's only public lever: it advances that - /// node's permission generation (a verdict computed concurrently under the old one is recomputed, not stored) - /// and drops the session's entries. Cost is one call per (notifier, session) pair — never per condition. The - /// node list is copied under Lock; the invalidation itself takes no lock of ours. + /// + /// Node-wide, session-agnostic. Each notifier's cache is cleared whole through the SDK's own private + /// MonitoredNode2.InvalidatePermissionCaches (what the SDK runs when namespace default permissions + /// change, and on Rebind): it advances the node's permission generation — a verdict computed + /// concurrently under the old one is recomputed, never stored — and clears the cache in one O(cache) pass. + /// The public InvalidatePermissionCacheForSession is NOT sufficient (review of #826): it reaches only + /// items whose Session is set, and an ORPHANED subscription's items (session closed without deleting + /// its subscriptions) have none — they kept a stale allow through a deploy, queued the moved alarm's events + /// and delivered them on TransferSubscriptions. It also rescanned the whole cache once per item + /// (items² × conditions per folder). + /// + /// + /// The private method is resolved once by reflection against the pinned SDK and pinned by + /// FolderEventVerdictInvalidationGuardTests. Should a future SDK drop it, this falls back to the + /// supported route: the per-session drop for items with a session, and a detach + re-attach of each + /// session-less item on its node (Remove drops that item's entries), which can miss an event racing + /// the swap — the reason it is only the fallback. + /// + /// + /// Skipped while enforcement is off ( aside): the verdicts are then the SDK's own + /// role check, which nothing here changes. The node list is copied under Lock; the invalidation + /// takes no lock of ours. + /// /// - private void InvalidateFolderEventVerdicts() + private void InvalidateFolderEventVerdicts(bool force = false) { - if (_root is null) return; + if (_root is null || (!force && !AclEnforced)) return; List nodes; lock (Lock) nodes = MonitoredNodes.Values.Where(n => n.EventMonitoredItems is { IsEmpty: false }).ToList(); foreach (var node in nodes) { + Interlocked.Increment(ref _folderEventVerdictInvalidations); + if (InvalidateNodePermissionCaches is { } invalidate) + { + invalidate(node); + continue; + } + var sessions = new HashSet(); + var sessionless = new List(); foreach (var item in node.EventMonitoredItems.Values) { - if (item?.Session?.Id is { IsNull: false } id) sessions.Add(id); + if (item is null) continue; + if (item.Session?.Id is { IsNull: false } id) sessions.Add(id); + else sessionless.Add(item); } foreach (var session in sessions) node.InvalidatePermissionCacheForSession(session); + foreach (var item in sessionless) + { + node.Remove(item); + node.Add(item); + } } } + /// Notifier caches has invalidated since start. Test + /// observability (it must not run while enforcement is off). + internal long FolderEventVerdictInvalidations => Interlocked.Read(ref _folderEventVerdictInvalidations); + + private long _folderEventVerdictInvalidations; + + /// + /// The SDK's private node-wide cache invalidation (MonitoredNode2.InvalidatePermissionCaches), or null + /// when the SDK no longer has it — see . + /// + internal static readonly Action? InvalidateNodePermissionCaches = + typeof(MonitoredNode2).GetMethod( + "InvalidatePermissionCaches", + System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic, + Type.EmptyTypes) is { ReturnType: var returns } method && returns == typeof(void) + ? method.CreateDelegate>() + : null; + /// Whether 's session may see a condition with the given scopes. /// The Server-object event item. /// From . diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DeploymentArtifact.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DeploymentArtifact.cs index d287bb9ef..882d3d408 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DeploymentArtifact.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DeploymentArtifact.cs @@ -95,7 +95,7 @@ public sealed record DataPlaneAuthorityProjection( /// /// RawPath → EquipmentIds from the artifact's UnsTagReferences (Gitea #826): which equipment - /// folders this generation projects each raw tag into — the folders a native alarm on that tag may be + /// folders this generation projects each ALARM-BEARING raw tag (TagConfig.alarm) into — the folders a native alarm on that tag may be /// delivered through. RawPaths come from the SAME as /// , so they equal the native conditions' ConditionIds by construction. Empty /// when the artifact carries no grants. @@ -1027,12 +1027,17 @@ internal sealed class ParsedDeploymentArtifact : IDisposable } var tags = new Dictionary(StringComparer.Ordinal); + // TagConfig kept unparsed: only a REFERENCED tag's is parsed below (Gitea #826 — alarm-bearing ones only). + var tagConfigs = new Dictionary(StringComparer.Ordinal); foreach (var el in EnumerateArray(root, "Tags")) { var id = ReadString(el, "TagId"); var deviceId = ReadString(el, "DeviceId"); if (id is null || deviceId is null) continue; tags[id] = (deviceId, ReadNullableString(el, "TagGroupId"), ReadString(el, "Name") ?? id); + tagConfigs[id] = el.TryGetProperty("TagConfig", out var tcEl) && tcEl.ValueKind == JsonValueKind.String + ? tcEl.GetString() + : null; } var areas = new Dictionary(StringComparer.Ordinal); @@ -1091,12 +1096,21 @@ internal sealed class ParsedDeploymentArtifact : IDisposable if (resolver.ResolveOne(NodeAclScopeKind.Equipment, equipmentId) is [var path]) equipmentPaths[equipmentId] = path; } - // Which equipment folders each raw tag is projected into (Gitea #826) — the folders a native alarm on that tag - // may legitimately be delivered through. Keyed on the RawPath the SAME resolver builds, which is the native - // condition's ConditionId. + // Which equipment folders each ALARM-BEARING raw tag is projected into (Gitea #826) — the folders a native alarm + // on that tag may legitimately be delivered through; only native conditions are ever looked up, so plain value + // tags are left out. Keyed on the RawPath the SAME resolver builds, which is the native condition's ConditionId. + // Alarm-bearing is decided by the SAME TagConfigIntent the composer materialises native conditions from. + var alarmBearing = new Dictionary(StringComparer.Ordinal); var referencing = new Dictionary>(StringComparer.Ordinal); foreach (var reference in references) { + if (!alarmBearing.TryGetValue(reference.TagId, out var isAlarm)) + { + isAlarm = TagConfigIntent.Parse(tagConfigs.GetValueOrDefault(reference.TagId)).Alarm is not null; + alarmBearing[reference.TagId] = isAlarm; + } + + if (!isAlarm) continue; if (resolver.ResolveTagRawPath(reference.TagId) is not { } rawPath) continue; if (!referencing.TryGetValue(rawPath, out var set)) referencing[rawPath] = set = new HashSet(StringComparer.OrdinalIgnoreCase); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/FolderEventVerdictInvalidationTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/FolderEventVerdictInvalidationTests.cs new file mode 100644 index 000000000..37fe86526 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/FolderEventVerdictInvalidationTests.cs @@ -0,0 +1,188 @@ +using Opc.Ua; +using Opc.Ua.Client; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; +using ZB.MOM.WW.OtOpcUa.Configuration.Entities; +using ZB.MOM.WW.OtOpcUa.Configuration.Enums; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Core.Authorization; + +namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests; + +/// +/// Review of Gitea #826: the SDK memoizes each folder event item's per-event verdict per +/// (item, event type, SourceNode), so the per-event decision is only as fresh as its invalidation. These pin +/// the three ways it went stale or cost too much: an ORPHANED subscription (no session to invalidate by), a stale +/// DENY left by a replaced instance's late event, and invalidation running with enforcement off. +/// +public sealed class FolderEventVerdictInvalidationTests +{ + private const string EqA = ServerEventRig.EqA; + private const string EqB = ServerEventRig.EqB; + + /// + /// A subscription orphaned by its session (closed without deleting its subscriptions) keeps its items on the + /// folder fan-out with NO session. A deployment that drops EqA's reference must still reach those items: the + /// native alarm's events raised while orphaned are not queued to the EqA item, so a TransferSubscriptions to a + /// new session delivers them only through EqB (positive control on the same subscription and event). + /// + [Fact] + public async Task An_orphaned_folder_item_does_not_keep_a_stale_allow_across_a_deploy_and_a_transfer() + { + var ct = TestContext.Current.CancellationToken; + await using var rig = await ServerEventRig.StartAsync("Folder826Orphan", encrypted: false, ct); + rig.KeyRoles = []; + rig.MaterialiseNative(); + NodeAcl[] grants = [KeyEquipmentGrant(EqA), KeyEquipmentGrant(EqB)]; + var state = rig.Enforce(true, [ServerEventRig.Key()], grants); + + using var first = await rig.ConnectAsync(anonymous: false, ct); + var events = new RecordedEvents(); + var subscription = await RecordedEvents.SubscribeAsync(first, events, ct); + var onA = RecordedEvents.AddEventItem(subscription, ServerEventRig.UnsNodeId(first, EqA)); + var onB = RecordedEvents.AddEventItem(subscription, ServerEventRig.UnsNodeId(first, EqB)); + await subscription.ApplyChangesAsync(ct); + onA.Status.Error.ShouldBeNull(); + + // Seed the SDK's cache with an ALLOW for the EqA item. + rig.Project(native: true, State("t1")); + await RecordedEvents.WaitUntilAsync(() => events.Has(onA, "t1") && events.Has(onB, "t1")); + + // Orphan the subscription: a raw CloseSession with deleteSubscriptions=false, so the subscription and its items + // stay on the server with no session. The raw service leaves the client session OBJECT alive (Session.CloseAsync + // would dispose it), as a reconnecting client's is, so the high-level transfer can take the subscription over. + var orphaned = new SubscriptionCollection(first.Subscriptions); + await ((ISessionClient)first).CloseSessionAsync(null, deleteSubscriptions: false, ct); + + // The deployment drops EqA's reference; the alarm reports while the subscription has no session. + Deploy(state, "rev-2", grants, EqB); + rig.Project(native: true, State("t2")); + + using var second = await rig.ConnectAsync(anonymous: false, ct); + (await second.TransferSubscriptionsAsync(orphaned, sendInitialValues: false, ct)).ShouldBeTrue(); + + rig.Project(native: true, State("t3")); + await RecordedEvents.WaitUntilAsync(() => events.Has(onB, "t2") && events.Has(onB, "t3")); + await Task.Delay(TimeSpan.FromMilliseconds(700), ct); + events.Has(onA, "t2").ShouldBeFalse("an event raised while orphaned must not be queued on a stale allow"); + events.Has(onA, "t3").ShouldBeFalse(); + + await second.CloseAsync(ct); + } + + /// + /// A surgical replace of a scripted condition under the SAME NodeId (remove + re-materialise, the folder item + /// survives), with the old instance's last event still waiting in the SDK's per-folder queue when the apply's + /// invalidation runs. That late event must not leave a cached DENY the new instance inherits: the new + /// instance's events keep reaching the folder subscriber. + /// + [Fact] + public async Task A_replaced_instances_late_event_does_not_leave_a_cached_deny_for_the_new_instance() + { + var ct = TestContext.Current.CancellationToken; + await using var rig = await ServerEventRig.StartAsync("Folder826Replace", encrypted: false, ct); + rig.KeyRoles = []; + rig.MaterialiseScripted(); + rig.MaterialiseNative(); // also wired into EqA: its event is the deterministic blocker below + rig.Enforce(true, [], ServerEventRig.EquipmentGrant(EqA, NodePermissions.ReadOnly)); + + using var session = await rig.ConnectAsync(anonymous: true, ct); + var events = new RecordedEvents(); + var subscription = await RecordedEvents.SubscribeAsync(session, events, ct); + var onA = RecordedEvents.AddEventItem(subscription, ServerEventRig.UnsNodeId(session, EqA)); + await subscription.ApplyChangesAsync(ct); + onA.Status.Error.ShouldBeNull(); + + rig.Project(native: false, State("s1")); + await RecordedEvents.WaitUntilAsync(() => events.Has(onA, "s1")); + + // Holding the node-manager lock parks EqA's SDK delivery task: the native event "n1" is queued first and its + // verdict (a cache miss after the first invalidation) needs the lock in the inherited role check — its node is + // never removed, so that check always takes it. The OLD scripted instance's "s2", queued behind it on the same + // single-reader queue, is therefore decided only after the replace and the apply's invalidation. + lock (rig.Nm.Lock) + { + rig.Nm.CompleteAddressSpaceApply(); // forget the cached allows, so both verdicts must be computed + rig.Project(native: true, State("n1")); + rig.Project(native: false, State("s2")); + ((ISurgicalAddressSpaceSink)rig.Sink) + .RemoveAlarmConditionNode(ServerEventRig.ScriptedAlarmId, AddressSpaceRealm.Uns).ShouldBeTrue(); + rig.MaterialiseScripted(); + rig.Nm.CompleteAddressSpaceApply(); + } + + rig.Project(native: false, State("s3")); + // The new instance's event arrives (a cached deny would withhold it until the next invalidation), and so does + // the old instance's late one — decided on the live instance, exactly as the live one's would be. + await RecordedEvents.WaitUntilAsync(() => events.Has(onA, "s3") && events.Has(onA, "s2")); + + await session.CloseAsync(ct); + } + + /// + /// Review of #826 (perf): with enforcement OFF the folder verdicts are the SDK's own role check, so nothing + /// invalidates them — not the revalidation tick, not a deploy, not the end of an apply. Positive control: the + /// same calls invalidate once enforcement is on. + /// + [Fact] + public async Task Folder_verdicts_are_not_invalidated_while_enforcement_is_off() + { + var ct = TestContext.Current.CancellationToken; + await using var rig = await ServerEventRig.StartAsync("Folder826AclOff", encrypted: false, ct); + rig.MaterialiseNative(); + var state = rig.Enforce(false, [], ServerEventRig.EquipmentGrant(EqA, NodePermissions.ReadOnly)); + + using var session = await rig.ConnectAsync(anonymous: true, ct); + var events = new RecordedEvents(); + var subscription = await RecordedEvents.SubscribeAsync(session, events, ct); + RecordedEvents.AddEventItem(subscription, ServerEventRig.UnsNodeId(session, EqA)); + await subscription.ApplyChangesAsync(ct); + + var before = rig.Nm.FolderEventVerdictInvalidations; + rig.Nm.RevalidateMonitoredItems(); + rig.Nm.CompleteAddressSpaceApply(); + state.Apply(ServerEventRig.Cluster, "rev-off-2", [], [ServerEventRig.EquipmentGrant(EqA, NodePermissions.ReadOnly)], + ServerEventRig.ScopePaths()); + rig.Nm.FolderEventVerdictInvalidations.ShouldBe(before, "enforcement off: nothing to invalidate"); + + rig.Enforce(true, [], ServerEventRig.EquipmentGrant(EqA, NodePermissions.ReadOnly)); + var enforced = rig.Nm.FolderEventVerdictInvalidations; + enforced.ShouldBeGreaterThan(before, "switching enforcement on invalidates the verdicts decided while off"); + rig.Nm.CompleteAddressSpaceApply(); + rig.Nm.FolderEventVerdictInvalidations.ShouldBeGreaterThan(enforced); + + await session.CloseAsync(ct); + } + + /// + /// The node-wide invalidation rests on a PRIVATE method of the pinned SDK (MonitoredNode2.InvalidatePermissionCaches). + /// If an SDK upgrade drops it the node manager silently falls back to the weaker supported route — this fails + /// first, so the upgrade is looked at. + /// + [Fact] + public void The_sdks_node_wide_permission_cache_invalidation_is_still_resolvable() + { + OtOpcUaNodeManager.InvalidateNodePermissionCaches.ShouldNotBeNull( + "MonitoredNode2.InvalidatePermissionCaches() is gone — re-check InvalidateFolderEventVerdicts against the new SDK"); + } + + private static AlarmConditionSnapshot State(string message) => + new(true, false, false, true, AlarmShelvingKind.Unshelved, 700, message); + + private static void Deploy(DataPlaneAuthorityState state, string revision, NodeAcl[] grants, params string[] referencing) => + state.Apply( + ServerEventRig.Cluster, revision, [ServerEventRig.Key()], grants, ServerEventRig.ScopePaths(), + ServerEventRig.EquipmentOwnerPaths(), ServerEventRig.ScriptedAlarmOwners(), ServerEventRig.FolderScopes(), + new Dictionary> { [ServerEventRig.RawAlarmPath] = referencing.ToHashSet() }); + + private static NodeAcl KeyEquipmentGrant(string equipment) => new() + { + NodeAclId = $"acl-key-{equipment}", + ClusterId = ServerEventRig.Cluster, + Principal = ServerEventRig.KeyPrincipal, + ScopeKind = NodeAclScopeKind.Equipment, + ScopeId = $"eq-{equipment}", + PermissionFlags = NodePermissions.ReadOnly, + }; +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactScopePathTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactScopePathTests.cs index b8df2b82e..f659051ba 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactScopePathTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactScopePathTests.cs @@ -314,27 +314,62 @@ public sealed class DeploymentArtifactScopePathTests } /// - /// Gitea #826 — the projection says which equipment each raw tag is projected into, keyed on the RawPath the - /// same resolver builds (a native alarm's ConditionId), so the node manager can tell a native alarm's - /// still-referenced equipment folders from one whose reference the generation dropped. + /// Gitea #826 — the projection says which equipment each ALARM-BEARING raw tag is projected into, keyed on the + /// RawPath the same resolver builds (a native alarm's ConditionId), so the node manager can tell a native + /// alarm's still-referenced equipment folders from one whose reference the generation dropped. Plain value tags + /// are never looked up and are left out (review: memory). /// [Fact] - public void The_projection_carries_each_raw_tags_referencing_equipment_under_its_raw_path() + public void The_projection_carries_each_alarm_bearing_raw_tags_referencing_equipment_under_its_raw_path() { - var projection = DeploymentArtifact.ParseDataPlaneAuthority(Encoding.UTF8.GetBytes(Artifact), NodeId); + var projection = DeploymentArtifact.ParseDataPlaneAuthority(Encoding.UTF8.GetBytes(AlarmArtifact), NodeId); - const string rawPath = "Plant/Line1/ModbusDrv/dev1/Fast/Speed"; - projection.TagReferencingEquipment.Keys.ShouldBe([rawPath]); - projection.TagReferencingEquipment[rawPath].ShouldBe(["EQ-mixer"]); + const string hot = "Plant/Line1/ModbusDrv/dev1/Fast/Hot"; + projection.TagReferencingEquipment.Keys.ShouldBe([hot], "the plain Speed tag is not alarm-bearing"); + projection.TagReferencingEquipment[hot].Order(StringComparer.Ordinal).ShouldBe(["EQ-mixer", "EQ-oven"]); var snapshot = new DataPlaneAuthorityState().Apply( "c1", "rev-1", projection.Keys, projection.Grants, projection.ScopePaths, projection.EquipmentScopePaths, projection.ScriptedAlarmOwners, projection.UnsFolderScopes, projection.TagReferencingEquipment); - snapshot.IsTagReferencedBy(rawPath, "EQ-mixer").ShouldBeTrue(); - snapshot.IsTagReferencedBy(rawPath, "eq-MIXER").ShouldBeTrue("equipment ids compare as identifiers"); - snapshot.IsTagReferencedBy(rawPath, "EQ-other").ShouldBeFalse(); - snapshot.IsTagReferencedBy("Plant/Line1/ModbusDrv/dev1/Fast/Other", "EQ-mixer").ShouldBeFalse(); + snapshot.IsTagReferencedBy(hot, "EQ-oven").ShouldBeTrue(); + snapshot.IsTagReferencedBy(hot, "eq-MIXER").ShouldBeTrue("equipment ids compare as identifiers"); + snapshot.IsTagReferencedBy(hot, "EQ-other").ShouldBeFalse(); + snapshot.IsTagReferencedBy("Plant/Line1/ModbusDrv/dev1/Fast/Speed", "EQ-mixer").ShouldBeFalse(); new DataPlaneAuthorityState().Apply("c1", "rev-1", [], [], projection.ScopePaths) - .IsTagReferencedBy(rawPath, "EQ-mixer").ShouldBeFalse("an absent map references nothing (fail closed)"); + .IsTagReferencedBy(hot, "EQ-mixer").ShouldBeFalse("an absent map references nothing (fail closed)"); } + + // The fixture's topology plus an alarm-bearing Hot tag referenced by two equipment; Speed stays a plain value. + private const string AlarmArtifact = """ + { + "RawFolders": [ + {"RawFolderId":"RF-plant","ClusterId":"c1","ParentRawFolderId":null,"Name":"Plant"}, + {"RawFolderId":"RF-line1","ClusterId":"c1","ParentRawFolderId":"RF-plant","Name":"Line1"} + ], + "DriverInstances": [ + {"DriverInstanceId":"DI-modbus","ClusterId":"c1","RawFolderId":"RF-line1","Name":"ModbusDrv"} + ], + "Devices": [{"DeviceId":"DV-1","DriverInstanceId":"DI-modbus","Name":"dev1"}], + "TagGroups": [{"TagGroupId":"TG-fast","DeviceId":"DV-1","ParentTagGroupId":null,"Name":"Fast"}], + "Tags": [ + {"TagId":"TAG-speed","DeviceId":"DV-1","TagGroupId":"TG-fast","Name":"Speed"}, + {"TagId":"TAG-hot","DeviceId":"DV-1","TagGroupId":"TG-fast","Name":"Hot", + "TagConfig":"{\"alarm\":{\"alarmType\":\"OffNormalAlarm\"}}"} + ], + "UnsAreas": [{"UnsAreaId":"UA-w","ClusterId":"c1","Name":"West"}], + "UnsLines": [{"UnsLineId":"UL-1","UnsAreaId":"UA-w","Name":"L1"}], + "Equipment": [ + {"EquipmentId":"EQ-mixer","UnsLineId":"UL-1","Name":"Mixer"}, + {"EquipmentId":"EQ-oven","UnsLineId":"UL-1","Name":"Oven"} + ], + "UnsTagReferences": [ + {"UnsTagReferenceId":"REF-1","EquipmentId":"EQ-mixer","TagId":"TAG-speed"}, + {"UnsTagReferenceId":"REF-2","EquipmentId":"EQ-mixer","TagId":"TAG-hot"}, + {"UnsTagReferenceId":"REF-3","EquipmentId":"EQ-oven","TagId":"TAG-hot"} + ], + "NodeAcls": [ + {"NodeAclId":"acl-eq","ClusterId":"c1","Principal":"p","ScopeKind":4,"ScopeId":"EQ-mixer","PermissionFlags":16} + ] + } + """; }