Files
lmxopcua/code-reviews
Joseph Doherty 412c4bbd40 fix(driver-opcuaclient): resolve Medium code-review finding (Driver.OpcUaClient-006)
Route all Session mutations through _probeLock so OnReconnectComplete, ShutdownAsync,
and OnKeepAlive cannot race each other when swapping or clearing the active session.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-22 10:35:11 -04:00
..

Code Reviews

Cross-module code review index for the OtOpcUa server codebase (lmxopcua). The review process is defined in ../REVIEW-PROCESS.md.

Each module's findings.md is the source of truth; this file is generated from them by regen-readme.py and must not be edited by hand.

Module status

Module Reviewer Date Commit Status Open Total
Admin Claude Code 2026-05-22 76d35d1 Reviewed 7 12
Analyzers Claude Code 2026-05-22 76d35d1 Reviewed 7 7
Client.CLI Claude Code 2026-05-22 76d35d1 Reviewed 10 10
Client.Shared Claude Code 2026-05-22 76d35d1 Reviewed 9 11
Client.UI Claude Code 2026-05-22 76d35d1 Reviewed 11 11
Configuration Claude Code 2026-05-22 76d35d1 Reviewed 9 11
Core Claude Code 2026-05-22 76d35d1 Reviewed 10 12
Core.Abstractions Claude Code 2026-05-22 76d35d1 Reviewed 8 8
Core.AlarmHistorian Claude Code 2026-05-22 76d35d1 Reviewed 7 11
Core.ScriptedAlarms Claude Code 2026-05-22 76d35d1 Reviewed 11 12
Core.Scripting Claude Code 2026-05-22 76d35d1 Reviewed 9 11
Core.VirtualTags Claude Code 2026-05-22 76d35d1 Reviewed 12 13
Driver.AbCip Claude Code 2026-05-22 76d35d1 Reviewed 11 15
Driver.AbCip.Cli Claude Code 2026-05-22 76d35d1 Reviewed 8 8
Driver.AbLegacy Claude Code 2026-05-22 76d35d1 Reviewed 11 13
Driver.AbLegacy.Cli Claude Code 2026-05-22 76d35d1 Reviewed 7 7
Driver.Cli.Common Claude Code 2026-05-22 76d35d1 Reviewed 5 6
Driver.FOCAS Claude Code 2026-05-22 76d35d1 Reviewed 10 12
Driver.FOCAS.Cli Claude Code 2026-05-22 76d35d1 Reviewed 5 5
Driver.Galaxy Claude Code 2026-05-22 76d35d1 Reviewed 11 14
Driver.Historian.Wonderware Claude Code 2026-05-22 76d35d1 Reviewed 11 12
Driver.Historian.Wonderware.Client Claude Code 2026-05-22 76d35d1 Reviewed 9 10
Driver.Modbus Claude Code 2026-05-22 76d35d1 Reviewed 11 12
Driver.Modbus.Addressing Claude Code 2026-05-22 76d35d1 Reviewed 8 9
Driver.Modbus.Cli Claude Code 2026-05-22 76d35d1 Reviewed 8 8
Driver.OpcUaClient Claude Code 2026-05-22 76d35d1 Reviewed 10 15
Driver.S7 Claude Code 2026-05-22 76d35d1 Reviewed 10 14
Driver.S7.Cli Claude Code 2026-05-22 76d35d1 Reviewed 7 7
Driver.TwinCAT Claude Code 2026-05-22 76d35d1 Reviewed 11 16
Driver.TwinCAT.Cli Claude Code 2026-05-22 76d35d1 Reviewed 7 7
Server Claude Code 2026-05-22 76d35d1 Reviewed 12 15

Pending findings

Findings with status Open or In Progress, ordered by severity.

