diff --git a/strix/agents/factory.py b/strix/agents/factory.py index b2fcbf08..93a019d7 100644 --- a/strix/agents/factory.py +++ b/strix/agents/factory.py @@ -50,6 +50,7 @@ from strix.tools.proxy.tools import ( from strix.tools.reporting.tool import ( create_dependency_report, create_vulnerability_report, + delete_vulnerability_report, get_report, list_reports, update_vulnerability_report, @@ -583,6 +584,7 @@ _BASE_TOOLS: tuple[Tool, ...] = ( create_vulnerability_report, create_dependency_report, update_vulnerability_report, + delete_vulnerability_report, list_reports, get_report, list_requests, diff --git a/strix/agents/prompts/system_prompt.jinja b/strix/agents/prompts/system_prompt.jinja index 18fffa99..8a2e1bcc 100644 --- a/strix/agents/prompts/system_prompt.jinja +++ b/strix/agents/prompts/system_prompt.jinja @@ -242,6 +242,7 @@ VALIDATION REQUIREMENTS: - 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. If your evidence proves more than the finding it matched (a working exploit where that one had only a static trace, a chain that raises the impact), revise that finding with update_vulnerability_report using the duplicate_of id — never re-file it. - HTTP EVIDENCE: a finding you validated through the proxy is not fully filed until `http_exchange_ids` carries the proxy request ids of the exchanges that prove it — the request that triggers the vulnerability plus the baseline/control request it differs from (an unauthenticated success next to the authenticated one, the payload response next to the benign one). Copy the ids exactly as `list_requests`/`view_request` show them, never invent or guess one, and never omit the field to bypass validation. Leave it out only when there is no captured HTTP exchange at all (static-only code findings, dependency CVEs). If you filed before the proving exchanges existed, attach them afterwards with update_vulnerability_report. Without the ids, the finding ships as prose nobody can replay. - REVISING A FINDING: use update_vulnerability_report (report id + the fields you want to replace + update_reason) when you learn something a finding already on file does not carry — you built the PoC after filing it, a chain raised its impact, further testing weakened it, or its counterevidence/remediation/code locations were wrong. Editing a finding needs no duplicate verdict, and it is always better than filing a second report for the same issue. Read the finding first with get_report, and pass only the fields that change. +- WITHDRAWING A FINDING: when re-testing proves a finding you filed is not a vulnerability at all — the exploit only worked because of your own test setup (a mixed-up session, a self-inflicted state change, a misread response), the behaviour is intended, or the control is in fact enforced — delete it with delete_vulnerability_report (report id + delete_reason). Never leave a disproved finding on file as a zero-CVSS, "retracted", or "false positive" report, and never rewrite its text into a note saying it was wrong: a report that exists is read as a finding. Delete is for a finding that does not exist; a finding that is real but weaker than filed is revised, not deleted. - 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. STATE & COORDINATION TOOLS (when and how): diff --git a/strix/interface/tui/internal/render/registry.go b/strix/interface/tui/internal/render/registry.go index a7cfc6c7..9b7f4f35 100644 --- a/strix/interface/tui/internal/render/registry.go +++ b/strix/interface/tui/internal/render/registry.go @@ -86,6 +86,8 @@ func Tool(data map[string]any) string { return renderVulnerabilityReport(args, result) case "update_vulnerability_report": return renderVulnerabilityReportUpdate(args, result) + case "delete_vulnerability_report": + return renderVulnerabilityReportDelete(args, result) case "create_dependency_report": return renderDependencyReport(args, result) case "list_reports": diff --git a/strix/interface/tui/internal/render/report.go b/strix/interface/tui/internal/render/report.go index 4640225a..8b82b602 100644 --- a/strix/interface/tui/internal/render/report.go +++ b/strix/interface/tui/internal/render/report.go @@ -21,6 +21,36 @@ func renderVulnerabilityReportUpdate(args map[string]any, result any) string { return renderReport(args, result, "Vulnerability Report Updated", "Updating report...") } +// A deletion names the report it withdraws and the reason; the result carries +// the title the report had, so the reader knows what left the scan. +func renderVulnerabilityReportDelete(args map[string]any, result any) string { + resultMap, _ := result.(map[string]any) + var b strings.Builder + b.WriteString("🐞 " + Bold(ReportHdr).Render("Vulnerability Report Deleted")) + + reportID := StringValue(args["report_id"]) + if reportID != "" { + b.WriteString("\n\n" + Bold(Field).Render("Report: ") + reportID) + } + if title := StringValue(resultMap["title"]); title != "" { + b.WriteString("\n\n" + Bold(Field).Render("Title: ") + title) + } + if sev := StringValue(resultMap["severity"]); sev != "" { + b.WriteString("\n\n" + Bold(Field).Render("Severity: ") + + lipgloss.NewStyle().Bold(true).Foreground(SeverityColor(sev)).Render(strings.ToUpper(sev))) + } + if reason := StringValue(args["delete_reason"]); reason != "" { + b.WriteString("\n\n" + Bold(Field).Render("Reason") + "\n" + reason) + } + if errMsg := StringValue(resultMap["error"]); errMsg != "" { + b.WriteString("\n\n" + Col(SevHigh).Render(errMsg)) + } + if reportID == "" { + b.WriteString("\n " + Dim().Render("Deleting report...")) + } + return "\n\n" + b.String() + "\n\n" +} + func renderReport(args map[string]any, result any, heading, pending string) string { resultMap, _ := result.(map[string]any) var b strings.Builder diff --git a/strix/interface/viewer/frontend/src/components/live/tool-renderers/index.ts b/strix/interface/viewer/frontend/src/components/live/tool-renderers/index.ts index dcc568fb..13b9ba05 100644 --- a/strix/interface/viewer/frontend/src/components/live/tool-renderers/index.ts +++ b/strix/interface/viewer/frontend/src/components/live/tool-renderers/index.ts @@ -116,7 +116,7 @@ const CATEGORY_TOOLS: Record = { filesystem: ["apply_patch", "view_image", "str_replace_editor", "list_files", "search_files"], // Caido proxy tools (legacy: send_request) proxy: ["list_requests", "view_request", "repeat_request", "list_sitemap", "view_sitemap_entry", "scope_rules", "send_request"], - reporting: ["create_vulnerability_report", "update_vulnerability_report", "list_reports", "get_report"], + reporting: ["create_vulnerability_report", "update_vulnerability_report", "delete_vulnerability_report", "list_reports", "get_report"], thinking: ["think"], agents: ["create_agent", "agent_finish", "send_message_to_agent", "wait_for_agents", "view_agent_graph", "stop_agent"], search: ["web_search"], diff --git a/strix/report/state.py b/strix/report/state.py index ac15db89..52f29dca 100644 --- a/strix/report/state.py +++ b/strix/report/state.py @@ -221,6 +221,7 @@ class ReportState: self.caido_url: str | None = None self.vulnerability_found_callback: Callable[[dict[str, Any]], None] | None = None self.vulnerability_updated_callback: Callable[[dict[str, Any]], None] | None = None + self.vulnerability_deleted_callback: Callable[[dict[str, Any]], None] | None = None self._sarif_repo_ctx: dict[str, Any] | None = None self._sarif_repo_ctx_ready: bool = False @@ -342,7 +343,7 @@ class ReportState: agent_id: str | None = None, agent_name: str | None = None, ) -> str: - report_id = f"vuln-{len(self.vulnerability_reports) + 1:04d}" + report_id = self._next_report_id() report: dict[str, Any] = { "id": report_id, @@ -418,6 +419,24 @@ class ReportState: self.save_run_data() return report_id + def _deleted_vulnerability_reports(self) -> list[dict[str, Any]]: + raw = self.run_record.get("deleted_vulnerability_reports") + return [e for e in raw if isinstance(e, dict)] if isinstance(raw, list) else [] + + def _next_report_id(self) -> str: + """Allocate the id after every id this run has ever handed out. + + A deleted report leaves the list, so counting entries would hand its id + to the next finding and let that finding overwrite the deleted MD on disk + and inherit its history in every consumer that keys on the id. + """ + used = 0 + for entry in [*self.vulnerability_reports, *self._deleted_vulnerability_reports()]: + match = re.fullmatch(r"vuln-(\d+)", str(entry.get("id", ""))) + if match: + used = max(used, int(match.group(1))) + return f"vuln-{used + 1:04d}" + def update_vulnerability_report( self, report_id: str, @@ -516,6 +535,63 @@ class ReportState: self.save_run_data() return report + def delete_vulnerability_report( + self, + report_id: str, + *, + delete_reason: str, + deleted_by_agent_id: str | None = None, + deleted_by_agent_name: str | None = None, + ) -> dict[str, Any] | None: + """Withdraw a report from the run, keeping a record of the withdrawal. + + Returns the removed report, or ``None`` when the id is unknown. The + report leaves ``vulnerability_reports`` and its rendered artifacts; the + run record keeps who withdrew it and why, so the id is never reissued. + """ + report = next((r for r in self.vulnerability_reports if r.get("id") == report_id), None) + if report is None: + logger.warning("cannot delete unknown vulnerability report %s", report_id) + return None + + entry: dict[str, Any] = { + "id": report_id, + "title": report.get("title"), + "severity": report.get("severity"), + "filed_at": report.get("timestamp"), + "deleted_at": datetime.now(UTC).strftime("%Y-%m-%d %H:%M:%S UTC"), + "reason": delete_reason.strip()[:500], + } + if report.get("agent_id"): + entry["filed_by_agent_id"] = report["agent_id"] + if deleted_by_agent_id: + entry["agent_id"] = deleted_by_agent_id + if deleted_by_agent_name: + entry["agent_name"] = deleted_by_agent_name + + # Persistence must accept the deletion before local state changes. A + # failed callback leaves the report on file and the deletion retryable. + if self.vulnerability_deleted_callback: + self.vulnerability_deleted_callback({**report, "deletion": entry}) + + self.vulnerability_reports.remove(report) + self._saved_vuln_ids.discard(report_id) + self.run_record["deleted_vulnerability_reports"] = [ + *self._deleted_vulnerability_reports(), + entry, + ] + + md_path = self.get_run_dir() / "vulnerabilities" / f"{report_id}.md" + try: + md_path.unlink(missing_ok=True) + except OSError: + logger.exception("could not remove %s", md_path) + + logger.info("Deleted vulnerability report %s - %s", report_id, report.get("title")) + + self.save_run_data() + return report + def get_existing_vulnerabilities(self) -> list[dict[str, Any]]: return list(self.vulnerability_reports) @@ -714,7 +790,9 @@ class ReportState: if self.final_scan_result: write_executive_report(run_dir, self.final_scan_result) - if self.vulnerability_reports: + # An index is written for an empty list too once a report was + # deleted, or the CSV/JSON on disk would still list it. + if self.vulnerability_reports or self._deleted_vulnerability_reports(): write_vulnerabilities(run_dir, self.vulnerability_reports, self._saved_vuln_ids) # SARIF 2.1.0 emitter for CI / ASPM integration. Always emit (even diff --git a/strix/tools/reporting/tool.py b/strix/tools/reporting/tool.py index 720d42a4..a392fb37 100644 --- a/strix/tools/reporting/tool.py +++ b/strix/tools/reporting/tool.py @@ -696,6 +696,83 @@ def _do_update( } +def _do_delete( + *, + report_id: str, + delete_reason: str, + agent_id: str | None = None, + agent_name: str | None = None, +) -> dict[str, Any]: + """Withdraw a report an agent filed and has since disproved. + + Deletion is for a finding that is not a vulnerability at all. A finding + that is real but weaker than filed is revised, not deleted. + """ + report_id = (report_id or "").strip() + delete_reason = (delete_reason or "").strip() + if not report_id or not delete_reason: + missing = "report_id" if not report_id else "delete_reason" + return { + "success": False, + "error": ( + f"{missing} cannot be empty - name the report you are withdrawing and state " + "what disproved it" + ), + } + + from strix.report.state import get_global_report_state + + report_state = get_global_report_state() + if report_state is None: + return { + "success": False, + "error": "Report state unavailable - no reports have been filed yet", + } + + try: + deleted = report_state.delete_vulnerability_report( + report_id, + delete_reason=delete_reason, + deleted_by_agent_id=agent_id, + deleted_by_agent_name=agent_name, + ) + except Exception as e: + logger.exception("delete_vulnerability_report persistence failed") + return { + "success": False, + "error": ( + f"Failed to delete report '{report_id}': {e!s}. " + "The report is still on file; retry the deletion." + ), + "report_id": report_id, + } + if deleted is None: + return { + "success": False, + "error": f"Report with id '{report_id}' not found", + "report_id": report_id, + } + + logger.info( + "Vulnerability report %s deleted by %s: %s", + report_id, + agent_name or agent_id or "an agent", + delete_reason[:200], + ) + return { + "success": True, + "action": "deleted", + "message": ( + f"Report '{report_id}' ({deleted.get('title')}) is withdrawn and no longer " + "counts as a finding of this scan. Do not file it again unless new evidence " + "proves it." + ), + "report_id": report_id, + "title": deleted.get("title"), + "severity": deleted.get("severity"), + } + + async def _do_create( *, title: str, @@ -1548,6 +1625,48 @@ async def update_vulnerability_report( return json.dumps(_with_warning(result, http_exchange_warning), ensure_ascii=False, default=str) +@function_tool(timeout=60) +async def delete_vulnerability_report( + ctx: RunContextWrapper, + report_id: str, + delete_reason: str, +) -> str: + """Withdraw a vulnerability report that later testing disproved. + + Use this when a finding you filed turns out not to be a vulnerability + at all: the exploit only worked because of a mistake in your own test + setup (a mixed-up session, a self-inflicted state change, a misread + response), the behaviour is documented and intended, or the control + you believed was missing is in fact enforced. The report is removed + from the scan; it does not stay behind as a zero-severity entry. + + Do NOT use this for a finding that is real but weaker than filed — + revise it with ``update_vulnerability_report`` so the severity, the + impact and the counterevidence come down together. Do NOT rewrite a + report into a "retracted" or "false positive" note either: delete it. + + Only withdraw a report you can name and have re-tested yourself. Call + ``get_report`` first to read what it claims, then state in + ``delete_reason`` exactly what disproved it, so the scan history shows + why the finding is gone. A withdrawn report cannot be restored; if new + evidence later proves the issue, file it again. + + Args: + report_id: Id of the report to withdraw (format ``vuln-NNNN``). + delete_reason: What disproved the finding, in one or two + sentences. + """ + agent_id, agent_name = _caller_identity(ctx) + result = await asyncio.to_thread( + _do_delete, + report_id=report_id, + delete_reason=delete_reason, + agent_id=agent_id, + agent_name=agent_name, + ) + return json.dumps(result, ensure_ascii=False, default=str) + + _DEP_SEVERITY_FROM_CVSS = { (9.0, 10.0): "critical", (7.0, 9.0): "high", diff --git a/tests/test_reporting_fields.py b/tests/test_reporting_fields.py index cd40fe69..1635ccd6 100644 --- a/tests/test_reporting_fields.py +++ b/tests/test_reporting_fields.py @@ -19,11 +19,13 @@ from strix.tools.reporting import tool as reporting_tool from strix.tools.reporting.tool import ( _do_create, _do_create_dependency, + _do_delete, _do_update, _normalize_http_exchange_ids, _verify_http_exchange_ids, create_dependency_report, create_vulnerability_report, + delete_vulnerability_report, update_vulnerability_report, ) @@ -1976,3 +1978,137 @@ def test_update_refuses_code_locations_it_cannot_use(report_state: ReportState) assert result["success"] is False assert any("start_line" in error for error in result["errors"]) + + +def _seed_saved_report(report_state: ReportState) -> Path: + """The weak report, written to disk the way a filed finding is.""" + _seed_weak_report(report_state) + report_state._saved_vuln_ids.clear() + report_state.save_run_data() + run_dir = report_state._run_dir + assert run_dir is not None + md_path = run_dir / "vulnerabilities" / "vuln-0009.md" + assert md_path.exists() + return run_dir + + +def test_delete_withdraws_a_disproved_report_and_its_artifacts( + report_state: ReportState, +) -> None: + """A deleted finding leaves the run's state and every rendered artifact.""" + run_dir = _seed_saved_report(report_state) + persisted: list[dict[str, Any]] = [] + report_state.vulnerability_deleted_callback = persisted.append + + result = _do_delete( + report_id="vuln-0009", + delete_reason="The cross-tenant read came from the harness reusing the victim's session.", + agent_id="834f79fb", + agent_name="Validation Agent", + ) + + assert result["success"] is True + assert result["action"] == "deleted" + assert result["report_id"] == "vuln-0009" + assert result["title"].startswith("Directus 11.5.1") + assert report_state.vulnerability_reports == [] + assert not (run_dir / "vulnerabilities" / "vuln-0009.md").exists() + assert json.loads((run_dir / "vulnerabilities.json").read_text(encoding="utf-8")) == [] + assert "vuln-0009" not in (run_dir / "vulnerabilities.csv").read_text(encoding="utf-8") + + assert len(persisted) == 1 + assert persisted[0]["id"] == "vuln-0009" + assert persisted[0]["agent_id"] == "aaaa1111", "the callback sees the finding as filed" + deletion = persisted[0]["deletion"] + assert deletion["agent_id"] == "834f79fb" + assert deletion["agent_name"] == "Validation Agent" + assert deletion["filed_by_agent_id"] == "aaaa1111" + assert deletion["reason"].startswith("The cross-tenant read") + + run = json.loads((run_dir / "run.json").read_text(encoding="utf-8")) + history = run["deleted_vulnerability_reports"] + assert [entry["id"] for entry in history] == ["vuln-0009"] + assert history[0]["severity"] == "medium" + assert history[0]["deleted_at"] + + +def test_delete_never_reissues_the_withdrawn_id(report_state: ReportState) -> None: + """The next finding must not inherit a deleted report's id, file or history.""" + _seed_saved_report(report_state) + assert report_state.delete_vulnerability_report("vuln-0009", delete_reason="Disproved.") + + new_id = report_state.add_vulnerability_report(title="A real finding", severity="high") + + assert new_id == "vuln-0010" + assert [r["id"] for r in report_state.vulnerability_reports] == ["vuln-0010"] + + +def test_delete_reports_persistence_failure_and_keeps_the_report( + report_state: ReportState, +) -> None: + run_dir = _seed_saved_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_deleted_callback = fail_persistence + + result = _do_delete(report_id="vuln-0009", delete_reason="Disproved.") + + assert result["success"] is False + assert "persistence failed" in result["error"] + assert "still on file" in result["error"] + assert report_state.vulnerability_reports == [original] + assert (run_dir / "vulnerabilities" / "vuln-0009.md").exists() + assert "deleted_vulnerability_reports" not in report_state.run_record + assert report_state._next_report_id() == "vuln-0010" + + +@pytest.mark.parametrize( + ("report_id", "delete_reason", "expected"), + [ + (" ", "reason", "report_id cannot be empty"), + ("vuln-0009", " ", "delete_reason cannot be empty"), + ("vuln-0404", "reason", "not found"), + ], +) +def test_delete_rejects_a_call_it_cannot_act_on( + report_state: ReportState, + report_id: str, + delete_reason: str, + expected: str, +) -> None: + _seed_weak_report(report_state) + calls: list[dict[str, Any]] = [] + report_state.vulnerability_deleted_callback = calls.append + + result = _do_delete(report_id=report_id, delete_reason=delete_reason) + + assert result["success"] is False + assert expected in result["error"] + assert len(report_state.vulnerability_reports) == 1 + assert calls == [], "nothing reaches persistence for a call that is refused" + + +async def test_delete_tool_carries_the_caller_identity(report_state: ReportState) -> None: + _seed_saved_report(report_state) + coordinator = type("Coordinator", (), {"names": {"834f79fb": "Validation Agent"}})() + ctx = ToolContext( + context={"agent_id": "834f79fb", "coordinator": coordinator}, + tool_name="delete_vulnerability_report", + tool_call_id="call-1", + tool_arguments="{}", + ) + + raw = await delete_vulnerability_report.on_invoke_tool( + ctx, + json.dumps({"report_id": "vuln-0009", "delete_reason": "Disproved on re-test."}), + ) + result = json.loads(raw) + + assert result["success"] is True + history = report_state.run_record["deleted_vulnerability_reports"] + assert history[0]["agent_id"] == "834f79fb" + assert history[0]["agent_name"] == "Validation Agent" + assert "update_vulnerability_report" in delete_vulnerability_report.description