mirror of
https://github.com/usestrix/strix.git
synced 2026-10-14 04:08:06 +00:00
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.
This commit is contained in:
parent
e63d805973
commit
a8bd182f8f
4 changed files with 52 additions and 4 deletions
|
|
@ -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)
|
||||
|
|
@ -266,7 +280,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"),
|
||||
}
|
||||
|
|
|
|||
|
|
@ -42,8 +42,10 @@ def csv_safe(value: object) -> str:
|
|||
scanned target, which is exactly the attacker-influenced input this guards
|
||||
against.
|
||||
|
||||
Prefixing with an apostrophe is the standard mitigation: spreadsheets treat
|
||||
the rest of the cell as literal text and hide the apostrophe on display.
|
||||
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):
|
||||
|
|
@ -197,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",
|
||||
|
|
|
|||
|
|
@ -27,6 +27,20 @@ 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 _seed(state: ReportState) -> None:
|
||||
state.add_vulnerability_report(
|
||||
title="Reflected XSS in search",
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
@ -210,6 +211,17 @@ def test_write_vulnerabilities_csv_leaves_benign_titles_unchanged(tmp_path: Path
|
|||
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"}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue