diff --git a/doc/security-assessment/elsa-core-architecture-patterns.md b/doc/security-assessment/elsa-core-architecture-patterns.md index 84c63b2ed..7fe1b83d2 100644 --- a/doc/security-assessment/elsa-core-architecture-patterns.md +++ b/doc/security-assessment/elsa-core-architecture-patterns.md @@ -107,7 +107,7 @@ | Deployment stamps | ❌ | No first-class multi-region stamp deployment tooling. | | Geode | ❌ | No geo-distribution support. | | Quarantine | ❌ | No quarantine/validation gate for inbound messages or data. | -| Timeout | ✅ | `Delay`, `StartAt`, `Timer`, and `Cron` activities implement time-based waits. `CancellationToken` propagation allows workflow-level and activity-level timeout cancellation. `JintOptions` exposes a JavaScript execution timeout. | +| Timeout | ✅ | `Delay`, `StartAt`, `Timer`, and `Cron` activities implement time-based waits. `CancellationToken` propagation allows workflow-level and activity-level timeout cancellation. `JintOptions.ExecutionTimeout` bounds how long a single JavaScript expression may run (30 seconds by default), and the ambient `CancellationToken` aborts a script that is still running; `JintOptions.MaxStatements`, `MemoryLimit` and `MaxRecursionDepth` add optional resource limits on top. | --- diff --git a/doc/security-assessment/elsa-core-profile.md b/doc/security-assessment/elsa-core-profile.md index 526412da4..1b633c575 100644 --- a/doc/security-assessment/elsa-core-profile.md +++ b/doc/security-assessment/elsa-core-profile.md @@ -177,7 +177,7 @@ Elsa provides a pluggable expression evaluation system via `IExpressionHandler` | ElsaScript DSL | `Elsa.Dsl.ElsaScript` | Custom regex-based parser + compiler | | Literal / Delegate | `Elsa.Expressions` | Native .NET | -The JavaScript engine (Jint) supports optional CLR access (`AllowClrAccess`) and optional `getConfig` access for reading `IConfiguration` values; both are disabled by default for security. Python requires a Python runtime path configured via `PYTHONNET_PYDLL` or application settings. +The JavaScript engine (Jint) supports optional CLR access (`AllowClrAccess`) and optional `getConfig` access for reading `IConfiguration` values; both are disabled by default for security. Script execution is bounded by `JintOptions.ExecutionTimeout` (30 seconds by default) and by the ambient `CancellationToken`; `MaxStatements`, `MemoryLimit` and `MaxRecursionDepth` are available as additional opt-in limits. Python requires a Python runtime path configured via `PYTHONNET_PYDLL` or application settings. --- diff --git a/src/modules/Elsa.Expressions.JavaScript/Options/JintOptions.cs b/src/modules/Elsa.Expressions.JavaScript/Options/JintOptions.cs index 51f5306b5..cd2dc9906 100644 --- a/src/modules/Elsa.Expressions.JavaScript/Options/JintOptions.cs +++ b/src/modules/Elsa.Expressions.JavaScript/Options/JintOptions.cs @@ -32,6 +32,35 @@ public class JintOptions /// public bool AllowConfigurationAccess { get; set; } + /// + /// The maximum wall-clock time a single JavaScript expression may run for. Set to null to remove the limit. + /// + /// + /// Without a limit, an expression such as while (true) {} occupies the calling thread indefinitely. + /// The default is deliberately generous so that legitimate expressions are unaffected; hosts that evaluate + /// user-defined workflows are encouraged to lower it. + /// + public TimeSpan? ExecutionTimeout { get; set; } = TimeSpan.FromSeconds(30); + + /// + /// The maximum number of statements a single JavaScript expression may execute. Set to null (the default) to remove the limit. + /// + public int? MaxStatements { get; set; } + + /// + /// The maximum amount of memory, in bytes, a single JavaScript expression may allocate. Set to null (the default) to remove the limit. + /// + public long? MemoryLimit { get; set; } + + /// + /// The maximum recursion depth allowed within a single JavaScript expression. Set to null (the default) to remove the limit. + /// + /// + /// Unbounded recursion in a script exhausts the CLR stack, which cannot be recovered from. Hosts that evaluate + /// user-defined workflows are encouraged to set a limit. + /// + public int? MaxRecursionDepth { get; set; } + /// /// The timeout for script caching. /// diff --git a/src/modules/Elsa.Expressions.JavaScript/Services/JintJavaScriptEvaluator.cs b/src/modules/Elsa.Expressions.JavaScript/Services/JintJavaScriptEvaluator.cs index 15c1818cb..ec6531f0b 100644 --- a/src/modules/Elsa.Expressions.JavaScript/Services/JintJavaScriptEvaluator.cs +++ b/src/modules/Elsa.Expressions.JavaScript/Services/JintJavaScriptEvaluator.cs @@ -63,6 +63,7 @@ public class JintJavaScriptEvaluator(IConfiguration configuration, INotification ConfigureClrAccess(engineOptions); ConfigureObjectWrapper(engineOptions); ConfigureObjectConverters(engineOptions); + ConfigureExecutionConstraints(engineOptions, cancellationToken); await mediator.SendAsync(new CreatingJavaScriptEngine(engineOptions, context), cancellationToken); _jintOptions.ConfigureEngineOptionsCallback(engineOptions, context); @@ -96,6 +97,25 @@ public class JintJavaScriptEvaluator(IConfiguration configuration, INotification }); } + private void ConfigureExecutionConstraints(Jint.Options options, CancellationToken cancellationToken) + { + // An expression that never returns would otherwise occupy the calling thread forever. + if (_jintOptions.ExecutionTimeout is { } executionTimeout) + options.TimeoutInterval(executionTimeout); + + if (_jintOptions.MaxStatements is { } maxStatements) + options.MaxStatements(maxStatements); + + if (_jintOptions.MemoryLimit is { } memoryLimit) + options.LimitMemory(memoryLimit); + + if (_jintOptions.MaxRecursionDepth is { } maxRecursionDepth) + options.LimitRecursion(maxRecursionDepth); + + // Cancelling the workflow should also abort a script that is still running. + options.CancellationToken(cancellationToken); + } + private void ConfigureObjectConverters(Jint.Options options) { options.Interop.ObjectConverters.AddRange(ObjectConverters); diff --git a/test/integration/Elsa.JavaScript.IntegrationTests/ExecutionConstraintTests.cs b/test/integration/Elsa.JavaScript.IntegrationTests/ExecutionConstraintTests.cs new file mode 100644 index 000000000..8267333c3 --- /dev/null +++ b/test/integration/Elsa.JavaScript.IntegrationTests/ExecutionConstraintTests.cs @@ -0,0 +1,134 @@ +using Elsa.Expressions.JavaScript.Contracts; +using Elsa.Expressions.JavaScript.Options; +using Elsa.Expressions.Models; +using Elsa.Extensions; +using Elsa.Testing.Shared; +using Jint; +using Jint.Runtime; +using Microsoft.Extensions.DependencyInjection; +using Xunit; +using Xunit.Abstractions; + +namespace Elsa.JavaScript.IntegrationTests; + +/// +/// Verifies that a runaway JavaScript expression cannot occupy the calling thread indefinitely. +/// +public class ExecutionConstraintTests(ITestOutputHelper testOutputHelper) +{ + private const string InfiniteLoop = "while (true) {}"; + + /// + /// Tests whose subject is a constraint other than the timeout still register a timeout, so that a regression + /// in the constraint under test fails that one test instead of leaving an infinite loop running until the CI + /// job is killed. It is orders of magnitude longer than those constraints need — each of them aborts within + /// milliseconds — so it cannot itself become a source of flaky failures on a loaded machine, and + /// reports a trip as a failsafe trip rather than as a + /// confusing exception type mismatch. + /// + private static readonly TimeSpan FailsafeTimeout = TimeSpan.FromSeconds(30); + + [Fact(DisplayName = "An expression that never returns is aborted by the execution timeout")] + public async Task ExecutionTimeoutAbortsRunawayExpression() + { + // No failsafe needed: the timeout is the subject here, so the test is bounded by the thing it asserts. + var services = BuildServices(options => options.ExecutionTimeout = TimeSpan.FromMilliseconds(250)); + + await Assert.ThrowsAsync(() => EvaluateAsync(services, InfiniteLoop)); + } + + [Fact(DisplayName = "An expression that never returns is aborted when the cancellation token is signalled")] + public async Task CancellationAbortsRunawayExpression() + { + var services = BuildServices(options => options.ExecutionTimeout = FailsafeTimeout); + using var cancellationTokenSource = new CancellationTokenSource(); + + // The script signals the token itself, so cancellation is guaranteed to land after the engine has been + // built and while the expression is running. A wall-clock timer would instead race engine construction + // and could fault the setup rather than the script on a loaded machine. + await AssertAbortedByAsync(() => EvaluateAsync( + services, + "cancel(); " + InfiniteLoop, + engine => engine.SetValue("cancel", (Action)cancellationTokenSource.Cancel), + cancellationTokenSource.Token)); + } + + [Fact(DisplayName = "An expression that exceeds the statement limit is aborted")] + public async Task StatementLimitAbortsRunawayExpression() + { + var services = BuildServices(options => + { + options.ExecutionTimeout = FailsafeTimeout; + options.MaxStatements = 100; + }); + + await AssertAbortedByAsync(() => EvaluateAsync(services, InfiniteLoop)); + } + + [Fact(DisplayName = "An expression that allocates without bound is aborted by the memory limit")] + public async Task MemoryLimitAbortsAllocationHeavyExpression() + { + var services = BuildServices(options => + { + options.ExecutionTimeout = FailsafeTimeout; + options.MemoryLimit = 4 * 1024 * 1024; + }); + + // The limit is checked between statements, so the script has to allocate in visible steps. Doubling the + // string crosses any limit within a couple of dozen iterations, which keeps the test fast and bounds how + // far past the limit the process can get before the check fires. + await AssertAbortedByAsync(() => EvaluateAsync(services, "var s = 'x'; while (true) { s += s; }")); + } + + [Fact(DisplayName = "An expression that recurses without bound is aborted")] + public async Task RecursionLimitAbortsRunawayExpression() + { + var services = BuildServices(options => + { + options.ExecutionTimeout = FailsafeTimeout; + options.MaxRecursionDepth = 32; + }); + + await AssertAbortedByAsync(() => EvaluateAsync(services, "function f() { return f(); } return f();")); + } + + [Fact(DisplayName = "A well-behaved expression is unaffected by the default constraints")] + public async Task DefaultConstraintsDoNotAffectNormalExpressions() + { + var services = BuildServices(_ => { }); + + Assert.Equal("3", await EvaluateAsync(services, "return '' + (1 + 2);")); + } + + private IServiceProvider BuildServices(Action configure) + { + return new TestApplicationBuilder(testOutputHelper) + .ConfigureElsa(elsa => elsa.UseJavaScript(configure)) + .Build(); + } + + private static async Task EvaluateAsync(IServiceProvider services, string script, Action? configureEngine = null, CancellationToken cancellationToken = default) + { + var evaluator = services.GetRequiredService(); + var context = new ExpressionExecutionContext(services, new()); + + return (string?)await evaluator.EvaluateAsync(script, typeof(string), context, configureEngine: configureEngine, cancellationToken: cancellationToken); + } + + /// + /// Asserts that the expression was aborted by , distinguishing that outcome + /// from a trip of the failsafe execution timeout that guards the test against running unbounded. + /// + private static async Task AssertAbortedByAsync(Func evaluate) where TException : Exception + { + var exception = await Record.ExceptionAsync(evaluate); + + if (exception is null) + Assert.Fail($"Expected the expression to be aborted by {typeof(TException).Name}, but it ran to completion."); + + if (exception is TimeoutException) + Assert.Fail($"The {FailsafeTimeout.TotalSeconds:0} second failsafe execution timeout fired before {typeof(TException).Name} was thrown: the constraint under test did not abort the expression."); + + Assert.IsType(exception); + } +}