Review remediation: Lane E — Scripting runtime safety + scripted alarms (fable for E1/E4, opus for E2/E3; review fable) #734

New Issue

2026-10-22T06:05:18-04:00

dohertj2 commented

2026-10-22 06:05:18 -04:00

From the 2026-10-21 Fable system review. Findings R0x:ID refer to docs/reviews/2026-10-21/0x-*.md (location, evidence, failure scenario, fix recipe). Plan: docs/superpowers/plans/2026-10-22-review-remediation.md.

Lane E — Scripting runtime safety + scripted alarms (fable for E1/E4, opus for E2/E3; review fable)

Owns:

src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/**, Core.ScriptedAlarms/**, Core.AlarmHistorian/**
Runtime/ScriptedAlarms/**, Runtime/VirtualTags/**
Host/Engines/**
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation/**
Configuration/Validation/DraftValidator.cs

Tests: Core.Scripting.Tests, Core.ScriptedAlarms.Tests, Core.AlarmHistorian.Tests, Driver.Calculation.Tests, Runtime.Tests (VirtualTags/ScriptedAlarms folders)

E1 Script quarantine (R03:1.1, R05:2, R07:L1, R07:P5):
- A shared ScriptRunGuard in Core.Scripting, keyed per script instance.
- It refuses to start a run while that script's previous orphan is still executing. The result is Bad "previous evaluation still running" plus a counter.
- It quarantines after 3 consecutive timeouts: stop evaluating, publish Bad, log Error, metric otopcua_script_quarantined, cleared on redeploy or source change.
- Pass a CancellationToken into scripts.
- Make the Calculation pump async, so pool starvation is no longer charged to scripts.
- Wired into the scripted-alarm engine, RoslynVirtualTagEvaluator/VirtualTagHostActor, and CalculationEvaluator.
- Tests: a while(true) script on a 1 Hz trigger leaves ≤ 1 orphaned thread after 30 triggers (ThreadPool.ThreadCount delta). Positive control: a healthy sibling keeps evaluating.
E2 Sandbox hardening (R05:10, R05:11):
- Deny System.Threading with an allow-list (CancellationToken, Interlocked, Volatile).
- Deny System.Type.GetType/GetMethod*/InvokeMember and System.Runtime.CompilerServices.RuntimeHelpers.
- Add an allocation budget: sample GC.GetAllocatedBytesForCurrentThread around each run and quarantine on breach (Scripting:MaxAllocBytesPerRun, default 64 MB). Document that a pre-emptive OOM still needs an out-of-process runner, and file a follow-up issue for that.
- Tests: each forbidden form gets an OTSCRIPT diagnostic, and the Monaco editor parity test still matches.
E3 Scripted-alarm engine (R05:1, 3, 6, 7, 9, 12):
- Clear _valueCache in LoadAsync; move its write below the _disposed guard.
- Bounded Dispose drain (5 s, cancelable token).
- ScriptedAlarmSource catch logs at Warning.
- LocalDbStoreAndForwardSink cached dead-letter counter, resynced with the queue counter.
- Script-log sinks rate-limited per source (20/min + summary).
E4 Alarm host restart + virtual-tag log storms (R02:H1, H3, P6, L3; R07:P4, H6, H8, U5):
- ScriptedAlarmHostActor receives a Func<(upstream, engine)> factory, building its engine per incarnation.
- Linked 10 s timeouts on OnAlarmCommand.
- DependencyMuxTagUpstreamSource.RetainOnly(loaded deps).
- VirtualTagActor per-tag failure-log rate limit.
- Calculation:
  - negative-cache compile failures by source hash, with the Warning/script-log only on the Good→Bad transition;
  - the mux retains the last (value,status,ts) and replays it on RegisterInterest, with inputs recorded regardless of _running;
  - structural array dedup;
  - ClearCompiledScripts from ShutdownAsync;
  - a DraftValidator rule CalculationNoEffectiveTrigger.
- Tests:
  - Throw inside a DependencyValueBatch handler, then a later ApplyScriptedAlarms loads cleanly.
  - A calc Restart republishes the current inputs.

Lane gate: suites green. Wave 4 docker-dev gate: author a while(true) virtual tag. The node stays responsive, the tag shows quarantined, and the thread count stays flat over 2 min.

From the 2026-10-21 Fable system review. Findings `R0x:ID` refer to `docs/reviews/2026-10-21/0x-*.md` (location, evidence, failure scenario, fix recipe). Plan: `docs/superpowers/plans/2026-10-22-review-remediation.md`. ### Lane E — Scripting runtime safety + scripted alarms (fable for E1/E4, opus for E2/E3; review fable) **Owns:** - `src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/**`, `Core.ScriptedAlarms/**`, `Core.AlarmHistorian/**` - `Runtime/ScriptedAlarms/**`, `Runtime/VirtualTags/**` - `Host/Engines/**` - `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation/**` - `Configuration/Validation/DraftValidator.cs` **Tests:** `Core.Scripting.Tests`, `Core.ScriptedAlarms.Tests`, `Core.AlarmHistorian.Tests`, `Driver.Calculation.Tests`, `Runtime.Tests` (VirtualTags/ScriptedAlarms folders) - **E1 Script quarantine** (`R03:1.1`, `R05:2`, `R07:L1`, `R07:P5`): - A shared `ScriptRunGuard` in Core.Scripting, keyed per script instance. - It refuses to start a run while that script's previous orphan is still executing. The result is Bad `"previous evaluation still running"` plus a counter. - It quarantines after 3 consecutive timeouts: stop evaluating, publish Bad, log Error, metric `otopcua_script_quarantined`, cleared on redeploy or source change. - Pass a `CancellationToken` into scripts. - Make the Calculation pump async, so pool starvation is no longer charged to scripts. - Wired into the scripted-alarm engine, `RoslynVirtualTagEvaluator`/`VirtualTagHostActor`, and `CalculationEvaluator`. - Tests: a `while(true)` script on a 1 Hz trigger leaves ≤ 1 orphaned thread after 30 triggers (`ThreadPool.ThreadCount` delta). Positive control: a healthy sibling keeps evaluating. - **E2 Sandbox hardening** (`R05:10`, `R05:11`): - Deny `System.Threading` with an allow-list (`CancellationToken`, `Interlocked`, `Volatile`). - Deny `System.Type.GetType`/`GetMethod*`/`InvokeMember` and `System.Runtime.CompilerServices.RuntimeHelpers`. - Add an allocation budget: sample `GC.GetAllocatedBytesForCurrentThread` around each run and quarantine on breach (`Scripting:MaxAllocBytesPerRun`, default 64 MB). Document that a pre-emptive OOM still needs an out-of-process runner, and file a follow-up issue for that. - Tests: each forbidden form gets an `OTSCRIPT` diagnostic, and the Monaco editor parity test still matches. - **E3 Scripted-alarm engine** (`R05:1`, `3`, `6`, `7`, `9`, `12`): - Clear `_valueCache` in `LoadAsync`; move its write below the `_disposed` guard. - Bounded `Dispose` drain (5 s, cancelable token). - `ScriptedAlarmSource` catch logs at Warning. - `LocalDbStoreAndForwardSink` cached dead-letter counter, resynced with the queue counter. - Script-log sinks rate-limited per source (20/min + summary). - **E4 Alarm host restart + virtual-tag log storms** (`R02:H1`, `H3`, `P6`, `L3`; `R07:P4`, `H6`, `H8`, `U5`): - `ScriptedAlarmHostActor` receives a `Func<(upstream, engine)>` factory, building its engine per incarnation. - Linked 10 s timeouts on `OnAlarmCommand`. - `DependencyMuxTagUpstreamSource.RetainOnly(loaded deps)`. - `VirtualTagActor` per-tag failure-log rate limit. - Calculation: - negative-cache compile failures by source hash, with the Warning/script-log only on the Good→Bad transition; - the mux retains the last `(value,status,ts)` and replays it on `RegisterInterest`, with inputs recorded regardless of `_running`; - structural array dedup; - `ClearCompiledScripts` from `ShutdownAsync`; - a DraftValidator rule `CalculationNoEffectiveTrigger`. - Tests: - Throw inside a `DependencyValueBatch` handler, then a later `ApplyScriptedAlarms` loads cleanly. - A calc Restart republishes the current inputs. **Lane gate:** suites green. Wave 4 docker-dev gate: author a `while(true)` virtual tag. The node stays responsive, the tag shows quarantined, and the thread count stays flat over 2 min.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: dohertj2/lmxopcua#734