feat(worker): route alarm acks through the operator-authenticated AckMsg writer

AlarmCommandHandler -> AlarmDispatcher -> IAlarmAckWriter: the dispatcher resolves the ack item through the consumer and the writer authenticates the operator and writes the comment. The executor builds AlarmAckOperator from operator_user/domain/password, maps AlarmAckOutcome to the reply (Applied/Unconfirmed Ok, AlarmNotFound/NotAckable InvalidRequest, AuthenticationFailed/WriteRejected MxaccessFailure) and reports escaping exceptions by type and HRESULT only, so operator_password cannot leak. Removes the inert managed/WNAL alarm-client ack path. Claude-Session: https://claude.ai/code/session_016J6isfcgqy4SB2RK3ZvFvT
2026-10-19 19:35:55 -04:00
parent 0d4b2aea2d
commit d6db0ff40d
14 changed files with 695 additions and 1282 deletions
@@ -116,7 +116,7 @@ public sealed class AlarmCommandExecutorTests
    [Fact]
    public void AcknowledgeAlarm_WithHandler_RoutesNativeStatusIntoHresultAndPayload()
    {
-        FakeAlarmHandler handler = new FakeAlarmHandler { AcknowledgeReturn = 0 };
+        FakeAlarmHandler handler = new FakeAlarmHandler();
        MxAccessCommandExecutor executor = NewExecutor(handler);
        Guid g = Guid.NewGuid();

@@ -143,7 +143,10 @@ public sealed class AlarmCommandExecutorTests
        Assert.NotNull(reply.AcknowledgeAlarm);
        Assert.Equal(0, reply.AcknowledgeAlarm.NativeStatus);
        Assert.Equal(g, handler.LastAckGuid);
-        Assert.Equal("alice", handler.LastAckOperatorName);
+        Assert.Equal("ack", handler.LastComment);
+        Assert.Equal("alice", handler.LastOperator!.UserName);
+        Assert.Equal("CORP", handler.LastOperator.Domain);
+        Assert.Equal(string.Empty, handler.LastOperator.Password);
    }

    /// <summary>Verifies that invalid alarm GUID returns invalid request.</summary>
@@ -168,11 +171,14 @@ public sealed class AlarmCommandExecutorTests
        Assert.Equal(ProtocolStatusCode.InvalidRequest, reply.ProtocolStatus.Code);
    }

