From d412fc36966556fa2afc85459202c398e3c60ce1 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 12 Aug 2026 16:44:44 -0400 Subject: [PATCH] docs(scripts): record the shared caching metadata resolver invariant --- CLAUDE.md | 2 +- docs/requirements/Component-ScriptAnalysis.md | 28 +++++++++++++++++++ docs/requirements/Component-SiteRuntime.md | 2 ++ 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index d43c2789..da8b3dd8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -207,7 +207,7 @@ spec for each is `docs/requirements/Component-.md`, and `README.md` carrie ### Akka.NET Conventions - Tell for hot-path internal communication; Ask reserved for system boundaries. - Cross-cluster communication is gRPC (per-site PSK-authenticated): site→central `CentralControlService`, central→site `SiteCommandService`, plus the `SiteStreamService` data stream. ClusterClient/ClusterClientReceptionist were removed in the migration's Phase 4 — service discovery is by dialling configured endpoints, not the receptionist. (Akka.Cluster.Tools remains for ClusterSingleton.) -- Script trust model: forbidden APIs (System.IO, Process, Threading, Reflection, raw network). The trust boundary is centralized in the Script Analysis component (#25) — `ScriptTrustPolicy` is the single source of truth; all four call sites (Template Engine, Site Runtime, Inbound API, Central UI) delegate to `ScriptTrustValidator`. The design-time deploy gate in Template Engine is authoritative (real semantic compile), not advisory. +- Script trust model: forbidden APIs (System.IO, Process, Threading, Reflection, raw network). The trust boundary is centralized in the Script Analysis component (#25) — `ScriptTrustPolicy` is the single source of truth; all four call sites (Template Engine, Site Runtime, Inbound API, Central UI) delegate to `ScriptTrustValidator`. The design-time deploy gate in Template Engine is authoritative (real semantic compile), not advisory. Every Roslyn script-compile surface must also attach the shared `CachingScriptMetadataResolver.Instance` (ScriptAnalysis) via `ScriptOptions.WithMetadataResolver` — the four today are Site Runtime `ScriptCompilationService`, Inbound API `InboundScriptExecutor`, Central UI `ScriptAnalysisService`, and ScriptAnalysis `RoslynScriptCompiler`; a new surface that omits it silently reintroduces a per-compile native metadata leak (see Component-ScriptAnalysis.md). - Application-level correlation IDs on all request/response messages. ## Tool Usage diff --git a/docs/requirements/Component-ScriptAnalysis.md b/docs/requirements/Component-ScriptAnalysis.md index f39a4e12..86319200 100644 --- a/docs/requirements/Component-ScriptAnalysis.md +++ b/docs/requirements/Component-ScriptAnalysis.md @@ -16,6 +16,7 @@ Referenced by: Template Engine, Site Runtime, Inbound API, Central UI, Transport - Provide an authoritative forbidden-API verdict (`ScriptTrustValidator.FindViolations`) that fuses semantic symbol resolution with syntactic reflection-gateway hardening. - Wrap Roslyn `CSharpScript` compilation (`RoslynScriptCompiler`) so callers share one implementation of compile + diagnostics extraction. - Provide compile-only globals stubs (`ScriptCompileSurface`, `TriggerCompileSurface`) that mirror the real execution-time globals member-for-member, allowing the design-time deploy gate to do a real type-checking compile without depending on the execution-time projects. +- Own the shared process-wide metadata reference resolver (`CachingScriptMetadataResolver`) that every Roslyn script-compile surface in the system attaches to its `ScriptOptions`. --- @@ -182,6 +183,32 @@ The static enforcement is **defence-in-depth**, not a true runtime sandbox. Scri --- +### REQ-SA-6: Shared Metadata Reference Resolver (`CachingScriptMetadataResolver`) + +`CachingScriptMetadataResolver` is a process-wide memoizing decorator over Roslyn's default script metadata resolver (`ScriptOptions.Default.MetadataResolver`), owned by this component because it is a property of *script compilation*, not of any one consumer. `CachingScriptMetadataResolver.Instance` is the single shared instance — the cache lives on the instance, so a second instance is a second (empty) cache. + +#### The invariant + +**Every Roslyn script-compile surface in the system MUST attach the shared instance** via `ScriptOptions.WithMetadataResolver(CachingScriptMetadataResolver.Instance)`. There are four such surfaces today: Site Runtime `ScriptCompilationService`, Inbound API `InboundScriptExecutor`, Central UI `ScriptAnalysisService`, and this component's own `RoslynScriptCompiler`. A surface that omits it — including a new fifth surface — reintroduces a per-compile native-memory leak, and does so silently. + +#### Why + +The `ScriptOptions` on each surface carry only the direct API-surface references; every `script.Compile()` binds their **transitive closure**, and each transitively-referenced assembly is resolved through the options' `MetadataReferenceResolver`. Roslyn's default resolver has **no cross-compilation cache**: each resolution calls `MetadataReference.CreateFromFile`, which eagerly copies the whole assembly into native memory (`AssemblyMetadata` → `PEReader` → `NativeHeapMemoryBlock`). So a 5-assembly explicit set re-resolves its ~74-assembly closure afresh on *every* compile, and those references are pinned for the process lifetime by the compile caches that hold the compiled scripts. Confirmed live (2026-08-12 gcdumps): 21 site scripts held 6,640 metadata objects, 6 central inbound methods 2,699. With the decorator, each distinct assembly is materialized **once per process** regardless of compile count. + +This is the second and dominant half of the leak whose first half — per-compile `ScriptOptions` construction — was fixed by making the options static (see Site Runtime). + +#### Trust-model neutrality + +The decorator changes **no resolution result**, only object identity: it returns exactly what the inner (undecorated) resolver returned for the same arguments, so it can never resolve anything the undecorated options would not have resolved. `DefaultReferences` stays minimal and the compile gate keeps seeing exactly what it saw before — this fix deliberately does not widen any explicit reference set, precisely to avoid semantic drift in a security gate. + +#### Cache-correctness assumptions + +- `ResolveMissingAssembly` keys on `AssemblyIdentity.GetDisplayName()` (case-insensitive, matching .NET simple-name binding) and deliberately ignores the requesting `definition`, whose directory is a search path in the inner resolver. Safe here: every node runs from a single publish directory plus the shared framework, so identity → path is stable process-wide. A `null` result is cached too — "not found" is a legitimate, sticky answer. +- `ResolveReference` keys on the full `(reference, baseFilePath, properties)` tuple, case-**sensitively**, because `baseFilePath` is a filesystem path on the Linux containers these nodes run in. +- A `GetOrAdd` factory race can mint one duplicate — bounded and benign. Entries are never disposed; the cache is bounded by the distinct assemblies on disk, the same order as the static `AnalysisReferences`. + +--- + ## Dependencies - **Commons**: Shared types referenced by `ScriptCompileSurface` and `TriggerCompileSurface` (e.g., `DataType`, attribute access types). @@ -196,4 +223,5 @@ 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`). +- **All script-compile surfaces**: Site Runtime `ScriptCompilationService`, Inbound API `InboundScriptExecutor`, Central UI `ScriptAnalysisService`, and this component's own `RoslynScriptCompiler` each attach `CachingScriptMetadataResolver.Instance` to their `ScriptOptions` (REQ-SA-6). - **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-SiteRuntime.md b/docs/requirements/Component-SiteRuntime.md index 125a24a9..3083a9e6 100644 --- a/docs/requirements/Component-SiteRuntime.md +++ b/docs/requirements/Component-SiteRuntime.md @@ -100,6 +100,8 @@ flowchart TD > **The Roslyn `ScriptOptions` are process-static, and must stay that way**: the reference set backing every site-side compile is built once per process, never per compile. `ScriptOptions.WithReferences(Assembly[])` resolves each assembly through `MetadataReference.CreateFromFile`, which does **not** cache — each call mints a fresh `MetadataReference` owning an `AssemblyMetadata` → `PEReader` → `NativeHeapMemoryBlock`, an unmanaged copy of the assembly metadata that nothing disposes. Building the options per compile leaks native memory permanently: no GC reclaims it, and it is invisible to gcdump and to the managed allocation counters, so the node's working set grows without the GC heap growing. This was a live defect (a Site node at 2,885 MB working set with 150 MB of live GC heap, ~2,469 MB of it on the default process heap across ~6,700 undisposed `AssemblyMetadata` instances). Note this is orthogonal to the compile cache above — the cache dedupes *identical* script bodies, so it bounds nothing when the bodies differ, and it clears wholesale on overflow, after which every script recompiles. Pinned by a reference-equality regression test rather than by watching memory. +> **Static options were only half the fix — the compile must also use the shared caching metadata resolver**: the static `ScriptOptions` carry only the *direct* API-surface references (5 assemblies here), and each `script.Compile()` still binds their **transitive closure** (~105 assemblies on a site node). Every transitively-referenced assembly is resolved through the options' `MetadataReferenceResolver`, and Roslyn's default one has no cross-compilation cache — it calls `MetadataReference.CreateFromFile` afresh on **every compile**, minting the same undisposed `AssemblyMetadata` → `PEReader` → `NativeHeapMemoryBlock` triple per assembly per compiled script, pinned for the process lifetime by the compile cache. That per-compile *re-resolution* — not the per-compile options construction — was the dominant term (measured live: 21 site scripts holding 6,640 metadata objects, counts bit-identical before and after the static-options fix). `ScriptCompilationService` therefore also attaches the process-wide `CachingScriptMetadataResolver.Instance` (owned by the Script Analysis component, see `Component-ScriptAnalysis.md`) via `ScriptOptions.WithMetadataResolver`, so each distinct assembly is materialized once per process. The decorator returns exactly what the undecorated resolver returned — only object identity is de-duplicated — so compile results and the trust gate are unchanged. + ### Deployment Handling - Receives flattened instance configurations from central via the Communication Layer. - Stores the new configuration in local SQLite.