diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/ScriptAnalysis/SandboxBoundRunPolicy.cs b/src/ZB.MOM.WW.ScadaBridge.CentralUI/ScriptAnalysis/SandboxBoundRunPolicy.cs new file mode 100644 index 00000000..9eab835d --- /dev/null +++ b/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; + +/// +/// 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. +/// +public static class SandboxBoundRunPolicy +{ + /// The tag stamped on the audit row. + public const string SourceTag = "test-run"; + + /// True when the request binds a deployed instance (and so performs real site operations). + /// The run request. + /// Whether the run is bound to an instance. + public static bool IsBound(SandboxRunRequest request) => + request.Kind != ScriptKind.InboundApi && !string.IsNullOrWhiteSpace(request.BindInstanceUniqueName); + + /// Checks role and site scope for a bound run; a null result means the run may proceed. + /// The run request. + /// The authenticated caller. + /// Policy evaluator for the Deployer requirement. + /// Resolves an instance's site identifier. + /// Maps the site identifier to its numeric id for the scope check. + /// Cancellation token. + /// A 403 result when denied; otherwise . + public static async Task 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; + } + + /// + /// 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). + /// + /// The run request. + /// The run outcome. + /// The operator. + /// When the run started. + /// Central audit writer, or null when none is registered. + /// Stamps SourceNode. + /// Failure logger. + /// Cancellation token. + /// A task that represents the asynchronous operation. + 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); +} diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/ScriptAnalysis/ScriptAnalysisEndpoints.cs b/src/ZB.MOM.WW.ScadaBridge.CentralUI/ScriptAnalysis/ScriptAnalysisEndpoints.cs index 9ca3a8a0..e0467f4c 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/ScriptAnalysis/ScriptAnalysisEndpoints.cs +++ b/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(), + sp.GetService(), sp.GetService(), 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(), sp.GetService(), + sp.GetRequiredService().CreateLogger(typeof(SandboxBoundRunPolicy)), + CancellationToken.None); + } + return Results.Ok(result); + }); return endpoints; } diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/ScriptAnalysis/ScriptAnalysisRequestGuard.cs b/src/ZB.MOM.WW.ScadaBridge.CentralUI/ScriptAnalysis/ScriptAnalysisRequestGuard.cs new file mode 100644 index 00000000..4a7f75ea --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/ScriptAnalysis/ScriptAnalysisRequestGuard.cs @@ -0,0 +1,82 @@ +namespace ZB.MOM.WW.ScadaBridge.CentralUI.ScriptAnalysis; + +/// +/// 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 Code or an unthrottled +/// caller can pin cores. Monaco's 500 ms debounce is the only other throttle. +/// +public sealed class ScriptAnalysisRequestGuard +{ + /// Maximum accepted script text length, in characters (256 KB). + public const int MaxCodeChars = 256 * 1024; + + /// Maximum accepted request body, in bytes (whole payload incl. editor context). + public const long MaxBodyBytes = 1024 * 1024; + + /// Maximum concurrently executing analysis requests per user. + public const int MaxConcurrentPerUser = 6; + + private readonly object _lock = new(); + private readonly Dictionary _inFlight = new(StringComparer.OrdinalIgnoreCase); + private readonly int _maxConcurrent; + + /// Initializes the guard. + /// Per-user concurrency ceiling. + public ScriptAnalysisRequestGuard(int maxConcurrentPerUser = MaxConcurrentPerUser) + { + _maxConcurrent = maxConcurrentPerUser; + } + + /// The process-wide instance used by the endpoint group. + public static ScriptAnalysisRequestGuard Shared { get; } = new(); + + /// + /// Tries to start a request for . Entries exist only while a request + /// is in flight, so the map is bounded by concurrent requests rather than by user count. + /// + /// The caller's identity key. + /// A lease to dispose on completion, or when over the ceiling. + 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); + } + + /// The script text carried by a request DTO, or null for a type without one. + /// The bound request argument. + /// The code length in characters, or null when the argument carries no code. + 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); + } + } +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/ScriptAnalysis/ScriptAnalysisEndpointGuardTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/ScriptAnalysis/ScriptAnalysisEndpointGuardTests.cs new file mode 100644 index 00000000..246ad122 --- /dev/null +++ b/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; + +/// +/// 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). +/// +public class ScriptAnalysisEndpointGuardTests +{ + private sealed class StubService : ScriptAnalysisService + { + public StubService(IServiceProvider sp) + : base(Substitute.For(), new MemoryCache(new MemoryCacheOptions { SizeLimit = 200 }), sp) { } + + public int Runs; + + public override Task 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 o, ILoggerFactory l, UrlEncoder e) + : AuthenticationHandler(o, l, e) + { + protected override Task HandleAuthenticateAsync() + { + var claims = new List { 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(); + var locator = Substitute.For(); + locator.GetSiteIdForInstanceAsync("Pump01", Arg.Any()).Returns("site-a"); + var siteRepo = Substitute.For(); + siteRepo.GetAllSitesAsync(Arg.Any()).Returns(new List + { + 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("Hdr", _ => { }); + s.AddScadaBridgeAuthorization(); + s.AddSingleton(audit); + s.AddSingleton(locator); + s.AddSingleton(siteRepo); + s.AddSingleton(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(); + 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; + } + + /// Script text over 256 KB is refused with 413 before any compilation. + /// A task that represents the asynchronous operation. + [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); + } + + /// A caller over the per-user concurrency ceiling gets 429; others are unaffected. + /// A task that represents the asynchronous operation. + [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(); } + } + + /// The guard's per-user map holds no entry once its requests finish. + [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")); + } + + /// A Designer without Deployer cannot run a bound Test Run. + /// A task that represents the asynchronous operation. + [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(), Arg.Any()); + } + + /// An unbound Test Run stays available to a Designer and writes no operator row. + /// A task that represents the asynchronous operation. + [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(), Arg.Any()); + } + + /// A site-scoped Deployer is refused for an instance at another site. + /// A task that represents the asynchronous operation. + [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); + } + + /// A permitted Deployer's bound run executes and writes one operator-attributed audit row. + /// A task that represents the asynchronous operation. + [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(e => e.Actor == "dana" && e.Target == "Pump01"), Arg.Any()); + } +}