From 64e6ba70e84ca921326701fb7f1bfe109ab913ec Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 19 Aug 2026 14:47:49 -0400 Subject: [PATCH] fix(management): site-scope ListDataConnections + hoist scope checks above resolve/status oracles (arch-review 2026-08-19 P6-4 + P6-5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P6-4: HandleListDataConnections took no user at all and returned EVERY site's connections to any authenticated caller, nullifying the row-98 fail-closed gate on HandleGetDataConnection — Get denied exactly what List handed over. DataConnectionPublicShape already scrubs secrets, so the leak was site topology plus a connection-id existence oracle, but it still defeated scoping. List now takes the AuthenticatedUser: an explicit SiteId goes through EnforceSiteScope (the ListAreas shape), and an unfiltered list from a site-scoped non-Administrator caller is narrowed to their permitted sites (the ListSites shape). GetRequiredRoles is untouched — scoping is not a role gate, so the frozen 138-command authz matrix is unaffected. The Central UI Design > Connections page bypasses the actor entirely (ISiteRepository.GetAllDataConnectionsAsync direct) and IS reachable by a scoped user: it is gated RequireDesign, and SiteScopeService explicitly does NOT exempt Designer (register row 86). It now filters sites through SiteScope.FilterSitesAsync and keeps only their connections, mirroring EventLogs/AlarmSummary. P6-5: four handlers resolved their target — and on approve/reject reported its status — BEFORE the scope check, so an out-of-scope caller could tell a real id from a fake one, and read a secured write's lifecycle status, from the error shape alone. The scope check is hoisted above the not-found and status checks in HandleSubmitSecuredWrite, HandleApproveSecuredWrite, HandleRejectSecuredWrite and HandleDebugSnapshot, with the unresolvable case routed through the existing DenyUnresolvedTargetIfScoped so the denial is identical whether or not the target exists. System-wide callers keep their ordinary not-found/status answers, and the P6-1 CAS semantics on approve/reject are unchanged (only the checks above the CAS moved). Tests: 8 new ManagementService tests + 2 CentralUI bUnit tests, all verified to FAIL against the unfixed handlers (src reverted, tests re-run) and pass after — including the unscoped/Administrator control cases proving behaviour for system-wide callers is unchanged. Full ManagementService.Tests 545 green, CentralUI.Tests 1073 green. --- .../Component-ManagementService.md | 2 +- .../Pages/Design/DataConnections.razor | 13 +- .../ManagementActor.cs | 101 +++++++++++--- .../DataConnectionsPageTests.cs | 65 +++++++++ .../ManagementActorTests.cs | 123 +++++++++++++++++ .../SecuredWriteHandlerTests.cs | 130 ++++++++++++++++++ 6 files changed, 410 insertions(+), 24 deletions(-) diff --git a/docs/requirements/Component-ManagementService.md b/docs/requirements/Component-ManagementService.md index 1cdcf1bf..a0fff021 100644 --- a/docs/requirements/Component-ManagementService.md +++ b/docs/requirements/Component-ManagementService.md @@ -141,7 +141,7 @@ The whole folder surface is reachable from the CLI as `template folder list|crea ### Data Connections -- **ListDataConnections** / **GetDataConnection**: Query data connection definitions. +- **ListDataConnections** / **GetDataConnection**: Query data connection definitions. Both are site-scoped for a site-scoped non-Administrator caller — `List` filters an explicit `SiteId` through `EnforceSiteScope` and narrows an unfiltered list to the caller's permitted sites, rather than returning the fleet and nullifying `Get`'s fail-closed gate (arch-review 2026-08-19 P6-4). - **CreateDataConnection** / **UpdateDataConnection** / **DeleteDataConnection**: Manage data connection definitions. - **AssignDataConnectionToSite** / **UnassignDataConnectionFromSite**: Manage site assignments. diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Design/DataConnections.razor b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Design/DataConnections.razor index b69f207b..a2134739 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Design/DataConnections.razor +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Design/DataConnections.razor @@ -8,6 +8,7 @@ @attribute [Authorize(Policy = AuthorizationPolicies.RequireDesign)] @implements IDisposable @inject ISiteRepository SiteRepository +@inject ZB.MOM.WW.ScadaBridge.CentralUI.Auth.SiteScopeService SiteScope @inject IConnectionHealthQueryService ConnectionHealthQuery @inject NavigationManager NavigationManager @inject IDialogService Dialog @@ -305,8 +306,16 @@ _errorMessage = null; try { - var sites = await SiteRepository.GetAllSitesAsync(); - _connections = (await SiteRepository.GetAllDataConnectionsAsync()).ToList(); + // Site scoping (arch-review 2026-08-19 P6-4): this page reads the repository + // directly, bypassing ManagementActor's scope gate, and Designer is NOT + // exempt from site scope (see SiteScopeService) — so a scoped Designer used + // to see every site's connections here. Filter the sites, then keep only the + // connections belonging to them, mirroring EventLogs/AlarmSummary. + var sites = await SiteScope.FilterSitesAsync(await SiteRepository.GetAllSitesAsync()); + var permittedSiteIds = sites.Select(s => s.Id).ToHashSet(); + _connections = (await SiteRepository.GetAllDataConnectionsAsync()) + .Where(c => permittedSiteIds.Contains(c.SiteId)) + .ToList(); var connBySite = _connections.GroupBy(c => c.SiteId).ToDictionary(g => g.Key, g => g.ToList()); _treeRoots = sites.Select(site => new DcTreeNode( diff --git a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs index dcfabc61..13b62ae0 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs @@ -394,7 +394,7 @@ public class ManagementActor : ReceiveActor MoveAreaCommand cmd => await HandleMoveArea(sp, cmd, user.Username), // Data Connections - ListDataConnectionsCommand cmd => await HandleListDataConnections(sp, cmd), + ListDataConnectionsCommand cmd => await HandleListDataConnections(sp, cmd, user), GetDataConnectionCommand cmd => await HandleGetDataConnection(sp, cmd, user), CreateDataConnectionCommand cmd => await HandleCreateDataConnection(sp, cmd, user.Username), UpdateDataConnectionCommand cmd => await HandleUpdateDataConnection(sp, cmd, user.Username), @@ -1413,13 +1413,22 @@ public class ManagementActor : ReceiveActor private static async Task HandleSubmitSecuredWrite( IServiceProvider sp, SubmitSecuredWriteCommand cmd, AuthenticatedUser user) { - var siteRepo = sp.GetRequiredService(); - var site = await siteRepo.GetSiteByIdentifierAsync(cmd.SiteId) - ?? throw new ManagementCommandException($"Site '{cmd.SiteId}' not found."); - // Site scope (arch-review R2 N3): an LDAP-mapping-scoped Operator may only // submit device writes to their permitted sites — same rule C2 applies to // DeployArtifacts, on a higher-consequence path. Checked before the row exists. + // + // The unresolvable case is fail-CLOSED and answered BEFORE the "not found" + // (arch-review 2026-08-19 P6-5): resolving first and reporting not-found made + // this handler a site-identifier existence oracle for an out-of-scope caller, + // who now gets the same scope denial whether or not the site exists. + var siteRepo = sp.GetRequiredService(); + var site = await siteRepo.GetSiteByIdentifierAsync(cmd.SiteId); + if (site is null) + { + DenyUnresolvedTargetIfScoped(user, $"site '{cmd.SiteId}'"); + throw new ManagementCommandException($"Site '{cmd.SiteId}' not found."); + } + EnforceSiteScope(user, site.Id); var connections = await siteRepo.GetDataConnectionsBySiteIdAsync(site.Id); @@ -1461,18 +1470,27 @@ public class ManagementActor : ReceiveActor IServiceProvider sp, ApproveSecuredWriteCommand cmd, AuthenticatedUser user) { var repo = sp.GetRequiredService(); - var row = await repo.GetAsync(cmd.Id) - ?? throw new ManagementCommandException($"Secured write {cmd.Id} not found."); - - if (!string.Equals(row.Status, "Pending", StringComparison.Ordinal)) - throw new ManagementCommandException( - $"Secured write {cmd.Id} is '{row.Status}', not Pending; it cannot be approved."); + var row = await repo.GetAsync(cmd.Id); + if (row is null) + { + DenyUnresolvedTargetIfScoped(user, $"secured write {cmd.Id}"); + throw new ManagementCommandException($"Secured write {cmd.Id} not found."); + } // Site scope (arch-review R2 N3): the approving Verifier must be permitted on // the row's target site — checked BEFORE the TTL/self-approval/CAS chain so an // out-of-scope verifier can neither consume the Pending transition nor relay. + // + // It is also checked BEFORE the status test (arch-review 2026-08-19 P6-5): the + // "is 'Executed', not Pending" message told an out-of-scope caller both that the + // id existed and what had happened to it. An out-of-scope caller now gets the + // same scope denial for every id, existing or not. await EnforceSiteScopeForIdentifier(sp, user, row.SiteId); + if (!string.Equals(row.Status, "Pending", StringComparison.Ordinal)) + throw new ManagementCommandException( + $"Secured write {cmd.Id} is '{row.Status}', not Pending; it cannot be approved."); + // Server-side TTL: a Pending write older than the configured age is transitioned // to Expired and can never be approved — the stale setpoint is never relayed // (arch-review S2). Checked BEFORE the self-approval/CAS so an overdue row never @@ -1631,17 +1649,24 @@ public class ManagementActor : ReceiveActor IServiceProvider sp, RejectSecuredWriteCommand cmd, AuthenticatedUser user) { var repo = sp.GetRequiredService(); - var entity = await repo.GetAsync(cmd.Id) - ?? throw new ManagementCommandException($"Secured write {cmd.Id} not found."); + var entity = await repo.GetAsync(cmd.Id); + if (entity is null) + { + DenyUnresolvedTargetIfScoped(user, $"secured write {cmd.Id}"); + throw new ManagementCommandException($"Secured write {cmd.Id} not found."); + } + + // Site scope (arch-review R2 N3): the rejecting Verifier must be permitted on + // the row's target site — checked BEFORE the TTL/self-approval chain, and (as on + // approve) BEFORE the status test, so the not-found/status errors stop being an + // existence-and-status oracle for an out-of-scope caller (arch-review 2026-08-19 + // P6-5). The CAS below is unchanged. + await EnforceSiteScopeForIdentifier(sp, user, entity.SiteId); if (!string.Equals(entity.Status, "Pending", StringComparison.Ordinal)) throw new ManagementCommandException( $"Secured write {cmd.Id} is '{entity.Status}', not Pending; it cannot be rejected."); - // Site scope (arch-review R2 N3): the rejecting Verifier must be permitted on - // the row's target site — checked BEFORE the TTL/self-approval chain. - await EnforceSiteScopeForIdentifier(sp, user, entity.SiteId); - // Server-side TTL: an overdue Pending write expires here too, so a reject on a // stale row deterministically reports the expiry rather than recording a // human decision (arch-review S2). @@ -1902,12 +1927,38 @@ public class ManagementActor : ReceiveActor }; } - private static async Task HandleListDataConnections(IServiceProvider sp, ListDataConnectionsCommand cmd) + /// + /// Lists data connections, constrained to the caller's permitted sites. + /// + /// + /// Arch-review 2026-08-19 P6-4. The list used to take no user at all and return + /// EVERY site's connections, which nullified the row-98 fail-closed gate on + /// — Get denied exactly what List handed + /// over, and the secret-elided still + /// leaked site topology plus an id-existence oracle. Scoping now mirrors + /// : an explicit site filter is scope-checked like + /// any other site-keyed command, and an unfiltered list from a site-scoped + /// non-admin caller is narrowed to their permitted sites. System-wide callers + /// (Administrator, or a Deployer with no site grants) are unaffected. + /// + private static async Task HandleListDataConnections( + IServiceProvider sp, ListDataConnectionsCommand cmd, AuthenticatedUser user) { var repo = sp.GetRequiredService(); if (cmd.SiteId.HasValue) + { + EnforceSiteScope(user, cmd.SiteId.Value); return (await repo.GetDataConnectionsBySiteIdAsync(cmd.SiteId.Value)).Select(DataConnectionPublicShape).ToList(); - return (await repo.GetAllDataConnectionsAsync()).Select(DataConnectionPublicShape).ToList(); + } + + IEnumerable connections = await repo.GetAllDataConnectionsAsync(); + if (user.PermittedSiteIds.Length > 0 && !user.Roles.Contains(Roles.Administrator, StringComparer.OrdinalIgnoreCase)) + { + var permittedIds = new HashSet(user.PermittedSiteIds); + connections = connections.Where(c => permittedIds.Contains(c.SiteId.ToString())); + } + + return connections.Select(DataConnectionPublicShape).ToList(); } /// @@ -3631,9 +3682,17 @@ public class ManagementActor : ReceiveActor private static async Task HandleDebugSnapshot(IServiceProvider sp, DebugSnapshotCommand cmd, AuthenticatedUser user) { + // Fail-CLOSED before the not-found (arch-review 2026-08-19 P6-5): reporting + // "Instance {id} not found" ahead of the scope check made this handler an + // instance-id existence oracle for an out-of-scope caller. The scope denial is + // now the same answer whether or not the instance exists. var instanceRepo = sp.GetRequiredService(); - var instance = await instanceRepo.GetInstanceByIdAsync(cmd.InstanceId) - ?? throw new ManagementCommandException($"Instance {cmd.InstanceId} not found."); + var instance = await instanceRepo.GetInstanceByIdAsync(cmd.InstanceId); + if (instance is null) + { + DenyUnresolvedTargetIfScoped(user, $"instance {cmd.InstanceId}"); + throw new ManagementCommandException($"Instance {cmd.InstanceId} not found."); + } EnforceSiteScope(user, instance.SiteId); diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/DataConnectionsPageTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/DataConnectionsPageTests.cs index b6b51192..603e3628 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/DataConnectionsPageTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/DataConnectionsPageTests.cs @@ -40,6 +40,9 @@ public class DataConnectionsPageTests : BunitContext // Satisfy the page's [Inject] IDialogService — the host that actually // renders the dialog lives in MainLayout, not in bUnit's render scope. Services.AddScoped(); + // arch-review P6-4: the page now filters its site/connection tree through + // SiteScopeService, so it must be resolvable in every render test. + Services.AddScoped(); AddTestAuth(); // Default: no health known. Tests that assert badge state override this. @@ -271,4 +274,66 @@ public class DataConnectionsPageTests : BunitContext Assert.Contains(optionLabels, l => l.Contains("Plant-C")); Assert.DoesNotContain(optionLabels, l => l.Contains("Plant-A")); } + + // ── arch-review P6-4: site scoping. This page reads ISiteRepository directly, + // bypassing ManagementActor's scope gate, and Designer is NOT exempt from + // site scope — so a scoped Designer used to see every site's connections. ── + + [Fact] + public void SiteScoping_ScopedDesigner_SeesOnlyPermittedSitesAndConnections() + { + SeedRepos( + sites: new[] + { + new Site("Plant-A", "plant-a") { Id = 1 }, + new Site("Plant-B", "plant-b") { Id = 2 } + }, + connections: new[] + { + new DataConnection("PLC-A", "OpcUa", 1) { Id = 100 }, + new DataConnection("PLC-B", "OpcUa", 2) { Id = 200 } + }); + + var scopedUser = new ClaimsPrincipal(new ClaimsIdentity(new[] + { + new Claim(JwtTokenService.UsernameClaimType, "scoped"), + new Claim(JwtTokenService.RoleClaimType, "Designer"), + new Claim(JwtTokenService.SiteIdClaimType, "1"), // Plant-A only + }, "TestAuth")); + Services.AddSingleton(new TestAuthStateProvider(scopedUser)); + + var cut = Render(); + FindToggleForLabel(cut, "Plant-A")!.Click(); + + Assert.Contains("Plant-A", cut.Markup); + Assert.Contains("PLC-A", cut.Markup); + // Neither the out-of-scope site nor its connection may appear anywhere. + Assert.DoesNotContain("Plant-B", cut.Markup); + Assert.DoesNotContain("PLC-B", cut.Markup); + } + + [Fact] + public void SiteScoping_SystemWideUser_SeesEveryConnection() + { + // The unscoped case is unchanged: the default test principal is an + // Administrator (system-wide), so the filter is a no-op. + SeedRepos( + sites: new[] + { + new Site("Plant-A", "plant-a") { Id = 1 }, + new Site("Plant-B", "plant-b") { Id = 2 } + }, + connections: new[] + { + new DataConnection("PLC-A", "OpcUa", 1) { Id = 100 }, + new DataConnection("PLC-B", "OpcUa", 2) { Id = 200 } + }); + + var cut = Render(); + FindToggleForLabel(cut, "Plant-A")!.Click(); + FindToggleForLabel(cut, "Plant-B")!.Click(); + + Assert.Contains("PLC-A", cut.Markup); + Assert.Contains("PLC-B", cut.Markup); + } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs index 61dc64dd..1fdf4d50 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs @@ -1309,6 +1309,129 @@ public class ManagementActorTests : TestKit, IDisposable Assert.Equal(envelope.CorrelationId, response.CorrelationId); } + // ======================================================================== + // Site scoping: list filtering + resolve-before-scope oracles + // (arch-review 2026-08-19 P6-4 + P6-5) + // ======================================================================== + + /// + /// P6-4: the unfiltered list used to return EVERY site's connections to any + /// authenticated caller, handing a scoped caller exactly the rows + /// GetDataConnection denies them. + /// + [Fact] + public void ListDataConnections_SiteScopedUser_ReturnsOnlyPermittedSitesConnections() + { + var siteRepo = Substitute.For(); + siteRepo.GetAllDataConnectionsAsync(Arg.Any()) + .Returns(new List + { + new("ConnA", "OpcUa", 1) { Id = 1 }, + new("ConnB", "OpcUa", 2) { Id = 2 }, + }); + _services.AddScoped(_ => siteRepo); + + var actor = CreateActor(); + actor.Tell(ScopedEnvelope(new ListDataConnectionsCommand(null), new[] { "1" }, "Deployer")); + + var response = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Contains("ConnA", response.JsonData); + Assert.DoesNotContain("ConnB", response.JsonData); + } + + [Fact] + public void ListDataConnections_SystemWideUser_ReturnsEveryConnection() + { + var siteRepo = Substitute.For(); + siteRepo.GetAllDataConnectionsAsync(Arg.Any()) + .Returns(new List + { + new("ConnA", "OpcUa", 1) { Id = 1 }, + new("ConnB", "OpcUa", 2) { Id = 2 }, + }); + _services.AddScoped(_ => siteRepo); + + var actor = CreateActor(); + actor.Tell(Envelope(new ListDataConnectionsCommand(null), "Deployer")); + + var response = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Contains("ConnA", response.JsonData); + Assert.Contains("ConnB", response.JsonData); + } + + [Fact] + public void ListDataConnections_ScopedAdministrator_ReturnsEveryConnection() + { + // Administrator bypasses scope even with PermittedSiteIds set — same rule + // EnforceSiteScope applies everywhere else. + var siteRepo = Substitute.For(); + siteRepo.GetAllDataConnectionsAsync(Arg.Any()) + .Returns(new List + { + new("ConnA", "OpcUa", 1) { Id = 1 }, + new("ConnB", "OpcUa", 2) { Id = 2 }, + }); + _services.AddScoped(_ => siteRepo); + + var actor = CreateActor(); + actor.Tell(ScopedEnvelope(new ListDataConnectionsCommand(null), new[] { "1" }, "Administrator")); + + var response = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Contains("ConnA", response.JsonData); + Assert.Contains("ConnB", response.JsonData); + } + + [Fact] + public void ListDataConnections_ExplicitOutOfScopeSiteFilter_ReturnsUnauthorized() + { + var siteRepo = Substitute.For(); + siteRepo.GetDataConnectionsBySiteIdAsync(2, Arg.Any()) + .Returns(new List { new("ConnB", "OpcUa", 2) { Id = 2 } }); + _services.AddScoped(_ => siteRepo); + + var actor = CreateActor(); + var envelope = ScopedEnvelope(new ListDataConnectionsCommand(2), new[] { "1" }, "Deployer"); + + actor.Tell(envelope); + + var response = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Equal(envelope.CorrelationId, response.CorrelationId); + } + + /// + /// P6-5: the not-found check used to run before the scope check, so an + /// out-of-scope caller could tell a real instance id from a fake one by the + /// error they got back. Both cases must now answer with the same denial. + /// + [Fact] + public void DebugSnapshot_NonexistentInstanceForSiteScopedUser_ReturnsUnauthorized() + { + _templateRepo.GetInstanceByIdAsync(999, Arg.Any()) + .Returns((Instance?)null); + + var actor = CreateActor(); + var envelope = ScopedEnvelope(new DebugSnapshotCommand(999), new[] { "1" }, "Deployer"); + + actor.Tell(envelope); + + var response = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Equal(envelope.CorrelationId, response.CorrelationId); + } + + [Fact] + public void DebugSnapshot_NonexistentInstanceForSystemWideUser_StillReportsNotFound() + { + // The fail-closed arm must not change the unscoped caller's answer. + _templateRepo.GetInstanceByIdAsync(999, Arg.Any()) + .Returns((Instance?)null); + + var actor = CreateActor(); + actor.Tell(Envelope(new DebugSnapshotCommand(999), "Deployer")); + + var response = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Contains("not found", response.Error); + } + // ======================================================================== // Serialization (finding ManagementService-007) // diff --git a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/SecuredWriteHandlerTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/SecuredWriteHandlerTests.cs index 0c6cf54c..90ff3df5 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/SecuredWriteHandlerTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/SecuredWriteHandlerTests.cs @@ -1138,4 +1138,134 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable var response = ExpectMsg(TimeSpan.FromSeconds(5)); Assert.IsNotType(response); } + + // ------------------------------------------------------------------------ + // Resolve-before-scope oracles (arch-review 2026-08-19 P6-5) + // + // Submit/approve/reject used to answer "not found" (and, on approve/reject, + // "is 'Executed', not Pending") BEFORE the scope gate ran, so an out-of-scope + // caller could enumerate row ids and read their lifecycle status by the error + // shape alone. Every out-of-scope caller must now get the SAME scope denial + // whether or not the target exists, and whatever its status. + // ------------------------------------------------------------------------ + + /// Seeds a non-Pending (already executed) row for the status-oracle tests. + private void SeedExecutedWrite(long id) => + _securedWriteRepo.GetAsync(id, Arg.Any()).Returns(new PendingSecuredWrite + { + Id = id, SiteId = "SITE1", ConnectionName = "Gw1", TagPath = "Tag.A", + ValueJson = "true", ValueType = "Boolean", Status = "Executed", + OperatorUser = "alice", SubmittedAtUtc = DateTime.UtcNow, + }); + + [Fact] + public void Approve_SiteScopedVerifier_NonexistentId_Unauthorized_NotNotFound() + { + _securedWriteRepo.GetAsync(999, Arg.Any()) + .Returns((PendingSecuredWrite?)null); + + var actor = CreateActor(); + actor.Tell(ScopedEnvelope(new ApproveSecuredWriteCommand(999, "ok"), "bob", ["3"], "Verifier")); + + ExpectMsg(TimeSpan.FromSeconds(5)); + } + + [Fact] + public void Approve_SiteScopedVerifier_OutOfScopeExecutedRow_Unauthorized_NotStatusError() + { + SeedSiteWithConnection(1, "SITE1", "Gw1", "MxGateway"); + SeedExecutedWrite(42); + + var actor = CreateActor(); + actor.Tell(ScopedEnvelope(new ApproveSecuredWriteCommand(42, "ok"), "bob", ["3"], "Verifier")); + + // A ManagementError naming 'Executed' would tell the caller the row exists + // and what happened to it — the oracle this hoist closes. + ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Equal(0, _comms.CallCount); + } + + [Fact] + public void Approve_SystemWideVerifier_NonexistentId_StillReportsNotFound() + { + // The fail-closed arm must leave the unscoped caller's answers untouched. + _securedWriteRepo.GetAsync(999, Arg.Any()) + .Returns((PendingSecuredWrite?)null); + + var actor = CreateActor(); + actor.Tell(Envelope(new ApproveSecuredWriteCommand(999, "ok"), "bob", "Verifier")); + + var response = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Contains("not found", response.Error); + } + + [Fact] + public void Reject_SiteScopedVerifier_NonexistentId_Unauthorized_NotNotFound() + { + _securedWriteRepo.GetAsync(999, Arg.Any()) + .Returns((PendingSecuredWrite?)null); + + var actor = CreateActor(); + actor.Tell(ScopedEnvelope(new RejectSecuredWriteCommand(999, "no"), "bob", ["3"], "Verifier")); + + ExpectMsg(TimeSpan.FromSeconds(5)); + } + + [Fact] + public void Reject_SiteScopedVerifier_OutOfScopeExecutedRow_Unauthorized_NotStatusError() + { + SeedSiteWithConnection(1, "SITE1", "Gw1", "MxGateway"); + SeedExecutedWrite(42); + + var actor = CreateActor(); + actor.Tell(ScopedEnvelope(new RejectSecuredWriteCommand(42, "no"), "bob", ["3"], "Verifier")); + + ExpectMsg(TimeSpan.FromSeconds(5)); + _securedWriteRepo.DidNotReceiveWithAnyArgs().TryMarkRejectedAsync( + default, default!, default, default, default); + } + + [Fact] + public void Reject_SystemWideVerifier_ExecutedRow_StillReportsStatusConflict() + { + // In-scope/unscoped behavior is unchanged: the status error still names + // the status that blocked the transition (the P6-1 CAS shape is intact). + SeedExecutedWrite(42); + + var actor = CreateActor(); + actor.Tell(Envelope(new RejectSecuredWriteCommand(42, "no"), "bob", "Verifier")); + + var response = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Contains("Executed", response.Error); + } + + [Fact] + public void Submit_SiteScopedOperator_NonexistentSite_Unauthorized_NotNotFound() + { + _siteRepo.GetSiteByIdentifierAsync("GHOST", Arg.Any()) + .Returns((Site?)null); + + var actor = CreateActor(); + actor.Tell(ScopedEnvelope( + new SubmitSecuredWriteCommand("GHOST", "Gw1", "Tag.A", "true", "Boolean", "go"), + "alice", ["3"], "Operator")); + + ExpectMsg(TimeSpan.FromSeconds(5)); + _securedWriteRepo.DidNotReceiveWithAnyArgs().AddAsync(default!, default); + } + + [Fact] + public void Submit_SystemWideOperator_NonexistentSite_StillReportsNotFound() + { + _siteRepo.GetSiteByIdentifierAsync("GHOST", Arg.Any()) + .Returns((Site?)null); + + var actor = CreateActor(); + actor.Tell(Envelope( + new SubmitSecuredWriteCommand("GHOST", "Gw1", "Tag.A", "true", "Boolean", "go"), + "alice", "Operator")); + + var response = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Contains("not found", response.Error); + } }