-    /// <summary>Verifies that nonzero native status carries a diagnostic message.</summary>
+    /// <summary>Verifies that a rejected write carries its native status and diagnostic.</summary>
    [Fact]
    public void AcknowledgeAlarm_WithNonzeroNativeStatus_CarriesDiagnostic()
    {
-        FakeAlarmHandler handler = new FakeAlarmHandler { AcknowledgeReturn = -123 };
+        FakeAlarmHandler handler = new FakeAlarmHandler
+        {
+            AckResult = AlarmAckResult.Create(AlarmAckOutcome.WriteRejected, -123, "AckMsg write rejected: detail -123."),
+        };
        MxAccessCommandExecutor executor = NewExecutor(handler);

        StaCommand command = new StaCommand(
@@ -188,7 +194,9 @@ public sealed class AlarmCommandExecutorTests

        MxCommandReply reply = executor.Execute(command);

+        Assert.Equal(ProtocolStatusCode.MxaccessFailure, reply.ProtocolStatus.Code);
        Assert.Equal(-123, reply.Hresult);
+        Assert.Equal(-123, reply.AcknowledgeAlarm.NativeStatus);
        Assert.Contains("-123", reply.DiagnosticMessage);
    }

@@ -196,7 +204,7 @@ public sealed class AlarmCommandExecutorTests
    [Fact]
    public void AcknowledgeAlarmByName_WithHandler_RoutesTupleToHandler()
    {
-        FakeAlarmHandler handler = new FakeAlarmHandler { AcknowledgeReturn = 0 };
+        FakeAlarmHandler handler = new FakeAlarmHandler();
        MxAccessCommandExecutor executor = NewExecutor(handler);

        StaCommand command = new StaCommand(
@@ -223,7 +231,133 @@ public sealed class AlarmCommandExecutorTests
        Assert.Equal("TestMachine_001.TestAlarm001", handler.LastAckByNameTuple!.Value.Name);
        Assert.Equal("Galaxy", handler.LastAckByNameTuple!.Value.Provider);
        Assert.Equal("TestArea", handler.LastAckByNameTuple!.Value.Group);
-        Assert.Equal("alice", handler.LastAckOperatorName);
+        Assert.Equal("ack", handler.LastComment);
+        Assert.Equal("alice", handler.LastOperator!.UserName);
+    }
+
+    /// <summary>
+    ///     Every ack outcome maps to its protocol status, carries the native
+    ///     status, and never echoes the operator password.
+    /// </summary>
+    /// <param name="outcome">The writer outcome.</param>
+    /// <param name="expected">The expected protocol status.</param>
+    [Theory]
+    [InlineData(AlarmAckOutcome.Applied, ProtocolStatusCode.Ok)]
+    [InlineData(AlarmAckOutcome.Unconfirmed, ProtocolStatusCode.Ok)]
+    [InlineData(AlarmAckOutcome.AlarmNotFound, ProtocolStatusCode.InvalidRequest)]
+    [InlineData(AlarmAckOutcome.NotAckable, ProtocolStatusCode.InvalidRequest)]
+    [InlineData(AlarmAckOutcome.AuthenticationFailed, ProtocolStatusCode.MxaccessFailure)]
+    [InlineData(AlarmAckOutcome.WriteRejected, ProtocolStatusCode.MxaccessFailure)]
+    public void AcknowledgeAlarm_MapsOutcomeWithoutLeakingPassword(AlarmAckOutcome outcome, ProtocolStatusCode expected)
+    {
+        int native = outcome is AlarmAckOutcome.Applied or AlarmAckOutcome.Unconfirmed ? 0 : 1008;
+        FakeAlarmHandler handler = new FakeAlarmHandler
+        {
+            AckResult = AlarmAckResult.Create(outcome, native, "diagnostic for alice"),
+        };
+
+        MxCommandReply reply = Execute(handler, new MxCommand
+        {
+            Kind = MxCommandKind.AcknowledgeAlarm,
+            AcknowledgeAlarmCommand = new AcknowledgeAlarmCommand
+            {
+                AlarmGuid = Guid.NewGuid().ToString(),
+                Comment = "checked",
+                OperatorUser = "alice",
+                OperatorDomain = "CORP",
+                OperatorPassword = "pw-secret",
+            },
+        });
+
+        AssertAckReply(reply, handler, expected, native);
+    }
+
+    /// <summary>The by-name ack maps outcomes identically and never echoes the operator password.</summary>
+    /// <param name="outcome">The writer outcome.</param>
+    /// <param name="expected">The expected protocol status.</param>
+    [Theory]
+    [InlineData(AlarmAckOutcome.Applied, ProtocolStatusCode.Ok)]
+    [InlineData(AlarmAckOutcome.Unconfirmed, ProtocolStatusCode.Ok)]
+    [InlineData(AlarmAckOutcome.AlarmNotFound, ProtocolStatusCode.InvalidRequest)]
+    [InlineData(AlarmAckOutcome.NotAckable, ProtocolStatusCode.InvalidRequest)]
+    [InlineData(AlarmAckOutcome.AuthenticationFailed, ProtocolStatusCode.MxaccessFailure)]
+    [InlineData(AlarmAckOutcome.WriteRejected, ProtocolStatusCode.MxaccessFailure)]
+    public void AcknowledgeAlarmByName_MapsOutcomeWithoutLeakingPassword(AlarmAckOutcome outcome, ProtocolStatusCode expected)
+    {
+        int native = outcome is AlarmAckOutcome.Applied or AlarmAckOutcome.Unconfirmed ? 0 : 1008;
+        FakeAlarmHandler handler = new FakeAlarmHandler
+        {
+            AckResult = AlarmAckResult.Create(outcome, native, "diagnostic for alice"),
+        };
+
+        MxCommandReply reply = Execute(handler, new MxCommand
+        {
+            Kind = MxCommandKind.AcknowledgeAlarmByName,
+            AcknowledgeAlarmByNameCommand = new AcknowledgeAlarmByNameCommand
+            {
+                AlarmName = "TestMachine_001.TestAlarm001",
+                ProviderName = "Galaxy",
+                GroupName = "TestArea",
+                Comment = "checked",
+                OperatorUser = "alice",
+                OperatorDomain = "CORP",
+                OperatorPassword = "pw-secret",
+            },
+        });
+
+        AssertAckReply(reply, handler, expected, native);
+    }
+
+    /// <summary>The writer's OnWriteComplete statuses are copied onto the reply.</summary>
+    [Fact]
+    public void AcknowledgeAlarm_CopiesWriteStatusesIntoReply()
+    {
+        MxStatusProxy status = new MxStatusProxy { Category = MxStatusCategory.Ok, Success = 1 };
+        FakeAlarmHandler handler = new FakeAlarmHandler
+        {
+            AckResult = AlarmAckResult.Create(AlarmAckOutcome.Applied, 0, string.Empty, new[] { status }),
+        };
+
+        MxCommandReply reply = Execute(handler, new MxCommand
+        {
+            Kind = MxCommandKind.AcknowledgeAlarm,
+            AcknowledgeAlarmCommand = new AcknowledgeAlarmCommand { AlarmGuid = Guid.NewGuid().ToString(), OperatorUser = "alice" },
+        });
+
+        Assert.Equal(status, Assert.Single(reply.Statuses));
+    }
+
+    /// <summary>
+    ///     An exception escaping the ack path (e.g. the writer's MXAccess
+    ///     Register failing) maps to MxaccessFailure with only the exception type
+    ///     and HRESULT — never the exception text, which is not under our control.
+    /// </summary>
+    [Fact]
+    public void AcknowledgeAlarmByName_WhenHandlerThrows_ReportsTypeAndHresultOnly()
+    {
+        FakeAlarmHandler handler = new FakeAlarmHandler
+        {
+            AcknowledgeException = new System.Runtime.InteropServices.COMException("echo pw-secret", unchecked((int)0x80004005)),
+        };
+
+        MxCommandReply reply = Execute(handler, new MxCommand
+        {
+            Kind = MxCommandKind.AcknowledgeAlarmByName,
+            AcknowledgeAlarmByNameCommand = new AcknowledgeAlarmByNameCommand
+            {
+                AlarmName = "TestMachine_001.TestAlarm001",
+                ProviderName = "Galaxy",
+                OperatorUser = "alice",
+                OperatorPassword = "pw-secret",
+            },
+        });
+
+        Assert.Equal(ProtocolStatusCode.MxaccessFailure, reply.ProtocolStatus.Code);
+        Assert.Equal(unchecked((int)0x80004005), reply.Hresult);
+        Assert.Contains("COMException", reply.DiagnosticMessage, StringComparison.Ordinal);
+        Assert.Contains("0x80004005", reply.DiagnosticMessage, StringComparison.Ordinal);
+        Assert.DoesNotContain("pw-secret", reply.DiagnosticMessage, StringComparison.Ordinal);
+        Assert.DoesNotContain("pw-secret", reply.ProtocolStatus.Message, StringComparison.Ordinal);
    }

    /// <summary>Verifies that empty alarm name returns invalid request.</summary>
@@ -393,7 +527,10 @@ public sealed class AlarmCommandExecutorTests
    [Fact]
    public void AcknowledgeAlarm_WhenHandlerThrows_ReturnsMxaccessFailure()
    {
-        FakeAlarmHandler handler = new FakeAlarmHandler { AcknowledgeThrow = true };
+        FakeAlarmHandler handler = new FakeAlarmHandler
+        {
+            AcknowledgeException = new InvalidOperationException("simulated alarm-handler failure"),
+        };
        MxAccessCommandExecutor executor = NewExecutor(handler);

        StaCommand command = new StaCommand(
@@ -410,7 +547,25 @@ public sealed class AlarmCommandExecutorTests
        MxCommandReply reply = executor.Execute(command);

        Assert.Equal(ProtocolStatusCode.MxaccessFailure, reply.ProtocolStatus.Code);
-        Assert.Contains("simulated", reply.DiagnosticMessage);
+        Assert.Contains("InvalidOperationException", reply.DiagnosticMessage, StringComparison.Ordinal);
+        Assert.DoesNotContain("simulated", reply.DiagnosticMessage, StringComparison.Ordinal);
+    }
+
+    private static MxCommandReply Execute(IAlarmCommandHandler handler, MxCommand command) =>
+        NewExecutor(handler).Execute(new StaCommand(SessionId, CorrelationId, command));
+
+    private static void AssertAckReply(MxCommandReply reply, FakeAlarmHandler handler, ProtocolStatusCode expected, int native)
+    {
+        Assert.Equal(expected, reply.ProtocolStatus.Code);
+        Assert.Equal(native, reply.Hresult);
+        Assert.Equal(native, reply.AcknowledgeAlarm.NativeStatus);
+        Assert.Equal("diagnostic for alice", reply.DiagnosticMessage);
+        Assert.Equal("checked", handler.LastComment);
+        Assert.Equal("alice", handler.LastOperator!.UserName);
+        Assert.Equal("CORP", handler.LastOperator.Domain);
+        Assert.Equal("pw-secret", handler.LastOperator.Password);
+        Assert.DoesNotContain("pw-secret", reply.DiagnosticMessage ?? string.Empty, StringComparison.Ordinal);
+        Assert.DoesNotContain("pw-secret", reply.ProtocolStatus.Message ?? string.Empty, StringComparison.Ordinal);
    }

    private static MxAccessCommandExecutor NewExecutor(IAlarmCommandHandler? alarmHandler)
@@ -442,14 +597,17 @@ public sealed class AlarmCommandExecutorTests
        /// <summary>Gets the last acknowledge alarm GUID.</summary>
        public Guid LastAckGuid { get; private set; }

-        /// <summary>Gets the last acknowledge operator name.</summary>
-        public string? LastAckOperatorName { get; private set; }
+        /// <summary>Gets the last acknowledge comment.</summary>
+        public string? LastComment { get; private set; }

-        /// <summary>Gets or sets the value returned by acknowledge.</summary>
-        public int AcknowledgeReturn { get; set; }
+        /// <summary>Gets the last acknowledge operator.</summary>
+        public AlarmAckOperator? LastOperator { get; private set; }

-        /// <summary>Gets or sets a value indicating whether acknowledge throws.</summary>
-        public bool AcknowledgeThrow { get; set; }
+        /// <summary>Gets or sets the result returned by both acknowledge methods.</summary>
+        public AlarmAckResult AckResult { get; set; } = AlarmAckResult.Create(AlarmAckOutcome.Applied, 0, string.Empty);
+
+        /// <summary>Gets or sets an exception both acknowledge methods throw.</summary>
+        public Exception? AcknowledgeException { get; set; }

        /// <summary>Gets or sets the query result snapshots.</summary>
        public IReadOnlyList<ActiveAlarmSnapshot> QueryResult { get; set; } =
@@ -484,28 +642,32 @@ public sealed class AlarmCommandExecutorTests
        }

        /// <inheritdoc />
-        public int Acknowledge(
-            Guid alarmGuid, string comment, string operatorUser,
-            string operatorNode, string operatorDomain, string operatorFullName)
+        public AlarmAckResult Acknowledge(Guid alarmGuid, string comment, AlarmAckOperator ackOperator)
        {
            LastAckGuid = alarmGuid;
-            LastAckOperatorName = operatorUser;
-            if (AcknowledgeThrow)
+            LastComment = comment;
+            LastOperator = ackOperator;
+            if (AcknowledgeException is not null)
            {
-                throw new InvalidOperationException("simulated alarm-handler failure");
+                throw AcknowledgeException;
            }
-            return AcknowledgeReturn;
+
+            return AckResult;
        }

        /// <inheritdoc />
-        public int AcknowledgeByName(
-            string alarmName, string providerName, string groupName,
-            string comment, string operatorUser, string operatorNode,
-            string operatorDomain, string operatorFullName)
+        public AlarmAckResult AcknowledgeByName(
+            string alarmName, string providerName, string groupName, string comment, AlarmAckOperator ackOperator)
        {
            LastAckByNameTuple = (alarmName, providerName, groupName);
-            LastAckOperatorName = operatorUser;
-            return AcknowledgeReturn;
+            LastComment = comment;
+            LastOperator = ackOperator;
+            if (AcknowledgeException is not null)
+            {
+                throw AcknowledgeException;
+            }
+
+            return AckResult;
        }

        /// <summary>Gets the last acknowledge by name tuple.</summary>