diff --git a/strix/agents/prompts/system_prompt.jinja b/strix/agents/prompts/system_prompt.jinja index 77688fa8..5fea2cb9 100644 --- a/strix/agents/prompts/system_prompt.jinja +++ b/strix/agents/prompts/system_prompt.jinja @@ -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. diff --git a/strix/report/state.py b/strix/report/state.py index 6bda587f..112ff0de 100644 --- a/strix/report/state.py +++ b/strix/report/state.py @@ -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) diff --git a/strix/report/writer.py b/strix/report/writer.py index 3c11cea5..1648e1f2 100644 --- a/strix/report/writer.py +++ b/strix/report/writer.py @@ -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) diff --git a/strix/tools/reporting/tool.py b/strix/tools/reporting/tool.py index 455e2d9b..6ce553fd 100644 --- a/strix/tools/reporting/tool.py +++ b/strix/tools/reporting/tool.py @@ -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) diff --git a/tests/test_vulnerability_update_tool.py b/tests/test_vulnerability_update_tool.py index b8332ea0..3aa1588a 100644 --- a/tests/test_vulnerability_update_tool.py +++ b/tests/test_vulnerability_update_tool.py @@ -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: