diff --git a/archreview/plans/PLAN-05-templates-deployment-transport.md.tasks.json b/archreview/plans/PLAN-05-templates-deployment-transport.md.tasks.json index bb754ab4..a113bd61 100644 --- a/archreview/plans/PLAN-05-templates-deployment-transport.md.tasks.json +++ b/archreview/plans/PLAN-05-templates-deployment-transport.md.tasks.json @@ -19,7 +19,7 @@ { "id": 16, "subject": "Task 16: Skip script compilation on read-only staleness/comparison paths", "status": "completed", "blockedBy": [] }, { "id": 17, "subject": "Task 17: Isolate the post-success deployment audit write", "status": "completed", "blockedBy": [16] }, { "id": 18, "subject": "Task 18: Delete NotDeployed instances without a site round-trip", "status": "completed", "blockedBy": [17] }, - { "id": 19, "subject": "Task 19: Import blocker heuristic — local-declaration exclusion, denylist additions, warning severity", "status": "pending", "blockedBy": [6] }, + { "id": 19, "subject": "Task 19: Import blocker heuristic — local-declaration exclusion, denylist additions, warning severity", "status": "completed", "blockedBy": [6] }, { "id": 20, "subject": "Task 20: Run the script trust gate on bundle import", "status": "pending", "blockedBy": [19] }, { "id": 21, "subject": "Task 21: Harden the ScriptTrustPolicy deny-list", "status": "completed", "blockedBy": [] }, { "id": 22, "subject": "Task 22: Bound LineDiffer input size", "status": "completed", "blockedBy": [] }, diff --git a/docs/requirements/Component-ScriptAnalysis.md b/docs/requirements/Component-ScriptAnalysis.md index b399a4e6..3a680046 100644 --- a/docs/requirements/Component-ScriptAnalysis.md +++ b/docs/requirements/Component-ScriptAnalysis.md @@ -2,13 +2,13 @@ ## Purpose -The Script Analysis component is the single authoritative source of truth for the ScadaBridge script trust model. It provides a unified forbidden-API deny-list, a fused semantic and syntactic trust validator, a Roslyn compile wrapper, and compile-only globals stubs used by the design-time deploy gate. All four call sites that enforce the script trust boundary — Template Engine, Site Runtime, Inbound API, and Central UI — delegate to this component rather than maintaining their own divergent implementations. +The Script Analysis component is the single authoritative source of truth for the ScadaBridge script trust model. It provides a unified forbidden-API deny-list, a fused semantic and syntactic trust validator, a Roslyn compile wrapper, and compile-only globals stubs used by the design-time deploy gate. All five call sites that enforce the script trust boundary — Template Engine, Site Runtime, Inbound API, Central UI, and Transport bundle import — delegate to this component rather than maintaining their own divergent implementations. ## Location `src/ZB.MOM.WW.ScadaBridge.ScriptAnalysis/` -Referenced by: Template Engine, Site Runtime, Inbound API, Central UI. +Referenced by: Template Engine, Site Runtime, Inbound API, Central UI, Transport. ## Responsibilities @@ -161,7 +161,7 @@ There is a **third** hand-maintained mirror of the runtime globals: the Central ### REQ-SA-5: Consumer Delegation -All four call sites that previously maintained their own script trust enforcement now delegate to this component. The key behavioral changes per consumer: +All five call sites that previously maintained their own script trust enforcement (or, for Transport, had none) now delegate to this component. The key behavioral changes per consumer: | Consumer | Before | After | |----------|--------|-------| @@ -170,6 +170,7 @@ All four 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 | 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. @@ -189,3 +190,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). diff --git a/docs/requirements/Component-Transport.md b/docs/requirements/Component-Transport.md index 6f9080a5..a5bd674f 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 two-tier semantic validation before committing: a minimal name-resolution scan over the merged target (fails fast on unresolved SharedScript / ExternalSystem identifiers), 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 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`. - 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). diff --git a/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs b/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs index 0ba96f57..bf134164 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs @@ -23,6 +23,7 @@ using ZB.MOM.WW.ScadaBridge.Commons.Types.Transport; using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums; using ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening; using ZB.MOM.WW.ScadaBridge.ConfigurationDatabase; +using ZB.MOM.WW.ScadaBridge.ScriptAnalysis; using ZB.MOM.WW.ScadaBridge.TemplateEngine; using ZB.MOM.WW.ScadaBridge.TemplateEngine.Validation; using ZB.MOM.WW.ScadaBridge.Transport.Encryption; @@ -846,6 +847,26 @@ public sealed class BundleImporter : IBundleImporter : $"Script references SharedScript or ExternalSystem '{candidate}' not present in bundle or target — advisory; the deploy-time gate re-validates.")); } + // #05-T20 — script trust gate (preview parity with the apply-time gate). + // A forbidden-API verdict is authoritative → a hard Blocker for every + // script kind. No resolution map at preview time, so all scripts are + // vetted. Blocker Name is entity-qualified so multiple offenders surface + // as distinct rows. + foreach (var (kind, entityName, scriptLabel, code) in EnumerateTrustGatedScripts(content, resolutionMap: null)) + { + foreach (var violation in ScriptTrustValidator.FindViolations(code)) + { + blockers.Add(new ImportPreviewItem( + EntityType: kind, + Name: $"{entityName}.{scriptLabel}", + ExistingVersion: null, + IncomingVersion: null, + Kind: ConflictKind.Blocker, + FieldDiffJson: null, + BlockerReason: $"Script trust violation — {violation}")); + } + } + return blockers; } @@ -906,6 +927,51 @@ public sealed class BundleImporter : IBundleImporter } } + /// + /// #05-T20 — enumerates every executable script body the trust gate must + /// vet: non-Skip template scripts, shared scripts, and ApiMethod scripts. + /// A null (preview time, before resolutions + /// are chosen) gates everything. Yields (kind, entityName, scriptLabel, + /// code) so callers can format either an error string or a preview row. + /// + private static IEnumerable<(string Kind, string EntityName, string ScriptLabel, string Code)> EnumerateTrustGatedScripts( + BundleContentDto content, + Dictionary<(string, string), ImportResolution>? resolutionMap) + { + foreach (var t in content.Templates) + { + if (IsSkipResolution(resolutionMap, "Template", t.Name)) continue; + foreach (var s in t.Scripts) + { + if (!string.IsNullOrEmpty(s.Code)) + { + yield return ("Template", t.Name, s.Name, s.Code); + } + } + } + foreach (var s in content.SharedScripts) + { + if (IsSkipResolution(resolutionMap, "SharedScript", s.Name)) continue; + if (!string.IsNullOrEmpty(s.Code)) + { + yield return ("SharedScript", s.Name, s.Name, s.Code); + } + } + foreach (var m in content.ApiMethods) + { + if (IsSkipResolution(resolutionMap, "ApiMethod", m.Name)) continue; + if (!string.IsNullOrEmpty(m.Script)) + { + yield return ("ApiMethod", m.Name, m.Name, m.Script); + } + } + } + + private static bool IsSkipResolution( + Dictionary<(string, string), ImportResolution>? resolutionMap, string entityType, string name) + => resolutionMap != null + && ResolveOrDefault(resolutionMap, entityType, name).Action == ResolutionAction.Skip; + /// /// Names that look like PascalCase references but are never user-defined /// SharedScripts or ExternalSystems. Filters the false-positive noise the @@ -4176,6 +4242,22 @@ public sealed class BundleImporter : IBundleImporter var errors = new List(); var warnings = new List(); + // ---- Pass 0: script trust gate ---- + // Bundle import is the fifth script-trust call site. Every executable + // script body (template, shared, ApiMethod) is run through the shared + // ScriptTrustValidator BEFORE name resolution — a forbidden-API verdict + // is authoritative (not the false-positive-prone name heuristic), so it + // is a HARD error for all kinds, and it must not be masked by a Pass-1 + // name-resolution error. Task 15's verdict cache keeps repeat cost nil. + foreach (var (kind, entityName, scriptLabel, code) in EnumerateTrustGatedScripts(content, resolutionMap)) + { + foreach (var violation in ScriptTrustValidator.FindViolations(code)) + { + errors.Add($"{kind} '{entityName}' script '{scriptLabel}': {violation}"); + } + } + if (errors.Count > 0) return (errors, warnings); + // ---- Pass 1: minimal name-resolution scan ---- // Build the known-resolvable set. For in-bundle entries, EXCLUDE the diff --git a/src/ZB.MOM.WW.ScadaBridge.Transport/ZB.MOM.WW.ScadaBridge.Transport.csproj b/src/ZB.MOM.WW.ScadaBridge.Transport/ZB.MOM.WW.ScadaBridge.Transport.csproj index 5ce58814..32463ec5 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Transport/ZB.MOM.WW.ScadaBridge.Transport.csproj +++ b/src/ZB.MOM.WW.ScadaBridge.Transport/ZB.MOM.WW.ScadaBridge.Transport.csproj @@ -16,6 +16,7 @@ + diff --git a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/BundleImporterPreviewTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/BundleImporterPreviewTests.cs index 23a795cf..0630be71 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/BundleImporterPreviewTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/BundleImporterPreviewTests.cs @@ -426,6 +426,38 @@ public sealed class BundleImporterPreviewTests : IDisposable && i.BlockerReason.Contains("MissingHelper", StringComparison.Ordinal)); } + [Fact] + public async Task PreviewAsync_emits_Blocker_when_template_script_has_forbidden_api() + { + // #05-T20 — the script trust gate runs in preview too, so a forbidden-API + // script surfaces as a Blocker row (parity with the apply-time + // SemanticValidationException). Trust violations block regardless of kind. + await using (var scope = _provider.CreateAsyncScope()) + { + var ctx = scope.ServiceProvider.GetRequiredService(); + var t = new Template("Malicious") { Description = "forbidden" }; + t.Scripts.Add(new TemplateScript("init", "System.Diagnostics.Process.Start(\"cmd\");")); + ctx.Templates.Add(t); + await ctx.SaveChangesAsync(); + } + + var bundleStream = await ExportTemplatesAsync(); + var bytes = await StreamToBytes(bundleStream); + + ImportPreview preview; + await using (var scope = _provider.CreateAsyncScope()) + { + var importer = scope.ServiceProvider.GetRequiredService(); + var session = await importer.LoadAsync(new MemoryStream(bytes), passphrase: null); + preview = await importer.PreviewAsync(session.SessionId); + } + + Assert.Contains(preview.Items, i => + i.Kind == ConflictKind.Blocker + && i.BlockerReason is not null + && i.BlockerReason.Contains("Process", StringComparison.Ordinal)); + } + [Fact] public async Task PreviewAsync_does_not_flag_opcua_tag_paths_in_DataSourceReference_as_blockers() { diff --git a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/SemanticValidatorImportTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/SemanticValidatorImportTests.cs index b4a5c05c..b3502076 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/SemanticValidatorImportTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/SemanticValidatorImportTests.cs @@ -292,6 +292,81 @@ public sealed class SemanticValidatorImportTests : IDisposable } } + [Fact] + public async Task Apply_TemplateScriptWithForbiddenApi_HardBlocks() + { + // #05-T20 — bundle import is the fifth script-trust call site. A template + // script that reaches a forbidden API (Process.Start) must be rejected at + // import review, NOT left to fail at runtime. Unlike the name-resolution + // heuristic, a trust verdict is authoritative → hard error for ALL kinds. + await using (var scope = _provider.CreateAsyncScope()) + { + var ctx = scope.ServiceProvider.GetRequiredService(); + var t = new Template("Malicious"); + t.Scripts.Add(new Commons.Entities.Templates.TemplateScript( + "init", + "System.Diagnostics.Process.Start(\"cmd\");")); + ctx.Templates.Add(t); + await ctx.SaveChangesAsync(); + } + + var sessionId = await ExportWipeAndLoadAsync(); + + SemanticValidationException ex = default!; + await using (var scope = _provider.CreateAsyncScope()) + { + var importer = scope.ServiceProvider.GetRequiredService(); + ex = await Assert.ThrowsAsync(() => + importer.ApplyAsync(sessionId, + new List + { + new("Template", "Malicious", ResolutionAction.Add, null), + }, + user: "bob")); + } + + Assert.NotEmpty(ex.Errors); + Assert.Contains(ex.Errors, err => err.Contains("Process", StringComparison.Ordinal)); + + await using (var scope = _provider.CreateAsyncScope()) + { + var ctx = scope.ServiceProvider.GetRequiredService(); + Assert.False(await ctx.Templates.AnyAsync(t => t.Name == "Malicious")); + } + } + + [Fact] + public async Task Apply_ApiMethodWithForbiddenApi_HardBlocks() + { + // #05-T20 — same trust gate over ApiMethod script bodies. + await using (var scope = _provider.CreateAsyncScope()) + { + var ctx = scope.ServiceProvider.GetRequiredService(); + ctx.ApiMethods.Add(new Commons.Entities.InboundApi.ApiMethod( + "Ingest", + "System.Diagnostics.Process.Start(\"cmd\"); return 1;")); + await ctx.SaveChangesAsync(); + } + + var sessionId = await ExportWipeAndLoadAsync(); + + SemanticValidationException ex = default!; + await using (var scope = _provider.CreateAsyncScope()) + { + var importer = scope.ServiceProvider.GetRequiredService(); + ex = await Assert.ThrowsAsync(() => + importer.ApplyAsync(sessionId, + new List + { + new("ApiMethod", "Ingest", ResolutionAction.Add, null), + }, + user: "bob")); + } + + Assert.NotEmpty(ex.Errors); + Assert.Contains(ex.Errors, err => err.Contains("Process", StringComparison.Ordinal)); + } + [Fact] public async Task SemanticValidator_catches_alarm_trigger_type_mismatch_at_import() {