fix(reporting): keep HTTP exchange evidence when the proxy is slow to verify ids (#1551)

Verifying http_exchange_ids fetched every request with its full raw
request and response bodies, serially, and timed out on busy proxy
projects; the ids were then dropped and the finding lost its evidence.

Check existence with an id-only GraphQL query, retry once, and when the
proxy still does not answer keep the ids with a warning instead of
discarding them.

strix/tools/proxy/tools.py

@@ -69,6 +69,15 @@ async def _call[T](client: Client, fn: Callable[[Client], Awaitable[T]]) -> T:
         return await fn(client)
 
 
+_REQUEST_EXISTS_QUERY = "query RequestExists($id: ID!) { request(id: $id) { id } }"
+_REQUEST_EXISTS_ATTEMPTS = 2
+
+
+async def _request_exists(client: Client, request_id: str) -> bool:
+    data = await client.graphql.query(_REQUEST_EXISTS_QUERY, {"id": request_id})
+    return bool(data) and data.get("request") is not None
+
+
 async def existing_request_ids(
     ctx: RunContextWrapper,
     request_ids: list[str],
@@ -81,16 +90,24 @@ async def existing_request_ids(
     if client is None:
         raise RuntimeError("Caido client is not available")
 
-    # Request IDs are not an HTTPQL field. Resolve each ID through the same
+    # Request IDs are not an HTTPQL field, so each one is resolved by id. Only
-    # project-bound lookup as view_request rather than constructing a filter.
+    # the id is selected: pulling the raw request and response bodies just to
+    # learn whether a row exists is what made this time out on busy projects.
     existing: set[str] = set()
     for request_id in request_ids:
-        result = await _call(
+        for attempt in range(1, _REQUEST_EXISTS_ATTEMPTS + 1):
-            client,
+            try:
-            functools.partial(caido_api.get_request_with_client, request_id=request_id),
+                exists = await _call(
-        )
+                    client,
-        if result is not None:
+                    functools.partial(_request_exists, request_id=request_id),
-            existing.add(str(result.request.id))
+                )
+            except Exception:
+                if attempt == _REQUEST_EXISTS_ATTEMPTS:
+                    raise
+                continue
+            if exists:
+                existing.add(request_id)
+            break
     return existing
 
 

strix/tools/reporting/tool.py

@@ -222,9 +222,9 @@ def _normalize_http_exchange_ids(raw: Any) -> tuple[list[str] | None, list[str]]
     return normalized, errors
 
 
-_HTTP_EXCHANGE_DROPPED_WARNING = (
+_HTTP_EXCHANGE_UNVERIFIED_WARNING = (
-    "http_exchange_ids were not stored: the proxy project could not be reached to verify "
+    "http_exchange_ids were stored without verification because the proxy project did "
-    "them. Attach them with update_vulnerability_report when the proxy responds again."
+    "not respond in time. Ids that do not exist in the project will not render as evidence."
 )
 
 
@@ -235,9 +235,9 @@ async def _verify_http_exchange_ids(
     """Verify proxy exchange IDs against the current Caido project.
 
     IDs the project does not know are rejected. When the proxy itself cannot be
-    queried the IDs are dropped and a warning is returned instead, so a proxy
+    queried the IDs are kept and a warning is returned instead: a slow or
-    outage never blocks a finding and unverified IDs are never recorded as
+    unreachable proxy must never strip evidence from a finding, and consumers
-    evidence.
+    resolve each id against the recorded traffic before rendering it.
     """
     request_ids, errors = _normalize_http_exchange_ids(raw)
     if request_ids is None or errors or not request_ids:
@@ -250,7 +250,7 @@ async def _verify_http_exchange_ids(
             "Could not verify HTTP exchange IDs against the current Caido project",
             exc_info=True,
         )
-        return None, [], _HTTP_EXCHANGE_DROPPED_WARNING
+        return request_ids, [], _HTTP_EXCHANGE_UNVERIFIED_WARNING
 
     missing_ids = [request_id for request_id in request_ids if request_id not in existing_ids]
     if missing_ids:

tests/test_proxy_client.py

@@ -9,7 +9,6 @@ call at a time against the shared client.
 from __future__ import annotations
 
 import asyncio
-from types import SimpleNamespace
 from typing import TYPE_CHECKING, Any, cast
 
 import pytest
@@ -27,6 +26,7 @@ class _FakeClient:
     def __init__(self, name: str) -> None:
         self.name = name
         self.closed = False
+        self.graphql: Any = None
 
     async def aclose(self) -> None:
         self.closed = True
@@ -231,20 +231,23 @@ async def test_ctx_client_degrades_when_bootstrap_failed() -> None:
     assert await tools._ctx_client(cast("Any", _Ctx({"caido_client": handle}))) is None
 
 
-async def test_existing_request_ids_queries_current_project(
+class _FakeGraphQL:
-    monkeypatch: pytest.MonkeyPatch,
+    def __init__(self, answers: dict[str, list[Any]]) -> None:
-) -> None:
+        self.answers = answers
+        self.queries: list[tuple[str, dict[str, Any]]] = []
+
+    async def query(self, document: str, variables: dict[str, Any]) -> dict[str, Any]:
+        self.queries.append((document, dict(variables)))
+        answer = self.answers[variables["id"]].pop(0)
+        if isinstance(answer, Exception):
+            raise answer
+        return {"request": answer}
+
+
+async def test_existing_request_ids_select_only_the_id() -> None:
     client = _FakeClient("host")
-    looked_up: list[str] = []
+    graphql = _FakeGraphQL({"1042": [{"id": "1042"}], "1088": [None]})
-
+    client.graphql = graphql
-    async def get_request_with_client(passed_client: Any, request_id: str) -> Any:
-        assert passed_client is client
-        looked_up.append(request_id)
-        if request_id == "1042":
-            return SimpleNamespace(request=SimpleNamespace(id="1042"))
-        return None
-
-    monkeypatch.setattr(caido_api, "get_request_with_client", get_request_with_client)
 
     existing = await tools.existing_request_ids(
         cast("Any", _Ctx({"caido_client": client})),
@@ -252,7 +255,34 @@ async def test_existing_request_ids_queries_current_project(
     )
 
     assert existing == {"1042"}
-    assert looked_up == ["1042", "1088"]
+    assert [variables["id"] for _, variables in graphql.queries] == ["1042", "1088"]
+    for document, _ in graphql.queries:
+        assert "raw" not in document
+        assert "{ id }" in document
+
+
+async def test_existing_request_ids_retry_once_before_giving_up() -> None:
+    client = _FakeClient("host")
+    graphql = _FakeGraphQL(
+        {
+            "1042": [TimeoutError("deadline exceeded"), {"id": "1042"}],
+            "1088": [TimeoutError("deadline exceeded"), TimeoutError("deadline exceeded")],
+        }
+    )
+    client.graphql = graphql
+
+    with pytest.raises(TimeoutError):
+        await tools.existing_request_ids(
+            cast("Any", _Ctx({"caido_client": client})),
+            ["1042", "1088"],
+        )
+
+    assert [variables["id"] for _, variables in graphql.queries] == [
+        "1042",
+        "1042",
+        "1088",
+        "1088",
+    ]
 
 
 def test_err_logs_caido_errors_as_warnings(caplog: pytest.LogCaptureFixture) -> None:

tests/test_reporting_fields.py

@@ -1640,7 +1640,7 @@ async def test_http_exchange_ids_must_exist_in_current_proxy_project(
     assert warning is None
 
 
-async def test_http_exchange_ids_are_dropped_when_proxy_cannot_be_queried(
+async def test_http_exchange_ids_are_kept_when_proxy_cannot_be_queried(
     monkeypatch: pytest.MonkeyPatch,
 ) -> None:
     async def existing_request_ids(
@@ -1656,11 +1656,10 @@ async def test_http_exchange_ids_are_dropped_when_proxy_cannot_be_queried(
         ["1042", "1042", "1088"],
     )
 
-    assert request_ids is None
+    assert request_ids == ["1042", "1088"]
     assert errors == []
     assert warning is not None
-    assert "not stored" in warning
+    assert "without verification" in warning
-    assert "update_vulnerability_report" in warning
 
 
 async def test_create_reports_persistence_failure_as_tool_error(
@@ -1679,7 +1678,7 @@ async def test_create_reports_persistence_failure_as_tool_error(
     assert report_state.vulnerability_reports == []
 
 
-async def test_evidence_only_update_reports_proxy_outage_as_retryable(
+async def test_evidence_only_update_keeps_ids_when_proxy_cannot_be_queried(
     report_state: ReportState,
     monkeypatch: pytest.MonkeyPatch,
 ) -> None:
@@ -1712,11 +1711,11 @@ async def test_evidence_only_update_reports_proxy_outage_as_retryable(
     )
     result = json.loads(raw)
 
-    assert result["success"] is False
+    assert result["success"] is True
-    assert "No fields to update" not in result["error"]
+    assert "without verification" in result["warning"]
-    assert "update_vulnerability_report" in result["error"]
     assert result["report_id"] == "vuln-0009"
-    assert report_state.vulnerability_reports[0] == original
+    assert report_state.vulnerability_reports[0]["http_exchange_ids"] == ["204"]
+    assert report_state.vulnerability_reports[0] != original
 
 
 def test_update_reports_persistence_failure_as_tool_error(report_state: ReportState) -> None: