Skip to content

fix(giskard-checks): catch TimeoutError in RegexMatching.run() - #2711

Open
kevinmessiaen wants to merge 1 commit into
mainfrom
claude/giskard-advisory-eval-c7713b
Open

fix(giskard-checks): catch TimeoutError in RegexMatching.run()#2711
kevinmessiaen wants to merge 1 commit into
mainfrom
claude/giskard-advisory-eval-c7713b

Conversation

@kevinmessiaen

Copy link
Copy Markdown
Member

Description

RegexMatching.run() called regex.search(pattern, text, timeout=self.match_timeout_seconds) but only caught regex.error. When the search hits its timeout, the regex library raises a bare TimeoutError, which was left uncaught. That exception then propagates up through TestCaseRunner._run_check() and, since Scenario.run() defaults to return_exception=False, aborts scenario execution entirely instead of surfacing as a normal check result.

This behavior was introduced intentionally in #2381 (which added the timeout to mitigate ReDoS) — the commit explicitly chose to let TimeoutError propagate, and the regression test was written to assert that (pytest.raises(TimeoutError)). This PR changes that: a timeout now returns CheckResult.error with a clear message, consistent with how invalid regex patterns are already handled in the same function.

Fix

  • RegexMatching.run() now catches TimeoutError in addition to regex.error and returns CheckResult.error.

Tests

  • Updated test_regex_redos_bounded_by_timeout to assert the graceful CheckResult.error outcome instead of asserting the propagating exception. Verified red (fails against pre-fix code with raw TimeoutError) → green.
  • Full giskard-checks suite: 805 passed, 4 skipped.
  • basedpyright --level error on touched files: 0 errors.

Related Issue

Reported in GHSA-7rwv-f9p2-g8rm. Reclassified from security advisory to bug fix: giskard-checks is a testing framework where the regex pattern is authored by the person writing the test, not supplied by an untrusted third party at runtime, so there's no trust boundary being crossed. The underlying inconsistency (one exception type from regex.search caught, its sibling not) is still worth fixing on its own merits.

Type of Change

  • 🔧 Bug fix (non-breaking change which fixes an issue)

Checklist

  • I've read the CODE_OF_CONDUCT.md document.
  • I've read the CONTRIBUTING.md guide.
  • I've written tests for all new methods and classes that I created.
  • I've written the docstring in NumPy format for all the methods and classes that I created or modified. (n/a — no new public methods/classes)
  • I've updated the uv.lock running uv lock. (n/a — no pyproject.toml changes)

RegexMatching only caught regex.error, so a regex timeout raised a bare
TimeoutError that escaped uncaught and aborted scenario execution when
return_exception=False (the default). Catch TimeoutError alongside
regex.error and return a CheckResult.error instead, matching the
handling already used for invalid patterns.

Reported in GHSA-7rwv-f9p2-g8rm.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

1 participant