dohertj2/ScadaBridge
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix(scriptanalysis): M3.6 — full-framework analysis refs close forbidden-type-in-allowed-ns blind spot; pin Process/Stopwatch; fix stale codec test; drop dead ContainsInCode

Browse Source
This commit is contained in:
Joseph Doherty
2026-06-16 20:00:28 -04:00
parent cf935d5744
commit 069757209a
6 changed files with 121 additions and 127 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/ZB.MOM.WW.ScadaBridge.ScriptAnalysis/ScriptTrustPolicy.cs
+62
View File
@@ -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();
/// <summary>
/// The full trusted-platform reference set used ONLY by
/// <see cref="ScriptTrustValidator"/>'s semantic analysis — NOT by
/// <see cref="RoslynScriptCompiler"/>. Unlike <see cref="DefaultReferences"/>
/// (the minimal, runtime-fidelity set used to decide script <i>validity</i>,
/// 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 <c>System.Diagnostics.Process</c> via
/// <c>using System.Diagnostics;</c> — 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.
/// </summary>
public static readonly IReadOnlyList<MetadataReference> AnalysisReferences = BuildAnalysisReferences();
private static IReadOnlyList<MetadataReference> BuildAnalysisReferences()
{
var byPath = new Dictionary<string, MetadataReference>(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;
}
/// <summary>
/// Default namespace imports made available to compiled scripts.
/// </summary>
src/ZB.MOM.WW.ScadaBridge.ScriptAnalysis/ScriptTrustValidator.cs
+6 -1
View File
@@ -79,7 +79,12 @@ public static class ScriptTrustValidator
var violations = new SortedSet<string>(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);
src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/CSharpDelimiterScanner.cs
+8 -124
View File
@@ -2,12 +2,11 @@ namespace ZB.MOM.WW.ScadaBridge.TemplateEngine.Validation;
/// <summary>
/// String/comment-aware scanner for the balanced-delimiter ("does it look like
/// valid C#") checks used by <see cref="ScriptCompiler"/> and
/// <c>SharedScriptService.ValidateSyntax</c>.
/// valid C#") check used by <c>SharedScriptService.ValidateSyntax</c>.
///
/// <para>
/// This is <b>not</b> a compiler. It is an interim structural check that walks
/// the source once and tracks <c>{}</c>, <c>[]</c> and <c>()</c> depth while
/// This is <b>not</b> a compiler. It is a structural check that walks the
/// source once and tracks <c>{}</c>, <c>[]</c> and <c>()</c> depth while
/// correctly skipping over the C# lexical constructs in which a delimiter is
/// inert: line/block comments, regular string literals (with <c>\</c> escapes),
/// verbatim strings (<c>@"..."</c>, where <c>""</c> escapes a quote and <c>\</c>
@@ -17,11 +16,11 @@ namespace ZB.MOM.WW.ScadaBridge.TemplateEngine.Validation;
/// </para>
///
/// <para>
/// It is intentionally conservative: when the real Roslyn-based compiler is
/// wired in (see <see cref="ScriptCompiler"/>) this hand-rolled scan should be
/// replaced by <c>CSharpSyntaxTree.ParseText</c> 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 <see cref="ScriptCompiler"/> (which delegates to the shared
/// <c>ZB.MOM.WW.ScadaBridge.ScriptAnalysis</c> validator + Roslyn compile). This
/// scanner only provides <c>SharedScriptService</c> a cheap pre-compile sanity
/// check for balanced delimiters.
/// </para>
/// </summary>
internal static class CSharpDelimiterScanner
@@ -41,121 +40,6 @@ internal static class CSharpDelimiterScanner
UnterminatedChar,
}
/// <summary>
/// Returns true when <paramref name="pattern"/> occurs in a <b>code</b>
/// region of <paramref name="code"/> — i.e. not wholly inside a string
/// literal, char literal, or comment. Used by the interim forbidden-API
/// scan so that the inert text <c>System.IO.</c> in a comment or string
/// literal is not flagged as a forbidden API call (TemplateEngine-006).
///
/// <para>
/// This removes the false-positive half of the substring scan. It does
/// <b>not</b> close the bypass half: namespace aliases, <c>using static</c>,
/// and <c>global::</c>-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.
/// </para>
/// </summary>
/// <param name="code">The C# source code to scan.</param>
/// <param name="pattern">The substring to search for in code regions only.</param>
/// <returns><c>true</c> if <paramref name="pattern"/> occurs in a code region (not inside a comment, string, or char literal); otherwise <c>false</c>.</returns>
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);
}
/// <summary>
/// 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.
/// </summary>
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);
}
/// <summary>
/// Walks <paramref name="code"/> once and reports the first structural
/// delimiter problem, or <see cref="Mismatch.None"/> when the source is
tests/ZB.MOM.WW.ScadaBridge.ScriptAnalysis.Tests/ScriptTrustValidatorTests.cs
+17
View File
@@ -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]
tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScopeAccessorTests.cs
+3 -2
View File
@@ -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<int> { 1, 2, 3 };
var encoded = AttributeValueCodec.Encode(list);
Assert.Equal("[\"1\",\"2\",\"3\"]", encoded);
Assert.Equal("[1,2,3]", encoded);
}
}
tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Validation/ScriptCompilerTests.cs
+25
View File
@@ -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]