diff --git a/strix/report/state.py b/strix/report/state.py index 6f111178f..00a59deef 100644 --- a/strix/report/state.py +++ b/strix/report/state.py @@ -312,7 +312,11 @@ def add_vulnerability_report( if self.vulnerability_found_callback: self.vulnerability_found_callback(report) - self.save_run_data() + try: + self.save_run_data() + except Exception: + self.vulnerability_reports.pop() + raise return report_id def get_existing_vulnerabilities(self) -> list[dict[str, Any]]: @@ -371,6 +375,10 @@ def update_scan_final_fields( technical_analysis: str, recommendations: str, ) -> None: + prev_scan_results = self.scan_results + prev_final_scan_result = self.final_scan_result + prev_run_scan_results = self.run_record.get("scan_results") + self.scan_results = { "scan_completed": True, "executive_summary": executive_summary.strip(), @@ -384,7 +392,17 @@ def update_scan_final_fields( self.run_record["scan_results"] = self.scan_results logger.info("Updated scan final fields") - self.save_run_data(mark_complete=True) + try: + self.save_run_data(mark_complete=True) + except Exception: + self.scan_results = prev_scan_results + self.final_scan_result = prev_final_scan_result + if prev_run_scan_results is None: + self.run_record.pop("scan_results", None) + else: + self.run_record["scan_results"] = prev_run_scan_results + raise + posthog.end(self, exit_reason="finished_by_tool") scarf.end(self, exit_reason="finished_by_tool") @@ -410,6 +428,9 @@ def set_scan_config(self, config: dict[str, Any]) -> None: ) def save_run_data(self, mark_complete: bool = False, status: str | None = None) -> None: + prev_end_time = self.end_time + prev_status = self.run_record.get("status") + if mark_complete: self.end_time = datetime.now(UTC).isoformat() self.run_record["end_time"] = self.end_time @@ -424,10 +445,26 @@ def save_run_data(self, mark_complete: bool = False, status: str | None = None) self.run_record["status"] = status self._sync_llm_usage_record() - self._save_artifacts() + try: + self._save_artifacts() + except Exception: + if mark_complete or status: + self.end_time = prev_end_time + if prev_end_time is None: + self.run_record.pop("end_time", None) + else: + self.run_record["end_time"] = prev_end_time + if prev_status is not None: + self.run_record["status"] = prev_status + else: + self.run_record.pop("status", None) + raise def cleanup(self, status: str = "stopped") -> None: - self.save_run_data(status=status) + try: + self.save_run_data(status=status) + except (OSError, RuntimeError): + logger.exception("Failed to save scan data during cleanup") def _format_final_scan_result(self, scan_results: dict[str, Any]) -> str: return f"""# Executive Summary @@ -480,6 +517,7 @@ def _save_artifacts(self) -> None: logger.info("Essential scan data saved to: %s", run_dir) except (OSError, RuntimeError): logger.exception("Failed to save scan data") + raise def _sarif_repository_context(self) -> dict[str, Any] | None: """Repo/commit/branch context for SARIF provenance (repo scans only). diff --git a/strix/tools/finish/tool.py b/strix/tools/finish/tool.py index b9704ad1a..9277aaba6 100644 --- a/strix/tools/finish/tool.py +++ b/strix/tools/finish/tool.py @@ -63,7 +63,7 @@ def _do_finish( recommendations=recommendations.strip(), ) vuln_count = len(report_state.vulnerability_reports) - except (ImportError, AttributeError) as e: + except (ImportError, AttributeError, OSError, RuntimeError) as e: logger.exception("finish_scan persistence failed") return {"success": False, "error": f"Failed to complete scan: {e!s}"} else: diff --git a/strix/tools/reporting/tool.py b/strix/tools/reporting/tool.py index 6cae75f1c..f1574d316 100644 --- a/strix/tools/reporting/tool.py +++ b/strix/tools/reporting/tool.py @@ -309,7 +309,7 @@ async def _do_create( # noqa: PLR0912 agent_id=agent_id if isinstance(agent_id, str) else None, agent_name=agent_name if isinstance(agent_name, str) else None, ) - except (ImportError, AttributeError) as e: + except (ImportError, AttributeError, OSError, RuntimeError) as e: logger.exception("create_vulnerability_report persistence failed") return {"success": False, "error": f"Failed to create vulnerability report: {e!s}"} else: @@ -1088,7 +1088,7 @@ async def _do_create_dependency( # noqa: PLR0912 agent_id=agent_id if isinstance(agent_id, str) else None, agent_name=agent_name if isinstance(agent_name, str) else None, ) - except (ImportError, AttributeError) as e: + except (ImportError, AttributeError, OSError, RuntimeError) as e: logger.exception("create_dependency_report persistence failed") return {"success": False, "error": f"Failed to create dependency report: {e!s}"} else: diff --git a/tests/test_reporting_fields.py b/tests/test_reporting_fields.py index d433db40f..7636ae225 100644 --- a/tests/test_reporting_fields.py +++ b/tests/test_reporting_fields.py @@ -12,7 +12,7 @@ check_duplicate, ) from strix.report.state import ReportState, set_global_report_state -from strix.tools.finish.tool import finish_scan +from strix.tools.finish.tool import _do_finish, finish_scan from strix.tools.reporting.tool import ( _do_create, _do_create_dependency, @@ -1087,3 +1087,103 @@ async def test_dependency_report_rejects_contextual_breakdown_without_reasoning( assert result["success"] is False assert any("contextual_cvss_reasoning is required" in error for error in result["errors"]) assert report_state.vulnerability_reports == [] + + +def test_save_artifacts_propagates_write_error( + report_state: ReportState, monkeypatch: pytest.MonkeyPatch +) -> None: + def _failing_write(*_args: object, **_kwargs: object) -> None: + raise OSError("Disk quota exceeded") + + monkeypatch.setattr("strix.report.state.write_executive_report", _failing_write) + report_state.final_scan_result = "Test report" + + with pytest.raises(OSError, match="Disk quota exceeded"): + report_state.save_run_data() + + +@pytest.mark.asyncio +async def test_finish_scan_reports_failure_on_artifact_write_error( + report_state: ReportState, monkeypatch: pytest.MonkeyPatch +) -> None: + def _failing_write(*_args: object, **_kwargs: object) -> None: + raise OSError("Permission denied") + + monkeypatch.setattr("strix.report.state.write_executive_report", _failing_write) + + result = _do_finish( + parent_id=None, + executive_summary="Summary", + methodology="Methodology", + technical_analysis="Analysis", + recommendations="Recommendations", + ) + + assert result["success"] is False + assert "Failed to complete scan: Permission denied" in result["error"] + assert "scan_completed" not in result or result.get("scan_completed") is not True + assert report_state.scan_results is None + + +@pytest.mark.asyncio +async def test_failed_finish_rolls_back_in_memory_completion_state( + report_state: ReportState, monkeypatch: pytest.MonkeyPatch +) -> None: + def _failing_write(*_args: object, **_kwargs: object) -> None: + raise OSError("Permission denied") + + monkeypatch.setattr("strix.report.state.write_executive_report", _failing_write) + + result = _do_finish( + parent_id=None, + executive_summary="Summary", + methodology="Methodology", + technical_analysis="Analysis", + recommendations="Recommendations", + ) + + assert result["success"] is False + assert report_state.scan_results is None + assert report_state.final_scan_result is None + assert report_state.run_record.get("status") == "running" + + monkeypatch.undo() + report_state.cleanup(status="stopped") + assert report_state.run_record.get("status") == "stopped" + assert report_state.run_record.get("scan_results") is None + + +@pytest.mark.asyncio +async def test_create_vulnerability_report_reports_failure_on_write_error( + report_state: ReportState, monkeypatch: pytest.MonkeyPatch +) -> None: + def _failing_write(*_args: object, **_kwargs: object) -> None: + raise OSError("Read-only file system") + + monkeypatch.setattr("strix.report.state.write_vulnerabilities", _failing_write) + + result = await _do_create( + title="SQL Injection in Login", + description="Exploitable SQL injection in authentication form.", + impact="Full database access.", + target="http://example.com/login", + technical_analysis="Unsanitized user input passed to query.", + poc_description="Submit ' OR '1'='1 payload.", + poc_script_code="curl http://example.com/login", + remediation_steps="Use parameterized queries.", + evidence="HTTP 500 SQL error trace", + assumptions="Target DB is PostgreSQL", + fix_effort="low", + cvss_breakdown=_CVSS, + endpoint="/login", + method="POST", + cve=None, + cwe="CWE-89", + code_locations=None, + fix_pr_body=None, + ) + + assert result["success"] is False + assert "Failed to create vulnerability report: Read-only file system" in result["error"] + assert report_state.vulnerability_reports == [] +