fix(template-engine): trigger-expression syntax check reports ALL violations/compile errors, not just the first (plan R2-05 T1)

This commit is contained in:
Joseph Doherty
2026-07-13 09:45:04 -04:00
parent 1429ddaa0e
commit 5573a9eeb9
2 changed files with 24 additions and 3 deletions
@@ -530,15 +530,17 @@ public class ValidationService
/// <returns>A human-readable error message if the expression is invalid; <c>null</c> if well-formed.</returns>
internal static string? CheckExpressionSyntax(string expression)
{
// Authoritative forbidden-API verdict first.
// Authoritative forbidden-API verdict first. Report ALL violations, not just
// the first, so an operator fixing one forbidden API isn't surprised by a
// second on the next deploy attempt (mirrors ScriptCompiler.TryCompile).
var violations = ScriptTrustValidator.FindViolations(expression);
if (violations.Count > 0)
return $"uses forbidden API: {violations[0]}";
return $"uses forbidden API: {string.Join("; ", violations)}";
// Real compile of the bare boolean expression against the trigger globals.
var errors = RoslynScriptCompiler.Compile(expression, typeof(TriggerCompileSurface));
if (errors.Count > 0)
return $"is not a valid expression: {errors[0]}";
return $"is not a valid expression: {string.Join("; ", errors)}";
return null;
}
@@ -149,6 +149,25 @@ public class ScriptCompilerTests
Assert.Contains("forbidden", error, StringComparison.OrdinalIgnoreCase);
}
[Fact]
public void CheckExpressionSyntax_MultipleForbiddenApis_ReportsAll()
{
var error = ValidationService.CheckExpressionSyntax(
"System.IO.File.Exists(\"x\") && System.Diagnostics.Process.GetProcesses().Length > 0");
Assert.NotNull(error);
Assert.Contains("System.IO", error); // FAILS today: only violations[0] is surfaced
Assert.Contains("Process", error);
}
[Fact]
public void CheckExpressionSyntax_MultipleCompileErrors_ReportsAll()
{
var error = ValidationService.CheckExpressionSyntax("NoSuchThingA > 1 && NoSuchThingB < 2");
Assert.NotNull(error);
Assert.Contains("NoSuchThingA", error); // FAILS today: only errors[0] is surfaced
Assert.Contains("NoSuchThingB", error);
}
// --- Compile-verdict cache: unchanged code compiles once per process ---
[Fact]