diff --git a/docs/security.md b/docs/security.md index 1d02687b5..a284f90c8 100644 --- a/docs/security.md +++ b/docs/security.md @@ -917,7 +917,8 @@ Behaviours worth knowing before enabling it: event type, source), so a refresh of a folder item could still be followed by older events. Now, at every notifier the node manager owns, the walk serves the attached items itself (`DeliverToNotifierItems`), exactly as it serves Server-object items: the SDK's audit guard, the verdict through `INodeManager3.ValidateEventRolePermissions` (SDK - type/source `ReceiveEvents` + the #826 decision under enforcement) **per event, uncached**, then the item's gate and + type/source `ReceiveEvents` + the #826 decision under enforcement — its item-independent part, the live condition, + the notifier check and the visibility scopes, resolved once per hop) **per event, uncached**, then the item's gate and cut, then `QueueEvent`. The `MonitoredNode2` stays for item attachment and for what still uses its channel (a condition's SDK method audit events, data-change items on the folder), so `InvalidateFolderEventVerdicts` stays. - **No notifier of ours defers any more.** `QueueEvent` never waits for a consumer (the item queue discards oldest at diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index 56e116e51..74104d3d6 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -1539,13 +1539,19 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3 return; } + DeferredConditionReports queue; lock (_eventReportGate) { - var queue = new DeferredConditionReports(); + queue = new DeferredConditionReports(); _deferredReports[condition] = queue; CountDeferred(+1, condition); - _ = DrainDeferredAsync(condition, queue, delivery.AsTask()); } + + // #855 review 2: started on the pool, never inline. The suspended delivery may complete between the check above and + // here; an inline drain would then run (and end the episode, and clear cuts over every item gate) on this thread, + // under Lock. The queue is registered first, so this condition's later reports wait behind it either way. + var inFlight = delivery.AsTask(); + _ = Task.Run(() => DrainDeferredAsync(condition, queue, inFlight)); } /// Queues a later event behind a condition's deferred one, coalescing past the budget. Under the gate. @@ -1719,13 +1725,33 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3 private void DeliverToNotifierItems(MonitoredNode2 monitored, IFilterTarget e) { var deferred = e is ConditionEventSnapshot { Deferred: true } d ? d : null; + + // #855 review 2: the #826 decision's item-independent part — the live condition, whether this notifier is still a + // legitimate notifier of it, and its visibility scopes — is resolved ONCE per hop, not per item (it is what + // IsNotifierEventVisibleTo resolves for an item attached here). Only the grant check and the SDK's role check + // remain per item. + var enforced = AclEnforced; + List? scopes = null; + if (enforced) + { + if (EventConditionOf(e) is not { } named + || !_alarmConditions.TryGetValue(MapKey(named.NodeId), out var live) + || !IsLiveNotifierOf(live, monitored.Node)) + { + return; // fail closed for every item on this notifier + } + + scopes = ConditionVisibilityScopes(live); + } + foreach (var item in monitored.EventMonitoredItems.Values) { if (item is null) continue; try { if (IsAuditEvent(e) && !IsAuditDeliverableTo(item)) continue; - if (ServiceResult.IsBad(((INodeManager3)this).ValidateEventRolePermissions(item, e))) continue; + if (ServiceResult.IsBad(ValidateEventRolePermissions(item, e))) continue; + if (scopes is not null && !IsConditionVisibleTo(item, scopes)) continue; QueueConditionEventToItem(item, e, deferred, DeferredFolderDeliveryProbe); } catch (Exception ex) @@ -6701,6 +6727,9 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3 generation, (item, e) => { + // #855 review 2: never replay a condition a deploy has removed or replaced since the cut — its frozen + // state would be a ghost (and, with the ACL on, a replaced scripted alarm would be judged on its old folder). + if (ConditionOf(e) is { } frozen && !IsLiveCondition(frozen)) return false; if (enforced) { if (ConditionOf(e) is not { } condition) return false; @@ -6981,7 +7010,10 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3 DirectRefreshProbe?.Invoke(true, Monitor.IsEntered(Lock)); // #855 review: re-decided per entry right before queueing — a deploy may have revoked since the verdicts. - QueueFrozenReplay(items, states, perItem, generation, (item, e) => IsDirectRefreshEventAllowed(item, e, enforced)); + QueueFrozenReplay( + items, states, perItem, generation, + (item, e) => (ConditionOf(e) is not AlarmConditionState frozen || IsLiveCondition(frozen)) + && IsDirectRefreshEventAllowed(item, e, enforced)); } finally { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/FolderConditionRefreshWireTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/FolderConditionRefreshWireTests.cs index 0a9bc98b9..59287d809 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/FolderConditionRefreshWireTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/FolderConditionRefreshWireTests.cs @@ -426,6 +426,69 @@ public sealed class FolderConditionRefreshWireTests after.ShouldBeEmpty("and nothing reaches the item after RefreshEnd either"); } + /// + /// #855 review 2: a condition removed by a deploy between the cut and the queueing of its frozen replay must not be + /// replayed — its frozen Retain=true state would be a ghost no Retain=false ever follows. ACL off, so nothing but the + /// liveness check stands in the way. + /// + /// Refresh a Server-object item instead of a folder item. + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task A_condition_removed_between_the_cut_and_its_replay_is_not_replayed(bool serverObject) + { + var ct = TestContext.Current.CancellationToken; + await using var rig = await ServerEventRig.StartAsync("Refresh855RemoveCut", encrypted: false, ct); + rig.KeyRoles = []; + rig.MaterialiseNative(); + rig.Enforce(false, [ServerEventRig.Key()], KeyEquipmentGrant(EqA)); + + // A second subscriber keeps EqA a notifier fan-out the stall applies to. + using var other = await rig.ConnectAsync(anonymous: true, ct); + var otherEvents = new RecordedEvents(); + var otherSub = await RecordedEvents.SubscribeAsync(other, otherEvents, ct); + RecordedEvents.AddEventItem(otherSub, ServerEventRig.UnsNodeId(other, EqA)); + await otherSub.ApplyChangesAsync(ct); + + using var session = await rig.ConnectAsync(anonymous: false, ct); + var events = new RecordedEvents(); + var subscription = await RecordedEvents.SubscribeAsync(session, events, ct); + var item = RecordedEvents.AddEventItem(subscription, serverObject ? ObjectIds.Server : ServerEventRig.UnsNodeId(session, EqA)); + await subscription.ApplyChangesAsync(ct); + + rig.Project(native: true, Active("seed")); + await RecordedEvents.WaitUntilAsync(() => events.Has(item, "seed")); + + var removed = 0; + void Remove(bool cut, bool _) + { + if (cut && Interlocked.Exchange(ref removed, 1) == 0) + rig.Nm.RemoveAlarmConditionNode(ServerEventRig.RawAlarmPath, AddressSpaceRealm.Raw).ShouldBeTrue(); + } + + if (serverObject) rig.Nm.ServerObjectRefreshProbe = Remove; + else rig.Nm.DirectRefreshProbe = Remove; + + using (var stall = NotifierStall.Hold(rig.Nm, EqA)) + { + for (var i = 0; i < FloodSize; i++) rig.Project(native: true, Active($"f{i}")); + rig.Nm.DeferredEventReports.ShouldBeGreaterThan(0); + await session.CallAsync(ObjectTypeIds.ConditionType, MethodIds.ConditionType_ConditionRefresh, ct, subscription.Id); + await RecordedEvents.WaitUntilAsync(() => events.HasType(item, ObjectTypeIds.RefreshEndEventType)); + stall.Release(); + } + + rig.Nm.ServerObjectRefreshProbe = null; + rig.Nm.DirectRefreshProbe = null; + Volatile.Read(ref removed).ShouldBe(1, "the refresh took the cut path"); + await RecordedEvents.WaitUntilAsync(() => rig.Nm.DeferredEventReports == 0); + await Task.Delay(TimeSpan.FromMilliseconds(500), ct); + + var (replayed, after) = Window(events.Of(item)); + replayed.ShouldBeEmpty("the condition was removed before its frozen replay was queued"); + after.ShouldBeEmpty("and none of its deferred events follows"); + } + /// /// #855 review: a delivery that fails inline (here: a hop that throws) is over, not suspended — it must not be booked /// as a deferral episode (which moved the episode a concurrent plain refresh checks, and cleared cuts under the