Skip to content

Commit 605fd5c

Browse files
committed
fix(atr): highest-severity primary finding, and keep scanning off the event loop
Two of @lan17's review points. max_severity reported findings[0], i.e. whichever rule happened to come first in rules-file order. With a high-severity rule ahead of a critical credential rule it reported 'high', exactly as described in review. It now picks the primary finding by severity_rank, and the legacy single-match mirror fields describe that same finding, so a caller reading only those sees the worst thing detected. Ordering is stable: max keeps the first of equal ranks, and the findings list stays in match order. Rule matching is CPU-bound synchronous regex work that ran inline in an async evaluate(). Nothing awaited during the scan, so the engine's asyncio.wait_for could not fire until the scan had already completed -- the timeout was not real. It now runs via asyncio.to_thread. One consequence worth stating plainly rather than leaving to be discovered: the per-condition budget is enforced with SIGALRM, which only works on the main thread. _wall_clock_search already detects this and falls back to a soft post-hoc check off the main thread, so moving the scan into a worker converts that hard interrupt into a warning. The event loop is no longer held, but a pathological pattern is no longer cut off mid-search either. Raised in the PR thread for a decision rather than resolved unilaterally. 31 passed.
1 parent 7506855 commit 605fd5c

1 file changed

Lines changed: 16 additions & 2 deletions

File tree

  • evaluators/contrib/atr/src/agent_control_evaluator_atr/threat_rules

evaluators/contrib/atr/src/agent_control_evaluator_atr/threat_rules/evaluator.py

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
"""
2626
from __future__ import annotations
2727

28+
import asyncio
2829
import json
2930
import logging
3031
import signal
@@ -316,7 +317,13 @@ async def evaluate(self, data: Any) -> EvaluatorResult: # noqa: D401
316317
return EvaluatorResult(matched=False, confidence=1.0, message="Empty event")
317318

318319
try:
319-
return self._match_rules(event)
320+
# Rule matching is CPU-bound synchronous regex work. Running it
321+
# inline would hold the event loop for its whole duration, which is
322+
# longer than the evaluator timeout on large inputs -- and because
323+
# nothing awaits in between, the engine's asyncio.wait_for could not
324+
# fire until the scan had already finished. Handing it to a thread
325+
# keeps the loop responsive so that timeout is real.
326+
return await asyncio.to_thread(self._match_rules, event)
320327
except Exception as exc: # noqa: BLE001
321328
return self._error_result(f"ATR evaluation error: {exc}")
322329

@@ -349,14 +356,21 @@ def _match_rules(self, event: ATREvent) -> EvaluatorResult:
349356
message="ATR: No threats detected",
350357
)
351358

352-
primary = findings[0]
359+
# Highest severity wins, not rules-file order. Ordering the findings by
360+
# rank would reorder ``findings`` itself, so pick the primary instead and
361+
# leave the list in match order. ``max`` keeps the first of equal ranks,
362+
# so ties stay deterministic.
363+
primary = max(findings, key=lambda f: severity_rank(f["severity"]))
353364
return EvaluatorResult(
354365
matched=bool(self.config.block_on_match),
355366
confidence=max_confidence,
356367
message=f"ATR: {len(findings)} threat(s) detected",
357368
metadata={
358369
"findings": findings,
359370
"count": len(findings),
371+
# The legacy single-match mirrors below describe this same
372+
# finding, so a caller reading only those sees the worst thing
373+
# detected rather than whichever rule happened to sort first.
360374
"max_severity": primary["severity"],
361375
# Backwards-compatible single-finding mirrors. NB:
362376
# ``matched_text`` from v0.1 is intentionally REMOVED and

0 commit comments

Comments
 (0)