diff --git a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs index d36d927f..98ec8964 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs @@ -1045,6 +1045,35 @@ public class ManagementActor : ReceiveActor await EmitSecuredWriteAuditAsync( sp, AuditKind.SecuredWriteApprove, AuditStatus.Submitted, row, actor: user.Username); + // D2 / T14 TOCTOU guard: re-assert the MxGateway protocol AT EXECUTE, not only at + // submit. The connection named on the row may have been reconfigured/recreated as a + // non-MxGateway (e.g. OPC UA) connection between submit and approval; relaying then + // would execute the secured write against a non-MxGateway adapter, violating the + // feature's core safety invariant. Re-load the connection with the same lookup + // submit uses (resolve site -> its data connections -> match by name) and fail the + // row deterministically (mirrors the decode-error containment below) if it is + // missing or no longer MxGateway — WITHOUT relaying. + var siteRepo = sp.GetRequiredService(); + var execSite = await siteRepo.GetSiteByIdentifierAsync(row.SiteId); + var execConn = execSite is null + ? null + : (await siteRepo.GetDataConnectionsBySiteIdAsync(execSite.Id)) + .FirstOrDefault(c => string.Equals(c.Name, row.ConnectionName, StringComparison.Ordinal)); + if (execConn is null || + !string.Equals(execConn.Protocol, "MxGateway", StringComparison.OrdinalIgnoreCase)) + { + row.Status = "Failed"; + row.ExecutedAtUtc = DateTime.UtcNow; + row.ExecutionError = execConn is null + ? $"connection '{row.ConnectionName}' not found" + : $"connection '{row.ConnectionName}' is no longer an MxGateway connection"; + await repo.UpdateAsync(row); + await EmitSecuredWriteAuditAsync( + sp, AuditKind.SecuredWriteExecute, AuditStatus.Failed, row, + actor: user.Username, errorMessage: row.ExecutionError); + return ToSecuredWriteDto(row); + } + // Validate the value type BEFORE attempting the relay. An unknown type can // never be decoded/written, so fail the row deterministically rather than // leaving it stuck Approved. (Addresses the C2 reviewer's deferred diff --git a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/SecuredWriteHandlerTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/SecuredWriteHandlerTests.cs index bf0a66c0..7ee7deb5 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/SecuredWriteHandlerTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/SecuredWriteHandlerTests.cs @@ -225,6 +225,12 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable SubmittedAtUtc = DateTime.UtcNow }; _securedWriteRepo.GetAsync(id, Arg.Any()).Returns(row); + // The approve handler re-asserts the MxGateway protocol AT EXECUTE (D2 / T14 + // TOCTOU guard), so the connection named on the row must still resolve to an + // MxGateway connection at approve-time for the relay to proceed. Seed it here + // so every approve test that should relay sees a valid MxGateway target; tests + // exercising the reconfigured/missing-connection case re-stub this lookup. + SeedSiteWithConnection(1, "SITE1", "Mx1", "MxGateway"); return row; } @@ -501,6 +507,75 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable Assert.Equal("unknown value type", updated.ExecutionError); } + [Fact] + public void Approve_WhenConnectionNoLongerMxGateway_FlipsStatusToFailed_NoRelay() + { + // D2 / T14 TOCTOU guard: the MxGateway protocol is re-asserted AT EXECUTE, not + // only at submit. The named connection was reconfigured/recreated as a + // non-MxGateway (OPC UA) connection between submit and approval; relaying then + // would execute the secured write against a non-MxGateway adapter, violating the + // feature's core safety invariant. The row must fail deterministically with the + // protocol error and NO relay — even though the CAS already won. + SeedPendingWrite(7, operatorUser: "alice"); + ArmCasSuccess(7); + // Re-stub the approve-time connection lookup to a non-MxGateway protocol, + // overriding the MxGateway seed inside SeedPendingWrite. + SeedSiteWithConnection(1, "SITE1", "Mx1", "OpcUa"); + var captured = CaptureAuditEvents(); + + PendingSecuredWrite? updated = null; + _securedWriteRepo + .When(r => r.UpdateAsync(Arg.Any(), Arg.Any())) + .Do(ci => updated = ci.Arg()); + + var actor = CreateActor(); + var envelope = Envelope(new ApproveSecuredWriteCommand(7, "approved"), "bob", "Verifier"); + + actor.Tell(envelope); + + ExpectMsg(TimeSpan.FromSeconds(5)); + // No relay — the connection is no longer an MxGateway target. + Assert.Equal(0, _comms.CallCount); + Assert.NotNull(updated); + Assert.Equal("Failed", updated!.Status); + Assert.NotNull(updated.ExecutedAtUtc); + Assert.Contains("no longer an MxGateway connection", updated.ExecutionError); + + // The Execute audit row records the failure (canonical Failure outcome). + WaitForAuditRows(captured, 2); + var execute = SingleOfKind(captured, AuditKind.SecuredWriteExecute); + Assert.Equal(ZB.MOM.WW.Audit.AuditOutcome.Failure, execute.Outcome); + } + + [Fact] + public void Approve_WhenConnectionDeleted_FlipsStatusToFailed_NotFound_NoRelay() + { + // TOCTOU companion: the connection named on the row was deleted between submit + // and approval. The re-load at execute returns no match → fail with "not found" + // and no relay. + SeedPendingWrite(7, operatorUser: "alice"); + ArmCasSuccess(7); + // Re-stub the approve-time lookup so the site resolves but exposes no connections. + _siteRepo.GetDataConnectionsBySiteIdAsync(1, Arg.Any()) + .Returns(new List()); + + PendingSecuredWrite? updated = null; + _securedWriteRepo + .When(r => r.UpdateAsync(Arg.Any(), Arg.Any())) + .Do(ci => updated = ci.Arg()); + + var actor = CreateActor(); + var envelope = Envelope(new ApproveSecuredWriteCommand(7, "approved"), "bob", "Verifier"); + + actor.Tell(envelope); + + ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Equal(0, _comms.CallCount); + Assert.NotNull(updated); + Assert.Equal("Failed", updated!.Status); + Assert.Contains("not found", updated.ExecutionError); + } + // ------------------------------------------------------------------------ // Audit emission (T14b — SecuredWrite AuditLog rows) // ------------------------------------------------------------------------