From 43a2886c6e90d102a2a22a0478a9565e492aa412 Mon Sep 17 00:00:00 2001 From: Sean Turner Date: Mon, 3 Aug 2026 10:29:49 +0100 Subject: [PATCH] fix(dep-verify): run the advisory check off the event loop (Greptile P1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit verify_dependency does blocking HTTP (requests.post) and was called directly from the async create_dependency_report path — a slow/unreachable provider would block the shared agent event loop for the request timeout, stalling every concurrent agent (worst case two sequential calls on the clean-version path). - Offload the call via asyncio.to_thread so the event loop stays free. - Tighten the provider timeout 20s -> 10s (named OsvProvider._TIMEOUT_S): a single advisory lookup shouldn't hold a worker thread that long, and it fails open fast. - Add an async test asserting the verifier runs on a worker thread (not the main/ event-loop thread) and the reject still propagates. Addresses the Greptile review comment on #963. 38 report/dep tests pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- strix/report/dep_verify.py | 10 +++++++-- strix/tools/reporting/tool.py | 6 ++++- tests/test_reporting_fields.py | 41 ++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 3 deletions(-) diff --git a/strix/report/dep_verify.py b/strix/report/dep_verify.py index ae7ce8652..31c6eefd5 100644 --- a/strix/report/dep_verify.py +++ b/strix/report/dep_verify.py @@ -91,7 +91,13 @@ class OsvProvider: """AdvisoryProvider backed by an OSV-schema /v1/query endpoint. Default api.osv.dev; point `url` at a self-hosted OSV mirror (identical contract) for air-gapped / data-residency deployments. Uses `requests` (a core dep) — no - extra package.""" + extra package. + + The caller runs this off the event loop (asyncio.to_thread), but keep the + per-request timeout short so a stalled provider fails open quickly rather than + holding a worker thread — this is a bounded advisory lookup, not bulk I/O.""" + + _TIMEOUT_S = 10 def __init__(self, url: str) -> None: self.url = url @@ -99,7 +105,7 @@ class OsvProvider: def _query(self, payload: dict[str, Any]) -> list[dict] | None: try: ca = os.environ.get("REQUESTS_CA_BUNDLE") - resp = requests.post(self.url, timeout=20, json=payload, + resp = requests.post(self.url, timeout=self._TIMEOUT_S, json=payload, verify=ca if ca else True) if resp.status_code != 200: logger.info("dep-verify: OSV %s for %s; can't answer", diff --git a/strix/tools/reporting/tool.py b/strix/tools/reporting/tool.py index 11f93b5ac..1596130bb 100644 --- a/strix/tools/reporting/tool.py +++ b/strix/tools/reporting/tool.py @@ -830,7 +830,11 @@ async def _do_create_dependency( # noqa: PLR0912 if load_settings().dep_verify.enabled: from strix.report.dep_verify import verify_dependency - dep_verdict = verify_dependency({ + # verify_dependency does blocking HTTP (advisory provider). Run it in a + # worker thread so a slow/unreachable provider can't stall the shared + # async agent loop (it would otherwise block every concurrent agent for + # up to the provider timeout, x2 on the clean-version path). + dep_verdict = await asyncio.to_thread(verify_dependency, { "cve": parsed_cve, "package_name": package_name, "installed_version": installed_version, "package_ecosystem": package_ecosystem, diff --git a/tests/test_reporting_fields.py b/tests/test_reporting_fields.py index 79fa4e9a5..f1f1c7e02 100644 --- a/tests/test_reporting_fields.py +++ b/tests/test_reporting_fields.py @@ -2,10 +2,14 @@ from __future__ import annotations +import threading +import time from typing import TYPE_CHECKING import pytest +import strix.report.dep_verify as depv +from strix.config import loader from strix.report.dedupe import ( _check_dependency_duplicate, _prepare_report_for_comparison, @@ -192,6 +196,43 @@ async def test_dependency_report_with_zero_cvss_remains_low_severity( assert report["cvss"] == 0.0 +async def test_dependency_verify_runs_off_the_event_loop( + report_state: ReportState, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The verifier does blocking HTTP; it must be offloaded (asyncio.to_thread) + so a slow/unreachable provider can't stall the shared async agent loop. Stub + verify_dependency with a blocking call that records its thread + rejects, and + assert (a) the reject propagates and (b) it ran on a NON-main thread.""" + monkeypatch.setenv("STRIX_DEP_VERIFY", "1") + loader._cached = None + + main_thread = threading.current_thread() + ran_on: dict[str, object] = {} + + def _blocking_verify(_candidate: dict) -> dict: + ran_on["thread"] = threading.current_thread() + time.sleep(0.05) # simulate a slow provider (blocking) + return {"success": False, "verify_rejected": True, + "dep_version_out_of_range": True, "error": "out of range"} + + monkeypatch.setattr(depv, "verify_dependency", _blocking_verify) + + result = await _do_create_dependency( + title="CVE-2021-44906 in minimist 1.2.6", + description="Prototype pollution.", target="repo/package.json", + cve="CVE-2021-44906", package_name="minimist", installed_version="1.2.6", + impact="x", remediation_steps="upgrade", assumptions="a", + package_ecosystem="npm", fixed_version=None, cwe=None, + advisory_cvss=7.5, technical_analysis=None, fix_effort="low", + ) + # reject propagated + finding NOT persisted + assert result["verify_rejected"] is True + assert not report_state.vulnerability_reports + # ran off the event loop (a worker thread, not the main thread) + assert ran_on["thread"] is not main_thread + + async def test_dependency_report_requires_advisory_cvss(report_state: ReportState) -> None: result = await _do_create_dependency( title="CVE-2024-0001 in sample 1.0.0",