fix(triggers): use explicit ValidationCategory + tighten expression syntax validation

This commit is contained in:
Joseph Doherty
2026-05-16 05:57:39 -04:00
parent bf3f572ad9
commit 5065384305
2 changed files with 69 additions and 34 deletions

View File

@@ -12,8 +12,13 @@ namespace ScadaLink.TemplateEngine.Validation;
/// </summary> /// </summary>
public class ScriptCompiler public class ScriptCompiler
{ {
// Forbidden namespace patterns - scripts must not use these /// <summary>
private static readonly string[] ForbiddenPatterns = /// Forbidden namespace patterns — scripts (and trigger expressions, via
/// <see cref="ValidationService"/>) must not use these. Trigger expressions run
/// under the same trust model as scripts, so the list is shared from here rather
/// than duplicated.
/// </summary>
internal static readonly string[] ForbiddenPatterns =
[ [
"System.IO.", "System.IO.",
"System.Diagnostics.Process", "System.Diagnostics.Process",

View File

@@ -22,21 +22,6 @@ public class ValidationService
private readonly SemanticValidator _semanticValidator; private readonly SemanticValidator _semanticValidator;
private readonly ScriptCompiler _scriptCompiler; private readonly ScriptCompiler _scriptCompiler;
/// <summary>
/// Forbidden namespace patterns for trigger expressions. Mirrors the list in
/// <see cref="ScriptCompiler"/> — trigger expressions run under the same trust
/// model as scripts.
/// </summary>
private static readonly string[] ForbiddenExpressionPatterns =
[
"System.IO.",
"System.Diagnostics.Process",
"System.Threading.",
"System.Reflection.",
"System.Net.Sockets.",
"System.Net.Http.",
];
public ValidationService(SemanticValidator semanticValidator, ScriptCompiler scriptCompiler) public ValidationService(SemanticValidator semanticValidator, ScriptCompiler scriptCompiler)
{ {
_semanticValidator = semanticValidator; _semanticValidator = semanticValidator;
@@ -224,7 +209,8 @@ public class ValidationService
continue; continue;
CheckExpressionTrigger( CheckExpressionTrigger(
"Script", script.CanonicalName, script.TriggerConfiguration, ValidationCategory.ScriptTriggerReference, "script",
script.CanonicalName, script.TriggerConfiguration,
attributeNames, errors, warnings); attributeNames, errors, warnings);
} }
@@ -234,7 +220,8 @@ public class ValidationService
continue; continue;
CheckExpressionTrigger( CheckExpressionTrigger(
"Alarm", alarm.CanonicalName, alarm.TriggerConfiguration, ValidationCategory.AlarmTriggerReference, "alarm",
alarm.CanonicalName, alarm.TriggerConfiguration,
attributeNames, errors, warnings); attributeNames, errors, warnings);
} }
@@ -248,8 +235,20 @@ public class ValidationService
/// Runs the blank / syntax / attribute-reference checks for a single /// Runs the blank / syntax / attribute-reference checks for a single
/// Expression-trigger entity and appends any findings to the shared lists. /// Expression-trigger entity and appends any findings to the shared lists.
/// </summary> /// </summary>
/// <param name="category">
/// The <see cref="ValidationCategory"/> to file every finding under
/// (<see cref="ValidationCategory.ScriptTriggerReference"/> for scripts,
/// <see cref="ValidationCategory.AlarmTriggerReference"/> for alarms). The same
/// category is used for blank, syntax, and attribute-reference findings so an
/// alarm's syntax error is not miscategorised as script compilation.
/// </param>
/// <param name="entityLabel">
/// Human-readable entity-type label (<c>"script"</c>/<c>"alarm"</c>) used in
/// message text only.
/// </param>
private static void CheckExpressionTrigger( private static void CheckExpressionTrigger(
string entityType, ValidationCategory category,
string entityLabel,
string entityName, string entityName,
string? triggerConfigJson, string? triggerConfigJson,
HashSet<string> attributeNames, HashSet<string> attributeNames,
@@ -260,11 +259,8 @@ public class ValidationService
if (string.IsNullOrWhiteSpace(expression)) if (string.IsNullOrWhiteSpace(expression))
{ {
warnings.Add(ValidationEntry.Warning( warnings.Add(ValidationEntry.Warning(category,
entityType == "Alarm" $"The {entityLabel} '{entityName}' has an expression trigger with no expression; it will never fire.",
? ValidationCategory.AlarmTriggerReference
: ValidationCategory.ScriptTriggerReference,
$"{entityType} '{entityName}' has an expression trigger with no expression; it will never fire.",
entityName)); entityName));
return; return;
} }
@@ -272,8 +268,8 @@ public class ValidationService
var syntaxError = CheckExpressionSyntax(expression); var syntaxError = CheckExpressionSyntax(expression);
if (syntaxError != null) if (syntaxError != null)
{ {
errors.Add(ValidationEntry.Error(ValidationCategory.ScriptCompilation, errors.Add(ValidationEntry.Error(category,
$"{entityType} '{entityName}' expression trigger failed validation: {syntaxError}", $"The {entityLabel} '{entityName}' expression trigger failed validation: {syntaxError}",
entityName)); entityName));
} }
@@ -281,11 +277,8 @@ public class ValidationService
{ {
if (!attributeNames.Contains(attrName)) if (!attributeNames.Contains(attrName))
{ {
errors.Add(ValidationEntry.Error( errors.Add(ValidationEntry.Error(category,
entityType == "Alarm" $"The {entityLabel} '{entityName}' expression trigger references attribute '{attrName}' which does not exist in the flattened configuration.",
? ValidationCategory.AlarmTriggerReference
: ValidationCategory.ScriptTriggerReference,
$"{entityType} '{entityName}' expression trigger references attribute '{attrName}' which does not exist in the flattened configuration.",
entityName)); entityName));
} }
} }
@@ -302,9 +295,12 @@ public class ValidationService
try try
{ {
using var doc = JsonDocument.Parse(triggerConfigJson); using var doc = JsonDocument.Parse(triggerConfigJson);
if (doc.RootElement.TryGetProperty("expression", out var prop)) if (doc.RootElement.TryGetProperty("expression", out var prop)
&& prop.ValueKind == JsonValueKind.String)
{
return prop.GetString(); return prop.GetString();
} }
}
catch (JsonException) catch (JsonException)
{ {
// Not valid JSON — treated as a blank expression by the caller. // Not valid JSON — treated as a blank expression by the caller.
@@ -321,7 +317,7 @@ public class ValidationService
/// </summary> /// </summary>
internal static string? CheckExpressionSyntax(string expression) internal static string? CheckExpressionSyntax(string expression)
{ {
foreach (var pattern in ForbiddenExpressionPatterns) foreach (var pattern in ScriptCompiler.ForbiddenPatterns)
{ {
if (expression.Contains(pattern, StringComparison.Ordinal)) if (expression.Contains(pattern, StringComparison.Ordinal))
{ {
@@ -335,10 +331,29 @@ public class ValidationService
var braceDepth = 0; var braceDepth = 0;
var inString = false; var inString = false;
var inChar = false; var inChar = false;
var inLineComment = false;
var inBlockComment = false;
for (int i = 0; i < expression.Length; i++) for (int i = 0; i < expression.Length; i++)
{ {
var c = expression[i]; var c = expression[i];
var next = i + 1 < expression.Length ? expression[i + 1] : '\0';
if (inLineComment)
{
if (c == '\n') inLineComment = false;
continue;
}
if (inBlockComment)
{
if (c == '*' && next == '/')
{
inBlockComment = false;
i++;
}
continue;
}
if (inString) if (inString)
{ {
@@ -354,6 +369,20 @@ public class ValidationService
continue; continue;
} }
if (c == '/' && next == '/')
{
inLineComment = true;
i++;
continue;
}
if (c == '/' && next == '*')
{
inBlockComment = true;
i++;
continue;
}
switch (c) switch (c)
{ {
case '"': inString = true; break; case '"': inString = true; break;
@@ -376,6 +405,7 @@ public class ValidationService
} }
} }
if (inBlockComment) return "unterminated block comment.";
if (inString) return "unterminated string literal."; if (inString) return "unterminated string literal.";
if (inChar) return "unterminated character literal."; if (inChar) return "unterminated character literal.";
if (parenDepth != 0) return $"mismatched parentheses ({parenDepth} unclosed)."; if (parenDepth != 0) return $"mismatched parentheses ({parenDepth} unclosed).";