From 3d632c3a69bc9b03e89d01af3f2a5372788e65fd Mon Sep 17 00:00:00 2001 From: bearsyankees Date: Mon, 7 Sep 2026 11:04:41 -0400 Subject: [PATCH] fix(reporting): resolve captured requests and preserve failed revisions --- strix/report/state.py | 18 +++++++++++------- strix/tools/proxy/tools.py | 21 ++++++++++++--------- strix/tools/reporting/tool.py | 9 +++++++-- tests/test_proxy_client.py | 25 ++++++++----------------- tests/test_reporting_fields.py | 26 ++++++++++++++++++++++++++ 5 files changed, 64 insertions(+), 35 deletions(-) diff --git a/strix/report/state.py b/strix/report/state.py index 2885a74d..ac15db89 100644 --- a/strix/report/state.py +++ b/strix/report/state.py @@ -490,11 +490,18 @@ class ReportState: ) history.append(entry) - report.update(changed) + revised = {**report, **changed} for dependent in superseded: - report.pop(dependent, None) - report["update_history"] = history - report["updated_at"] = entry["timestamp"] + revised.pop(dependent, None) + revised["update_history"] = history + revised["updated_at"] = entry["timestamp"] + + # Persistence must accept the revision before local state changes. A + # failed callback leaves the old evidence intact and the update retryable. + if self.vulnerability_updated_callback: + self.vulnerability_updated_callback(revised) + report.clear() + report.update(revised) # The markdown on disk still shows the superseded evidence, so let the # writer re-render it. @@ -506,9 +513,6 @@ class ReportState: ", ".join(entry["fields"]) or "no field replaced", ) - if self.vulnerability_updated_callback: - self.vulnerability_updated_callback(report) - self.save_run_data() return report diff --git a/strix/tools/proxy/tools.py b/strix/tools/proxy/tools.py index 05432502..4aa9a2f8 100644 --- a/strix/tools/proxy/tools.py +++ b/strix/tools/proxy/tools.py @@ -79,16 +79,19 @@ async def existing_request_ids( if client is None: raise RuntimeError("Caido client is not available") - httpql_filter = " OR ".join(f"id.eq:{request_id}" for request_id in request_ids) - connection = await _call( - client, - lambda client: caido_api.list_requests_with_client( + # Request IDs are not an HTTPQL field. Resolve each ID through the same + # project-bound lookup as view_request rather than constructing a filter. + existing: set[str] = set() + for request_id in request_ids: + result = await _call( client, - httpql_filter=httpql_filter, - first=len(request_ids), - ), - ) - return {str(edge.node.request.id) for edge in connection.edges} + lambda client, request_id=request_id: caido_api.get_request_with_client( + client, request_id + ), + ) + if result is not None: + existing.add(str(result.request.id)) + return existing def _to_tool_json(value: Any) -> Any: diff --git a/strix/tools/reporting/tool.py b/strix/tools/reporting/tool.py index 54229bbc..e001dab1 100644 --- a/strix/tools/reporting/tool.py +++ b/strix/tools/reporting/tool.py @@ -1124,8 +1124,13 @@ async def create_vulnerability_report( code_locations: White-box findings — list of location objects. http_exchange_ids: Proxy request IDs that prove this finding. Copy these IDs from ``list_requests`` or ``view_request``. - Include only requests that support this finding. Keep the ids - out of ``evidence`` and all other report text. + For a finding validated over HTTP, capture and inspect the + supporting exchanges and include their IDs here before filing. + Include relevant baseline/control requests as well as the exploit. + Omit only when the finding has no captured HTTP evidence (for + example a static-only code finding). Never invent IDs or drop + them to bypass a verification error; retry the capture instead. + Keep IDs out of ``evidence`` and all other report text. **How ``fix_before`` / ``fix_after`` work**: they're used as literal GitHub/GitLab PR suggestion blocks. When a reviewer diff --git a/tests/test_proxy_client.py b/tests/test_proxy_client.py index fe89152d..ec21925c 100644 --- a/tests/test_proxy_client.py +++ b/tests/test_proxy_client.py @@ -234,26 +234,16 @@ async def test_existing_request_ids_queries_current_project( monkeypatch: pytest.MonkeyPatch, ) -> None: client = _FakeClient("host") - connection = SimpleNamespace( - edges=[ - SimpleNamespace( - node=SimpleNamespace(request=SimpleNamespace(id="1042")), - ) - ] - ) + looked_up: list[str] = [] - async def list_requests_with_client( - passed_client: Any, - *, - httpql_filter: str, - first: int, - ) -> Any: + async def get_request_with_client(passed_client: Any, request_id: str) -> Any: assert passed_client is client - assert httpql_filter == "id.eq:1042 OR id.eq:1088" - assert first == 2 - return connection + looked_up.append(request_id) + if request_id == "1042": + return SimpleNamespace(request=SimpleNamespace(id="1042")) + return None - monkeypatch.setattr(caido_api, "list_requests_with_client", list_requests_with_client) + monkeypatch.setattr(caido_api, "get_request_with_client", get_request_with_client) existing = await tools.existing_request_ids( cast("Any", _Ctx({"caido_client": client})), @@ -261,3 +251,4 @@ async def test_existing_request_ids_queries_current_project( ) assert existing == {"1042"} + assert looked_up == ["1042", "1088"] diff --git a/tests/test_reporting_fields.py b/tests/test_reporting_fields.py index e9f57fd6..88273323 100644 --- a/tests/test_reporting_fields.py +++ b/tests/test_reporting_fields.py @@ -152,6 +152,32 @@ def test_create_report_does_not_commit_when_callback_fails( assert report_state.vulnerability_reports == [] +def test_failed_revision_keeps_old_evidence_and_can_be_retried( + report_state: ReportState, +) -> None: + report_id = report_state.add_vulnerability_report( + title="Original finding", severity="high", http_exchange_ids=["1042"] + ) + original = dict(report_state.vulnerability_reports[0]) + + def fail_persistence(revised: dict[str, Any]) -> None: + assert revised["http_exchange_ids"] == ["1088"] + assert report_state.vulnerability_reports[0] == original + raise RuntimeError("persistence failed") + + report_state.vulnerability_updated_callback = fail_persistence + changes = {"title": "Revised finding", "http_exchange_ids": ["1088"]} + with pytest.raises(RuntimeError, match="persistence failed"): + report_state.update_vulnerability_report(report_id, changes) + assert report_state.vulnerability_reports[0] == original + + report_state.vulnerability_updated_callback = None + revised = report_state.update_vulnerability_report(report_id, changes) + assert revised is not None + assert revised["http_exchange_ids"] == ["1088"] + assert len(revised["update_history"]) == 1 + + async def test_create_report_requires_evidence_and_assumptions( report_state: ReportState, ) -> None: