Bound JavaScript expression execution (#7891)
* feat(javascript): bound JavaScript expression execution
JavaScript expressions were evaluated with no execution constraints at all: no
timeout, no statement limit, no memory limit, no recursion limit, and the
ambient `CancellationToken` — already available at the call site and already
passed into `IJavaScriptEvaluator.EvaluateAsync` — was never handed to Jint.
An expression as simple as `while (true) {}` therefore occupied the calling
thread for the lifetime of the process, and cancelling the workflow did not
stop it.
This adds:
* `JintOptions.ExecutionTimeout` — wall-clock limit for a single expression,
defaulting to 30 seconds. Deliberately generous so that existing expressions
are unaffected; set to `null` to remove the limit.
* `JintOptions.MaxStatements`, `JintOptions.MemoryLimit` and
`JintOptions.MaxRecursionDepth` — opt-in resource limits, off by default.
* The cancellation token is now passed to Jint, so cancelling a workflow aborts
a script that is still running.
The security assessment documents already described a JavaScript execution
timeout as present; they now describe what is actually configurable.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0179sA2T7HuRfRfSc2JirFik
* test(javascript): bound the constraint tests and cover the memory limit
Three of the execution-constraint tests removed the execution timeout entirely
and then ran `while (true) {}`, relying solely on the constraint under test to
stop them. If that constraint regressed, the test did not fail — it ran until
the CI job was killed, taking the rest of the suite with it.
Each such test now registers a generous 30 second failsafe timeout instead of
disabling the timeout. That is two orders of magnitude more than any of these
constraints needs (the slowest aborts in ~270 ms), so it cannot become a flaky
failure on a loaded machine, and `AssertAbortedByAsync` reports a failsafe trip
as exactly that rather than as an unexplained exception type mismatch.
The cancellation test also no longer races a wall-clock timer against engine
construction: the script signals the token itself through a host function, so
cancellation is guaranteed to land while the expression is running. The test
went from a 250 ms wall-clock wait to 5 ms and has no timing dependency left.
Adds the missing `MemoryLimit` test — the one configurable limit the suite did
not exercise. Doubling a string crosses the limit within a couple of dozen
statements, so it asserts `MemoryLimitExceededException` in ~40 ms and bounds
how far past the limit the process can get before the check fires.
Finally, the `ExpressionExecutionContext` is now built on the test host's
`IServiceProvider` rather than a throwaway empty one, matching every other test
in this project. An empty provider does not reflect real evaluation and can hide
failures in notification handlers that resolve services.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0179sA2T7HuRfRfSc2JirFik
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
2e9a3ad04f
commit
66b9079d3e
|
|
@ -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. |
|
||||
|
||||
---
|
||||
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
||||
---
|
||||
|
||||
|
|
|
|||
|
|
@ -32,6 +32,35 @@ public class JintOptions
|
|||
/// </summary>
|
||||
public bool AllowConfigurationAccess { get; set; }
|
||||
|
||||
/// <summary>
|
||||
/// The maximum wall-clock time a single JavaScript expression may run for. Set to <c>null</c> to remove the limit.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Without a limit, an expression such as <c>while (true) {}</c> 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.
|
||||
/// </remarks>
|
||||
public TimeSpan? ExecutionTimeout { get; set; } = TimeSpan.FromSeconds(30);
|
||||
|
||||
/// <summary>
|
||||
/// The maximum number of statements a single JavaScript expression may execute. Set to <c>null</c> (the default) to remove the limit.
|
||||
/// </summary>
|
||||
public int? MaxStatements { get; set; }
|
||||
|
||||
/// <summary>
|
||||
/// The maximum amount of memory, in bytes, a single JavaScript expression may allocate. Set to <c>null</c> (the default) to remove the limit.
|
||||
/// </summary>
|
||||
public long? MemoryLimit { get; set; }
|
||||
|
||||
/// <summary>
|
||||
/// The maximum recursion depth allowed within a single JavaScript expression. Set to <c>null</c> (the default) to remove the limit.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// 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.
|
||||
/// </remarks>
|
||||
public int? MaxRecursionDepth { get; set; }
|
||||
|
||||
/// <summary>
|
||||
/// The timeout for script caching.
|
||||
/// </summary>
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
||||
/// <summary>
|
||||
/// Verifies that a runaway JavaScript expression cannot occupy the calling thread indefinitely.
|
||||
/// </summary>
|
||||
public class ExecutionConstraintTests(ITestOutputHelper testOutputHelper)
|
||||
{
|
||||
private const string InfiniteLoop = "while (true) {}";
|
||||
|
||||
/// <summary>
|
||||
/// 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
|
||||
/// <see cref="AssertAbortedByAsync{TException}"/> reports a trip as a failsafe trip rather than as a
|
||||
/// confusing exception type mismatch.
|
||||
/// </summary>
|
||||
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<TimeoutException>(() => 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<ExecutionCanceledException>(() => 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<StatementsCountOverflowException>(() => 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<MemoryLimitExceededException>(() => 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<RecursionDepthOverflowException>(() => 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<JintOptions> configure)
|
||||
{
|
||||
return new TestApplicationBuilder(testOutputHelper)
|
||||
.ConfigureElsa(elsa => elsa.UseJavaScript(configure))
|
||||
.Build();
|
||||
}
|
||||
|
||||
private static async Task<string?> EvaluateAsync(IServiceProvider services, string script, Action<Engine>? configureEngine = null, CancellationToken cancellationToken = default)
|
||||
{
|
||||
var evaluator = services.GetRequiredService<IJavaScriptEvaluator>();
|
||||
var context = new ExpressionExecutionContext(services, new());
|
||||
|
||||
return (string?)await evaluator.EvaluateAsync(script, typeof(string), context, configureEngine: configureEngine, cancellationToken: cancellationToken);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Asserts that the expression was aborted by <typeparamref name="TException"/>, distinguishing that outcome
|
||||
/// from a trip of the failsafe execution timeout that guards the test against running unbounded.
|
||||
/// </summary>
|
||||
private static async Task AssertAbortedByAsync<TException>(Func<Task> 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<TException>(exception);
|
||||
}
|
||||
}
|
||||
Loading…
Reference in a new issue