From d40497747fb801097159b61c35a9286cfdbc9810 Mon Sep 17 00:00:00 2001 From: Alex Schapiro Date: Fri, 4 Sep 2026 20:29:37 +0000 Subject: [PATCH] fix(reporting): verify HTTP evidence references --- strix/tools/proxy/tools.py | 24 +++++++++++++ strix/tools/reporting/tool.py | 65 ++++++++++++++++++++++++++++++++-- tests/test_proxy_client.py | 34 ++++++++++++++++++ tests/test_reporting_fields.py | 32 ++++++++++++++++- 4 files changed, 152 insertions(+), 3 deletions(-) diff --git a/strix/tools/proxy/tools.py b/strix/tools/proxy/tools.py index e465578a..05432502 100644 --- a/strix/tools/proxy/tools.py +++ b/strix/tools/proxy/tools.py @@ -67,6 +67,30 @@ async def _call[T](client: Client, fn: Callable[[Client], Awaitable[T]]) -> T: return await fn(client) +async def existing_request_ids( + ctx: RunContextWrapper, + request_ids: list[str], +) -> set[str]: + """Return request IDs that exist in the current Caido project.""" + if not request_ids: + return set() + + client = await _ctx_client(ctx) + 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( + client, + httpql_filter=httpql_filter, + first=len(request_ids), + ), + ) + return {str(edge.node.request.id) for edge in connection.edges} + + def _to_tool_json(value: Any) -> Any: """Recursively convert SDK dataclasses/Pydantic objects to tool JSON values.""" if value is None or isinstance(value, str | int | float | bool): diff --git a/strix/tools/reporting/tool.py b/strix/tools/reporting/tool.py index 6cec35d9..54229bbc 100644 --- a/strix/tools/reporting/tool.py +++ b/strix/tools/reporting/tool.py @@ -17,6 +17,7 @@ from typing import TYPE_CHECKING, Any from agents import RunContextWrapper, function_tool from strix.tools.nullish import clean_optional +from strix.tools.proxy.tools import existing_request_ids if TYPE_CHECKING: @@ -185,8 +186,6 @@ def _normalize_http_exchange_ids(raw: Any) -> tuple[list[str] | None, list[str]] return None, [] if not isinstance(raw, list): return None, ["http_exchange_ids must be a list of proxy request ids"] - if len(raw) > _MAX_HTTP_EXCHANGE_IDS: - return None, [f"http_exchange_ids can contain at most {_MAX_HTTP_EXCHANGE_IDS} request ids"] normalized: list[str] = [] errors: list[str] = [] @@ -208,12 +207,44 @@ def _normalize_http_exchange_ids(raw: Any) -> tuple[list[str] | None, list[str]] if any(ord(char) < 0x21 or ord(char) > 0x7E for char in request_id): errors.append(f"http_exchange_ids[{index}] must contain only visible ASCII characters") continue + if not request_id.isdigit(): + errors.append(f"http_exchange_ids[{index}] must be a numeric proxy request id") + continue if request_id not in seen: seen.add(request_id) normalized.append(request_id) + if len(normalized) > _MAX_HTTP_EXCHANGE_IDS: + errors.append( + f"http_exchange_ids can contain at most " + f"{_MAX_HTTP_EXCHANGE_IDS} distinct request ids" + ) + break return normalized, errors +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.""" + request_ids, errors = _normalize_http_exchange_ids(raw) + if request_ids is None or errors or not request_ids: + return request_ids, errors + + 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"] + + 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, [] + + def _validate_cvss_breakdown(breakdown: Any) -> list[str]: """Check the 8 CVSS metrics are all present with legal values.""" if not isinstance(breakdown, dict) or not breakdown: @@ -1242,6 +1273,21 @@ 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, + ) + if http_exchange_errors: + return json.dumps( + { + "success": False, + "error": "Validation failed", + "errors": http_exchange_errors, + }, + ensure_ascii=False, + default=str, + ) + agent_id, agent_name = _caller_identity(ctx) result = await _do_create( @@ -1382,6 +1428,21 @@ 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, + ) + if http_exchange_errors: + return json.dumps( + { + "success": False, + "error": "Validation failed", + "errors": http_exchange_errors, + }, + ensure_ascii=False, + default=str, + ) + agent_id, agent_name = _caller_identity(ctx) result = await asyncio.to_thread( _do_update, diff --git a/tests/test_proxy_client.py b/tests/test_proxy_client.py index 1a3aa849..fe89152d 100644 --- a/tests/test_proxy_client.py +++ b/tests/test_proxy_client.py @@ -9,6 +9,7 @@ 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 @@ -227,3 +228,36 @@ async def test_ctx_client_degrades_when_bootstrap_failed() -> None: handle = CaidoBootstrapHandle(asyncio.ensure_future(_bootstrap())) assert await tools._ctx_client(cast("Any", _Ctx({"caido_client": handle}))) is None + + +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")), + ) + ] + ) + + async def list_requests_with_client( + passed_client: Any, + *, + httpql_filter: str, + first: int, + ) -> Any: + assert passed_client is client + assert httpql_filter == "id.eq:1042 OR id.eq:1088" + assert first == 2 + return connection + + monkeypatch.setattr(caido_api, "list_requests_with_client", list_requests_with_client) + + existing = await tools.existing_request_ids( + cast("Any", _Ctx({"caido_client": client})), + ["1042", "1088"], + ) + + assert existing == {"1042"} diff --git a/tests/test_reporting_fields.py b/tests/test_reporting_fields.py index 872bdc0e..e9f57fd6 100644 --- a/tests/test_reporting_fields.py +++ b/tests/test_reporting_fields.py @@ -2,7 +2,7 @@ from __future__ import annotations -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, cast import pytest @@ -13,10 +13,13 @@ from strix.report.dedupe import ( ) from strix.report.state import ReportState, set_global_report_state from strix.tools.finish.tool import finish_scan +from strix.tools.reporting import tool as reporting_tool from strix.tools.reporting.tool import ( _do_create, _do_create_dependency, _do_update, + _normalize_http_exchange_ids, + _verify_http_exchange_ids, create_dependency_report, create_vulnerability_report, update_vulnerability_report, @@ -1405,6 +1408,33 @@ async def test_create_rejects_invalid_http_exchange_ids(report_state: ReportStat assert report_state.vulnerability_reports == [] +def test_http_exchange_id_limit_applies_after_deduplication() -> None: + request_ids, errors = _normalize_http_exchange_ids(["1042"] * 11) + + assert errors == [] + assert request_ids == ["1042"] + + +async def test_http_exchange_ids_must_exist_in_current_proxy_project( + monkeypatch: pytest.MonkeyPatch, +) -> None: + async def existing_request_ids( + _ctx: Any, + _request_ids: list[str], + ) -> set[str]: + return {"1042"} + + monkeypatch.setattr(reporting_tool, "existing_request_ids", existing_request_ids) + + request_ids, errors = 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"] + + def test_update_vulnerability_report_ignores_identical_content(report_state: ReportState) -> None: _seed_weak_report(report_state) assert report_state.update_vulnerability_report("vuln-0009", {"severity": "medium"}) is None