fix: bound exitCondition regex matches on the caller thread

[NaomiiAP]

closes #450

Pathological exitCondition patterns no longer block the evaluator or leak ForkJoinPool.commonPool threads; all triggers share a deadline CharSequence helper with substring fallback.

What changes are being made and why?

ScriptTrigger / CommandsTrigger compile user exitCondition as a regex and match it on every poll. Catastrophic backtracking never throws, so:

• Unguarded modules stayed stuck in Matcher.find() on the evaluating thread
• Ruby/Perl used CompletableFuture with a 5s timeout, but the match kept running on commonPool at 100% CPU after each poll

This PR adds ExitConditionRegex in plugin-script (same deadline-CharSequence approach as Kestra core RegexUtils, shimmed while we target 1.3.x). Every trigger uses it. Matching runs on the caller thread with a 1s deadline, then falls back to substring on timeout. Ruby/Perl move from a 5s to a 1s matching budget and no longer submit regex work to commonPool.

Additional scope and deadline limitation

• The Python, Node and Bash timer assertions use Duration.toNanos() instead of getNano(). The latter returns only the fractional-second component and can incorrectly report zero for a positive whole-second duration. These are unrelated flaky-test fixes included in this PR.
• The 1s regex deadline is cooperative: it is checked every 1024 CharSequence.charAt calls. Regex work that does not read characters (including pattern compilation) is not interrupted by this guard. This matches the approach used by core RegexUtils; it is not a hard wall-clock limit on every regex operation.

How the changes have been QAed?

Unit tests cover the timeout path with (.*a){20}$ (not (a+)+$, which JDK 21+/25 memoizes). Helper, Perl, Ruby, Go, Bun, Deno, .NET, PowerShell, and R condition tests passed.

Repro flow (CPU should stay flat after the fix; previously climbed ~1 core per poll):

id: exit_condition_redos_repro
namespace: company.team

triggers:
  - id: redos
    type: io.kestra.plugin.scripts.shell.ScriptTrigger
    interval: PT10S
    exitCondition: "(.*a){20}$"
    edge: true
    script: |
      echo '::{"outputs":{"k":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!"}}::'

tasks:
  - id: log
    type: io.kestra.plugin.core.log.Log
    message: "should never fire: {{ trigger.vars }}"

Watch for ~1 minute:

docker stats --no-stream <kestra-container>
docker exec <kestra-container> sh -c "top -H -b -n1 | head -20"

Expected: trigger does not fire; no rising ForkJoinPool.commonPool-worker-* threads at 100% CPU.

Contributor Checklist

• I have read and followed the plugin contribution guidelines

[fdelbrayelle]

The approach is sound: the deadline CharSequence runs on the caller thread, so the commonPool leak is gone and the bound is real. It meets #450. One regression and a few smaller points.

Needs a fix

1. StackOverflowError now escapes evaluate() in Ruby and Perl.
ExitConditionRegex.java:46-52 (find(Pattern, String, Duration)). Patterns like (a|b)* on a long haystack overflow the stack inside java.util.regex. The old CompletableFuture.supplyAsync(...).get() wrapped that error in an ExecutionException, and catch (Exception) fell back to substring. Now only RegexTimeoutException is caught. The haystack is script output, so a user-supplied condition can trigger it.

Fix: use the same fallback as the timeout.

} catch (RegexTimeoutException | StackOverflowError e) {
    return haystack.contains(pattern.pattern());
}

Add a test with (a|b)*c against a 100k character haystack.

Suggestions

2. The evaluator thread can still block for 5s per poll (ExitConditionRegex.java:21, TIMEOUT). A pathological pattern holds the thread for 5s on every poll. Consider ~1s (exit conditions run against a few KB of output), or document the limit.

3. Timeout tests are timing-sensitive (ExitConditionRegexTest catastrophicPattern_*, Perl ScriptTriggerConditionTest / CommandsTriggerConditionTest). assertTimeoutPreemptively(timeout.multipliedBy(2)) leaves a 500ms margin. Perl also asserts elapsedMs >= 4000 against a 10s limit. Use a 3x to 4x margin, or assert only the fallback result and a lower bound on elapsed time.

4. Only Perl and the helper have timeout-path tests. Add one catastrophic-pattern assertion in Ruby and one in a cached-pattern module such as Bun or R.

5. TimeoutCharSequence.subSequence restarts the counter (:88-91). Harmless today. Optionally share the counter.

Nits

6. Unrelated formatting churn: import reordering in node/python/r triggers, record ExtractedFailure reflow, @CsvSource and assertThat reflows in Perl tests, the )@Plugin( fix in ruby/CommandsTrigger, joined EXIT_CONDITION_PATTERN lines. Consider a separate commit.

7. Javadoc on ExitConditionRegex is multi-line. One short line is enough.

8. Perl imports ExitConditionRegexTest only for a static helper. A small shared test support class would be cleaner.

[NaomiiAP]

Thanks for the review @fdelbrayelle , addressed in the follow-up commits.

1. Catching StackOverflowError alongside RegexTimeoutException and falling back to substring; added (a|b)*c vs a 100k-char haystack test.
2. Match budget reduced to 1s and documented on ExitConditionRegex.
3. Timeout tests now use a 4× ceiling plus a softer lower bound on elapsed time (helper + Perl).
4. Added catastrophic-pattern coverage for Ruby and Bun (cached conditionPattern path).
5. TimeoutCharSequence.subSequence now shares the counter with the parent.
6. Dropped the unrelated formatting churn so the PR stays focused on the functional change.
7. Javadoc shortened.
8. Moved assertNoRegexOnCommonPool into ExitConditionRegexTestSupport so Perl no longer imports the test class.

[fdelbrayelle]

Review: changes needed

The helper is sound: caller thread, deadline enforced through a CharSequence wrapper, PatternSyntaxException and StackOverflowError handled, and the Ruby/Perl commonPool leak is gone. But the migration is incomplete.

Must fix

1. jbang still uses the unguarded match (ReDoS stays open):

   • plugin-script-jbang/src/main/java/io/kestra/plugin/scripts/jbang/ScriptTrigger.java:265
   • plugin-script-jbang/src/main/java/io/kestra/plugin/scripts/jbang/CommandsTrigger.java:237

   Replace Pattern.compile(cond).matcher(haystack).find() with ExitConditionRegex.find(cond, haystack).

2. lua is not migrated (landed in #447, already on main):

   • plugin-script-lua/src/main/java/io/kestra/plugin/scripts/lua/ScriptTrigger.java:241
   • plugin-script-lua/src/main/java/io/kestra/plugin/scripts/lua/CommandsTrigger.java:246

   Use ExitConditionRegex.find(conditionPattern(cond), haystack).

3. Tests: add one catastrophic-pattern timeout test for jbang and one for lua. I also found no timeout tests for Bun CommandsTrigger, R, PowerShell, .NET or Node.

Should fix

• PR body says matching stops after 5s, but TIMEOUT is 1s. Ruby and Perl go from a 5s to a 1s budget, so the body should say so.
• ExitConditionRegex.java:48 falls back to substring matching silently. Log a warn naming the exitCondition (also for the invalid-regex fallback).
• Too many comments and javadoc blocks in ExitConditionRegex.java, ExitConditionRegexTestSupport.java and the Bun test. Keep one short line at most.
• Go ScriptTriggerTest re-implements exitConditionMatches instead of calling production matchesCondition. Make it package-private and test it directly.
• ExitConditionRegexTestSupport.assertNoRegexOnCommonPool uses Thread.sleep(200) and a global thread scan, which can be flaky. The caller-thread design makes it mostly redundant, so consider dropping it.

Nit

• CONDITION_PATTERNS in the Bun, R and Lua triggers is an unbounded static cache. Not a regression.

[fdelbrayelle]

Review: no blocking findings

The ReDoS fix meets issue #450. Regex now runs on the caller thread with a 1s deadline, so the commonPool leak is gone. Invalid regex and stack overflow fall back to a substring match. I did not run builds or tests.

Suggestions (non-blocking)

1. Slow and possibly flaky tests. CommandsTriggerConditionTest.java:15 and the same test in Bun, Deno, .NET, jbang, Lua, Ruby, R and Go repeat ExitConditionRegexTest. They use the real 1s deadline and a >= 500ms check. Keep one wiring test per trigger, make the deadline injectable, and drop the elapsed-time lower bound.
2. Log noise. ExitConditionRegex.java:84-87: a bad exitCondition burns 1s CPU and logs a WARN on every poll. Cache known-bad patterns, or WARN once and then DEBUG.
3. Unrelated change. PythonTest.java:262 and ScriptTest.java:306 switch getNano() to toNanos(). It is a valid flaky-test fix, but please mention it in the PR description or split it out.
4. Deadline limit. The deadline is checked every 1024 charAt calls, so a pattern that does not read characters is not interrupted. Acceptable and consistent with core RegexUtils, but worth a note in the PR.
5. Comments. Remove the leftover Javadoc in dotnet ScriptTriggerTest.java:17 and the one on ExitConditionRegex.java:12.

[fdelbrayelle]

Review: looks good, suggestions only

No blocking issues. The core fix is sound:

• Regex matching runs on the caller thread with a 1s deadline, so nothing is left on ForkJoinPool.commonPool.
• StackOverflowError falls back to substring matching.
• All 27 call sites go through ExitConditionRegex.
• Issue #450 requirements are met. No security or performance findings.

Check first

1. The PR shows "Checks 0/1 passed". Please confirm it is green or explain the failure before merge.

Suggestions

2. The wiring tests use mockStatic, so they only prove each trigger calls the helper. Consider one non-mocked (.*a){20}$ test per module family, wrapped in assertTimeoutPreemptively.
3. The Duration.toNanos() fixes in NodeTest, PythonTest, ScriptTest and AbstractBashTest are correct but unrelated. A separate PR would be easier to review and revert.
4. Go matchesCondition changed from private to package-private only for tests. Low impact, and it matches other modules.
5. A pathological pattern still blocks the evaluating thread for 1s per poll. This is documented in the PR body. Optionally cache the compiled Pattern, or skip regex after the first timeout.

Nits

6. Bun and R use catch (Exception invalidRegex) around ExitConditionRegex.find. Catch PatternSyntaxException only, or compile in the try and call find outside it.
7. timeoutMatchesTheDocumentedMatchBudget only asserts a constant equals itself. It can be dropped.
8. matchesCondition and buildHaystack are duplicated across about 27 triggers. A follow-up could move them into a shared base class.

[fdelbrayelle]

Edition: OSS | Docker tag: develop (image kestra/kestra:develop, plugin JARs built from branch fix/exit-condition-redos-timeout)
Instance: http://pvkm1.kestra.docker.localhost:1355/ (left running for manual review, docker rm -f kestra-pvkm1 to stop)
Screenshots: https://claude.ai/artifact/6YjTqsKjzb3bkkr9nnV7wy (private artifact, owner access only)

QA summary

Flow	Expected	Result
exit_condition_redos_repro	never fires, CPU stays bounded	✅ 0 executions, CPU bursts end after about 1s
exit_condition_fallback	fires via substring fallback	✅ fired every poll, SUCCESS
exit_condition_regex_match	fires on normal regex	✅ SUCCESS
exit_condition_no_match	never fires	✅ 0 executions
exit_condition_edge_once	fires once with edge: true	⚠️ fired on every poll (see note)
exit_condition_commands	CommandsTrigger fires on normal regex	✅ SUCCESS

Observation window: about 6 minutes, interval: PT10S, all 6 flows running together.

ReDoS CPU check

• Pathological regex (.*a){20}$ (redos_repro and fallback flows, polling every 10s).
• Sampled top -H -b -n1 inside the container every 1s for 40s: 35 samples had 0 threads above 50% CPU. 5 samples caught hot threads (1 to 2 in four of them, 10 in one), each time back to 0 on the next sample. Single top samples are noisy, so treat the counts as rough. Bursts are consistent with the 1s deadline.
• 0 ForkJoinPool.commonPool threads at any point. Thread count did not grow.
• Not done: baseline run on main to compare against the old behavior.

Note on edge: true

exit_condition_edge_once produced a new execution on every poll (17 executions in about 3 minutes), although the output never changed. The PR does not touch edge handling (the shell diff only swaps the regex call for ExitConditionRegex.find), so this looks pre-existing and unrelated, but I did not verify it on main. Worth a separate issue if edge is expected to fire once.

Flows

Flow 1: exit_condition_redos_repro (✅ no execution, as expected)

Flow YAML

id: exit_condition_redos_repro
namespace: company.team

triggers:
  - id: t
    type: io.kestra.plugin.scripts.shell.ScriptTrigger
    interval: PT10S
    exitCondition: '(.*a){20}$'
    edge: true
    script: |
      echo '::{"outputs":{"k":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!"}}::'

tasks:
  - id: log
    type: io.kestra.plugin.core.log.Log
    message: "fired: {{ trigger.vars }}"

**Executions** ([screenshot](https://claude.ai/artifact/6YjTqsKjzb3bkkr9nnV7wy#exit_condition_redos_repro-executions)): none created. **Logs**: no warnings or errors related to the trigger.

Flow 2: exit_condition_fallback (✅ SUCCESS)

Flow YAML

id: exit_condition_fallback
namespace: company.team

triggers:
  - id: t
    type: io.kestra.plugin.scripts.shell.ScriptTrigger
    interval: PT10S
    exitCondition: '(.*a){20}$'
    edge: true
    script: |
      echo '::{"outputs":{"k":"(.*a){20}$ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!"}}::'

tasks:
  - id: log
    type: io.kestra.plugin.core.log.Log
    message: "fired: {{ trigger.vars }}"

**Gantt** ([screenshot](https://claude.ai/artifact/6YjTqsKjzb3bkkr9nnV7wy#exit_condition_fallback-gantt)) | Task | Status | Duration | |------|--------|----------| | `log` | SUCCESS | 0.03s | | **Total** | **SUCCESS** | **0.05s** |

Logs synthesis: Flow started, log completed, flow SUCCESS. No errors.
Outputs synthesis: trigger.vars: exitCode: 0, condition: (.*a){20}$, vars.k contains the literal condition text, so the substring fallback matched.

Flow 3: exit_condition_regex_match (✅ SUCCESS)

Flow YAML

id: exit_condition_regex_match
namespace: company.team

triggers:
  - id: t
    type: io.kestra.plugin.scripts.shell.ScriptTrigger
    interval: PT10S
    exitCondition: 'ok.*done'
    edge: true
    script: |
      echo '::{"outputs":{"k":"ok all done"}}::'

tasks:
  - id: log
    type: io.kestra.plugin.core.log.Log
    message: "fired: {{ trigger.vars }}"

**Gantt** ([screenshot](https://claude.ai/artifact/6YjTqsKjzb3bkkr9nnV7wy#exit_condition_regex_match-gantt)) | Task | Status | Duration | |------|--------|----------| | `log` | SUCCESS | 0.05s | | **Total** | **SUCCESS** | **0.05s** |

Logs synthesis: Clean run, no warnings.
Outputs synthesis: vars.k = "ok all done", exitCode: 0.

Flow 4: exit_condition_no_match (✅ no execution, as expected)

Flow YAML

id: exit_condition_no_match
namespace: company.team

triggers:
  - id: t
    type: io.kestra.plugin.scripts.shell.ScriptTrigger
    interval: PT10S
    exitCondition: 'zzz[0-9]+'
    edge: true
    script: |
      echo '::{"outputs":{"k":"ok all done"}}::'

tasks:
  - id: log
    type: io.kestra.plugin.core.log.Log
    message: "fired: {{ trigger.vars }}"

**Executions** ([screenshot](https://claude.ai/artifact/6YjTqsKjzb3bkkr9nnV7wy#exit_condition_no_match-executions)): none created.

Flow 5: exit_condition_edge_once (⚠️ SUCCESS, fires every poll)

Flow YAML

id: exit_condition_edge_once
namespace: company.team

triggers:
  - id: t
    type: io.kestra.plugin.scripts.shell.ScriptTrigger
    interval: PT10S
    exitCondition: 'ok.*done'
    edge: true
    script: |
      echo '::{"outputs":{"k":"ok all done"}}::'

tasks:
  - id: log
    type: io.kestra.plugin.core.log.Log
    message: "fired: {{ trigger.vars }}"

**Gantt** ([screenshot](https://claude.ai/artifact/6YjTqsKjzb3bkkr9nnV7wy#exit_condition_edge_once-gantt)) | Task | Status | Duration | |------|--------|----------| | `log` | SUCCESS | 0.07s | | **Total** | **SUCCESS** | **0.07s** |

Logs synthesis: Clean runs, one execution per poll.
Outputs synthesis: vars.k = "ok all done".

Flow 6: exit_condition_commands (✅ SUCCESS)

Flow YAML

id: exit_condition_commands
namespace: company.team

triggers:
  - id: t
    type: io.kestra.plugin.scripts.shell.CommandsTrigger
    interval: PT10S
    exitCondition: 'ok.*done'
    edge: true
    commands:
      - echo '::{"outputs":{"k":"ok all done"}}::'

tasks:
  - id: log
    type: io.kestra.plugin.core.log.Log
    message: "fired: {{ trigger.vars }}"

**Gantt** ([screenshot](https://claude.ai/artifact/6YjTqsKjzb3bkkr9nnV7wy#exit_condition_commands-gantt)) | Task | Status | Duration | |------|--------|----------| | `log` | SUCCESS | 0.04s | | **Total** | **SUCCESS** | **0.04s** |

Logs synthesis: Clean run, no warnings.
Outputs synthesis: vars.k = "ok all done", exitCode: 0.

Scope and timeouts

• Only the shell ScriptTrigger and CommandsTrigger were exercised. Other languages covered by the PR (Ruby, Perl, Go, Bun, Deno, .NET, PowerShell, R) were not tested here.
• No scenario hit the 30s timeout.
• Topology view: out of scope (no UI or artifact change).