diff --git a/CLAUDE.md b/CLAUDE.md index b8ce6646..7fa48eac 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -95,7 +95,7 @@ Other peers in the `scadaproj` family (see `scadaproj/CLAUDE.md` for details): ` 21. Notification Outbox — Central component ingesting store-and-forwarded notifications, `Notifications` audit table, dispatcher loop, retry/parking, delivery KPIs. 22. Site Call Audit — Central component auditing site cached calls (`CachedCall`/`CachedWrite`); `SiteCalls` audit table, telemetry ingest, reconciliation, KPIs, central→site Retry/Discard relay; sites remain the source of truth. 23. Audit Log — Central append-only AuditLog table spanning every script-trust-boundary action (outbound API sync+cached, outbound DB sync+cached, notifications, inbound API). Site SQLite hot-path + gRPC telemetry + reconciliation; combined telemetry with Site Call Audit; central direct-write for Notification Outbox dispatch + Inbound API; monthly partitioning, 365-day retention. -24. Transport — File-based, encrypted bundle export/import via Central UI. Templates (with **all** child collections — attributes/alarms/scripts/compositions/**native-alarm-sources**, field-complete incl. `LockedInDerived` + script cadence/timeout), system artifacts, central-only configuration, plus site/instance-scoped config (`Site`s, site `DataConnection`s, `Instance`s + real `AreaName` by name) reconciled across environments by a `BundleNameMap` name-mapping subsystem. Per-conflict resolution with a per-line Myers diff. Runs the **script trust gate** at import review (5th `ScriptTrustValidator` call site — forbidden-API scripts rejected pre-runtime; template-script name-resolution findings are advisory warnings, ApiMethod findings hard-block). Correlated audit via `BundleImportId`. Never touches site runtime nodes (imported instances land `NotDeployed`). +24. Transport — File-based, encrypted bundle export/import via Central UI. Templates (with **all** child collections — attributes/alarms/scripts/compositions/**native-alarm-sources**, field-complete incl. `LockedInDerived` + script cadence/timeout), system artifacts, central-only configuration, plus site/instance-scoped config (`Site`s, site `DataConnection`s, `Instance`s + real `AreaName` by name) reconciled across environments by a `BundleNameMap` name-mapping subsystem. Per-conflict resolution with a per-line Myers diff. Runs the **script trust gate** at import review (5th `ScriptTrustValidator` call site — forbidden-API scripts rejected pre-runtime; covers template/shared/ApiMethod bodies, template script + alarm Expression-trigger bodies, **and instance alarm-override trigger expressions** — all hard-block; template-script name-resolution findings are advisory warnings, ApiMethod findings hard-block). Correlated audit via `BundleImportId`. Never touches site runtime nodes (imported instances land `NotDeployed`). 25. Script Analysis — Shared authoritative script-trust analyzer: unified forbidden-API deny-list (`ScriptTrustPolicy`), fused semantic+syntactic validator (`ScriptTrustValidator`), Roslyn compile wrapper (`RoslynScriptCompiler`), and compile-only globals stubs (`ScriptCompileSurface`/`TriggerCompileSurface`); consumed by Template Engine, Site Runtime, Inbound API, and Central UI. 26. KPI History — Reusable central KPI-history backbone: tall/EAV `KpiSample` store in central MS SQL, `KpiHistoryRecorderActor` cluster singleton (`kpi-history-recorder`, not readiness-gated) sampling DI-registered `IKpiSampleSource`s every minute, bucketed query (`GetRawSeriesAsync` + `KpiSeriesBucketer`) + scoped `KpiHistoryQueryService`, and a reusable custom-SVG `KpiTrendChart`; ships trends for Notification Outbox, Site Call Audit, Audit Log, and Site Health. 27. DelmiaNotifier — Standalone external client tool (NOT a cluster component, NOT in the Host): a compact Native-AOT (`win-x64`) console app (`WWNotifier.exe`) that DELMIA Apriso shells out to per recipe download. POSTs to the Inbound API `DelmiaRecipeDownload` method (`X-API-Key`, key from `SCADABRIDGE_API_KEY`), with connect-failure-only failover across a comma-list of base URLs, and reports the legacy `YES`/`NO` + exit-code stdout contract — a drop-in replacement for the legacy `WWNotifier` (see `docs/former-api-specs/dnc/`). Zero-dependency BCL-only, `System.Text.Json` source-gen. Project README: `src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/README.md`; design: `docs/plans/2026-06-26-delmia-recipe-notifier-design.md`. @@ -150,7 +150,7 @@ Other peers in the `scadaproj` family (see `scadaproj/CLAUDE.md` for details): ` - Last-write-wins for concurrent template editing (no optimistic concurrency on templates). - Optimistic concurrency on deployment status records. - Naming collisions in composed feature modules are design-time errors. -- Transport (#24, M8): bundles now move site/instance config (`Site`s, site-scoped `DataConnection`s, `Instance`s + override children + `Area` by name), not just central-only config — but still never touch site runtime nodes (imported instances land `NotDeployed`). A `BundleNameMap` name-mapping subsystem (Commons: `SiteMapping`/`ConnectionMapping`/`MappingAction`) reconciles environment-specific identifiers (sites by `SiteIdentifier`, connections by site-qualified `{SiteIdentifier}/{Name}`, instances by `UniqueName`): auto-match → operator override via the import-wizard Map step / CLI `--map-site`/`--map-connection`/`--create-missing-*` → blocker rows for the unresolved. D3 "carry full config (encrypted secrets)": Site addresses travel; `DataConnection` config rides the encrypted `SecretsBlock` (presence-only in diffs). Per-line Myers diff for code fields via the pure `LineDiffer` (`ArtifactDiff` embeds a size-capped structured `lineDiff`; input capped at `MaxInputLines`=4000 → summary-only, so a crafted bundle can't OOM the active node); `ImportResult.StaleInstanceIds` is real via the `IStaleInstanceProbe` seam (Commons; implemented in DeploymentManager). `schemaVersion` 1.0→1.1 (additive); `bundleFormatVersion` stays 1; no new EF tables/columns. **Arch-review 05 (plan 05):** template child fidelity completed (round-trip-guard-verified: `LockedInDerived`, native-alarm-sources, script cadence/timeout, real `AreaName`, ES retry config + methods-on-Add, notification recipients, folder parent edges); import runs the **script trust gate** (template/shared/ApiMethod bodies **and** Expression-trigger bodies) as a hard pre-runtime gate + a name-resolution severity split (template=advisory `ConflictKind.Warning`+`ImportResult.Warnings`, ApiMethod=hard error); `ArtifactDiff` resolves folder/base/composition names (unchanged structured templates classify Identical again); `MaxConcurrentImportSessions`=8 session cap. Rename does NOT rewrite call sites (deploy gate on the target is the real check). Plan-06 handoff: `ScriptArtifactsChanged`/`IScriptArtifactChangeBus` seam + Host in-process bus + Transport post-commit publisher shipped (consumers = plan 06). +- Transport (#24, M8): bundles now move site/instance config (`Site`s, site-scoped `DataConnection`s, `Instance`s + override children + `Area` by name), not just central-only config — but still never touch site runtime nodes (imported instances land `NotDeployed`). A `BundleNameMap` name-mapping subsystem (Commons: `SiteMapping`/`ConnectionMapping`/`MappingAction`) reconciles environment-specific identifiers (sites by `SiteIdentifier`, connections by site-qualified `{SiteIdentifier}/{Name}`, instances by `UniqueName`): auto-match → operator override via the import-wizard Map step / CLI `--map-site`/`--map-connection`/`--create-missing-*` → blocker rows for the unresolved. D3 "carry full config (encrypted secrets)": Site addresses travel; `DataConnection` config rides the encrypted `SecretsBlock` (presence-only in diffs). Per-line Myers diff for code fields via the pure `LineDiffer` (`ArtifactDiff` embeds a size-capped structured `lineDiff`; input capped at `MaxInputLines`=4000 → summary-only, so a crafted bundle can't OOM the active node); `ImportResult.StaleInstanceIds` is real via the `IStaleInstanceProbe` seam (Commons; implemented in DeploymentManager). `schemaVersion` 1.0→1.1 (additive); `bundleFormatVersion` stays 1; no new EF tables/columns. **Arch-review 05 (plan 05):** template child fidelity completed (round-trip-guard-verified: `LockedInDerived`, native-alarm-sources, script cadence/timeout, real `AreaName`, ES retry config + methods-on-Add, notification recipients, folder parent edges); import runs the **script trust gate** (template/shared/ApiMethod bodies, template script + alarm Expression-trigger bodies, **and instance alarm-override trigger expressions** — R2-05) as a hard pre-runtime gate + a name-resolution severity split (template=advisory `ConflictKind.Warning`+`ImportResult.Warnings`, ApiMethod=hard error); `ArtifactDiff` resolves folder/base/composition names (unchanged structured templates classify Identical again); `MaxConcurrentImportSessions`=8 session cap (startup-validated `>0`, R2-05). **R2-05 also:** the post-commit `ScriptArtifactsChanged` publisher now covers `Add` resolutions (not just Overwrite/Rename), import warns on instance overrides targeting locked template members (advisory, rows still written), and the Expression-trigger syntax check is surface-keyed-cached + skipped on read-only staleness/comparison paths (`validateScriptCompilation:false`). Rename does NOT rewrite call sites (deploy gate on the target is the real check). Plan-06 handoff: `ScriptArtifactsChanged`/`IScriptArtifactChangeBus` seam + Host in-process bus + Transport post-commit publisher shipped (consumers = plan 06). ### Store-and-Forward - Fixed retry interval, no max buffer size. Only transient failures buffered. diff --git a/docs/requirements/Component-ScriptAnalysis.md b/docs/requirements/Component-ScriptAnalysis.md index 3a680046..eaf96b2e 100644 --- a/docs/requirements/Component-ScriptAnalysis.md +++ b/docs/requirements/Component-ScriptAnalysis.md @@ -151,6 +151,8 @@ This allows the compile to bind `Attributes["name"]`, `Notify.To("x").Send(...)` Mirrors `TriggerExpressionGlobals` in the same way. Used by `ValidationService.CheckExpressionSyntax` in the Template Engine for conditional and expression trigger validation. +`CheckExpressionSyntax` memoises its verdict in the Template Engine's process-wide `ScriptCompileVerdictCache`, whose key is the pair **(globals surface, SHA-256 of the code)** — not the code alone. The surface discriminator is load-bearing: a trigger expression vetted against `TriggerCompileSurface` is **not** interchangeable with a `ScriptCompileSurface` script-body verdict (different globals resolve different identifiers), so a code-only key could return a stale "clean" for code never compiled against the caller's surface. Keying by surface makes that cross-surface verdict reuse structurally impossible. + #### Parity guard A reflection-based parity test in `SiteRuntime.Tests` compares the public member names on `ScriptCompileSurface` against `ScriptGlobals` (and `TriggerCompileSurface` against `TriggerExpressionGlobals`). Any drift between the stub and the real globals causes this test to fail, ensuring the stubs cannot silently fall out of sync. @@ -170,7 +172,7 @@ All five call sites that previously maintained their own script trust enforcemen | **Site Runtime** `ScriptCompilationService.ValidateTrustModel` | Semantic resolver, no reflection-gateway hardening | Delegates to `FindViolations`; retains `CSharpScript.Compile` against real `ScriptGlobals` for execution | | **Inbound API** `ForbiddenApiChecker.FindViolations` | Syntactic walker, forbade all `System.Diagnostics` | Thin shim delegating to `ScriptTrustValidator.FindViolations`; `System.Diagnostics` loosened to `.Process`-only | | **Central UI** `ScriptAnalysisService` | Semantic + full compile, lenient threading | Delegates forbidden-API verdict and sources editor-marker deny-list from `ScriptTrustPolicy`; retains Test-Run execution host | -| **Transport** `BundleImporter.RunSemanticValidationAsync` / `DetectBlockersAsync` | No trust gate — a bundle's forbidden-API scripts imported clean, deferred to runtime (fifth write path) | Runs `ScriptTrustValidator.FindViolations` over every non-Skip template / shared / ApiMethod body at import review — hard error (apply) / Blocker row (preview) for all kinds | +| **Transport** `BundleImporter.RunSemanticValidationAsync` / `DetectBlockersAsync` | No trust gate — a bundle's forbidden-API scripts imported clean, deferred to runtime (fifth write path) | Runs `ScriptTrustValidator.FindViolations` over every non-Skip template / shared / ApiMethod body, template script + alarm Expression-trigger bodies, **and instance alarm-override trigger expressions** at import review — hard error (apply) / Blocker row (preview) for all kinds | The static enforcement is **defence-in-depth**, not a true runtime sandbox. Scripts execute in-process; the denied API list prevents obvious escapes at compile time but does not provide the isolation guarantees of an out-of-process sandbox or a restricted `AssemblyLoadContext`. This caveat applies to all consumers. @@ -190,4 +192,4 @@ No dependency on Akka.NET, ASP.NET Core, Entity Framework, or any other ScadaBri - **Site Runtime (#3)**: `ScriptCompilationService.ValidateTrustModel` delegates the trust verdict to `ScriptTrustValidator.FindViolations`; retains its own `CSharpScript.Compile` against the real `ScriptGlobals` for execution-time compilation. - **Inbound API (#14)**: `ForbiddenApiChecker.FindViolations` is a thin shim over `ScriptTrustValidator.FindViolations`. - **Central UI (#9)**: `ScriptAnalysisService` delegates the run-gate forbidden-API verdict and sources the editor-marker deny-list from `ScriptTrustPolicy`; retains the Test-Run execution host (`SandboxScriptHost`). -- **Transport (#24)**: `BundleImporter` runs `ScriptTrustValidator.FindViolations` over every non-Skip template, shared, and ApiMethod script body during import validation (`RunSemanticValidationAsync` Pass 0) and preview (`DetectBlockersAsync`), rejecting forbidden-API scripts at import review rather than at runtime. Trust violations are hard errors for all script kinds (unlike the false-positive-prone name-resolution heuristic, whose template-script findings are advisory). +- **Transport (#24)**: `BundleImporter` runs `ScriptTrustValidator.FindViolations` over every non-Skip template, shared, and ApiMethod script body, template script + alarm Expression-trigger bodies, **and instance alarm-override trigger expressions** (`InstanceAlarmOverrideDto.TriggerConfigurationOverride`) during import validation (`RunSemanticValidationAsync` Pass 0) and preview (`DetectBlockersAsync`), rejecting forbidden-API scripts at import review rather than at runtime. Trust violations are hard errors for all script kinds (unlike the false-positive-prone name-resolution heuristic, whose template-script findings are advisory). diff --git a/docs/requirements/Component-TemplateEngine.md b/docs/requirements/Component-TemplateEngine.md index 0dd4f20f..9542f82c 100644 --- a/docs/requirements/Component-TemplateEngine.md +++ b/docs/requirements/Component-TemplateEngine.md @@ -184,6 +184,8 @@ Before a deployment is sent to a site, the Template Engine performs comprehensiv - **Script compilation and trust check**: All instance scripts and alarm on-trigger scripts are compiled via `ScriptCompiler.TryCompile`, which performs a real Roslyn type-checking compile (using `ScriptCompileSurface` from the Script Analysis component as the globals type) and an authoritative forbidden-API check (via `ScriptTrustValidator.FindViolations`). Scripts with compilation errors, type errors, or forbidden-API violations are rejected. This gate is authoritative — not advisory — meaning alias, `using static`, and `global::` bypasses are caught by semantic symbol resolution. - **Alarm trigger references**: Alarm trigger definitions reference attributes that exist in the flattened configuration. - **Script trigger references**: Script triggers (value change, conditional) reference attributes that exist in the flattened configuration. +- **Expression-trigger syntax/trust check**: Each Expression-triggered script/alarm has its boolean expression run through `ValidationService.CheckExpressionSyntax` — a `ScriptTrustValidator.FindViolations` forbidden-API verdict followed by a real Roslyn compile against `TriggerCompileSurface`. Both stages report **all** findings (every forbidden API, every compile error), not just the first, so an operator fixing a multi-error expression sees the full list in one round-trip. The verdict is memoised in the surface-keyed `ScriptCompileVerdictCache`, so an unchanged expression compiles once per process. +- **Read-only skip (`validateScriptCompilation: false`)**: The `Validate` flag that read-only staleness/comparison callers (DeploymentManager's `FlatteningPipeline` on its comparison/probe paths) pass to skip the expensive Roslyn script-body compile now **also** skips the Expression-trigger syntax/compile check — the two compile-bearing stages. The cheap structural checks (blank-expression, attribute-reference scan) still run, and the deploy gate keeps the default (`true`), so it remains the authoritative check for both stages. - **Data connection binding completeness**: Every attribute with a data source reference has a data connection binding assigned on the instance, and the bound data connection name exists as a defined connection at the instance's site. - **Exception**: Validation does **not** verify that data source relative paths resolve to real tags on physical devices — that is a runtime concern. diff --git a/docs/requirements/Component-Transport.md b/docs/requirements/Component-Transport.md index 6b34b30a..8ff0a1df 100644 --- a/docs/requirements/Component-Transport.md +++ b/docs/requirements/Component-Transport.md @@ -26,7 +26,7 @@ As of M8 (T18), Transport is no longer limited to central-only configuration: it - Validate `manifest.json` on upload: format version gating, SHA-256 content hash verification. - Manage in-memory `BundleSession` objects: 30-minute TTL, 3-strike passphrase lockout per session. - Compute a per-artifact diff between bundle contents and the target environment, classifying each artifact as Identical, Modified, New, a Blocker (import-stopping), or a Warning (advisory, non-blocking). -- Apply user-supplied conflict resolutions (Add, Overwrite, Skip, Rename) in a single EF transaction, running multi-pass semantic validation before committing: a **script trust gate** (`ScriptTrustValidator.FindViolations` over every non-Skip template / shared / ApiMethod body — bundle import is the fifth script-trust call site, so forbidden-API scripts are rejected at import review, not deferred to runtime), then a minimal name-resolution scan over the merged target (ApiMethod findings fail fast on unresolved SharedScript / ExternalSystem identifiers; template-script findings are advisory warnings), then the full `SemanticValidator` from `ZB.MOM.WW.ScadaBridge.TemplateEngine` over each imported template's per-template `FlattenedConfiguration`. +- Apply user-supplied conflict resolutions (Add, Overwrite, Skip, Rename) in a single EF transaction, running multi-pass semantic validation before committing: a **script trust gate** (`ScriptTrustValidator.FindViolations` over every non-Skip executable C# surface a bundle carries — template / shared / ApiMethod script bodies, template script **and** alarm **Expression-trigger** bodies, **and instance alarm-override trigger expressions** (`InstanceAlarmOverrideDto.TriggerConfigurationOverride`); bundle import is the fifth script-trust call site, so forbidden-API scripts are rejected at import review, not deferred to runtime — all these surfaces hard-block uniformly, since the trust gate's semantic verdict is authoritative with no false-positive channel, and the advisory-vs-hard severity split is a property of the name-resolution heuristic only), then a minimal name-resolution scan over the merged target (ApiMethod findings fail fast on unresolved SharedScript / ExternalSystem identifiers; template-script findings are advisory warnings), then the full `SemanticValidator` from `ZB.MOM.WW.ScadaBridge.TemplateEngine` over each imported template's per-template `FlattenedConfiguration`. - Emit `BundleExported`, `BundleImported`, `BundleImportFailed`, `UnencryptedBundleExport`, `BundleImportUnlockFailed`, `BundleImportAlarmScriptUnresolved`, `BundleImportCompositionUnresolved`, and `BundleImportBaseTemplateUnresolved` audit events via `IAuditService`. - Thread a `BundleImportId` correlation GUID through every per-entity `AuditLogEntry` written during `ApplyAsync` via a scoped `IAuditCorrelationContext`. - Enforce `RequireDesign` on export and `RequireAdmin` on import both at the Razor page layer and inside the service entrypoints (defense in depth). @@ -133,7 +133,7 @@ flowchart TD class T muted ``` -The component is central-hosted. It is registered in `ZB.MOM.WW.ScadaBridge.Host` for central roles only, never for site roles — even when a bundle carries site-scoped config (T18), Transport only writes the central configuration database, never the site clusters. All persistence flows through existing audited repository interfaces in `ZB.MOM.WW.ScadaBridge.ConfigurationDatabase` — the component does not call `DbContext.SaveChangesAsync` directly. `BundleSessionStore` is in-process on the active central node (matching Blazor Server circuit affinity): 30-minute TTL, eviction on expiry, 3-strike passphrase lockout per session, and a `MaxConcurrentImportSessions` cap (default 8; arch-review 05 Task 24) bounding the decrypted-content footprint since each open session pins a fully-decrypted bundle in memory. +The component is central-hosted. It is registered in `ZB.MOM.WW.ScadaBridge.Host` for central roles only, never for site roles — even when a bundle carries site-scoped config (T18), Transport only writes the central configuration database, never the site clusters. All persistence flows through existing audited repository interfaces in `ZB.MOM.WW.ScadaBridge.ConfigurationDatabase` — the component does not call `DbContext.SaveChangesAsync` directly. `BundleSessionStore` is in-process on the active central node (matching Blazor Server circuit affinity): 30-minute TTL, eviction on expiry, 3-strike passphrase lockout per session, and a `MaxConcurrentImportSessions` cap (default 8; arch-review 05 Task 24 — startup-validated as `> 0` by `TransportOptionsValidator`, since a zero/negative cap would reject every import session forever) bounding the decrypted-content footprint since each open session pins a fully-decrypted bundle in memory. **Preview→apply window (recorded decision).** The diff/preview an operator reviews is a point-in-time snapshot; there is **no optimistic concurrency** between preview and apply. If the target config changes in the window between `PreviewAsync` and `ApplyAsync` (another admin edits a template, say), the apply proceeds against the current state — the resolutions are keyed by `(EntityType, Name)`, so they still target the right rows, but the operator may not have seen the latest state in the diff. This is accepted for v1 (single-operator import is the expected workflow, and the terminal Confirm step already gates a typo-resistant environment-name entry). The bundle-format `version` fields are reserved for a future optimistic-concurrency check but are not yet enforced (Underdeveloped 5). @@ -233,7 +233,11 @@ Bundle references that cannot be satisfied in either the bundle or the target DB **Blocker-scan heuristic boundaries.** The scanner walks `TemplateScript.Code`, `TemplateAttribute.Value`, and `ApiMethod.Script` looking for top-level `Identifier(` or `Identifier.` tokens. To keep the heuristic usable on real script bodies it (a) skips identifiers preceded by `.` (member access — `obj.Method()` does not flag `Method`); (b) does NOT scan `TemplateAttribute.DataSourceReference` (an OPC UA address path, never script source); (c) excludes names **declared locally in the body** — every `LocalFunctionStatement` / `MethodDeclaration` identifier is collected via a Roslyn script-kind parse, so a script's own helper (`decimal ComputeRate(...) => …; return ComputeRate(x);`) is not mistaken for a missing SharedScript; and (d) filters out a `KnownNonReferenceNames` denylist of .NET stdlib types (`Convert`, `DateTimeOffset`, `Regex`, `StringBuilder`, `Parse`, LINQ operators, …), ScadaBridge runtime API roots (`Notify`, `Database`, `ExternalSystem`, `Scripts`, `Instance`, `Parameters`, `Attributes`, `Route`, …), and common SQL keywords that appear inside string literals (`COUNT`, `SELECT`, `FROM`, `HAVING`, `VALUES`, …). Both the diff-step `DetectBlockersAsync` and the Apply-time `RunSemanticValidationAsync` Pass 1 share this filter, so the diff preview and the Apply gate agree. -**Severity split — template-script findings are advisory, ApiMethod findings block.** A residual unresolved identifier (one the heuristic could not filter) is classified by its **origin**. Findings from **template scripts** and **attribute default expressions** are non-blocking **warnings**: they surface as `ConflictKind.Warning` rows in the preview and ride on `ImportResult.Warnings` after a successful apply — the import proceeds because the authoritative design-time **deploy gate re-validates** the script with a real compile (`ScriptCompiler.TryCompile`), so a heuristic false positive here must never stop a valid import. Findings from **ApiMethod scripts** remain **hard errors** (`ConflictKind.Blocker` in preview; `SemanticValidationException` at apply) because inbound API methods have no downstream design-time gate — the import review is their only pre-runtime check. The split is applied identically in `DetectBlockersAsync` and `RunSemanticValidationAsync` Pass 1 so preview and apply agree. +**Severity split — template-script findings are advisory, ApiMethod findings block.** A residual unresolved identifier (one the heuristic could not filter) is classified by its **origin**. Findings from **template scripts** and **attribute default expressions** are non-blocking **warnings**: they surface as `ConflictKind.Warning` rows in the preview and ride on `ImportResult.Warnings` after a successful apply — the import proceeds because the authoritative design-time **deploy gate re-validates** the script with a real compile (`ScriptCompiler.TryCompile`), so a heuristic false positive here must never stop a valid import. Findings from **ApiMethod scripts** remain **hard errors** (`ConflictKind.Blocker` in preview; `SemanticValidationException` at apply) because inbound API methods have no downstream design-time gate — the import review is their only pre-runtime check. The split is applied identically in `DetectBlockersAsync` and `RunSemanticValidationAsync` Pass 1 so preview and apply agree. (This advisory-vs-hard split is a property of the name-resolution **heuristic** only — it is false-positive-prone; the **script trust gate** above has no false-positive channel, so its verdict is a hard error for every gated surface, template trigger expressions and instance alarm-override trigger expressions included.) + +**Locked-member override warnings (advisory).** An instance override — attribute (`FlatteningService` drop), alarm, or native-alarm-source — that targets a **locked** template member is silently inert: the flattener ignores it. To stop that being invisible, a best-effort scan (`CollectLockedOverrideWarnings`) surfaces each such override as a `ConflictKind.Warning` preview row and on `ImportResult.Warnings` after apply, matching locked members by **direct name** (own + inherited placeholder rows both carry the `IsLocked` flag). It never blocks the import and never changes what is written — the override row is still persisted verbatim (bundle fidelity); the flattener remains the behavioural authority, and composed path-qualified members or derived-shadow (`LockedInDerived`) locks a placeholder row does not reflect emit no warning (never a false positive — the ManagementActor and flattener stay the enforcement points). + +**Script-artifact change bus (post-commit).** After a successful apply the importer publishes one `ScriptArtifactsChanged` per script-bearing kind (ApiMethod / SharedScript / Template) for **every non-Skip resolution — Overwrite, Rename, AND Add** — because an artifact deleted on the target and re-imported as `Add` under the same name can still leave a stale compiled-handler/`_knownBadMethods` entry on a node; over-approximation is safe (the bus is advisory, at-least-once, and consumers self-heal). The **subscriber** side (Inbound API cache invalidation wiring) lives with the Inbound API component — see PLAN-R2-06. **Step 5 — Confirm.** Final summary plus a "N instances will become stale" warning enumerating affected instances (a real count as of M8 — see "Stale-Instance Signaling"). User types the source environment name to confirm (typo-resistant gate at the prod boundary). diff --git a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/ScriptCompileVerdictCache.cs b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/ScriptCompileVerdictCache.cs index 794fe3fc..f1b7a599 100644 --- a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/ScriptCompileVerdictCache.cs +++ b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/ScriptCompileVerdictCache.cs @@ -5,14 +5,25 @@ using System.Text; namespace ZB.MOM.WW.ScadaBridge.TemplateEngine.Validation; /// -/// Process-wide cache of script compile verdicts, keyed on a SHA-256 hash of the -/// script code. The verdict (clean/compiles, or the name-free error text) is a pure -/// function of the code plus the compile-time-static ScriptTrustPolicy and -/// globals surface, so it is sound to memoise it across the whole process: an +/// Process-wide cache of script compile verdicts, keyed on the pair +/// (globals surface, SHA-256 hash of the script code). The verdict (clean/compiles, +/// or the name-free error text) is a pure function of the code plus the +/// compile-time-static ScriptTrustPolicy AND the globals surface it was +/// compiled against, so it is sound to memoise it across the whole process: an /// unchanged script runs the forbidden-API analysis and the Roslyn compile exactly /// once, removing both the repeat CPU cost and the repeat compiled-assembly load. /// /// +/// The globals surface is part of the key because a verdict is NOT interchangeable +/// across surfaces: a trigger expression that compiles clean against +/// TriggerCompileSurface is not vetted for a ScriptCompileSurface +/// script body (different globals resolve different identifiers), so a code-only key +/// could return a stale "clean" for code never compiled against the caller's surface. +/// Keeping the surface in the key makes that cross-surface reuse structurally +/// impossible even as more compile surfaces gain cache entries. +/// +/// +/// /// The stored error is deliberately name-free — the caller's script name is /// formatted into the returned message at read time, so two callers submitting the /// same bad code each see their own name in the failure text while sharing one verdict. @@ -40,17 +51,22 @@ public static class ScriptCompileVerdictCache public static int Count => Cache.Count; /// - /// Returns the cached verdict for , or computes it via - /// and caches it on a miss. A hit increments - /// . The verdict's error text must be name-free — the caller - /// formats the script name in on return. + /// Returns the cached verdict for the pair (, + /// ), or computes it via and + /// caches it on a miss. A hit increments . The verdict's error + /// text must be name-free — the caller formats the script name in on return. /// + /// + /// The globals-surface discriminator (e.g. nameof(ScriptCompileSurface) or + /// nameof(TriggerCompileSurface)). Part of the key so a verdict computed + /// against one surface is never reused for another. + /// /// The script source code to look up (hashed to form the cache key). /// Computes the verdict on a cache miss. /// The cached or newly computed compile verdict. - public static (bool Ok, string? Error) GetOrAdd(string code, Func<(bool Ok, string? Error)> factory) + public static (bool Ok, string? Error) GetOrAdd(string surface, string code, Func<(bool Ok, string? Error)> factory) { - var key = Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(code))); + var key = surface + ":" + Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(code))); if (Cache.TryGetValue(key, out var verdict)) { diff --git a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/ScriptCompiler.cs b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/ScriptCompiler.cs index 80246ba4..5f23f0e0 100644 --- a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/ScriptCompiler.cs +++ b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/ScriptCompiler.cs @@ -39,7 +39,7 @@ public class ScriptCompiler // globals surface are compile-time static), so memoise it process-wide: an // unchanged script compiles exactly once per process. The cached error is // name-free — the script name is formatted in below at read time. - var (ok, error) = ScriptCompileVerdictCache.GetOrAdd(code, () => + var (ok, error) = ScriptCompileVerdictCache.GetOrAdd(nameof(ScriptCompileSurface), code, () => { // Authoritative forbidden-API verdict first — a security violation must // gate the script regardless of whether it otherwise compiles. Report diff --git a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/ValidationService.cs b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/ValidationService.cs index c29b3053..1f1d5502 100644 --- a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/ValidationService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/ValidationService.cs @@ -97,11 +97,16 @@ public class ValidationService /// /// /// true (default) runs the authoritative Roslyn - /// stage — the single most expensive check (a non-collectible assembly load per script). - /// false skips ONLY that stage; every other structural/semantic check still runs. - /// Read-only staleness/comparison callers (DeploymentManager's FlatteningPipeline on its - /// comparison/probe paths) pass false because they need only the flattened config and - /// revision hash, not a compile. The deploy gate keeps the default (true). + /// stage — the single most expensive check (a non-collectible assembly load per script) — + /// AND the Expression-trigger syntax/compile check + /// (, a forbidden-API verdict plus a real compile of every + /// Expression-triggered script/alarm body against ). + /// false skips ONLY those two compile stages; every other structural/semantic check + /// still runs — including, for Expression triggers, the blank-expression check and the + /// cheap attribute-reference scan. Read-only staleness/comparison callers (DeploymentManager's + /// FlatteningPipeline on its comparison/probe paths) pass false because they need only + /// the flattened config and revision hash, not a compile. The deploy gate keeps the default + /// (true) — it is the authoritative check for both stages. /// /// A merged aggregating all pipeline stage outcomes. public ValidationResult Validate( @@ -128,7 +133,7 @@ public class ValidationService validateScriptCompilation ? ValidateScriptCompilation(configuration) : ValidationResult.Success(), ValidateAlarmTriggerReferences(configuration), ValidateScriptTriggerReferences(configuration), - ValidateExpressionTriggers(configuration), + ValidateExpressionTriggers(configuration, validateScriptCompilation), ValidateConnectionBindingCompleteness(configuration, enforceConnectionBindings, siteConnectionNames), ValidateSchemaReferences(configuration, sharedScripts, resolveSchemaRef), _semanticValidator.Validate(configuration, sharedScripts, alarmCapableConnectionNames) @@ -366,8 +371,16 @@ public class ValidationService /// /// /// The flattened configuration to validate. + /// + /// true (default) runs the authoritative + /// forbidden-API + compile stage for each Expression trigger. false skips ONLY + /// that stage — the blank-expression check and the attribute-reference scan still run — + /// so read-only staleness/comparison callers do not pay the per-expression compile. + /// /// A with errors and warnings from all expression trigger checks. - public static ValidationResult ValidateExpressionTriggers(FlattenedConfiguration configuration) + public static ValidationResult ValidateExpressionTriggers( + FlattenedConfiguration configuration, + bool validateExpressionSyntax = true) { var errors = new List(); var warnings = new List(); @@ -382,7 +395,7 @@ public class ValidationService CheckExpressionTrigger( ValidationCategory.ScriptTriggerReference, "script", script.CanonicalName, script.TriggerConfiguration, - attributeNames, errors, warnings); + attributeNames, errors, warnings, validateExpressionSyntax); } foreach (var alarm in configuration.Alarms) @@ -393,7 +406,7 @@ public class ValidationService CheckExpressionTrigger( ValidationCategory.AlarmTriggerReference, "alarm", alarm.CanonicalName, alarm.TriggerConfiguration, - attributeNames, errors, warnings); + attributeNames, errors, warnings, validateExpressionSyntax); } return new ValidationResult { Errors = errors, Warnings = warnings }; @@ -424,7 +437,8 @@ public class ValidationService string? triggerConfigJson, HashSet attributeNames, List errors, - List warnings) + List warnings, + bool validateExpressionSyntax = true) { var expression = ExtractExpressionFromTriggerConfig(triggerConfigJson); var strict = IsStrictAnalysis(triggerConfigJson); @@ -443,12 +457,20 @@ public class ValidationService return; } - var syntaxError = CheckExpressionSyntax(expression); - if (syntaxError != null) + // The syntax/compile stage is the expensive one (a forbidden-API verdict + a + // real Roslyn compile of the expression). Read-only staleness/comparison + // callers pass validateExpressionSyntax: false to skip ONLY this stage; the + // blank-expression check above and the attribute-reference scan below still run. + // The deploy gate keeps the default (true) — the authoritative check. + if (validateExpressionSyntax) { - errors.Add(ValidationEntry.Error(category, - $"The {entityLabel} '{entityName}' expression trigger failed validation: {syntaxError}", - entityName)); + var syntaxError = CheckExpressionSyntax(expression); + if (syntaxError != null) + { + errors.Add(ValidationEntry.Error(category, + $"The {entityLabel} '{entityName}' expression trigger failed validation: {syntaxError}", + entityName)); + } } foreach (var attrName in ExtractAttributeReferences(expression)) @@ -530,17 +552,27 @@ public class ValidationService /// A human-readable error message if the expression is invalid; null if well-formed. internal static string? CheckExpressionSyntax(string expression) { - // Authoritative forbidden-API verdict first. - var violations = ScriptTrustValidator.FindViolations(expression); - if (violations.Count > 0) - return $"uses forbidden API: {violations[0]}"; + // Memoise the verdict under the TRIGGER surface key (see ScriptCompileVerdictCache): + // the verdict is a pure function of expression + policy + TriggerCompileSurface, and + // it is the exact hot-path cost N1 flagged — every staleness sweep / import probe of + // an Expression-triggered config re-ran a full trust compilation + script compile. + var (_, error) = ScriptCompileVerdictCache.GetOrAdd(nameof(TriggerCompileSurface), expression, () => + { + // Authoritative forbidden-API verdict first. Report ALL violations, not + // just the first, so an operator fixing one forbidden API isn't surprised + // by a second on the next deploy attempt (mirrors ScriptCompiler.TryCompile). + var violations = ScriptTrustValidator.FindViolations(expression); + if (violations.Count > 0) + return (false, $"uses forbidden API: {string.Join("; ", violations)}"); - // Real compile of the bare boolean expression against the trigger globals. - var errors = RoslynScriptCompiler.Compile(expression, typeof(TriggerCompileSurface)); - if (errors.Count > 0) - return $"is not a valid expression: {errors[0]}"; + // Real compile of the bare boolean expression against the trigger globals. + var errors = RoslynScriptCompiler.Compile(expression, typeof(TriggerCompileSurface)); + if (errors.Count > 0) + return (false, $"is not a valid expression: {string.Join("; ", errors)}"); - return null; + return (true, (string?)null); + }); + return error; } /// diff --git a/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs b/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs index 50100d3b..8b8a880d 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs @@ -684,8 +684,9 @@ public sealed class BundleImporter : IBundleImporter // its new name, but at preview time no resolution has been chosen yet, so // the in-bundle name is the original DTO name — which is what an instance // references. (Explicit rename remap is a D-wave apply-time concern.) + var targetTemplates = await _templateRepo.GetAllTemplatesAsync(ct).ConfigureAwait(false); var targetTemplateNames = new HashSet(StringComparer.Ordinal); - foreach (var t in await _templateRepo.GetAllTemplatesAsync(ct).ConfigureAwait(false)) + foreach (var t in targetTemplates) { targetTemplateNames.Add(t.Name); } @@ -883,6 +884,22 @@ public sealed class BundleImporter : IBundleImporter } } + // N4: advisory warning rows for instance overrides that target a LOCKED + // template member — the flattener drops them, so surface the inert row in the + // wizard (Kind: Warning, non-blocking). resolutionMap is null at preview time, + // so every instance is scanned. + foreach (var (instance, message) in CollectLockedOverrideWarnings(content, resolutionMap: null, targetTemplates)) + { + blockers.Add(new ImportPreviewItem( + EntityType: "Instance", + Name: instance, + ExistingVersion: null, + IncomingVersion: null, + Kind: ConflictKind.Warning, + FieldDiffJson: null, + BlockerReason: message)); + } + return blockers; } @@ -946,8 +963,11 @@ public sealed class BundleImporter : IBundleImporter /// /// Enumerates every executable C# surface a bundle carries that the /// trust gate must vet: non-Skip template scripts + their Expression-trigger - /// bodies, template alarm Expression-trigger bodies, shared scripts, and - /// ApiMethod scripts. Expression triggers compile and execute at the site + /// bodies, template alarm Expression-trigger bodies, shared scripts, + /// ApiMethod scripts, AND instance alarm-override trigger expressions + /// (InstanceAlarmOverrideDto.TriggerConfigurationOverride). Expression + /// triggers — template alarms AND the instance overrides that replace them in + /// the flattened config — compile and execute at the site /// (`TriggerExpressionGlobals`), so they are a genuine trust surface, not just /// script bodies. A null (preview time, before /// resolutions are chosen) gates everything. Yields (kind, entityName, @@ -998,6 +1018,25 @@ public sealed class BundleImporter : IBundleImporter yield return ("ApiMethod", m.Name, m.Name, m.Script); } } + foreach (var i in content.Instances) + { + if (IsSkipResolution(resolutionMap, "Instance", i.UniqueName)) continue; + foreach (var o in i.AlarmOverrides) + { + // The DTO carries no trigger type (the type lives on the template + // alarm), so gate ANY override config carrying an {"expression":...} + // string body: if the overridden alarm is not Expression-triggered + // the body never executes, but vetting it anyway is fail-safe — the + // structured (HiLo/threshold) configs have no "expression" key, so + // there is no false-positive channel. + var expr = ExtractExpressionBody(o.TriggerConfigurationOverride); + if (!string.IsNullOrEmpty(expr)) + { + yield return ("Instance", i.UniqueName, + $"{o.AlarmCanonicalName} (alarm trigger override expression)", expr); + } + } + } } /// @@ -1009,8 +1048,24 @@ public sealed class BundleImporter : IBundleImporter private static string? ExtractTriggerExpression(string? triggerType, string? triggerConfigJson) { if (triggerType is null - || !triggerType.Equals("Expression", StringComparison.OrdinalIgnoreCase) - || string.IsNullOrWhiteSpace(triggerConfigJson)) + || !triggerType.Equals("Expression", StringComparison.OrdinalIgnoreCase)) + { + return null; + } + return ExtractExpressionBody(triggerConfigJson); + } + + /// + /// Extracts the C# boolean expression from an {"expression":"…"} trigger + /// configuration, WITHOUT a trigger-type guard — used where the caller has no + /// trigger type to check (an instance alarm override; the type lives on the + /// template alarm). A config carrying no "expression" string key (the + /// structured HiLo/threshold shapes) or malformed JSON yields null — nothing to + /// gate. + /// + private static string? ExtractExpressionBody(string? triggerConfigJson) + { + if (string.IsNullOrWhiteSpace(triggerConfigJson)) { return null; } @@ -1037,6 +1092,85 @@ public sealed class BundleImporter : IBundleImporter => resolutionMap != null && ResolveOrDefault(resolutionMap, entityType, name).Action == ResolutionAction.Skip; + /// + /// Best-effort scan for instance overrides that target a LOCKED template member. + /// The flattener drops such overrides (an override on a locked attribute / alarm / + /// native-alarm-source never takes effect), so the bundle carries an inert row — + /// this surfaces an advisory warning so the operator learns the config won't apply, + /// WITHOUT blocking the import or changing what gets written (bundle fidelity keeps + /// the row; the flattener stays the behavioural authority). + /// + /// Matches locked members by DIRECT name only — own + inherited placeholder rows + /// both carry the IsLocked flag, so a lock on either matches. Overrides + /// addressing composed path-qualified members (y1.z.Val) or derived-shadow + /// locks (LockedInDerived on a base the placeholder row doesn't reflect) may + /// not match a direct row — an unmatched name emits NO warning (never a false + /// positive; the ManagementActor and flattener remain the enforcement points). The + /// instance's template is resolved from the bundle DTO (the version about to be + /// written) first, else the pre-existing target template. + /// + /// + private static IReadOnlyList<(string Instance, string Message)> CollectLockedOverrideWarnings( + BundleContentDto content, + Dictionary<(string, string), ImportResolution>? resolutionMap, + IReadOnlyList