diff --git a/src/ZB.MOM.WW.ScadaBridge.ScriptAnalysis/ScriptTrustPolicy.cs b/src/ZB.MOM.WW.ScadaBridge.ScriptAnalysis/ScriptTrustPolicy.cs index c819ae1b..0d8a4901 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ScriptAnalysis/ScriptTrustPolicy.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ScriptAnalysis/ScriptTrustPolicy.cs @@ -1,3 +1,4 @@ +using System.IO; using System.Reflection; using Microsoft.CodeAnalysis; using ZB.MOM.WW.ScadaBridge.Commons.Types; @@ -121,6 +122,67 @@ public static class ScriptTrustPolicy .Select(a => (MetadataReference)MetadataReference.CreateFromFile(a.Location)) .ToList(); + /// + /// The full trusted-platform reference set used ONLY by + /// 's semantic analysis — NOT by + /// . Unlike + /// (the minimal, runtime-fidelity set used to decide script validity, + /// which must mirror exactly what the site runtime compiles against), the + /// trust validator references the entire framework so that EVERY type a + /// script names resolves to a real symbol and is judged by its true + /// namespace. Without this, a forbidden TYPE that sits inside an ALLOWED + /// namespace and is reached as a bare identifier — the only such case in the + /// policy being System.Diagnostics.Process via + /// using System.Diagnostics; — would not resolve against a minimal + /// reference set and would slip past the semantic pass (still blocked + /// downstream as an undefined-symbol compile error, but with a misleading + /// message). Referencing the full framework lets the validator flag it + /// authoritatively as a forbidden API. Enriching the analysis reference set + /// can only IMPROVE detection — the verdict is by namespace/type, so more + /// resolvable symbols means more correct verdicts, never a false allow. + /// + public static readonly IReadOnlyList AnalysisReferences = BuildAnalysisReferences(); + + private static IReadOnlyList BuildAnalysisReferences() + { + var byPath = new Dictionary(StringComparer.OrdinalIgnoreCase); + + // Trusted platform assemblies = the full framework reference set the host + // started with; lets the semantic pass resolve any BCL type. + if (AppContext.GetData("TRUSTED_PLATFORM_ASSEMBLIES") is string tpa) + { + foreach (var path in tpa.Split(Path.PathSeparator)) + { + if (path.Length == 0 || + !path.EndsWith(".dll", StringComparison.OrdinalIgnoreCase) || + byPath.ContainsKey(path) || + !File.Exists(path)) + { + continue; + } + + try { byPath[path] = MetadataReference.CreateFromFile(path); } + catch { /* skip an unreadable assembly rather than fail validation */ } + } + } + + // Ensure app assemblies the script API surface needs are present even if + // not in the TPA list (e.g. Commons / DynamicJsonElement). + foreach (var asm in DefaultAssemblies) + { + var loc = asm.Location; + if (loc.Length == 0 || byPath.ContainsKey(loc) || !File.Exists(loc)) + continue; + + try { byPath[loc] = MetadataReference.CreateFromFile(loc); } + catch { /* ignore */ } + } + + // Fallback to the minimal set if the TPA list was unavailable (e.g. a + // single-file/AOT host) so validation still functions. + return byPath.Count > 0 ? byPath.Values.ToList() : DefaultReferences; + } + /// /// Default namespace imports made available to compiled scripts. /// diff --git a/src/ZB.MOM.WW.ScadaBridge.ScriptAnalysis/ScriptTrustValidator.cs b/src/ZB.MOM.WW.ScadaBridge.ScriptAnalysis/ScriptTrustValidator.cs index d0335f31..bbe2c7c1 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ScriptAnalysis/ScriptTrustValidator.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ScriptAnalysis/ScriptTrustValidator.cs @@ -79,7 +79,12 @@ public static class ScriptTrustValidator var violations = new SortedSet(StringComparer.Ordinal); // ---- Pass 1: semantic symbol analysis (ported from SiteRuntime) ---- - var references = ScriptTrustPolicy.DefaultReferences.ToList(); + // Use the full trusted-platform reference set (not the minimal + // runtime-fidelity DefaultReferences) so EVERY type a script names + // resolves and is judged by its true namespace — closing the + // forbidden-type-in-allowed-namespace blind spot (e.g. a bare + // System.Diagnostics.Process via `using System.Diagnostics;`). + var references = ScriptTrustPolicy.AnalysisReferences.ToList(); if (extraReferences != null) references.AddRange(extraReferences); diff --git a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/CSharpDelimiterScanner.cs b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/CSharpDelimiterScanner.cs index 3fc99c74..b9a7c41a 100644 --- a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/CSharpDelimiterScanner.cs +++ b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/CSharpDelimiterScanner.cs @@ -2,12 +2,11 @@ namespace ZB.MOM.WW.ScadaBridge.TemplateEngine.Validation; /// /// String/comment-aware scanner for the balanced-delimiter ("does it look like -/// valid C#") checks used by and -/// SharedScriptService.ValidateSyntax. +/// valid C#") check used by SharedScriptService.ValidateSyntax. /// /// -/// This is not a compiler. It is an interim structural check that walks -/// the source once and tracks {}, [] and () depth while +/// This is not a compiler. It is a structural check that walks the +/// source once and tracks {}, [] and () depth while /// correctly skipping over the C# lexical constructs in which a delimiter is /// inert: line/block comments, regular string literals (with \ escapes), /// verbatim strings (@"...", where "" escapes a quote and \ @@ -17,11 +16,11 @@ namespace ZB.MOM.WW.ScadaBridge.TemplateEngine.Validation; /// /// /// -/// It is intentionally conservative: when the real Roslyn-based compiler is -/// wired in (see ) this hand-rolled scan should be -/// replaced by CSharpSyntaxTree.ParseText diagnostics. Until then this -/// scanner removes the false positives that a naive character count produced -/// for valid scripts containing a delimiter inside a string or comment. +/// Trust enforcement and full compilation are NOT done here — those are the +/// authoritative (which delegates to the shared +/// ZB.MOM.WW.ScadaBridge.ScriptAnalysis validator + Roslyn compile). This +/// scanner only provides SharedScriptService a cheap pre-compile sanity +/// check for balanced delimiters. /// /// internal static class CSharpDelimiterScanner @@ -41,121 +40,6 @@ internal static class CSharpDelimiterScanner UnterminatedChar, } - /// - /// Returns true when occurs in a code - /// region of — i.e. not wholly inside a string - /// literal, char literal, or comment. Used by the interim forbidden-API - /// scan so that the inert text System.IO. in a comment or string - /// literal is not flagged as a forbidden API call (TemplateEngine-006). - /// - /// - /// This removes the false-positive half of the substring scan. It does - /// not close the bypass half: namespace aliases, using static, - /// and global::-qualified references still evade a pure text match. - /// Authoritative forbidden-API enforcement requires Roslyn semantic symbol - /// analysis and is deferred to the real script compiler / Site Runtime - /// sandbox; this check is advisory only. - /// - /// - /// The C# source code to scan. - /// The substring to search for in code regions only. - /// true if occurs in a code region (not inside a comment, string, or char literal); otherwise false. - internal static bool ContainsInCode(string code, string pattern) - { - if (string.IsNullOrEmpty(pattern)) - return false; - - // Blank out every string/char-literal/comment span, then do an ordinary - // substring search over what remains (the code regions). - var codeOnly = BlankNonCodeSpans(code); - return codeOnly.Contains(pattern, StringComparison.Ordinal); - } - - /// - /// Replaces the content of every comment, string literal, and char literal - /// with spaces (newlines preserved), leaving only code regions intact. - /// Delimiter characters themselves are also blanked so a pattern cannot - /// straddle a literal boundary. - /// - private static string BlankNonCodeSpans(string code) - { - var buffer = code.ToCharArray(); - int n = code.Length; - int i = 0; - - void Blank(int from, int to) - { - for (int k = from; k < to && k < n; k++) - if (buffer[k] != '\n' && buffer[k] != '\r') - buffer[k] = ' '; - } - - while (i < n) - { - char c = code[i]; - char next = i + 1 < n ? code[i + 1] : '\0'; - int start = i; - - if (c == '/' && next == '/') - { - i += 2; - while (i < n && code[i] != '\n') i++; - Blank(start, i); - continue; - } - if (c == '/' && next == '*') - { - i += 2; - while (i < n && !(code[i] == '*' && i + 1 < n && code[i + 1] == '/')) i++; - if (i < n) i += 2; - Blank(start, i); - continue; - } - if (c == '"' && next == '"' && i + 2 < n && code[i + 2] == '"') - { - SkipRawString(code, ref i); - Blank(start, i); - continue; - } - if (c == '$') - { - int j = i + 1; - bool verbatim = false; - if (j < n && code[j] == '@') { verbatim = true; j++; } - if (j < n && code[j] == '"') - { - i = j; - SkipInterpolatedString(code, ref i, verbatim); - Blank(start, i); - continue; - } - } - if (c == '@' && next == '"') - { - i++; - SkipVerbatimString(code, ref i); - Blank(start, i); - continue; - } - if (c == '"') - { - SkipRegularString(code, ref i); - Blank(start, i); - continue; - } - if (c == '\'') - { - SkipCharLiteral(code, ref i); - Blank(start, i); - continue; - } - - i++; - } - - return new string(buffer); - } - /// /// Walks once and reports the first structural /// delimiter problem, or when the source is diff --git a/tests/ZB.MOM.WW.ScadaBridge.ScriptAnalysis.Tests/ScriptTrustValidatorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ScriptAnalysis.Tests/ScriptTrustValidatorTests.cs index 431b6714..be77f8c2 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.ScriptAnalysis.Tests/ScriptTrustValidatorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.ScriptAnalysis.Tests/ScriptTrustValidatorTests.cs @@ -147,6 +147,23 @@ public class ScriptTrustValidatorTests Assert.NotEmpty(ScriptTrustValidator.FindViolations(code)); } + [Fact] + public void Rejects_Process_QualifiedType() + { + var code = "var p = System.Diagnostics.Process.Start(\"x\");"; + Assert.NotEmpty(ScriptTrustValidator.FindViolations(code)); + } + + [Fact] + public void Rejects_Process_BareIdentifier_ViaUsing() + { + // System.Diagnostics is an ALLOWED namespace (Stopwatch/Debug ok), so the + // using directive is not flagged; Process is a forbidden TYPE reached as a + // bare identifier. This pins whether FindViolations resolves it. + var code = "using System.Diagnostics; var p = Process.Start(\"x\");"; + Assert.NotEmpty(ScriptTrustValidator.FindViolations(code)); + } + // ---- Clean (empty violations) ------------------------------------------- [Fact] diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScopeAccessorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScopeAccessorTests.cs index 73458ab8..048a2449 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScopeAccessorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScopeAccessorTests.cs @@ -130,9 +130,10 @@ public class ScopeAccessorTests [Fact] public void AttributeValueCodec_Encode_IntList_ProducesJsonArray() { - // Integer list elements encode via InvariantCulture IFormattable. + // Integer list elements encode as native-typed JSON numbers (NJ-1): + // [1,2,3], not the old quoted-element form ["1","2","3"]. var list = new List { 1, 2, 3 }; var encoded = AttributeValueCodec.Encode(list); - Assert.Equal("[\"1\",\"2\",\"3\"]", encoded); + Assert.Equal("[1,2,3]", encoded); } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Validation/ScriptCompilerTests.cs b/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Validation/ScriptCompilerTests.cs index e066cfcb..2f37665a 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Validation/ScriptCompilerTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Validation/ScriptCompilerTests.cs @@ -56,6 +56,31 @@ public class ScriptCompilerTests Assert.Contains("forbidden", result.Error, StringComparison.OrdinalIgnoreCase); } + [Fact] + public void TryCompile_ForbiddenTypeInAllowedNamespace_RejectedAsForbidden() + { + // System.Diagnostics is an ALLOWED namespace (Stopwatch/Debug ok), so the + // `using` directive can't be flagged; Process is a forbidden TYPE reached + // as a bare identifier. The validator's full-framework semantic resolution + // must catch it authoritatively as a forbidden API (not merely as an + // undefined-symbol compile error). + var result = _sut.TryCompile( + "using System.Diagnostics; var p = Process.Start(\"x\");", "Test"); + Assert.True(result.IsFailure); + Assert.Contains("forbidden", result.Error, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public void TryCompile_StopwatchInAllowedDiagnostics_ReturnsSuccess() + { + // The companion to the Process case: Stopwatch lives in the same allowed + // System.Diagnostics namespace and must NOT be flagged. + var result = _sut.TryCompile( + "using System.Diagnostics; var sw = Stopwatch.StartNew(); var e = sw.ElapsedMilliseconds;", + "Test"); + Assert.True(result.IsSuccess, result.IsFailure ? result.Error : null); + } + // --- Real-compile gate (the win over the old structural-only scan) --- [Fact]