docs(m3): implementation plan + tasks for shared ScriptAnalysis consolidation
This commit is contained in:
@@ -0,0 +1,89 @@
|
||||
# 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.2–M3.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.
|
||||
@@ -0,0 +1,13 @@
|
||||
{
|
||||
"planPath": "docs/plans/2026-06-16-script-analysis-consolidation.md",
|
||||
"tasks": [
|
||||
{"id": 103, "subject": "M3.0: Spike — compile-surface stub feasibility + package refs", "status": "pending"},
|
||||
{"id": 104, "subject": "M3.1: Build ScriptAnalysis project (policy+validator+compiler+surfaces+tests)", "status": "pending", "blockedBy": [103]},
|
||||
{"id": 105, "subject": "M3.2: TemplateEngine deploy gate → shared analyzer", "status": "pending", "blockedBy": [104]},
|
||||
{"id": 106, "subject": "M3.3: SiteRuntime → shared analyzer + parity test", "status": "pending", "blockedBy": [104]},
|
||||
{"id": 107, "subject": "M3.4: InboundAPI → shared analyzer", "status": "pending", "blockedBy": [104]},
|
||||
{"id": 108, "subject": "M3.5: CentralUI → shared analyzer (keep markers + Test-Run host)", "status": "pending", "blockedBy": [104]},
|
||||
{"id": 109, "subject": "M3.6: Integration verification + docs + fixture cleanup", "status": "pending", "blockedBy": [105, 106, 107, 108]}
|
||||
],
|
||||
"lastUpdated": "2026-06-16"
|
||||
}
|
||||
Reference in New Issue
Block a user