Merge branch 'worktree-agent-aa92b89ae2edc1ead' into fix/flow-debugger-arch-review

src/ZB.MOM.WW.ScadaBridge.InboundAPI/InboundScriptExecutor.cs

@@ -4,6 +4,8 @@ using Microsoft.CodeAnalysis.CSharp.Scripting;
 using Microsoft.CodeAnalysis.Scripting;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Options;
+using ZB.MOM.WW.ScadaBridge.AuditLog.Configuration;
 using ZB.MOM.WW.ScadaBridge.Commons.Entities.InboundApi;
 using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
 using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
@@ -411,12 +413,23 @@ public class InboundScriptExecutor
             // Also bind the inbound request's ExecutionId (ParentExecutionId) so a
             // routed call carries it as ParentExecutionId — the
             // spawned site script execution points back at this inbound request.
+            // Arch-review F11: the always-on RouteCall row must honour the method's
+            // SkipBodyCapture override — the same per-target lookup AuditWriteMiddleware
+            // performs for the inbound row (AuditWriteMiddleware.cs:183-187) — so an
+            // operator who suppressed a method's body from the audit log doesn't get it
+            // back via the routed call's arguments (derived from that same body).
+            var auditOptions = scope?.ServiceProvider.GetService<IOptionsMonitor<AuditLogOptions>>()?.CurrentValue;
+            var skipBodyCapture = auditOptions != null
+                && auditOptions.PerTargetOverrides.TryGetValue(method.Name, out var perTargetOverride)
+                && perTargetOverride.SkipBodyCapture;
+
             var context = new InboundScriptContext(
                 parameters,
                 route.WithDeadline(cts.Token)
                      .WithRequestAborted(cancellationToken)
                      .WithParentExecutionId(parentExecutionId)
-                     .WithTrace(trace),
+                     .WithTrace(trace)
+                     .WithBodyCapture(!skipBodyCapture),
                 dbHelper,
                 cts.Token);
 

src/ZB.MOM.WW.ScadaBridge.InboundAPI/RouteHelper.cs

@@ -86,6 +86,7 @@ public class RouteHelper
     private readonly bool _trace;
     private readonly Action<AuditEvent>? _traceSink;
     private readonly IReadOnlySet<string>? _permittedSiteIdentifiers;
+    private readonly bool _captureBodies;
 
     /// <summary>
     /// Initializes a new <see cref="RouteHelper"/> with no deadline token and no parent execution id.
@@ -102,7 +103,7 @@ public class RouteHelper
         ICentralAuditWriter? auditWriter = null,
         INodeIdentityProvider? nodeIdentity = null)
         : this(instanceLocator, instanceRouter, CancellationToken.None, CancellationToken.None, parentExecutionId: null,
-               auditWriter, nodeIdentity, trace: false, traceSink: null, permittedSiteIdentifiers: null)
+               auditWriter, nodeIdentity, trace: false, traceSink: null, permittedSiteIdentifiers: null, captureBodies: true)
     {
     }
 
@@ -116,7 +117,8 @@ public class RouteHelper
         INodeIdentityProvider? nodeIdentity,
         bool trace,
         Action<AuditEvent>? traceSink,
-        IReadOnlySet<string>? permittedSiteIdentifiers)
+        IReadOnlySet<string>? permittedSiteIdentifiers,
+        bool captureBodies)
     {
         _permittedSiteIdentifiers = permittedSiteIdentifiers;
         _instanceLocator = instanceLocator;
@@ -128,6 +130,7 @@ public class RouteHelper
         _nodeIdentity = nodeIdentity;
         _trace = trace;
         _traceSink = traceSink;
+        _captureBodies = captureBodies;
     }
 
     /// <summary>
@@ -141,7 +144,7 @@ public class RouteHelper
     /// <returns>A new <see cref="RouteHelper"/> inheriting the given deadline token.</returns>
     public RouteHelper WithDeadline(CancellationToken deadlineToken) =>
         new(_instanceLocator, _instanceRouter, deadlineToken, _requestAbortedToken, _parentExecutionId,
-            _auditWriter, _nodeIdentity, _trace, _traceSink, _permittedSiteIdentifiers);
+            _auditWriter, _nodeIdentity, _trace, _traceSink, _permittedSiteIdentifiers, _captureBodies);
 
     /// <summary>
     /// Returns a <see cref="RouteHelper"/> carrying the raw request-abort
@@ -155,7 +158,7 @@ public class RouteHelper
     /// <returns>A new <see cref="RouteHelper"/> carrying the given request-abort token.</returns>
     public RouteHelper WithRequestAborted(CancellationToken requestAbortedToken) =>
         new(_instanceLocator, _instanceRouter, _deadlineToken, requestAbortedToken, _parentExecutionId,
-            _auditWriter, _nodeIdentity, _trace, _traceSink, _permittedSiteIdentifiers);
+            _auditWriter, _nodeIdentity, _trace, _traceSink, _permittedSiteIdentifiers, _captureBodies);
 
     /// <summary>
     /// Audit Log (ParentExecutionId): returns a <see cref="RouteHelper"/> whose
@@ -170,7 +173,7 @@ public class RouteHelper
     /// <returns>A new <see cref="RouteHelper"/> carrying the given parent execution id.</returns>
     public RouteHelper WithParentExecutionId(Guid? parentExecutionId) =>
         new(_instanceLocator, _instanceRouter, _deadlineToken, _requestAbortedToken, parentExecutionId,
-            _auditWriter, _nodeIdentity, _trace, _traceSink, _permittedSiteIdentifiers);
+            _auditWriter, _nodeIdentity, _trace, _traceSink, _permittedSiteIdentifiers, _captureBodies);
 
     /// <summary>
     /// Flow Debugger (design 2026-09-05 §5.2): returns a <see cref="RouteHelper"/> whose routed
@@ -181,7 +184,7 @@ public class RouteHelper
     /// <returns>A new <see cref="RouteHelper"/> carrying the flag.</returns>
     public RouteHelper WithTrace(bool trace) =>
         new(_instanceLocator, _instanceRouter, _deadlineToken, _requestAbortedToken, _parentExecutionId,
-            _auditWriter, _nodeIdentity, trace, _traceSink, _permittedSiteIdentifiers);
+            _auditWriter, _nodeIdentity, trace, _traceSink, _permittedSiteIdentifiers, _captureBodies);
 
     /// <summary>
     /// Flow Debugger (design 2026-09-05 §7.1): returns a <see cref="RouteHelper"/> that also
@@ -193,7 +196,7 @@ public class RouteHelper
     /// <returns>A new <see cref="RouteHelper"/> carrying the sink.</returns>
     public RouteHelper WithTraceSink(Action<AuditEvent>? traceSink) =>
         new(_instanceLocator, _instanceRouter, _deadlineToken, _requestAbortedToken, _parentExecutionId,
-            _auditWriter, _nodeIdentity, _trace, traceSink, _permittedSiteIdentifiers);
+            _auditWriter, _nodeIdentity, _trace, traceSink, _permittedSiteIdentifiers, _captureBodies);
 
     /// <summary>
     /// Flow Debugger outbound calls (plan 2026-09-06, D5/D7): the trace sink this helper carries,
@@ -227,7 +230,22 @@ public class RouteHelper
     /// <returns>A new <see cref="RouteHelper"/> carrying the scope.</returns>
     public RouteHelper WithSiteScope(IReadOnlySet<string>? permittedSiteIdentifiers) =>
         new(_instanceLocator, _instanceRouter, _deadlineToken, _requestAbortedToken, _parentExecutionId,
-            _auditWriter, _nodeIdentity, _trace, _traceSink, permittedSiteIdentifiers);
+            _auditWriter, _nodeIdentity, _trace, _traceSink, permittedSiteIdentifiers, _captureBodies);
+
+    /// <summary>
+    /// Arch-review F11: returns a <see cref="RouteHelper"/> whose always-on <c>RouteCall</c> row
+    /// carries no request/response summary when <paramref name="captureBodies"/> is <c>false</c>.
+    /// <see cref="InboundScriptExecutor"/> sets this from the executing method's
+    /// <c>AuditLogOptions.PerTargetOverrides[method.Name].SkipBodyCapture</c> — the same override
+    /// <c>AuditWriteMiddleware</c> honours for the inbound row — so an operator who suppressed a
+    /// method's body from the audit log doesn't get it back via the routed call's arguments
+    /// (which are derived from that same body). Default <c>true</c> (unchanged behaviour).
+    /// </summary>
+    /// <param name="captureBodies">False to omit the request/response summary on the <c>RouteCall</c> row.</param>
+    /// <returns>A new <see cref="RouteHelper"/> carrying the flag.</returns>
+    public RouteHelper WithBodyCapture(bool captureBodies) =>
+        new(_instanceLocator, _instanceRouter, _deadlineToken, _requestAbortedToken, _parentExecutionId,
+            _auditWriter, _nodeIdentity, _trace, _traceSink, _permittedSiteIdentifiers, captureBodies);
 
     /// <summary>
     /// Creates a route target for the specified instance.
@@ -238,7 +256,7 @@ public class RouteHelper
     {
         return new RouteTarget(
             instanceCode, _instanceLocator, _instanceRouter, _deadlineToken, _requestAbortedToken, _parentExecutionId,
-            _auditWriter, _nodeIdentity, _trace, _traceSink, _permittedSiteIdentifiers);
+            _auditWriter, _nodeIdentity, _trace, _traceSink, _permittedSiteIdentifiers, _captureBodies);
     }
 }
 
@@ -258,6 +276,7 @@ public class RouteTarget
     private readonly bool _trace;
     private readonly Action<AuditEvent>? _traceSink;
     private readonly IReadOnlySet<string>? _permittedSiteIdentifiers;
+    private readonly bool _captureBodies;
 
     /// <summary>
     /// Initializes a new <see cref="RouteTarget"/> for the specified instance.
@@ -273,6 +292,7 @@ public class RouteTarget
     /// <param name="trace">True to trace the routed site run.</param>
     /// <param name="traceSink">Receives the emitted audit events in order; null = none.</param>
     /// <param name="permittedSiteIdentifiers">Arch-review F2: site identifiers this call may reach; null = unscoped.</param>
+    /// <param name="captureBodies">Arch-review F11: false omits the request/response summary on the <c>RouteCall</c> row.</param>
     internal RouteTarget(
         string instanceCode,
         IInstanceLocator instanceLocator,
@@ -284,7 +304,8 @@ public class RouteTarget
         INodeIdentityProvider? nodeIdentity = null,
         bool trace = false,
         Action<AuditEvent>? traceSink = null,
-        IReadOnlySet<string>? permittedSiteIdentifiers = null)
+        IReadOnlySet<string>? permittedSiteIdentifiers = null,
+        bool captureBodies = true)
     {
         _permittedSiteIdentifiers = permittedSiteIdentifiers;
         _instanceCode = instanceCode;
@@ -297,6 +318,7 @@ public class RouteTarget
         _nodeIdentity = nodeIdentity;
         _trace = trace;
         _traceSink = traceSink;
+        _captureBodies = captureBodies;
     }
 
     /// <summary>
@@ -385,8 +407,10 @@ public class RouteTarget
                 parentExecutionId: null,
                 durationMs: (int)Math.Min(elapsedMs, int.MaxValue),
                 errorMessage: error,
-                requestSummary: TraceJson(args),
-                responseSummary: TraceJson(returnValue),
+                // Arch-review F11: a method with SkipBodyCapture set must not leak the routed
+                // call's arguments (derived from the same request body) or its return value.
+                requestSummary: _captureBodies ? TraceJson(args) : null,
+                responseSummary: _captureBodies ? TraceJson(returnValue) : null,
                 extra: childExecutionId is null ? null : new ScriptTraceExtra { ChildExecutionId = childExecutionId }.ToJson());
         }
         catch (Exception)

tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/InboundScriptExecutorTests.cs

@@ -4,10 +4,13 @@ using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Logging.Abstractions;
 using NSubstitute;
+using ZB.MOM.WW.Audit;
+using ZB.MOM.WW.ScadaBridge.AuditLog.Configuration;
 using ZB.MOM.WW.ScadaBridge.Commons.Entities.InboundApi;
 using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
 using ZB.MOM.WW.ScadaBridge.Commons.Messages.InboundApi;
 using ZB.MOM.WW.ScadaBridge.Commons.Types;
+using ZB.MOM.WW.ScadaBridge.Commons.Types.Audit;
 
 namespace ZB.MOM.WW.ScadaBridge.InboundAPI.Tests;
 
@@ -57,6 +60,77 @@ public class InboundScriptExecutorTests
         Assert.False(captured[1].Trace);
     }
 
+    [Fact]
+    public async Task Executor_MethodWithSkipBodyCapture_RoutesWithBodyCaptureOff()
+    {
+        // Arch-review F11: a method configured with PerTargetOverrides[method].SkipBodyCapture
+        // must build its Route helper WithBodyCapture(false), so the always-on RouteCall row
+        // carries no request/response summary — mirroring AuditWriteMiddleware's own
+        // SkipBodyCapture lookup for the inbound row (AuditWriteMiddleware.cs:183-187).
+        var locator = Substitute.For<IInstanceLocator>();
+        var router = Substitute.For<IInstanceRouter>();
+        locator.GetSiteIdForInstanceAsync("inst", Arg.Any<CancellationToken>()).Returns("SiteA");
+        router.RouteToCallAsync("SiteA", Arg.Any<RouteToCallRequest>(), Arg.Any<CancellationToken>())
+            .Returns(ci => new RouteToCallResponse(((RouteToCallRequest)ci[1]).CorrelationId, true, 1, null, DateTimeOffset.UtcNow));
+
+        var writer = Substitute.For<ICentralAuditWriter>();
+        AuditEvent? written = null;
+        writer.WriteAsync(Arg.Do<AuditEvent>(e => written = e), Arg.Any<CancellationToken>())
+            .Returns(Task.CompletedTask);
+        var route = new RouteHelper(locator, router, writer, null);
+
+        var services = new ServiceCollection();
+        services.Configure<AuditLogOptions>(o =>
+            o.PerTargetOverrides["routed"] = new PerTargetRedactionOverride { SkipBodyCapture = true });
+        using var provider = services.BuildServiceProvider();
+
+        var executor = new InboundScriptExecutor(NullLogger<InboundScriptExecutor>.Instance, provider);
+        var method = new ApiMethod("routed", "x") { Id = 9, TimeoutSeconds = 10 };
+        executor.RegisterHandler("routed", ctx => ctx.Route.To("inst").Call("work", new { x = 1 }));
+
+        var result = await executor.ExecuteAsync(method, new Dictionary<string, object?>(), route, TimeSpan.FromSeconds(10));
+
+        Assert.True(result.Success, result.ErrorMessage);
+        Assert.NotNull(written);
+        var row = written!.AsRow();
+        Assert.Null(row.RequestSummary);
+        Assert.Null(row.ResponseSummary);
+    }
+
+    [Fact]
+    public async Task Executor_MethodWithoutSkipBodyCapture_RoutesWithBodyCaptureOn()
+    {
+        // Control: no PerTargetOverrides entry for this method → default behaviour unchanged.
+        var locator = Substitute.For<IInstanceLocator>();
+        var router = Substitute.For<IInstanceRouter>();
+        locator.GetSiteIdForInstanceAsync("inst", Arg.Any<CancellationToken>()).Returns("SiteA");
+        router.RouteToCallAsync("SiteA", Arg.Any<RouteToCallRequest>(), Arg.Any<CancellationToken>())
+            .Returns(ci => new RouteToCallResponse(((RouteToCallRequest)ci[1]).CorrelationId, true, 1, null, DateTimeOffset.UtcNow));
+
+        var writer = Substitute.For<ICentralAuditWriter>();
+        AuditEvent? written = null;
+        writer.WriteAsync(Arg.Do<AuditEvent>(e => written = e), Arg.Any<CancellationToken>())
+            .Returns(Task.CompletedTask);
+        var route = new RouteHelper(locator, router, writer, null);
+
+        var services = new ServiceCollection();
+        services.Configure<AuditLogOptions>(o =>
+            o.PerTargetOverrides["other-method"] = new PerTargetRedactionOverride { SkipBodyCapture = true });
+        using var provider = services.BuildServiceProvider();
+
+        var executor = new InboundScriptExecutor(NullLogger<InboundScriptExecutor>.Instance, provider);
+        var method = new ApiMethod("routed", "x") { Id = 9, TimeoutSeconds = 10 };
+        executor.RegisterHandler("routed", ctx => ctx.Route.To("inst").Call("work", new { x = 1 }));
+
+        var result = await executor.ExecuteAsync(method, new Dictionary<string, object?>(), route, TimeSpan.FromSeconds(10));
+
+        Assert.True(result.Success, result.ErrorMessage);
+        Assert.NotNull(written);
+        var row = written!.AsRow();
+        Assert.Contains("\"x\":1", row.RequestSummary);
+        Assert.Equal("1", row.ResponseSummary);
+    }
+
     [Fact]
     public async Task RegisteredHandler_ExecutesSuccessfully()
     {

tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/RouteHelperTests.cs

@@ -1,7 +1,9 @@
 using NSubstitute;
+using ZB.MOM.WW.Audit;
 using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
 using ZB.MOM.WW.ScadaBridge.Commons.Messages.InboundApi;
 using ZB.MOM.WW.ScadaBridge.Commons.Types;
+using ZB.MOM.WW.ScadaBridge.Commons.Types.Audit;
 
 namespace ZB.MOM.WW.ScadaBridge.InboundAPI.Tests;
 
@@ -703,4 +705,81 @@ public class RouteHelperTests
         await Assert.ThrowsAsync<RouteSiteScopeException>(
             () => helper.To("inst-1").Call("doWork"));
     }
+
+    // --- Arch-review F11: WithBodyCapture ---
+
+    [Fact]
+    public async Task RouteCallRow_WithBodyCaptureOff_HasNoRequestOrResponseSummary()
+    {
+        SiteResolves("inst-1", "SiteA");
+        _router.RouteToCallAsync("SiteA", Arg.Any<RouteToCallRequest>(), Arg.Any<CancellationToken>())
+            .Returns(ci => new RouteToCallResponse(
+                ((RouteToCallRequest)ci[1]).CorrelationId, true, 99, null, DateTimeOffset.UtcNow));
+        var writer = Substitute.For<ICentralAuditWriter>();
+        AuditEvent? written = null;
+        writer.WriteAsync(Arg.Do<AuditEvent>(e => written = e), Arg.Any<CancellationToken>())
+            .Returns(Task.CompletedTask);
+        var helper = new RouteHelper(_locator, _router, writer, null).WithBodyCapture(false);
+
+        var result = await helper.To("inst-1").Call("doWork", new { x = 1 });
+
+        Assert.Equal(99, result);
+        Assert.NotNull(written);
+        var row = written!.AsRow();
+        Assert.Null(row.RequestSummary);
+        Assert.Null(row.ResponseSummary);
+    }
+
+    [Fact]
+    public async Task RouteCallRow_WithBodyCaptureOn_ByDefault_HasRequestAndResponseSummary()
+    {
+        SiteResolves("inst-1", "SiteA");
+        _router.RouteToCallAsync("SiteA", Arg.Any<RouteToCallRequest>(), Arg.Any<CancellationToken>())
+            .Returns(ci => new RouteToCallResponse(
+                ((RouteToCallRequest)ci[1]).CorrelationId, true, 99, null, DateTimeOffset.UtcNow));
+        var writer = Substitute.For<ICentralAuditWriter>();
+        AuditEvent? written = null;
+        writer.WriteAsync(Arg.Do<AuditEvent>(e => written = e), Arg.Any<CancellationToken>())
+            .Returns(Task.CompletedTask);
+        var helper = new RouteHelper(_locator, _router, writer, null);
+
+        await helper.To("inst-1").Call("doWork", new { x = 1 });
+
+        Assert.NotNull(written);
+        var row = written!.AsRow();
+        Assert.Contains("\"x\":1", row.RequestSummary);
+        Assert.Equal("99", row.ResponseSummary);
+    }
+
+    [Fact]
+    public async Task WithBodyCapture_SurvivesTheOtherWithCopies()
+    {
+        // Mirrors WithSiteScope_SurvivesTheOtherWithCopies (Task 3): the flag must not be
+        // dropped by the copies InboundScriptExecutor makes (WithDeadline /
+        // WithRequestAborted / WithParentExecutionId / WithTrace / WithSiteScope).
+        SiteResolves("inst-1", "SiteA");
+        _router.RouteToCallAsync("SiteA", Arg.Any<RouteToCallRequest>(), Arg.Any<CancellationToken>())
+            .Returns(ci => new RouteToCallResponse(
+                ((RouteToCallRequest)ci[1]).CorrelationId, true, 99, null, DateTimeOffset.UtcNow));
+        var writer = Substitute.For<ICentralAuditWriter>();
+        AuditEvent? written = null;
+        writer.WriteAsync(Arg.Do<AuditEvent>(e => written = e), Arg.Any<CancellationToken>())
+            .Returns(Task.CompletedTask);
+
+        var helper = new RouteHelper(_locator, _router, writer, null)
+            .WithBodyCapture(false)
+            .WithDeadline(CancellationToken.None)
+            .WithRequestAborted(CancellationToken.None)
+            .WithParentExecutionId(Guid.NewGuid())
+            .WithTrace(true)
+            .WithTraceSink(null)
+            .WithSiteScope(Scope("SiteA"));
+
+        await helper.To("inst-1").Call("doWork", new { x = 1 });
+
+        Assert.NotNull(written);
+        var row = written!.AsRow();
+        Assert.Null(row.RequestSummary);
+        Assert.Null(row.ResponseSummary);
+    }
 }