mirror of
https://github.com/usestrix/strix.git
synced 2026-08-20 02:23:35 +02:00
Refine vulnerability report amendments
This commit is contained in:
@@ -218,7 +218,7 @@ VALIDATION REQUIREMENTS:
|
||||
- Keep going until you find something that matters
|
||||
- A vulnerability is ONLY considered reported when a reporting agent uses create_vulnerability_report or update_vulnerability_report (or create_dependency_report for known-CVE dependency/supply-chain findings) with full details. Mentions in agent_finish, finish_scan, or generic messages are NOT sufficient
|
||||
- Reporting and fixing are ONE step, not two: when source is available, the reporting agent derives the concrete fix and files it INLINE via create_vulnerability_report (`code_locations` with `fix_before`/`fix_after` + `fix_pr_body`) — the report is not complete without it. Do NOT report first and then spawn a separate downstream agent to re-derive and re-apply the same patch; that just re-does the analysis and wastes tokens. (Do not silently patch a finding WITHOUT filing a report — the report, with its embedded fix, is the deliverable.)
|
||||
- DEDUPLICATION: The create_vulnerability_report tool uses LLM-based deduplication. If it rejects your report as a duplicate, DO NOT attempt to re-submit the same vulnerability. Accept the rejection and move on to testing other areas. The vulnerability has already been reported by another agent
|
||||
- DEDUPLICATION: The create_vulnerability_report tool uses LLM-based deduplication. If it rejects your report as a duplicate, do not re-submit the same vulnerability. If new validated evidence shows greater or lower impact for the same finding, use update_vulnerability_report with the returned duplicate_of id. Otherwise, move on to testing other areas. The vulnerability has already been reported
|
||||
- DISTINCT ISSUE VS AMENDMENT: File a distinct issue with create_vulnerability_report. Use update_vulnerability_report when new evidence proves greater or lower impact for the same root cause on the same asset. Supply an evidence-backed update_reason and recalculate the rating from cvss_breakdown
|
||||
- REVIEWING FILED FINDINGS (orchestrator/root agent): use list_reports to see every vulnerability filed so far in this scan (by any agent, root or child) — metadata-first with per-severity counts — and get_report to read one finding in full by its id. These are read-only orchestration tools: the root agent uses them to track coverage, avoid dispatching work on already-covered ground, assemble the finish_scan executive summary, and reason about attack-chaining across confirmed findings. Leaf/specialist agents should NOT call them — just do your assigned testing and file findings. Each entry shows which agent filed it (agent_name), and your own entries are flagged by_you. list_notes/get_note do the same for notes.
|
||||
</execution_guidelines>
|
||||
|
||||
@@ -502,11 +502,7 @@ class ReportState:
|
||||
run_dir.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
if self.final_scan_result:
|
||||
write_executive_report(
|
||||
run_dir,
|
||||
self.final_scan_result,
|
||||
self.vulnerability_reports,
|
||||
)
|
||||
write_executive_report(run_dir, self.final_scan_result)
|
||||
|
||||
if self.vulnerability_reports:
|
||||
write_vulnerabilities(run_dir, self.vulnerability_reports, self._saved_vuln_ids)
|
||||
|
||||
@@ -113,20 +113,12 @@ def write_run_record(run_dir: Path, run_record: dict[str, Any]) -> None:
|
||||
)
|
||||
|
||||
|
||||
def write_executive_report(
|
||||
run_dir: Path,
|
||||
final_scan_result: str,
|
||||
vulnerability_reports: list[dict[str, Any]] | None = None,
|
||||
) -> None:
|
||||
def write_executive_report(run_dir: Path, final_scan_result: str) -> None:
|
||||
path = run_dir / "penetration_test_report.md"
|
||||
with path.open("w", encoding="utf-8") as f:
|
||||
f.write("# Security Penetration Test Report\n\n")
|
||||
f.write(f"**Generated:** {datetime.now(UTC).strftime('%Y-%m-%d %H:%M:%S UTC')}\n\n")
|
||||
f.write(f"{final_scan_result}\n")
|
||||
if vulnerability_reports:
|
||||
f.write("\n## Vulnerability Findings\n\n")
|
||||
for report in vulnerability_reports:
|
||||
f.write(f"{render_vulnerability_md(report)}\n")
|
||||
logger.info("Saved final penetration test report to: %s", path)
|
||||
|
||||
|
||||
|
||||
@@ -295,7 +295,10 @@ async def _do_create( # noqa: PLR0912
|
||||
"success": False,
|
||||
"error": (
|
||||
f"Potential duplicate of '{duplicate_title}' "
|
||||
f"(id={duplicate_id[:8]}...) — do not re-report the same vulnerability"
|
||||
f"(id={duplicate_id[:8]}...) — do not re-report the same vulnerability. "
|
||||
f"If new validated evidence shows greater or lower impact than filed, "
|
||||
f"amend this finding with update_vulnerability_report using id "
|
||||
f"'{duplicate_id}' instead."
|
||||
),
|
||||
"duplicate_of": duplicate_id,
|
||||
"duplicate_title": duplicate_title,
|
||||
@@ -366,30 +369,8 @@ async def _do_update( # noqa: PLR0911, PLR0912, PLR0915
|
||||
method: str | None = None,
|
||||
cwe: str | None = None,
|
||||
code_locations: list[dict[str, Any]] | None = None,
|
||||
target: str | None = None,
|
||||
cve: str | None = None,
|
||||
dependency_metadata: dict[str, Any] | None = None,
|
||||
) -> dict[str, Any]:
|
||||
"""Validate and amend one known dynamic vulnerability report."""
|
||||
immutable_attempts = {
|
||||
name: value
|
||||
for name, value in {
|
||||
"target": target,
|
||||
"cve": cve,
|
||||
"dependency_metadata": dependency_metadata,
|
||||
}.items()
|
||||
if value is not None
|
||||
}
|
||||
if immutable_attempts:
|
||||
fields = ", ".join(immutable_attempts)
|
||||
return {
|
||||
"success": False,
|
||||
"error": (
|
||||
f"Cannot amend {fields}. File a new report instead because "
|
||||
"the target and dependency metadata identify the finding."
|
||||
),
|
||||
}
|
||||
|
||||
if not report_id.strip():
|
||||
return {"success": False, "error": "report_id cannot be empty"}
|
||||
if not update_reason.strip():
|
||||
@@ -493,13 +474,15 @@ async def _do_update( # noqa: PLR0911, PLR0912, PLR0915
|
||||
updates["cwe"] = parsed_cwe
|
||||
|
||||
if "code_locations" in raw_updates:
|
||||
if not isinstance(code_locations, list):
|
||||
errors.append("code_locations must be a list")
|
||||
if not isinstance(code_locations, list) or not code_locations:
|
||||
errors.append("code_locations must contain at least one location")
|
||||
else:
|
||||
parsed_locations = _normalize_code_locations(code_locations)
|
||||
if parsed_locations:
|
||||
if not parsed_locations:
|
||||
errors.append("code_locations must contain at least one valid location")
|
||||
else:
|
||||
errors.extend(_validate_code_locations(parsed_locations))
|
||||
updates["code_locations"] = parsed_locations or []
|
||||
updates["code_locations"] = parsed_locations
|
||||
|
||||
if errors:
|
||||
return {"success": False, "error": "Validation failed", "errors": errors}
|
||||
@@ -622,8 +605,10 @@ async def create_vulnerability_report(
|
||||
|
||||
Automatic LLM-based **deduplication** rejects reports that describe
|
||||
the same root cause on the same asset as an existing report. If you
|
||||
get a ``duplicate_of`` response, do NOT retry — move on to other
|
||||
areas.
|
||||
get a ``duplicate_of`` response, do not re-submit the same vulnerability.
|
||||
If new validated evidence shows greater or lower impact for that finding,
|
||||
amend it with ``update_vulnerability_report`` using the returned id.
|
||||
Otherwise, move on to other areas.
|
||||
|
||||
**Report output rules** (this content may be rendered into generated
|
||||
reports):
|
||||
@@ -935,9 +920,6 @@ async def update_vulnerability_report(
|
||||
method: str | None = None,
|
||||
cwe: str | None = None,
|
||||
code_locations: list[dict[str, Any]] | None = None,
|
||||
target: str | None = None,
|
||||
cve: str | None = None,
|
||||
dependency_metadata: dict[str, Any] | None = None,
|
||||
) -> str:
|
||||
"""Amend a known vulnerability report when new evidence changes its impact.
|
||||
|
||||
@@ -977,9 +959,6 @@ async def update_vulnerability_report(
|
||||
method=method,
|
||||
cwe=cwe,
|
||||
code_locations=code_locations,
|
||||
target=target,
|
||||
cve=cve,
|
||||
dependency_metadata=dependency_metadata,
|
||||
)
|
||||
return json.dumps(result, ensure_ascii=False, default=str)
|
||||
|
||||
|
||||
@@ -61,6 +61,14 @@ def report_state(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> ReportState
|
||||
endpoint="/redirect",
|
||||
method="GET",
|
||||
cwe="CWE-601",
|
||||
code_locations=[
|
||||
{
|
||||
"file": "src/redirect.py",
|
||||
"start_line": 10,
|
||||
"end_line": 12,
|
||||
"snippet": "return redirect(url)",
|
||||
},
|
||||
],
|
||||
)
|
||||
return state
|
||||
|
||||
@@ -99,20 +107,6 @@ async def test_update_requires_nonempty_reason() -> None:
|
||||
assert result == {"success": False, "error": "update_reason cannot be empty"}
|
||||
|
||||
|
||||
async def test_update_rejects_target_and_cve_changes() -> None:
|
||||
result = await _do_update(
|
||||
report_id="vuln-0001",
|
||||
update_reason="The chain reached a second asset.",
|
||||
target="https://other.example.com",
|
||||
cve="CVE-2024-12345",
|
||||
)
|
||||
|
||||
assert result["success"] is False
|
||||
assert "File a new report instead" in result["error"]
|
||||
assert "target" in result["error"]
|
||||
assert "cve" in result["error"]
|
||||
|
||||
|
||||
async def test_update_changes_impact_only(report_state: ReportState) -> None:
|
||||
result = await _do_update(
|
||||
report_id="vuln-0001",
|
||||
@@ -180,9 +174,6 @@ async def test_update_callback_fires(report_state: ReportState) -> None:
|
||||
|
||||
|
||||
async def test_update_persists_all_report_artifacts(report_state: ReportState) -> None:
|
||||
report_state.final_scan_result = "# Executive Summary\n\nThe scan is complete."
|
||||
report_state.save_run_data()
|
||||
|
||||
result = await _do_update(
|
||||
report_id="vuln-0001",
|
||||
update_reason="The chain proves account takeover.",
|
||||
@@ -194,13 +185,11 @@ async def test_update_persists_all_report_artifacts(report_state: ReportState) -
|
||||
assert result["success"] is True
|
||||
run_dir = report_state.get_run_dir()
|
||||
finding_md = (run_dir / "vulnerabilities" / "vuln-0001.md").read_text(encoding="utf-8")
|
||||
executive_md = (run_dir / "penetration_test_report.md").read_text(encoding="utf-8")
|
||||
findings = json.loads((run_dir / "vulnerabilities.json").read_text(encoding="utf-8"))
|
||||
sarif = json.loads((run_dir / "findings.sarif").read_text(encoding="utf-8"))
|
||||
sarif_finding = sarif["runs"][0]["results"][0]
|
||||
|
||||
assert "An attacker can take over an account." in finding_md
|
||||
assert "An attacker can take over an account." in executive_md
|
||||
assert findings[0]["impact"] == "An attacker can take over an account."
|
||||
assert findings[0]["severity"] == "critical"
|
||||
assert sarif_finding["properties"]["strix"]["impact"] == (
|
||||
@@ -209,6 +198,27 @@ async def test_update_persists_all_report_artifacts(report_state: ReportState) -
|
||||
assert sarif_finding["properties"]["strix"]["severity"] == "critical"
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"code_locations",
|
||||
[[], [{"file": "../invalid.py", "start_line": 1}]],
|
||||
)
|
||||
async def test_update_rejects_empty_code_locations(
|
||||
report_state: ReportState,
|
||||
code_locations: list[dict[str, Any]],
|
||||
) -> None:
|
||||
original_locations = report_state.vulnerability_reports[0]["code_locations"]
|
||||
|
||||
result = await _do_update(
|
||||
report_id="vuln-0001",
|
||||
update_reason="The source review did not provide a valid location.",
|
||||
code_locations=code_locations,
|
||||
)
|
||||
|
||||
assert result["success"] is False
|
||||
assert any("code_locations" in error for error in result["errors"])
|
||||
assert report_state.vulnerability_reports[0]["code_locations"] == original_locations
|
||||
|
||||
|
||||
async def test_update_fails_without_global_report_state(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
|
||||
Reference in New Issue
Block a user