fix(centralui): bound script-analysis payload/concurrency; bound Test Run needs Deployer, site scope and audit row (review 2026-10-21 07/P-4, 07/C-2)

src/ZB.MOM.WW.ScadaBridge.CentralUI/ScriptAnalysis/SandboxBoundRunPolicy.cs

@@ -0,0 +1,112 @@
+using System.Security.Claims;
+using Microsoft.AspNetCore.Authorization;
+using Microsoft.AspNetCore.Http;
+using Microsoft.Extensions.Logging;
+using ZB.MOM.WW.ScadaBridge.CentralUI.Auth;
+using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
+using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
+using ZB.MOM.WW.ScadaBridge.Commons.Types.Audit;
+using ZB.MOM.WW.ScadaBridge.Commons.Types.Debugging;
+using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums;
+using ZB.MOM.WW.ScadaBridge.Security;
+
+namespace ZB.MOM.WW.ScadaBridge.CentralUI.ScriptAnalysis;
+
+/// <summary>
+/// Policy for a Test Run bound to a deployed instance (review 2026-10-21 07/C-2). A bound run
+/// performs REAL tag writes / script calls / waits against a site, so — like the Flow Debugger —
+/// it requires the Deployer role (an unbound Test Run stays Designer-only), is confined to the
+/// operator's permitted sites, and leaves an operator-attributed audit row.
+/// </summary>
+public static class SandboxBoundRunPolicy
+{
+    /// <summary>The <see cref="ScriptTraceExtra.Source"/> tag stamped on the audit row.</summary>
+    public const string SourceTag = "test-run";
+
+    /// <summary>True when the request binds a deployed instance (and so performs real site operations).</summary>
+    /// <param name="request">The run request.</param>
+    /// <returns>Whether the run is bound to an instance.</returns>
+    public static bool IsBound(SandboxRunRequest request) =>
+        request.Kind != ScriptKind.InboundApi && !string.IsNullOrWhiteSpace(request.BindInstanceUniqueName);
+
+    /// <summary>Checks role and site scope for a bound run; a null result means the run may proceed.</summary>
+    /// <param name="request">The run request.</param>
+    /// <param name="user">The authenticated caller.</param>
+    /// <param name="authorization">Policy evaluator for the Deployer requirement.</param>
+    /// <param name="locator">Resolves an instance's site identifier.</param>
+    /// <param name="sites">Maps the site identifier to its numeric id for the scope check.</param>
+    /// <param name="ct">Cancellation token.</param>
+    /// <returns>A 403 result when denied; otherwise <see langword="null"/>.</returns>
+    public static async Task<IResult?> AuthorizeAsync(
+        SandboxRunRequest request, ClaimsPrincipal user, IAuthorizationService authorization,
+        IInstanceLocator? locator, ISiteRepository? sites, CancellationToken ct)
+    {
+        if (!IsBound(request)) return null;
+
+        var allowed = await authorization.AuthorizeAsync(user, AuthorizationPolicies.RequireDeployment);
+        if (!allowed.Succeeded)
+        {
+            return Denied("Running a script bound to a deployed instance requires the Deployer role.");
+        }
+
+        var (systemWide, permitted) = SiteScopeService.Resolve(user);
+        if (systemWide) return null;
+
+        // A scoped operator must be able to prove the instance's site is theirs; an unknown
+        // instance or an unresolvable site is refused rather than waved through.
+        var siteIdentifier = locator is null
+            ? null
+            : await locator.GetSiteIdForInstanceAsync(request.BindInstanceUniqueName!, ct);
+        var site = siteIdentifier is null || sites is null
+            ? null
+            : (await sites.GetAllSitesAsync(ct)).FirstOrDefault(s => s.SiteIdentifier == siteIdentifier);
+        if (site is null || !permitted.Contains(site.Id))
+        {
+            return Denied("You are not permitted to run scripts against instances at that site.");
+        }
+        return null;
+    }
+
+    /// <summary>
+    /// Writes one operator-attributed row for a bound run (best-effort: a failure is logged and
+    /// never fails the run, which has already had its real side effects).
+    /// </summary>
+    /// <param name="request">The run request.</param>
+    /// <param name="result">The run outcome.</param>
+    /// <param name="username">The operator.</param>
+    /// <param name="startedAtUtc">When the run started.</param>
+    /// <param name="writer">Central audit writer, or null when none is registered.</param>
+    /// <param name="nodeIdentity">Stamps <c>SourceNode</c>.</param>
+    /// <param name="logger">Failure logger.</param>
+    /// <param name="ct">Cancellation token.</param>
+    /// <returns>A task that represents the asynchronous operation.</returns>
+    public static async Task AuditAsync(
+        SandboxRunRequest request, SandboxRunResult result, string username, DateTime startedAtUtc,
+        ICentralAuditWriter? writer, INodeIdentityProvider? nodeIdentity, ILogger logger, CancellationToken ct)
+    {
+        if (writer is null) return;
+        try
+        {
+            var row = ScadaBridgeAuditEventFactory.Create(
+                AuditChannel.ScriptTrace, AuditKind.RouteCall,
+                result.Success ? AuditStatus.Delivered : AuditStatus.Failed,
+                occurredAtUtc: startedAtUtc,
+                actor: username,
+                target: request.BindInstanceUniqueName,
+                sourceNode: nodeIdentity?.NodeName,
+                executionId: Guid.NewGuid(),
+                sourceInstanceId: request.BindInstanceUniqueName,
+                durationMs: (int)Math.Min(result.DurationMs, int.MaxValue),
+                errorMessage: result.Error,
+                extra: new ScriptTraceExtra { Source = SourceTag }.ToJson());
+            await writer.WriteAsync(row, ct);
+        }
+        catch (Exception ex) when (ex is not OperationCanceledException)
+        {
+            logger.LogWarning(ex, "Test Run audit write failed for instance {Instance}", request.BindInstanceUniqueName);
+        }
+    }
+
+    private static IResult Denied(string message) =>
+        Results.Json(new { error = message, code = "FORBIDDEN" }, statusCode: StatusCodes.Status403Forbidden);
+}

src/ZB.MOM.WW.ScadaBridge.CentralUI/ScriptAnalysis/ScriptAnalysisEndpoints.cs

@@ -1,7 +1,12 @@
+using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Routing;
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
+using ZB.MOM.WW.ScadaBridge.CentralUI.Auth;
+using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
+using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
 using ZB.MOM.WW.ScadaBridge.Security;
 
 namespace ZB.MOM.WW.ScadaBridge.CentralUI.ScriptAnalysis;
@@ -19,6 +24,31 @@ public static class ScriptAnalysisEndpoints
         var group = endpoints.MapGroup("/api/script-analysis")
             .RequireAuthorization(AuthorizationPolicies.RequireDesign);
 
+        // Review 2026-10-21 07/P-4: every call compiles fresh, so bound the payload and the
+        // per-user concurrency. Oversized -> 413, over the concurrency ceiling -> 429.
+        group.AddEndpointFilter(async (ctx, next) =>
+        {
+            var http = ctx.HttpContext;
+            if (http.Request.ContentLength is > ScriptAnalysisRequestGuard.MaxBodyBytes)
+            {
+                return Results.StatusCode(StatusCodes.Status413PayloadTooLarge);
+            }
+            foreach (var arg in ctx.Arguments)
+            {
+                if (ScriptAnalysisRequestGuard.CodeLength(arg) is > ScriptAnalysisRequestGuard.MaxCodeChars)
+                {
+                    return Results.StatusCode(StatusCodes.Status413PayloadTooLarge);
+                }
+            }
+
+            using var lease = ScriptAnalysisRequestGuard.Shared.TryEnter(http.User.GetUsername());
+            if (lease is null)
+            {
+                return Results.StatusCode(StatusCodes.Status429TooManyRequests);
+            }
+            return await next(ctx);
+        });
+
         group.MapPost("/diagnostics", (DiagnoseRequest req, ScriptAnalysisService svc) =>
             Results.Ok(svc.Diagnose(req)));
 
@@ -38,7 +68,27 @@ public static class ScriptAnalysisEndpoints
             Results.Ok(svc.InlayHints(req)));
 
         group.MapPost("/run", async (SandboxRunRequest req, ScriptAnalysisService svc, HttpContext http) =>
-            Results.Ok(await svc.RunInSandboxAsync(req, http.RequestAborted)));
+        {
+            // Review 2026-10-21 07/C-2: a run bound to a deployed instance does real site
+            // operations — Deployer role, site scope, and an operator-attributed audit row.
+            var sp = http.RequestServices;
+            var denied = await SandboxBoundRunPolicy.AuthorizeAsync(
+                req, http.User, sp.GetRequiredService<IAuthorizationService>(),
+                sp.GetService<IInstanceLocator>(), sp.GetService<ISiteRepository>(), http.RequestAborted);
+            if (denied is not null) return denied;
+
+            var startedAt = DateTime.UtcNow;
+            var result = await svc.RunInSandboxAsync(req, http.RequestAborted);
+            if (SandboxBoundRunPolicy.IsBound(req))
+            {
+                await SandboxBoundRunPolicy.AuditAsync(
+                    req, result, http.User.GetUsername(), startedAt,
+                    sp.GetService<ICentralAuditWriter>(), sp.GetService<INodeIdentityProvider>(),
+                    sp.GetRequiredService<ILoggerFactory>().CreateLogger(typeof(SandboxBoundRunPolicy)),
+                    CancellationToken.None);
+            }
+            return Results.Ok(result);
+        });
 
         return endpoints;
     }

src/ZB.MOM.WW.ScadaBridge.CentralUI/ScriptAnalysis/ScriptAnalysisRequestGuard.cs

@@ -0,0 +1,82 @@
+namespace ZB.MOM.WW.ScadaBridge.CentralUI.ScriptAnalysis;
+
+/// <summary>
+/// Abuse bounds for the Roslyn-backed script-analysis endpoints (review 2026-10-21 07/P-4):
+/// every request does a fresh compilation, so an unbounded <c>Code</c> or an unthrottled
+/// caller can pin cores. Monaco's 500 ms debounce is the only other throttle.
+/// </summary>
+public sealed class ScriptAnalysisRequestGuard
+{
+    /// <summary>Maximum accepted script text length, in characters (256 KB).</summary>
+    public const int MaxCodeChars = 256 * 1024;
+
+    /// <summary>Maximum accepted request body, in bytes (whole payload incl. editor context).</summary>
+    public const long MaxBodyBytes = 1024 * 1024;
+
+    /// <summary>Maximum concurrently executing analysis requests per user.</summary>
+    public const int MaxConcurrentPerUser = 6;
+
+    private readonly object _lock = new();
+    private readonly Dictionary<string, int> _inFlight = new(StringComparer.OrdinalIgnoreCase);
+    private readonly int _maxConcurrent;
+
+    /// <summary>Initializes the guard.</summary>
+    /// <param name="maxConcurrentPerUser">Per-user concurrency ceiling.</param>
+    public ScriptAnalysisRequestGuard(int maxConcurrentPerUser = MaxConcurrentPerUser)
+    {
+        _maxConcurrent = maxConcurrentPerUser;
+    }
+
+    /// <summary>The process-wide instance used by the endpoint group.</summary>
+    public static ScriptAnalysisRequestGuard Shared { get; } = new();
+
+    /// <summary>
+    /// Tries to start a request for <paramref name="user"/>. Entries exist only while a request
+    /// is in flight, so the map is bounded by concurrent requests rather than by user count.
+    /// </summary>
+    /// <param name="user">The caller's identity key.</param>
+    /// <returns>A lease to dispose on completion, or <see langword="null"/> when over the ceiling.</returns>
+    public IDisposable? TryEnter(string user)
+    {
+        lock (_lock)
+        {
+            _inFlight.TryGetValue(user, out var n);
+            if (n >= _maxConcurrent) return null;
+            _inFlight[user] = n + 1;
+        }
+        return new Lease(this, user);
+    }
+
+    /// <summary>The script text carried by a request DTO, or null for a type without one.</summary>
+    /// <param name="request">The bound request argument.</param>
+    /// <returns>The code length in characters, or null when the argument carries no code.</returns>
+    public static int? CodeLength(object? request) => request switch
+    {
+        DiagnoseRequest r => r.Code?.Length,
+        CompletionsRequest r => r.CodeText?.Length,
+        HoverRequest r => r.CodeText?.Length,
+        SignatureHelpRequest r => r.CodeText?.Length,
+        FormatRequest r => r.Code?.Length,
+        InlayHintsRequest r => r.Code?.Length,
+        SandboxRunRequest r => r.Code?.Length,
+        _ => null,
+    };
+
+    private void Exit(string user)
+    {
+        lock (_lock)
+        {
+            if (!_inFlight.TryGetValue(user, out var n)) return;
+            if (n <= 1) _inFlight.Remove(user); else _inFlight[user] = n - 1;
+        }
+    }
+
+    private sealed class Lease(ScriptAnalysisRequestGuard owner, string user) : IDisposable
+    {
+        private int _disposed;
+        public void Dispose()
+        {
+            if (Interlocked.Exchange(ref _disposed, 1) == 0) owner.Exit(user);
+        }
+    }
+}

tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/ScriptAnalysis/ScriptAnalysisEndpointGuardTests.cs

@@ -0,0 +1,220 @@
+using System.Net;
+using System.Net.Http.Json;
+using System.Security.Claims;
+using System.Text.Encodings.Web;
+using Microsoft.AspNetCore.Authentication;
+using Microsoft.AspNetCore.Builder;
+using Microsoft.AspNetCore.Hosting;
+using Microsoft.AspNetCore.TestHost;
+using Microsoft.Extensions.Caching.Memory;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Options;
+using NSubstitute;
+using ZB.MOM.WW.Audit;
+using ZB.MOM.WW.ScadaBridge.CentralUI.ScriptAnalysis;
+using ZB.MOM.WW.ScadaBridge.Commons.Entities.Sites;
+using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
+using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
+using ZB.MOM.WW.ScadaBridge.Security;
+
+namespace ZB.MOM.WW.ScadaBridge.CentralUI.Tests.ScriptAnalysis;
+
+/// <summary>
+/// Review 2026-10-21 07/P-4 (payload + concurrency bounds) and 07/C-2 (bound Test Run needs the
+/// Deployer role, site scope and an operator-attributed audit row).
+/// </summary>
+public class ScriptAnalysisEndpointGuardTests
+{
+    private sealed class StubService : ScriptAnalysisService
+    {
+        public StubService(IServiceProvider sp)
+            : base(Substitute.For<ISharedScriptCatalog>(), new MemoryCache(new MemoryCacheOptions { SizeLimit = 200 }), sp) { }
+
+        public int Runs;
+
+        public override Task<SandboxRunResult> RunInSandboxAsync(SandboxRunRequest request, CancellationToken ct)
+        {
+            Interlocked.Increment(ref Runs);
+            return Task.FromResult(new SandboxRunResult(true, null, null, "", null, SandboxErrorKind.None, 7, null));
+        }
+    }
+
+    private sealed class HeaderAuth(
+        IOptionsMonitor<AuthenticationSchemeOptions> o, ILoggerFactory l, UrlEncoder e)
+        : AuthenticationHandler<AuthenticationSchemeOptions>(o, l, e)
+    {
+        protected override Task<AuthenticateResult> HandleAuthenticateAsync()
+        {
+            var claims = new List<Claim> { new(JwtTokenService.UsernameClaimType, Request.Headers["X-User"].ToString()) };
+            foreach (var r in Request.Headers["X-Roles"].ToString().Split(',', StringSplitOptions.RemoveEmptyEntries))
+                claims.Add(new Claim(JwtTokenService.RoleClaimType, r));
+            foreach (var s in Request.Headers["X-Sites"].ToString().Split(',', StringSplitOptions.RemoveEmptyEntries))
+                claims.Add(new Claim(JwtTokenService.SiteIdClaimType, s));
+            var id = new ClaimsIdentity(claims, "Hdr");
+            return Task.FromResult(AuthenticateResult.Success(
+                new AuthenticationTicket(new ClaimsPrincipal(id), "Hdr")));
+        }
+    }
+
+    private static async Task<(HttpClient Client, IHost Host, StubService Svc, ICentralAuditWriter Audit)> BuildAsync()
+    {
+        var audit = Substitute.For<ICentralAuditWriter>();
+        var locator = Substitute.For<IInstanceLocator>();
+        locator.GetSiteIdForInstanceAsync("Pump01", Arg.Any<CancellationToken>()).Returns("site-a");
+        var siteRepo = Substitute.For<ISiteRepository>();
+        siteRepo.GetAllSitesAsync(Arg.Any<CancellationToken>()).Returns(new List<Site>
+        {
+            new("Site A", "site-a") { Id = 1 },
+            new("Site B", "site-b") { Id = 2 },
+        });
+        StubService? svc = null;
+        var host = await new HostBuilder().ConfigureWebHost(web =>
+        {
+            web.UseTestServer();
+            web.ConfigureServices(s =>
+            {
+                s.AddRouting();
+                s.AddLogging();
+                s.AddAuthentication("Hdr").AddScheme<AuthenticationSchemeOptions, HeaderAuth>("Hdr", _ => { });
+                s.AddScadaBridgeAuthorization();
+                s.AddSingleton(audit);
+                s.AddSingleton(locator);
+                s.AddSingleton(siteRepo);
+                s.AddSingleton<ScriptAnalysisService>(sp => svc = new StubService(sp));
+            });
+            web.Configure(app =>
+            {
+                app.UseRouting();
+                app.UseAuthentication();
+                app.UseAuthorization();
+                app.UseEndpoints(e => e.MapScriptAnalysisEndpoints());
+            });
+        }).StartAsync();
+        var client = host.GetTestClient();
+        _ = host.Services.GetRequiredService<ScriptAnalysisService>();
+        return (client, host, svc!, audit);
+    }
+
+    private static HttpRequestMessage Post(string url, object body, string user, string roles, string sites = "")
+    {
+        var req = new HttpRequestMessage(HttpMethod.Post, url) { Content = JsonContent.Create(body) };
+        req.Headers.Add("X-User", user);
+        req.Headers.Add("X-Roles", roles);
+        req.Headers.Add("X-Sites", sites);
+        return req;
+    }
+
+    /// <summary>Script text over 256 KB is refused with 413 before any compilation.</summary>
+    /// <returns>A task that represents the asynchronous operation.</returns>
+    [Fact]
+    public async Task OversizedCode_Returns413()
+    {
+        var (client, host, _, _) = await BuildAsync();
+        using var _h = host;
+        var big = new string('x', ScriptAnalysisRequestGuard.MaxCodeChars + 1);
+
+        var resp = await client.SendAsync(Post("/api/script-analysis/format", new FormatRequest(big), "alice", "Designer"));
+
+        Assert.Equal(HttpStatusCode.RequestEntityTooLarge, resp.StatusCode);
+    }
+
+    /// <summary>A caller over the per-user concurrency ceiling gets 429; others are unaffected.</summary>
+    /// <returns>A task that represents the asynchronous operation.</returns>
+    [Fact]
+    public async Task OverConcurrencyCeiling_Returns429_ForThatUserOnly()
+    {
+        var (client, host, _, _) = await BuildAsync();
+        using var _h = host;
+        var leases = Enumerable.Range(0, ScriptAnalysisRequestGuard.MaxConcurrentPerUser)
+            .Select(_ => ScriptAnalysisRequestGuard.Shared.TryEnter("busy-user")).ToList();
+        try
+        {
+            var busy = await client.SendAsync(Post("/api/script-analysis/format", new FormatRequest("var a = 1;"), "busy-user", "Designer"));
+            var other = await client.SendAsync(Post("/api/script-analysis/format", new FormatRequest("var a = 1;"), "other-user", "Designer"));
+
+            Assert.Equal(HttpStatusCode.TooManyRequests, busy.StatusCode);
+            Assert.Equal(HttpStatusCode.OK, other.StatusCode);
+        }
+        finally { foreach (var l in leases) l?.Dispose(); }
+    }
+
+    /// <summary>The guard's per-user map holds no entry once its requests finish.</summary>
+    [Fact]
+    public void Guard_ReleasesLeases_AndEnforcesCeiling()
+    {
+        var guard = new ScriptAnalysisRequestGuard(2);
+        var a = guard.TryEnter("u");
+        var b = guard.TryEnter("u");
+        Assert.NotNull(a); Assert.NotNull(b);
+        Assert.Null(guard.TryEnter("u"));
+        a!.Dispose(); a.Dispose(); // double dispose must not release twice
+        Assert.NotNull(guard.TryEnter("u"));
+    }
+
+    /// <summary>A Designer without Deployer cannot run a bound Test Run.</summary>
+    /// <returns>A task that represents the asynchronous operation.</returns>
+    [Fact]
+    public async Task BoundRun_WithoutDeployerRole_Is403_AndNotExecuted()
+    {
+        var (client, host, svc, audit) = await BuildAsync();
+        using var _h = host;
+        var body = new SandboxRunRequest("return 1;", null, null, ScriptKind.Template, "Pump01");
+
+        var resp = await client.SendAsync(Post("/api/script-analysis/run", body, "alice", "Designer"));
+
+        Assert.Equal(HttpStatusCode.Forbidden, resp.StatusCode);
+        Assert.Equal(0, svc.Runs);
+        await audit.DidNotReceive().WriteAsync(Arg.Any<AuditEvent>(), Arg.Any<CancellationToken>());
+    }
+
+    /// <summary>An unbound Test Run stays available to a Designer and writes no operator row.</summary>
+    /// <returns>A task that represents the asynchronous operation.</returns>
+    [Fact]
+    public async Task UnboundRun_ByDesigner_StillWorks()
+    {
+        var (client, host, svc, audit) = await BuildAsync();
+        using var _h = host;
+        var body = new SandboxRunRequest("return 1;", null, null, ScriptKind.Template, null);
+
+        var resp = await client.SendAsync(Post("/api/script-analysis/run", body, "alice", "Designer"));
+
+        Assert.Equal(HttpStatusCode.OK, resp.StatusCode);
+        Assert.Equal(1, svc.Runs);
+        await audit.DidNotReceive().WriteAsync(Arg.Any<AuditEvent>(), Arg.Any<CancellationToken>());
+    }
+
+    /// <summary>A site-scoped Deployer is refused for an instance at another site.</summary>
+    /// <returns>A task that represents the asynchronous operation.</returns>
+    [Fact]
+    public async Task BoundRun_AtSiteOutsideScope_Is403()
+    {
+        var (client, host, svc, _) = await BuildAsync();
+        using var _h = host;
+        var body = new SandboxRunRequest("return 1;", null, null, ScriptKind.Template, "Pump01");
+
+        // Pump01 lives at site-a (Id 1); this operator is scoped to site Id 2.
+        var resp = await client.SendAsync(Post("/api/script-analysis/run", body, "dana", "Designer,Deployer", "2"));
+
+        Assert.Equal(HttpStatusCode.Forbidden, resp.StatusCode);
+        Assert.Equal(0, svc.Runs);
+    }
+
+    /// <summary>A permitted Deployer's bound run executes and writes one operator-attributed audit row.</summary>
+    /// <returns>A task that represents the asynchronous operation.</returns>
+    [Fact]
+    public async Task BoundRun_ByPermittedDeployer_RunsAndAuditsTheOperator()
+    {
+        var (client, host, svc, audit) = await BuildAsync();
+        using var _h = host;
+        var body = new SandboxRunRequest("return 1;", null, null, ScriptKind.Template, "Pump01");
+
+        var resp = await client.SendAsync(Post("/api/script-analysis/run", body, "dana", "Designer,Deployer", "1"));
+
+        Assert.Equal(HttpStatusCode.OK, resp.StatusCode);
+        Assert.Equal(1, svc.Runs);
+        await audit.Received(1).WriteAsync(
+            Arg.Is<AuditEvent>(e => e.Actor == "dana" && e.Target == "Pump01"), Arg.Any<CancellationToken>());
+    }
+}