diff --git a/code-reviews/AdminUI/findings.md b/code-reviews/AdminUI/findings.md index 947586d9..f8720f66 100644 --- a/code-reviews/AdminUI/findings.md +++ b/code-reviews/AdminUI/findings.md @@ -7,7 +7,7 @@ | Review date | 2026-06-19 | | Commit reviewed | `7286d320` | | Status | Reviewed | -| Open findings | 2 | +| Open findings | 0 | ## Checklist coverage @@ -33,7 +33,7 @@ | Severity | Medium | | Category | Security | | Location | `Components/Pages/ScriptEdit.razor:5`, `Components/Pages/Scripts.razor` | -| Status | Open | +| Status | Resolved | **Description:** The Script CRUD surface (`/scripts/new`, `/scripts/{id}`, and the `/scripts` list) is gated by only `[Authorize]` (any authenticated user), whereas the *same editor's* Roslyn-analysis @@ -50,7 +50,14 @@ and `Scripts` with `Authorize(Roles = "Administrator,Designer")` (matching `/dep confirm the intended tier with the security owner, since it changes who can reach the editor. Left Open pending that decision (an authorization-policy decision, not a self-contained bug fix). -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-06-20 — `ScriptEdit.razor:5` and `Scripts.razor:2` changed from +`[Authorize]` to `[Authorize(Roles = "Administrator,Designer")]` (namespace-qualified form, matching +`Deployments.razor`). `ScriptAnalysisEndpoints.cs` MapGroup changed from `.RequireAuthorization("FleetAdmin")` +to `.RequireAuthorization(new AuthorizeAttribute { Roles = "Administrator,Designer" })` so the +IntelliSense backend matches the page gate (Designer was previously 403'd by the FleetAdmin policy). +`using Microsoft.AspNetCore.Authorization;` added to `ScriptAnalysisEndpoints.cs`. Build-verified: +`dotnet build ZB.MOM.WW.OtOpcUa.AdminUI.csproj` — 0 errors. Live page-gating verification deferred +to docker-dev `/run`. ### AdminUI-002 @@ -107,7 +114,7 @@ CTS (and its timer) is disposed when the handler returns. Razor-behavioural chan | Severity | Low | | Category | Security | | Location | `Hubs/FleetStatusHub.cs`, `Hubs/AlertHub.cs`, `Hubs/ScriptLogHub.cs` (vs `Hubs/DriverStatusHub.cs:12`) | -| Status | Open | +| Status | Resolved | **Description:** Of the four SignalR hubs mapped by `MapOtOpcUaHubs`, only `DriverStatusHub` carries an explicit `[Authorize]` attribute. `FleetStatusHub`, `AlertHub`, and `ScriptLogHub` (which broadcast @@ -124,7 +131,11 @@ host's fallback policy. Low-risk (the fallback already requires auth, so observa unchanged) but left Open because hub authorization is a cross-cutting concern worth confirming with the Host/Security owners rather than silently changing in a single-module pass. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-06-20 — `[Authorize]` added to `FleetStatusHub`, `AlertHub`, and +`ScriptLogHub`, each with `using Microsoft.AspNetCore.Authorization;`, mirroring the exact form used by +`DriverStatusHub`. Observable behaviour is unchanged (the global FallbackPolicy already required auth); +authorization is now explicit and hub-local. Build-verified: `dotnet build ZB.MOM.WW.OtOpcUa.AdminUI.csproj` +— 0 errors. ### AdminUI-005 diff --git a/code-reviews/Configuration/findings.md b/code-reviews/Configuration/findings.md index 040f864c..24aac669 100644 --- a/code-reviews/Configuration/findings.md +++ b/code-reviews/Configuration/findings.md @@ -7,7 +7,7 @@ | Review date | 2026-06-19 (re-review; first reviewed 2026-05-22) | | Commit reviewed | `7286d320` (re-review; was `76d35d1`) | | Status | Reviewed | -| Open findings | 2 | +| Open findings | 1 | ## Checklist coverage @@ -247,10 +247,10 @@ Prior findings Configuration-001…011 remain Resolved. Notable since the first | Severity | Low | | Category | Documentation & comments | | Location | `src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Services/ILdapGroupRoleMappingService.cs:23`, `src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Services/LdapGroupRoleMappingService.cs:24` | -| Status | Open | +| Status | Resolved | **Description:** `ILdapGroupRoleMappingService.GetByGroupsAsync`'s XML doc asserts "Case-insensitive per LDAP conventions", but the implementation is `db.LdapGroupRoleMappings.Where(m => groupSet.Contains(m.LdapGroup))`, which translates to a SQL `IN (…)` whose case-sensitivity is entirely determined by the column's collation. On the default case-insensitive server collation (the dev/Docker SQL) the doc holds; on a case-sensitive-collation deployment the lookup would silently miss rows that differ only in case — and the in-memory EF unit tests (which match exact case) would not catch it. This is a hot path: it fires on every Admin-UI sign-in, so a silent miss denies the user their role grant. The doc overstates a guarantee the code does not enforce. **Recommendation:** Either (a) soften the doc to state the match is collation-dependent and document the required CI collation as a deployment constraint, or (b) pin the `LdapGroup` column to an explicit `_CI_` collation in the model so the guarantee is enforced regardless of server default. Option (b) is a schema/migration change and must be deferred (no migration edits in this review); option (a) is a one-line doc change but only papers over the gap. Left **Open** pending a decision on which guarantee to commit to — no code change applied because the documented behaviour currently matches the deployment reality (CI server collation). -**Resolution:** _(open — preferred fix (b) pins the column collation, which needs an EF migration; deferred per the no-migration rule)_ +**Resolution:** Fixed 2026-06-20 (option a). Replaced the "Case-insensitive per LDAP conventions" claim in `ILdapGroupRoleMappingService.GetByGroupsAsync`'s XML `` with a note that the match is a SQL `IN (…)` whose case-sensitivity is determined by the `LdapGroup` column collation, and that case-insensitive behaviour requires a CI server or column collation as a deployment requirement. The impl (`LdapGroupRoleMappingService`) carries no duplicate claim. Build confirmed 0 errors. Schema/migration change (option b) remains deferred. diff --git a/code-reviews/Core.AlarmHistorian/findings.md b/code-reviews/Core.AlarmHistorian/findings.md index 01bc78e2..e33ea508 100644 --- a/code-reviews/Core.AlarmHistorian/findings.md +++ b/code-reviews/Core.AlarmHistorian/findings.md @@ -7,7 +7,7 @@ | Review date | 2026-06-19 | | Commit reviewed | `621d00e4` | | Status | Reviewed | -| Open findings | 1 | +| Open findings | 0 | ## Checklist coverage @@ -249,10 +249,10 @@ All 11 prior findings confirmed resolved at HEAD. The re-review covered all 10 c | Severity | Low | | Category | Error handling & resilience | | Location | `src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/SqliteStoreAndForwardSink.cs:515-555,558-568` | -| Status | Open | +| Status | Resolved | **Description:** `GetStatus()` and `RetryDeadLettered()` have no `_disposed` guard. After `Dispose()` is called both methods still open new `SqliteConnection` objects against the database file (which persists on disk after disposal). This works because SQLite is file-backed, but the behavior is surprising and inconsistent with the convention that post-disposal calls on an `IDisposable` object throw `ObjectDisposedException`. `RetryDeadLettered()` in particular performs a write (`UPDATE Queue …`) against a sink that the owner has declared finished. If a lingering Admin UI polling loop calls `GetStatus()` after the host shuts down and disposes the sink, no error is surfaced — the method silently succeeds. **Recommendation:** Add `if (_disposed) throw new ObjectDisposedException(nameof(SqliteStoreAndForwardSink));` guards at the top of `GetStatus()` and `RetryDeadLettered()`, consistent with the guards already present on `EnqueueAsync` and `StartDrainLoop`. -**Deferred:** The current behavior (silently succeeds post-disposal) is benign in production because the Admin UI is also shutting down when the host disposes the sink. Adding the guard is a one-line change per method but requires coordinating with the Admin UI callers to confirm they tolerate `ObjectDisposedException` after shutdown — that is a cross-module concern. Deferring until the Admin UI shutdown sequence is reviewed. +**Resolution:** Resolved 2026-06-20 via TDD. Added `if (_disposed) throw new ObjectDisposedException(nameof(SqliteStoreAndForwardSink));` as the first statement in both `GetStatus()` and `RetryDeadLettered()`, matching the identical guard form used by `EnqueueAsync` and `StartDrainLoop`. Regression test `Disposed_sink_throws_ObjectDisposedException_on_GetStatus_and_RetryDeadLettered` added; confirmed fail-before / pass-after. All 29 tests green. diff --git a/code-reviews/Core.Scripting.Abstractions/findings.md b/code-reviews/Core.Scripting.Abstractions/findings.md index 70107594..ecab2c22 100644 --- a/code-reviews/Core.Scripting.Abstractions/findings.md +++ b/code-reviews/Core.Scripting.Abstractions/findings.md @@ -11,7 +11,7 @@ | Review date | 2026-06-19 | | Commit reviewed | `7286d320` | | Status | Reviewed | -| Open findings | 2 | +| Open findings | 0 | ## Checklist coverage @@ -112,7 +112,7 @@ current naming convention. Verified by build (no test project for this module). | Severity | Low | | Category | Correctness & logic bugs | | Location | `src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Abstractions/ScriptContext.cs:84` | -| Status | Open | +| Status | Resolved | **Description:** `ScriptContext.Deadband` has no guard or documentation for invalid `tolerance` values. A negative `tolerance` makes `Math.Abs(current - previous) > tolerance` trivially `true` @@ -132,9 +132,9 @@ changes the public API contract for authored scripts and needs a plan-level deci /// Negative values cause the function to always return true; NaN always returns false. ``` -**Resolution:** _(Open — the doc-only fix is zero-risk but the tolerance param is part of the -locked script API surface; the decision on whether to also add a guard belongs at the plan level. -Deferred until that call is made. No test project exists for this module.)_ +**Resolution:** Applied doc-only fix — updated the `` XML doc in +`ScriptContext.cs:90` to document that negative values always return true and NaN always returns +false. No guard added (API contract change deferred per owner decision). 2026-06-20. --- @@ -189,7 +189,7 @@ remarks in `ScriptContext.cs`. Verified by build (no test project for this modul | Severity | Low | | Category | Design-document adherence | | Location | `docs/ScriptedAlarms.md:298` | -| Status | Open | +| Status | Resolved | **Description:** `docs/ScriptedAlarms.md` line 298 lists the file path as `src/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/AlarmPredicateContext.cs`. The file was moved to @@ -204,5 +204,7 @@ only thing that changed. - `src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Abstractions/AlarmPredicateContext.cs` — script-side `ScriptContext` (read-only, write rejected) ``` -**Resolution:** _(Open — out of scope for this module's src-only fix pass. Should be fixed in a -docs-cleanup commit targeting `docs/ScriptedAlarms.md`.)_ +**Resolution:** Updated `docs/ScriptedAlarms.md` line 298 — replaced the stale +`Core.ScriptedAlarms/AlarmPredicateContext.cs` path with the correct +`Core.Scripting.Abstractions/AlarmPredicateContext.cs` path. Verified the file exists at the new +location before editing. 2026-06-20. diff --git a/code-reviews/Driver.TwinCAT.Contracts/findings.md b/code-reviews/Driver.TwinCAT.Contracts/findings.md index 2e3ea405..ff958a64 100644 --- a/code-reviews/Driver.TwinCAT.Contracts/findings.md +++ b/code-reviews/Driver.TwinCAT.Contracts/findings.md @@ -7,7 +7,7 @@ | Review date | 2026-06-19 | | Commit reviewed | `a19b0f86` | | Status | Reviewed | -| Open findings | 1 | +| Open findings | 0 | ## Checklist coverage @@ -83,7 +83,7 @@ the actual `TryGetInt32` call; no behaviour change. Verified by build: 0 errors, | Severity | Low | | Category | Documentation & comments | | Location | `TwinCATDataType.cs:33` (`Structure` member) | -| Status | Open | +| Status | Resolved | **Description:** The `Structure` enum member's XML summary says "UDT / FB instance. Resolved per member at discovery time." This is accurate for the discovery path, but the comment omits @@ -108,4 +108,9 @@ not appear in operator-authored config. Structure, ``` -**Resolution:** _(empty until closed)_ +**Resolution:** Resolved 2026-06-20 — expanded the `Structure` XML doc on `TwinCATDataType.cs:33` +to document the full contract: discovery-only sentinel, rejected at pre-declaration by +`TwinCATDriverFactoryExtensions.BuildTag` (`InvalidOperationException`), and not offered by the +AdminUI typed editor (parser falls back to `DInt`). Facts verified against +`TwinCATDriverFactoryExtensions.cs` (line 96–99) and `TwinCATEquipmentTagParser.cs` (line 31). +Build: 0 errors, 0 warnings. diff --git a/code-reviews/Host/findings.md b/code-reviews/Host/findings.md index f78818f6..d7fc548d 100644 --- a/code-reviews/Host/findings.md +++ b/code-reviews/Host/findings.md @@ -11,7 +11,7 @@ | Review date | 2026-06-19 | | Commit reviewed | `7286d320` | | Status | Reviewed | -| Open findings | 1 | +| Open findings | 0 | ## Checklist coverage @@ -78,13 +78,13 @@ a category produced nothing rather than leaving it blank. | Severity | Low | | Category | Design-document adherence | | Location | `docs/ServiceHosting.md` (section "Per-role configuration overlays") | -| Status | Open | +| Status | Resolved | **Description:** `docs/ServiceHosting.md` states the configuration loading order as "base `appsettings.json` → role overlay (`appsettings.{role}.json`) → environment overlay (`appsettings.{Environment}.json`) — later layers win." This is incorrect. The actual order established by `Program.cs:59–71` is: `appsettings.json` → `appsettings.{Environment}.json` (WebApplicationBuilder default) → `appsettings.{role}.json` (appended by Program.cs) → environment variables (re-appended) → command-line args (re-appended). The role overlay therefore **wins over** `appsettings.{Environment}.json`, not the other way around. The code behaviour is correct and intentional (explained by the comment at Program.cs:66–70); only the doc is wrong. **Recommendation:** Update `docs/ServiceHosting.md` to reflect the actual precedence: `appsettings.json` < `appsettings.{Environment}.json` < `appsettings.{role}.json` < environment variables < command-line args. Note that the role overlay intentionally outranks the environment-specific JSON so role-level security defaults cannot be overridden by a developer's local `appsettings.Development.json`, while environment variables and command-line args still outrank everything. Docs-only change; no src change needed. -**Resolution:** _(open — docs/ edit outside this pass's src-only scope; no code change required)_ +**Resolution:** Fixed 2026-06-20. Rewrote the "Per-role configuration overlays" loading-order sentence in `docs/ServiceHosting.md` to show the correct ascending-precedence chain (`appsettings.json` < `appsettings.{Environment}.json` < `appsettings.{role}.json` < env vars < CLI args) and added a sentence explaining that the role overlay intentionally outranks the environment-specific JSON so role-level security defaults (e.g. `DevStubMode = false`) cannot be overridden by a developer's local `appsettings.Development.json`. No src change required. --- diff --git a/docs/ScriptedAlarms.md b/docs/ScriptedAlarms.md index d9babbd1..c2a8ab74 100644 --- a/docs/ScriptedAlarms.md +++ b/docs/ScriptedAlarms.md @@ -295,7 +295,7 @@ Both engine and source are disposed on server shutdown via the driver-role host - `src/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/ScriptedAlarmDefinition.cs` — runtime definition record - `src/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/Part9StateMachine.cs` — pure-function state machine + `TransitionResult` / `EmissionKind` - `src/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/AlarmConditionState.cs` — persisted state record + `AlarmComment` audit entry + `ShelvingState` -- `src/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/AlarmPredicateContext.cs` — script-side `ScriptContext` (read-only, write rejected) +- `src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Abstractions/AlarmPredicateContext.cs` — script-side `ScriptContext` (read-only, write rejected) - `src/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/AlarmTypes.cs` — `AlarmKind` + `ShelvingKind` + four Part 9 state enums (`AlarmEnabledState`, `AlarmActiveState`, `AlarmAckedState`, `AlarmConfirmedState`); `AlarmSeverity` (`Low`/`Medium`/`High`/`Critical`) lives in `src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IAlarmSource.cs` - `src/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/MessageTemplate.cs` — `{path}` placeholder resolver - `src/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/IAlarmStateStore.cs` — persistence contract + `InMemoryAlarmStateStore` default diff --git a/docs/ServiceHosting.md b/docs/ServiceHosting.md index 872d0448..c925fefd 100644 --- a/docs/ServiceHosting.md +++ b/docs/ServiceHosting.md @@ -33,7 +33,11 @@ Single-node dev: `OTOPCUA_ROLES=admin,driver`. Production: typically two admin n - `appsettings.driver.json` — driver-only nodes - `appsettings.admin-driver.json` — fused single-node dev / small deployments -All three carry Serilog log-level overrides + `Security:Ldap:DevStubMode = false`. Loading order is **base `appsettings.json` → role overlay (`appsettings.{role}.json`) → environment overlay (`appsettings.{Environment}.json`)** — later layers win. Overlays are optional; the base file boots a node on its own. +All three carry Serilog log-level overrides + `Security:Ldap:DevStubMode = false`. Configuration loading order (lowest to highest precedence) is: + +`appsettings.json` < `appsettings.{Environment}.json` < `appsettings.{role}.json` < environment variables < command-line args + +The role overlay intentionally outranks `appsettings.{Environment}.json` so role-level security defaults (such as `DevStubMode = false`) cannot be silently overridden by a developer's local `appsettings.Development.json`; environment variables and command-line args still outrank everything for deployment-level overrides. Overlays are optional; the base file boots a node on its own. ## Akka cluster diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Services/ILdapGroupRoleMappingService.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Services/ILdapGroupRoleMappingService.cs index a34f15fd..7b7348e5 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Services/ILdapGroupRoleMappingService.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Services/ILdapGroupRoleMappingService.cs @@ -20,7 +20,11 @@ public interface ILdapGroupRoleMappingService /// List every mapping whose LDAP group matches one of . /// /// Hot path — fires on every sign-in. The default EF implementation relies on the - /// IX_LdapGroupRoleMapping_Group index. Case-insensitive per LDAP conventions. + /// IX_LdapGroupRoleMapping_Group index. The match is a SQL IN (…) whose + /// case-sensitivity is determined by the LdapGroup column collation. Case-insensitive + /// behaviour requires a case-insensitive (CI) server or column collation — this is a + /// deployment requirement. On a case-sensitive-collation server the lookup will silently + /// miss rows that differ only in case. /// /// The LDAP groups to search for. /// The cancellation token. diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/SqliteStoreAndForwardSink.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/SqliteStoreAndForwardSink.cs index 5127bf71..c9bada22 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/SqliteStoreAndForwardSink.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/SqliteStoreAndForwardSink.cs @@ -520,6 +520,7 @@ public sealed class SqliteStoreAndForwardSink : IAlarmHistorianSink, IDisposable /// Gets the current status of the historian sink including queue depth and drain state. public HistorianSinkStatus GetStatus() { + if (_disposed) throw new ObjectDisposedException(nameof(SqliteStoreAndForwardSink)); // Core.AlarmHistorian-008: read the non-dead-lettered count from the in-memory // counter so a busy Admin UI / health probe does not hammer the DB. Dead-letter // depth is rare-path only (it lives in the queue until retention) so a real @@ -563,6 +564,7 @@ public sealed class SqliteStoreAndForwardSink : IAlarmHistorianSink, IDisposable /// Operator action from Admin UI — retry every dead-lettered row. Non-cascading: they rejoin the regular queue + get a fresh backoff. public int RetryDeadLettered() { + if (_disposed) throw new ObjectDisposedException(nameof(SqliteStoreAndForwardSink)); using var conn = OpenConnection(); using var cmd = conn.CreateCommand(); cmd.CommandText = "UPDATE Queue SET DeadLettered = 0, AttemptCount = 0, LastError = NULL WHERE DeadLettered = 1"; diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Abstractions/ScriptContext.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Abstractions/ScriptContext.cs index c0d0720b..e0f389b9 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Abstractions/ScriptContext.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Abstractions/ScriptContext.cs @@ -87,7 +87,8 @@ public abstract class ScriptContext /// /// The current value to check. /// The previous value to compare against. - /// The minimum difference threshold for a change to be detected. + /// The minimum change magnitude to detect (must be >= 0). + /// Negative values cause the function to always return true; NaN always returns false. public static bool Deadband(double current, double previous, double tolerance) => Math.Abs(current - previous) > tolerance; } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Contracts/TwinCATDataType.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Contracts/TwinCATDataType.cs index 8f6851d1..ee2b0290 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Contracts/TwinCATDataType.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Contracts/TwinCATDataType.cs @@ -29,6 +29,12 @@ public enum TwinCATDataType Date, // DATE — seconds since 1970-01-01 truncated to a day boundary, stored as UDINT DateTime, // DT (DATE_AND_TIME) — seconds since 1970-01-01, stored as UDINT TimeOfDay,// TOD — milliseconds since midnight, stored as UDINT - /// UDT / FB instance. Resolved per member at discovery time. + /// + /// UDT / FB instance. Used internally by DiscoverAsync when browsing controller + /// symbols — members are resolved to atomic types and emitted individually. Must not be + /// used in pre-declared tags (rejected at initialisation by the factory with + /// ) or in AdminUI equipment-tag configs (the typed + /// editor does not offer it; the parser falls back to DInt). + /// Structure, } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/ScriptEdit.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/ScriptEdit.razor index 21cbd53c..8dc24919 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/ScriptEdit.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/ScriptEdit.razor @@ -2,7 +2,7 @@ @page "/scripts/{ScriptId}" @* Script CRUD. SourceHash is computed automatically from SourceCode on save so the integrity check in v2's deployment pipeline doesn't require operator action. *@ -@attribute [Microsoft.AspNetCore.Authorization.Authorize] +@attribute [Microsoft.AspNetCore.Authorization.Authorize(Roles = "Administrator,Designer")] @rendermode RenderMode.InteractiveServer @using Microsoft.AspNetCore.Components.Forms @using Microsoft.EntityFrameworkCore diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Scripts.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Scripts.razor index e8965d25..598110bb 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Scripts.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Scripts.razor @@ -1,5 +1,5 @@ @page "/scripts" -@attribute [Microsoft.AspNetCore.Authorization.Authorize] +@attribute [Microsoft.AspNetCore.Authorization.Authorize(Roles = "Administrator,Designer")] @rendermode RenderMode.InteractiveServer @using Microsoft.EntityFrameworkCore @using ZB.MOM.WW.OtOpcUa.Configuration diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/AlertHub.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/AlertHub.cs index 50ed8157..3b1961a7 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/AlertHub.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/AlertHub.cs @@ -1,3 +1,4 @@ +using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.SignalR; namespace ZB.MOM.WW.OtOpcUa.AdminUI.Hubs; @@ -8,6 +9,7 @@ namespace ZB.MOM.WW.OtOpcUa.AdminUI.Hubs; /// clears, or is acknowledged on any cluster node. Bridge: AlertSignalRBridge subscribes /// to the alerts DPS topic and forwards to every connected SignalR client. /// +[Authorize] public sealed class AlertHub : Hub { public const string Endpoint = "/hubs/alerts"; diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/FleetStatusHub.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/FleetStatusHub.cs index 5f273efb..07b5f0f3 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/FleetStatusHub.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/FleetStatusHub.cs @@ -1,3 +1,4 @@ +using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.SignalR; using ZB.MOM.WW.OtOpcUa.Commons.Messages.Fleet; @@ -10,6 +11,7 @@ namespace ZB.MOM.WW.OtOpcUa.AdminUI.Hubs; /// Server pushes fleet-status updates to connected clients via FleetStatusSignalRBridge /// (DistributedPubSub 'fleet-status' → IHubContext<FleetStatusHub>). /// +[Authorize] public sealed class FleetStatusHub : Hub { public const string Endpoint = "/hubs/fleet-status"; diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/ScriptLogHub.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/ScriptLogHub.cs index 087ebe6c..bbcb8fa7 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/ScriptLogHub.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/ScriptLogHub.cs @@ -1,3 +1,4 @@ +using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.SignalR; namespace ZB.MOM.WW.OtOpcUa.AdminUI.Hubs; @@ -9,6 +10,7 @@ namespace ZB.MOM.WW.OtOpcUa.AdminUI.Hubs; /// ScriptLogSignalRBridge subscribes to the script-logs DPS topic and forwards /// to every connected SignalR client. /// +[Authorize] public sealed class ScriptLogHub : Hub { public const string Endpoint = "/hubs/script-log"; diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisEndpoints.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisEndpoints.cs index 0bdbfe23..0d4f48db 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisEndpoints.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisEndpoints.cs @@ -1,3 +1,4 @@ +using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing; @@ -9,7 +10,9 @@ public static class ScriptAnalysisEndpoints { public static IEndpointRouteBuilder MapScriptAnalysisEndpoints(this IEndpointRouteBuilder endpoints) { - var group = endpoints.MapGroup("/api/script-analysis").RequireAuthorization("FleetAdmin"); + // Require Administrator or Designer — matches the Script page gate and the /deployments gate. + var group = endpoints.MapGroup("/api/script-analysis") + .RequireAuthorization(new AuthorizeAttribute { Roles = "Administrator,Designer" }); group.MapPost("/diagnostics", (DiagnoseRequest r, ScriptAnalysisService s) => Results.Ok(s.Diagnose(r))); group.MapPost("/completions", async (CompletionsRequest r, ScriptAnalysisService s) => Results.Ok(await s.CompleteAsync(r))); group.MapPost("/hover", async (HoverRequest r, ScriptAnalysisService s) => Results.Ok(await s.Hover(r))); diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian.Tests/SqliteStoreAndForwardSinkTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian.Tests/SqliteStoreAndForwardSinkTests.cs index 4096f275..3f86e70f 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian.Tests/SqliteStoreAndForwardSinkTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian.Tests/SqliteStoreAndForwardSinkTests.cs @@ -850,6 +850,25 @@ public sealed class SqliteStoreAndForwardSinkTests : IDisposable sink.GetStatus().QueueDepth.ShouldBe(live, "GetStatus must agree with a fresh COUNT(*)"); } + /// + /// Regression for Core.AlarmHistorian-014: + /// and must throw + /// after + /// is called, consistent with the guards already present on + /// and + /// . + /// + [Fact] + public void Disposed_sink_throws_ObjectDisposedException_on_GetStatus_and_RetryDeadLettered() + { + var writer = new FakeWriter(); + var sink = new SqliteStoreAndForwardSink(_dbPath, writer, _log); + sink.Dispose(); + + Should.Throw(() => sink.GetStatus()); + Should.Throw(() => sink.RetryDeadLettered()); + } + /// Insert a queue row whose PayloadJson cannot deserialize into an AlarmHistorianEvent. private void InsertCorruptRow(string alarmId) {