ID Severity Category Location Description
Admin-006 Medium Security Components/Layout/MainLayout.razor:47-49, Program.cs:129,131-135 app.UseAntiforgery() is enabled, but the Sign-out form (<form method="post" action="/auth/logout">) renders no antiforgery token, and the MapPost("/auth/logout", ...) endpoint does not call .DisableAntiforgery() or otherwise opt ou…
Admin-007 Medium Design-document adherence Components/Pages/Clusters/NewCluster.razor:91,95-96 NewCluster.CreateAsync hardcodes CreatedBy = "admin-ui" (both on the ServerCluster row and the draft generation) instead of the signed-in operator principal name. admin-ui.md section "Audit" requires "the operator principal" be rec…
Admin-008 Medium Error handling & resilience Services/ReservationService.cs:28-37 ReservationService.ReleaseAsync calls sp_ReleaseExternalIdReservation with only @Kind, @Value, @ReleaseReason. admin-ui.md section "Release an external-ID reservation" specifies the proc sets ReleasedBy to the FleetAdmin who…
Admin-009 Medium Testing coverage src/Server/ZB.MOM.WW.OtOpcUa.Admin (whole module) The module most security-critical behaviours have no enforced test coverage at the boundary that matters. There is no test that an unauthenticated request to a page or hub is rejected (which would have caught Admin-001/002/003), no test of…
Analyzers-001 Medium Correctness & logic bugs src/Tooling/ZB.MOM.WW.OtOpcUa.Analyzers/UnwrappedCapabilityCallAnalyzer.cs:135-139 IsInsideWrapperLambda treats a guarded call as "wrapped" if it is textually inside ANY lambda that is an argument to ANY invocation whose containing type is CapabilityInvoker or AlarmSurfaceInvoker. It matches the containing type onl…
Analyzers-006 Medium Testing coverage tests/Tooling/ZB.MOM.WW.OtOpcUa.Analyzers.Tests/UnwrappedCapabilityCallAnalyzerTests.cs The test suite exercises only 3 of the 7 guarded interfaces (IReadable, IWritable, ITagDiscovery) and one positive / one negative lambda case. Significant untested behaviour for an analyzer that gates a repo-wide resilience invariant…
Client.CLI-001 Medium Correctness & logic bugs Commands/HistoryReadCommand.cs:73, Commands/HistoryReadCommand.cs:76 The start and end options are parsed with DateTime.Parse(StartTime) with no IFormatProvider or DateTimeStyles. Parsing therefore depends on the current OS culture: the same --start "03/04/2026" resolves to March 4 on an en-US box a…
Client.CLI-005 Medium Concurrency & thread safety Commands/SubscribeCommand.cs:66-78, Commands/AlarmsCommand.cs:52-64 The DataChanged and AlarmEvent handlers write to console.Output (a System.IO.TextWriter) directly from the OPC UA SDK subscription/notification thread, while the command main flow is awaiting Task.Delay(Timeout.Infinite, ct) and…
Client.Shared-001 Medium Correctness & logic bugs OpcUaClientService.cs:552 OnAlarmEventNotification returns early when eventFields.EventFields has fewer than 6 entries. The event filter built by CreateAlarmEventFilter always registers 13 select clauses, so a conforming server returns 13 fields. The < 6 th…
Client.Shared-002 Medium Correctness & logic bugs OpcUaClientService.cs:351-355, OpcUaClientService.cs:373 GetRedundancyInfoAsync performs unguarded unboxing casts on values read from the server: (int)redundancySupportValue.Value and (byte)serviceLevelValue.Value. Unlike the ServerUriArray/ServerArray reads below them, the `Redundancy…
Client.Shared-007 Medium Concurrency & thread safety OpcUaClientService.cs:581-622 In the alarm fallback path, the Task.Run closure mutates the captured locals activeState, ackedState, time, and capturedMessage, then reads them when invoking AlarmEvent. Because the captured _session reference can be replace…
Client.Shared-008 Medium Error handling & resilience OpcUaClientService.cs:170-180, Helpers/ValueConverter.cs:15-31 WriteValueAsync coerces a string input to the target type by reading the node's current value and inferring the type from currentDataValue.Value. When the node has never been written, or the read returns a Bad status with a null `Val…
Client.UI-001 Medium Correctness & logic bugs ViewModels/HistoryViewModel.cs:76, ViewModels/HistoryViewModel.cs:77 ReadHistoryAsync runs as a RelayCommand body, which is invoked on the UI thread, so the bare IsLoading = true at line 76 happens to land on the right thread today. But Results.Clear() on the very next line is wrapped in `_dispatche…
Client.UI-002 Medium Correctness & logic bugs ViewModels/MainWindowViewModel.cs:255, ViewModels/MainWindowViewModel.cs:333 ConnectAsync calls await BrowseTree.LoadRootsAsync() and ViewHistoryForSelectedNode calls History.SelectedNodeId = ... by dereferencing the nullable child view-model properties (BrowseTreeViewModel?, HistoryViewModel?) without…
Client.UI-005 Medium Concurrency & thread safety ViewModels/MainWindowViewModel.cs:286-304, ViewModels/MainWindowViewModel.cs:155-189 SubscriptionsViewModel and AlarmsViewModel attach handlers to the long-lived _service events (DataChanged, AlarmEvent) in their constructors and detach them only via Teardown(). Teardown() is called from DisconnectAsync (op…
Client.UI-007 Medium Security Services/UserSettings.cs:22-23, Services/JsonSettingsService.cs:38-50, ViewModels/MainWindowViewModel.cs:393-408 The OPC UA UserName-token password is persisted in cleartext. UserSettings.Password is a plain string, JsonSettingsService.Save serializes the whole settings object to settings.json under LocalApplicationData, and `SaveSettings…
Client.UI-008 Medium Performance & resource management ViewModels/MainWindowViewModel.cs:18, ViewModels/MainWindowViewModel.cs:125-148, App.axaml.cs:18-32 IOpcUaClientService is declared IDisposable (IOpcUaClientService.cs:10), and the concrete service owns an OPC UA session plus SDK resources. MainWindowViewModel holds _service for the lifetime of the app but never calls `_service…
Configuration-002 Medium Correctness & logic bugs src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Migrations/20260417215224_StoredProcedures.cs:325 sp_RollbackToGeneration opens its own BEGIN TRANSACTION, clones rows into a new Draft, then EXEC dbo.sp_PublishGeneration, which itself runs BEGIN TRANSACTION (nesting @@TRANCOUNT to 2) and on its failure paths executes a bare `R…
Configuration-003 Medium Correctness & logic bugs src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftValidator.cs:73 ValidatePathLength computes path length with hard-coded constants — it always charges 64 chars for Enterprise+Site (32 + 32 + ...) regardless of the cluster's actual values. This over-rejects: a short Enterprise/Site is penalised by up…
Configuration-006 Medium Error handling & resilience src/Core/ZB.MOM.WW.OtOpcUa.Configuration/LocalCache/ResilientConfigReader.cs:79 The fallback catch filters on ex is not OperationCanceledException. A SQL command timeout surfaced by ADO.NET as a TaskCanceledException (derives from OperationCanceledException) is then treated as caller cancellation and propagate…
Configuration-009 Medium Security src/Core/ZB.MOM.WW.OtOpcUa.Configuration/DesignTimeDbContextFactory.cs:14 DefaultConnectionString embeds a plaintext sa password with User Id=sa directly in source, checked into the repository. Although used only at design time (dotnet ef), a checked-in sa credential normalises committing DB passwords…
Core-003 Medium Correctness & logic bugs src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/PermissionTrie.cs:80-98 WalkSystemPlatform records every Galaxy folder-segment grant with NodeAclScopeKind.Equipment (see the comment at lines 82-86) because NodeAclScopeKind has no FolderSegment member. The functional union of permission flags is unaffec…
Core-005 Medium Concurrency & thread safety src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/PermissionTrieCache.cs:59-70 Prune mutates the ConcurrentDictionary with a plain indexer assignment (_byCluster[clusterId] = new ClusterEntry(...)) after a separate TryGetValue read. If Install runs concurrently for the same cluster, the AddOrUpdate in `In…
Core-006 Medium Concurrency & thread safety src/Core/ZB.MOM.WW.OtOpcUa.Core/OpcUa/GenericDriverNodeManager.cs:42-64 BuildAddressSpaceAsync is not guarded against being called more than once. A second call subscribes a second _alarmForwarder to IAlarmSource.OnAlarmEvent and overwrites the _alarmForwarder field, so the first delegate is leaked (st…
Core-007 Medium Error handling & resilience src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/AlarmSurfaceInvoker.cs:75-83 UnsubscribeAsync always routes through _defaultHost, even when an IPerCallHostResolver is wired and the original SubscribeAsync fanned the subscription out to a non-default host. The IAlarmSubscriptionHandle is opaque here and ca…
Core.Abstractions-001 Medium Correctness & logic bugs src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/PollGroupEngine.cs:112 PollOnceAsync detects a change with !Equals(lastSeen?.Value, current.Value). object.Equals falls back to reference equality for reference types that do not override it — including T[] array values. The capability interfaces explici…
Core.Abstractions-002 Medium Correctness & logic bugs src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/PollGroupEngine.cs:105-109 PollOnceAsync iterates state.TagReferences and indexes the reader's result with snapshots[i], assuming the driver-supplied _reader delegate returns exactly one snapshot per input reference in input order. The contract is documented…
Core.Abstractions-003 Medium Concurrency & thread safety src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/PollGroupEngine.cs:64,121-130 Subscribe starts the poll loop with a fire-and-forget Task.Run and keeps no reference to the returned Task. Neither Unsubscribe nor DisposeAsync awaits the loop's completion — they only cancel the CancellationTokenSource and di…
Core.AlarmHistorian-003 Medium OtOpcUa conventions src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/SqliteStoreAndForwardSink.cs:107-127,218-243,246-253 EnqueueAsync is declared async-shaped (Task EnqueueAsync(...)) and the IAlarmHistorianSink contract explicitly states "the sink MUST NOT block the emitting thread … EnqueueAsync returns as soon as the queue row is committed." But…
Core.AlarmHistorian-005 Medium Concurrency & thread safety src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/SqliteStoreAndForwardSink.cs:66-71,141-143,199,386-388 The mutable status fields _lastDrainUtc, _lastSuccessUtc, _lastError, _drainState, and _backoffIndex are written by the drain timer thread inside DrainOnceAsync and read concurrently by GetStatus() / CurrentBackoff on Admin…
Core.AlarmHistorian-007 Medium Error handling & resilience src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/SqliteStoreAndForwardSink.cs:172-174 When the writer returns a wrong-cardinality result, the code throws InvalidOperationException after WriteBatchAsync has already succeeded. The events were potentially delivered to the historian, but no rows are deleted or dead-lettered…
Core.AlarmHistorian-009 Medium Design-document adherence src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/SqliteStoreAndForwardSink.cs:317-347 docs/AlarmTracking.md and the IAlarmHistorianSink contract present the SQLite queue as the durability guarantee — "Durably enqueue the event", "operator acks never block on the historian being reachable". But EnforceCapacity silently…
Core.AlarmHistorian-010 Medium Testing coverage tests/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian.Tests/SqliteStoreAndForwardSinkTests.cs The test suite covers the happy paths well (Ack/Retry/PermanentFail, capacity eviction, retention purge, ctor validation) but leaves critical paths untested: (a) no test exercises a corrupt / null-deserializing PayloadJson row, so the…
Core.ScriptedAlarms-002 Medium Correctness & logic bugs ScriptedAlarmEngine.cs:162, ScriptedAlarmEngine.cs:90 LoadAsync is written to be re-callable — it begins by calling UnsubscribeFromUpstream(), _alarms.Clear(), and _alarmsReferencing.Clear() (lines 90-92), which only makes sense if a reload is supported. But at line 162 it uncondition…
Core.ScriptedAlarms-004 Medium Concurrency & thread safety ScriptedAlarmEngine.cs:138-143, ScriptedAlarmEngine.cs:227-234 During LoadAsync, _upstream.SubscribeTag(path, OnUpstreamChange) is called inside the _evalGate critical section (line 142). If an upstream implementation delivers an initial value synchronously from inside SubscribeTag (a common p…
Core.ScriptedAlarms-005 Medium Concurrency & thread safety ScriptedAlarmEngine.cs:365-369, ScriptedAlarmEngine.cs:416-424 Dispose sets _disposed = true, disposes _shelvingTimer, and clears _alarms. A RunShelvingCheck callback already in flight on a thread-pool thread can have passed its if (_disposed) return; check (line 367) before Dispose ran,…
Core.ScriptedAlarms-007 Medium Error handling & resilience ScriptedAlarmEngine.cs:216, ScriptedAlarmEngine.cs:251, ScriptedAlarmEngine.cs:154, ScriptedAlarmEngine.cs:387 Every state mutation calls await _store.SaveAsync(...) and relies on it succeeding. If the production SQL-backed IAlarmStateStore (Stream E) throws — transient SQL outage, deadlock, timeout — the exception propagates: in ApplyAsync i…
Core.ScriptedAlarms-012 Medium Testing coverage tests/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms.Tests/ScriptedAlarmEngineTests.cs Several engine behaviours central to the module have no test coverage: (1) the 5-second shelving timer / timed-shelve auto-expiry through the engine — only the pure Part9StateMachine.ApplyShelvingCheck is tested, never `ScriptedAlarmEn…
Core.Scripting-003 Medium Security TimedScriptEvaluator.cs:9, ScriptSandbox.cs:30 There is no bound on memory a script may allocate or on the number of threads/tasks a script may spawn. The class docs acknowledge unbounded memory as "a budget concern" deferred to v3, but in-process execution means a script doing `new by…
Core.Scripting-004 Medium Correctness & logic bugs DependencyExtractor.cs:73 The walker matches tag-access calls purely by spelling — any InvocationExpressionSyntax whose member name is GetTag or SetVirtualTag is treated as a ScriptContext tag access, regardless of the receiver. A script that defines a loca…
Core.Scripting-007 Medium Error handling & resilience TimedScriptEvaluator.cs:60 RunAsync wraps the inner run in Task.Run(...) and then awaits WaitAsync(Timeout, ct). If the caller-supplied ct cancels at roughly the same time the timeout elapses, the order in which WaitAsync observes the timeout vs. the cance…
Core.Scripting-010 Medium Testing coverage tests/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Tests/ScriptSandboxTests.cs:54 The sandbox-escape test suite covers only the four obvious vectors (File / Http / Process / Reflection) as direct member-access calls. It does not test: typeof(forbidden), generic type arguments (List<FileInfo>), cast expressions to fo…
Core.VirtualTags-002 Medium Correctness & logic bugs src/Core/ZB.MOM.WW.OtOpcUa.Core.VirtualTags/VirtualTagEngine.cs:237 The cold-start guard if (!AreInputsReady(ctxCache)) return; silently abandons the evaluation when any input is null or Bad-quality. For a chained virtual tag (C depends on B depends on driver tag A), if A is still Bad at startup, B is sk…
Core.VirtualTags-003 Medium Correctness & logic bugs src/Core/ZB.MOM.WW.OtOpcUa.Core.VirtualTags/VirtualTagEngine.cs:117-120 The upstream-subscription loop in Load iterates definitions.SelectMany(d => _tags[d.Path].Reads). If definitions contains two rows with the same Path, the first registers _tags[Path] and the second overwrites it, but definitions
Core.VirtualTags-005 Medium Concurrency & thread safety src/Core/ZB.MOM.WW.OtOpcUa.Core.VirtualTags/VirtualTagSource.cs:50-64 SubscribeAsync registers the per-path engine observers first (lines 52-56), then in a second loop reads the current value and fires the initial-data callback (lines 60-64). Between those two loops an upstream change can cascade and the e…
Core.VirtualTags-008 Medium Performance & resource management src/Core/ZB.MOM.WW.OtOpcUa.Core.VirtualTags/DependencyGraph.cs:81-115 TransitiveDependentsInOrder calls TopologicalSort() (a full O(V+E) Kahn pass plus a Dictionary rank build) on every invocation, and it is invoked from CascadeAsync on every upstream change event (OnUpstreamChange). On a large graph…
Core.VirtualTags-012 Medium Testing coverage tests/Core/ZB.MOM.WW.OtOpcUa.Core.VirtualTags.Tests/ Several behaviours of the engine have no test coverage: (1) the cold-start AreInputsReady guard -- no test exercises an upstream that is null/Bad at evaluation time and asserts the resulting tag state (see Core.VirtualTags-002); (2) `ctx…
Driver.AbCip-004 Medium Correctness & logic bugs AbCipDataType.cs:51-58, LibplctagTagRuntime.cs:47-49,53 ToDriverDataType maps LInt/ULInt to DriverDataType.Int32 (a TODO comment notes the gap) and Dt to Int32. But LibplctagTagRuntime.DecodeValueAt returns an actual long for LInt/ULInt (_tag.GetInt64, `(long)_tag.GetUInt6…
Driver.AbCip-005 Medium Correctness & logic bugs AbCipDriver.cs:124-141 In InitializeAsync, when a Structure tag declares Members, the loop registers each fanned-out member into _tagsByName but the parent Structure tag itself is also left in _tagsByName (added at line 125 before the member check). A…
Driver.AbCip-006 Medium OtOpcUa conventions PlcTagHandle.cs:28-59, AbCipDriver.cs:806-807,832-833, LibplctagTagRuntime.cs:117 driver-specs.md makes the SafeHandle-wrapped native handle a non-negotiable Tier-B protection ("Wrap every libplctag handle in a SafeHandle with finalizer calling plc_tag_destroy"). The repo ships PlcTagHandle : SafeHandle for this, bu…
Driver.AbCip-009 Medium Concurrency & thread safety AbCipDriver.cs:621-648, AbCipDriver.cs:591-614 EnsureTagRuntimeAsync and EnsureParentRuntimeAsync are check-then-act on a non-thread-safe Dictionary (device.Runtimes / device.ParentRuntimes). ReadAsync is IReadable and may be invoked concurrently: the server read path, ea…
Driver.AbCip-010 Medium Error handling & resilience AbCipDriver.cs:621-648, AbCipDriver.cs:346-391 Once EnsureTagRuntimeAsync successfully creates and initializes a LibplctagTagRuntime, that runtime is cached for the lifetime of the device and never re-created on failure. If the underlying native tag enters a permanently-bad state (…
Driver.AbCip-014 Medium Testing coverage tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipStatusMapperTests.cs:28-40 AbCipStatusMapperTests.MapLibplctagStatus_maps_known_codes asserts the mapper against the same wrong integer constants (-5, -7, -14, -16, -17) the production code uses (see Driver.AbCip-002). The test locks in the bug rather than catchin…
Driver.AbCip.Cli-001 Medium Error handling & resilience src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/WriteCommand.cs:70-85 ParseValue parses every numeric Logix type with the BCL *.Parse methods (sbyte.Parse, short.Parse, int.Parse, float.Parse, ...). These throw the raw FormatException and OverflowException on bad operator input. The module's…
Driver.AbCip.Cli-002 Medium Correctness & logic bugs src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/ProbeCommand.cs:21-23; Commands/ReadCommand.cs:24-25; Commands/SubscribeCommand.cs:20-22 ProbeCommand, ReadCommand, and SubscribeCommand expose --type as a free AbCipDataType enum option with no exclusion of AbCipDataType.Structure. Only WriteCommand rejects Structure (with an explicit CommandException). Pass…
Driver.AbLegacy-002 Medium Correctness & logic bugs AbLegacyDriver.cs:368 In WriteBitInWordAsync the parent word is decoded with Convert.ToInt32(parentRuntime.DecodeValue(AbLegacyDataType.Int, ...)). LibplctagLegacyTagRuntime.DecodeValue for AbLegacyDataType.Int returns (int)_tag.GetInt16(0) - a sign-e…
Driver.AbLegacy-003 Medium Correctness & logic bugs AbLegacyAddress.cs:62-95 TryParse does not reject several malformed PCCC addresses that the XML docs imply are invalid: - A sub-element and a bit index together (T4:0.ACC/2) parse successfully even though no PCCC element supports both. - I/O/S files with a fil…
Driver.AbLegacy-004 Medium Correctness & logic bugs LibplctagLegacyTagRuntime.cs:36-37 DecodeValue for AbLegacyDataType.Bit with bitIndex == null returns _tag.GetInt8(0) != 0. A bit-file element (B3:0/0) is a single bit inside a 16-bit word; reading only the low byte (GetInt8(0)) means a Bit tag whose live bit…
Driver.AbLegacy-007 Medium Concurrency & thread safety AbLegacyDriver.cs:411-438, AbLegacyDriver.cs:386-409 EnsureTagRuntimeAsync and EnsureParentRuntimeAsync are check-then-act: device.Runtimes.TryGetValue(...) then, after await runtime.InitializeAsync, device.Runtimes[def.Name] = runtime. Dictionary is not thread-safe, and two conc…
Driver.AbLegacy-008 Medium Concurrency & thread safety AbLegacyDriver.cs:21, AbLegacyDriver.cs:138-146, AbLegacyDriver.cs:216-229 _health is a plain non-volatile reference field mutated from ReadAsync, WriteAsync (both can run on multiple threads / poll loops) and InitializeAsync/ShutdownAsync, and read by GetHealth() from yet another thread. There is no…
Driver.AbLegacy-009 Medium Error handling & resilience AbLegacyDriver.cs:41-74 InitializeAsync starts probe loops with Task.Run inside the try block. If InitializeAsync fails - or is re-entered - after some probe loops are already started, the catch only sets _health = Faulted and rethrows; it does not cancel…
Driver.AbLegacy-010 Medium Error handling & resilience AbLegacyStatusMapper.cs:26-56 MapLibplctagStatus maps the integer codes -5/-7/-14/-16/-17. These do not match the native libplctag PLCTAG_ERR_* constants (PLCTAG_ERR_TIMEOUT = -32, PLCTAG_ERR_NOT_FOUND = -22, PLCTAG_ERR_NOT_ALLOWED = -21, PLCTAG_ERR_OUT_OF_BOUNDS = -…
Driver.AbLegacy-012 Medium Design-document adherence PlcFamilies/AbLegacyPlcFamilyProfile.cs:7-54, AbLegacyDriver.cs:48-52 AbLegacyPlcFamilyProfile declares four record properties - DefaultCipPath, MaxTagBytes, SupportsStringFile, SupportsLongFile - and only LibplctagPlcAttribute is ever consumed. In particular: - DefaultCipPath is dead: the per-…
Driver.AbLegacy.Cli-001 Medium Error handling & resilience Commands/WriteCommand.cs:46, Commands/WriteCommand.cs:62-72 WriteCommand.ExecuteAsync calls ParseValue(Value, DataType) at line 46, before the try block and outside any catch. ParseValue uses short.Parse / int.Parse / float.Parse, which throw FormatException on malformed input (`-…
Driver.Cli.Common-002 Medium Correctness & logic bugs src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/SnapshotFormatter.cs:101-122 FormatStatus matches the full 32-bit status word for exact equality against the shortlist. OPC UA status codes carry sub-code/flag bits in the low 16 bits (info type, structure-changed, semantics-changed, limit bits, overflow, etc.). A d…
Driver.Cli.Common-003 Medium Concurrency & thread safety src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/DriverCommandBase.cs:51-59 ConfigureLogging assigns the process-global Serilog.Log.Logger without disposing the previously assigned logger and the library never calls Log.CloseAndFlush(). Each call creates a fresh Logger via CreateLogger() and overwrites `…
Driver.Cli.Common-005 Medium Testing coverage tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common.Tests/SnapshotFormatterTests.cs:27-37 The FormatStatus_names_well_known_status_codes [Theory] asserts 0x80060000 => "BadTimeout", which encodes the wrong spec value (see Driver.Cli.Common-001). The test passes because it validates the formatter against the same incorrect…
Driver.FOCAS-003 Medium Correctness & logic bugs FocasDriver.cs:71-79 In InitializeAsync, capability-matrix validation only runs when _devices.TryGetValue(tag.DeviceHostAddress, out var device) succeeds. A tag whose DeviceHostAddress does not match any configured device (a common config typo, e.g. a tr…
Driver.FOCAS-004 Medium OtOpcUa conventions FocasDriver.cs:374-379, WireFocasClient.cs:48-50 DiscoverAsync emits user tags with SecurityClass = tag.Writable ? SecurityClassification.Operate : SecurityClassification.ViewOnly, and FocasTagDefinition.Writable defaults to true (also defaulted to true in the factory - `t.Writ…
Driver.FOCAS-005 Medium Concurrency & thread safety FocasDriver.cs:28, FocasDriver.cs:206-215, FocasDriver.cs:261, FocasDriver.cs:274 _health is a plain (non-volatile) field mutated from multiple concurrent contexts - ReadAsync, WriteAsync, and the per-device ProbeLoopAsync can all run on different threads simultaneously (subscriptions go through `PollGroupEngine…
Driver.FOCAS-006 Medium Error handling & resilience FocasDriver.cs:859-874, WireFocasClient.cs:22-31 EnsureConnectedAsync reuses the cached IFocasClient instance across a transient disconnect: it only checks device.Client is { IsConnected: true } and otherwise calls ConnectAsync again on the same object. For a WireFocasClient wh…
Driver.FOCAS-012 Medium Testing coverage FocasDriverFactoryExtensions.cs, FocasDriver.cs:495-629 (FixedTreeLoopAsync) The unit test project does not exercise FocasDriverFactoryExtensions.CreateInstance with FixedTree / AlarmProjection / HandleRecycle config sections - which is why the config-mapping gap in Driver.FOCAS-001 was not caught. There is…
Driver.Galaxy-003 Medium Correctness & logic bugs Runtime/StatusCodeMap.cs:86 FromMxStatus returns Good whenever status.Success != 0. The intent (per the surrounding comment "Honors the success flag") is that a non-zero Success means success. But if MxStatusProxy.Success is itself a native HRESULT/return c…
Driver.Galaxy-004 Medium Correctness & logic bugs GalaxyDriver.cs:901 OnPumpDataChange reconstructs a raw OPC DA quality byte from an OPC UA StatusCode for the probe watcher: it shifts StatusCode >> 30 and maps 0->192, 1->64, _->0. The StatusCode was itself produced upstream by `StatusCodeMap.FromQ…
Driver.Galaxy-006 Medium Concurrency & thread safety GalaxyDriver.cs:848-861 OnAlarmFeedTransition picks the "owner" handle with _alarmSubscriptions.First() under _alarmHandlersLock. HashSet<T>.First() enumeration order is unspecified and unstable across mutations — when multiple alarm subscriptions are act…
Driver.Galaxy-007 Medium Concurrency & thread safety GalaxyDriver.cs:937-968 Dispose() is not synchronized against the capability methods. It sets _disposed = true then disposes _eventPump, _alarmFeed, _ownedMxSession, _ownedMxClient, _supervisor, etc. A concurrent SubscribeAsync/ReadAsync/`WriteA…
Driver.Galaxy-009 Medium Error handling & resilience GalaxyDriver.cs:354-371 StartDeployWatcher launches the watch loop with _ = _deployWatcher.StartAsync(CancellationToken.None) — a fire-and-forget with a discarded Task. StartAsync can throw synchronously (InvalidOperationException if already started); t…
Driver.Galaxy-011 Medium Performance & resource management GalaxyDriver.cs:411 GetMemoryFootprint() unconditionally returns 0 with a comment "PR 4.4 sets this from SubscriptionRegistry size" — PR 4.4 has shipped (the registry exists and is used) but the method was never updated. `IHostConnectivityProbe.GetMemoryF…
Driver.Galaxy-014 Medium Testing coverage src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy (module-wide) The reconnect/recovery path is the module's highest-risk surface and is effectively untested at the integration seam. The ReconnectSupervisor has a clean test seam (injectable reopen/replay/backoffDelay), but because nothing wires…
Driver.Historian.Wonderware-002 Medium Correctness and logic bugs Ipc/HistorianFrameHandler.cs:162, :181 HandleWriteAlarmEventsAsync dereferences req.Events.Length in both the _alarmWriter is null branch (line 162) and the catch block (line 181). MessagePack deserializes an absent or explicit-nil array field as a null reference, not `…
Driver.Historian.Wonderware-003 Medium Correctness and logic bugs Backend/HistorianDataSource.cs:320-323, :457-460 Raw and at-time reads decide whether a sample is a string or a numeric with if (!string.IsNullOrEmpty(result.StringValue) && result.Value == 0). The result.Value == 0 clause is intended to distinguish a real numeric zero from a string…
Driver.Historian.Wonderware-006 Medium Error handling and resilience Ipc/PipeServer.cs:120-128 RunAsync re-accepts connections in a while loop. If RunOneConnectionAsync throws synchronously and immediately on every iteration (for example new NamedPipeServerStream(...) fails because the pipe name is already in use, or `PipeAc…
Driver.Historian.Wonderware-009 Medium Performance and resource management Backend/HistorianDataSource.cs:382-395, Ipc/Contracts.cs:85-99 ReadAggregateAsync drains query.MoveNext into results with no upper bound, unlike ReadRawAsync, which honours maxValues / MaxValuesPerRead and breaks. ReadProcessedRequest carries no max-buckets field. A processed read over a…
Driver.Historian.Wonderware.Client-002 Medium Correctness & logic bugs WonderwareHistorianClient.cs:154-199, IAlarmHistorianSink.cs:66-74 WriteBatchAsync can never return HistorianWriteOutcome.PermanentFail. HistorianWriteOutcome defines three states (Ack, RetryPlease, PermanentFail) and the drain worker is documented to move the event to the dead-letter table on…
Driver.Historian.Wonderware.Client-005 Medium Error handling & resilience Ipc/FrameReader.cs:31-32 After reading the 4-byte length prefix, ReadFrameAsync reads the kind byte with the synchronous, blocking _stream.ReadByte() and ignores the CancellationToken. On a NamedPipeClientStream with PipeOptions.Asynchronous, a synchrono…
Driver.Historian.Wonderware.Client-007 Medium Security WonderwareHistorianClient.cs:276 ToSnapshots deserializes peer-supplied bytes with MessagePackSerializer.Deserialize<object>(dto.ValueBytes), typeless MessagePack deserialization. The object overload resolves runtime types from the wire payload. The client treats th…
Driver.Historian.Wonderware.Client-009 Medium Testing coverage tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Client.Tests/WonderwareHistorianClientTests.cs The suite covers happy paths, server-error, bad-secret, a single reconnect and health counters, but several critical paths are untested: (1) ReadAtTimeAsync with a partial/reordered sidecar reply, the contract-alignment case from finding…
Driver.Modbus-002 Medium Correctness & logic bugs ModbusDriver.cs:127-186 ShutdownAsync never clears _tagsByName, and InitializeAsync repopulates it with _tagsByName[t.Name] = t (ModbusDriver.cs:134) without clearing first. ReinitializeAsync calls ShutdownAsync then InitializeAsync. Because `_opt…
Driver.Modbus-004 Medium Performance & resource management ModbusDriver.cs:1468-1473 DisposeAsync() only disposes _transport. Unlike ShutdownAsync, it does not cancel/dispose _probeCts or _reprobeCts, nor dispose _poll (the PollGroupEngine). A caller that uses await using or using without first calling `S…
Driver.Modbus-005 Medium Correctness & logic bugs ModbusDriver.cs:777-798,323-330 ReadRegisterBlockAsync and ReadBitBlockAsync index resp[1] and call Buffer.BlockCopy(resp, 2, ..., resp[1]) with no bounds validation. ModbusTcpTransport.SendOnceAsync validates only the MBAP length field and the exception high-b…
Driver.Modbus-006 Medium Error handling & resilience ModbusDriver.cs:514-524,532-550 RunReprobeOnceForTestAsync reads _transport once at the top (var transport = _transport ?? throw ...). If ShutdownAsync runs (setting _transport = null and disposing it) while a re-probe pass is mid-iteration, the loop keeps issu…
Driver.Modbus.Addressing-002 Medium Correctness & logic bugs ModbusAddressParser.cs:86-94 In the 3-field disambiguation, an empty 3rd field (40001:F:) reaches parts[2].All(char.IsDigit). Enumerable.All returns true for an empty sequence, so the empty string is classified as a valid-shaped array count, assigned to `countPa…
Driver.Modbus.Addressing-003 Medium Correctness & logic bugs ModbusAddressParser.cs:405-406, ModbusAddressParser.cs:128 LooksLikeByteOrderToken classifies any 4-letter token as a byte-order token. A 3-field address whose 3rd field is a 4-letter type-like token (e.g. 40001:S:BOOL) is routed into TryParseByteOrder, producing the misleading diagnostic "U…
Driver.Modbus.Addressing-004 Medium Correctness & logic bugs ModbusAddressParser.cs:182-194 The bit suffix is stripped using text.IndexOf('.') — the first dot. An input such as 40001.5.3 produces a bit text of "5.3", rejected by byte.TryParse with the generic "Bit index must be 0..15" message. A Modicon-style decimal-point…
Driver.Modbus.Addressing-005 Medium Error handling & resilience ModbusAddressParser.cs:200-213 TryParseRegionAndOffset tries family-native, then mnemonic, then Modicon. When all three fail it returns false with whatever error the Modicon parser last wrote (comment: "the Modicon error is the more specific diagnostic"). For a non-Ge…
Driver.Modbus.Addressing-008 Medium Testing coverage tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.Tests/ Several edge cases of the address arithmetic are untested or asserted wrong: (a) DL205 system V-memory mapping is tested only with the incorrect expected value (ModbusFamilyParserTests.cs:20, see finding -001); (b) there is no test for `…
Driver.Modbus.Cli-001 Medium Correctness & logic bugs src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/Commands/SubscribeCommand.cs:43-51 SubscribeCommand synthesises its ModbusTagDefinition with only Name, Region, Address, DataType, Writable, and ByteOrder — it never exposes or passes --bit-index, --string-length, or --string-byte-order. A user running…
Driver.Modbus.Cli-002 Medium Correctness & logic bugs src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/Commands/WriteCommand.cs:54-89 WriteCommand rejects read-only regions (DiscreteInputs / InputRegisters) but does not validate that --type is meaningful for the Coils region. write -r Coils -a 5 -t UInt16 -v 42 builds a Coils tag with DataType = UInt16; t…
Driver.OpcUaClient-006 Medium Concurrency & thread safety OpcUaClientDriver.cs:1330-1359 OnReconnectComplete mutates Session (line 1347) directly from the reconnect-handler callback thread with no synchronization against ReadAsync/WriteAsync/DiscoverAsync/ShutdownAsync. Session is a plain auto-property with no memory barrier…
Driver.OpcUaClient-007 Medium Concurrency & thread safety OpcUaClientDriver.cs:1374, :1376-1383, :508 Two disposal races. (1) Dispose() does DisposeAsync().AsTask().GetAwaiter().GetResult(), synchronous blocking on async work. The Galaxy stability review (driver-stability.md, the 2026-04-13 findings) explicitly calls out sync-over-async…
Driver.OpcUaClient-008 Medium Error handling & resilience OpcUaClientDriver.cs:1092-1099 AcknowledgeAsync issues the batched CallAsync and then catches all exceptions with a best-effort empty catch; it also never inspects the per-call results in the success path (_ = await session.CallAsync(...)). An alarm acknowledgment the…
Driver.OpcUaClient-009 Medium Error handling & resilience OpcUaClientDriver.cs:560-564 WriteAsync's catch block fans out BadCommunicationError across the whole batch on any exception. Writes are non-idempotent by default (IWritable remarks, decision #44/#45): a timeout exception may fire after the upstream server already app…
Driver.OpcUaClient-010 Medium Correctness & logic bugs OpcUaClientDriver.cs:823-824 MapUpstreamDataType maps DataTypeIds.Byte (the OPC UA unsigned 8-bit type) to DriverDataType.Int16. Byte should map to an unsigned driver type (UInt16 is the smallest unsigned available, matching how SByte belongs with the signed family).…
Driver.OpcUaClient-012 Medium Security OpcUaClientDriver.cs:210-217 When AutoAcceptCertificates is true the driver registers a CertificateValidation handler that accepts only StatusCodes.BadCertificateUntrusted. A self-signed or otherwise untrusted server certificate frequently fails validation with a diff…
Driver.OpcUaClient-013 Medium Performance & resource management OpcUaClientDriver.cs:436-437 GetMemoryFootprint() is hard-coded to return 0 and FlushOptionalCachesAsync is a no-op Task.CompletedTask. docs/v2/driver-stability.md section "In-process only (Tier A/B)" makes per-instance allocation tracking a contract requirement, and…
Driver.OpcUaClient-015 Medium Testing coverage tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/*, tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/OpcUaClientSmokeTests.cs Unit-test coverage is solid for the pure mappers (MapSeverity, MapUpstreamDataType, MapSecurityPolicy, MapAggregateToNodeId, BuildCertificateIdentity, ResolveEndpointCandidates) and for "throws before init" guards, but the highest-risk beh…
Driver.S7-002 Medium Correctness & logic bugs S7Driver.cs:350 MapDataType collapses S7DataType.UInt32 to DriverDataType.Int32. UInt32 values above int.MaxValue (2^31-1) wrap to negative when surfaced to the OPC UA client, silently corrupting the value. The inline comment only flags Int64/UInt64 as "w…
Driver.S7-004 Medium OtOpcUa conventions S7Driver.cs (whole file) The driver performs no logging. CLAUDE.md Library Preferences mandate Serilog with a rolling daily file sink. Every error path is an empty catch block (Initialize cleanup line 130, ShutdownAsync lines 142/149/153, ProbeLoop line 483, PollL…
Driver.S7-008 Medium Error handling & resilience S7Driver.cs:286 WriteAsync catch ladder is coarser than ReadAsync and loses information. The generic catch (Exception) maps everything - socket errors, timeouts, OverflowException from Convert.ToInt16 of an out-of-range value, NullReferenceException from…
Driver.S7-012 Medium Design-document adherence S7DriverOptions.cs:59, S7Driver.cs:457 S7ProbeOptions.ProbeAddress is configured (default "MW0"), documented at length ("the driver runs a tick loop that issues a cheap read against S7ProbeOptions.ProbeAddress"), surfaced in the factory DTO (S7ProbeDto.ProbeAddress), and parsed…
Driver.S7-014 Medium Testing coverage tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/ Test coverage has notable gaps for the driver behavioural core: (1) no test exercises the ReadOneAsync type-reinterpret switch (Int16 from ushort, Int32 from uint, Float32 from UInt32 bits) - the most logic-heavy method in the driver is un…
Driver.S7.Cli-001 Medium Error handling & resilience src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli/Commands/WriteCommand.cs:65-80 WriteCommand.ParseValue parses numeric and DateTime values with the raw BCL parsers (short.Parse, float.Parse, DateTime.Parse, etc.). On malformed input these throw FormatException / OverflowException, which are not `CliFx.…
Driver.S7.Cli-002 Medium Design-document adherence src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli/Commands/ReadCommand.cs:22-29, Commands/WriteCommand.cs:21-33, Commands/SubscribeCommand.cs:18-21; docs/Driver.S7.Cli.md:70-73,80-81 The --type option help text on read, write, and subscribe advertises the full S7DataType set (Int64 / UInt64 / Float64 / String / DateTime), and docs/Driver.S7.Cli.md shows a worked read ... -t String --string-length 80 exa…
Driver.S7.Cli-003 Medium Error handling & resilience src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli/Commands/ProbeCommand.cs:38-50 ProbeCommand XML doc and the Driver.S7.Cli.md "fastest is the device talking" framing say the probe "connects ... prints health" and "surfaces BadNotSupported" when PUT/GET is disabled. But when the PLC is unreachable (connection ref…
Driver.TwinCAT-003 Medium Correctness & logic bugs AdsTwinCATClient.cs:264-281, 283-300 MapToClrType has a _ => typeof(int) fallthrough and ConvertForWrite has a _ => throw NotSupportedException fallthrough. TwinCATDataType.Structure is a declared enum member, and a config-supplied tag can carry `DataType: "Structur…
Driver.TwinCAT-005 Medium OtOpcUa conventions TwinCATDriver.cs (whole file), AdsTwinCATClient.cs (whole file) The driver performs no logging. CLAUDE.md Library Preferences mandate Serilog with a rolling daily file sink. Connect failures, ADS error codes, symbol-browse failures (DiscoverAsync swallows them in a bare catch), notification-regis…
Driver.TwinCAT-009 Medium Concurrency & thread safety TwinCATDriver.cs:80-99, 41-72, 366-388 ShutdownAsync mutates _devices, _tagsByName, and _nativeSubs with no synchronization while ReadAsync/WriteAsync/SubscribeAsync may be iterating or indexing those same plain Dictionary<> instances on other threads (`_devices…
Driver.TwinCAT-010 Medium Error handling & resilience AdsTwinCATClient.cs:178-195 BrowseSymbolsAsync checks cancellationToken.IsCancellationRequested and does yield break (a clean completion) rather than throwing OperationCanceledException. DiscoverAsync (TwinCATDriver.cs:274) explicitly has `catch (Operatio…
Driver.TwinCAT-011 Medium Error handling & resilience TwinCATStatusMapper.cs:29-42 ADS error-code mapping has gaps and an inconsistency versus docs/v2/driver-specs.md section 6. The spec documents symbol-not-found as 0x0701 (1793 decimal) and symbol-version-changed as 0x0702 (1794 decimal). MapAdsError maps decimal 1…
Driver.TwinCAT-012 Medium Performance & resource management TwinCATDriver.cs:102, AdsTwinCATClient.cs:178-195 GetMemoryFootprint() returns a hard-coded 0. docs/v2/driver-stability.md section "In-process only (Tier A/B) — driver-instance allocation tracking" requires the footprint to reflect "bytes attributable to their own caches (symbol cache…
Server-003 Medium Correctness & logic bugs src/Server/ZB.MOM.WW.OtOpcUa.Server/Phase7/RingBufferHistoryWriter.cs:96-119 ReadRawAsync's XML doc claims "newest-first," but TagRingBuffer.Snapshot() returns oldest-to-newest and the loop preserves that order — so results are oldest-first. Also maxValuesPerNode is capped against total buffer size before t…
Server-005 Medium Concurrency & thread safety src/Server/ZB.MOM.WW.OtOpcUa.Server/Alarms/AlarmConditionService.cs:166, src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs:303-311 OnValueChanged raises TransitionRaised on the value-change thread; the subscriber OnAlarmServiceTransition drives ConditionSink.OnTransitionalarm.ReportEvent. DriverNodeManager.Dispose detaches the handler but does not synch…
Server-007 Medium Error handling & resilience src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/OpcUaApplicationHost.cs:179-183 HealthEndpointsHost is built without a configDbHealthy delegate, so the default () => true is used — /healthz always reports configDbReachable = true and never 503s on a DB outage. _staleConfigFlag is also never supplied by `Pr…
Server-010 Medium Security src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/OpcUaServerOptions.cs:59, src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/OpcUaApplicationHost.cs:284-291 AutoAcceptUntrustedClientCertificates defaults to true (Program.cs reads ?? true). BuildConfiguration wires a handler that accepts any client cert failing with BadCertificateUntrusted. A deployment that forgets to flip the flag…
Server-011 Medium Security src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/OpcUaApplicationHost.cs:322-346 BuildUserTokenPolicies advertises a UserName token policy only when SecurityProfile == Basic256Sha256SignAndEncrypt && Ldap.Enabled. With the default SecurityProfile = None and Ldap.Enabled = true, the LDAP authenticator is wired…
Server-013 Medium Design-document adherence src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/OpcUaServerOptions.cs:9-19, src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/OpcUaApplicationHost.cs:296-346, src/Server/ZB.MOM.WW.OtOpcUa.Server/Program.cs:89 docs/security.md documents 7 transport security profiles and CLAUDE.md references a SecurityProfileResolver. The code's OpcUaSecurityProfile enum has only None and Basic256Sha256SignAndEncrypt; BuildSecurityPolicies adds a po…
Admin-010 Low OtOpcUa conventions Components/App.razor:9,16 App.razor loads Bootstrap CSS and JS from the cdn.jsdelivr.net CDN. admin-ui.md section "Tech Stack" specifies "Bootstrap 5 vendored under wwwroot/lib/bootstrap/" precisely so the Admin app has no third-party runtime dependency. A…
Admin-011 Low Concurrency & thread safety Hubs/FleetStatusPoller.cs:24-26,98-103 FleetStatusPoller keeps three plain Dictionary<> fields (_last, _lastRole, _lastResilience) mutated from PollOnceAsync. The poller ExecuteAsync loop is single-threaded so the steady-state poll path is safe, but ResetCache()
Admin-012 Low Design-document adherence Services/EquipmentCsvImporter.cs:18-19,33-37,229,232 EquipmentCsvImporter declares EquipmentId as a required CSV column and parses it into a required field. admin-ui.md section "Equipment CSV import" (revised after adversarial review finding #4) is explicit: "No EquipmentId column…
Analyzers-002 Low Correctness & logic bugs src/Tooling/ZB.MOM.WW.OtOpcUa.Analyzers/UnwrappedCapabilityCallAnalyzer.cs:46-50,130 AlarmSurfaceInvoker is listed in WrapperTypes, but AlarmSurfaceInvoker's public methods (SubscribeAsync, UnsubscribeAsync, AcknowledgeAsync) take no lambda arguments at all — callers pass IReadOnlyList<...> / `IAlarmSubscript…
Analyzers-003 Low Error handling & resilience src/Tooling/ZB.MOM.WW.OtOpcUa.Analyzers/UnwrappedCapabilityCallAnalyzer.cs:80,114-116 IsInsideWrapperLambda is passed context.Operation.SemanticModel and returns false when that model is null. A false return means "not wrapped", so a null semantic model produces a false-positive diagnostic rather than silently ski…
Analyzers-004 Low Performance & resource management src/Tooling/ZB.MOM.WW.OtOpcUa.Analyzers/UnwrappedCapabilityCallAnalyzer.cs:95-112 ImplementsGuardedInterface runs on every invocation operation in the compilation (every keystroke in the IDE). For each candidate it allocates via AllInterfaces.Concat(new[] { method.ContainingType }), builds a fully-qualified display…
Analyzers-005 Low Design-document adherence src/Tooling/ZB.MOM.WW.OtOpcUa.Analyzers/UnwrappedCapabilityCallAnalyzer.cs:33-43 CapabilityInvoker's XML doc (src/Core/.../Resilience/CapabilityInvoker.cs:15-17) enumerates the routed capability surface as IReadable, IWritable, ITagDiscovery, ISubscribable, IHostConnectivityProbe, IAlarmSource, and all…
Analyzers-007 Low Documentation & comments src/Tooling/ZB.MOM.WW.OtOpcUa.Analyzers/UnwrappedCapabilityCallAnalyzer.cs:21-26 The <remarks> block states the analyzer "matches by receiver-interface identity using Roslyn's semantic model, not by method name". This is accurate for the guarded-call detection (ImplementsGuardedInterface uses symbols), but the wrap…
Client.CLI-002 Low Correctness & logic bugs Commands/SubscribeCommand.cs:129-137 The summary computes neverWentBad as every target whose node-id key is absent from the everBad dictionary. A node that received no update at all is also absent from everBad, so it is counted in neverWentBad and printed under the he…
Client.CLI-003 Low Correctness & logic bugs Commands/BrowseCommand.cs:29-30, Commands/SubscribeCommand.cs:20-27, Commands/AlarmsCommand.cs:28-29, Commands/HistoryReadCommand.cs:42-43 Numeric command options accept any value with no range validation. --depth, --interval, --max-depth, --max, and the history --interval can all be supplied as 0 or a negative number. A negative --depth/--max-depth silently d…
Client.CLI-004 Low OtOpcUa conventions Commands/SubscribeCommand.cs:13-37 SubscribeCommand is the only command in the module whose constructor and all [CommandOption] properties have no XML doc comments. Every other command (ConnectCommand, ReadCommand, WriteCommand, BrowseCommand, AlarmsCommand, `…
Client.CLI-006 Low Error handling & resilience Commands/HistoryReadCommand.cs:73, Commands/HistoryReadCommand.cs:76, Helpers/NodeIdParser.cs:39 Operator input-format errors surface as raw .NET exceptions rather than clean CLI errors. An unparseable start/end value throws FormatException straight out of DateTime.Parse; an invalid node id throws FormatException/`ArgumentExcept…
Client.CLI-007 Low Performance & resource management CommandBase.cs:112-123 ConfigureLogging builds a new Serilog LoggerConfiguration, creates a logger, and assigns it to the static Log.Logger without disposing the previously assigned logger. For a single CLI invocation this leaks at most one logger and the…
Client.CLI-008 Low Documentation & comments docs/Client.CLI.md:158-217 docs/Client.CLI.md is stale relative to the code at this commit. (1) The subscribe command section documents only -n and -i, but the code (SubscribeCommand) also exposes -r/--recursive, --max-depth, -q/--quiet, --duration
Client.CLI-009 Low Code organization & conventions Commands/SubscribeCommand.cs:66-165, Commands/AlarmsCommand.cs:52-91 Both long-running commands attach an event handler (service.DataChanged += ..., service.AlarmEvent += ...) with a lambda and never detach it. Because the handler closes over console, the captured console and the closure remain refere…
Client.CLI-010 Low Testing coverage tests/Client/ZB.MOM.WW.OtOpcUa.Client.CLI.Tests/SubscribeCommandTests.cs The new SubscribeCommand capabilities are largely untested. The four SubscribeCommandTests cover only single-node subscribe, unsubscribe-on-cancel, disconnect-in-finally, and the subscription message. There is no test for the `--recurs…
Client.Shared-003 Low Correctness & logic bugs Adapters/DefaultSessionAdapter.cs:76, Adapters/DefaultSessionAdapter.cs:273 WriteValueAsync returns response.Results[0] and CallMethodAsync reads result.Results[0] without first checking the Results collection is non-empty. A malformed or service-level-faulted response (empty Results alongside a servic…
Client.Shared-004 Low OtOpcUa conventions Adapters/DefaultSessionAdapter.cs:228, Adapters/DefaultSessionAdapter.cs:121, Adapters/DefaultSessionAdapter.cs:172 CloseAsync, HistoryReadRawAsync, and HistoryReadAggregateAsync are declared async Task but call the synchronous Session.Close() / Session.HistoryRead(...) APIs and contain no await. The history methods run a blocking synchron…
Client.Shared-009 Low Error handling & resilience / Documentation & comments OpcUaClientService.cs:302-322 AcknowledgeAlarmAsync is typed Task<StatusCode> and its XML doc implies the returned code reports the ack outcome, but the method unconditionally return StatusCodes.Good. The actual failure path is `DefaultSessionAdapter.CallMethodAs…
Client.Shared-010 Low Performance & resource management Models/ConnectionSettings.cs:48, OpcUaClientService.cs:408-417 ConnectionSettings.CertificateStorePath is initialized to ClientStoragePaths.GetPkiPath() as a property initializer, so every ConnectionSettings instantiation runs Environment.GetFolderPath + Path.Combine and, on the first call p…
Client.Shared-011 Low Testing coverage tests/Client/ZB.MOM.WW.OtOpcUa.Client.Shared.Tests/OpcUaClientServiceTests.cs The test suite is solid for the happy paths, connection lifecycle, and single-failover behavior. Gaps relative to the findings above: (a) no test exercises concurrent SubscribeAsync/failover to expose the _activeDataSubscriptions race…
Client.UI-003 Low OtOpcUa conventions ZB.MOM.WW.OtOpcUa.Client.UI.csproj:20-21, Program.cs:14-20 The csproj references Serilog and Serilog.Sinks.Console, and docs/Client.UI.md lists Serilog as the logging technology, but no source file in the module uses Serilog. Program.BuildAvaloniaApp() uses Avalonia's LogToTrace() and th…
Client.UI-004 Low OtOpcUa conventions Views/MainWindow.axaml.cs:125-138 OnBrowseCertPathClicked uses OpenFolderDialog, which is obsolete in Avalonia 11.x (the version pinned in the csproj). The supported replacement is the StorageProvider API (StorageProvider.OpenFolderPickerAsync). Using the obsolete…
Client.UI-006 Low Error handling & resilience ViewModels/MainWindowViewModel.cs:244-252, ViewModels/AlarmsViewModel.cs:88-112, ViewModels/SubscriptionsViewModel.cs:79-94 Many catch blocks swallow exceptions silently with an empty body and only a comment (// Redundancy info not available, // Subscribe failed, // Subscription failed; no item added, and others). When a subscribe, alarm-subscribe, or red…
Client.UI-009 Low Design-document adherence ViewModels/HistoryViewModel.cs:44-54 HistoryViewModel.AggregateTypes exposes eight entries: null (Raw) plus Average, Minimum, Maximum, Count, Start, End, and StandardDeviation. docs/Client.UI.md ("Query Options" table) lists only "Raw (default), Average, Minimum, Maxi…
Client.UI-010 Low Code organization & conventions Controls/DateTimeRangePicker.axaml.cs:33-37, Controls/DateTimeRangePicker.axaml.cs:70-80 DateTimeRangePicker declares MinDateTimeProperty / MaxDateTimeProperty styled properties with public CLR accessors, but neither is read anywhere in the control. TryParseDateTime, OnStartLostFocus, and OnEndLostFocus never clamp…
Client.UI-011 Low Documentation & comments Views/MainWindow.axaml:81, Services/JsonSettingsService.cs:11-15 The certificate-store-path TextBox watermark reads (default: AppData/LmxOpcUaClient/pki), referencing the legacy pre-task-#208 folder name. Per CLAUDE.md / docs/Client.UI.md the canonical path is now {LocalAppData}/OtOpcUaClient/
Configuration-004 Low OtOpcUa conventions src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Enums/NodePermissions.cs:8, src/Core/ZB.MOM.WW.OtOpcUa.Configuration/OtOpcUaConfigDbContext.cs:417 NodePermissions is declared [Flags] enum ... : uint, while its XML doc and NodeAcl.PermissionFlags' doc both say "stored as int", and ConfigureNodeAcl uses HasConversion<int>() — a uintint conversion. Only bits 011 are used…
Configuration-005 Low Concurrency & thread safety src/Core/ZB.MOM.WW.OtOpcUa.Configuration/LocalCache/LiteDbConfigCache.cs:50 PutAsync performs a non-atomic find-then-insert/update. Two concurrent PutAsync calls for the same (ClusterId, GenerationId) can both observe existing is null and both Insert, producing two rows for one generation. The constructo…
Configuration-007 Low Error handling & resilience src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Apply/GenerationApplier.cs:44 ApplyPass wraps each callback in catch (Exception ex). This swallows OperationCanceledException — a cancellation during a callback is recorded as just another entity error string and the applier keeps walking the remaining passes ins…
Configuration-010 Low Security src/Core/ZB.MOM.WW.OtOpcUa.Configuration/LocalCache/ResilientConfigReader.cs:81 On central-DB read failure the warning log records the full exception object. Callers pass arbitrary centralFetch delegates; if any delegate closes over a connection string, an exception thrown from it (or a SqlException carrying serve…
Configuration-011 Low Testing coverage src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Apply/GenerationApplier.cs:7, src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftValidator.cs:60 The companion test project covers the cache, schema compliance, stored procedures, and DraftValidator well, but two flagged behaviours are not pinned: (a) GenerationApplier ordering/cancellation when a Removed callback fails — no test…
Core-004 Low OtOpcUa conventions src/Core/ZB.MOM.WW.OtOpcUa.Core/Hosting/DriverHost.cs:55,72,87 DriverHost is a library type whose async calls (driver.InitializeAsync, driver.ShutdownAsync) do not use ConfigureAwait(false), whereas the sibling CapabilityInvoker and AlarmSurfaceInvoker in the same module consistently do. T…
Core-008 Low Error handling & resilience src/Core/ZB.MOM.WW.OtOpcUa.Core/OpcUa/GenericDriverNodeManager.cs:42-64 The XML summary of BuildAddressSpaceAsync states "Driver exceptions are isolated per decision #12 — the driver's subtree is marked Faulted, but other drivers remain available." The method body contains no such isolation: an exception fro…
Core-009 Low Performance & resource management src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/CapabilityInvoker.cs:121-128 ExecuteWriteAsync calls _optionsAccessor() three times for a single non-idempotent write (once for the with expression, once inside the dictionary initializer for .Resolve(...), plus the discarded base). On the per-write hot path i…
Core-010 Low Code organization & conventions src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResilienceOptions.cs:45-52 DriverResilienceOptions.Resolve indexes the tier-default dictionary directly (defaults[capability]) with no fallback. Any future addition to DriverCapability that is not also added to all three tier tables in GetTierDefaults will m…
Core-011 Low Testing coverage src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/PermissionTrieBuilder.cs:58-75 PermissionTrieBuilder.Descend has a two-branch behaviour: with a scopePaths lookup it descends the real hierarchy; without one it falls back to placing every non-cluster row directly under the root keyed by ScopeId ("works for determ…
Core-012 Low Documentation & comments src/Core/ZB.MOM.WW.OtOpcUa.Core/Stability/WedgeDetector.cs:26, src/Core/ZB.MOM.WW.OtOpcUa.Core/Observability/DriverHealthReport.cs:11-22 Two stale doc comments. (1) WedgeDetector — the <summary> above the constructor reads "Whether the driver reported itself DriverState.Healthy at construction." The constructor takes only a TimeSpan threshold and the detector is doc…
Core.Abstractions-004 Low Concurrency & thread safety src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverTypeRegistry.cs:23-40 Register performs a check-then-act sequence (snapshot.ContainsKey then build next then Interlocked.Exchange) that is not atomic. Two threads registering concurrently can both pass the duplicate check and both build a next diction…
Core.Abstractions-005 Low Error handling & resilience src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/PollGroupEngine.cs:90,99 Both the initial-poll and steady-state catch blocks use a bare catch { } that swallows every exception type, including non-transient programmer errors such as NullReferenceException and ArgumentOutOfRangeException (see Core.Abstracti…
Core.Abstractions-006 Low Code organization & conventions src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IHistoryProvider.cs:63,84-86, src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/Historian/IHistorianDataSource.cs:30,63 The two history-read surfaces use inconsistent integer types for the same "maximum rows" concept. IHistoryProvider.ReadRawAsync and IHistorianDataSource.ReadRawAsync take uint maxValuesPerNode, but ReadEventsAsync (on both interfac…
Core.Abstractions-007 Low Testing coverage tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/PollGroupEngineTests.cs PollGroupEngine is the only behavioural (non-DTO) type in the module and its tests, while solid for the happy paths, miss two paths that this review identifies as defect-prone: (a) no test exercises an array-valued tag whose contents are…
Core.Abstractions-008 Low Documentation & comments src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverHealth.cs:9, src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IHistoryProvider.cs:39-43,65-69 Two XML-doc inaccuracies: 1. DriverHealth.LastError is documented as "Most recent error message; null when state is Healthy." The DriverState enum also defines Degraded, Reconnecting, and Faulted states, all of which carry an err…
Core.AlarmHistorian-008 Low Performance & resource management src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/SqliteStoreAndForwardSink.cs:107-127,255-278 Each EnqueueAsync (one per alarm transition — a hot path on a busy plant) opens a connection, runs EnforceCapacity (a COUNT(*) over the queue table on every single enqueue), serializes JSON, inserts, and closes the connection. The un…
Core.AlarmHistorian-011 Low Documentation & comments src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/IAlarmHistorianSink.cs:5-9,76, AlarmHistorianEvent.cs:20 Several doc-comments reference the retired v1 architecture. The IAlarmHistorianSink summary says ingestion "routes through Galaxy.Host's pipe" and IAlarmHistorianWriter says "Stream G wires this to the Galaxy.Host IPC client", but `doc…
Core.ScriptedAlarms-003 Low Documentation & comments ScriptedAlarmEngine.cs:343, docs/ScriptedAlarms.md:107 docs/ScriptedAlarms.md (Composition step 3) and the OnUpstreamChange comment ("Fire-and-forget so driver-side dispatch isn't blocked", line 225-226) describe the OnEvent emission path as non-blocking / fire-and-forget. In the code, `…
Core.ScriptedAlarms-006 Low Concurrency & thread safety ScriptedAlarmEngine.cs:232, ScriptedAlarmEngine.cs:369 OnUpstreamChange and RunShelvingCheck both launch fire-and-forget tasks (_ = ReevaluateAsync(...), _ = ShelvingCheckAsync(...)) with CancellationToken.None. There is no tracking of these in-flight tasks, so Dispose cannot await…
Core.ScriptedAlarms-008 Low Performance & resource management Part9StateMachine.cs:261-268 AppendComment copies the entire existing comment list into a new List on every audit-producing transition (ack, confirm, shelve, unshelve, enable, disable, add-comment, auto-unshelve). The Comments list is append-only and unbounded —…
Core.ScriptedAlarms-009 Low Performance & resource management ScriptedAlarmEngine.cs:309-315, ScriptedAlarmEngine.cs:271 BuildReadCache allocates a fresh Dictionary<string, DataValueSnapshot> on every predicate evaluation, i.e. on every upstream tag change for every referencing alarm. On a busy line where many tags feeding many alarms change frequently,…
Core.ScriptedAlarms-010 Low Design-document adherence ScriptedAlarmEngine.cs:325-336, AlarmPredicateContext.cs:33-40, MessageTemplate.cs:47 Quality handling is inconsistent across the three places that inspect a DataValueSnapshot.StatusCode. AreInputsReady (engine, line 333) treats only outright Bad (bit 31) as not-ready, so an Uncertain-quality input is fed to the predica…
Core.ScriptedAlarms-011 Low Code organization & conventions Part9StateMachine.cs:275 TransitionResult.NoOp(state, reason) takes a reason string parameter that is documented in the calling code as a diagnostic ("disabled — predicate result ignored", "already acknowledged", etc.) but the factory method silently discards…
Core.Scripting-005 Low Correctness & logic bugs DependencyExtractor.cs:97 A raw string literal token passed as the tag path (a raw triple-quote literal) tokenizes as SingleLineRawStringLiteralToken / MultiLineRawStringLiteralToken, not StringLiteralToken. The check `literal.Token.IsKind(SyntaxKind.StringLi…
Core.Scripting-006 Low Concurrency & thread safety CompiledScriptCache.cs:55 On a failed compile the catch block calls _cache.TryRemove(key, out _) without a value comparison. If two threads race a miss for the same bad source, both observe the same faulted Lazy and throw, and both call TryRemove(key). If a…
Core.Scripting-008 Low Performance & resource management CompiledScriptCache.cs:34, ScriptEvaluator.cs:34 CompiledScriptCache has no capacity bound (acknowledged in the class remarks) and no eviction. Each cached ScriptEvaluator holds a Roslyn ScriptRunner<T> delegate, which keeps the dynamically emitted script assembly loaded for the pr…
Core.Scripting-009 Low Design-document adherence ForbiddenTypeAnalyzer.cs:45 The Phase 7 plan decision #6 (docs/v2/implementation/phase-7-scripting-and-alarming.md) enumerates the forbidden surface as "No HttpClient / File / Process / reflection". ForbiddenTypeAnalyzer actually denies a broader set — `System.Th…
Core.Scripting-011 Low Testing coverage tests/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Tests/ Two source files have no direct test coverage: ScriptContext (Deadband static helper is exercised only indirectly through ScriptSandboxTests, and not for its boundary tolerance behaviour) and ScriptSandbox.Build itself (the `Argu…
Core.VirtualTags-004 Low Correctness & logic bugs src/Core/ZB.MOM.WW.OtOpcUa.Core.VirtualTags/VirtualTagEngine.cs:349 CoerceResult's switch has a default arm (_ => raw) that returns the script's raw return value uncoerced for any DriverDataType not in the explicit list (e.g. an array type, Byte, or a future enum member). The resulting `DataValueSnap…
Core.VirtualTags-006 Low Concurrency & thread safety src/Core/ZB.MOM.WW.OtOpcUa.Core.VirtualTags/VirtualTagEngine.cs:177-182, :395-401 Subscribe does _observers.GetOrAdd(path, _ => []) then lock (list) { list.Add(observer); }. When Unsub.Dispose removes the last observer, the now-empty List is left in _observers and the dictionary entry is never removed. For a l…
Core.VirtualTags-007 Low Error handling & resilience src/Core/ZB.MOM.WW.OtOpcUa.Core.VirtualTags/TimerTriggerScheduler.cs:58 Tick calls _engine.EvaluateOneAsync(p, _cts.Token).GetAwaiter().GetResult(), blocking the System.Threading.Timer callback thread (a thread-pool thread) for the full duration of the evaluation. Because EvaluateInternalAsync serialis…
Core.VirtualTags-009 Low Performance & resource management src/Core/ZB.MOM.WW.OtOpcUa.Core.VirtualTags/DependencyGraph.cs:64-65, :72-73 DirectDependencies and DirectDependents allocate a fresh empty HashSet<string> on every call for an unregistered node. DirectDependents is called inside the TopologicalSort Kahn loop and the CascadeAsync DFS, so for a graph wit…
Core.VirtualTags-010 Low Documentation & comments src/Core/ZB.MOM.WW.OtOpcUa.Core.VirtualTags/ITagUpstreamSource.cs:18, VirtualTagContext.cs:30, VirtualTagDefinition.cs:28 Several XML docs reference component names that do not exist in the codebase. ITagUpstreamSource XML doc says the subscription path "feeds the engine's ChangeTriggerDispatcher" -- there is no ChangeTriggerDispatcher; the actual path is `…
Core.VirtualTags-011 Low Code organization & conventions src/Core/ZB.MOM.WW.OtOpcUa.Core.VirtualTags/VirtualTagEngine.cs:404-409 VirtualTagState records a Writes set (the ctx.SetVirtualTag targets extracted by DependencyExtractor), but nothing in the engine reads it -- it is captured at Load and never used. Declared write targets are not validated against th…
Core.VirtualTags-013 Low Documentation & comments src/Core/ZB.MOM.WW.OtOpcUa.Core.VirtualTags/DependencyGraph.cs:266-270 DependencyCycleException.BuildMessage renders each cycle as string.Join(" -> ", c) + " -> " + c[0], presenting the SCC member list as a traversable edge path that loops back to its first element. Tarjan's algorithm returns the members…
Driver.AbCip-007 Low OtOpcUa conventions AbCipDriver.cs (whole file), AbCipAlarmProjection.cs, LibplctagTagRuntime.cs CLAUDE.md Library Preferences mandate Serilog with a rolling daily file sink. The driver has no logging at all: no ILogger/Serilog dependency is injected or used. Failure paths instead swallow exceptions into the _health string (`Rea…
Driver.AbCip-011 Low Error handling & resilience AbCipDriver.cs:144-152, AbCipDriverOptions.cs:131-143 InitializeAsync only starts probe loops when _options.Probe.Enabled is true AND Probe.ProbeTagPath is non-blank. When Probe.Enabled is true (the default) but ProbeTagPath is null (also the default; the doc comment says "PR 8 wire…
Driver.AbCip-012 Low Performance & resource management LibplctagTemplateReader.cs:15-35, AbCipDriver.cs:88-92 LibplctagTemplateReader is created per FetchUdtShapeAsync call, and each call constructs a fresh libplctag Tag for the @udt pseudo-tag, initializes it (a CIP connection handshake), reads, and disposes it. There is no reuse of the `Ta…
Driver.AbCip-013 Low Design-document adherence AbCipDriverOptions.cs:70-73, PlcFamilies/AbCipPlcFamilyProfile.cs:13-19, LibplctagTagRuntime.cs:16-27 driver-specs.md specifies the AB CIP per-device connection settings as discrete fields: Host, Path, PlcType, TimeoutMs, AllowPacking, ConnectionSize. The implementation instead collapses host + path into a single opaque ab:// URL string…
Driver.AbCip-015 Low Documentation & comments AbCipDriver.cs:9-11, PlcTagHandle.cs:23-27,53-58, AbCipTemplateCache.cs:12-15, IAbCipTagEnumerator.cs:6-11, AbCipDriverOptions.cs:21 Numerous comments are stale relative to the commit under review. AbCipDriver.cs:9-11 says the driver "Implements IDriver only for now" with capabilities shipping "in subsequent PRs (3-8)" while the class already implements all of them. `…
Driver.AbCip.Cli-003 Low Concurrency & thread safety src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/SubscribeCommand.cs:50-56,60-61 The OnDataChange handler writes change lines to console.Output (a TextWriter) from the driver's poll-engine callback thread, while the command's main flow concurrently writes the "Subscribed to ... Ctrl+C to stop." line on the CLI th…
Driver.AbCip.Cli-004 Low Error handling & resilience src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/SubscribeCommand.cs:28,58; AbCipCommandBase.cs:26-34 --interval-ms (IntervalMs) is taken verbatim and passed as TimeSpan.FromMilliseconds(IntervalMs) to SubscribeAsync with no validation. A zero or negative value produces a non-positive TimeSpan; the option description claims "Poll…
Driver.AbCip.Cli-005 Low Performance & resource management src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/DriverCommandBase.cs:51-59 ConfigureLogging assigns a freshly created Serilog logger to the process-global Log.Logger but never calls Log.CloseAndFlush(). For a short-lived one-shot command (probe, read, write) the process exit flushes the console sink,…
Driver.AbCip.Cli-006 Low Design-document adherence src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/AbCipCommandBase.cs:29-34 AbCipCommandBase overrides the abstract DriverCommandBase.Timeout property with a getter derived from TimeoutMs and an empty init body (init { /* driven by TimeoutMs */ }). Because the override has no [CommandOption] attribute,…
Driver.AbCip.Cli-007 Low Testing coverage tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli.Tests/WriteCommandParseValueTests.cs The only test file covers WriteCommand.ParseValue and ReadCommand.SynthesiseTagName — both pure static helpers. There is no coverage for AbCipCommandBase.BuildOptions (the flag-to-AbCipDriverOptions mapping that all four commands d…
Driver.AbCip.Cli-008 Low Documentation & comments docs/Driver.AbCip.Cli.md:8-9 docs/Driver.AbCip.Cli.md opens with "Second of four driver test-client CLIs (Modbus -> AB CIP -> AB Legacy -> S7 -> TwinCAT)." The count "four" contradicts the chain that follows it (five names) and contradicts docs/DriverClis.md, whic…
Driver.AbLegacy-005 Low OtOpcUa conventions AbLegacyDriver.cs (whole file) The driver uses no ILogger/Serilog at all. Probe-loop failures, runtime initialisation failures, libplctag non-zero statuses, and read/write exceptions are folded into DriverHealth.Detail strings but never logged. CLAUDE.md names Seril…
Driver.AbLegacy-011 Low Performance & resource management AbLegacyDriver.cs:440 Dispose() is implemented as DisposeAsync().AsTask().GetAwaiter().GetResult() - sync-over-async. ShutdownAsync awaits _poll.DisposeAsync() (which completes synchronously) and does no other real async work, so a deadlock is unlikely…
Driver.AbLegacy-013 Low Code organization & conventions AbLegacyDriver.cs:340-345, AbLegacyDriver.cs:238-264 Two minor organisational issues: 1. ResolveHost returns _options.Devices.FirstOrDefault()?.HostAddress ?? DriverInstanceId when the reference is unknown and no devices are configured. DriverInstanceId is not a host address (ab://...)…
Driver.AbLegacy.Cli-002 Low Correctness & logic bugs Commands/WriteCommand.cs:27-29, Program.cs:6-9 The --value option help text states "booleans accept true/false/1/0", but ParseBool (WriteCommand.cs:74-80) and the error message also accept on/off and yes/no, and DriverClis.md documents the full `true/false/1/0/yes/no/on/off…
Driver.AbLegacy.Cli-003 Low Concurrency & thread safety Commands/SubscribeCommand.cs:47-53 The OnDataChange handler calls console.Output.WriteLine(line) (the synchronous overload) directly from the PollGroupEngine poll thread. The poll engine raises change events from a background timer/loop thread, so two ticks that fire…
Driver.AbLegacy.Cli-004 Low Error handling & resilience Commands/ProbeCommand.cs:37-56, Commands/ReadCommand.cs:39-50, Commands/WriteCommand.cs:48-59, Commands/SubscribeCommand.cs:41-76 Every command does await using var driver = new AbLegacyDriver(...) and an explicit await driver.ShutdownAsync(...) in the finally. AbLegacyDriver DisposeAsync itself calls ShutdownAsync, so the driver is shut down twice on t…
Driver.AbLegacy.Cli-005 Low Design-document adherence Commands/SubscribeCommand.cs:23-25, docs/Driver.AbLegacy.Cli.md:94-96 The subscribe command interval option is --interval-ms (default 1000). docs/Driver.AbLegacy.Cli.md shows the subscribe example as otopcua-ablegacy-cli subscribe ... -i 500, which works because of the short alias 'i', but the doc ne…
Driver.AbLegacy.Cli-006 Low Code organization & conventions Commands/ProbeCommand.cs:20-22 ProbeCommand declares its --type option with no short alias, while ReadCommand, WriteCommand, and SubscribeCommand all declare --type with the short alias 't'. ProbeCommand also gives --address the alias 'a', matching t…
Driver.AbLegacy.Cli-007 Low Testing coverage tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli.Tests/WriteCommandParseValueTests.cs The only test file in the CLI test project covers WriteCommand.ParseValue and ReadCommand.SynthesiseTagName. Two behaviours that are pure logic (testable without a device) are uncovered: (1) AbLegacyCommandBase.BuildOptions — that it…
Driver.Cli.Common-004 Low Error handling & resilience src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/SnapshotFormatter.cs:68-70 FormatTable calls rows.Max(r => r.Tag.Length) (and the same for the value and status columns) without guarding against empty input. When tagNames and snapshots are both empty (equal length, so the mismatch check at line 56 passes),…
Driver.Cli.Common-006 Low Documentation & comments src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/SnapshotFormatter.cs:71, src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/DriverCommandBase.cs:9 Two minor doc inaccuracies. (1) The comment at SnapshotFormatter.cs:71 states the "source-time column is fixed-width (ISO-8601 to ms) so no max-measurement needed" — true only when every snapshot has a non-null SourceTimestampUtc. `For…
Driver.FOCAS-007 Low Error handling & resilience FocasDriver.cs:140-148, FocasDriver.cs:478-484, FocasDriver.cs:529-533, FocasAlarmProjection.cs:61-63 Numerous try { ... } catch {} blocks swallow every exception with no logging - ShutdownAsync (CTS cancel/dispose), RecycleLoopAsync (DisposeClient), FixedTreeLoopAsync transient catches, ProbeLoopAsync, and the alarm projection…
Driver.FOCAS-008 Low Performance & resource management FocasDriver.cs:201, FocasDriver.cs:253 ReadAsync and WriteAsync call FocasAddress.TryParse(def.Address) on every operation, even though InitializeAsync already parsed and validated every tag address. On a subscription hot path (each poll tick re-enters ReadAsync) this…
Driver.FOCAS-009 Low Design-document adherence FocasDriverOptions.cs:110-115, FocasDriver.cs:468-486, FocasDriverFactoryExtensions.cs:75-80 FocasProbeOptions.Timeout is parsed by the factory (FocasProbeDto.TimeoutMs to FocasProbeOptions.Timeout) but never consumed. ProbeLoopAsync calls client.ProbeAsync(ct) with only the probe-loop cancellation token; no per-probe ti…
Driver.FOCAS-010 Low Code organization & conventions IFocasClient.cs:210-227 (FocasOpMode), FocasConstants.cs:42-78 (FocasOperationMode) There are two parallel operation-mode-to-text mappings with divergent labels. FocasOpMode.ToText (used by the driver fixed-tree OperationMode/ModeText node) yields "TJOG", "TEACH_IN_HANDLE"; FocasOperationModeExtensions.ToText (i…
Driver.FOCAS-011 Low Code organization & conventions IFocasClient.cs:275-287 (FocasAlarmType), FocasAlarmProjection.cs:149-175 FocasAlarmType declares its constants as public const int, but the only consumers - FocasAlarmProjection.MapAlarmType(short type) and MapSeverity(short type) - take a short and switch against these int constants. It compiles…
Driver.FOCAS.Cli-001 Low Error handling & resilience Commands/WriteCommand.cs:58-68 WriteCommand.ParseValue parses the numeric --value types (Byte/Int16/Int32/Float32/Float64) with sbyte.Parse / short.Parse / etc. These throw raw FormatException or OverflowException for malformed or out-of-range inpu…
Driver.FOCAS.Cli-002 Low Concurrency & thread safety Commands/SubscribeCommand.cs:45-51 The subscribe command attaches an OnDataChange handler that calls the synchronous console.Output.WriteLine. OnDataChange is raised from the driver's PollGroupEngine tick thread, while the command's main flow writes the "Subscribe…
Driver.FOCAS.Cli-003 Low Error handling & resilience FocasCommandBase.cs:19 (CncPort), FocasCommandBase.cs:27 (TimeoutMs), Commands/SubscribeCommand.cs:23 (IntervalMs) The numeric command options --cnc-port, --timeout-ms, and --interval-ms are accepted without range validation. A zero or negative --cnc-port produces an invalid focas://host:<n> string; --timeout-ms 0 yields a zero TimeSpan o…
Driver.FOCAS.Cli-004 Low Performance & resource management Commands/ProbeCommand.cs:37,54; Commands/ReadCommand.cs:37,46; Commands/WriteCommand.cs:45,54; Commands/SubscribeCommand.cs:39,73 Every command declares await using var driver = new FocasDriver(...)
Driver.FOCAS.Cli-005 Low Design-document adherence Commands/WriteCommand.cs:50, Commands/ProbeCommand.cs:50 (via SnapshotFormatter.FormatStatus) docs/Driver.FOCAS.Cli.md documents BadDeviceFailure and BadCommunicationError as the key diagnostic signals an operator reads off probe / write output ("A BadCommunicationError means ... BadDeviceFailure after a successful co…
Driver.Galaxy-005 Low OtOpcUa conventions Runtime/EventPump.cs:81-88 The BoundedChannelOptions comment states "Newest-dropped policy: when full, the producer's TryWrite returns false ... We do this manually rather than relying on BoundedChannelFullMode.DropWrite" — but the option is then set to `FullMod…
Driver.Galaxy-010 Low Security GalaxyDriver.cs:311-341 ResolveApiKey supports an env:/file: indirection and otherwise treats the config string as the literal API key ("Anything else — used as the literal API key. Convenient for dev"). GalaxyGatewayOptions' own XML doc claims "the API k…
Driver.Galaxy-012 Low Performance & resource management Runtime/SubscriptionRegistry.cs:65-67, GalaxyDriver.cs:538, GalaxyDriver.cs:675 Several hot paths are O(n^2) per call. SubscriptionRegistry.ResolveSubscribers does entry.Bindings.FirstOrDefault(b => b.ItemHandle == itemHandle) — a linear scan of the whole binding list for every event dispatch; at 50k tags this is…
Driver.Galaxy-013 Low Design-document adherence GalaxyDriver.cs:14-27, GalaxyDriver.cs:374-382, Config/GalaxyDriverOptions.cs:84-86 Multiple doc comments are stale relative to the shipped code. GalaxyDriver's class summary still describes the file as "the project skeleton with IDriver bodies that wire to a future IGalaxyGatewayClient abstraction. Capability inter…
Driver.Historian.Wonderware-004 Low Correctness and logic bugs Backend/SdkAlarmHistorianWriteBackend.cs:198-201 ToHistorianEvent only assigns historianEvent.Id when Guid.TryParse(dto.EventId, ...) succeeds. If EventId is not a parseable GUID (or is empty), Id stays Guid.Empty and the event is written to the historian with an all-zeros id…
Driver.Historian.Wonderware-005 Low Concurrency and thread safety Backend/HistorianDataSource.cs:124, :126-127 GetHealthSnapshot reads _activeProcessNode and _activeEventNode inside _healthLock, but those two fields are written under _connectionLock / _eventConnectionLock (lines 183, 243, 209-210, 266-269) — a different lock. The health…
Driver.Historian.Wonderware-007 Low Error handling and resilience Ipc/PipeServer.cs:70-75 When VerifyCaller rejects the peer SID, the server logs the reason and calls _current.Disconnect() with no HelloAck frame sent. The shared-secret-mismatch and major-version-mismatch paths below it both send a rejecting HelloAck so…
Driver.Historian.Wonderware-008 Low Error handling and resilience Backend/HistorianDataSource.cs:301-307, :374-380 When query.StartQuery returns false, ReadRawAsync and ReadAggregateAsync call HandleConnectionError() and return an empty result list. A failed StartQuery is not necessarily a connection failure — it can be a bad tag name, an i…
Driver.Historian.Wonderware-010 Low Performance and resource management Backend/HistorianConfiguration.cs:32-36, Backend/HistorianDataSource.cs (all read methods) HistorianConfiguration.RequestTimeoutSeconds is documented as the "outer safety timeout applied to sync-over-async Historian operations" and is copied around (SdkAlarmHistorianWriteBackend.CloneConfigWithServerName:346), but it is neve…
Driver.Historian.Wonderware-011 Low Design-document adherence Backend/HistorianDataSource.cs:9-12, Backend/IHistorianDataSource.cs:9-11, Backend/HistorianSample.cs:7-9, Backend/HistorianConfiguration.cs:7-9 Several XML doc comments reference the retired v1 architecture as if it were current: "inside Galaxy.Host", "the Proxy maps returned samples", "the Host returns these across the IPC boundary as GalaxyDataValue", "Populated from ... the P…
Driver.Historian.Wonderware-012 Low Testing coverage Backend/HistorianDataSource.cs, tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Tests/ The unit-test suite covers HistorianQualityMapper, HistorianClusterEndpointPicker, SdkAlarmHistorianWriteBackend, AahClientManagedAlarmEventWriter, the IPC round trip, and Program alarm-writer wiring. HistorianDataSource itself…
Driver.Historian.Wonderware.Client-003 Low Concurrency & thread safety WonderwareHistorianClient.cs:207, WonderwareHistorianClient.cs:132-150 _totalQueries is mutated with Interlocked.Increment in Invoke, but read inside GetHealthSnapshot under _healthLock, and every other counter (_totalSuccesses, _totalFailures, _consecutiveFailures) is mutated only under `_hea…
Driver.Historian.Wonderware.Client-004 Low Concurrency & thread safety WonderwareHistorianClient.cs:203-267 A sidecar-reported failure is recorded in two non-atomic steps under separate lock acquisitions: Invoke calls RecordSuccess() (line 211) and then the caller calls ThrowIfFailed which calls ReclassifySuccessAsFailure() (line 256), d…
Driver.Historian.Wonderware.Client-006 Low Error handling & resilience Internal/PipeChannel.cs:96-107, WonderwareHistorianClientOptions.cs:11-12 PipeChannel.InvokeAsync retries exactly once on transport failure and otherwise propagates. The options expose ReconnectInitialBackoff and ReconnectMaxBackoff and WonderwareHistorianClientOptions documents them as exponential backo…
Driver.Historian.Wonderware.Client-008 Low Security ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Client.csproj:29-32 The csproj suppresses two NuGet audit advisories (GHSA-37gx-xxp4-5rgx, GHSA-w3x6-4m5h-cxqf) for the MessagePack 2.5.187 dependency with no inline comment recording why the suppression is safe, who reviewed it, or when it should be re…
Driver.Historian.Wonderware.Client-010 Low Documentation & comments WonderwareHistorianClient.cs:355-361, WonderwareHistorianClient.cs:132-150 Two doc/behaviour mismatches. (1) The Dispose() XML comment asserts the underlying channel async cleanup is non-blocking so the GetAwaiter()/GetResult() bridge is safe. PipeChannel.DisposeAsync calls ResetTransport(), which invokes…
Driver.Modbus-003 Low Concurrency & thread safety ModbusDriver.cs:59,188,241,259,266,726,745,759 _health is a non-volatile reference field written from multiple threads (concurrent ReadAsync callers, the coalesced-read path, WriteAsync indirectly, and ProbeLoopAsync) and read by GetHealth(). Reference assignment is atomic…
Driver.Modbus-007 Low Design-document adherence ModbusDriver.cs:1392, ModbusDriverOptions.cs:74-80 Two design-vs-code drifts. (1) MapDataType maps Int64/UInt64 to DriverDataType.Int32 with the inline comment "widening to Int32 loses precision; PR 25 adds Int64 to DriverDataType". The address-space node for a 64-bit Modbus tag is…
Driver.Modbus-008 Low Documentation & comments ModbusDriver.cs:411-417,700-703,737-744 Stale/misleading comments. (1) The <summary> block at ModbusDriver.cs:411-417 says auto-prohibited ranges are "Cleared by ReinitializeAsync ... or by an explicit re-probe API (not yet shipped)" — the re-probe loop has shipped (#151, `R…
Driver.Modbus-009 Low Correctness & logic bugs ModbusDriver.cs:1160-1167, ModbusTcpTransport.cs:94-95 Two edge cases. (1) RegisterCount for ModbusDataType.String computes (tag.StringLength + 1) / 2; a tag configured with StringLength = 0 yields a register count of 0, flowing into ReadOneAsync as totalRegs = 0 and producing an F…
Driver.Modbus-010 Low Error handling & resilience ModbusDriver.cs:864-868, ModbusDriverOptions.cs:116-125 When WriteOnChangeOnly is enabled and IsRedundantWrite returns true, WriteAsync returns WriteResult(0u) (Good) without touching the wire. The suppression baseline (_lastWrittenByRef) is only invalidated by a read that returns a…
Driver.Modbus-011 Low Code organization & conventions ModbusDriver.cs:23-43,89-97,408-432 Field and member declarations are interleaved with methods throughout ModbusDriver. ResolveHost (a public method) is the first member of the class, followed by BuildSlaveHostName, then a block of fields; _lastPublishedByRef/`_lastW…
Driver.Modbus-012 Low Testing coverage tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ The unit suite is broad (coalescing, bisection, auto-recovery, byte order, arrays, BCD, RMW, caps, multi-unit, probe, reconnect, subscription). Gaps relative to the findings above: (1) no test exercises concurrent multi-subscription publis…
Driver.Modbus.Addressing-006 Low Error handling & resilience ModbusAddressParser.cs:297-301 TryParseFamilyNative catches only ArgumentException and OverflowException. The current helpers throw only those (including ArgumentOutOfRangeException, which derives from ArgumentException), so today it is correct. But the parser…
Driver.Modbus.Addressing-007 Low Design-document adherence ModbusDataType.cs:91-95, docs/v2/dl205.md section Strings ModbusStringByteOrder (HighByteFirst / LowByteFirst) is defined in this assembly and documented as the DL205 low-byte-first string-packing knob, but ParsedModbusAddress has no field for it and ModbusAddressParser never produces or co…
Driver.Modbus.Addressing-009 Low Documentation & comments ModbusModiconAddress.cs:55-64, ModbusModiconAddress.cs:104-110 The comments on ModbusModiconAddress.TryParse are slightly inaccurate. The remark that 5-digit Modicon is always exactly 5 chars (40001..49999) and 6-digit is exactly 6 (400001..465536-shaped) implies the leading digit is always 4, but t…
Driver.Modbus.Cli-003 Low Correctness & logic bugs src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/ModbusCommandBase.cs:14-24 Port (int) and TimeoutMs (int) accept any 32-bit value, including negatives and ports above 65535. UnitId is a byte, so it accepts 0-255 even though the option description and docs/Driver.Modbus.Cli.md both say the valid rang…
Driver.Modbus.Cli-004 Low Concurrency & thread safety src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/Commands/SubscribeCommand.cs:61-67 The OnDataChange handler is invoked from the driver's PollGroupEngine background thread and calls console.Output.WriteLine synchronously. An exception thrown inside this handler (e.g. an IOException on a redirected or closed stdout…
Driver.Modbus.Cli-005 Low Error handling & resilience src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/Commands/ProbeCommand.cs:21-54; Commands/ReadCommand.cs:46-75; Commands/WriteCommand.cs:54-89 All three commands call ConfigureLogging() then console.RegisterCancellationHandler(), but if the operator presses Ctrl+C before InitializeAsync completes, the resulting OperationCancelledException propagates out of ExecuteAsync
Driver.Modbus.Cli-006 Low Error handling & resilience src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/Commands/ProbeCommand.cs:35-53 probe reports Health: {health.State} from GetHealth(). After a successful InitializeAsync the driver sets state to Healthy regardless of whether the subsequent probe register read returns Good or a Bad status code. ReadAsync do…
Driver.Modbus.Cli-007 Low Design-document adherence docs/Driver.Modbus.Cli.md:124-156; src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/Commands/ReadCommand.cs docs/Driver.Modbus.Cli.md devotes a whole "v2 addressing grammar" section to the industry-standard tag-address strings (40001:F:CDAB, HR1:I, C100, V2000:F:CDAB, etc.) and says "set the per-tag addressString field instead of the…
Driver.Modbus.Cli-008 Low Testing coverage tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli.Tests/ The test project covers only the two pure-function seams: ReadCommand.SynthesiseTagName and WriteCommand.ParseValue. There is no coverage for WriteCommand's read-only-region rejection (Region is not (Coils or HoldingRegisters)), no…
Driver.OpcUaClient-011 Low Documentation & comments OpcUaClientDriver.cs:783-784 The comment on the isArray computation states "-1 = scalar; 1+ = array dimensions; 0 = one-dimensional array". This is inaccurate against OPC UA ValueRank semantics: -3 is ScalarOrOneDimension, -2 is Any, -1 is Scalar, and 0 is OneOrMoreDi…
Driver.OpcUaClient-014 Low Performance & resource management OpcUaClientDriver.cs:904, :1035 MonitoredItem.Notification += (mi, args) => ... (and the alarm-event equivalent) attaches a closure-capturing lambda to each monitored item's event. The lambda is never detached. When UnsubscribeAsync removes a subscription it calls Subs…
Driver.S7-003 Low Correctness & logic bugs S7Driver.cs:172, S7Driver.cs:255 ReadAsync and WriteAsync dereference fullReferences.Count / writes.Count with no null guard. A null argument throws NullReferenceException rather than ArgumentNullException, and the NRE escapes before the _gate is taken so it is not wrappe…
Driver.S7-005 Low OtOpcUa conventions S7Driver.cs:33, S7Driver.cs:433 System.Collections.Concurrent.ConcurrentDictionary is written out with a fully-qualified namespace at the field declarations instead of a using System.Collections.Concurrent directive. ImplicitUsings is enabled and the rest of the codebase…
Driver.S7-009 Low Error handling & resilience S7Driver.cs:392 The subscription poll loop never reflects sustained polling failure anywhere an operator can see it. PollLoopAsync swallows every non-cancellation exception with an empty catch and the comment claims "the health surface reflects it" - but…
Driver.S7-010 Low Performance & resource management S7Driver.cs:504 Dispose() is implemented as DisposeAsync().AsTask().GetAwaiter().GetResult() - sync-over-async. Inside the generic host this is currently safe (no captured SynchronizationContext), but it is a known deadlock pattern. The only async work be…
Driver.S7-013 Low Code organization & conventions S7DriverOptions.cs:90, S7Driver.cs:300 S7TagDefinition.StringLength is a public configured/JSON-bound parameter (default 254) but is dead: S7DataType.String reads and writes both throw NotSupportedException ("...land in a follow-up PR"), so StringLength is never consumed. Likew…
Driver.S7.Cli-004 Low Performance & resource management src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli/Commands/ProbeCommand.cs:36,53, Commands/ReadCommand.cs:45,54, Commands/WriteCommand.cs:51,60, Commands/SubscribeCommand.cs:39,73 Every command declares the driver with await using var driver = new S7Driver(...) and also calls await driver.ShutdownAsync(...) in a finally block. S7Driver.DisposeAsync itself calls ShutdownAsync, so shutdown runs twice per c…
Driver.S7.Cli-005 Low Code organization & conventions tests/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli.Tests/ A stale directory tests/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli.Tests/ exists containing only an obj/ folder — no .csproj, no source. The real test project lives at tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli.Tests/. The empty direct…
Driver.S7.Cli-006 Low Testing coverage tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli.Tests/WriteCommandParseValueTests.cs The only test file covers WriteCommand.ParseValue and ReadCommand.SynthesiseTagName. S7CommandBase.BuildOptions — which maps the host / port / CPU / rack / slot / timeout flags onto an S7DriverOptions and forces `Probe.Enabled = fa…
Driver.S7.Cli-007 Low Documentation & comments src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli/Commands/SubscribeCommand.cs:45-51 The Modbus CLI SubscribeCommand carries an explanatory comment on the OnDataChange handler ("Route every data-change event to the CliFx console (not System.Console — the analyzer flags it + IConsole is the testable abstraction)"). The…
Driver.TwinCAT-004 Low Correctness & logic bugs TwinCATDataType.cs:24-27 The inline comments for the IEC time types are inaccurate. TwinCAT TIME is a duration (32-bit, milliseconds) — not "ms since epoch of day". DATE is stored as seconds since 1970-01-01 (truncated to a day boundary), not "days since 1970-…
Driver.TwinCAT-006 Low OtOpcUa conventions TwinCATDriver.cs:406-411 ResolveHost falls back to DriverInstanceId when there are no configured devices and the reference is unknown. DriverInstanceId is a logical config-DB identifier, not a host address; IPerCallHostResolver consumers expect a host key…
Driver.TwinCAT-014 Low Design-document adherence TwinCATDriverOptions.cs:41-43, TwinCATDriverOptions.cs:57-62, AdsTwinCATClient.cs:145 Several drifts between the implemented config surface and docs/v2/driver-specs.md section 6. The spec connection-settings list has separate Host (IP), AmsNetId, and AmsPort fields; the implementation collapses these into a single `…
Driver.TwinCAT-015 Low Code organization & conventions TwinCATDriver.cs:431-432 Dispose() runs DisposeAsync().AsTask().GetAwaiter().GetResult() — sync-over-async. docs/v2/driver-stability.md section Galaxy explicitly lists "sync-over-async on the OPC UA stack thread" among the four 2026-04-13 stability findings…
Driver.TwinCAT-016 Low Testing coverage tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/ Unit coverage exists for AMS-address parsing, symbol-path parsing, read/write, native notifications, symbol browse, and the capability surface. Gaps tied to the findings above: no test exercises ReinitializeAsync with a changed config (D…
Driver.TwinCAT.Cli-001 Low Correctness & logic bugs TwinCATCommandBase.cs:23-24, Commands/SubscribeCommand.cs:23-24, Commands/BrowseCommand.cs:21-24 Numeric command options are accepted without range validation. --timeout-ms feeds Timeout => TimeSpan.FromMilliseconds(TimeoutMs); passing --timeout-ms 0 or a negative value yields TimeSpan.Zero/a negative TimeSpan, which is then…
Driver.TwinCAT.Cli-002 Low Concurrency & thread safety Commands/SubscribeCommand.cs:46-58 The OnDataChange handler calls console.Output.WriteLine(line) synchronously. In native ADS-notification mode the event is raised from the Beckhoff.TwinCAT.Ads notification callback thread (see TwinCATDriver.SubscribeAsync, which in…
Driver.TwinCAT.Cli-003 Low Error handling & resilience Commands/SubscribeCommand.cs:56-58 The subscribe banner reports the mechanism purely from the --poll-only flag (var mode = PollOnly ? "polling" : "ADS notification"). The doc (docs/Driver.TwinCAT.Cli.md) states the banner "announces which mechanism is in play". The CL…
Driver.TwinCAT.Cli-004 Low Design-document adherence TwinCATCommandBase.cs:26-29, Commands/BrowseCommand.cs --poll-only is declared on TwinCATCommandBase, so it is inherited by browse. BrowseCommand only ever calls DiscoverAsync — it never subscribes — so UseNativeNotifications = !PollOnly has no observable effect on a browse run. Th…
Driver.TwinCAT.Cli-005 Low Code organization & conventions Commands/ProbeCommand.cs:23, Commands/ReadCommand.cs:20, Commands/WriteCommand.cs:20, Commands/SubscribeCommand.cs:18 The --type option is declared with the short alias -t on read, write, and subscribe, but ProbeCommand declares [CommandOption("type", ...)] with no short alias. An operator who has internalised -t from the other three verbs…
Driver.TwinCAT.Cli-006 Low Testing coverage tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Cli.Tests/WriteCommandParseValueTests.cs The only test file covers WriteCommand.ParseValue and ReadCommand.SynthesiseTagName. Other deterministic, router-independent logic is untested: TwinCATCommandBase.Gateway (the ads://{netId}:{port} string the driver's `TwinCATAmsAdd…
Driver.TwinCAT.Cli-007 Low Documentation & comments TwinCATCommandBase.cs:31-36 The Timeout override has an empty init accessor with the comment /* driven by TimeoutMs */. Because the base DriverCommandBase.Timeout is declared abstract { get; init; }, the override must supply an init, but here it silently…
Server-004 Low OtOpcUa conventions src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/OtOpcUaServer.cs:187-200 RoleBasedIdentity declares its own Display property, but the base UserIdentity already has a settable DisplayName. DriverNodeManager.ResolveCallUser/RouteScriptedAlarmMethodCalls read the base DisplayName, never Display. Si…
Server-006 Low Concurrency & thread safety src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs:478-482, 1342-1348 OnReadValue/OnWriteValue are synchronous stack hooks that block on async driver calls via .GetAwaiter().GetResult() with CancellationToken.None. With MaxRequestThreadCount = 100, a burst of reads/writes into a stalled driver pins…
Server-008 Low Error handling & resilience src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs:736 RouteScriptedAlarmMethodCalls marks a handled slot by setting errors[i] = ServiceResult.Good, assuming base.Call skips non-null Good error slots. The stack and GateCallMethodRequests only ever pre-populate Bad slots; the skip-o…
Server-012 Low Performance & resource management src/Server/ZB.MOM.WW.OtOpcUa.Server/Hosting/PeerHttpProbeLoop.cs:78-79 ProbeAsync creates an IHttpClientFactory client and mutates client.Timeout on every 2-second probe tick. The timeout belongs on the request or on the named-client registration, not set per call on a factory-vended instance.
Server-014 Low Code organization & conventions src/Server/ZB.MOM.WW.OtOpcUa.Server/SealedBootstrap.cs SealedBootstrap claims in its xml-doc to "close release blocker #2" by consuming the generation-sealed cache + resilient reader + stale-config flag, but Program.cs registers and uses NodeBootstrap instead. SealedBootstrap is never…
Server-015 Low Documentation & comments src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/OtOpcUaServer.cs:16-21, src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/OpcUaServerOptions.cs:21-26 OtOpcUaServer's class doc still says "PR 16 minimum-viable scope ... no security ... LDAP + security profiles are deferred." OpcUaServerOptions's says "PR 17 minimum-viable scope: no LDAP, no security profiles beyond None." Both are st…

Closed findings

Findings with status Resolved, Won't Fix, or Deferred.

ID Severity Status Category Location
Admin-001 Critical Resolved Security Components/Routes.razor:4-11, Program.cs:150
Admin-002 Critical Resolved Security Components/Pages/Clusters/NewCluster.razor:1-7, Home.razor, Fleet.razor, Hosts.razor, AlarmsHistorian.razor, Clusters/ClustersList.razor, Clusters/Generations.razor, Drivers/FocasDetail.razor
Core.AlarmHistorian-001 Critical Resolved Correctness & logic bugs src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/SqliteStoreAndForwardSink.cs:255-278
Core.Scripting-001 Critical Resolved Security ForbiddenTypeAnalyzer.cs:45, ScriptSandbox.cs:54
Driver.Galaxy-001 Critical Resolved Error handling & resilience Runtime/EventPump.cs:128, GalaxyDriver.cs:222
Server-001 Critical Resolved Correctness & logic bugs src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs:1791
Admin-003 High Resolved Security Program.cs:137-139, Hubs/FleetStatusHub.cs:11, Hubs/AlertHub.cs:10, Hubs/ScriptLogHub.cs:30
Admin-004 High Resolved Security appsettings.json:3,13-14
Admin-005 High Resolved Correctness & logic bugs Components/Pages/Login.razor:15,107-110
Client.Shared-005 High Resolved Concurrency & thread safety OpcUaClientService.cs:19, OpcUaClientService.cs:226-249, OpcUaClientService.cs:499-521
Client.Shared-006 High Resolved Concurrency & thread safety OpcUaClientService.cs:97-100, OpcUaClientService.cs:432-497
Configuration-001 High Resolved Correctness & logic bugs src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Migrations/20260417215224_StoredProcedures.cs:282
Configuration-008 High Resolved Security src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Migrations/20260417215224_StoredProcedures.cs:150, :373, :468
Core-001 High Resolved Correctness & logic bugs src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/UserAuthorizationState.cs:50-68
Core-002 High Resolved Security src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/TriePermissionEvaluator.cs:24-50
Core.AlarmHistorian-002 High Resolved Correctness & logic bugs src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/SqliteStoreAndForwardSink.cs:99-105,386-388
Core.AlarmHistorian-004 High Resolved Concurrency & thread safety src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/SqliteStoreAndForwardSink.cs:90,112,176,259
Core.AlarmHistorian-006 High Resolved Error handling & resilience src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/SqliteStoreAndForwardSink.cs:103,135-216
Core.ScriptedAlarms-001 High Resolved Concurrency & thread safety ScriptedAlarmEngine.cs:175, ScriptedAlarmEngine.cs:178, ScriptedAlarmEngine.cs:73, ScriptedAlarmEngine.cs:368
Core.Scripting-002 High Resolved Security ForbiddenTypeAnalyzer.cs:70
Core.VirtualTags-001 High Resolved Correctness & logic bugs src/Core/ZB.MOM.WW.OtOpcUa.Core.VirtualTags/VirtualTagEngine.cs:306
Driver.AbCip-001 High Resolved Correctness & logic bugs AbCipDriver.cs:111, AbCipDriver.cs:163-167
Driver.AbCip-002 High Resolved Correctness & logic bugs AbCipStatusMapper.cs:65-78
Driver.AbCip-003 High Resolved Correctness & logic bugs AbCipUdtMemberLayout.cs:32-54, AbCipDriver.cs:426-430, AbCipUdtReadPlanner.cs:48
Driver.AbCip-008 High Resolved Concurrency & thread safety AbCipDriver.cs:144-152, AbCipDriver.cs:169-183, AbCipDriver.cs:235-281
Driver.AbLegacy-001 High Resolved Correctness & logic bugs AbLegacyAddress.cs:54, AbLegacyDriver.cs:368-374
Driver.AbLegacy-006 High Resolved Concurrency & thread safety AbLegacyDriver.cs:107-158, AbLegacyDriver.cs:162-234, LibplctagLegacyTagRuntime.cs
Driver.Cli.Common-001 High Resolved Correctness & logic bugs src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/SnapshotFormatter.cs:106-119
Driver.FOCAS-001 High Resolved Correctness & logic bugs FocasDriverFactoryExtensions.cs:54-86, FocasDriverFactoryExtensions.cs:132-140
Driver.FOCAS-002 High Resolved Correctness & logic bugs WireFocasClient.cs:164-179, FocasDriver.cs:513, FocasDriver.cs:593
Driver.Galaxy-002 High Resolved Correctness & logic bugs Browse/DataTypeMap.cs:13, Runtime/MxValueDecoder.cs:9
Driver.Galaxy-008 High Resolved Error handling & resilience GalaxyDriver.cs:264-276, Runtime/EventPump.cs:97-103
Driver.Historian.Wonderware-001 High Resolved Correctness and logic bugs Backend/SdkAlarmHistorianWriteBackend.cs:68, Backend/AahClientManagedAlarmEventWriter.cs:82-103
Driver.Historian.Wonderware.Client-001 High Resolved Correctness & logic bugs WonderwareHistorianClient.cs:98-113
Driver.Modbus-001 High Resolved Concurrency & thread safety ModbusDriver.cs:92,99-122
Driver.Modbus.Addressing-001 High Resolved Correctness & logic bugs ModbusAddressParser.cs:230-235, DirectLogicAddress.cs:66-73
Driver.OpcUaClient-001 High Resolved Correctness & logic bugs OpcUaClientDriver.cs:444, :466, :517, :540, :599, :610
Driver.OpcUaClient-002 High Resolved Error handling & resilience OpcUaClientDriver.cs:1330-1359
Driver.OpcUaClient-003 High Resolved Correctness & logic bugs OpcUaClientDriver.cs:644-711
Driver.OpcUaClient-004 High Resolved Design-document adherence OpcUaClientDriver.cs:596-632, :789, OpcUaClientDriverOptions.cs
Driver.OpcUaClient-005 High Resolved Concurrency & thread safety OpcUaClientDriver.cs:1297-1319
Driver.S7-001 High Resolved Correctness & logic bugs S7AddressParser.cs:93, S7Driver.cs:231
Driver.S7-006 High Resolved Concurrency & thread safety S7Driver.cs:140, S7Driver.cs:457, S7Driver.cs:506
Driver.S7-007 High Resolved Error handling & resilience S7Driver.cs:200, S7DriverOptions.cs:13, docs/v2/driver-specs.md:434
Driver.S7-011 High Resolved Design-document adherence S7Driver.cs:82, S7Driver.cs:134, IDriver.cs:24
Driver.TwinCAT-001 High Resolved Correctness & logic bugs TwinCATDriver.cs:41-78
Driver.TwinCAT-002 High Resolved Correctness & logic bugs TwinCATDataType.cs:34-48, AdsTwinCATClient.cs:264-281
Driver.TwinCAT-007 High Resolved Concurrency & thread safety TwinCATDriver.cs:413-429
Driver.TwinCAT-008 High Resolved Concurrency & thread safety AdsTwinCATClient.cs:162-169, TwinCATDriver.cs:319-324
Driver.TwinCAT-013 High Resolved Design-document adherence TwinCATDriver.cs:11-12 (capability list), whole file
Server-002 High Resolved Correctness & logic bugs src/Server/ZB.MOM.WW.OtOpcUa.Server/Security/AuthorizationGate.cs:60-63
Server-009 High Resolved Security src/Server/ZB.MOM.WW.OtOpcUa.Server/Security/LdapOptions.cs:44, src/Server/ZB.MOM.WW.OtOpcUa.Server/Program.cs:74