fix(#5886): bound SQL MATCHES and openCypher =~ against catastrophic regex backtracking

[lvca]

Summary

Fixes #5886. java.util.regex never polls an interrupt flag or checks a deadline while backtracking - the only thing it calls on every backtracking step is CharSequence.charAt() on the input. A pathological pattern like (.*a){20}$ against a 41-character string triggers catastrophic backtracking and is still running 30+ seconds later, and arcadedb.command.timeout cannot stop it. Anyone who can reach a regex-based entry point with an attacker-controlled pattern - or who forwards user-supplied patterns from an application layer - can permanently tie up a worker thread with a handful of such operations.

What started as a fix for SQL MATCHES and openCypher =~ (the two entry points named in the issue) grew, through the review process on this PR, into an audit of every user-reachable regex call site in the engine.

Fix

TimeBoundRegex (new, com.arcadedb.utility) wraps the matched input in a CharSequence whose charAt() throws once a deadline elapses - the one interception point java.util.regex offers, for matches(), replaceAll(), and split() alike. The deadline comes from a new dedicated setting, arcadedb.command.regexTimeout (default 1000ms, per-database), independent of arcadedb.command.timeout and active even when that setting is left at its own default of disabled (0). The deadline is checked every 256 charAt() calls (a bitmask check) rather than every call, keeping it off the hot path for the overwhelming majority of patterns that never come close to that many backtracking steps.

Covered entry points:

• SQL MATCHES and openCypher =~
• SQL LIKE/ILIKE, including the native query engine's SelectOperator path (a sequence of unparenthesized .*/* segments reproduces the same catastrophic shape without any grouping/alternation/nested quantifiers - the original "LIKE is structurally safe" audit conclusion was wrong)
• text.regexReplace() and the .normalize() SQL method's optional pattern argument
• Full-text search's Lucene RegexpQuery (/pattern/) and WildcardQuery (*/?) support
• PromQL's =~/!~ label matchers, including every step of a range query
• Schema-level REGEXP property validation (CREATE PROPERTY ... REGEXP <pattern>) - the most exposed of the group, since it runs on every insert/update of a validated property through any write path (REST, any wire protocol) with no query privileges needed

.split(delimiter)'s SQL method got a different fix: unlike its three siblings (split() function, text.split(), Cypher split()), which all treat the delimiter as a literal via Pattern.quote(...), it passed the delimiter straight to String.split(regex) unescaped. Matched to its siblings, removing the risk entirely rather than just bounding it.

Deadline sharing. A regex evaluated once per row/token/item/property over a scan must not get a fresh timeout budget per item, or a table/index/document shaped so every item triggers catastrophic backtracking costs itemCount * regexTimeout instead of one bounded operation. MATCHES, =~, text.regexReplace(), and .normalize() share one deadline across an entire query execution via a new CommandContext.getOrComputeRegexDeadline() helper; full-text and schema REGEXP validation share one deadline across the whole scan/document; PromQL shares one deadline across an entire query's lifetime, including every step of a range query. LIKE/ILIKE are the one exception: BinaryCompareOperator has no CommandContext to cache a deadline on, and widening that interface was judged out of proportion here, so each row gets its own budget.

Upgrade note. Because the deadline is shared per query/scan rather than per match, and defaults to an enabled 1000ms, a legitimate non-catastrophic operation that previously ran slowly to completion can now fail with TimeoutException instead. Raise arcadedb.command.regexTimeout for any database with large-scan regex-heavy workloads.

Full detail in docs/release-26.9.1.md.

Testing

Dedicated TimeBoundRegexTest for the utility, plus regression tests at every covered entry point (MatchesConditionTest, OpenCypherWhereClauseTest, QueryHelperLikeRegexTimeoutTest, SelectExecutionTest, LikeOperatorTest/ILikeOperatorTest, TextRegexReplaceTest, SQLMethodNormalizeTest, SQLMethodSplitTest, FullTextQueryExecutorTest, PromQLEvaluatorIntegrationTest, DocumentValidationTest), each covering "matches/replaces normally", "catastrophic pattern is aborted near the deadline", and (where sharing applies) "one budget is shared across N rows/items/steps/properties".

Two real bugs were caught and fixed by the review process, both with regression tests that fail without the fix:

• A deadline cache key colliding with an existing pattern-cache key namespace for a specific literal pattern text (MatchesConditionTest#patternTextDeadlineDoesNotCollideWithTheDeadlineCacheKey).
• Caching a deadline as an instance field on a Cypher AST node reused across executions via CypherStatementCache, causing spurious timeouts on later benign executions of the same cached query (OpenCypherWhereClauseTest#regexDeadlineDoesNotLeakAcrossCachedQueryExecutions).

Test plan

• mvn -pl engine compile / test-compile clean
• Full affected test suite green, no regressions
• New regression tests prove the abort happens near the configured deadline and that shared-deadline budgets hold across rows/items/steps/properties

[mergify]

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

• Queue this pull request

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 13 complexity

Metric	Results
Complexity	13

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[claude]

Review: bound SQL MATCHES / openCypher =~ against catastrophic regex backtracking (#5886)

Solid, well-scoped fix. Wrapping the matched CharSequence so charAt() becomes the interruption point is the right idea, since java.util.regex genuinely offers no other hook during backtracking, and doing the deadline check via a bitmask every 256 calls instead of on every call is a sensible way to keep it off the hot path. Nice test coverage too: unit-level (TimeBoundRegexTest) plus end-to-end through both MATCHES and =~ with the exact issue reproducer, and the LIKE/ILIKE audit in the PR description is a good call, confirmed by reading QueryHelper.convertForRegExp - since only %/? are turned into unescaped metacharacters, those can never form grouping/alternation/nested quantifiers, so leaving them alone is correct.

A few things worth a look:

1. New default behavior could surface as spurious failures on legitimate long-running matches.
arcadedb.command.regexTimeout defaults to 1000ms and is enabled by default, independent of arcadedb.command.timeout (which defaults to disabled). That means any existing, non-catastrophic MATCHES/=~ usage that legitimately takes >1s (e.g. a complex-but-safe pattern evaluated against a very large text/blob field) will now throw TimeoutException after upgrade, where before it just ran slowly. This is a reasonable trade-off for the security fix, and it's called out in the release notes, but it's a behavior change on the default path (not just "now bounded", but "now bounded at 1s by default") - might be worth flagging prominently as a potential upgrade gotcha, or reconsidering whether the default should be a bit more generous (e.g. 5-10s) given arcadedb.command.timeout itself defaults to unlimited.

2. Minor: per-row config lookup.
MatchesCondition.matches() and RegexExpression.evaluateTernary() both call context.getConfiguration().getValueAsLong(GlobalConfiguration.COMMAND_REGEX_TIMEOUT) on every invocation (i.e. once per row for a WHERE ... MATCHES scan). ContextConfiguration backs this with a ConcurrentHashMap lookup plus a possible Long.parseLong, so it's cheap, but since the value can't change mid-query, it could be read once per query/condition evaluation rather than per row if this ever shows up in profiling. Not blocking given the relative cost of the regex match itself.

3. Minor: allocation on every bounded match.
Since the new default enables the bound for essentially all MATCHES/=~ evaluations, every call now allocates a DeadlineBoundCharSequence wrapper (previously a direct Pattern.matcher(CharSequence) call), and matching now goes through an extra charAt() indirection versus operating on the raw String/CharSequence. Given CLAUDE.md's performance emphasis, it'd be worth a quick sanity check (even informal) that this doesn't measurably regress throughput on a tight WHERE field MATCHES 'literal-ish-pattern' loop over many rows - I'd expect it to be negligible next to Matcher.matches() itself, but it's the kind of thing that's cheap to verify and easy to assume away.

4. DeadlineBoundCharSequence.subSequence() resets its own call counter.
Each subSequence() call returns a new wrapper with calls = 0, so the 256-call check cadence restarts per sub-sequence. java.util.regex's core backtracking for matches()/quantifiers doesn't normally call CharSequence.subSequence() (it indexes with charAt() directly), so this is likely dead code in practice for the catastrophic-backtracking case this class targets - but if some construct in the pattern language does route through subSequence(), the deadline enforcement could become measurably coarser (still bounded by the same absolute deadlineNanos, just checked less often) for that portion of the match. Worth a comment noting the assumption, or simply confirming no current call path exercises it.

5. Test timing assertions.
The new tests assert wall-clock elapsed time (assertThat(elapsedMillis).isLessThan(5000)) against a 200ms configured deadline. The 5s margin is generous enough to be robust on a loaded CI runner, so this is low-risk, just flagging since timing-based assertions are inherently a bit more failure-prone than logic-based ones.

Nothing above blocks merging - #1 is the only point I'd actually want a decision on (keep 1000ms default vs. raise it), the rest are optional polish. Nice, tightly-scoped security fix with good regression coverage.

[lvca]

Thanks for the review. Responses to each point:

1. Default regexTimeout value (1000ms) vs. command.timeout defaulting to unlimited. Agreed this is the one real decision here — asking @lvca directly rather than picking a number myself.

2. Per-row config lookup. Leaving as-is: it's a ConcurrentHashMap get, negligible next to the regex match itself (confirmed below), and caching it would add a cache-invalidation surface for a value that can't change mid-query for no measurable benefit.

3. Allocation/indirection overhead. Ran an informal check: 200k matches/round, 20 rounds, warmed up, literal-ish pattern (email shape) — plain Matcher.matches() and TimeBoundRegex.matches() both averaged ~6ms/round (~30ns/match either way), i.e. noise-level difference. Not a throughput concern.

4. Fixed in 16af3ef — DeadlineBoundCharSequence's call counter is now shared by reference across a sequence and everything derived from it via subSequence(), so the check cadence no longer resets per sub-sequence. You're right that java.util.regex doesn't currently route matches() backtracking through subSequence(), but sharing the counter removes the assumption instead of just documenting it, at effectively zero cost.

5. Test timing margin. Agreed low-risk as noted; keeping the 5s margin on a 200ms deadline for CI-runner headroom.

[lvca]

Decision on point 1: keeping the 1000ms default. Regex matches are normally sub-millisecond even on large text, legitimate patterns needing more time are rare, and it's a per-database setting anyone hitting the edge case can raise. A tight default closes the security gap immediately for anyone running with arcadedb.command.timeout at its own default (disabled).

[claude]

Review: fix(#5886) bound SQL MATCHES / openCypher =~ against catastrophic regex backtracking

Nice, focused fix. The core idea, wrap the matched CharSequence so charAt() becomes the interruption point, since that's the one call site java.util.regex hits on every backtracking step, is the right way to reach into Matcher.matches() when it doesn't poll interrupts or a deadline. RegexDeadlineExceeded correctly disables the stack trace/suppression (super(null, null, false, false)) since it's a singleton thrown potentially millions of times per aborted match, and the check-every-256-calls bitmask keeps the hot path cheap for well-behaved patterns.

Findings

1. Per-item timeout on multi-value MATCHES, not an aggregate bound (MatchesCondition.java lines ~81-94). regexTimeout is fetched once, but TimeBoundRegex.matches() computes a fresh deadline = System.nanoTime() + timeoutMillis on every call. When value is a multi-value (list) property, each item in the loop gets its own full timeout budget rather than sharing one deadline for the whole evaluation. A list-typed property with many entries, each crafted to backtrack for just under the configured timeout, could still tie up the thread for N * regexTimeout instead of being bounded by regexTimeout overall. Given the PR's stated goal (prevent a handful of queries from permanently pinning a worker thread), it may be worth computing one shared deadline before the loop and passing that deadline into TimeBoundRegex.matches for each item, so the whole MATCHES evaluation is bounded, not just each individual item.

2. Minor: docs/release-26.9.1.md and the GlobalConfiguration Javadoc describe this as bounding a "single regular expression evaluation" - consistent with current per-call semantics, but worth cross-checking against point 1 if that's changed, so the docs and behavior stay in sync.

Things that look solid

• TimeBoundRegex is a clean, dependency-free utility; the shared calls counter across subSequence()-derived instances (added in the second commit) is a good defensive fix even though Matcher.matches() doesn't currently exercise subSequence().
• Both entry points (MatchesCondition for SQL, RegexExpression for openCypher) are updated symmetrically, and the new arcadedb.command.regexTimeout setting is independent of arcadedb.command.timeout and on by default (1000ms), so out-of-the-box installs are protected, not just configurations that explicitly opt in.
• LIKE/ILIKE audit is reasonable: QueryHelper.convertForRegExp escapes grouping/alternation/quantifier metacharacters, so it structurally can't produce a catastrophic pattern. Worth double-checking there's no code path where a user-supplied literal is concatenated into a LIKE pattern unescaped (bypassing convertForRegExp), but that would be a separate, pre-existing concern if it exists.
• Test coverage is good: unit tests for TimeBoundRegex (match/non-match/disabled bound/catastrophic case), and end-to-end regression tests through both MATCHES and =~ reproducing the issue's exact pattern, with a generous upper bound assertion (< 5000ms) that avoids CI flakiness while still proving the abort happens near the deadline rather than "eventually."
• TimeoutException is an existing unchecked ArcadeDBException, so it propagates through the normal query-error path without needing new exception-handling plumbing.
• Performance trade-off (one extra CharSequence wrapper allocation per match call, nanoTime() polled only every 256 charAt() calls) is reasonable and consistent with the project's performance-first guidance, given this is a security fix that's on by default.

Overall this is a well-reasoned, well-tested fix for a real DoS vector. The main thing I'd want addressed or explicitly acknowledged before merge is whether the multi-value MATCHES per-item timeout budget (finding 1) is intentional or should be tightened to an aggregate deadline.

[lvca]

Pushed fc07cb1 addressing both points from the second review:

Finding 1 (multi-value per-item timeout budget): fixed as suggested. TimeBoundRegex now exposes newDeadline(timeoutMillis) / matchesUntil(pattern, input, deadlineNanos) so MatchesCondition computes one deadline before the multi-value loop and shares it across every item, instead of each item getting its own fresh timeoutMillis budget.

Writing the regression test for that surfaced a more serious, separate bug: both MatchesCondition and RegexExpression were reading the timeout via context.getConfiguration(), which for a plain SELECT/=~ evaluation is a fresh, disconnected ContextConfiguration — nothing wires it to the database's settings. It silently fell back to the compiled-in 1000ms default and ignored any per-database arcadedb.command.regexTimeout override, exactly the way db.getConfiguration().setValue(...) is used in the original issue's own reproducer. SelectExecutionPlanner reads arcadedb.command.timeout via context.getDatabase().getConfiguration() for precisely this reason; both regex entry points now match that pattern.

Net effect: the fix was still correct and on by default (the compiled-in 1000ms default was always being applied), but a database operator lowering or raising arcadedb.command.regexTimeout from its default had no effect until this commit. Added multiValueMatchesSharesOneTimeoutBudgetAcrossItems to cover finding 1; the config-resolution bug is what made that test's failure mode obvious (it was consistently landing at ~1000ms regardless of the 200ms configured in the test, rather than N × 200ms as expected from finding 1 alone).

All tests green: TimeBoundRegexTest, MatchesConditionTest (7 tests), OpenCypherWhereClauseTest regex-related tests (13 tests).

[claude]

Review

Solid fix for a real DoS vector, and the iteration history in the commits (sharing the deadline across subSequence(), then across multi-value items, then fixing the disconnected-config bug) shows the design was pressure-tested well. A few notes below, nothing blocking.

Correctness

• The charAt()-interception trick is sound: Matcher.matches()/backtracking nodes call CharSequence.charAt() on every step and don't special-case non-String inputs, so throwing from a wrapped charAt() is a real, well-established way to reach into JDK regex internals that offer no interrupt/deadline hook otherwise.
• Sharing the calls[0] counter by reference across subSequence() derivatives (TimeBoundRegex.java:149) is correct, and the commit message explains why it's currently unreachable but still worth doing defensively.
• The shared-deadline fix for multi-value MATCHES (MatchesCondition.java:85-98) correctly closes the N x timeout gap, and reading the timeout from context.getDatabase().getConfiguration() instead of context.getConfiguration() matches the established SelectExecutionPlanner pattern for COMMAND_TIMEOUT (SelectExecutionPlanner.java:151).
• RegexDeadlineExceeded as a single stack-trace-less singleton thrown across concurrent matcher invocations is safe since it carries no per-occurrence state.

Worth considering

• Per-match overhead on the hot path: every MATCHES/=~ evaluation now goes through a custom CharSequence wrapper instead of matching directly against the String, even when nowhere near the timeout. This adds a virtual-dispatch charAt() indirection plus an array read/mask check on every character access for all matches, not just pathological ones - java.util.regex has fast paths for String input that this bypasses unconditionally. Given the project's stated performance mantra, and that MATCHES/=~ can run in a per-row filter loop over large result sets, a quick before/after benchmark on a representative non-catastrophic workload (e.g. a WHERE MATCHES filter over 100k short strings) would help confirm the overhead is negligible in practice.
• Default of 1000ms is a behavior change enabled by default: because arcadedb.command.regexTimeout is active independently of arcadedb.command.timeout (which defaults to disabled), any existing query that legitimately takes >1s to evaluate a regex - e.g. a non-catastrophic but expensive pattern against a large text-ish string field - will now throw TimeoutException where it previously just ran slowly. The release note calls this out, which is good, but it's worth double-checking there isn't a realistic legitimate use case (e.g. matching multi-KB/MB text fields) that regresses from "slow" to "broken" after upgrade.
• TimeoutException failing the whole query vs. just the row: for the multi-value case in particular, a single catastrophic item aborts the entire MATCHES evaluation rather than just treating that row as non-matching. That's a reasonable, probably intentional security-first choice (silently swallowing it would let a crafted pattern keep consuming the full budget per query undetected) - just flagging it as a semantic change worth being explicit about in the release note (it documents the abort but not what happens to the enclosing query/transaction).

Test coverage

Good coverage of the actual bug (issue reproducer at both the TimeBoundRegex unit level and end-to-end through SQL MATCHES/Cypher =~), the multi-value shared-budget fix, and the non-positive-timeout-disables-the-bound case. One gap: no test asserts that a normal, fast match still behaves identically to plain Pattern.matches() for patterns that exercise subSequence() (e.g. some lookaround shapes) - low priority given the commit message already documents this path as defensive-only for now, but it would make the "not reachable today" claim self-verifying rather than asserted.

Minor

• docs/release-26.9.1.md is well-written and matches the fix accurately.
• GlobalConfiguration.java: the new COMMAND_REGEX_TIMEOUT entry follows the existing text-block description style already used elsewhere in the file - consistent.

No security or compilation concerns found beyond the performance/default-value considerations above.

[lvca]

Thanks — responses to the remaining points, none of which needed a code change beyond one doc line:

Per-match overhead on the hot path. Already measured this for the earlier review (200k matches/round, 20 rounds, warmed up, literal-ish pattern): plain Matcher.matches() and TimeBoundRegex.matches() both averaged ~6ms/round, i.e. noise-level difference (~30ns/match either way). The charAt() indirection and mask check are cheap enough not to show up against Matcher overhead itself.

Default 1000ms being a behavior change on upgrade. Decision already made and recorded above: keeping 1000ms. Regex matches are normally sub-millisecond even on large text; the setting is per-database and easy to raise for the rare legitimate case that needs more.

Whole-query abort vs. per-row on multi-value timeout. Agreed this deserved to be explicit rather than implied — added a paragraph to the release notes (dfc7817) confirming it's intentional: a catastrophic item fails the query with TimeoutException rather than silently excluding itself, since swallowing it would let the pattern keep consuming its full budget on every future scan without ever surfacing.

Test gap: no test proving a normal match is unaffected by a pattern that exercises subSequence(). Not adding this one — java.util.regex's matches()/backtracking path doesn't route through CharSequence.subSequence() for any pattern shape I could construct (confirmed while writing the original fix), so there's no way to write this test without depending on JDK-internal behavior that isn't part of any documented contract. The shared-counter fix stays in as defense-in-depth per the earlier review; asserting "not reachable today" would just be asserting the same JDK-internal fact the fix already doesn't rely on.

At this point I believe all findings from both review passes are either fixed or addressed, and nothing outstanding is blocking. Let me know if anything above should be revisited.

[claude]

Review

Solid fix for a real ReDoS gap, and the charAt() interception approach is the right technique (it's the one call site java.util.regex backtracking actually reaches). The commit history shows the design was iterated on in response to review already (shared counter across subSequence(), shared deadline across multi-value items, reading the per-database config instead of a disconnected ContextConfiguration), which is a good sign of care. A few things worth a look before/after merge:

Security - the stated audit missed two other user-reachable regex sites

The PR says "LIKE/ILIKE were audited too... left unchanged," which is correct for those, but the same catastrophic-backtracking class the PR fixes for MATCHES/=~ still applies unmitigated to at least two other entry points that take a raw user-supplied pattern straight into Pattern.compile(...).matches()/.replaceAll():

• com.arcadedb.function.text.TextRegexReplace (text.regexReplace(str, regex, replacement) SQL function) - its only guard is a 500-char pattern-length cap plus a catch (StackOverflowError). The issue's own reproducer, (.*a){20}$, is 11 characters and (per the PR description) runs 30+ seconds without necessarily overflowing the stack, so this "ReDoS" comment/guard doesn't actually bound catastrophic backtracking.
• com.arcadedb.index.fulltext.FullTextQueryExecutor#collectRegexpMatches - full-text search's Lucene RegexpQuery support (the /pattern/ syntax in a query string) compiles an unescaped user pattern and matches it against every token, in what can be a full index scan (iterateAndMatch(null, ...) when there's no literal prefix). This is arguably a worse instance of the same bug (one pathological pattern evaluated against every token in the index) and isn't mentioned anywhere in the PR.

TimeBoundRegex was written as small, static, reusable methods, so wiring both of these through it (or at minimum TimeBoundRegex.matches(pattern, input, regexTimeout)) should be cheap. Worth doing here or as an immediate fast-follow, since otherwise the fix is easy to bypass via a different query surface. (wildcardToRegex in the same file is fine - it only ever emits .*/. plus Pattern.quote-escaped literals, same shape as the audited LIKE/ILIKE case.)

Possible bug - new NPE surface via context.getDatabase()

Both RegexExpression.evaluateTernary and MatchesCondition.matches now call context.getDatabase().getConfiguration()... unconditionally. CommandContext.getDatabase() (BasicCommandContext.getDatabase()) walks up the parent chain and returns null if nothing in the chain has a database set, whereas the previous code (context.getConfiguration()) never touched the database and was always safe. This mirrors the existing SelectExecutionPlanner pattern for COMMAND_TIMEOUT, so it may be a safe invariant already relied on elsewhere - but worth double-checking there's no path (constraint/validation evaluation, embedded scripting, a bare expression evaluated outside a full query context) where MATCHES/=~ gets evaluated with no database attached, since that would now NPE where it previously degraded gracefully to the compiled-in default.

Minor - StackOverflowError isn't caught

java.util.regex's backtracking engine is recursive, so a sufficiently pathological pattern/input combination can throw StackOverflowError from deep recursion before the 256-call deadline checkpoint is even reached, rather than being converted to the intended TimeoutException. TextRegexReplace (see above) already defends against exactly this with a catch (StackOverflowError); TimeBoundRegex.match() doesn't. Probably low-priority given the primary threat model here is time, not depth, but worth a one-line catch for defense-in-depth/consistency with the other regex call site in the codebase.

Nit - System.nanoTime() comparison

match() uses System.nanoTime() > deadlineNanos, rather than the overflow-safe idiom System.nanoTime() - deadlineNanos >= 0 that System.nanoTime()'s own Javadoc recommends. Only matters after ~292 years of continuous JVM uptime (or an absurdly large configured regexTimeout), so purely cosmetic - not asking for a change, just flagging in case there's a house style for this elsewhere.

Performance

Since arcadedb.command.regexTimeout defaults to 1000ms (enabled out of the box, not opt-in), every MATCHES/=~ evaluation - not just pathological ones - now indirects each charAt() call through DeadlineBoundCharSequence instead of matching directly against the original String. The 256-call check cadence correctly keeps System.nanoTime() off the hot path, but the extra virtual dispatch per character is still there for the common case. That's a reasonable, clearly-explained trade-off for the security property gained, so not a blocker - just flagging in case there's a regex-heavy benchmark elsewhere in the suite worth spot-checking post-merge.

Test coverage

Good: TimeBoundRegexTest covers matching/non-matching/disabled-timeout/catastrophic cases, and both MatchesConditionTest and OpenCypherWhereClauseTest have end-to-end regression tests through SQL and Cypher respectively, including the shared-deadline multi-value case. The < 800ms bound in multiValueMatchesSharesOneTimeoutBudgetAcrossItems (vs. the 5x margin used elsewhere) is a little tighter than the rest of the suite's "generous upper bound" philosophy - worth a wider margin if this ever shows up flaky on a loaded CI runner, since a slow-but-still-under-1000ms failure mode here is specifically the one this test exists to catch.

Nice touches

• RegexDeadlineExceeded as a single shared, stack-trace-less exception instance is the right call given it can be thrown a huge number of times on one runaway match.
• The docs/release-26.9.1.md addition and inline comments do a good job explaining why (disconnected ContextConfiguration, shared deadline for multi-value, shared counter across subSequence()) rather than just what - useful for whoever touches this next.

[lvca]

Pushed 691bc73 addressing everything from this review:

Security - two missed entry points (the big one): fixed.

• text.regexReplace(string, regex, replacement): TimeBoundRegex gained a replaceAll() counterpart to matches(); the function now reads arcadedb.command.regexTimeout the same way MatchesCondition/RegexExpression do, with the existing IllegalArgumentException contract preserved (a TimeoutException from the bound is caught and rewrapped, same as the existing PatternSyntaxException handling).
• Full-text RegexpQuery (/pattern/ syntax): collectRegexpMatches() now shares one deadline across the whole token scan (same N-times-budget concern as the multi-value MATCHES fix, since this evaluates one pattern against potentially every token in the index). LSMTreeFullTextIndex gained a getDatabase() accessor mirroring the underlyingIndex.getMutableIndex().getDatabase() idiom already used internally in that class 6+ times, so FullTextQueryExecutor has a way to reach the config at all.

Agreed this was arguably worse than the two originally-fixed sites (RegexpQuery runs the pathological pattern against every token in a full scan) and appreciate the catch — the original audit only looked at the SQL/Cypher condition operators, not SQL functions or search-query engines.

Possible NPE via context.getDatabase(): investigated, no NPE risk for MatchesCondition/RegexExpression, but a real one for text.regexReplace — fixed.
Verified every top-level SQL and openCypher statement (SelectStatement, UpdateStatement, InsertStatement, CypherExecutionPlan, etc.) calls context.setDatabase(...) before any WHERE/RETURN expression can be evaluated, so a MATCHES/=~ condition always has a database bound — this mirrors the existing unguarded SelectExecutionPlanner pattern for COMMAND_TIMEOUT, confirmed as the same safe invariant. No change needed there.

text.regexReplace is different: it's a lower-level SQL-function entry point, and this exact test file's siblings (TextStatelessFunctionsTest) already call functions directly with a null CommandContext as an established convention. That would have NPE'd. Fixed by falling back to a fresh ContextConfiguration() (itself just a proxy for the compiled-in default) when context/context.getDatabase() is unset — added functionIsSafeWithoutACommandContext to cover it.

StackOverflowError not caught: fixed. TimeBoundRegex's internal run() now catches RegexDeadlineExceeded | StackOverflowError together and converts either to TimeoutException — a pathological pattern that blows the recursive backtracking stack before the next 256-call checkpoint gets the same bound as one that trips the deadline first. This also made TextRegexReplace's own separate catch (StackOverflowError) dead code, so it's now just the shared TimeoutException path (still surfaced as IllegalArgumentException per that function's existing contract).

nanoTime() overflow-unsafe comparison: fixed, switched to System.nanoTime() - deadlineNanos >= 0 per the Javadoc-recommended idiom.

Test coverage gap (multi-value margin too tight): fixed. Widened from 5 items/800ms to 10 items/1000ms — same discriminating power (expected ~200-300ms if shared, ~2000ms if not) with a wider absolute margin on both sides for CI-runner slack.

Performance / default value: no new action, already addressed in earlier rounds (measured negligible overhead, decision made to keep the 1000ms default).

New tests: TimeBoundRegexTest gained replaceAll coverage (normal/disabled-bound/catastrophic), TextRegexReplaceTest is new (basic replace, null-context safety, catastrophic-pattern abort, existing length-cap guard still fires), FullTextQueryExecutorTest gained catastrophicRegexpQueryIsAbortedByRegexTimeout. All green, plus reran CypherBuiltInFunctionsTest (145), CypherFunctionSecurityTest (28), OpenCypherTextFunctionsTest (86), and the regex-related OpenCypherWhereClauseTest methods (13) to check nothing else touching regexReplace/=~ regressed.

[claude]

Review

Nice fix overall - wrapping the CharSequence so charAt() becomes the interruption point is the correct technique for java.util.regex catastrophic backtracking (it's the only call site the engine reaches on every backtracking step), and the follow-up commits already tightened it well: sharing the deadline across multi-value evaluations, reading regexTimeout from context.getDatabase().getConfiguration() instead of a disconnected ContextConfiguration, the StackOverflowError fallback, and the overflow-safe nanoTime() comparison. TimeBoundRegex's CHECK_INTERVAL bitmask keeps the common (non-pathological) path essentially free, and skipping the wrapper entirely when the deadline is disabled is a good touch.

Bug: sibling wildcard-query path in the same file is still unbounded

FullTextQueryExecutor.collectRegexpMatches was fixed to route through TimeBoundRegex.matchesUntil, but collectWildcardMatches (same file, ~30 lines above) still calls the raw, unbounded match in both branches:

// engine/src/main/java/com/arcadedb/index/fulltext/FullTextQueryExecutor.java:625 and :635
return regex.matcher(token).matches();

wildcardToRegex() (line 774) turns each * into .* and each ? into . with no grouping/alternation - but that's exactly the sequential-.* shape that causes catastrophic backtracking on its own (the classic a*a*a*a*...b ReDoS pattern doesn't need nested quantifiers or alternation, just several .*/. segments in a row against input that doesn't fully match). A full-text query like field:*a*a*a*a*a*a*a*a*a*a*a*a*a*a*a*a*a*a*a*a*! against a reasonably long token reproduces the same multi-second-plus hang this PR exists to fix.

Two things make this concretely reachable, not just theoretical:

• The else branch (line 627-637, non-leading-wildcard, range-scan case) runs under the default allowLeadingWildcard = false - no special index config is needed to hit it, only a wildcard pattern that doesn't start with */?.
• Even the full-scan branch (leading wildcard) is the exact same full-index-scan shape as collectRegexpMatches, which this PR explicitly called "arguably the worse instance" of the bug and fixed - so leaving its sibling unfixed seems like an oversight rather than an intentional scope cut.

Suggest wrapping both regex.matcher(token).matches() calls in collectWildcardMatches with TimeBoundRegex.matchesUntil(regex, token, deadline), using the same TimeBoundRegex.newDeadline(index.getDatabase().getConfiguration().getValueAsLong(GlobalConfiguration.COMMAND_REGEX_TIMEOUT)) pattern already used in collectRegexpMatches, and adding a regression test mirroring catastrophicRegexpQueryIsAbortedByRegexTimeout (e.g. catastrophicWildcardQueryIsAbortedByRegexTimeout) so this doesn't regress silently.

Minor: null-database handling is inconsistent between entry points

TextRegexReplace explicitly falls back to a fresh ContextConfiguration when context/context.getDatabase() is null (with a comment explaining unit tests call it directly with a null context). MatchesCondition.matches() and RegexExpression.evaluateTernary() instead call context.getDatabase().getConfiguration() unconditionally. Today this looks safe in practice - every BasicCommandContext chain used during real query execution ends up with a database set on some ancestor (mirroring the existing SelectExecutionPlanner idiom for COMMAND_TIMEOUT), and before this PR neither method touched context.getDatabase() at all. It's worth double-checking there's no code path (e.g. a WHERE condition evaluated during planning/cost-estimation rather than execution, or a context built for scripting without a bound database) that could now NPE where MATCHES/=~ previously worked fine. If you're confident that can't happen, a one-line comment stating the invariant (like the one already next to the config lookup) would help the next reader.

Nits

• TimeBoundRegex.run()'s TimeoutException message uses input.length() (fine, since input is the original, un-wrapped CharSequence) - just flagging that it's easy to misread given bounded is the name used inside the lambda for the wrapped sequence.
• The new arcadedb.command.regexTimeout default (1000ms) is independent of arcadedb.command.timeout's default of disabled (0) - worth calling out explicitly in the release notes for anyone who parses MATCHES/=~ results that legitimately need >1s (e.g. very large multi-value lists), since they'll now need to raise the new setting even if they never touched the old one. The docs entry does mention this, so just a docs-nit if anything.

Test coverage

Good regression coverage for the entry points that were fixed (TimeBoundRegexTest, MatchesConditionTest, OpenCypherWhereClauseTest, TextRegexReplaceTest, FullTextQueryExecutorTest#catastrophicRegexpQueryIsAbortedByRegexTimeout), including the multi-value shared-budget case. The one gap is the wildcard-query path noted above.

I reviewed statically (couldn't execute mvn in this sandboxed session) - based on reading TimeBoundRegex, MatchesCondition, RegexExpression, TextRegexReplace, and FullTextQueryExecutor end to end plus the new/changed tests.

[lvca]

Pushed 87f33fc. This round found something worth flagging explicitly: the wildcard-query finding didn't just mean fixing a sibling method - it invalidated this PR's own earlier "LIKE/ILIKE are structurally safe" conclusion.

Bug: sibling wildcard-query path unbounded — fixed, and it corrects an earlier mistake of mine.
wildcardToRegex() turns * into .* with no grouping/alternation/nested quantifiers - which is exactly the property my original LIKE/ILIKE audit relied on to call them safe. But a sequence of several .* segments reproduces catastrophic backtracking on its own; nesting isn't required, just several ambiguous .*s in a row against input that ultimately fails to match. I verified this empirically before touching anything: .*a.*a.*a... (20 reps) against a plain string of as hangs exactly like the issue's own (.*a){20}$ reproducer. Since QueryHelper.convertForRegExp (% → .*) produces the identical shape, that means SQL LIKE/ILIKE were vulnerable too, contrary to what I stated in this PR's description and the release notes.

Fixed all of it: QueryHelper.like()/new likeUntil() now route through TimeBoundRegex, mirroring matches()/matchesUntil(). Updated all three independent call sites - LikeOperator, ILikeOperator, and the native query engine's SelectOperator (a third, separate LIKE/ILIKE evaluation path I hadn't previously touched) - plus collectWildcardMatches in FullTextQueryExecutor, sharing one deadline across its full scan the same way collectRegexpMatches already does. Corrected the release notes to say so plainly rather than just supplementing the wrong claim.

Caught my own regression before it shipped: LikeOperator.execute(DatabaseInternal database, ...) and ILikeOperator's equivalent are called with database == null directly in the existing LikeOperatorTest/ILikeOperatorTest (an established unit-test convention for these operators). My first pass at wiring in the config lookup would have NPE'd on exactly those tests. Applied the same null-safe ContextConfiguration fallback TextRegexReplace already needed.

New tests: QueryHelperLikeRegexTimeoutTest (utility-level like()/likeUntil() plus SQL-level LIKE and ILIKE regression tests through a real query), FullTextQueryExecutorTest#catastrophicWildcardQueryIsAbortedByRegexTimeout (careful to avoid a leading wildcard, since allowLeadingWildcard defaults to false and I didn't want the test coupled to that config). Reran LikeOperatorTest, ILikeOperatorTest, QueryTest (66), SelectStatementExecutionTest (159), SelectOperatorsExtendedTest (12), SelectExecutionTest (24) - all green, including the null-database convention.

At this point every entry point named across four review rounds - MATCHES, =~, text.regexReplace, full-text RegexpQuery, and now LIKE/ILIKE/full-text WildcardQuery - is bounded the same way, and the audit conclusion that was wrong is corrected rather than just left standing next to a patch. Open to another pass if there's more, but I believe this closes the class of bug the issue was about.

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 17 complexity

Metric	Results
Complexity	17

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[claude]

Review: bound SQL MATCHES/openCypher =~ (and friends) against catastrophic regex backtracking

Overall this is a well-executed fix for a real ReDoS exposure. The TimeBoundRegex.charAt()-throws-past-deadline trick is the right approach given java.util.regex never polls interrupts during backtracking, and the commit history shows the author iterated through several rounds of genuine self-review (shared deadline across multi-value items, reading the timeout from the database-bound config instead of a disconnected ContextConfiguration, and correcting the initial "LIKE is structurally safe" conclusion). That kind of follow-through is exactly what this class of fix needs, since regex-based ReDoS entry points tend to hide in more places than the first audit finds.

Strengths

• Central TimeBoundRegex utility is clean: sentinel Long.MAX_VALUE deadline for the disabled case avoids any wrapping overhead when regexTimeout <= 0, the every-256-calls bitmask check keeps charAt() cheap on the hot path, and the overflow-safe nanoTime() comparison is correct.
• matches()/matchesUntil() split (single-shot vs. shared deadline) correctly fixes the "N items = N * timeout" trap for multi-value MATCHES/=~/LIKE/ILIKE, and the fix is applied consistently to all four operators plus the two full-text search entry points.
• New setting arcadedb.command.regexTimeout at SCOPE.DATABASE mirrors COMMAND_TIMEOUT's pattern and is well documented in GlobalConfiguration and the release notes.
• Good test coverage: unit tests on the utility itself, SQL/Cypher integration tests for the actual reproducer, multi-value sharing verified with timing assertions, and the disabled (<= 0) case is tested.

Issues / things worth a second look

1. Inconsistent null-database fallback (minor, low risk today). LikeOperator, ILikeOperator, and TextRegexReplace all guard against context.getDatabase() == null / database == null because their existing unit tests call them directly with a bare/null context. MatchesCondition.matches() and RegexExpression.evaluateTernary() do not: both now call context.getDatabase().getConfiguration() unguarded (MatchesCondition.java:85, RegexExpression.java:77). Today this is safe since both classes are only ever constructed via the SQL/Cypher parsers under a real query context, but it's an inconsistency within the same PR - if a future test or refactor ever calls either directly with a disconnected context, it'll NPE instead of falling back to the compiled-in default like its siblings do. Worth a one-line guard for consistency/defense-in-depth, or at least a comment noting why it's safe here (parser-only construction) the way the other three explain why they needed the fallback.

2. Deadline overflow isn't guarded (minor, low practical risk). TimeBoundRegex.newDeadline(): System.nanoTime() + (timeoutMillis * 1_000_000L) has no overflow protection. arcadedb.command.regexTimeout is a user/admin-configurable Long, so a very large value (misconfiguration, or someone converting from a different unit by mistake) could overflow the multiplication or the addition and produce a deadline that's already "in the past," causing every regex match to abort immediately instead of applying the intended (very long) timeout. Given the setting is meant to hold millisecond values in a sane range this is unlikely to bite in practice, but a clamp against a sane ceiling would make it robust against a fat-fingered config value.

3. GC pressure on the common path (minor, matches the project's own performance guidance). Every bounded match now allocates a DeadlineBoundCharSequence wrapper plus a backing int[1] counter, even though the overwhelming majority of matches are well-behaved and never approach the 256-call checkpoint. Since arcadedb.command.regexTimeout defaults to enabled (1000ms), this allocation happens on every MATCHES/LIKE/ILIKE/=~ evaluation in a hot filter/scan path, not just for pathological patterns. This is a small, probably negligible cost relative to the Pattern.compile() that's already happening per call in QueryHelper.like()/MatchesCondition, but worth a quick profile on a large LIKE-filtered scan given the project's "prefer primitives, minimize GC pressure" guidance. Not a blocker, just flagging for awareness.

4. Worth double-checking (not confirmed as a bug): Matcher.replaceAll()/find() in some JDK versions uses a Boyer-Moore fast path for literal-heavy patterns. If that path ever reads through something other than CharSequence.charAt()/length() (e.g. a cached char[] snapshot), it would bypass the deadline check entirely for those patterns. The PR's own tests already exercise literal-heavy patterns (%a%a%a..., the wildcard a*a*a*... case) and they pass, which is decent evidence this isn't an issue, but it may be worth a short comment in TimeBoundRegex's Javadoc noting this was verified empirically for both matches() and replaceAll(), so a future reader doesn't have to re-derive that confidence.

Test coverage

Comprehensive. Dedicated TimeBoundRegexTest for the utility, end-to-end SQL/Cypher tests for the original reproducer, a multi-value shared-budget test with a sensible timing margin (10 items/1000ms to avoid CI flakiness), full-text regex and wildcard query coverage, and the text.regexReplace function. Nothing obvious missing; the null-context edge case in point 1 above is the only gap worth calling out.

Nit

docs/release-26.9.1.md and the GlobalConfiguration description do a good job explaining why arcadedb.command.timeout alone couldn't have caught this - that context will save future readers some head-scratching.

[lvca]

Pushed 9adab87 addressing all four points:

1. Inconsistent null-database fallback: investigated (already done in an earlier round) and documented rather than guarded. Confirmed MatchesCondition/RegexExpression are only ever constructed by the SQL/openCypher parsers and evaluated by the query execution engine, which always binds a database to the context before any WHERE/RETURN expression runs — there's no direct-unit-test-style entry point into these classes the way there is for a standalone operator (LikeOperatorTest et al. construct and call LikeOperator directly) or SQL function. Added a comment at each call site explaining this instead of an unneeded guard, per your own suggested resolution.

2. Deadline overflow: fixed, and it's a real one. newDeadline() now uses Math.addExact/multiplyExact, falling back to the disabled (Long.MAX_VALUE) sentinel on overflow — the safe direction, since an admin fat-fingering a huge arcadedb.command.regexTimeout clearly meant "a lot of time," not "abort every match immediately," which is what the unguarded arithmetic would have silently produced. Added newDeadlineDoesNotOverflowOnAnOversizedTimeout.

3. GC pressure: no new action — already measured at ~30ns/match (noise-level) in an earlier round, consistent with your own assessment that it's "probably negligible relative to Pattern.compile()."

4. Boyer-Moore/fast-path concern: documented. You're right that this PR's own tests already provide decent evidence (the literal-heavy %a%a%a... and a*a*a*... patterns correctly abort, so charAt() is definitely still on the path for those shapes) — added that to TimeBoundRegex's class Javadoc explicitly, so a future reader doesn't have to re-derive the confidence from test behavior.

All tests green: TimeBoundRegexTest (8, incl. the new overflow test), MatchesConditionTest, TextRegexReplaceTest, FullTextQueryExecutorTest, QueryHelperLikeRegexTimeoutTest, LikeOperatorTest, ILikeOperatorTest.

[claude]

Review: fix(#5886) bound SQL MATCHES / openCypher =~ against catastrophic regex backtracking

Overall this is a well-executed, carefully-scoped security fix. The core insight (CharSequence.charAt() is the one call site java.util.regex backtracking always touches, so wrapping the input to throw past a deadline is the only interception point available without patching the JDK) is solid, and the commit history shows real self-review discipline: multi-value budget sharing, reading the timeout from context.getDatabase().getConfiguration() instead of the disconnected context.getConfiguration(), overflow-safe deadline math, and finding two more unguarded entry points (text.regexReplace, full-text RegexpQuery/WildcardQuery) plus correcting the initial "LIKE is safe" conclusion. That's the review process working as intended.

Possible gap: PromQL label matchers look like the same bug, unfixed

PromQLEvaluator (engine/src/main/java/com/arcadedb/engine/timeseries/promql/PromQLEvaluator.java) has its own user-reachable regex entry point: the =~/!~ label matchers (RE/NRE ops, matchesPostFilters() lines ~497-503) call compilePattern(m.value()).matcher(strVal).matches() directly, with no TimeBoundRegex. Its only defense is REDOS_CHECK (line ~80), a static pattern that only flags parenthesized nested-quantifier/alternation shapes (\(...[+*]...\)[+*{]).

That's exactly the shape of gap this PR's own "3rd review pass" found in SQL LIKE: a sequence of unparenthesized quantified segments (e.g. a*a*a*a*a*a*a*a*a*a*a*a*a*a*a*a*a*a*a*ac, PromQL's equivalent of %a%a%a%...%c) reproduces catastrophic backtracking without ever containing a (, so REDOS_CHECK won't catch it, and there's no timeout backstop once it gets through. Since this PR treats the "sequential-.*-without-nesting" shape as squarely in scope (that's what motivated fixing LIKE/wildcard), it's worth deciding whether PromQL's =~ is in scope here too or a deliberate follow-up - as written, someone with query access can hang a worker thread via a PromQL label matcher the same way the original issue describes for MATCHES.

Minor points

• Misleading message on disabled timeout + stack overflow: TimeBoundRegex.run() catches StackOverflowError unconditionally, even when deadlineNanos == Long.MAX_VALUE (i.e. arcadedb.command.regexTimeout explicitly disabled via <= 0). The resulting TimeoutException message says "aborted (arcadedb.command.regexTimeout)", which would be confusing for an admin who deliberately disabled that setting - the real cause there is an unrelated stack depth issue, not the (disabled) timeout.
• Possible silent fallback to the compiled-in default: LikeOperator/ILikeOperator fall back to new ContextConfiguration() (compiled-in default of 1000ms) when database is null, and BinaryCondition.evaluate(Result, CommandContext) (pre-existing, not touched here) already tolerates a null context (operator.execute(context != null ? context.getDatabase() : null, ...)). If that null-context path is ever hit outside of unit tests, a per-database regexTimeout override would be silently ignored for LIKE/ILIKE there - the same class of bug this PR fixed for MatchesCondition's old context.getConfiguration() call, just via a different null path. Worth double-checking whether that's reachable in real query execution or purely a test-only convenience.
• Pattern compilation still happens per call in QueryHelper.like/likeUntil (Pattern.compile(value) on every invocation) - unchanged from the prior currentValue.matches(value) behavior, so not a regression, but LIKE is likely a hotter path than MATCHES and now also allocates a DeadlineBoundCharSequence (+ int[1]) per call. Possibly worth a follow-up given the project's performance-sensitivity guidance, not a blocker for this fix.

Test coverage

Good - the new tests assert both correctness (normal matches still work) and the abort-near-deadline behavior with generous timing margins to avoid CI flakiness, and the multi-value budget-sharing regression test (10 items within ~1s instead of ~2s) is a meaningful regression guard, not just a smoke test. TimeBoundRegexTest.newDeadlineDoesNotOverflowOnAnOversizedTimeout is a nice catch for the admin-configurable-overflow edge case.

Nothing here blocks merging as a fix for the originally reported issue - the PromQL point is the only thing I'd want a maintainer opinion on before calling the ReDoS surface fully closed.

[lvca]

Pushed c9d816b addressing everything from this round:

PromQL =~/!~ label matchers: fixed, and it's real. Verified empirically before touching anything, same discipline as the LIKE finding: a*a*a*a*a*a*a*a*a*a*a*a*a*a*a*a*a*a*a*ac (no parens, so REDOS_CHECK — which only flags parenthesized nested-quantifier/alternation shapes — never sees it) against a 40-char string of as hangs exactly like every other reproducer in this issue. matchesPostFilters() now routes both RE/NRE cases through TimeBoundRegex.matchesUntil(), sharing one deadline across the whole row scan (it runs once per row, so this needed the same shared-deadline treatment as the multi-value MATCHES/full-text-scan cases, not a fresh budget per row). Kept REDOS_CHECK in place as a cheap static pre-filter alongside the new runtime bound — same layering text.regexReplace's length cap got.

Misleading message on disabled timeout + stack overflow: fixed. TimeBoundRegex.run() now only converts StackOverflowError to TimeoutException when the bound is actually active; when arcadedb.command.regexTimeout <= 0 it propagates as itself, since blaming a setting the caller deliberately disabled would be actively confusing. Didn't add an automated test for this specific branch — reliably triggering a genuine StackOverflowError from java.util.regex in a fast, portable, CI-safe way is its own small research project (JVM stack size varies, and a pattern deep enough to guarantee it risks being slow/flaky), and the change is a message-clarity fix, not a behavior change in whether the operation eventually completes or fails.

Possible silent fallback via BinaryCondition's pre-existing null-context tolerance: investigated, not reachable today. Traced every call site in the engine that invokes .evaluate(Result, CommandContext) with a literal null context. Found exactly two, both openCypher-only and both narrow constant-folding cases (ExpressionEvaluator#evaluatePropertyAccess for plain property reads, CypherASTBuilder's static-list-literal folding) — neither can reach LikeOperator/ILikeOperator (SQL-only, never touched by openCypher) or RegexExpression/MatchesCondition. No live SQL-engine caller passes BinaryCondition.evaluate(Result, CommandContext) a null context today. Even if that changes, the null-safe fallback already in place resolves to the compiled-in default rather than crashing, which is the same safe direction taken everywhere else in this PR.

Pattern compilation per call in QueryHelper.like: no action — pre-existing behavior (String.matches() never cached either), not a regression from this PR, and out of scope for a security fix.

New test: PromQLEvaluatorIntegrationTest#unparenthesizedCatastrophicPatternIsAbortedByRegexTimeout. All tests green: that one (17 total in the class now), TimeBoundRegexTest (8), MatchesConditionTest, TextRegexReplaceTest, FullTextQueryExecutorTest, QueryHelperLikeRegexTimeoutTest, LikeOperatorTest, ILikeOperatorTest.

At this point every regex-based query surface I can find in the codebase — MATCHES, =~ (Cypher), LIKE/ILIKE, text.regexReplace, full-text RegexpQuery/WildcardQuery, and PromQL =~/!~ — routes through the same TimeBoundRegex bound.

[claude]

Reviewed the diff (docs + GlobalConfiguration, TimeBoundRegex + 10 call sites, and the new/updated tests). Solid fix for a real DoS gap, and the multi-pass audit (MATCHES/=~ -> LIKE/ILIKE -> full-text wildcard/regexp -> PromQL =~/!~) genuinely found entry points that a narrower fix would have missed. A few things worth a look before merge:

1. Inconsistent null-safety for context.getDatabase() (moderate)
MatchesCondition.matches() and RegexExpression.evaluateTernary() now call context.getDatabase().getConfiguration()... unguarded, while the sibling LikeOperator/ILikeOperator/TextRegexReplace all defensively fall back to new ContextConfiguration() when the database is null. The in-code comments explain this is a deliberate choice based on an audit ("only ever constructed by the SQL parser... which always binds a database before any WHERE/RETURN expression runs"), and BasicCommandContext.getDatabase() genuinely can return null if no context in the parent chain has one set. This is also a new access path - the old code (p.matcher(sequence).matches()) never touched the database at all. If any current or future code path (a unit test constructing a bare context, a future refactor of expression evaluation, a default-value/computed-property evaluation) ever reaches MatchesCondition/RegexExpression without a bound database, this becomes a hard NPE instead of a graceful default, unlike every other modified call site. Since the defensive version costs nothing, I'd apply the same database != null ? ... : new ContextConfiguration() pattern here too for consistency/defense-in-depth, rather than relying solely on the audit holding forever.

2. Shared deadline over a full scan makes the 1000ms default fairly aggressive for bulk paths (moderate/design)
For FullTextQueryExecutor.collectWildcardMatches/collectRegexpMatches and PromQLEvaluator.matchesPostFilters, one regexTimeout budget is now shared across the entire scan (every token in a full-text index, every row in a time-series scan) rather than per item - which is the right call to prevent an N-items-times-timeout bypass. But combined with the new 1000ms default, this means a legitimate (non-catastrophic) scan over a large full-text index or large time-series result set that simply takes >1s of wall-clock time, purely because there's a lot of data, will now throw TimeoutException where previously (with arcadedb.command.timeout disabled by default) it just ran to completion. Worth calling out explicitly as a behavior change on upgrade for anyone with big full-text indexes or wide PromQL scans, and maybe worth a documentation note recommending arcadedb.command.regexTimeout be raised for such workloads.

3. Test coverage gaps (minor)

• com.arcadedb.query.select.SelectOperator's like/ilike (the native, non-SQL query API) was updated to pass regexTimeout through, but there's no regression test proving the catastrophic-backtracking-abort behavior for that path, unlike every other modified entry point (SQL LIKE/ILIKE, MATCHES, =~, regexReplace, full-text wildcard/regexp, PromQL).
• LikeOperator/ILikeOperator's multi-value (BY ITEM) shared-deadline branch has no dedicated test proving the shared-budget behavior, unlike MatchesConditionTest#multiValueMatchesSharesOneTimeoutBudgetAcrossItems, which explicitly proves it for MATCHES. This "N items x timeout vs. one shared budget" distinction is exactly the kind of thing that can regress silently, so it'd be good to mirror that test for LIKE/ILIKE.

4. Minor duplication (nit)
QueryHelper.like(...) and likeUntil(...) repeat the same null/empty-string guard and pattern-compile logic. like() could just delegate to likeUntil(currentValue, value, TimeBoundRegex.newDeadline(timeoutMillis)). Not a bug, just a small cleanup opportunity.

Things that look right:

• The core technique - wrapping the matched CharSequence so charAt() throws past a deadline - is a correct and well-known way to bound java.util.regex, since Matcher.matches()/replaceAll() expose no interrupt/deadline hook of their own. subSequence() correctly propagates the shared deadlineNanos/call-counter so lookaround/region logic stays bounded too.
• newDeadline()'s overflow handling via Math.addExact/Math.multiplyExact, falling back to "unbounded" rather than silently wrapping into a past deadline, is exactly right - the reverse mistake (an oversized timeout instantly aborting every match) would be a nasty surprise for an admin turning the value up.
• Zero-overhead disable path (deadlineNanos == Long.MAX_VALUE skips the wrapper entirely) means callers who leave the setting at 0 (documented as "not recommended" but still supported) don't pay any tax.
• COMMAND_REGEX_TIMEOUT is SCOPE.DATABASE, consistent with COMMAND_TIMEOUT, so it composes correctly with per-database overrides in tests without leaking global state.
• The StackOverflowError catch in TimeBoundRegex.run() mirrors what TextRegexReplace already did before this PR, and is scoped tightly around one Matcher call, so it's a reasonable pragmatic carryover rather than new risk.
• Tests consistently assert an upper elapsed-time bound (proving the abort happens near the configured deadline, not "eventually"), which is the right way to guard against this regressing into "still correct but silently unbounded again."

Nice find on the follow-on entry points (full-text RegexpQuery/WildcardQuery, PromQL label matchers) - those genuinely look like the same class of bug and would've been an easy thing to miss if the fix had stopped at the two entry points named in the issue.

[lvca]

Pushed bf3bfdb addressing all four points:

1. Inconsistent null-safety: fixed. Agreed on the second ask — MatchesCondition/RegexExpression now fall back to a plain ContextConfiguration() when context.getDatabase() is null, matching the other three modified entry points. Investigated and confirmed not reachable twice now, but the guard costs nothing and removes the asymmetry, so not worth relying on the audit holding forever against every future refactor.

2. Shared-deadline-over-full-scan + 1000ms default is aggressive for bulk paths: documented explicitly. Added a standalone paragraph to the release notes calling this out as its own tradeoff (distinct from the earlier "any single match >1s now fails" note) — a legitimate, non-catastrophic full-text or PromQL scan over a lot of data can now fail on TimeoutException where it previously just ran slowly, and recommends raising arcadedb.command.regexTimeout for such workloads. This is a natural, unavoidable consequence of the shared-deadline fix being correct (per-item budgets reopen the bypass), not something further code can resolve — it's a real behavior change worth surfacing to whoever reads the release notes, not something to silently absorb.

3. Test coverage gaps: both closed.

• SelectExecutionTest#catastrophicLikeIsAbortedByRegexTimeout — the native query engine's SelectOperator.like/ilike (a third, independent LIKE/ILIKE path, distinct from the SQL parser's operators) now has its own regression test, matching every other modified entry point.
• QueryHelperLikeRegexTimeoutTest#multiValueLikeSharesOneTimeoutBudgetAcrossItems — mirrors MatchesConditionTest's multi-value test for LikeOperator/ILikeOperator's BY ITEM branch, which had the shared-deadline logic already but no test proving it.

4. QueryHelper.like/likeUntil duplication: fixed. like() now just delegates to likeUntil(currentValue, value, TimeBoundRegex.newDeadline(timeoutMillis)).

All tests green: MatchesConditionTest, TimeBoundRegexTest, QueryHelperLikeRegexTimeoutTest (6, incl. the new multi-value test), SelectExecutionTest (25, incl. the new catastrophic-LIKE test), LikeOperatorTest, ILikeOperatorTest, and the regex-related OpenCypherWhereClauseTest methods.

[claude]

Reviewed the diff (didn't build locally - git fetch for the PR ref wasn't approved in this session, so this is diff-based, not build-verified). Overall this is a well-engineered fix: the charAt()-interception trick in TimeBoundRegex is the right way to bound java.util.regex without an external dependency, the overflow-safe deadline math, StackOverflowError handling, and the shared-deadline-across-items logic are all solid, and the self-review commit trail shows genuine iteration (LIKE/wildcard/PromQL gaps were real catches). Test coverage per entry point is thorough.

One finding I think is worth resolving before merge, plus a couple of smaller points.

Potential issue: DocumentValidator REGEXP property validation is still fully unbounded

engine/src/main/java/com/arcadedb/database/DocumentValidator.java:58:

if (!fieldValue.toString().matches(p.getRegexp()))

This runs on every document insert/update for a type with a schema-level REGEXP property constraint (CREATE PROPERTY ... REGEXP <pattern>), with no length cap, no TimeBoundRegex, nothing. It's the same String.matches() -> java.util.regex catastrophic-backtracking exposure this PR fixes everywhere else, except the trigger here doesn't require SQL/Cypher query access at all - any write path (REST, any wire protocol) into a validated type is enough. If the admin-defined pattern happens to have a vulnerable shape (this is exactly the historical ReDoS class - e.g. classic email/URL validation regexes are notorious for it), an attacker who can just write a document with the wrong-shaped value can hang a worker thread indefinitely, which is arguably a more exposed surface than the MATCHES/=~ entry points this PR targets (those need query privileges; this needs only insert/update). Given the PR already built TimeBoundRegex for exactly this purpose, wiring this call through it (with the same null-database-safe config lookup pattern used elsewhere in this PR) seems like a small, in-scope addition worth including here rather than as a follow-up.

Design question: per-row deadline vs. per-scan deadline for MATCHES/=~/LIKE

For full-text RegexpQuery/WildcardQuery and PromQL =~/!~, the PR deliberately computes one deadline for the whole scan, explicitly because "each item getting its own full budget would let a crafted list/index run for item count * regexTimeout." That reasoning applies equally to a WHERE ... MATCHES/LIKE/=~ clause evaluated row-by-row over a table scan: each row still gets its own fresh regexTimeout budget (MatchesCondition.matches()/LikeOperator.execute() compute a new deadline per call), so a table with many rows shaped to force worst-case backtracking could still cost up to rowCount * regexTimeout overall - unbounded by default, since arcadedb.command.timeout is disabled out of the box. That's a much smaller blast radius than the original bug (bounded per row instead of infinite), so it may be an acceptable tradeoff, but it's inconsistent with the "share one deadline across the whole operation" principle applied elsewhere in this same PR, and it'd be good to see that inconsistency called out as a conscious decision (or fixed the same way) rather than left implicit.

Minor: repeated boilerplate across call sites

The "database != null ? database.getConfiguration() : new ContextConfiguration(), then getValueAsLong(COMMAND_REGEX_TIMEOUT)" snippet is duplicated near-verbatim in MatchesCondition, RegexExpression, LikeOperator, ILikeOperator, and TextRegexReplace. Worth a small shared helper (e.g. a static method on TimeBoundRegex or ContextConfiguration) so the null-safety fallback can't drift out of sync across sites in a future change - not a correctness issue today, just a maintainability one given how many places now carry this exact logic.

Minor: default regexTimeout (1000ms) is a real behavior change for large scans

Already called out honestly in the release notes, but worth flagging in review too: full-text RegexpQuery/WildcardQuery and PromQL =~/!~ now share a single 1s budget across an entire scan, so a legitimately large (non-catastrophic) scan that previously ran slowly to completion will now fail with TimeoutException post-upgrade at the default setting. Good that it's documented; just flagging since it's a behavioral regression for some existing workloads, not just a security hardening.

Nits

• TimeBoundRegex.likeUntil/like javadoc and code style are consistent with the rest of the engine (final params, imports over FQNs, etc.) - no style complaints.
• The RegexDeadlineExceeded singleton with a suppressed stack trace is the right call given it can fire very frequently on one runaway match.

Nice, thorough piece of work otherwise - the multi-round self-review catching the LIKE/wildcard/PromQL gaps in particular shows real diligence.

[claude]

Review: ReDoS protection for MATCHES/=~/LIKE/ILIKE/full-text/PromQL/REGEXP (#5886)

This is a thorough, well-executed security fix. I read through TimeBoundRegex and every call site it touches (MatchesCondition, RegexExpression, LikeOperator/ILikeOperator, SelectOperator, QueryHelper, TextRegexReplace, SQLMethodNormalize/Split, FullTextQueryExecutor, PromQLEvaluator, DocumentValidator) plus the new/updated tests. Overall assessment: solid.

Strengths

• Root-cause fix, not a band-aid. Wrapping charAt() is the correct interception point since Matcher never polls interrupts/deadlines while backtracking - this is verified empirically (per the class javadoc) to work across both matches() and replaceAll().
• Deadline-sharing is handled correctly, and it's the trickiest part of this change. Per-row/per-item budgets would just reopen an N * timeout bypass, and the PR correctly shares one deadline across a whole scan/query execution for MatchesCondition, RegexExpression, full-text scans, and PromQL post-filters. The RegexExpression fix in particular - caching the deadline on CommandContext instead of as an AST instance field - is the right call, since CypherStatementCache reuses AST nodes across executions of the same query text and an instance-field deadline would go stale and cause spurious TimeoutExceptions on later, completely benign executions. There's a dedicated regression test for exactly this (regexDeadlineDoesNotLeakAcrossCachedQueryExecutions).
• Test coverage is excellent - every modified entry point gets a dedicated catastrophic-backtracking regression test with a real timing assertion, plus the two cache-collision regressions (the MATCHES_DEADLINE key colliding with pattern-cache keys) each have their own targeted test.
• Correctly widens the "safe" audit conclusion for LIKE/ILIKE and full-text wildcards: sequential .*/. segments reproduce catastrophic backtracking without needing grouping/alternation, which the original convertForRegExp escaping analysis alone would have missed.
• SQLMethodSplit's switch from raw String.split(regex) to Pattern.quote() is a nice consistency fix (matches split()/text.split()/Cypher split()) and removes that particular risk entirely rather than just bounding it.
• No new dependencies, no leftover System.out, TimeoutException reuses the existing exception type consistently.

Minor observations (non-blocking)

1. SelectOperator LIKE/ILIKE bypass the new GlobalConfiguration.getValueAsLong(Database) helper, calling record.getDatabase().getConfiguration().getValueAsLong(GlobalConfiguration.COMMAND_REGEX_TIMEOUT) directly instead (engine/src/main/java/com/arcadedb/query/select/SelectOperator.java). Functionally fine since record.getDatabase() is never null there, but it's the one call site that doesn't go through the helper whose stated purpose (per its own javadoc) is exactly to centralize this null-safe fallback so call sites don't drift apart. Worth using the helper there too for consistency.
2. LikeOperator/ILikeOperator intentionally keep a per-row budget rather than per-scan (documented in-code as a conscious tradeoff, since BinaryCompareOperator has no CommandContext to cache a query-wide deadline on). Worth flagging explicitly: a WHERE ... LIKE scan over a huge table where many rows individually trigger catastrophic backtracking is no longer indefinite (each row is capped at regexTimeout), but the aggregate worst case is still rowCount * regexTimeout rather than one shared budget - an asymmetry with MatchesCondition/RegexExpression. Given it's called out as a deliberate scope decision (widening BinaryCompareOperator was judged out of proportion for this fix), leaving it as-is seems reasonable, but might be worth a tracking issue if LIKE DoS resistance at scale matters later.
3. Default arcadedb.command.regexTimeout = 1000ms is shared per query execution / per scan, not per call, for MATCHES/=~/full-text/PromQL/REGEXP validation. That's a real behavior change for legitimate (non-catastrophic) heavy workloads - e.g. a MATCHES query scanning a very large table, or REGEXP validation against very large field values, could now fail with TimeoutException where it previously just ran slowly. This is already called out extensively in the release notes and the config javadoc, which is great - just worth making sure it's prominent in the upgrade/breaking-changes notes too, since a default that turns a previously-succeeding (if slow) write/query into a hard failure post-upgrade is the kind of thing that surprises people in production.

Nit

• The release notes doc narrates the review process itself ("A third review pass corrected a wrong conclusion from the first...", up through a 9th pass). Great for traceability during development, but might be worth trimming before merge so the published release notes read as user-facing documentation rather than an internal changelog of the review iterations.

No correctness bugs or security gaps found in the reviewed logic. Solid work closing out a real DoS vector across every regex entry point in the engine, including some beyond the original issue report (schema REGEXP validation, full-text RegexpQuery/WildcardQuery, PromQL label matchers).

[lvca]

Pushed 4cab15f. This round came back clean — "No correctness bugs or security gaps found" — with three minor, non-blocking points, all addressed:

1. SelectOperator bypassing the new getValueAsLong(Database) helper: fixed. Switched both like/ilike cases to it. Functionally identical either way (record.getDatabase() is never null there), but the helper exists specifically to keep this lookup from drifting across call sites, so it should be used everywhere, not everywhere-but-one.

2. LIKE/ILIKE residual per-row budget & broad StackOverflowError catch: no new action — this round's framing reads as re-confirming both as acceptable, already-documented trade-offs from earlier rounds rather than raising something new.

3. Release notes reading as an internal changelog: rewritten. Fair nit — ten review rounds had left the notes narrating "a 6th review pass found...", "a 7th review pass caught...", which is exactly the kind of process detail that belongs in this PR thread, not in what ships as user-facing documentation. Rewrote the whole section as a single, direct description of the final state: what's covered, how the deadline-sharing design works (including the one documented exception for LIKE/ILIKE), and the upgrade-behavior tradeoff — with none of the round-by-round narration. All the substantive technical content survived the rewrite, just not the "which review pass found it" framing.

All tests green: SelectExecutionTest (25), SelectOperatorsExtendedTest (12).

Ten review rounds, two of which caught genuine, independently-verified bugs in the fix itself (not just missed entry points), and this one found none. I think this is a solid stopping point unless something new surfaces.

[claude]

Review: ReDoS hardening for regex-based query/validation surfaces (#5886)

This is a well-scoped, thoroughly-tested fix for a real DoS vector (CWE-1333, catastrophic regex backtracking). The design is sound: TimeBoundRegex intercepts CharSequence.charAt(), the one call site java.util.regex actually makes on every backtracking step, since Matcher never polls interrupts or a deadline on its own. The arcadedb.command.regexTimeout deadline-sharing across a scan/query (rather than per-item) is the right call, and the fix is applied consistently across a genuinely wide set of entry points: SQL MATCHES, openCypher =~, LIKE/ILIKE (both the parser operators and the native SelectOperator path), text.regexReplace(), .normalize()'s pattern argument, full-text RegexpQuery/WildcardQuery, PromQL label matchers, and schema-level REGEXP validation. The last one (reachable from any write path with no query privileges) is the most important catch in this PR - good find.

Code quality

• TimeBoundRegex (engine/src/main/java/com/arcadedb/utility/TimeBoundRegex.java) is clean and well documented. The newDeadline() overflow guard (Math.addExact/Math.multiplyExact falling back to Long.MAX_VALUE) correctly treats an oversized admin-configured timeout as "unbounded" rather than accidentally landing a deadline in the past.
• Sharing the mutable calls counter array across subSequence()-derived DeadlineBoundCharSequence instances (instead of resetting per subsequence) is a subtle but correct detail - without it, a pattern that leans on lookaheads/regions could reset the check cadence and dodge the bound.
• RegexDeadlineExceeded as a stack-trace-less singleton RuntimeException is the right choice given it can be thrown extremely often on one runaway match.
• The in-flight review history baked into the comments (collision between the MATCHES_ pattern-cache key and a naive MATCHES_DEADLINE key; RegexExpression's instance-field deadline going stale across CypherStatementCache-reused AST nodes) shows real edge cases were caught and fixed before merge, not just theorized about.

Potential issues / follow-ups

1. Inconsistent exception contract for the timeout itself. TextRegexReplace catches TimeoutException from TimeBoundRegex and rewraps it as IllegalArgumentException (to preserve its pre-existing contract), but SQLMethodNormalize's new pattern-argument path lets TimeoutException propagate directly (confirmed by SQLMethodNormalizeTest#catastrophicPatternArgumentIsAbortedByRegexTimeout, which asserts TimeoutException, not IllegalArgumentException). Two SQL string functions in the same family now report the identical failure mode differently to callers. Worth a follow-up to unify, or at least a doc note on text.normalize().
2. LIKE/ILIKE are bounded per-row, not per-scan. This is called out explicitly in the PR description/release notes as a conscious scope decision (BinaryCompareOperator has no CommandContext to cache a shared deadline on), but it does mean a WHERE ... LIKE scan over a table shaped so every row is catastrophic can still cost up to rowCount * regexTimeout (default 1000ms) before fully aborting - i.e., minutes on a large table, versus the single bounded operation MATCHES/=~/full-text/PromQL now get. Given LIKE is the far more commonly used operator of the two, this residual gap seems worth a tracked follow-up issue rather than staying as a comment.
3. Default-on behavior is a breaking change for legitimately slow queries. With arcadedb.command.regexTimeout defaulting to 1000ms and shared per scan, a large table LIKE/MATCHES scan, a big full-text/PromQL scan, or REGEXP validation against a large field that previously just ran slowly to completion will now throw TimeoutException after upgrade. This is documented in the release notes' "Upgrade note," which is good, but given it changes behavior for non-malicious existing workloads out of the box, it'd be worth a one-line callout in the top-level changelog/release highlights too, not just the detailed section.
4. Minor, non-blocking: DocumentValidator.validateField() still compiles Pattern.compile(p.getRegexp()) fresh on every single insert/update rather than caching it on the Property (pre-existing behavior, not a regression from this PR - String.matches() did the same compile-per-call before). Since this code path is already being touched for the ReDoS fix and is on the hot insert/update path, it could be a good opportunistic follow-up to cache the compiled Pattern on Property alongside this fix.

Performance

The zero-cost-when-disabled design (newDeadline() returns a sentinel that skips wrapping entirely) is good, but note the default is enabled (1000ms), so by default every regex match in the engine now goes through the DeadlineBoundCharSequence indirection on charAt() (extra virtual dispatch + bitmask branch + shared-array increment every 256th call is cheap, but it's still overhead added to a hot path touched by LIKE/MATCHES/full-text everywhere). Given CLAUDE.md's performance mandate, it'd be worth a quick before/after benchmark on a LIKE-heavy or full-text-heavy workload to confirm the overhead is negligible in practice, since this is now unconditionally in the hot path for the overwhelming majority of installs that never change the default.

Security

Solid improvement. Confirms LIKE/ILIKE's %/?-only translation isn't as "structurally safe" as an initial audit concluded (sequential .* segments alone reproduce catastrophic backtracking without grouping/alternation) - good catch, and the SQLMethodSplit delimiter fix (switching to Pattern.quote() to match its three siblings) closes a real, separate gap where a delimiter was passed unescaped to String.split(regex).

Test coverage

Excellent - TimeBoundRegexTest unit-tests the utility in isolation (including the overflow edge case), and every modified entry point gets its own regression test proving the abort happens near the configured deadline rather than the tens-of-seconds an unbounded catastrophic match takes. The deadline-sharing tests (multiValueMatchesSharesOneTimeoutBudgetAcrossItems, matchesSharesOneTimeoutBudgetAcrossAllRowsInTheScan, regexSharesOneTimeoutBudgetAcrossAllRowsInTheScan, the cache-key-collision regression, and the stale-deadline-across-cached-Cypher-executions regression) are exactly the kind of tests that catch the subtle bugs this class of fix is prone to, and it's clear from the comments they were added because those bugs were actually hit during development.

Overall: nice, careful piece of security engineering. The open items above are refinements/follow-ups, not blockers.

[lvca]

Pushed 989392f. Thanks for the thorough final pass — this is a great summary to close on.

1. Inconsistent exception contract (TextRegexReplace rewraps, .normalize() doesn't): documented, not unified. Added a code comment explaining the reasoning rather than changing behavior: TextRegexReplace rewraps to IllegalArgumentException because it already had that contract before this issue (it already converted PatternSyntaxException the same way, for invalid patterns). .normalize()'s pattern argument had no such prior contract — letting TimeoutException propagate as-is is actually the more consistent choice across this PR as a whole, since that's the shape MatchesCondition, RegexExpression, LIKE/ILIKE, full-text, and PromQL all surface. Unifying in the other direction (making TextRegexReplace also throw raw TimeoutException) would be a breaking change to an existing, pre-issue contract for a function that predates this PR, which felt like the wrong tradeoff for a security fix.

2. LIKE/ILIKE per-row budget as a tracked follow-up issue: agreed this is worth tracking separately rather than living only as a code comment, given LIKE is likely the more commonly used operator. Deliberately not filing a new GitHub issue myself here — that's a call for @lvca as maintainer, and it's clearly framed as a follow-up rather than something blocking this PR.

3. Upgrade-behavior callout prominence: looked at the document structure — there's no separate top-level "highlights" list this section reports into; each ## heading is its own self-contained unit, and the "Upgrade note" is already a clearly-labeled, bolded paragraph within it (same pattern the rest of this release-notes file uses for callouts). Didn't see anywhere else in the doc to duplicate it into without inventing a mechanism that doesn't otherwise exist here.

4. DocumentValidator not caching the compiled Pattern per-Property: agreed this is a good opportunistic follow-up, but declined for this PR specifically — it's explicitly pre-existing behavior (not a regression from this fix), and expanding scope into a performance optimization unrelated to the security fix risks the kind of scope creep this project's own guidelines caution against. Worth its own follow-up if it matters in practice.

All tests green (SQLMethodNormalizeTest unaffected by the doc-only change).

This round confirmed no bugs and no blockers — just refinements. Across 11 rounds: 2 real, independently-verified bugs in the fix itself (a RegexExpression stale-deadline regression and a MatchesCondition cache-key collision, both caught and fixed before merge), and the regex-entry-point audit grew from the 2 named in the original issue to 11 (MATCHES, =~, LIKE/ILIKE across 3 independent evaluation paths, text.regexReplace(), .normalize(), .split(), full-text RegexpQuery/WildcardQuery, PromQL =~/!~, and schema REGEXP validation). I believe this is ready to merge.

PR: #5983

[claude]

Review

This is a thorough, well-tested fix for a real ReDoS gap (java.util.regex never checks a deadline while backtracking). TimeBoundRegex's charAt()-interception approach is a sound way to reach into the one place the JDK regex engine offers a hook, the CHECK_INTERVAL bitmask check keeps the hot path cheap, and the overflow-safe deadline math and shared subSequence() counter are handled correctly. The commit history shows the "share one deadline across a whole scan, not per item" principle was applied carefully to MATCHES, LIKE/ILIKE, full-text RegexpQuery/WildcardQuery, and PromQL's post-filter loop. That said, the same principle looks like it was missed at three more call sites, one of which the PR itself calls the most exposed surface.

1. DocumentValidator.validateField doesn't share a deadline across a document's properties (engine/src/main/java/com/arcadedb/database/DocumentValidator.java:65-66)

if (!TimeBoundRegex.matches(Pattern.compile(p.getRegexp()), fieldValue.toString(),
    GlobalConfiguration.COMMAND_REGEX_TIMEOUT.getValueAsLong(document.getDatabase())))

validate() loops over every property of a document and calls validateField once per property (DocumentValidator.java:41-43). Each call to TimeBoundRegex.matches(...) computes a brand-new deadline via newDeadline(timeoutMillis) internally, there's no caching/sharing the way MatchesCondition and RegexExpression cache their deadline on the CommandContext. So a single document type with, say, three REGEXP-constrained string properties (email/phone/zip-style validation is exactly where this shows up), each holding a crafted value, can tie up the thread for up to 3 x regexTimeout on one document write, the identical "N x timeout" pattern this PR eliminates everywhere else, just reintroduced at the property level.

This is also the entry point the PR's own description calls out as the most exposed ("reachable with no query privileges whatsoever... through any write path"), so the gap matters more here than elsewhere.

Separately, docs/release-26.9.1.md states "full-text, PromQL, and REGEXP validation share one deadline across the whole scan they run against" - based on the code, that's not accurate for REGEXP validation; there's no scan/shared-deadline concept there at all, each property validation is fully independent. Worth correcting the release note regardless of whether the sharing gets implemented.

2. PromQL range queries recompute the deadline per step, not once per query (engine/src/main/java/com/arcadedb/engine/timeseries/promql/PromQLEvaluator.java:135-136, 205, 252)

evaluateRange() loops for (long t = startMs; t <= endMs; t += stepMs) calling evaluate(expr, t, ...), which for a VectorSelector/MatrixSelector re-enters evaluateVectorSelector/evaluateMatrixSelector, and each of those computes its own fresh regexDeadline (lines 205 and 252). The deadline is correctly shared across all rows within one step's scan, but not across steps. MAX_RANGE_STEPS is 1,000,000 (line 73), so a range query using a crafted =~/!~ label matcher could in the worst case cost up to stepCount x regexTimeout, at the 1000ms default that's a theoretical ~11.5 days, far beyond the "one deadline per scan" bound the docs describe. In practice a real range query would likely hit other limits first, but the bound as implemented doesn't actually cap total time the way it does for evaluateInstant/full-text/MATCHES. Worth threading a single deadline through evaluateRange's loop (e.g. computed once outside the for, similar to how evaluateVectorSelector shares it across rowIter) if that's easy to do without restructuring the recursive evaluate().

3. TextRegexReplace and SQLMethodNormalize have a CommandContext available but don't use it to share a deadline across rows (engine/src/main/java/com/arcadedb/function/text/TextRegexReplace.java:80, engine/src/main/java/com/arcadedb/query/sql/method/string/SQLMethodNormalize.java:63)

Both functions receive CommandContext context and both recompute GlobalConfiguration.COMMAND_REGEX_TIMEOUT.getValueAsLong(...) (and thus a fresh deadline) on every call. Unlike LikeOperator/ILikeOperator, which the PR explicitly documents as a conscious per-row trade-off because BinaryCompareOperator.execute() has no CommandContext to cache on, these two functions do have the context MatchesCondition/RegexExpression use for exactly this purpose (context.getCachedValue/setCachedValue), so the same "cache the deadline on the context, computed once per query" fix used for MATCHES/=~ seems directly applicable. As-is, SELECT text.regexReplace(content, :pattern, 'x') FROM LargeType (or .normalize(form, pattern)) with a pathological pattern and many matching rows is bounded per-row (rowCount x regexTimeout) rather than per-query, and this isn't called out anywhere as an accepted trade-off the way the LIKE/ILIKE one is.

Minor / non-blocking

• MatchesCondition.java:96 - the deadline cache key is the literal "5886_MATCHES_CONTEXT_DEADLINE". The comment explaining why it can't collide with the "MATCHES_" + <regex text> pattern-cache namespace is good and worth keeping, but embedding the GitHub issue number in a production cache key string is an odd thing for a future reader to parse. Something like "__MATCHES_DEADLINE__" (still provably outside the "MATCHES_" prefix) would read more clearly without losing the collision-avoidance property.
• GlobalConfiguration.getValueAsLong(Database) allocates a new ContextConfiguration() on every call when database is null. Only exercised on the null-database fallback paths (direct/unit-test invocations), so not a hot-path concern, just noting it in case any of those fallback paths turn out to be hotter than assumed.
• DocumentValidator.validateField now allocates a DeadlineBoundCharSequence plus backing int[1] on every REGEXP-validated write (when regexTimeout > 0, the default). Given arcadedb.command.regexTimeout defaults to a nonzero 1000ms, this is extra GC pressure on the document-insert/update hot path for every write touching a REGEXP-constrained property, even for entirely benign patterns. Likely an acceptable cost for the security benefit, but flagging per the project's performance-first guidance.

Test coverage

Given how carefully the multi-item/multi-row sharing behavior was tested for MATCHES, LIKE/ILIKE's BY-ITEM branch, full-text, and PromQL's per-row post-filter, the three gaps above are also gaps in the test suite: there's no test with more than one REGEXP-validated property on one document, no multi-step PromQL range-query regression test, and no multi-row text.regexReplace()/.normalize() regression test. Tests analogous to MatchesConditionTest#matchesSharesOneTimeoutBudgetAcrossAllRowsInTheScan for these three sites would both prove the current per-item behavior and pin down a fix if one is made.

Overall

The core mechanism (TimeBoundRegex) and its application to the primary MATCHES/=~/LIKE paths are solid and well-tested. The three gaps above are all instances of the exact "shared deadline across repeated evaluations" principle this PR establishes and applies rigorously elsewhere, they read like the same class of oversight the PR's own review process caught repeatedly for other entry points, just not yet caught for these three.

[lvca]

Pushed 8e3a0f5. All three points were real, valid instances of the exact principle this PR establishes elsewhere — fixed all of them.

1. DocumentValidator not sharing a deadline across a document's properties: fixed. This is the entry point the PR itself calls "most exposed" (no query privileges needed), so this gap mattered more here than anywhere else. validate() now computes one deadline and threads it through validateField() via a new 3-arg overload; the existing 2-arg validateField() (no evidence of external callers, but it's public API) keeps its own per-call deadline for standalone single-field use. Corrected the release note's claim that REGEXP validation already shared a deadline — it didn't until this commit, now it does.

2. PromQL evaluateRange() recomputing the deadline per step: fixed. You're right this was a real gap, not just theoretical — at the 1000ms default, MAX_RANGE_STEPS (1,000,000) times over is genuinely ~11.5 days worst case. Fixed by caching the deadline as a PromQLEvaluator instance field, computed once lazily. Confirmed this is safe (unlike the earlier RegexExpression AST-instance-field mistake) by checking PromQLEvaluator's only construction site (SQLFunctionPromQL) — a fresh evaluator is created per top-level query, never cached/reused across executions the way RegexExpression's AST nodes are by CypherStatementCache. That distinction is exactly what made an instance field wrong there and right here.

3. TextRegexReplace/.normalize() having a CommandContext but not using it: fixed. Both now share one deadline across a query's rows via context.getCachedValue/setCachedValue, the same pattern MatchesCondition/RegexExpression already use. Added TimeBoundRegex.replaceAllUntil() (the deadline-based counterpart to replaceAll(), mirroring matches()/matchesUntil()) to support it.

Minor cache-key naming: fixed too, since I was touching that file anyway — MatchesCondition's deadline key is now "__MATCHES_DEADLINE__" instead of embedding the issue number, per your earlier suggestion.

New tests for all four: DocumentValidationTest#regExpValidationSharesOneTimeoutBudgetAcrossAllPropertiesOnOneDocument, PromQLEvaluatorIntegrationTest#rangeQuerySharesOneTimeoutBudgetAcrossAllSteps, TextRegexReplaceTest#multiRowReplaceSharesOneTimeoutBudgetAcrossRows, SQLMethodAdditionalCoverageTest#normalizeMultiRowSharesOneTimeoutBudgetAcrossRows, TimeBoundRegexTest#replaceAllUntilSharesOneDeadlineAcrossASeries. All follow the same "N independent budgets would exceed X, a shared deadline keeps it under X" proof pattern as the earlier multi-row/multi-value tests. Full affected suite (135 tests) plus a broader sweep (CRUDTest, SQLFunctionsTest, QueryTest) all green.

This was a genuinely valuable round — three real architectural gaps in the core principle this PR is built around, all now closed consistently across every entry point.

[claude]

Review: ReDoS hardening for MATCHES/=~/LIKE/full-text/PromQL/REGEXP

This is a well-scoped, thoroughly tested fix for a real DoS vulnerability. I checked out the merge ref locally, read every changed file (not just the diff), and cross-verified the trickier claims against the current code rather than taking the PR description at face value. Summary below.

What's solid

• Correct root cause fix. java.util.regex's Matcher never polls an interrupt or deadline while backtracking; the only hook available is CharSequence.charAt(), and TimeBoundRegex's DeadlineBoundCharSequence wrapper is the right place to intercept it. Verified matches() and replaceAll() both go through charAt() for the reproducer shapes used in the tests.
• Overflow-safe deadline math (newDeadline, engine/src/main/java/com/arcadedb/utility/TimeBoundRegex.java): Math.addExact/multiplyExact with a Long.MAX_VALUE fallback so an admin setting an oversized regexTimeout can't accidentally wrap into a deadline in the past. Good defensive touch, and it's tested (newDeadlineDoesNotOverflowOnAnOversizedTimeout).
• Null-database fallback is centralized via the new GlobalConfiguration.getValueAsLong(Database). Traced ContextConfiguration's no-arg constructor ("just a proxy for the GlobalConfiguration, no values set") and confirmed getValueAsLong on an empty one correctly falls through to the compiled-in default, so every "database can be null in direct/unit-test invocations" comment sprinkled through this PR actually holds up.
• Shared-deadline design is the right call. Per-row/per-item timeouts would let an attacker multiply the cost by row/item count (N * regexTimeout); caching one deadline on the CommandContext (SQL MATCHES, text.regexReplace(), .normalize()), on the PromQLEvaluator instance, or across a full-text/schema-validation scan closes that. This is genuinely easy to get subtly wrong, and it shows: the PR's own commit history caught and fixed two real bugs before merge - a cache-key collision ("MATCHES_DEADLINE" colliding with the pre-existing "MATCHES_" + <regex text> pattern-cache key when the regex literal was DEADLINE) and a stale-deadline-on-cached-AST bug in RegexExpression (Cypher's CypherStatementCache reuses AST nodes across executions, so an instance-field deadline would go stale and start throwing spurious timeouts on ordinary, non-catastrophic matches on the second execution of the same cached query text). Both have regression tests now.
• SQLMethodSplit's unescaped delimiter (String.split(regex) instead of Pattern.quote(...)) is a legitimate related bug catch - it was the odd one out among split()/text.split()/Cypher split(), all of which already quote the delimiter.
• Tests are timing-based but written with generous margins (asserting "aborted near the ~200ms deadline" via an upper bound like 1-5s, not a tight window), which should keep them from being flaky on a loaded CI runner while still catching a real regression (the N-times-timeout tests assert well under what 10 independent budgets would take).

Points worth a second look

1. Default behavior change for existing large scans. Because the deadline is shared per query/scan (not per match) and defaults to an enabled 1000ms, a legitimate non-catastrophic MATCHES/full-text/PromQL/REGEXP-validation operation over a large table can now fail with TimeoutException where it previously just ran slowly to completion. This is called out in the release notes' "Upgrade note," which is good, but it's a real production-risk vector on upgrade for anyone with big tables and non-trivial patterns - worth flagging prominently, not just in the docs page, since it's a behavior change with a fairly aggressive default (1s) rather than a pure bug fix.
2. New allocation on every regex/LIKE/MATCHES/full-text-wildcard evaluation. Since the default regexTimeout is 1000 (enabled), TimeBoundRegex.run() now wraps the input in a DeadlineBoundCharSequence (plus its backing int[1] counter) on essentially every call, even trivial already-fast matches - an allocation plus a layer of virtual dispatch added to a hot path (QueryHelper.likeUntil in particular, which already re-compiles the Pattern on every call - pre-existing behavior, not touched here). Given this codebase's stated performance/GC sensitivity, this is a reasonable trade-off for closing a real DoS, but worth confirming perf tests don't regress meaningfully for LIKE/MATCHES-heavy workloads before release.
3. Minor: StackOverflowError -> TimeoutException conversion is a bit blunt. In TimeBoundRegex.run(), when the bound is active, any StackOverflowError during the match gets relabeled as a TimeoutException with a "regexTimeout" message, even one caused by unrelated deep recursion unconnected to catastrophic backtracking. Low severity, but the resulting error message/log could be mildly misleading.
4. Minor duplication: the "check CommandContext.getCachedValue(key), compute via TimeBoundRegex.newDeadline(...) if absent, setCachedValue" get-or-compute pattern is hand-copied with slightly different literal keys in MatchesCondition, RegexExpression, TextRegexReplace, and SQLMethodNormalize. Given that this exact pattern is what produced the cache-key-collision bug caught during review, a small shared helper would remove the need for every future call site to pick its own collision-free key by hand. Not a blocker, just a place a future call site could reintroduce the same class of bug.

Testing

Compiled and cross-checked call sites against the current tree (checked out the PR merge ref) rather than running the full Maven build in this sandbox (build tooling wasn't available here). The test suite itself looks comprehensive: a dedicated TimeBoundRegexTest for the utility, plus regression tests at every touched call site (MatchesConditionTest, OpenCypherWhereClauseTest, QueryHelperLikeRegexTimeoutTest, SelectExecutionTest, FullTextQueryExecutorTest, PromQLEvaluatorIntegrationTest, DocumentValidationTest, TextRegexReplaceTest, SQLMethodNormalizeTest, SQLMethodSplitTest) covering both "aborts near the deadline" and "shares one budget across N rows/items/steps/properties". No gap found in entry-point coverage; a grep for other Pattern.compile/.matches(/.split( sites in engine and the wire-protocol modules didn't turn up other user-controlled-regex call sites outside what's already handled here.

Overall: solid fix, worth merging. The main thing I'd want a second pair of eyes on before release is point 1 (default-on behavior change for existing large-scan workloads) and confirming perf numbers for point 2.

[lvca]

Addressed the duplication point from the last review round: MatchesCondition, RegexExpression, TextRegexReplace, and SQLMethodNormalize all hand-rolled the same get-cached-or-compute-and-cache-deadline pattern with their own literal cache key - exactly the shape that produced the earlier cache-key-collision bug. Extracted it into a shared CommandContext.getOrComputeRegexDeadline(String cacheKey) default method (and a (cacheKey, timeoutMillis) overload for the no-CommandContext-available null-safe fallback paths), and migrated all four call sites onto it. Each call site still owns its own collision-free key, but no longer hand-writes the get-or-compute logic.

Compiled clean and re-ran the full affected suite (TimeBoundRegexTest, MatchesConditionTest, OpenCypherWhereClauseTest - including the stale-deadline regression test - TextRegexReplaceTest, SQLMethodNormalizeTest, SQLMethodAdditionalCoverageTest, SQLMethodSplitTest, DocumentValidationTest, PromQLEvaluatorIntegrationTest, FullTextQueryExecutorTest, QueryHelperLikeRegexTimeoutTest, SelectExecutionTest, LikeOperatorTest, ILikeOperatorTest): all green, no regressions.

On the other reconfirmed-but-not-actioned points from the last round: no code change intended.

• Default arcadedb.command.regexTimeout of 1000ms was an explicit decision, not an oversight.
• The per-context allocation overhead of the shared-deadline caching was already measured negligible relative to a regex evaluation itself.
• TimeBoundRegex converting a StackOverflowError into a TimeoutException (only when the bound is active) is a deliberate, narrowly-scoped choice: that class exists specifically to bound catastrophic-backtracking cost, and a stack overflow under an active deadline is itself evidence of the same catastrophic-backtracking class of pattern, so surfacing it as the same TimeoutException the caller already handles is more useful than a raw StackOverflowError escaping.

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 18 complexity

Metric	Results
Complexity	18

View in Codacy

🟢 Coverage 86.90% diff coverage · -6.56% coverage variation

Metric	Results
Coverage variation	✅ -6.56% coverage variation
Diff coverage	✅ 86.90% diff coverage

View coverage diff in Codacy

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (46c507f)	153675	119612	77.83%
Head commit (9124855)	186103 (+32428)	132639 (+13027)	71.27% (-6.56%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#5983)	84	73	86.90%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[claude]

Review: bound SQL MATCHES/openCypher =~ against catastrophic regex backtracking (#5886)

This is a well-scoped, well-tested security fix. Overall assessment: solid, ready to merge modulo a couple of minor points below.

Strengths

• Correct mechanism. TimeBoundRegex exploits the one interception point java.util.regex actually offers - CharSequence.charAt() on the input - since Matcher never polls interrupts/deadlines while backtracking. The DeadlineBoundCharSequence wrapper with a bitmask-gated System.nanoTime() check every 256 calls is a sensible way to keep the check off the hot path while still bounding runaway matches.
• Deadline sharing is handled correctly and is non-trivial to get right. The PR consistently avoids the "N-times-timeout" trap (each row/item getting its own fresh budget, so a crafted table costs rowCount * regexTimeout) by sharing one deadline across a whole scan/query via CommandContext.getOrComputeRegexDeadline, a PromQLEvaluator instance field, or an explicitly threaded deadline parameter, as appropriate per call site.
• Two real bugs were caught and fixed in the process, both evidenced by regression tests that would fail without the fix:
  • The original "MATCHES_DEADLINE" cache key collided with the pattern cache's "MATCHES_" + <regex text> keys for the literal pattern DEADLINE (ClassCastException) - fixed by moving the deadline key out of that namespace (MatchesConditionTest#patternTextDeadlineDoesNotCollideWithTheDeadlineCacheKey).
  • Caching the deadline as an instance field on RegexExpression would have gone stale across repeated executions of a CypherStatementCache-cached query (a benign match would start throwing spurious TimeoutException after the first execution's deadline lapsed) - fixed by caching on CommandContext instead (OpenCypherWhereClauseTest#regexDeadlineDoesNotLeakAcrossCachedQueryExecutions). Good catch, this is a subtle one.
• The audit went further than the original "LIKE/ILIKE is structurally safe" conclusion and correctly found that a sequence of unparenthesized .*/* segments (no grouping/alternation/nested quantifiers needed) reproduces the same catastrophic-backtracking shape. That same gap was independently rediscovered and closed for SQL LIKE/ILIKE, full-text WildcardQuery, and PromQL's REDOS_CHECK static pre-filter (which only flags parenthesized shapes). This is exactly the kind of thing that's easy to miss in a first pass.
• SQLMethodSplit fix is a legitimate, well-identified bug, not scope creep: the other three siblings (split() function, text.split(), Cypher split()) already wrap the delimiter in Pattern.quote(); SQLMethodSplit was the odd one out, passing the delimiter straight to String.split(regex). Bringing it in line removes the risk entirely rather than just bounding it, and is documented as a behavior change in the release notes with its own regression test.
• Test coverage is extensive: every modified entry point gets a "matches/replaces normally" test, a "catastrophic pattern is aborted near the deadline" test, and (where relevant) a "shared budget across items/rows/properties/steps" test with an assertion that would fail if the sharing regressed to per-item budgets. newDeadline's overflow handling for an oversized arcadedb.command.regexTimeout is also covered.
• Correctly avoids extending NeedRetryException, so a regex timeout won't get silently retried into another timeout by any commit-retry machinery.
• No new dependency, no System.out debug leftovers, license headers present, config setting placed/documented consistently with the existing GlobalConfiguration style.
• I independently swept engine, server, and all wire-protocol modules (gremlin, graphql, mongodbw, redisw, postgresw, bolt, grpcw) for other user-reachable java.util.regex call sites (Pattern.compile, .matches(, .matcher().find(), .replaceAll(, .split() that take an attacker-controlled pattern rather than just attacker-controlled input. Found nothing left unbounded: Gremlin has no custom regex-based TextP, GraphQL has no regex usage at all, and the remaining SQL/Cypher split-family functions already use Pattern.quote(...). Coverage looks complete for this issue's scope.

Minor points

1. DocumentValidator.validate() now unconditionally computes a deadline (System.nanoTime() + two arithmetic ops) on every document validation, even for document types with zero REGEXP-constrained properties (engine/src/main/java/com/arcadedb/database/DocumentValidator.java:48). That's the common case for most schemas. It's a small cost relative to the rest of validation/insert/update, but since this runs on the write hot path for every document, it might be worth only computing the deadline lazily (mirroring the lazy PromQLEvaluator.regexDeadline() field pattern already used elsewhere in this same PR) rather than unconditionally per document. Not a blocker, just flagging given the project's stated performance sensitivity.

2. Catching StackOverflowError in TimeBoundRegex.run() (and re-catching it in TextRegexReplace) is generally risky per JVM guidance (the JVM's state after a stack overflow is not fully guaranteed), though the intent here is reasonable - a sufficiently pathological pattern can blow the backtracking stack before the first 256-call checkpoint. Given this is narrowly scoped (only around the regex match/replace call, only rethrown as a typed exception, and the existing TextRegexReplace code already did this before the PR), I'd call this an acceptable, pre-existing-pattern tradeoff rather than a new problem - just flagging for awareness.

3. Upgrade/behavior-change note is good but worth double-checking in practice: the docs correctly call out that a legitimate slow (non-catastrophic) MATCHES/LIKE/full-text/PromQL/REGEXP operation over a large dataset can now fail with TimeoutException where it previously just ran slowly to completion, since the default arcadedb.command.regexTimeout is 1000ms and shared per query/scan rather than per match. This is a reasonable default and well-documented, but it's a behavior change existing deployments with large datasets and legitimately expensive regex-based scans should be aware of before upgrading.

4. Nit: the PR description says "LIKE/ILIKE were audited too ... Left unchanged," which reads as slightly stale relative to the final diff (LikeOperator/ILikeOperator/QueryHelper/SelectOperator were, in fact, changed once the "sequential wildcard" gap was found). docs/release-26.9.1.md has the accurate, up-to-date version. Might be worth updating the PR description body itself before merge so the summary and the actual diff agree.

Nice work on this one, especially the multi-pass self-review that caught the two real bugs above before merge.

[lvca]

Addressed the two easy, actionable points from the last round; the other two are genuinely non-blocking nits with no further action needed.

1. DocumentValidator.validate() unconditionally computing a deadline: fixed. Now computed lazily, only on the first REGEXP-constrained property actually encountered, mirroring the lazy PromQLEvaluator.regexDeadline() field pattern already used elsewhere in this PR. Document types with no REGEXP-constrained properties (the common case) no longer pay System.nanoTime() + the overflow-safe arithmetic on every insert/update. DocumentValidationTest (24 tests) still green.

4. Stale PR description: fixed. Rewrote it to describe the actual final scope (all covered entry points, deadline sharing, the two bugs caught during review) instead of the original narrow MATCHES/=~-only description that predated the audit growing to 11 entry points.

2. StackOverflowError catching risk: no change, per the review's own assessment - "narrowly scoped... I'd call this an acceptable, pre-existing-pattern tradeoff rather than a new problem." Agreed, and this exact tradeoff was reconfirmed in the prior round's response too.

3. Upgrade-note double-check: no code change - this is a request to verify behavior in practice before release, not a code or doc gap. The upgrade note itself is already accurate and prominent (bolded paragraph in docs/release-26.9.1.md).

Compiled clean, DocumentValidationTest re-run green, pushed as 9ed0608.

[claude]

Review: ReDoS mitigation for regex entry points (#5886)

This is a well-executed, security-critical change. I read through TimeBoundRegex and every listed call site (MatchesCondition, RegexExpression, LikeOperator/ILikeOperator, QueryHelper, SelectOperator, DocumentValidator, TextRegexReplace, SQLMethodNormalize/SQLMethodSplit, FullTextQueryExecutor, PromQLEvaluator) plus the associated tests, and traced the shared-deadline mechanism through the actual SQL/Cypher execution paths.

Correctness

• TimeBoundRegex itself is solid: the charAt()-interception trick is the only interception point java.util.regex offers, the bitmask check interval keeps the hot path cheap, newDeadline() is overflow-safe (falls back to "unbounded" rather than an already-expired deadline on overflow, the right direction for an admin-configurable value), and the StackOverflowError -> TimeoutException conversion is narrowly scoped and correctly excluded when the bound is explicitly disabled.
• The "shared deadline across a scan/query" claim is empirically verified, not just asserted: MatchesConditionTest#matchesSharesOneTimeoutBudgetAcrossAllRowsInTheScan, OpenCypherWhereClauseTest#regexSharesOneTimeoutBudgetAcrossAllRowsInTheScan, and the equivalent tests for LIKE/full-text/PromQL/REGEXP-validation all run a real 10-item scan and assert on wall-clock time, which is the right way to catch a regression in the sharing mechanism itself (a unit test that just calls getOrComputeRegexDeadline() twice wouldn't catch a per-row context being recreated upstream).
• The two bugs called out in the PR description as caught during review (deadline/pattern cache key collision in MatchesCondition, and the instance-field deadline leaking across CypherStatementCache-reused RegexExpression AST nodes) both have dedicated regression tests (patternTextDeadlineDoesNotCollideWithTheDeadlineCacheKey, regexDeadlineDoesNotLeakAcrossCachedQueryExecutions) that would fail without the fix, good evidence the fixes are real, not just described.

One nuance worth a look (not a security hole)

I traced CommandContext identity through the actual scan/filter execution paths to check whether "one deadline for the whole query" is literally true. For the sequential path (the default, and what all the tests above exercise) it is: SelectExecutionPlan/CypherExecutionPlan build one context and every FilterStep/ScanWithFilterStep/FilterOperator row-evaluation reuses that same instance, so getOrComputeRegexDeadline() computes once and every row shares it.

For SQL's parallel bucket scan, though, FetchFromTypeExecutionStep.syncPullParallel calls context.copy() once per bucket/worker (not per row), and each worker thread then calls getOrComputeRegexDeadline() on its own workerContext. So a type spread across N buckets scanned in parallel gets N independent regexTimeout budgets, not one, i.e. worst case is N_buckets * regexTimeout, not regexTimeout. This is a much smaller gap than the vulnerability being closed (bucket count is bounded and admin-controlled, not attacker-controlled the way a regex pattern is), so I don't think it's blocking, but it does slightly undercut the "shares one deadline across an entire query execution" framing in CommandContext.getOrComputeRegexDeadline's Javadoc and the release notes. Might be worth either a one-line caveat in the docs, or a test that exercises .parallel() explicitly if the intent really is a single query-wide budget regardless of execution mode.

Everything else

• Entry-point coverage looks complete, I grepped for other Pattern.compile(...)/.matches(...) call sites in engine/src/main/java outside the modified files; the remaining ones all compile a fixed, hardcoded pattern (not attacker-controlled), which is a different threat model than what this issue addresses, so they're correctly out of scope.
• The LIKE/ILIKE per-row (rather than per-scan) budget is a reasonable, clearly-documented tradeoff given BinaryCompareOperator has no CommandContext to cache on, widening that interface for this alone would be disproportionate.
• The SQLMethodSplit delimiter fix (literal via Pattern.quote() instead of raw String.split(regex)) is a real behavior change but brings it in line with its three siblings, and is called out in the release notes.
• Docs (docs/release-26.9.1.md) are thorough and include an explicit upgrade note that a legitimately slow (non-catastrophic) operation can now hit the new 1000ms default, good, since that's a real behavior change for large-scan workloads.

Test coverage

Excellent, every covered entry point has both a "matches/replaces normally" test and a "catastrophic pattern aborted near the deadline" test, and every entry point with shared-budget semantics has a dedicated multi-row/multi-item timing test that would fail (>=2000ms instead of <1000ms) if the sharing regressed. TimeBoundRegexTest covers the utility in isolation including the overflow edge case.

Nice work on this one, the audit trail through the review passes (documented inline as "Nth review pass") and the regression tests that specifically target bugs caught during that process are a good pattern.

[lvca]

Verified the parallel-bucket-scan nuance from the last round rigorously before deciding how to respond - traced it end to end:

• FetchFromTypeExecutionStep.syncPullParallel calls context.copy() once per bucket/worker (engine/src/main/java/com/arcadedb/query/sql/executor/FetchFromTypeExecutionStep.java:282).
• BasicCommandContext.copy() does not carry cachedValues over to the copy - it's left null on the new instance (engine/src/main/java/com/arcadedb/query/sql/executor/BasicCommandContext.java:401-415).

So the claim is correct: each parallel worker's getOrComputeRegexDeadline() computes its own deadline from scratch, independent of every other worker. A type scanned across N buckets in parallel is bounded by N * regexTimeout overall, not one shared budget the way the sequential scan path is.

Not fixing the sharing itself. I looked at actually sharing one deadline across workers and concluded it would trade a narrow, non-attacker-controlled gap for a real bug: cachedValues is a plain HashMap, not thread-safe, and getOrComputeRegexDeadline's check-then-compute-then-set has no synchronization. Aliasing that same map across N worker threads (the way inputParameters is deliberately aliased, since it's read-only during execution) would let two workers race on populating the same key on first use - a real concurrency bug, for a gap the review itself already assessed as "not a security hole" and "much smaller... bucket count is bounded and admin-controlled, not attacker-controlled." Trading a documented, bounded gap for an actual race condition isn't a good trade.

Fixed instead: corrected the "shares one deadline across an entire query execution" claim - both the CommandContext.getOrComputeRegexDeadline() Javadoc and the release notes' "Deadline sharing" section now explicitly caveat the parallel-scan case, with the reasoning above for why it's accepted as-is rather than fixed.

Compiled clean; this is a doc/Javadoc-only change with no test impact. Pushed as 96d3376.

[claude]

Review: bound regex against catastrophic backtracking

This is a large, iteratively self-reviewed PR (18 commits, each addressing a previous review round), and it shows: the commit history documents several real bugs (the MATCHES_DEADLINE cache-key collision, the RegexExpression instance-field deadline going stale across CypherStatementCache reuse, the missing wildcard/full-text/PromQL/schema-validation entry points) that were caught and fixed before merge rather than left for a reviewer to find. I read the full diff, cross-checked the claimed call-site audit against the rest of the engine (grep-swept mongodbw/redisw/postgresw/gremlin for other user-controlled-pattern entry points, none found today), and traced the CommandContext/GlobalConfiguration plumbing by hand since mvn compile was not runnable in my review sandbox (harness limitation, not a code issue) - worth having CI confirm a clean mvn -pl engine -am verify given the size of this change.

Finding: promql() SQL function still multiplies the regex budget by row count

SQLFunctionPromQL.execute() (engine/src/main/java/com/arcadedb/function/sql/time/SQLFunctionPromQL.java:82) constructs a brand-new PromQLEvaluator on every call: new PromQLEvaluator(database). PromQLEvaluator.regexDeadline() (the new lazy instance field added by this PR) computes a fresh regexTimeout budget the first time it's needed per evaluator instance, which is correct for the two cases this PR explicitly fixed (rows within one step, steps within one range query), since those both happen inside a single PromQLEvaluator created once by SQLFunctionPromQL.

But promql(...) is an ordinary SQL function, evaluated once per row when used in a projection over a scan, e.g.:

SELECT promql(patternColumn) FROM BigTypeWithManyRows

Each row's invocation gets its own SQLFunctionPromQL.execute() call, its own new PromQLEvaluator(...), and therefore its own fresh regexTimeout budget for any =~/!~ label matcher inside that expression. A table shaped so every row's PromQL expression triggers catastrophic backtracking (the same a*-repeated shape already proven catastrophic in this PR's own PromQLEvaluatorIntegrationTest) can still cost rowCount * regexTimeout overall, exactly the amplification this PR closes for MATCHES/=~ (WHERE-clause), full-text, and schema REGEXP validation.

This is different from the documented LikeOperator/ILikeOperator per-row exception: that one is an accepted tradeoff because BinaryCompareOperator.execute() genuinely has no CommandContext to cache a deadline on. SQLFunctionPromQL.execute(..., final CommandContext context) does receive one, it's sitting right there, unused for this purpose. The same context.getOrComputeRegexDeadline(cacheKey) pattern this PR already threads through TextRegexReplace/SQLMethodNormalize (both are ordinary per-row SQL functions with exactly this shape) would close it: resolve the deadline from the context and pass it into the PromQLEvaluator instead of letting it compute its own from GlobalConfiguration in isolation.

Given every other entry point in this PR got a dedicated "shares one budget across N rows" regression test, this one seems worth the same treatment before merge, or at least a tracked follow-up if deferred, it's currently the one untested, unfixed instance of the exact bug class this PR exists to close.

Everything else

• TimeBoundRegex itself is solid: the shared int[] call counter across subSequence(), the overflow-safe deadline arithmetic (Math.addExact/multiplyExact with a Long.MAX_VALUE fallback), the stack-trace-less singleton RegexDeadlineExceeded, and the "only rewrap StackOverflowError when the bound is actually active" fix all look correct and are exercised by TimeBoundRegexTest.
• Verified the MATCHES_ / __MATCHES_DEADLINE__ cache-key non-collision claim by hand: "MATCHES_" + regex can never equal "__MATCHES_DEADLINE__" since the two strings necessarily differ at the first character (M vs _). Correct.
• Verified PromQLEvaluator's per-instance regexDeadline field is safe for the two cases it's meant for: evaluateRange()'s per-step loop is a plain sequential for, no fork/join, so no concurrent-mutation risk on that lazily-initialized field for those use cases.
• Verified LSMTreeFullTextIndex.getDatabase() and FullTextQueryExecutor.index's field type line up (both LSMTreeFullTextIndex), and CommandContext.getDatabase() already existed for the new default method to call.
• Confirmed (via grep) that DocumentValidator.validateField(document, p) (the 2-arg overload, still public) has no callers outside DocumentValidator itself, so the "no evidence of external callers" claim in the commit message holds against the current tree.
• The SQLMethodSplit delimiter fix (Pattern.quote(...)) is a correctness fix consistent with its three siblings, but it's also a silent behavior change for anyone (ab)using .split(delimiter) with real regex metacharacters, e.g. .split("\\d+") no longer splits on digit runs, it now splits on the literal string \d+. That's the right security call, but since it changes output for existing queries rather than just adding a bound, it might deserve its own "Breaking change" callout in the release notes rather than being folded into the general ReDoS section.
• The two documented, deliberately-accepted residual gaps (LIKE/ILIKE per-row budget, parallel bucket-scan giving each worker its own budget) are clearly explained in both code comments and docs/release-26.9.1.md, no action needed, just noting they were reviewed and the reasoning holds up.

Test coverage

Very thorough: every covered entry point gets both a "still matches/replaces correctly" test and a "catastrophic pattern aborts near the configured deadline" test with a timing assertion, plus dedicated multi-row/multi-item/multi-step "shares one budget" tests wherever sharing applies. regexDeadlineDoesNotLeakAcrossCachedQueryExecutions and patternTextDeadlineDoesNotCollideWithTheDeadlineCacheKey are exactly the kind of regression tests that should exist for the two subtle bugs the commit history says were caught by review, good discipline reverting-and-confirming-failure before restoring those fixes.

[lvca]

Verified the promql() finding rigorously before deciding how to respond, and want to be transparent about what I actually found (it's more nuanced than the review framing).

The code characteristic is real: SQLFunctionPromQL.execute() does construct new PromQLEvaluator(database) on every call, with no wiring to the CommandContext-cached deadline. Confirmed.

The specific claimed consequence didn't hold up empirically. I reproduced the exact scenario described (SELECT promql(patternColumn) FROM BigTypeWithManyRows, 10 rows, all with the same catastrophic label matcher) and measured: 220ms elapsed, not the ~2000ms ten independent 200ms budgets would take. Reason: the SQL engine aborts the whole query on the first row's TimeoutException rather than continuing to evaluate subsequent rows - it doesn't catch-and-continue past it. So the real-world cost is bounded to ~1 regexTimeout (plus whatever fast, non-catastrophic work preceded it) regardless of row count, the same "abort on first throw" characteristic that already applies to WHERE ... MATCHES scans and DocumentValidator's per-property loop.

Fixed anyway, for a narrower, real reason. Rather than leave this as a documented non-issue, I added PromQLEvaluator.setRegexDeadline() and wired SQLFunctionPromQL to resolve the deadline from context.getOrComputeRegexDeadline(), same as text.regexReplace()/.normalize(). This closes the actual inconsistency (every other per-row SQL function with a CommandContext shares the deadline; promql() was the one exception) and removes any dependency on the specific abort-on-first-exception behavior continuing to hold in future refactors, at effectively zero cost.

Test added honestly. I didn't reuse the "many identical rows" pattern from other tests in this PR, since I'd just shown it doesn't discriminate a regression here. Instead: setRegexDeadlineOverridesTheLazilyComputedDefault proves the injected deadline is actually honored (using an already-expired deadline to force an immediate abort on an otherwise-instant, non-catastrophic match), and promQLSqlFunctionAbortsOnCatastrophicPattern adds missing end-to-end coverage for the promql() SQL-function path specifically (previously only PromQLEvaluator.evaluateInstant() called directly was covered for the catastrophic-pattern case).

Full PromQLEvaluatorIntegrationTest suite (20 tests, including the 2 new ones) green. Pushed as 9124855.

[claude]

Review: ReDoS hardening (#5886)

This is a thorough, well-scoped fix for a real problem: java.util.regex never polls interrupts/deadlines while backtracking, so arcadedb.command.timeout can't stop a catastrophic pattern. The TimeBoundRegex approach (wrapping the input CharSequence so charAt() becomes the interception point) is a sound technique, and the audit across every regex-reachable entry point (SQL MATCHES, Cypher =~, LIKE/ILIKE, full-text RegexpQuery/WildcardQuery, PromQL label matchers, and schema REGEXP validation) is commendably complete — the schema-validation path in particular (reachable pre-auth on any write, per the PR description) is an important catch. Test coverage is extensive and each fix has a regression test that measures elapsed time to prove the abort actually happened rather than merely asserting an exception type.

A few things worth a look before merge:

1. Every regex/LIKE/MATCHES evaluation now allocates on the hot path, by default

TimeBoundRegex.run() (engine/src/main/java/com/arcadedb/utility/TimeBoundRegex.java) wraps the input in a new DeadlineBoundCharSequence(input, deadlineNanos) (which itself allocates a new int[1] counter) plus a capturing lambda, on every single call, whenever deadlineNanos != Long.MAX_VALUE. Since arcadedb.command.regexTimeout defaults to 1000 (enabled), this isn't an opt-in cost — it applies to every MATCHES/=~/LIKE/ILIKE evaluation out of the box, including trivial, non-catastrophic matches that used to be allocation-free. For a full-table scan filtered by LIKE, that's a new small object + array allocation per row. Given CLAUDE.md's "always bear in mind PERFORMANCE... lightweight on garbage collector" guidance, it'd be worth confirming this was benchmarked against a representative LIKE/MATCHES scan, or at least calling it out explicitly as an accepted trade-off in the PR description (the regexTimeout upgrade note covers the correctness behavior change well, but not this GC-pressure angle).

2. LIKE/ILIKE now call System.nanoTime() on every row, not just once per scan

Documented and clearly a deliberate, narrower trade-off (no CommandContext on BinaryCompareOperator to cache a scan-wide deadline on), but worth flagging explicitly: unlike MATCHES/=~ (one nanoTime() call per query, cached on the context), a WHERE ... LIKE scan now pays TimeBoundRegex.newDeadline() — and therefore System.nanoTime() — per row. Individually cheap, but it's a new per-row cost on one of the most common WHERE-clause operators. If this turns out to matter in practice, one option would be to at least cache the resolved regexTimeout value (the GlobalConfiguration lookup) once per scan even without a full shared deadline, though that's a smaller win than the deadline-sharing MATCHES gets.

3. Inconsistent exception type for the same failure mode across sibling call sites

TextRegexReplace catches TimeoutException from TimeBoundRegex and rewraps it as IllegalArgumentException (to preserve its pre-existing contract), while SQLMethodNormalize's pattern argument — fixed for the identical ReDoS gap — lets TimeoutException propagate directly (confirmed by each one's own test: TextRegexReplaceTest.catastrophicPatternIsAbortedByRegexTimeout expects IllegalArgumentException, SQLMethodNormalizeTest.catastrophicPatternArgumentIsAbortedByRegexTimeout expects TimeoutException). Both are documented as intentional, but from a caller's perspective, two conceptually identical failures (a regex timing out) surface as different exception types depending on which SQL function triggered it. Might be worth a follow-up to unify, if not in scope here.

Nice touches worth calling out

• The MATCHES_DEADLINE / MATCHES_ + pattern-text cache-key collision (caught in an earlier review pass, per the comments) is exactly the kind of subtle bug this class of "shared mutable cache keyed by string" design invites — good that it has a dedicated regression test (patternTextDeadlineDoesNotCollideWithTheDeadlineCacheKey).
• Correctly identifying that caching the deadline as an instance field on RegexExpression would go stale across CypherStatementCache-cached executions (vs. the content-addressed compiledPattern, which is safe to cache) — this is a real, easy-to-miss class of bug, and regexDeadlineDoesNotLeakAcrossCachedQueryExecutions is a good regression test for it.
• newDeadline()'s overflow handling for an oversized arcadedb.command.regexTimeout (falling back to "unbounded" rather than silently landing a deadline in the past) is the right failure direction for an admin-configurable value.
• Parallel bucket-scan workers each computing their own deadline via CommandContext.copy() (rather than racing on a shared non-thread-safe cache) is correctly reasoned, and I verified BasicCommandContext.copy() does leave cachedValues unset in the copy, so there's no shared-map race there.

Overall this looks solid and mergeable; the two performance notes above are the main thing I'd want a second look at, not blockers.

[lvca]

This round re-confirms three trade-offs already settled in earlier rounds rather than raising new ones - no code changes needed, restating why for the record:

1. Hot-path allocation from TimeBoundRegex.run(): reconfirmed as an accepted trade-off (round 13's response: "the per-context allocation overhead of the shared-deadline caching was already measured negligible relative to a regex evaluation itself"). A DeadlineBoundCharSequence + int[1] per call is a small, bounded cost against closing an unbounded-hang vulnerability.

2. LIKE/ILIKE per-row System.nanoTime(): this is the documented, deliberate exception to deadline-sharing (BinaryCompareOperator has no CommandContext to cache a scan-wide deadline on - widening that interface was judged out of proportion). Already called out explicitly in docs/release-26.9.1.md's "Deadline sharing" section.

3. TextRegexReplace vs SQLMethodNormalize exception-type inconsistency: addressed directly in round 15's response - TextRegexReplace rewraps to IllegalArgumentException because it already had that contract before this issue; .normalize() had no such prior contract, so letting TimeoutException propagate is actually the more consistent choice relative to the rest of this PR (MatchesCondition, RegexExpression, LIKE/ILIKE, full-text, PromQL all surface TimeoutException directly). Unifying the other direction would be a breaking change to TextRegexReplace's pre-existing, documented contract.

The review's own conclusion: "solid and mergeable... not blockers." Agreed - no further action this round.

[lvca]

`unit-tests` failed on the last CI run (2 failures: `Issue4960LSMCloseDuringCompactionTest#closeCancelsScheduledCompaction` and `DictionaryMultiPageTest#aDictionaryPageThatIsClaimedButMissingFailsLoudly`). Investigated rigorously before treating as a real regression:

• Neither test touches any file this PR modifies (both are in unrelated LSM index / dictionary-page subsystems - nothing to do with regex, `CommandContext`, PromQL, or `DocumentValidator`).
• Both failure symptoms are classic CI-resource-contention artifacts: `Issue4960LSMCloseDuringCompactionTest` failed a post-test database-integrity check with WAL-preservation warnings during a close/compaction race; `DictionaryMultiPageTest` failed to observe an expected exception during concurrent page access - both timing-sensitive under a loaded runner.
• Ran both locally on this exact branch (with all of this PR's changes applied): both pass cleanly, 15/15 tests green.

Conclusion: pre-existing CI flakes unrelated to this PR, not a regression. Re-running the failed job once the run finishes (can't re-run failed jobs while the workflow run is still in progress).

[codecov]

Codecov Report

❌ Patch coverage is 83.33333% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.10%. Comparing base (46c507f) to head (9124855).
⚠️ Report is 42 commits behind head on main.

Files with missing lines	Patch %	Lines
...main/java/com/arcadedb/utility/TimeBoundRegex.java	82.85%	6 Missing ⚠️
...dedb/engine/timeseries/promql/PromQLEvaluator.java	55.55%	2 Missing and 2 partials ⚠️
.../java/com/arcadedb/database/DocumentValidator.java	72.72%	2 Missing and 1 partial ⚠️
...a/com/arcadedb/function/text/TextRegexReplace.java	85.71%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5983      +/-   ##
==========================================
+ Coverage   69.08%   69.10%   +0.01%     
==========================================
  Files        1797     1798       +1     
  Lines      153675   153760      +85     
  Branches    32514    32525      +11     
==========================================
+ Hits       106168   106249      +81     
- Misses      34102    34108       +6     
+ Partials    13405    13403       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 18 complexity

Metric	Results
Complexity	18

View in Codacy

🟢 Coverage 86.90% diff coverage · -6.56% coverage variation

Metric	Results
Coverage variation	✅ -6.56% coverage variation
Diff coverage	✅ 86.90% diff coverage

View coverage diff in Codacy

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (46c507f)	153675	119612	77.83%
Head commit (9124855)	186103 (+32428)	132648 (+13036)	71.28% (-6.56%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#5983)	84	73	86.90%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[lvca]

`ha-integration-tests` failed a second consecutive time on retry, same two tests as before: `Issue5410AbandonedTicketReleaseIT#abandonedEntryReleasesItsPhase2TicketOnceApplied` and `Issue5569SlotMergeDeleteRaftIT#mergedDeletesReplicateIntact`. Per the instruction to treat a repeat failure more seriously than a first one, compared both runs in detail rather than just retrying again blind:

	Run 1	Run 2
`Issue5410...` assertion	"Node 0 must hold both vertices"	"Node 2 must hold both vertices"
`Issue5569...` assertion	"Update returned null: record=4 seq=25"	"Update returned null: record=7 seq=40"

Same test identities fail both times, but the specific node index and record/seq numbers differ between runs. That pattern is what a genuine timing/replication race looks like (non-deterministic which node/record loses the race), not what a deterministic bug from this PR's diff would produce (which would fail the same way every time, or not fail at all if truly unrelated). Both tests are in `arcadedb-ha-raft` - Raft leader-election, ticket-release, and slot-merge-delete replication internals - with zero file overlap with this PR's changes (regex/`CommandContext`/PromQL/`DocumentValidator`). This also matches project history: `ha-integration-tests` is a documented chronic flake independent of PR content (tracked separately, running red a large fraction of the time regardless of what's being merged).

Retrying once more. If it fails a third time with the same non-deterministic pattern, I'll treat that as sufficient confirmation of the known pre-existing chronic flake rather than continuing to retry indefinitely, and will flag that explicitly rather than silently blocking on it.

[lvca]

`ha-integration-tests` has now failed 3 consecutive times. Summary of the investigation across all 3 attempts before stopping the retry loop, per plan:

Attempts 1 & 2: same two tests failed both times - `Issue5410AbandonedTicketReleaseIT#abandonedEntryReleasesItsPhase2TicketOnceApplied` and `Issue5569SlotMergeDeleteRaftIT#mergedDeletesReplicateIntact` - with different specific node indices and record/seq numbers each run (Node 0/record=4/seq=25, then Node 2/record=7/seq=40), the signature of a non-deterministic timing/replication race, not a deterministic bug.

Attempt 3: the same two tests failed again (Node 2/record=8/seq=17 this time - again different data), plus a third, different test failed: `Issue4743LargeRecordReplicationIT#recordLargerThanTheOldCeilingReplicatesToEveryNode`, with `Schema Cannot create type 'Blob' because already exists` - a test-isolation/schema-state-pollution error, a different failure class entirely from the other two.

All three failing tests are in `arcadedb-ha-raft` (Raft leader-election, ticket-release, slot-merge-delete, and large-record replication internals) - zero file overlap with this PR's changes (regex/`CommandContext`/PromQL/`DocumentValidator`). The variety of failure types and non-deterministic data across 3 independent runs points to general instability of this test suite/CI runner rather than a regression introduced by this PR. This also matches this suite's documented status as a chronic flake independent of PR content.

Not retrying a 4th time. Every other check on this PR is green, including `unit-tests` (which failed once and passed clean on retry) and the code review itself (round 18: "solid and mergeable... not blockers"). Bringing the merge decision to @lvca given `ha-integration-tests` is a required check that hasn't gone green across 3 attempts despite being unrelated to this PR's diff.