From 63ec45e2e3a4e3da6fb0cabcbacb86b6327c1daf Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 9 Nov 2026 22:22:44 -0500 Subject: [PATCH] fix(flow-debugger): refuse method-entry Live runs for site-scoped users (#55) Owner decision (b), 2026-11-09. An inbound API method has no site affinity, so the F2 route-time scope only bounded routed SITE traffic; the method's central-side effects (Database.*, ExternalSystem, notifications) still ran for a scoped Deployer. FlowDebugRunner now refuses FlowDebugRunMethodCommand for a site-scoped caller (site ids, not Administrator) before the script executes, audits the refusal as a failed InboundRequest under the operator's name, and raises SiteScopeViolationException -> ManagementUnauthorized, so the Central UI and the CLI (same /management command) share the gate. The page disables the "Inbound API method" entry for scoped Deployers with the reason (tooltip + inline note), starts them on script entry, and ignores ?methodId= / a stored method choice. Replay and the instance-scoped script / value-change Live runs are unchanged. The route-time scope is kept as defence in depth. Tests: scoped refused (in- and out-of-scope routing; script never runs, refusal audited), unscoped/Administrator allowed, scoped Replay allowed, scoped in-scope value-change Live still runs; bUnit for the disabled entry, the drill-in, and the unscoped control. Mutation-proven (gate and UI flag reverted -> 4 red). Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV --- CLAUDE.md | 2 +- docs/components/CLI.md | 2 +- docs/plans/2026-09-05-flow-debugger-design.md | 7 ++ docs/requirements/Component-CentralUI.md | 1 + .../Component-ManagementService.md | 2 +- .../Commands/FlowDebugCommands.cs | 2 +- .../Pages/Deployment/FlowDebugger.razor | 11 ++- .../Pages/Deployment/FlowDebugger.razor.cs | 21 +++++- .../FlowDebug/FlowDebugRunner.cs | 50 +++++++++++++- .../Deployment/FlowDebuggerPageTests.cs | 58 +++++++++++++++- .../FlowDebugRunnerTests.cs | 67 ++++++++++++++----- 11 files changed, 198 insertions(+), 25 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 60503be4..870808f5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -211,7 +211,7 @@ spec for each is `docs/requirements/Component-.md`, and `README.md` carrie - Central UI: Blazor Server (ASP.NET Core + SignalR) with Bootstrap CSS. No third-party component frameworks (no Blazorise, MudBlazor, Radzen, etc.). Build custom Blazor components for tables, grids, forms, etc. - UI design: Clean, corporate, internal-use aesthetic. Not flashy. Use the `frontend-design` skill when designing UI pages/components. - Debug view: real-time streaming via DebugStreamBridgeActor + gRPC (events via `SiteStreamGrpcClient`, snapshot via the `SiteCommandService` query surface — `QueryReply.DebugViewSnapshot`). Health dashboard: 10s polling timer. Deployment status: real-time push via SignalR. -- **Flow Debugger (2026-09-05, Deployment → `/deployment/flow-debugger` + CLI `flow-debug run-method|run-script|run-value-change|replay`).** Runs one entry point (an inbound API method, or a site script) and draws every step as a Central / Site / Tags / **External** swimlane; *Live* really runs it, *Replay* rebuilds a past execution. **v1.1 (2026-09-06, `docs/plans/2026-09-06-flow-debugger-outbound-calls.md`):** every `ExternalSystem.*` / `Database.*` / `Notify.Send` a traced run makes is an `ApiCall` / `DbCall` / `Notify` node on the External lane — the site routes those already-existing `ApiOutbound` / `DbOutbound` / `Notification` rows through the same inline trace collector, the replay query widens to those channels, and `FlowGraphBuilder` folds a cached operation's rows by `(kind, CorrelationId)` into ONE node with a `Lifecycle` (sync calls in-chain, cached/notify on dashed `Async` edges, trigger cascades on `Spawn` edges, the Response linked from the inbound execution's last step). Later lifecycle rows are not inline, so after a Live run the page polls the Replay command (5 s, ≤ 2 min) while any node is `Pending`, plus a *Refresh outcomes* button. **A refresh MERGES, never replaces (2026-09-06, found live in the browser):** the page unions the re-read rows into the run's own rows by `EventId` and rebuilds the graph, because a Live run carries its site rows inline while the same rows reach central's audit store by telemetry ~5–30 s later — a Replay read taken in between lacks them, and the old wholesale swap made an already-drawn node (a resolved cached DB write) vanish until the next manual refresh. The inline rows ARE the audited rows (`FlowTraceCollector` sinks the same `AuditEvent` objects the site writer persists), so the union is exact. Central method-script `Database.*` calls (`InboundDatabaseHelper`) are now audited (`DbOutbound`/`DbWrite`) — they were not. Rig demo: `docker/seed-flow-outbound-demo.sh` + `verify-delmia-notifier.sh` step I. **Trace steps ARE audit events** — `AuditChannel.ScriptTrace` with kinds `RouteCall` (central, **always-on** per `Route.To().Call`, joins the two halves of a Replay) and `ScriptRun` / `AttributeWrite` / `AttributeWait` (site, **gated** by a `bool Trace` riding beside `ParentExecutionId` on `RouteToCallRequest` / `ScriptCallRequest` / `SetStaticAttributeCommand` / `AttributeValueChanged`; proto fields additive, `site_command.proto` imports `sitestream.proto`). A Live run gets the site rows **inline** on `RouteToCallResponse.TraceEvents` (+ `ChildExecutionId`) because the 5 s telemetry cadence is too slow; Replay reads them from `AuditLog` (`GetTraceEventsAsync`). A value-change on a data-sourced attribute is a REAL tag write; the Instance Actor's **trace arm** (`SiteRuntime:TraceArmWindowSeconds`, 30) stamps the device echo with the writer's execution id so the triggered run is parented and traced, and central polls the site's new `PullTraceEvents` RPC (point read, not reconciliation) for it. Execution path is four management commands on the **per-node `ManagementActor`** (`FlowDebug*`; the three run commands are Deployer-only, Replay is wider) — a standby refuses the three run commands with **`STANDBY_NODE`** (shown verbatim); `ApiMethod.TraceAllCalls` (migration `AddApiMethodTraceAllCalls`, CLI `--trace-all-calls`) traces every real call to a method for later Replay (7–10 rows/call). Prediction before a run is a Roslyn syntax walk (`FlowShapePredictor`, REQ-SA-7, DB-free); the graph (`FlowStep`/`FlowGraphBuilder`, Commons) is a central projection, never a wire type; the renderer is custom SVG (`FlowGraphView`). Rig: `AuditLog:PerChannelRetentionDays:ScriptTrace=30` on both centrals; `verify-delmia-notifier.sh` step H proves live + replay. **Mixed-version caveat:** an older site ignores the flag → central `RouteCall` only, page says *site not traced*. **v1.2 (2026-09-06, `docs/plans/2026-09-06-flow-debugger-followups.md`):** a `Parked` External node offers **Retry / Discard in place** (same site-scope check, confirm and relay as the report pages, via `IFlowDebugService.RetryOutboundAsync`/`DiscardOutboundAsync`, then the page re-polls Replay until the node leaves Parked) and every folded node deep-links to `/site-calls/report?correlationId=` / `/notifications/report?correlationId=` (both pages accept it through additive `TrackedOperationIdFilter` / `NotificationIdFilter` query params and auto-open the row); register row 111 closed. **Trap closed the same day:** `FlowDebugService.PredictMethodAsync` used to resolve route targets with `GetAwaiter().GetResult()` inside the synchronous predictor walk — on the Blazor renderer sync context that DEADLOCKED the whole circuit (page inert, healthy websocket, no error) whenever a method routing to a site script was chosen; now a resolve-and-rewalk loop, pinned by a single-thread-pump test. Never block on a task from a Blazor event/lifecycle path. Browser checks of the page go through Playwright (`FlowDebuggerTests`, fixture login + an ephemeral method), not the login-disabled override. **Arch-reviewed 2026-09-06** (`docs/plans/2026-09-06-flow-debugger-arch-review.md`, remediation `docs/plans/2026-09-06-flow-debugger-arch-review-remediation.md`): the inline Live trace copy is now redacted and capped by the shared `FlowTraceCollector` (500 events **and** 1 MB of `DetailsJson`, F1), a method-entry Live run enforces the operator's site scope at route time (`RouteHelper.WithSiteScope` → `SITE_SCOPE_DENIED` → `ManagementUnauthorized`, F2), and the page/Replay opened to the `OperationalAudit` roles under the new `RequireFlowDebugger` policy while everything that executes stays Deployer-only (F3). Design `docs/plans/2026-09-05-flow-debugger-design.md`; plan `docs/plans/2026-09-05-flow-debugger.md`. **v1.3 (2026-09-08, `docs/plans/2026-09-08-flow-debugger-script-entry-design.md`):** the second entry point is site → instance → **script**; ValueChange/Conditional scripts are fired by writing the trigger's attribute (derived from the trigger config, not picked), every other kind is a direct traced `RouteToCall` with a `ParameterValueForm` from the script's `ParameterDefinitions` and one operator-attributed `RouteCall` root row (`FlowDebugRunScriptCommand`, Deployer-only, long-running); `run-value-change` stays as the attribute-level CLI escape hatch (fires Expression triggers). **Instance picker (2026-09-08, `docs/plans/2026-09-08-instance-picker-design.md`):** Flow Debugger and Debug View both replaced their flat instance `` with the shared **`InstancePicker`** combobox — a text field whose dropdown is the site's area tree (`InstanceTreeBuilder`, all areas collapsed on open), typed text filters and expands to the matches; `Areas`/`Instances` are compared by reference, so callers must assign the loaded lists once per site change, never per render, or the panel closes on every re-render. **v1.4 (2026-09-09, `docs/plans/2026-09-09-flow-debugger-step-logs-design.md`):** scripts can log — a `Log.Debug/Info/Warn/Error(message, exception?)` global on all four surfaces (site `ScriptGlobals`, inbound `InboundScriptContext`, the `ScriptCompileSurface` deploy gate as `CompileLog`, and both Test Run sandboxes/editor hosts, where it appends to the console pane), always reaching the node `ILogger` and emitting a `ScriptTrace`/**`ScriptLog`** audit row (one per line) **only when the run is traced**, so an untraced timer script costs no audit volume. `FlowGraphBuilder` **folds those rows onto the step that produced them** (`FlowStep.Logs`) and never draws them as nodes — attribution is `Extra.stepEventId`, pre-minted by `SetAttribute`/`WriteBatchAndWait`/`WaitInternal`/sync `ExternalSystem.Call` through an `AsyncLocal` `TraceStepScope`, else the execution's anchor row, else its first step, else `FlowGraph.UnattachedLogLines`; the runtime's own Information+ entries are captured for free by a `TraceCapturingLogger` decorator instead of editing every `_logger.LogWarning` call site. The failing step now carries the **whole exception** (`errorDetail = ex.ToString()` on the site `ScriptRun` row and on the central inbound row — the latter never on the HTTP body, which stays `SCRIPT_ERROR`). **The two halves are NOT gated alike:** the central inbound row's `errorDetail` is stamped on **every failing method request, traced or not** (`EndpointExtensions` stashes it on `HttpContext.Items`, `AuditWriteMiddleware` consumes it), bounded only by the redactor's `ApiInbound` cap `AuditLog:InboundMaxBytes` (1 MiB default; the 64 KB `ErrorCapBytes` is what bounds every OTHER channel's error rows, the site `ScriptRun` row included) — tracing only decides whether the row is later readable as a Flow Debugger step; the site `ScriptRun` `errorDetail` is **traced-only**, because an untraced run emits no `ScriptTrace` row at all, rendered as a red `Type: message` subtitle plus a **Log (n)** tab; the node badge is `✕ n ! n ≡ n` and the strip counts `Log: n errors · n warnings · n info` (a `Log.Error` is a line, not a failed step — it never moves the state chip). Bounded: `ScadaBridge:SiteRuntime:MaxLogLinesPerRun` / `ScadaBridge:InboundApi:MaxLogLinesPerRun` (200, validated > 0), 2 000-char messages, one closing `warn`/`runtime` row naming the dropped count, and a **low-priority inline lane** (`FlowTraceCollector.Add(evt, lowPriority: true)`, log rows stop 50 events short of the count cap and at 75 % of the byte budget, reserving the rest for the step rows written at run end) whose overflow sets `LogsTruncated` → the page badge *inline log lines capped — refresh outcomes pulls the rest*, which clears once a refresh merges in more log rows. **"Never `await` an audit write on a script path" still holds** — every log row is enqueued fire-and-continue, exactly like the 2026-09-06 cached-call telemetry fix. **Trap:** build `ScriptLogger` over the UNDECORATED logger — `TraceCapturingLogger` refuses to wrap another `TraceCapturingLogger` (it would emit two rows per line). CLI `flow-debug … --show-logs` + a `logs` column; rig demo `docker/seed-flow-log-demo.sh` + `verify-delmia-notifier.sh` step J. Mixed-version: an OLD central projects a NEW site's `ScriptLog` rows as `Unknown` nodes — upgrade central first, as always. **Method pane structured inputs (2026-09-10, Gitea #37, `docs/plans/2026-09-10-flow-debugger-method-params-design.md`):** the *Run inbound API method* pane now renders the same `ParameterValueForm` as the script pane (keyed on the method id) plus a **Form | Raw JSON** whole-payload escape hatch (Monaco, schema-validated on every edit); the page keeps ONE value tree and serializes it as-is, the old per-field text coercion is gone, and Run is disabled while the form or the raw document is invalid — so a required array-of-object parameter (MesMoveOut `WorkOrders`) can be entered, and a failed parse is never sent as a string. The per-method `localStorage` entry stores the value tree and is dropped on restore if it no longer validates (that also retires the pre-#37 flat string entries). - Health reports: 30s interval, 60s offline threshold, monotonic sequence numbers, raw error counts per interval. - Dead letter monitoring as a health metric. - Site Event Logging: 30-day retention, 1GB storage cap, daily purge, paginated queries with keyword search (keyword searches are clamped to `ScadaBridge:SiteEventLog:KeywordSearchMaxWindow`, 7 d; reads run off the writer lock on their own connection). **The cap measures `site_events` + its four indexes, NOT the consolidated LocalDb file** (register row 81, 2026-08-17): whole-file measurement made the trim unwinnable on a replicated node — each batch freed event pages while adding an equal count of oplog rows + tombstones to the same file, so it stopped 2.5x over a 64 MB cap and a node with a down peer could not enforce the cap at all. `dbstat` (the exact per-table measurement) is **NOT compiled into the bundled `SQLitePCLRaw.lib.e_sqlite3`** — probed and pinned by `SqliteCapabilityTests`, so don't reach for it — the footprint is modelled from sampled row width instead (−0.8%…+12.8%, biased high). Consequence: `MaxStorageMb` no longer bounds the data volume alone; LocalDb bookkeeping is separate overhead bounded by `MaxOplogRows`/`TombstoneRetention` and is deliberately not this component's job. The cap trim re-reads the footprint count on batch 1 then every `CapTrimCountRefreshBatches` (10th) batch and vacuums once per tick. Decision: `docs/plans/2026-08-17-site-event-log-cap-semantics-decision.md`. diff --git a/docs/components/CLI.md b/docs/components/CLI.md index ee500c0f..40ebac58 100644 --- a/docs/components/CLI.md +++ b/docs/components/CLI.md @@ -116,7 +116,7 @@ scadabridge instance deploy --id 7 | `shared-schema` | `list`, `get`, `create`, `update`, `delete` | Read: authenticated; mutations: Designer | | `secured-write` | `list`, `submit`, `approve`, `reject` | Operator / Verifier / Administrator, enforced per operation | | `cached-call` | `retry`, `discard` (list parked messages with `health parked-messages`) | Server-enforced operational roles and site scope | -| `flow-debug` | `run-method`, `run-script`, `run-value-change`, `replay` | Deployer; replay also accepts OperationalAudit roles | +| `flow-debug` | `run-method`, `run-script`, `run-value-change`, `replay` | Deployer; `run-method` is refused for a site-scoped Deployer (Gitea #55); replay also accepts OperationalAudit roles | | `shared-script` | `list`, `get`, `create`, `update`, `delete` | `Designer` | | `db-connection` | `list`, `get`, `create`, `update`, `delete` | `Designer` | | `api-method` | `list`, `get`, `create`, `update`, `delete`, `validate`, `recompile` | `Designer` | diff --git a/docs/plans/2026-09-05-flow-debugger-design.md b/docs/plans/2026-09-05-flow-debugger-design.md index 1ab66902..47e6a66e 100644 --- a/docs/plans/2026-09-05-flow-debugger-design.md +++ b/docs/plans/2026-09-05-flow-debugger-design.md @@ -292,6 +292,13 @@ surface); the walker is syntax-only and never executes anything. **before any site traffic**; the denial is rethrown as `SiteScopeViolationException` *after* the inbound audit row is written, so the attempt is recorded under the operator's name. Administrators and unscoped users are exempt, exactly as `EnforceSiteScope` treats them. + **Superseded for scoped users by Gitea #55 (owner decision (b), 2026-11-09):** route-time scope only + bounds routed site traffic — the method's central-side effects (`Database.*`, `ExternalSystem`, + notifications) have no site to scope against — so a site-scoped operator is now refused a method + Live run outright, before the script executes (`FlowDebugRunner.IsSiteScoped`, audited failed + `InboundRequest` row, `SiteScopeViolationException` → `ManagementUnauthorized`). The page disables the + method entry for them with the reason; Replay and instance-scoped script / value-change Live runs are + unchanged. The route-time scope stays as defence in depth. - Trace payloads pass through the existing redaction policy on BOTH copies: the stored row (writer) and the inline Live copy (`FlowTraceCollector` applies `IAuditRedactor` and a 1 MB byte budget beside the 500-event cap — arch-review F1). The row-level caps are unchanged (auth headers diff --git a/docs/requirements/Component-CentralUI.md b/docs/requirements/Component-CentralUI.md index 12895125..9212230e 100644 --- a/docs/requirements/Component-CentralUI.md +++ b/docs/requirements/Component-CentralUI.md @@ -26,6 +26,7 @@ Central cluster only. Sites have no user interface. - Cookie login validates an antiforgery token before reading credentials or calling LDAP; the login form supplies the token. The JSON token endpoint retains its non-cookie client contract. - Manual failover goes through a scoped `AuthorizedManualFailoverService` that refreshes the circuit's Administrator grant before calling the host singleton. The component's visibility policy is an additional presentation guard. +- Flow Debugger method-entry Live runs are unavailable to a site-scoped Deployer (Gitea #55): the *Inbound API method* entry is disabled with the reason as a tooltip and an inline note, the page starts on script entry, a `?methodId=` drill-in or a stored method choice is ignored, and the server refuses the command regardless. Script entry on a permitted instance and Replay stay available. - Flow Debugger retry/discard validates the current Deployer grant and resolves site scope inside its service. Scoped callers cannot relay an unknown site. Refresh merges preserve Replay's event ordering and append only rows known exclusively to the live run. - Health and Alarm Summary observe the whole posted refresh task and release their poll gate even when renderer dispatch fails during disposal. - The Topology initially loads permitted sites **and their area metadata** (Gitea #39 — every permitted site's areas are read at open, out-of-scope sites never) and opens with each site expanded to its top-level areas (`TreeView.InitiallyExpanded`; a browser's saved expansion state always wins). Instances stay lazy: select **Load instances** to load the first root-instance page of a site; each expanded area has its own loader. **Previous** and **Next** replace that branch's 50-instance page. Queries are untracked and omit member collections; templates are not loaded. Live refresh and staleness checks cover loaded pages. Search and counts explicitly describe loaded data. **Deploy all…** (Gitea #40): the Site and Area node context menus offer a subtree bulk deploy (Deployer role + current site scope re-checked at click). The page asks `DeploymentService.PreviewSubtreeDeployAsync` for the instance count and Disabled count (a cheap read, no staleness probe); a zero-instance subtree shows an info toast and no dialog; otherwise `DeployAllDialog` (via `IDialogService.ShowAsync`) shows the counts and a **Force redeploy up-to-date instances** checkbox with confirm label **Deploy all**. Confirm calls `DeploySiteAsync` (site node) or `DeployAreaAsync` (area node — the area plus all descendant areas) with the Force choice — same rules as the CLI/management bulk deploy (Disabled always skipped, up-to-date skipped unless forced, not all-or-nothing) — then toasts `N deployed, M skipped, K failed` (an error toast naming the first three failures when any failed) and reloads the tree. The menu item is disabled while another page action is in flight. diff --git a/docs/requirements/Component-ManagementService.md b/docs/requirements/Component-ManagementService.md index 7b0db6ac..781f7ead 100644 --- a/docs/requirements/Component-ManagementService.md +++ b/docs/requirements/Component-ManagementService.md @@ -249,7 +249,7 @@ All four commands return `FlowDebugRunResult`: the chain root execution ID, comp | Command | Behaviour | Required role | |---|---|---| -| `FlowDebugRunMethodCommand` | Validates an inbound API method's parameters and runs it with tracing; routed calls enforce the caller's site scope. | Deployer | +| `FlowDebugRunMethodCommand` | Validates an inbound API method's parameters and runs it with tracing; routed calls enforce the caller's site scope. **Refused for a site-scoped caller** (a Deployer carrying site ids, not an Administrator) with `ManagementUnauthorized` before the script executes — a method has no site, so its central-side effects cannot be scoped (Gitea #55); the refusal is audited as a failed `InboundRequest` under the operator's name. | Deployer (system-wide, or Administrator) | | `FlowDebugRunValueChangeCommand` | Writes an instance attribute through the real site path and polls for the triggered execution tree. | Deployer | | `FlowDebugRunScriptCommand` | Resolves the effective script; writes its trigger attribute for write-trigger kinds, otherwise invokes it directly with validated parameters. | Deployer | | `FlowDebugReplayCommand` | Reads the stored execution tree for any execution ID, enforcing caller scope and retaining the repository's chain anchor. | Any of Deployer, Administrator, Viewer | diff --git a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/FlowDebugCommands.cs b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/FlowDebugCommands.cs index 8c85cf56..3baeb643 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/FlowDebugCommands.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/FlowDebugCommands.cs @@ -52,7 +52,7 @@ public static class FlowDebugCommands DefaultValueFactory = _ => 120, }; var showLogsOption = new Option("--show-logs") { Description = "Print each step's ScriptLog lines and exception detail as indented lines under its row (table output only)" }; - var cmd = new Command("run-method") { Description = "Run an inbound API method on the active central node with full tracing and print the flow's steps" }; + var cmd = new Command("run-method") { Description = "Run an inbound API method on the active central node with full tracing and print the flow's steps (system-wide Deployer or Administrator; refused for a site-scoped user — use replay or run-script)" }; cmd.Add(methodIdOption); cmd.Add(paramsOption); cmd.Add(timeoutOption); diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/FlowDebugger.razor b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/FlowDebugger.razor index 93f98cf4..d7717f18 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/FlowDebugger.razor +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/FlowDebugger.razor @@ -23,7 +23,9 @@
+ data-test="flow-entry-method" disabled="@(_running || _methodLiveRefused)" + title="@(_methodLiveRefused ? MethodLiveRefusedHint : null)" + @onclick="() => SetEntryAsync(EntryKind.Method)">Inbound API method
@@ -128,6 +130,13 @@ + @if (_methodLiveRefused && _mode == FlowMode.Live) + { +
+ @MethodLiveRefusedHint +
+ } + @if (!_canLive) {
diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/FlowDebugger.razor.cs b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/FlowDebugger.razor.cs index 46e2fb4d..d45c813a 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/FlowDebugger.razor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/FlowDebugger.razor.cs @@ -113,6 +113,17 @@ public partial class FlowDebugger : ComponentBase, IDisposable /// private bool _canLive; + // Gitea #55 (owner decision (b)): a site-scoped Deployer may not Live-run an inbound API + // method — the method has no site, so its central-side effects cannot be bounded by the + // scope. The server refuses it (FlowDebugRunner); the page disables the entry and says why. + // Script entry (instance-scoped) and Replay stay available. + private bool _methodLiveRefused; + + private const string MethodLiveRefusedHint = + "Live runs of an inbound API method are not available to a site-scoped Deployer: a method has no site, " + + "so its central-side effects (Database, ExternalSystem, notifications) cannot be bounded by your scope. " + + "Run a site script on a permitted instance, or use Replay."; + private List _methods = []; private List _sites = []; @@ -258,7 +269,7 @@ public partial class FlowDebugger : ComponentBase, IDisposable // Script entry: a write kind needs the value to write; a call kind needs a parameter form // that parses (a script declaring no parameters is runnable as soon as it is chosen). private bool CanRun => _canLive && !_running && _mode == FlowMode.Live && ( - _entry == EntryKind.Method ? _selectedMethodId > 0 && (_inputFields.Count == 0 || _methodParamsValid) + _entry == EntryKind.Method ? !_methodLiveRefused && _selectedMethodId > 0 && (_inputFields.Count == 0 || _methodParamsValid) : SelectedScript is { } s && (s.IsWriteKind ? !string.IsNullOrWhiteSpace(_valueChangeValue) : string.IsNullOrWhiteSpace(s.ParameterDefinitions) || _scriptParamsValid)); @@ -304,6 +315,9 @@ public partial class FlowDebugger : ComponentBase, IDisposable .HasClaim(JwtTokenService.RoleClaimType, Roles.Deployer); if (!_canLive) _mode = FlowMode.Replay; + _methodLiveRefused = _canLive && !await SiteScope.IsSystemWideAsync(); + if (_methodLiveRefused) + _entry = EntryKind.Script; _methods = (await InboundApi.GetAllApiMethodsAsync()).OrderBy(m => m.Name, StringComparer.OrdinalIgnoreCase).ToList(); _sites = await SiteScope.FilterSitesAsync(await SiteRepository.GetAllSitesAsync()); @@ -339,7 +353,7 @@ public partial class FlowDebugger : ComponentBase, IDisposable } } } - else if (MethodId is int mid && mid > 0 && _methods.Any(m => m.Id == mid)) + else if (!_methodLiveRefused && MethodId is int mid && mid > 0 && _methods.Any(m => m.Id == mid)) { _entry = EntryKind.Method; _selectedMethodId = mid; @@ -390,7 +404,7 @@ public partial class FlowDebugger : ComponentBase, IDisposable } } } - else + else if (!_methodLiveRefused) { var method = await JS.InvokeAsync("localStorage.getItem", StorageMethod); if (int.TryParse(method, NumberStyles.Integer, CultureInfo.InvariantCulture, out var mid) && _methods.Any(m => m.Id == mid)) @@ -411,6 +425,7 @@ public partial class FlowDebugger : ComponentBase, IDisposable private async Task SetEntryAsync(EntryKind entry) { if (_entry == entry) return; + if (entry == EntryKind.Method && _methodLiveRefused) return; _entry = entry; ResetResult(); _predicted = null; diff --git a/src/ZB.MOM.WW.ScadaBridge.ManagementService/FlowDebug/FlowDebugRunner.cs b/src/ZB.MOM.WW.ScadaBridge.ManagementService/FlowDebug/FlowDebugRunner.cs index 2dc90cf8..24976d3f 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ManagementService/FlowDebug/FlowDebugRunner.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ManagementService/FlowDebug/FlowDebugRunner.cs @@ -151,6 +151,27 @@ public sealed class FlowDebugRunner var method = await repo.GetApiMethodByIdAsync(cmd.ApiMethodId, ct).ConfigureAwait(false) ?? throw new ManagementCommandException($"ApiMethod with ID {cmd.ApiMethodId} not found.", NotFoundCode); + // Gitea #55 (owner decision (b), 2026-11-09): a site-scoped operator may not Live-run a + // method at all. ApiMethods have no site affinity, so the route-time scope below (F2) + // only bounds routed SITE traffic — the method's central-side effects (Database.*, + // ExternalSystem, notifications) would still run for another plant. Refused before + // anything executes; Replay and the instance-scoped script / value-change Live runs + // stay available to them. The refusal is audited against the operator's name. + if (IsSiteScoped(user)) + { + var refusal = MethodLiveRefusedMessage(user); + await WriteCentralAsync(sp, ScadaBridgeAuditEventFactory.Create( + AuditChannel.ApiInbound, AuditKind.InboundRequest, AuditStatus.Failed, + occurredAtUtc: DateTime.UtcNow, + actor: user.Username, + target: method.Name, + sourceNode: sp.GetService()?.NodeName, + executionId: Guid.NewGuid(), + errorMessage: refusal, + extra: new ScriptTraceExtra { Source = SourceTag }.ToJson()), ct).ConfigureAwait(false); + throw new SiteScopeViolationException(refusal); + } + var resolveRef = await SchemaRefResolver.BuildAsync( sp.GetService(), [method.ParameterDefinitions], ct).ConfigureAwait(false); var validation = ParameterValidator.Validate(cmd.Parameters, method.ParameterDefinitions, resolveRef); @@ -173,7 +194,9 @@ public sealed class FlowDebugRunner // Arch-review F2: a method Live run has no single target site to check up front the // way RunValueChangeAsync does — the operator picks a METHOD, and where it routes is // only known once the script runs. So the operator's scope is pushed into the - // RouteHelper and enforced at route time, before any site traffic. + // RouteHelper and enforced at route time, before any site traffic. Since Gitea #55 a + // scoped operator is refused above, so this resolves to null (unscoped) for every + // caller that reaches it; it is kept as defence in depth should that gate ever loosen. var scope = await ResolvePermittedSiteIdentifiersAsync(sp, user, ct).ConfigureAwait(false); // Gitea #54: a routed site run whose own collector dropped rows reports it here; the flags // are OR-ed into the result so the page labels the short site lane as truncated. @@ -708,6 +731,31 @@ public sealed class FlowDebugRunner } } + /// + /// True when is restricted to a site subset — carries site ids and + /// is not an Administrator — by the same rule applies. + /// + /// The operator. + /// true for a site-scoped operator. + public static bool IsSiteScoped(AuthenticatedUser user) + { + ArgumentNullException.ThrowIfNull(user); + return user.PermittedSiteIds.Length > 0 + && !user.Roles.Contains(Roles.Administrator, StringComparer.OrdinalIgnoreCase); + } + + /// The refusal a site-scoped operator gets for a method-entry Live run (Gitea #55). + /// The site-scoped operator. + /// The operator-facing refusal message. + public static string MethodLiveRefusedMessage(AuthenticatedUser user) + { + ArgumentNullException.ThrowIfNull(user); + return $"Access denied: your Deployment role is scoped to sites [{string.Join(", ", user.PermittedSiteIds)}], " + + "and an inbound API method has no site, so its central-side effects (Database, ExternalSystem, " + + "notifications) cannot be bounded by your scope. Live method runs require a system-wide Deployer " + + "or an Administrator. Replay, and Live runs of a site script on a permitted instance, remain available."; + } + private static void EnforceSiteScope(AuthenticatedUser user, int siteId) { if (user.PermittedSiteIds.Length == 0) return; diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Deployment/FlowDebuggerPageTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Deployment/FlowDebuggerPageTests.cs index 1b6904ce..c8f8f1ab 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Deployment/FlowDebuggerPageTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Deployment/FlowDebuggerPageTests.cs @@ -167,6 +167,55 @@ public class FlowDebuggerPageTests : BunitContext Assert.NotNull(cut.Find("button[data-test='flow-mode-replay']")); } + // ---- Gitea #55: a site-scoped Deployer gets no method-entry Live run ------------------------- + + /// Verifies that a site-scoped deployer sees the method entry disabled with an explanation, and starts on script entry. + [Fact] + public void ScopedDeployer_MethodEntryIsDisabledAndExplained_ScriptEntryStaysAvailable() + { + RegisterServices(extraClaims: [new Claim(JwtTokenService.SiteIdClaimType, "1")]); + + var cut = Render(); + + var method = cut.Find("button[data-test='flow-entry-method']"); + Assert.True(method.HasAttribute("disabled")); + Assert.Contains("site-scoped", method.GetAttribute("title")); + Assert.Contains("site-scoped", cut.Find("[data-test='flow-method-live-refused']").TextContent); + // Script entry is the starting entry, so its site picker — not the method select — is shown. + Assert.False(cut.Find("button[data-test='flow-entry-script']").HasAttribute("disabled")); + Assert.NotNull(cut.Find("select[data-test='flow-site-select']")); + Assert.Empty(cut.FindAll("select[data-test='flow-method-select']")); + // Replay stays one click away. + Assert.NotNull(cut.Find("button[data-test='flow-mode-replay']")); + } + + /// Verifies that a site-scoped deployer drilling in with a method id never reaches a runnable method pane. + [Fact] + public void ScopedDeployer_DrillInWithAMethodId_DoesNotSelectTheMethod() + { + RegisterServices(extraClaims: [new Claim(JwtTokenService.SiteIdClaimType, "1")]); + Services.GetRequiredService().NavigateTo("/deployment/flow-debugger?methodId=4"); + + var cut = Render(); + + Assert.Empty(cut.FindAll("select[data-test='flow-method-select']")); + Assert.True(cut.Find("button[data-test='flow-run']").HasAttribute("disabled")); + _flow.DidNotReceive().PredictMethodAsync(Arg.Any(), Arg.Any()); + } + + /// Verifies that an unscoped deployer keeps the method entry enabled with no refusal note. + [Fact] + public void UnscopedDeployer_MethodEntryStaysEnabled() + { + RegisterServices(); + + var cut = Render(); + + Assert.False(cut.Find("button[data-test='flow-entry-method']").HasAttribute("disabled")); + Assert.Empty(cut.FindAll("[data-test='flow-method-live-refused']")); + Assert.NotNull(cut.Find("select[data-test='flow-method-select']")); + } + // ---- F3: Replay opens to the OperationalAudit roles; Live stays Deployer-only ---------------- /// Verifies that the page renders replay-only for a viewer, with no live run controls. @@ -457,10 +506,17 @@ public class FlowDebuggerPageTests : BunitContext public void ParkedNode_OutOfScopeSite_RefusedBeforeConfirm() { // Deployer scoped to site 2 only; the parked call belongs to site-a (id 1). + // A scoped Deployer cannot Live-run a method (Gitea #55), so the parked node is reached through Replay. RegisterServices(extraClaims: [new Claim(JwtTokenService.SiteIdClaimType, "2")]); var root = Guid.NewGuid(); var (parked, _, _) = ParkedThenResolved(root); - var cut = RunToParkedNode(root, parked); + _flow.ReplayAsync(root, Arg.Any()).Returns(parked); + Services.GetRequiredService().NavigateTo($"/deployment/flow-debugger?executionId={root}"); + var cut = Render(); + cut.Instance.Delay = (_, _) => Task.CompletedTask; + cut.WaitForAssertion(() => Assert.Equal("Completed", cut.Find("[data-test='flow-state']").TextContent.Trim())); + cut.Find("g[data-test='flow-node'][data-kind='ApiCall']").Click(); + cut.WaitForElement("button[data-test='flow-step-retry']"); cut.Find("button[data-test='flow-step-retry']").Click(); diff --git a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/FlowDebugRunnerTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/FlowDebugRunnerTests.cs index 0ba7212b..c6d24f67 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/FlowDebugRunnerTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/FlowDebugRunnerTests.cs @@ -575,37 +575,74 @@ public class FlowDebugRunnerTests return sites; } - /// Verifies that a site-scoped deployer routing to another site is unauthorized and the inbound row is still written. + // --- Gitea #55 (owner decision (b)): a site-scoped operator may not Live-run a method at all --- + // + // F2's route-time scope only bounds routed SITE traffic; a method's central-side effects + // (Database.*, ExternalSystem, notifications) have no site to scope against. So a scoped + // operator is refused before the script executes — whether or not it would route in scope. + + /// Verifies that a site-scoped deployer is refused a method Live run, nothing executes, and the refusal is audited. + /// The site the method would route to — in or out of the operator's scope. /// A task that represents the asynchronous operation. - [Fact] - public async Task RunMethod_ScopedDeployer_RouteToOtherSite_IsUnauthorized_AndInboundRowIsWritten() + [Theory] + [InlineData("site-a")] // within scope (id 1 → site-a): still refused, the central effects are unscoped + [InlineData("site-b")] // outside scope + public async Task RunMethod_ScopedDeployer_IsRefused_BeforeTheScriptRuns_AndTheRefusalIsAudited(string routesTo) { - WireRoutedMethod(11, "inst", "site-b"); + WireRoutedMethod(11, "inst", routesTo); + var ran = false; + _executor.RegisterHandler("M", _ => { ran = true; return Task.FromResult(1); }); var scoped = new AuthenticatedUser("ops", "Ops", new[] { "Deployer" }, new[] { "1" }); - await Assert.ThrowsAsync(() => Runner().RunMethodAsync( + var ex = await Assert.ThrowsAsync(() => Runner().RunMethodAsync( _services.BuildServiceProvider(), new FlowDebugRunMethodCommand(11, null), scoped, CancellationToken.None)); - // The site was never contacted... + Assert.Equal(FlowDebugRunner.MethodLiveRefusedMessage(scoped), ex.Message); + Assert.Contains("Replay", ex.Message); + Assert.False(ran, "the method script must not execute for a site-scoped operator"); await _router.DidNotReceiveWithAnyArgs().RouteToCallAsync(default!, default!, default); - // ...but the denied attempt IS recorded against the operator's name. - Assert.Contains(_central, e => e.AsRow().Kind == AuditKind.InboundRequest && e.Actor == "ops"); + // The refused attempt IS recorded against the operator's name. + var row = Assert.Single(_central, e => e.AsRow().Kind == AuditKind.InboundRequest); + Assert.Equal("ops", row.Actor); + Assert.Equal(AuditStatus.Failed, row.AsRow().Status); } - /// Verifies that a site-scoped deployer routing within its scope runs the method. + /// Verifies that a site-scoped deployer can still Replay a method execution. /// A task that represents the asynchronous operation. [Fact] - public async Task RunMethod_ScopedDeployer_RouteWithinScope_Runs() + public async Task Replay_ScopedDeployer_IsAllowed() { - // The same scoped user, routing to a site their numeric id DOES resolve to. - WireRoutedMethod(12, "inst", "site-a"); + var repo = Substitute.For(); + var root = Guid.NewGuid(); + var rows = new[] + { + ScadaBridgeAuditEventFactory.Create(AuditChannel.ApiInbound, AuditKind.InboundRequest, AuditStatus.Delivered, executionId: root), + }; + repo.GetTraceEventsAsync(root, Arg.Any(), Arg.Is>(s => s.SequenceEqual(new[] { "1" }))) + .Returns(new ZB.MOM.WW.ScadaBridge.Commons.Types.Audit.TraceEventBatch(root, rows, false)); + _services.AddSingleton(repo); var scoped = new AuthenticatedUser("ops", "Ops", new[] { "Deployer" }, new[] { "1" }); - var result = await Runner().RunMethodAsync( - _services.BuildServiceProvider(), new FlowDebugRunMethodCommand(12, null), scoped, CancellationToken.None); + var result = await Runner().ReplayAsync(_services.BuildServiceProvider(), new FlowDebugReplayCommand(root), CancellationToken.None, scoped); Assert.True(result.Completed, result.Error); - await _router.ReceivedWithAnyArgs(1).RouteToCallAsync(default!, default!, default); + Assert.Equal(rows, result.Events); + } + + /// Verifies that a site-scoped deployer's value-change Live run on a permitted instance still runs (instance-scoped entry is unchanged by #55). + /// A task that represents the asynchronous operation. + [Fact] + public async Task RunValueChange_ScopedDeployer_InScopeSite_StillRuns() + { + var (_, _, puller) = WireValueChange(); + puller.PullAsync(default!, default!, default, default, default, default).ReturnsForAnyArgs(Array.Empty()); + var scoped = new AuthenticatedUser("ops", "Ops", new[] { "Deployer" }, new[] { "2" }); // instance 5 lives on site 2 + + var result = await Runner().RunValueChangeAsync(_services.BuildServiceProvider(), + new FlowDebugRunValueChangeCommand(5, "Status", "Running", WaitSeconds: 3), scoped, CancellationToken.None); + + await _router.ReceivedWithAnyArgs(1).RouteToSetAttributesAsync(default!, default!, default); + Assert.Equal(FlowDebugRunner.NoTriggerCode, result.ErrorCode); // ran to the poll, not refused } /// Verifies that an administrator is never restricted by site scope.