mirror of
https://github.com/usestrix/strix.git
synced 2026-10-13 03:57:59 +00:00
feat(reporting): link HTTP exchange evidence
This commit is contained in:
parent
afa7c4a77f
commit
677ae1effa
3 changed files with 98 additions and 2 deletions
|
|
@ -73,6 +73,7 @@ UPDATABLE_REPORT_FIELDS = frozenset(
|
|||
"cve",
|
||||
"cwe",
|
||||
"code_locations",
|
||||
"http_exchange_ids",
|
||||
"fix_verification",
|
||||
"fix_pr_body",
|
||||
}
|
||||
|
|
@ -333,6 +334,7 @@ class ReportState:
|
|||
cve: str | None = None,
|
||||
cwe: str | None = None,
|
||||
code_locations: list[dict[str, Any]] | None = None,
|
||||
http_exchange_ids: list[str] | None = None,
|
||||
fix_verification: str | None = None,
|
||||
fix_pr_body: str | None = None,
|
||||
finding_class: str | None = None,
|
||||
|
|
@ -391,6 +393,8 @@ class ReportState:
|
|||
report["cwe"] = cwe.strip()
|
||||
if code_locations:
|
||||
report["code_locations"] = code_locations
|
||||
if http_exchange_ids:
|
||||
report["http_exchange_ids"] = http_exchange_ids
|
||||
if fix_verification:
|
||||
report["fix_verification"] = fix_verification.strip()
|
||||
if fix_pr_body:
|
||||
|
|
|
|||
|
|
@ -168,6 +168,8 @@ _REQUIRED_FIELDS = {
|
|||
|
||||
_VALID_FIX_EFFORT = frozenset({"trivial", "low", "medium", "high"})
|
||||
_VALID_CONFIDENCE = frozenset({"high", "medium", "low"})
|
||||
_MAX_HTTP_EXCHANGE_IDS = 10
|
||||
_MAX_HTTP_EXCHANGE_ID_CHARS = 128
|
||||
|
||||
|
||||
def _validate_required_text(fields: dict[str, str]) -> list[str]:
|
||||
|
|
@ -177,6 +179,41 @@ def _validate_required_text(fields: dict[str, str]) -> list[str]:
|
|||
]
|
||||
|
||||
|
||||
def _normalize_http_exchange_ids(raw: Any) -> tuple[list[str] | None, list[str]]:
|
||||
"""Return distinct proxy exchange ids in their original order."""
|
||||
if raw is None:
|
||||
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] = []
|
||||
seen: set[str] = set()
|
||||
for index, value in enumerate(raw):
|
||||
if not isinstance(value, str):
|
||||
errors.append(f"http_exchange_ids[{index}] must be a string")
|
||||
continue
|
||||
request_id = value.strip()
|
||||
if not request_id:
|
||||
errors.append(f"http_exchange_ids[{index}] cannot be empty")
|
||||
continue
|
||||
if len(request_id) > _MAX_HTTP_EXCHANGE_ID_CHARS:
|
||||
errors.append(
|
||||
f"http_exchange_ids[{index}] must be {_MAX_HTTP_EXCHANGE_ID_CHARS} "
|
||||
"characters or fewer"
|
||||
)
|
||||
continue
|
||||
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 request_id not in seen:
|
||||
seen.add(request_id)
|
||||
normalized.append(request_id)
|
||||
return normalized, errors
|
||||
|
||||
|
||||
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:
|
||||
|
|
@ -299,7 +336,7 @@ _UPDATE_TEXT_FIELDS = (
|
|||
)
|
||||
|
||||
|
||||
def _collect_update_changes( # noqa: PLR0912
|
||||
def _collect_update_changes( # noqa: PLR0912, PLR0915
|
||||
fields: dict[str, Any],
|
||||
) -> tuple[dict[str, Any], list[str]]:
|
||||
"""Validate the fields a revision replaces and return them with any errors."""
|
||||
|
|
@ -368,6 +405,12 @@ def _collect_update_changes( # noqa: PLR0912
|
|||
if cwe:
|
||||
changes["cwe"] = cwe
|
||||
|
||||
raw_http_exchange_ids = fields.get("http_exchange_ids")
|
||||
http_exchange_ids, http_exchange_errors = _normalize_http_exchange_ids(raw_http_exchange_ids)
|
||||
errors.extend(http_exchange_errors)
|
||||
if raw_http_exchange_ids is not None and not http_exchange_errors:
|
||||
changes["http_exchange_ids"] = http_exchange_ids or []
|
||||
|
||||
return changes, errors
|
||||
|
||||
|
||||
|
|
@ -378,6 +421,7 @@ _DYNAMIC_ONLY_UPDATE_FIELDS = (
|
|||
"method",
|
||||
"poc_description",
|
||||
"poc_script_code",
|
||||
"http_exchange_ids",
|
||||
)
|
||||
|
||||
# A dependency finding is rated in the context of the codebase that pins it, and
|
||||
|
|
@ -606,6 +650,7 @@ async def _do_create(
|
|||
cve: str | None,
|
||||
cwe: str | None,
|
||||
code_locations: list[dict[str, Any]] | None,
|
||||
http_exchange_ids: list[str] | None = None,
|
||||
confidence_rationale: str | None = None,
|
||||
fix_verification: str | None = None,
|
||||
fix_pr_body: str | None = None,
|
||||
|
|
@ -651,6 +696,10 @@ async def _do_create(
|
|||
errors.extend(_validate_fix_verification(parsed_locations, fix_verification))
|
||||
cve, cwe, identifier_errors = _validate_identifiers(cve, cwe)
|
||||
errors.extend(identifier_errors)
|
||||
normalized_http_exchange_ids, http_exchange_errors = _normalize_http_exchange_ids(
|
||||
http_exchange_ids
|
||||
)
|
||||
errors.extend(http_exchange_errors)
|
||||
|
||||
if errors:
|
||||
return {"success": False, "error": "Validation failed", "errors": errors}
|
||||
|
|
@ -712,6 +761,7 @@ async def _do_create(
|
|||
"code_locations": parsed_locations,
|
||||
"fix_verification": fix_verification,
|
||||
"fix_pr_body": fix_pr_body,
|
||||
"http_exchange_ids": normalized_http_exchange_ids,
|
||||
}
|
||||
|
||||
dedupe = await check_duplicate(candidate, existing)
|
||||
|
|
@ -796,6 +846,7 @@ async def create_vulnerability_report(
|
|||
cve: str | None = None,
|
||||
cwe: str | None = None,
|
||||
code_locations: list[dict[str, Any]] | None = None,
|
||||
http_exchange_ids: list[str] | None = None,
|
||||
confidence_rationale: str | None = None,
|
||||
fix_verification: str | None = None,
|
||||
fix_pr_body: str | None = None,
|
||||
|
|
@ -1040,6 +1091,10 @@ async def create_vulnerability_report(
|
|||
cve: ``CVE-YYYY-NNNNN`` if certain, else omit.
|
||||
cwe: ``CWE-NNN`` (most specific child) if certain, else omit.
|
||||
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.
|
||||
|
||||
**How ``fix_before`` / ``fix_after`` work**: they're used as
|
||||
literal GitHub/GitLab PR suggestion blocks. When a reviewer
|
||||
|
|
@ -1211,6 +1266,7 @@ async def create_vulnerability_report(
|
|||
cve=cve,
|
||||
cwe=cwe,
|
||||
code_locations=code_locations,
|
||||
http_exchange_ids=http_exchange_ids,
|
||||
fix_verification=fix_verification,
|
||||
fix_pr_body=fix_pr_body,
|
||||
agent_id=agent_id,
|
||||
|
|
@ -1245,6 +1301,7 @@ async def update_vulnerability_report(
|
|||
cve: str | None = None,
|
||||
cwe: str | None = None,
|
||||
code_locations: list[dict[str, Any]] | None = None,
|
||||
http_exchange_ids: list[str] | None = None,
|
||||
fix_verification: str | None = None,
|
||||
fix_pr_body: str | None = None,
|
||||
contextual_cvss_reasoning: str | None = None,
|
||||
|
|
@ -1317,6 +1374,8 @@ async def update_vulnerability_report(
|
|||
cve: Replacement CVE id.
|
||||
cwe: Replacement CWE id.
|
||||
code_locations: Replacement code locations.
|
||||
http_exchange_ids: Replacement proxy request ids. Pass an empty
|
||||
list to remove all linked exchanges.
|
||||
fix_verification: Verification statement for an applyable fix.
|
||||
fix_pr_body: Replacement fix PR body.
|
||||
contextual_cvss_reasoning: Dependency findings only. What you
|
||||
|
|
@ -1350,6 +1409,7 @@ async def update_vulnerability_report(
|
|||
"cve": cve,
|
||||
"cwe": cwe,
|
||||
"code_locations": code_locations,
|
||||
"http_exchange_ids": http_exchange_ids,
|
||||
"fix_verification": fix_verification,
|
||||
"fix_pr_body": fix_pr_body,
|
||||
"contextual_cvss_reasoning": contextual_cvss_reasoning,
|
||||
|
|
|
|||
|
|
@ -115,6 +115,7 @@ async def test_create_report_persists_new_fields(report_state: ReportState) -> N
|
|||
cve=None,
|
||||
cwe="CWE-79",
|
||||
code_locations=None,
|
||||
http_exchange_ids=["1042", "1042", "1088"],
|
||||
fix_pr_body="## Fix\nEncode output.",
|
||||
)
|
||||
assert result["success"] is True
|
||||
|
|
@ -127,6 +128,7 @@ async def test_create_report_persists_new_fields(report_state: ReportState) -> N
|
|||
assert report["counterevidence"] == "No output encoding or CSP observed on this response."
|
||||
assert report["confidence"] == "high"
|
||||
assert report["severity_change_conditions"] == "A strict CSP would lower the severity."
|
||||
assert report["http_exchange_ids"] == ["1042", "1088"]
|
||||
|
||||
|
||||
async def test_create_report_requires_evidence_and_assumptions(
|
||||
|
|
@ -1040,7 +1042,13 @@ def test_tool_descriptions_include_formatting_guidance() -> None:
|
|||
|
||||
def test_vuln_tool_exposes_new_params() -> None:
|
||||
props = create_vulnerability_report.params_json_schema["properties"]
|
||||
for field in ("evidence", "assumptions", "fix_effort", "fix_pr_body"):
|
||||
for field in (
|
||||
"evidence",
|
||||
"assumptions",
|
||||
"fix_effort",
|
||||
"fix_pr_body",
|
||||
"http_exchange_ids",
|
||||
):
|
||||
assert field in props
|
||||
|
||||
dep_props = create_dependency_report.params_json_schema["properties"]
|
||||
|
|
@ -1355,6 +1363,30 @@ def test_update_vulnerability_report_records_chained_impact(report_state: Report
|
|||
assert report_state.update_vulnerability_report("vuln-0404", {"severity": "high"}) is None
|
||||
|
||||
|
||||
def test_update_replaces_http_exchange_ids(report_state: ReportState) -> None:
|
||||
_seed_weak_report(report_state)
|
||||
|
||||
result = _do_update(
|
||||
report_id="vuln-0009",
|
||||
update_reason="A replay produced a clearer proving exchange.",
|
||||
fields={"http_exchange_ids": ["204", "204", "205"]},
|
||||
)
|
||||
|
||||
assert result["success"] is True
|
||||
assert report_state.vulnerability_reports[0]["http_exchange_ids"] == ["204", "205"]
|
||||
|
||||
|
||||
async def test_create_rejects_invalid_http_exchange_ids(report_state: ReportState) -> None:
|
||||
result = await _do_create(
|
||||
**_CONFIRMED_KWARGS,
|
||||
http_exchange_ids=["ok", "contains space"],
|
||||
)
|
||||
|
||||
assert result["success"] is False
|
||||
assert any("visible ASCII" in error for error in result["errors"])
|
||||
assert report_state.vulnerability_reports == []
|
||||
|
||||
|
||||
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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue