From 02e3b6ddf05411093aad0bad766ffbb78910b8cb Mon Sep 17 00:00:00 2001 From: thejesh23 Date: Tue, 21 Jul 2026 16:47:15 -0700 Subject: [PATCH] fix(report): write the executive report atomically `write_executive_report` was the only artifact writer in `writer.py` not routed through the module's own `_atomic_write_text` helper. It opened the destination with `path.open("w")`, truncating `penetration_test_report.md` in place before writing any new bytes. Every sibling writer is already atomic: `write_run_record`, the per-vulnerability markdown, `vulnerabilities.csv` and `vulnerabilities.json` all go through `_atomic_write_text`, which writes a temp file in the same directory and `Path.replace()`s it into place. So a failure between the `open("w")` and the last `write` (ENOSPC, a disconnected mount, SIGINT, an OOM kill) left the primary human-readable deliverable truncated or empty, with the previous complete report already gone. Build the payload first, then hand it to `_atomic_write_text`, so a failed render leaves the previous report intact. Adds a regression test that fails on main: write a report, make the timestamp render raise, and assert the first report survives. --- strix/report/writer.py | 11 +++++++---- tests/test_report_writer.py | 26 ++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/strix/report/writer.py b/strix/report/writer.py index 686792633..389d67c38 100644 --- a/strix/report/writer.py +++ b/strix/report/writer.py @@ -57,10 +57,13 @@ def write_run_record(run_dir: Path, run_record: dict[str, Any]) -> 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") + generated = datetime.now(UTC).strftime("%Y-%m-%d %H:%M:%S UTC") + _atomic_write_text( + path, + "# Security Penetration Test Report\n\n" + f"**Generated:** {generated}\n\n" + f"{final_scan_result}\n", + ) logger.info("Saved final penetration test report to: %s", path) diff --git a/tests/test_report_writer.py b/tests/test_report_writer.py index b9d855a08..94c304d0f 100644 --- a/tests/test_report_writer.py +++ b/tests/test_report_writer.py @@ -8,6 +8,7 @@ from typing import TYPE_CHECKING, Any import pytest +from strix.report import writer from strix.report.writer import ( read_run_record, render_vulnerability_md, @@ -177,3 +178,28 @@ def test_write_executive_report_writes_markdown(tmp_path: Path) -> None: content = (tmp_path / "penetration_test_report.md").read_text(encoding="utf-8") assert "# Security Penetration Test Report" in content assert "Scan complete. No critical issues." in content + + +def test_write_executive_report_keeps_previous_report_when_rendering_fails( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + write_executive_report(tmp_path, "First complete report.") + path = tmp_path / "penetration_test_report.md" + + class _BoomDatetime: + @staticmethod + def now(_tz: Any) -> Any: + raise OSError("clock unavailable") + + monkeypatch.setattr(writer, "datetime", _BoomDatetime) + + with pytest.raises(OSError, match="clock unavailable"): + write_executive_report(tmp_path, "Second report.") + + assert "First complete report." in path.read_text(encoding="utf-8") + + +def test_write_executive_report_leaves_no_temp_files(tmp_path: Path) -> None: + write_executive_report(tmp_path, "Scan complete.") + assert [p.name for p in tmp_path.iterdir()] == ["penetration_test_report.md"]