Single commit covering the four small/medium fixes from the updated
code review.
Core.Scripting-014 (Medium, Concurrency):
CompiledScriptCache.Clear() used the key-only TryRemove(key, out var
lazy) overload — same race shape Core.Scripting-006 closed in
GetOrCompile's catch block. A concurrent re-add between snapshot and
TryRemove was evicted + disposed while the new caller still held it.
Replaced with the value-scoped TryRemove(KeyValuePair<,>) overload.
Regression test
Clear_uses_value_scoped_TryRemove_so_a_race_inserted_entry_survives
added.
Core.Scripting-013 (Medium, Security):
Hand-rolled BuildWrapperSource pastes user source between literal
braces; brace-balanced source could inject sibling methods/classes
alongside CompiledScript.Run. Analyzer still walked the injected
members so it wasn't a direct escape, but it relaxed the documented
'method body' authoring contract. Added EnforceSingleRunMember:
after ParseText, the compilation unit must hold exactly one type
(CompiledScript) and that type must hold exactly one member (the Run
method). Any deviation throws CompilationErrorException with LMX001/
LMX002 diagnostic IDs and a Core.Scripting-013 reference in the
message. Two regression tests added covering the sibling-method and
sibling-class injection vectors.
Core.Scripting-015 (Low, Correctness, latent):
ToCSharpTypeName's generic branch truncated at the first backtick via
IndexOf, silently dropping closed args of nested-generic shapes
(Outer<T>.Inner<U>). No production caller exercises this shape today
(all TContext/TResult are top-level non-nested), so the bug was
latent. Rewrote the generic branch to walk the FullName segment-by-
segment, consuming generic args per segment so nested shapes emit
valid C# (global::Ns.Outer<T>.Inner<U> rather than the broken
Outer<T,U>).
Core.ScriptedAlarms-013 (Low, Documentation):
The internal test accessors TryGetScratchReadCacheForTest /
TryGetScratchContextForTest return live mutable scratch refilled in
place under _evalGate. XML docs didn't warn future test authors about
the synchronization contract. Added a <remarks> block to each
documenting the only-safe-on-quiesced-engine + identity-or-single-key
contract.
Verification (suites green):
Core.Scripting.Tests: 110/110 (was 107 — +3 new rejection/race tests)
Core.ScriptedAlarms.Tests: 67/67 (unchanged — doc-only fix)
Core.VirtualTags.Tests: 57/57 (unchanged)
After this commit, all 12 findings from the updated re-review are
closed (10 Resolved, 1 Won't Fix none, 1 Deferred — Driver.Galaxy-017).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Core.Scripting-012 (High, Security) resolution.
The Core.Scripting-008 rewrite broadened the BCL references list from a
narrow allow-list to the full System.* + netstandard +
Microsoft.Win32.Registry set, delegating the security gate entirely to
ForbiddenTypeAnalyzer. Three categories of dangerous BCL types were
reachable from script source without a deny-list entry:
- System.Threading.ThreadPool — QueueUserWorkItem re-introduces the
background-fanout threat Core.Scripting-003 closed against
System.Threading.Tasks.
- System.Threading.Timer — schedules unbounded callback work that
outlives the per-evaluation timeout.
- System.Runtime.Loader.AssemblyLoadContext — loads arbitrary DLLs.
Defense-in-depth gap; invocation needs reflection (already denied)
but the load itself was reachable.
Fix:
- Added 'System.Runtime.Loader' to ForbiddenNamespacePrefixes
(preferred over type-granular per the recommendation so future BCL
additions to that namespace are denied by default).
- Added 'System.Threading.ThreadPool' and 'System.Threading.Timer'
to ForbiddenFullTypeNames — both live in System.Threading shared
with allowed primitives so they must be type-granular.
Regression tests added to ScriptSandboxTests:
Rejects_ThreadPool_QueueUserWorkItem_at_compile
Rejects_Timer_new_at_compile
Rejects_AssemblyLoadContext_at_compile
Docs:
docs/v2/implementation/phase-7-scripting-and-alarming.md decision #6
and the Sandbox-escape compliance-check row both updated to enumerate
the new entries per the Core.Scripting-009 doc-sync convention.
Two lower-impact suggestions from the finding's recommendation
(System.Console, CultureInfo.DefaultThreadCurrentCulture) were
intentionally not addressed and are recorded as accepted minor risks
in the resolution.
Verification: Core.Scripting.Tests 107/107 (was 104 + 3 new rejection
tests).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add ScriptSandboxTests cases for all forbidden-namespace deny-list
vectors that lacked test coverage: System.Threading.Thread,
System.Threading.Tasks.Task.Run (newly denied per Core.Scripting-003),
System.Runtime.InteropServices.Marshal, and Microsoft.Win32.Registry.
The 001/002 type-granular and node-form vectors were already covered by
the -001/-002 resolution commits. All 79 tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The ForbiddenTypeAnalyzer syntax walker only inspected four node kinds
(ObjectCreation, Invocation-with-member-access, MemberAccess, bare
Identifier), so a forbidden type named through typeof, a generic type
argument, a cast, an is/as type pattern, default(T), an array-creation
element type, or an explicitly-typed local declaration produced no
examined node and bypassed the sandbox check.
Analyze now runs a second pass that resolves GetTypeInfo on every
TypeSyntax node and recursively unwraps array element types and generic
type arguments, so forbidden types nested at any depth are rejected at
compile. The original member/call node-kind switch is kept deliberately
narrow (rather than resolving GetSymbolInfo on every node) to avoid
flagging harmless inherited members such as typeof(int).Name, whose Name
property is declared by System.Reflection.MemberInfo. A span+type dedupe
keeps the two passes from emitting duplicate rejections.
Regression tests added in ScriptSandboxTests cover typeof, generic type
arguments, casts, default(T), is/as patterns, array element types, and
typed local declarations with forbidden types, plus over-block guards
asserting allowed generics and typeof still compile.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ForbiddenTypeAnalyzer used only a namespace-prefix deny-list. System.Environment,
System.AppDomain, System.GC and System.Activator live directly in the System
namespace, which must stay allowed for primitives (Math, String, ...), so they
were never caught — an operator-authored predicate could call
System.Environment.Exit(0) and terminate the in-process OPC UA server.
Add a type-granular deny-list (ForbiddenFullTypeNames) checked by
fully-qualified type name after the namespace-prefix check; legitimate System
types are unaffected.
Regression tests assert scripts referencing Environment/AppDomain/GC/Activator
are rejected at analysis time. Core.Scripting suite: 68/68 pass.
Resolves code-review finding Core.Scripting-001 (Critical).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Group all 69 projects into category subfolders under src/ and tests/ so the
Rider Solution Explorer mirrors the module structure. Folders: Core, Server,
Drivers (with a nested Driver CLIs subfolder), Client, Tooling.
- Move every project folder on disk with git mv (history preserved as renames).
- Recompute relative paths in 57 .csproj files: cross-category ProjectReferences,
the lib/ HintPath+None refs in Driver.Historian.Wonderware, and the external
mxaccessgw refs in Driver.Galaxy and its test project.
- Rebuild ZB.MOM.WW.OtOpcUa.slnx with nested solution folders.
- Re-prefix project paths in functional scripts (e2e, compliance, smoke SQL,
integration, install).
Build green (0 errors); unit tests pass. Docs left for a separate pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
