diff --git a/strix/tools/proxy/tools.py b/strix/tools/proxy/tools.py index 4aa9a2f8..0dc2a03c 100644 --- a/strix/tools/proxy/tools.py +++ b/strix/tools/proxy/tools.py @@ -4,6 +4,7 @@ from __future__ import annotations import asyncio import dataclasses +import functools import json import logging import re @@ -85,9 +86,7 @@ async def existing_request_ids( for request_id in request_ids: result = await _call( client, - lambda client, request_id=request_id: caido_api.get_request_with_client( - client, request_id - ), + functools.partial(caido_api.get_request_with_client, request_id=request_id), ) if result is not None: existing.add(str(result.request.id)) diff --git a/strix/tools/reporting/tool.py b/strix/tools/reporting/tool.py index e001dab1..25ce269a 100644 --- a/strix/tools/reporting/tool.py +++ b/strix/tools/reporting/tool.py @@ -222,27 +222,51 @@ def _normalize_http_exchange_ids(raw: Any) -> tuple[list[str] | None, list[str]] return normalized, errors +_HTTP_EXCHANGE_UNVERIFIED_WARNING = ( + "http_exchange_ids were stored unverified: the proxy project could not be reached" +) + + async def _verify_http_exchange_ids( ctx: RunContextWrapper, raw: Any, -) -> tuple[list[str] | None, list[str]]: - """Verify proxy exchange IDs against the current Caido project.""" +) -> tuple[list[str] | None, list[str], str | None]: + """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 kept as given and a warning is returned instead, so a + proxy outage never blocks a finding from being filed. + """ request_ids, errors = _normalize_http_exchange_ids(raw) if request_ids is None or errors or not request_ids: - return request_ids, errors + return request_ids, errors, None try: existing_ids = await existing_request_ids(ctx, request_ids) - except Exception: - logger.exception("Could not verify HTTP exchange IDs against the current Caido project") - return None, ["http_exchange_ids could not be verified against the current proxy project"] + except Exception: # noqa: BLE001 + logger.warning( + "Could not verify HTTP exchange IDs against the current Caido project", + exc_info=True, + ) + 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: - return None, [ - "http_exchange_ids do not exist in the current proxy project: " + ", ".join(missing_ids) - ] - return request_ids, [] + return ( + None, + [ + "http_exchange_ids do not exist in the current proxy project: " + + ", ".join(missing_ids) + ], + None, + ) + return request_ids, [], None + + +def _with_warning(result: dict[str, Any], warning: str | None) -> dict[str, Any]: + if warning and result.get("success"): + result["warning"] = warning + return result def _validate_cvss_breakdown(breakdown: Any) -> list[str]: @@ -626,13 +650,24 @@ def _do_update( if class_error is not None: return class_error - updated = report_state.update_vulnerability_report( - report_id, - changes, - update_reason=update_reason, - updated_by_agent_id=agent_id, - updated_by_agent_name=agent_name, - ) + try: + updated = report_state.update_vulnerability_report( + report_id, + changes, + update_reason=update_reason, + updated_by_agent_id=agent_id, + updated_by_agent_name=agent_name, + ) + except Exception as e: + logger.exception("update_vulnerability_report persistence failed") + return { + "success": False, + "error": ( + f"Failed to revise report '{report_id}': {e!s}. " + "The report still carries its previous content; retry the update." + ), + "report_id": report_id, + } if updated is None: known = [r.get("id") for r in report_state.get_existing_vulnerabilities()] if report_id not in known: @@ -819,9 +854,15 @@ async def _do_create( agent_id=agent_id if isinstance(agent_id, str) else None, agent_name=agent_name if isinstance(agent_name, str) else None, ) - except (ImportError, AttributeError) as e: + except Exception as e: logger.exception("create_vulnerability_report persistence failed") - return {"success": False, "error": f"Failed to create vulnerability report: {e!s}"} + return { + "success": False, + "error": ( + f"Failed to create vulnerability report: {e!s}. " + "The finding was not stored; file it again." + ), + } else: logger.info( "Vulnerability report created: id=%s severity=%s cvss=%.1f title=%s", @@ -1278,10 +1319,11 @@ async def create_vulnerability_report( reduce impact and lower the severity. fix_effort: "low" """ - http_exchange_ids, http_exchange_errors = await _verify_http_exchange_ids( - ctx, + ( http_exchange_ids, - ) + http_exchange_errors, + http_exchange_warning, + ) = await _verify_http_exchange_ids(ctx, http_exchange_ids) if http_exchange_errors: return json.dumps( { @@ -1323,7 +1365,7 @@ async def create_vulnerability_report( agent_id=agent_id, agent_name=agent_name, ) - return json.dumps(result, ensure_ascii=False, default=str) + return json.dumps(_with_warning(result, http_exchange_warning), ensure_ascii=False, default=str) @function_tool(timeout=60, strict_mode=False) @@ -1433,10 +1475,11 @@ async def update_vulnerability_report( observed in this codebase that justifies the contextual ``cvss_breakdown``. """ - http_exchange_ids, http_exchange_errors = await _verify_http_exchange_ids( - ctx, + ( http_exchange_ids, - ) + http_exchange_errors, + http_exchange_warning, + ) = await _verify_http_exchange_ids(ctx, http_exchange_ids) if http_exchange_errors: return json.dumps( { @@ -1483,7 +1526,7 @@ async def update_vulnerability_report( agent_id=agent_id, agent_name=agent_name, ) - return json.dumps(result, ensure_ascii=False, default=str) + return json.dumps(_with_warning(result, http_exchange_warning), ensure_ascii=False, default=str) _DEP_SEVERITY_FROM_CVSS = { @@ -1873,9 +1916,15 @@ async def _do_create_dependency( # noqa: PLR0912 agent_id=agent_id if isinstance(agent_id, str) else None, agent_name=agent_name if isinstance(agent_name, str) else None, ) - except (ImportError, AttributeError) as e: + except Exception as e: logger.exception("create_dependency_report persistence failed") - return {"success": False, "error": f"Failed to create dependency report: {e!s}"} + return { + "success": False, + "error": ( + f"Failed to create dependency report: {e!s}. " + "The finding was not stored; file it again." + ), + } else: logger.info( "Dependency report created: id=%s cve=%s package=%s severity=%s", diff --git a/tests/test_reporting_fields.py b/tests/test_reporting_fields.py index 88273323..30e0c28e 100644 --- a/tests/test_reporting_fields.py +++ b/tests/test_reporting_fields.py @@ -1452,13 +1452,73 @@ async def test_http_exchange_ids_must_exist_in_current_proxy_project( monkeypatch.setattr(reporting_tool, "existing_request_ids", existing_request_ids) - request_ids, errors = await _verify_http_exchange_ids( + request_ids, errors, warning = await _verify_http_exchange_ids( cast("Any", object()), ["1042", "1088"], ) assert request_ids is None assert errors == ["http_exchange_ids do not exist in the current proxy project: 1088"] + assert warning is None + + +async def test_http_exchange_ids_are_kept_when_proxy_cannot_be_queried( + monkeypatch: pytest.MonkeyPatch, +) -> None: + async def existing_request_ids( + _ctx: Any, + _request_ids: list[str], + ) -> set[str]: + raise RuntimeError("Caido client is not available") + + monkeypatch.setattr(reporting_tool, "existing_request_ids", existing_request_ids) + + request_ids, errors, warning = await _verify_http_exchange_ids( + cast("Any", object()), + ["1042", "1042", "1088"], + ) + + assert request_ids == ["1042", "1088"] + assert errors == [] + assert warning is not None + assert "unverified" in warning + + +async def test_create_reports_persistence_failure_as_tool_error( + report_state: ReportState, +) -> None: + def fail_persistence(_report: dict[str, Any]) -> None: + raise RuntimeError("persistence failed") + + report_state.vulnerability_found_callback = fail_persistence + + result = await _do_create(**_CONFIRMED_KWARGS, http_exchange_ids=["1042"]) + + assert result["success"] is False + assert "persistence failed" in result["error"] + assert "file it again" in result["error"] + assert report_state.vulnerability_reports == [] + + +def test_update_reports_persistence_failure_as_tool_error(report_state: ReportState) -> None: + _seed_weak_report(report_state) + original = dict(report_state.vulnerability_reports[0]) + + def fail_persistence(_report: dict[str, Any]) -> None: + raise RuntimeError("persistence failed") + + report_state.vulnerability_updated_callback = fail_persistence + + result = _do_update( + report_id="vuln-0009", + update_reason="A replay produced a clearer proving exchange.", + fields={"http_exchange_ids": ["204"]}, + ) + + assert result["success"] is False + assert "persistence failed" in result["error"] + assert result["report_id"] == "vuln-0009" + assert report_state.vulnerability_reports[0] == original def test_update_vulnerability_report_ignores_identical_content(report_state: ReportState) -> None: