diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptCompilationService.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptCompilationService.cs index 68f71779..326987d4 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptCompilationService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptCompilationService.cs @@ -1,49 +1,23 @@ using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Scripting; -using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Scripting; using Microsoft.Extensions.Logging; using ZB.MOM.WW.ScadaBridge.Commons.Types; +using ZB.MOM.WW.ScadaBridge.ScriptAnalysis; namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts; /// /// WP-19: Script Trust Model — compiles C# scripts using Roslyn with restricted API access. -/// Forbidden APIs: System.IO, Process, Threading (except async/await), Reflection, -/// System.Net.Sockets, System.Net.Http. +/// The forbidden-API verdict is delegated to the shared authoritative +/// (M3.1 consolidation); this service keeps the real +/// execution-path compile of the script against / +/// . /// public class ScriptCompilationService { private readonly ILogger _logger; - /// - /// Forbidden API roots. Each entry is matched as a prefix against both the resolved - /// symbol's containing namespace and its fully-qualified containing type name, so an - /// entry may name a whole namespace ("System.IO") or a single type - /// ("System.Diagnostics.Process"). - /// - private static readonly string[] ForbiddenNamespaces = - [ - "System.IO", - "System.Diagnostics.Process", - "System.Threading", - "System.Reflection", - "System.Net.Sockets", - "System.Net.Http" - ]; - - /// - /// Specific namespaces/types allowed even though they sit under a forbidden root. - /// async/await and cancellation tokens are OK despite System.Threading being blocked. - /// - private static readonly string[] AllowedExceptions = - [ - "System.Threading.Tasks", - "System.Threading.CancellationToken", - "System.Threading.CancellationTokenSource" - ]; - /// Initializes a new instance of the ScriptCompilationService class. /// Logger instance. public ScriptCompilationService(ILogger logger) @@ -54,160 +28,24 @@ public class ScriptCompilationService /// /// SiteRuntime-011: validates that the script does not reference forbidden APIs. /// - /// Validation is performed with Roslyn semantic analysis rather than a raw substring - /// scan of the source text. The script is parsed and a semantic model is built; every - /// identifier, type reference, member access, and object creation is resolved to its - /// symbol and the symbol's containing namespace is checked against the forbidden list. - /// - /// This is reliable in both directions a textual scan was not: - /// - it catches forbidden types regardless of how they are written (global:: - /// prefixes, aliases, transitively-imported namespaces) because it inspects the - /// resolved symbol, not the spelling; - /// - it does not raise false positives for the namespace string appearing in a - /// comment, a string literal, or an unrelated identifier. + /// As of the M3.1 script-analysis consolidation this delegates to the shared + /// authoritative , + /// which is the same Roslyn semantic-symbol analysis this service previously hosted + /// plus reflection-gateway / dynamic / Activator hardening ported from + /// the InboundAPI checker. The shared validator is the single source of truth for the + /// forbidden-API deny-list; SiteRuntime retains only the real execution-path compile + /// in . /// /// Returns a list of violation messages, empty if clean. /// /// The script code to validate. /// A list of trust-model violation messages; empty if the script is clean. public IReadOnlyList ValidateTrustModel(string code) - { - var tree = CSharpSyntaxTree.ParseText( - code, new CSharpParseOptions(kind: SourceCodeKind.Script)); - - var compilation = CSharpCompilation.CreateScriptCompilation( - "TrustValidation", - tree, - ScriptReferences, - new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); - - var model = compilation.GetSemanticModel(tree); - var root = tree.GetRoot(); - - // Deduplicate so a forbidden symbol used many times is reported once but - // distinct forbidden symbols are all reported. - var violations = new SortedSet(StringComparer.Ordinal); - - foreach (var node in root.DescendantNodes()) - { - // Only inspect nodes that name a type or member; skip declarations, - // string literals and comments entirely. Member-access and qualified-name - // parents are evaluated as a whole, so their nested name parts are skipped. - if (node is not (SimpleNameSyntax or MemberAccessExpressionSyntax - or QualifiedNameSyntax or ObjectCreationExpressionSyntax)) - { - continue; - } - - var info = model.GetSymbolInfo(node); - var symbol = info.Symbol ?? info.CandidateSymbols.FirstOrDefault(); - - // The set of fully-qualified scopes this reference touches: the resolved - // symbol's containing namespace and type, or — when the symbol could not - // be resolved (a type from an unreferenced assembly) — the syntactic - // fully-qualified name written in source as a safe fallback. - var scopes = symbol != null - ? GetSymbolScopes(symbol) - : GetSyntacticScopes(node); - if (scopes.Count == 0) - continue; - - var forbidden = ForbiddenNamespaces.FirstOrDefault( - f => scopes.Any(s => IsUnderScope(s, f))); - if (forbidden == null) - continue; - - // Allow specific exception namespaces/types (async/await, cancellation). - if (scopes.Any(s => AllowedExceptions.Any(a => IsUnderScope(s, a)))) - continue; - - var name = symbol?.Name ?? node.ToString(); - violations.Add($"Forbidden API reference: '{forbidden}' ({scopes[0]}.{name})"); - } - - return violations.ToList(); - } + => ScriptTrustValidator.FindViolations(code); /// - /// Returns the fully-qualified scopes a resolved symbol belongs to — its containing - /// namespace and, for a type or member, the fully-qualified containing type. A bare - /// namespace symbol is intentionally ignored: a namespace name on its own performs - /// no action; harm requires referencing a type or a member. - /// - private static List GetSymbolScopes(ISymbol symbol) - { - var scopes = new List(); - - switch (symbol) - { - case INamespaceSymbol: - // A namespace reference alone is harmless — skip it. (This avoids a - // false positive on the "System.Threading" qualifier of the allowed - // "System.Threading.Tasks.Task".) - break; - case ITypeSymbol typeSymbol: - scopes.Add(typeSymbol.ToDisplayString()); - if (typeSymbol.ContainingNamespace is { IsGlobalNamespace: false } typeNs) - scopes.Add(typeNs.ToDisplayString()); - break; - default: - if (symbol.ContainingType != null) - { - scopes.Add(symbol.ContainingType.ToDisplayString()); - if (symbol.ContainingType.ContainingNamespace is { IsGlobalNamespace: false } memberNs) - scopes.Add(memberNs.ToDisplayString()); - } - else if (symbol.ContainingNamespace is { IsGlobalNamespace: false } ns) - { - scopes.Add(ns.ToDisplayString()); - } - break; - } - - return scopes; - } - - /// - /// Fallback used when a name could not be resolved to a symbol (e.g. a type from an - /// assembly the script is not allowed to reference). The fully-qualified name as - /// written in source is used directly — a script that names - /// System.Net.Http.HttpClient is still rejected even though that assembly is - /// deliberately absent from the script's metadata references. - /// - private static List GetSyntacticScopes(SyntaxNode node) - { - // A dotted name written in source is itself the fully-qualified scope. Only - // consider names that actually contain a dot — bare local identifiers cannot - // reach a forbidden namespace. - var text = node switch - { - QualifiedNameSyntax q => q.ToString(), - MemberAccessExpressionSyntax m => m.ToString(), - _ => string.Empty - }; - - // Strip whitespace/newlines that a multi-line member-access chain may contain. - text = new string(text.Where(c => !char.IsWhiteSpace(c)).ToArray()); - - return string.IsNullOrEmpty(text) || !text.Contains('.') - ? [] - : [text]; - } - - /// - /// True if is exactly, or nested within, - /// (e.g. "System.IO.Compression" is under "System.IO", - /// "System.Diagnostics.Process" is under "System.Diagnostics.Process"). - /// - private static bool IsUnderScope(string actual, string root) - => actual.Equals(root, StringComparison.Ordinal) - || actual.StartsWith(root + ".", StringComparison.Ordinal); - - /// - /// Assemblies referenced by compiled scripts. Shared between the Roslyn scripting - /// options and the semantic-analysis compilation built for trust validation - /// (SiteRuntime-011), so the validator resolves symbols against exactly the same - /// metadata the script is compiled against. + /// Assemblies referenced by compiled scripts, used to build the Roslyn scripting + /// options for the real execution-path compile. /// private static readonly System.Reflection.Assembly[] ScriptAssemblies = [ @@ -218,14 +56,6 @@ public class ScriptCompilationService typeof(Commons.Types.DynamicJsonElement).Assembly ]; - /// - /// Metadata references for the trust-validation semantic compilation. - /// - private static readonly MetadataReference[] ScriptReferences = - ScriptAssemblies - .Select(a => (MetadataReference)MetadataReference.CreateFromFile(a.Location)) - .ToArray(); - /// /// Shared Roslyn scripting options (references + imports) used by both full /// script compilation and trigger-expression compilation. diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/ZB.MOM.WW.ScadaBridge.SiteRuntime.csproj b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/ZB.MOM.WW.ScadaBridge.SiteRuntime.csproj index ea57dae4..ce9bce60 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/ZB.MOM.WW.ScadaBridge.SiteRuntime.csproj +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/ZB.MOM.WW.ScadaBridge.SiteRuntime.csproj @@ -35,6 +35,7 @@ + diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/CompileSurfaceParityTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/CompileSurfaceParityTests.cs new file mode 100644 index 00000000..685ff5a8 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/CompileSurfaceParityTests.cs @@ -0,0 +1,105 @@ +using System.Reflection; +using ZB.MOM.WW.ScadaBridge.ScriptAnalysis; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts; + +namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Scripts; + +/// +/// M3.3: compile-surface parity guard. The shared +/// / are +/// compile-only stubs the design-time deploy gate binds candidate scripts +/// against; they must mirror the real SiteRuntime / +/// bind surfaces. If a member a script can +/// reference on the real globals is missing from the compile surface, a script +/// that uses it would pass the design-time gate but fail at the site — so the +/// compile surface's public top-level member NAMES must be a SUPERSET of the +/// real globals' names. This test fails loudly (listing the missing names) when +/// the surface drifts behind the runtime globals. +/// +/// +/// Top-level member-name parity is sufficient: the design-time compile against +/// the surface catches deeper signature mismatches itself; this guard only +/// ensures every entry point a script can name on the real globals exists on the +/// stub. +/// +/// +public class CompileSurfaceParityTests +{ + [Fact] + public void ScriptCompileSurface_MemberNames_AreSupersetOf_ScriptGlobals() + { + AssertSurfaceCoversGlobals( + surface: typeof(ScriptCompileSurface), + globals: typeof(ScriptGlobals)); + } + + [Fact] + public void TriggerCompileSurface_MemberNames_AreSupersetOf_TriggerExpressionGlobals() + { + AssertSurfaceCoversGlobals( + surface: typeof(TriggerCompileSurface), + globals: typeof(TriggerExpressionGlobals)); + } + + /// + /// Asserts that the public instance property + method member names of + /// are a superset of those of + /// . Inherited members + /// (ToString/GetHashCode/Equals/GetType) are excluded. Fails with the exact + /// list of missing names when the surface does not cover the globals. + /// + private static void AssertSurfaceCoversGlobals(Type surface, Type globals) + { + var surfaceNames = PublicInstanceMemberNames(surface); + var globalsNames = PublicInstanceMemberNames(globals); + + var missing = globalsNames + .Where(name => !surfaceNames.Contains(name)) + .OrderBy(name => name, StringComparer.Ordinal) + .ToList(); + + Assert.True( + missing.Count == 0, + $"Compile surface '{surface.Name}' is missing {missing.Count} member name(s) " + + $"present on '{globals.Name}': {string.Join(", ", missing)}. " + + "The compile-only surface must mirror the runtime globals — add the " + + "missing member(s) to the ScriptAnalysis surface type."); + } + + /// + /// Public instance property + method member names declared on or inherited by + /// , excluding the inherited + /// members (ToString, GetHashCode, Equals, GetType) so only script-reachable + /// API names remain. Property/method names are compared (no signatures). + /// + private static HashSet PublicInstanceMemberNames(Type type) + { + const BindingFlags flags = BindingFlags.Public | BindingFlags.Instance; + + var objectMembers = new HashSet(StringComparer.Ordinal) + { + nameof(ToString), + nameof(GetHashCode), + nameof(Equals), + nameof(GetType), + }; + + var names = new HashSet(StringComparer.Ordinal); + + foreach (var property in type.GetProperties(flags)) + names.Add(property.Name); + + foreach (var method in type.GetMethods(flags)) + { + // Skip compiler-generated property accessors (get_X / set_X) — the + // property itself is already counted by name above. + if (method.IsSpecialName) + continue; + if (objectMembers.Contains(method.Name)) + continue; + names.Add(method.Name); + } + + return names; + } +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScriptCompilationServiceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScriptCompilationServiceTests.cs index 39d1f960..f02aca5d 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScriptCompilationServiceTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScriptCompilationServiceTests.cs @@ -5,6 +5,16 @@ namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Scripts; /// /// WP-19: Script Trust Model tests — validates forbidden API detection and compilation. +/// +/// As of the M3.3 consolidation, ScriptCompilationService.ValidateTrustModel +/// delegates its forbidden-API verdict to the shared authoritative +/// ScriptAnalysis.ScriptTrustValidator, which is stricter than SiteRuntime's +/// original deny-list: ALL of System.Net is forbidden (not just Sockets/Http), +/// plus reflection gateways, dynamic, Activator, +/// System.Runtime.InteropServices and Microsoft.Win32. Only +/// System.Diagnostics.Process is blocked under System.Diagnostics — +/// Stopwatch stays allowed. The real execution-path compile against +/// ScriptGlobals / TriggerExpressionGlobals is unchanged. /// public class ScriptCompilationServiceTests { @@ -108,4 +118,61 @@ public class ScriptCompilationServiceTests Assert.False(result.IsSuccess); Assert.NotEmpty(result.Errors); } + + // ── M3.3: stricter shared-validator behavior ── + + [Fact] + public void ValidateTrustModel_SystemNetDns_Forbidden() + { + // The shared validator forbids ALL of System.Net — not just Sockets/Http. + // System.Net.Dns was allowed under the old SiteRuntime list; now blocked. + var violations = _service.ValidateTrustModel( + "System.Net.Dns.GetHostName()"); + Assert.NotEmpty(violations); + Assert.Contains(violations, v => v.Contains("System.Net")); + } + + [Fact] + public void ValidateTrustModel_ReflectionGatewayViaPermittedType_Forbidden() + { + // typeof(x).Assembly.GetType(...) never spells a forbidden namespace, but + // the shared validator rejects the reflection-gateway members regardless of + // receiver — this was NOT caught by the old SiteRuntime list. + var violations = _service.ValidateTrustModel( + "typeof(string).Assembly.GetType(\"System.IO.File\")"); + Assert.NotEmpty(violations); + } + + [Fact] + public void ValidateTrustModel_Dynamic_Forbidden() + { + var violations = _service.ValidateTrustModel("dynamic d = 1; return d;"); + Assert.NotEmpty(violations); + } + + [Fact] + public void ValidateTrustModel_Activator_Forbidden() + { + var violations = _service.ValidateTrustModel( + "Activator.CreateInstance(typeof(string))"); + Assert.NotEmpty(violations); + } + + [Fact] + public void ValidateTrustModel_InteropServices_Forbidden() + { + var violations = _service.ValidateTrustModel( + "System.Runtime.InteropServices.Marshal.SizeOf()"); + Assert.NotEmpty(violations); + } + + [Fact] + public void ValidateTrustModel_Stopwatch_Allowed() + { + // Only System.Diagnostics.Process is blocked under System.Diagnostics — + // Stopwatch stays allowed. + var violations = _service.ValidateTrustModel( + "var sw = System.Diagnostics.Stopwatch.StartNew(); return sw.ElapsedMilliseconds;"); + Assert.Empty(violations); + } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/TrustModelSemanticTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/TrustModelSemanticTests.cs index 6c21a277..e38ca2f8 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/TrustModelSemanticTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/TrustModelSemanticTests.cs @@ -8,6 +8,12 @@ namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Scripts; /// The previous implementation was a raw substring scan of the source text — it both /// missed forbidden APIs (no literal namespace string) and raised false positives on /// the namespace string appearing in comments, string literals or unrelated identifiers. +/// +/// As of M3.3, ValidateTrustModel delegates to the shared authoritative +/// ScriptAnalysis.ScriptTrustValidator, which retains this same Roslyn +/// semantic-symbol analysis (plus reflection-gateway hardening), so these bypass / +/// false-positive / allowed-exception regressions continue to hold through the +/// delegating service. /// public class TrustModelSemanticTests { diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.csproj b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.csproj index 05546e40..0f1db5ff 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.csproj +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.csproj @@ -25,6 +25,7 @@ +