Files
ScadaBridge/docs/plans/2026-06-16-script-analysis-consolidation.md
T

90 lines
11 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Shared Script-Analysis Consolidation (M3) — Implementation Plan
> **For Claude:** executed via superpowers-extended-cc:subagent-driven-development in this session.
**Goal:** Extract one shared, authoritative script-trust analyzer (`ZB.MOM.WW.ScadaBridge.ScriptAnalysis`) and make all four call sites delegate to it, turning the fake design-time deploy gate into a real semantic compile + authoritative forbidden-API check.
**Architecture:** New project just above Commons (refs Commons + Microsoft.CodeAnalysis.CSharp.Scripting/Workspaces). Hosts the trust policy, the fused semantic+syntactic validator, a Roslyn compile wrapper, and compile-only globals stubs. TemplateEngine/SiteRuntime/InboundAPI/CentralUI delegate; the executing globals stay with their owners. Design: `docs/plans/2026-06-16-script-analysis-consolidation-design.md`.
**Tech Stack:** C#/.NET 10, Microsoft.CodeAnalysis (Roslyn) C# Scripting + Workspaces, xUnit.
**Conventions:** targeted builds/tests per task (`dotnet build <project>`, `dotnet test --filter`); full-solution build only in M3.6. Implementers do NOT create worktrees (already in one) and commit with pathspec form `git commit -m "..." -- <paths>` (retry on index.lock).
---
# Wave 0 — Spike
### Task M3.0: Confirm compile-surface stub feasibility + package refs
**Classification:** small · **Estimated implement time:** ~4 min · **Parallelizable with:** none
**Files (read-only spike):** `src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptCompilationService.cs` (ScriptGlobals), `TriggerExpressionGlobals.cs`, `src/ZB.MOM.WW.ScadaBridge.CentralUI/ScriptAnalysis/SandboxScriptHost.cs`, `Directory.Packages.props`.
**Steps:**
1. Enumerate the full public member surface of `ScriptGlobals` and `TriggerExpressionGlobals` (names + rough shapes) that a script can reference.
2. Confirm every member's referenced types resolve from Commons alone (e.g. `ScriptParameters`, `AlarmContext`, `ScriptScope` in `Commons.Types(.Scripts)`); list any member whose type lives in SiteRuntime/CentralUI and would need a loose (`object`/`dynamic`-returning) stand-in on the stub.
3. Confirm `Microsoft.CodeAnalysis.CSharp.Scripting` (5.0.0) + `Microsoft.CodeAnalysis.CSharp.Workspaces` (5.0.0) are in `Directory.Packages.props`.
**AC:** a short written inventory the M3.1 implementer can build `ScriptCompileSurface`/`TriggerCompileSurface` from, with each member's compile-surface return type decided (Commons type vs loose `object`). No code committed.
---
# Wave 1 — Foundation
### Task M3.1: Build `ZB.MOM.WW.ScadaBridge.ScriptAnalysis` (policy + validator + compiler + surfaces + tests)
**Classification:** high-risk (security boundary) · **Estimated implement time:** ~5 min · **Parallelizable with:** none · **Depends on:** M3.0
**Files:**
- Create: `src/ZB.MOM.WW.ScadaBridge.ScriptAnalysis/ZB.MOM.WW.ScadaBridge.ScriptAnalysis.csproj` (refs Commons + the two CodeAnalysis packages; add to `ZB.MOM.WW.ScadaBridge.slnx`).
- Create: `ScriptTrustPolicy.cs`, `ScriptTrustValidator.cs`, `RoslynScriptCompiler.cs`, `ScriptCompileSurface.cs`, `TriggerCompileSurface.cs`.
- Create: `tests/ZB.MOM.WW.ScadaBridge.ScriptAnalysis.Tests/...` (+ add to slnx).
**Design points:**
- `ScriptTrustPolicy`: ForbiddenScopes = `System.IO`, `System.Diagnostics.Process`, `System.Threading`, `System.Reflection`, `System.Net`, `System.Runtime.InteropServices`, `Microsoft.Win32`. AllowedExceptions = `System.Threading.Tasks`, `System.Threading.CancellationToken`, `System.Threading.CancellationTokenSource`. ReflectionGatewayMembers = the InboundAPI `ForbiddenMemberNames` set. ForbiddenIdentifiers = `dynamic`, `Activator`. DefaultReferences = the SiteRuntime `ScriptAssemblies` set (object, Enumerable, Math, CSharpArgumentInfo, Commons). DefaultImports = `System`, `System.Collections.Generic`, `System.Linq`, `System.Threading.Tasks`.
- `ScriptTrustValidator.FindViolations(code, extraReferences?)`: port SiteRuntime `ValidateTrustModel` (CreateScriptCompilation + semantic model; `GetSymbolScopes`/`GetSyntacticScopes`/`IsUnderScope` symbol-vs-syntax resolution; dedup via `SortedSet`), and FUSE in InboundAPI's reflection-gateway member-name + `dynamic`/`Activator` syntactic checks. Returns combined violations.
- `RoslynScriptCompiler.Compile(code, globalsType?, extraReferences?, extraImports?)`: `CSharpScript.Create<object?>(code, options, globalsType).Compile()`, map error diagnostics to messages. `ParseDiagnostics(code)`: parse with `SourceCodeKind.Script`, return syntax error diagnostics.
- `ScriptCompileSurface`/`TriggerCompileSurface`: mirror the M3.0 inventory; compile-only, never executed.
**Tests (TDD):** adversarial corpus → violations (alias, `using static`, `global::`, `typeof(x).Assembly.GetType("System.IO.File")`, `dynamic`, `Activator`, `System.Runtime.InteropServices`, `Microsoft.Win32`); legit → clean (`await Task.Delay`, `Stopwatch`, LINQ, `Math`, `CancellationToken`); `ParseDiagnostics` flags syntax error; `Compile(..., typeof(ScriptCompileSurface))` rejects undefined symbol AND accepts a real script using `Attributes`/`Notify`/`ExternalSystem`; parity test: public member names of `ScriptCompileSurface` ⊇ those of … (parity asserted in M3.3 once SiteRuntime references the project — here assert the surface compiles a representative real script).
**AC:** `dotnet build` + `dotnet test` for ScriptAnalysis green; adversarial corpus all rejected; legit all clean.
---
# Wave 2 — Consumers delegate (parallelizable; disjoint projects)
### Task M3.2: TemplateEngine deploy gate → shared analyzer
**Classification:** high-risk (the authoritative deploy gate) · **~5 min** · **Parallelizable with:** M3.3, M3.4, M3.5 · **Depends on:** M3.1
**Files:** `src/ZB.MOM.WW.ScadaBridge.TemplateEngine/ZB.MOM.WW.ScadaBridge.TemplateEngine.csproj` (+ ProjectReference to ScriptAnalysis), `Validation/ScriptCompiler.cs`, `Validation/ValidationService.cs` (`CheckExpressionSyntax`), `tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Validation/ScriptCompilerTests.cs`.
**Changes:** `ScriptCompiler.TryCompile` → empty-check, then `ScriptTrustValidator.FindViolations`, then `RoslynScriptCompiler.Compile(code, typeof(ScriptCompileSurface))`; return first failure as `Result<bool>.Failure`. `CheckExpressionSyntax``FindViolations` + `Compile(expr, typeof(TriggerCompileSurface))`. Delete `ForbiddenPatterns` + the substring/brace path (keep `CSharpDelimiterScanner` only if still referenced elsewhere — else remove). Re-point/adjust `ScriptCompilerTests` expected messages.
**AC:** invalid C# (undefined symbol) fails validation; adversarial alias/`using static`/`global::`/reflection-gateway scripts fail; valid real scripts pass. `dotnet test` TemplateEngine green.
### Task M3.3: SiteRuntime → shared analyzer (keep real-globals compile)
**Classification:** high-risk (runtime trust gate) · **~4 min** · **Parallelizable with:** M3.2, M3.4, M3.5 · **Depends on:** M3.1
**Files:** `src/ZB.MOM.WW.ScadaBridge.SiteRuntime/ZB.MOM.WW.ScadaBridge.SiteRuntime.csproj` (+ ref), `Scripts/ScriptCompilationService.cs`, `tests/.../Scripts/TrustModelSemanticTests.cs`, `ScriptCompilationServiceTests.cs`.
**Changes:** `ValidateTrustModel` → delegate to `ScriptTrustValidator.FindViolations`. Keep `CompileCore`/`CSharpScript.Compile` against the real `ScriptGlobals`/`TriggerExpressionGlobals`. Remove the now-duplicated `ForbiddenNamespaces`/`AllowedExceptions`/scope helpers. **Add the parity test** (`ScriptCompileSurface` public member-name set ⊇ `ScriptGlobals`; `TriggerCompileSurface``TriggerExpressionGlobals`) here, where both types are referenceable.
**AC:** SiteRuntime trust tests green; parity test green; reflection-gateway bypass now rejected at the site too.
### Task M3.4: InboundAPI → shared analyzer
**Classification:** standard · **~3 min** · **Parallelizable with:** M3.2, M3.3, M3.5 · **Depends on:** M3.1
**Files:** `src/ZB.MOM.WW.ScadaBridge.InboundAPI/ZB.MOM.WW.ScadaBridge.InboundAPI.csproj` (+ ref to ScriptAnalysis), `ForbiddenApiChecker.cs`, `InboundScriptExecutor.cs` (call site if needed), `tests/.../ForbiddenApiCheckerTests.cs`.
**Changes:** `ForbiddenApiChecker.FindViolations` → delegate to `ScriptTrustValidator.FindViolations` (keep the public method as a thin shim so callers are untouched). Remove the local walker/lists. Adjust test expectations where wording changed; the now-loosened `System.Diagnostics` (Stopwatch allowed) and now-semantic resolution should be reflected.
**AC:** InboundAPI forbidden-API tests green against the shared validator; reflection-gateway + `dynamic` + `Activator` still rejected.
### Task M3.5: CentralUI → shared analyzer (keep editor markers + Test-Run host)
**Classification:** standard · **~4 min** · **Parallelizable with:** M3.2, M3.3, M3.4 · **Depends on:** M3.1
**Files:** `src/ZB.MOM.WW.ScadaBridge.CentralUI/ZB.MOM.WW.ScadaBridge.CentralUI.csproj` (+ ref), `ScriptAnalysis/ScriptAnalysisService.cs`, `tests/.../ScriptAnalysis/ScriptAnalysisServiceTests.cs`.
**Changes:** `FindForbiddenApiUsages`/`ValidateTrustModel` → delegate to `ScriptTrustValidator.FindViolations` for the trust verdict, then map to `DiagnosticMarker`s (keep marker positions/codes). Remove the local `ForbiddenNamespacePrefixes`/`ForbiddenNameFor`/`IsForbiddenName`. Keep `Diagnose` compile + `SandboxScriptHost` Test-Run execution unchanged. Adjust tests where the (now stricter) threading policy changes a marker.
**AC:** CentralUI script-analysis tests green; editor now flags the previously-allowed `System.Threading` usages.
---
# Wave 3 — Integration + docs
### Task M3.6: Integration verification + docs + fixture cleanup
**Classification:** high-risk (final integration reviewer) · **~5 min** · **Parallelizable with:** none · **Depends on:** M3.2M3.5
**Steps:**
1. `dotnet build ZB.MOM.WW.ScadaBridge.slnx` (full solution).
2. Targeted tests across ScriptAnalysis, TemplateEngine, SiteRuntime, InboundAPI, CentralUI; fix any latent-invalid fixtures/scripts the real compile surfaces.
3. Docs: `docs/requirements/Component-TemplateEngine.md` (real compile gate), `Component-SiteRuntime.md`, `Component-InboundAPI.md`, `Component-Commons.md` if surfaces move; add the new component to `README.md` table + `CLAUDE.md` component list (24 → 25); remove the "SECURITY LIMITATION / advisory" notes in `ScriptCompiler.cs`/`ValidationService.cs` now that the gate is authoritative.
4. Commit; final integration code review of the whole `8e99f22..HEAD` diff.
**AC:** full build green; trust tests + adversarial deploy test green; no doc claims the design-time gate is advisory; component list updated.
---
## Native tasks & dependencies
Sub-tasks created as native tasks under umbrella #14 (M3). Edges: M3.1 ⟵ M3.0; M3.2/M3.3/M3.4/M3.5 ⟵ M3.1; M3.6 ⟵ M3.2..M3.5.