dohertj2/lmxopcua
1
0
Fork 0
Code Issues 32 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(opcua): Part 9 condition children get instance-scoped NodeIds (#629)

Browse Source 
Create(assignNodeIds: true) asked CustomNodeManager2.New, whose default returns the
node's current id, so every condition child kept the type's ns=0 instance-declaration id
(or an SDK-generated ns=0 number): translate of a child path returned BadNoMatch and a
Read of a browsed child id was BadNodeIdUnknown or served from the TYPE (UnshelveTime 0
on a condition timed-shelved for a minute), bypassing the data-plane ACL.

OtOpcUaNodeManager.New now derives {conditionId}/{browsePath} in the condition's
namespace, so the ACL scope of a child is its condition's scope plus the path. The
ShelvingState methods adopt their original AlarmConditionType_ShelvingState_* id as
MethodDeclarationId so clients calling them by that id still resolve. Condition removal
and rebuild forget the whole PredefinedNodes subtree so a removed condition's children
stop answering.

Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
This commit is contained in:
Joseph Doherty
2026-10-08 14:45:46 -04:00
parent d898262c4a
commit d0052d9fed
5 changed files with 443 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs
+78 -4
View File
@@ -1243,7 +1243,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
// path; keep it symmetric with RemoveAlarmConditionNode / RebuildAddressSpace.
UnwireAlarmNotifiers(conditionKey);
existing.Parent?.RemoveChild(existing);
PredefinedNodes?.Remove(existing.NodeId);
ForgetPredefinedSubtree(existing);
// The materialise tail raises only the new instance's unit (no full reconcile), so give the
// dropped instance's back here rather than leave it tracked until the next full reconcile.
if (_allEventsMonitoredConditions.Remove(existing))
@@ -2499,6 +2499,80 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
_ => new AlarmConditionState(parent),
};
/// <summary>
/// Gitea #629 — the <see cref="INodeIdFactory"/> hook <c>NodeState.Create(…, assignNodeIds: true)</c> calls
/// for the node being created and then, top-down, for every child it built from the type's definition.
/// A node that already carries an id in one of our namespaces (the condition root, whose id
/// <see cref="MaterialiseAlarmCondition"/> passes to <c>Create</c>) keeps it; a child is given
/// <c>{parent id}/{BrowseName}</c> in its parent's namespace.
/// </summary>
/// <remarks>
/// <para>
/// The SDK default (<c>CustomNodeManager2.New</c>) returns the node's CURRENT id, which for a child is
/// the condition type's instance-declaration id (<c>i=9189</c> for <c>ShelvingState/UnshelveTime</c>) or
/// an SDK-generated ns=0 number. Every condition's children therefore sat in ns=0: a
/// TranslateBrowsePathsToNodeIds of any child path returned <c>BadNoMatch</c>, and a Read of a browsed
/// child id either hit <c>BadNodeIdUnknown</c> or was served by the core node manager from the TYPE
/// (UnshelveTime read <c>0</c> on a condition timed-shelved for a minute). Event delivery and the Part 9
/// methods were unaffected — events copy the instance's fields and the SDK resolves a method by its
/// declaration id.
/// </para>
/// <para>
/// The derived id is deterministic and unique per condition: the parent part is the condition's own
/// id (a RawPath in the raw namespace, a ScriptedAlarmId in the UNS namespace) and sibling BrowseNames
/// are unique. It cannot collide with a raw node — a condition's RawPath is a tag, a leaf, so nothing
/// else is ever authored beneath it — and the namespace keeps the two realms apart. It is also
/// slash-joined on purpose: <c>DataPlaneAuthorizer.BuildScope</c> splits the identifier on <c>/</c>,
/// so a child's scope is its condition's scope plus the browse path, and the permission trie's
/// downward inheritance governs a child exactly as it governs its condition.
/// </para>
/// </remarks>
/// <param name="context">The system context of the create.</param>
/// <param name="node">The node being assigned an id.</param>
/// <returns>The id the node is to carry.</returns>
public override NodeId New(ISystemContext context, NodeState node)
{
if (OwnsNamespace(node.NodeId)) return node.NodeId;
if (node is BaseInstanceState { Parent.NodeId: { IdType: IdType.String } parentId }
&& OwnsNamespace(parentId)
&& !QualifiedName.IsNull(node.BrowseName))
{
// A client calls a Part 9 method by its TYPE's method id (ObjectId = the condition, MethodId =
// e.g. AlarmConditionType_ShelvingState_OneShotShelve, i=9212), and the SDK resolves it by NodeId OR
// MethodDeclarationId. The condition's own methods (Acknowledge, …) carry the right declaration id
// already, over an SDK-generated ns=0 NodeId. The ShelvingState methods do NOT: their declaration id
// is the ShelvedStateMachineType's (i=2948), and they were found only because their NodeId WAS the
// AlarmConditionType instance declaration every client uses. Adopt that id as the declaration before
// it is replaced — but only when it is a real node of the standard address space, never an
// SDK-generated number — or every shelve/unshelve call fails BadMethodInvalid.
if (node is MethodState method
&& method.NodeId is { NamespaceIndex: 0 } declarationId
&& declarationId != method.MethodDeclarationId
&& FindNodeInAddressSpace(declarationId) is MethodState)
{
method.MethodDeclarationId = declarationId;
}
return new NodeId($"{parentId.Identifier}/{node.BrowseName.Name}", parentId.NamespaceIndex);
}
return node.NodeId;
}
/// <summary>
/// Gitea #629 — drops a removed condition AND every descendant from <c>PredefinedNodes</c>.
/// <c>AddPredefinedNode</c> indexes the whole subtree, so removing only the root would leave the
/// condition's (now instance-scoped, hence reachable) children answering a direct Read with the dead
/// condition's last value. MUST be called under <c>Lock</c>.
/// </summary>
/// <param name="node">The node whose subtree is forgotten.</param>
private void ForgetPredefinedSubtree(NodeState node)
{
PredefinedNodes?.Remove(node.NodeId);
var children = new List<BaseInstanceState>();
node.GetChildren(SystemContext, children);
foreach (var child in children) ForgetPredefinedSubtree(child);
}
/// <summary>Promote <paramref name="folder"/> to <see cref="EventNotifiers.SubscribeToEvents"/> and
/// register it as a root notifier (idempotent — guarded by <see cref="_notifierFolders"/>) so the
/// folder is a subscribable event source linked under the Server object (Server-object delivery itself is
@@ -3123,7 +3197,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
foreach (var alarm in _alarmConditions.Values)
{
alarm.Parent?.RemoveChild(alarm);
PredefinedNodes?.Remove(alarm.NodeId);
ForgetPredefinedSubtree(alarm);
}
// v3 Batch 4 WP4: tear down every native-alarm→equipment-folder notifier pair bidirectionally
// BEFORE the conditions + folders are cleared below, so no inverse-notifier entry leaks across the
@@ -3249,7 +3323,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
// this they would keep a dangling inverse-notifier entry to the removed condition.
UnwireAlarmNotifiers(key);
condition.Parent?.RemoveChild(condition);
PredefinedNodes?.Remove(condition.NodeId);
ForgetPredefinedSubtree(condition);
_nativeAlarmNodeIds.Remove(key);
ReconcileAllEventsMonitoring();
e = BuildNodesRemovedModelChange(alarmNodeId, realm);
@@ -3302,7 +3376,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
if (_alarmConditions.TryRemove(id, out var c))
{
c.Parent?.RemoveChild(c);
PredefinedNodes?.Remove(c.NodeId);
ForgetPredefinedSubtree(c);
}
_nativeAlarmNodeIds.Remove(id);
}