From 52c8a212a1cfc410037a170a4090038e93b3df41 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 09:28:04 -0400 Subject: [PATCH] fix(management): reject native alarm source names that are not legal actor names (review 2026-10-21 lane F follow-up to 01/SR-C1) --- .../Component-ManagementService.md | 2 +- .../ManagementActor.cs | 16 +++++ .../NativeAlarmSourceNameValidationTests.cs | 63 +++++++++++++++++++ 3 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/NativeAlarmSourceNameValidationTests.cs diff --git a/docs/requirements/Component-ManagementService.md b/docs/requirements/Component-ManagementService.md index 38b1b7e7..9a6591f9 100644 --- a/docs/requirements/Component-ManagementService.md +++ b/docs/requirements/Component-ManagementService.md @@ -113,7 +113,7 @@ The whole folder surface is reachable from the CLI as `template folder list|crea - **AddTemplateAttribute** / **UpdateTemplateAttribute** / **DeleteTemplateAttribute**: Manage attributes on a template. - **AddTemplateAlarm** / **UpdateTemplateAlarm** / **DeleteTemplateAlarm**: Manage alarm definitions on a template. -- **AddTemplateNativeAlarmSource** / **UpdateTemplateNativeAlarmSource** / **DeleteTemplateNativeAlarmSource**: Manage native alarm source bindings (the read-only OPC UA Alarms & Conditions / MxAccess Gateway mirror) on a template — `Name`, `ConnectionName`, `SourceReference`, optional `ConditionFilter` / `Description`, `IsLocked`. Mutations gated to the **Design** role; handlers write through `ITemplateEngineRepository` directly. +- **AddTemplateNativeAlarmSource** / **UpdateTemplateNativeAlarmSource** / **DeleteTemplateNativeAlarmSource**: Manage native alarm source bindings (the read-only OPC UA Alarms & Conditions / MxAccess Gateway mirror) on a template — `Name`, `ConnectionName`, `SourceReference`, optional `ConditionFilter` / `Description`, `IsLocked`. Mutations gated to the **Design** role; handlers write through `ITemplateEngineRepository` directly — so, unlike scripts/alarms/composition slots (checked by `TemplateService`), Add/Update validate `Name` with `ActorNameRules` themselves and reject an illegal Akka actor-name element (space, `/`, leading `$`, …) with a readable error, because the name becomes part of the site actor name `native-alarm-{canonical}` (review 2026-10-21, follow-up to 01/SR-C1). - **ListTemplateNativeAlarmSources** (`TemplateId`): List a template's native alarm source bindings (read-only; any authenticated user). - **AddTemplateScript** / **UpdateTemplateScript** / **DeleteTemplateScript**: Manage scripts on a template. - **AddTemplateComposition** / **DeleteTemplateComposition**: Manage feature module compositions on a template. diff --git a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs index 1ea6c920..81f133db 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs @@ -3309,8 +3309,23 @@ public class ManagementActor : ReceiveActor // ── Native alarm source bindings (read-only mirror; repository-direct CRUD) ── + /// + /// A native alarm source's name becomes part of the site actor name + /// native-alarm-{canonical}; an illegal character makes the Instance Actor's + /// ActorOf throw at deploy. Scripts, alarms and composition slots are checked by + /// TemplateService, but native-source CRUD is repository-direct here, so it applies + /// itself (review 2026-10-21, lane F follow-up to 01/SR-C1). + /// + private static void ValidateNativeAlarmSourceName(string? name) + { + var error = ActorNameRules.Describe(name); + if (error is not null) + throw new ManagementCommandException($"Native alarm source name is invalid: {error}"); + } + private static async Task HandleAddNativeAlarmSource(IServiceProvider sp, AddTemplateNativeAlarmSourceCommand cmd, string user) { + ValidateNativeAlarmSourceName(cmd.Name); var repo = sp.GetRequiredService(); var source = new TemplateNativeAlarmSource(cmd.Name) { @@ -3331,6 +3346,7 @@ public class ManagementActor : ReceiveActor private static async Task HandleUpdateNativeAlarmSource(IServiceProvider sp, UpdateTemplateNativeAlarmSourceCommand cmd, string user) { + ValidateNativeAlarmSourceName(cmd.Name); var repo = sp.GetRequiredService(); var source = await repo.GetTemplateNativeAlarmSourceByIdAsync(cmd.NativeAlarmSourceId) ?? throw new ManagementCommandException($"Native alarm source {cmd.NativeAlarmSourceId} not found."); diff --git a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/NativeAlarmSourceNameValidationTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/NativeAlarmSourceNameValidationTests.cs new file mode 100644 index 00000000..c4ebfcf3 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/NativeAlarmSourceNameValidationTests.cs @@ -0,0 +1,63 @@ +using Akka.Actor; +using Akka.TestKit.Xunit2; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using ZB.MOM.WW.ScadaBridge.Commons.Entities.Templates; +using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.Management; + +namespace ZB.MOM.WW.ScadaBridge.ManagementService.Tests; + +/// +/// Review 2026-10-21 (lane F follow-up to 01/SR-C1): a native alarm source's name becomes +/// part of the site actor name native-alarm-{canonical}. Template scripts, alarms and +/// composition slots are validated by TemplateService, but native-source CRUD is +/// repository-direct in the ManagementActor, so it must apply ActorNameRules itself. +/// +public class NativeAlarmSourceNameValidationTests : TestKit +{ + private readonly ITemplateEngineRepository _repo = Substitute.For(); + private readonly ServiceCollection _services = new ServiceCollection().AddTestConfigurationTransaction(); + + /// Initializes the harness. + public NativeAlarmSourceNameValidationTests() + { + _services.AddScoped(_ => _repo); + _repo.GetTemplateNativeAlarmSourceByIdAsync(4, Arg.Any()) + .Returns(new TemplateNativeAlarmSource("Pressure") { Id = 4, TemplateId = 1 }); + } + + private IActorRef CreateActor() => + Sys.ActorOf(Props.Create(() => new ManagementActor( + _services.BuildServiceProvider(), NullLogger.Instance))); + + private static ManagementEnvelope Designer(object command) => + new(new AuthenticatedUser("d", "D", new[] { "Designer" }, Array.Empty()), + command, Guid.NewGuid().ToString("N")); + + /// Adding a source whose name is not a legal actor name is rejected before any write. + [Theory] + [InlineData("Tank Pressure")] + [InlineData("$system")] + [InlineData("a/b")] + public void Add_InvalidActorName_IsRejected(string name) + { + CreateActor().Tell(Designer(new AddTemplateNativeAlarmSourceCommand(1, name, "Opc", "ns=2;s=T01", null, null, false))); + + var error = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Contains("Native alarm source name is invalid", error.Error); + _repo.DidNotReceive().AddTemplateNativeAlarmSourceAsync(Arg.Any(), Arg.Any()); + } + + /// Renaming a source to an illegal actor name is rejected before any write. + [Fact] + public void Update_RenameToInvalidActorName_IsRejected() + { + CreateActor().Tell(Designer(new UpdateTemplateNativeAlarmSourceCommand(4, "Tank Pressure", "Opc", "ns=2;s=T01", null, null, false))); + + var error = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Contains("Native alarm source name is invalid", error.Error); + _repo.DidNotReceive().UpdateTemplateNativeAlarmSourceAsync(Arg.Any(), Arg.Any()); + } +}