docs(scripts): record the shared caching metadata resolver invariant

This commit is contained in:
Joseph Doherty
2026-08-12 16:44:44 -04:00
parent 702de910ad
commit d412fc3696
3 changed files with 31 additions and 1 deletions
@@ -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).