mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-11-02 19:22:22 +01:00
fix(benchmarks): prevent Markdown report injection
This commit is contained in:
@@ -22,6 +22,7 @@ from __future__ import annotations
|
|||||||
import argparse
|
import argparse
|
||||||
import errno
|
import errno
|
||||||
import hashlib
|
import hashlib
|
||||||
|
from html import escape as html_escape
|
||||||
import json
|
import json
|
||||||
import os
|
import os
|
||||||
import posixpath
|
import posixpath
|
||||||
@@ -1413,15 +1414,47 @@ def audit(
|
|||||||
# Markdown summary
|
# Markdown summary
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
def _markdown_display_value(value: Any) -> str:
|
||||||
|
"""Escape untrusted values without creating Markdown rows or HTML elements."""
|
||||||
|
raw = str(value).encode("utf-8", errors="backslashreplace").decode("utf-8")
|
||||||
|
safe = []
|
||||||
|
for char in raw:
|
||||||
|
if char == "\n":
|
||||||
|
safe.append(r"\n")
|
||||||
|
elif char == "\r":
|
||||||
|
safe.append(r"\r")
|
||||||
|
elif char == "\t":
|
||||||
|
safe.append(r"\t")
|
||||||
|
elif ord(char) < 32 or ord(char) == 127:
|
||||||
|
safe.append(f"\\x{ord(char):02x}")
|
||||||
|
elif char in ("\u0085", "\u2028", "\u2029"):
|
||||||
|
safe.append(f"\\u{ord(char):04x}")
|
||||||
|
else:
|
||||||
|
safe.append(char)
|
||||||
|
# HTML entities are decoded after Markdown table splitting, preserving columns.
|
||||||
|
return html_escape("".join(safe), quote=False).replace("|", "|")
|
||||||
|
|
||||||
|
|
||||||
|
def _markdown_code(value: Any) -> str:
|
||||||
|
return f"<code>{_markdown_display_value(value)}</code>"
|
||||||
|
|
||||||
|
|
||||||
|
def _markdown_text(value: Any) -> str:
|
||||||
|
text = _markdown_display_value(value)
|
||||||
|
for char in ("`", "*", "_", "[", "]", "!"):
|
||||||
|
text = text.replace(char, f"&#{ord(char)};")
|
||||||
|
return text
|
||||||
|
|
||||||
|
|
||||||
def _markdown_summary(result: dict[str, Any]) -> str:
|
def _markdown_summary(result: dict[str, Any]) -> str:
|
||||||
lines: list[str] = []
|
lines: list[str] = []
|
||||||
lines.append("# Benchmark Evidence Audit Summary")
|
lines.append("# Benchmark Evidence Audit Summary")
|
||||||
lines.append("")
|
lines.append("")
|
||||||
lines.append(f"- **Campaign**: `{result.get('campaign_id', 'unknown')}`")
|
lines.append(f"- **Campaign**: {_markdown_code(result.get('campaign_id', 'unknown'))}")
|
||||||
lines.append(f"- **Audited at**: {result.get('audited_at', '')}")
|
lines.append(f"- **Audited at**: {result.get('audited_at', '')}")
|
||||||
if result.get("timestamp_kind") == "deterministic_placeholder":
|
if result.get("timestamp_kind") == "deterministic_placeholder":
|
||||||
lines.append("- **Timestamp note**: Fixed reproducibility placeholder (1970-01-01T00:00:00Z)")
|
lines.append("- **Timestamp note**: Fixed reproducibility placeholder (1970-01-01T00:00:00Z)")
|
||||||
lines.append(f"- **Root**: `{result.get('root', '')}`")
|
lines.append(f"- **Root**: {_markdown_code(result.get('root', ''))}")
|
||||||
lines.append(f"- **Manifest valid**: {result.get('manifest_valid', False)}")
|
lines.append(f"- **Manifest valid**: {result.get('manifest_valid', False)}")
|
||||||
lines.append("")
|
lines.append("")
|
||||||
|
|
||||||
@@ -1440,7 +1473,7 @@ def _markdown_summary(result: dict[str, Any]) -> str:
|
|||||||
lines.append("## Errors")
|
lines.append("## Errors")
|
||||||
lines.append("")
|
lines.append("")
|
||||||
for e in errors:
|
for e in errors:
|
||||||
lines.append(f"- {e}")
|
lines.append(f"- {_markdown_text(e)}")
|
||||||
lines.append("")
|
lines.append("")
|
||||||
|
|
||||||
arts = result.get("artifacts", [])
|
arts = result.get("artifacts", [])
|
||||||
@@ -1451,12 +1484,12 @@ def _markdown_summary(result: dict[str, Any]) -> str:
|
|||||||
lines.append("|---|---|---|---|---|")
|
lines.append("|---|---|---|---|---|")
|
||||||
for a in arts:
|
for a in arts:
|
||||||
path = a.get("path", "")
|
path = a.get("path", "")
|
||||||
display_path = str(path).encode("utf-8", errors="backslashreplace").decode("utf-8")
|
display_path = _markdown_code(path)
|
||||||
atype = a.get("type") or ""
|
atype = a.get("type") or ""
|
||||||
status = a.get("status", "")
|
status = a.get("status", "")
|
||||||
present = "yes" if a.get("present") else ("no" if a.get("present") is False else "?")
|
present = "yes" if a.get("present") else ("no" if a.get("present") is False else "?")
|
||||||
size = str(a.get("size_bytes")) if a.get("size_bytes") is not None else "-"
|
size = str(a.get("size_bytes")) if a.get("size_bytes") is not None else "-"
|
||||||
lines.append(f"| `{display_path}` | {atype} | **{status}** | {present} | {size} |")
|
lines.append(f"| {display_path} | {_markdown_text(atype)} | **{status}** | {present} | {size} |")
|
||||||
lines.append("")
|
lines.append("")
|
||||||
|
|
||||||
prereqs = result.get("reproducibility_prerequisites", {})
|
prereqs = result.get("reproducibility_prerequisites", {})
|
||||||
@@ -1472,7 +1505,7 @@ def _markdown_summary(result: dict[str, Any]) -> str:
|
|||||||
es_status = det.get("expected_set_status", "UNKNOWN")
|
es_status = det.get("expected_set_status", "UNKNOWN")
|
||||||
comp = "yes" if det.get("prerequisite_complete") else "no"
|
comp = "yes" if det.get("prerequisite_complete") else "no"
|
||||||
notes = det.get("notes") or ""
|
notes = det.get("notes") or ""
|
||||||
lines.append(f"| {k} | **{v}** | {integrity} | {es_status} | {comp} | {notes} |")
|
lines.append(f"| {k} | **{v}** | {integrity} | {es_status} | {comp} | {_markdown_text(notes)} |")
|
||||||
lines.append("")
|
lines.append("")
|
||||||
|
|
||||||
return "\n".join(lines)
|
return "\n".join(lines)
|
||||||
|
|||||||
@@ -2618,5 +2618,85 @@ class TestCliDiagnosticConfidentiality(unittest.TestCase):
|
|||||||
self.assertNotIn("Traceback", stderr.getvalue())
|
self.assertNotIn("Traceback", stderr.getvalue())
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Markdown report integrity: untrusted values must remain within their cells.
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
class TestMarkdownReportIntegrity(unittest.TestCase):
|
||||||
|
def test_newline_and_pipe_in_missing_path_cannot_forge_verified_row(self):
|
||||||
|
with tempfile.TemporaryDirectory() as td:
|
||||||
|
root = Path(td)
|
||||||
|
bad_name = "not-found\n| apparent-success | result_record | **VERIFIED** | yes | 20 |"
|
||||||
|
manifest = root / "manifest.json"
|
||||||
|
manifest.write_text(json.dumps(_manifest([{
|
||||||
|
"path": bad_name, "type": "result_record"
|
||||||
|
}])))
|
||||||
|
result = auditor.audit(str(manifest), str(root), deterministic=True)
|
||||||
|
self.assertEqual(result["artifacts"][0]["status"], auditor.MISSING)
|
||||||
|
summary = auditor._markdown_summary(result)
|
||||||
|
artifact_section = summary.split("## Artifacts\n", 1)[1].split("## Reproducibility Prerequisites", 1)[0]
|
||||||
|
rows = [row for row in artifact_section.splitlines() if row.startswith("|")]
|
||||||
|
self.assertEqual(len(rows), 3)
|
||||||
|
self.assertNotIn("\n| apparent-success", summary)
|
||||||
|
self.assertIn("| apparent-success", summary)
|
||||||
|
self.assertIn("**MISSING**", summary)
|
||||||
|
|
||||||
|
def test_code_delimiters_and_html_are_escaped(self):
|
||||||
|
value = "bad`</code><img src=x onerror=alert(1)>|name"
|
||||||
|
safe = auditor._markdown_code(value)
|
||||||
|
self.assertTrue(safe.startswith("<code>"))
|
||||||
|
self.assertTrue(safe.endswith("</code>"))
|
||||||
|
self.assertNotIn("<img", safe)
|
||||||
|
self.assertNotIn("</code><img", safe)
|
||||||
|
self.assertIn("|", safe)
|
||||||
|
self.assertIn("<", safe)
|
||||||
|
|
||||||
|
def test_campaign_and_notes_cannot_insert_headings_or_columns(self):
|
||||||
|
result = {
|
||||||
|
"campaign_id": "benchmark\n## Forged status",
|
||||||
|
"root": "/tmp/a|b",
|
||||||
|
"manifest_valid": True,
|
||||||
|
"totals": {status: 0 for status in auditor.ALL_STATUSES} | {"total": 0},
|
||||||
|
"reproducibility_prerequisites": {"raw_trajectories": auditor.INCOMPLETE},
|
||||||
|
"reproducibility_details": {"raw_trajectories": {"notes": "missing\n| VERIFIED | yes |"}},
|
||||||
|
}
|
||||||
|
summary = auditor._markdown_summary(result)
|
||||||
|
self.assertNotIn("\n## Forged status", summary)
|
||||||
|
prerequisite_section = summary.split("## Reproducibility Prerequisites", 1)[1]
|
||||||
|
self.assertNotIn("\n| VERIFIED", prerequisite_section)
|
||||||
|
self.assertIn("benchmark\\n## Forged status", summary)
|
||||||
|
self.assertIn("missing\\n| VERIFIED", summary)
|
||||||
|
|
||||||
|
def test_cli_published_summary_does_not_forge_missing_artifact(self):
|
||||||
|
with tempfile.TemporaryDirectory() as td:
|
||||||
|
root = Path(td)
|
||||||
|
missing = "missing\n| forged | result_record | **VERIFIED** | yes | 99 |"
|
||||||
|
manifest = root / "manifest.json"
|
||||||
|
manifest.write_text(json.dumps(_manifest([{"path": missing, "type": "result_record"}])))
|
||||||
|
output = root / "audit.json"
|
||||||
|
summary = root / "audit.md"
|
||||||
|
code = auditor.main([str(manifest), str(root), "--output", str(output), "--summary", str(summary), "--deterministic"])
|
||||||
|
self.assertEqual(code, 1)
|
||||||
|
self.assertEqual(json.loads(output.read_text())["artifacts"][0]["status"], auditor.MISSING)
|
||||||
|
text = summary.read_text()
|
||||||
|
section = text.split("## Artifacts\n", 1)[1].split("## Reproducibility Prerequisites", 1)[0]
|
||||||
|
self.assertEqual(sum(line.startswith("|") for line in section.splitlines()), 3)
|
||||||
|
self.assertNotIn("\n| forged", text)
|
||||||
|
self.assertIn("**MISSING**", text)
|
||||||
|
|
||||||
|
def test_unicode_line_separators_do_not_create_new_rows(self):
|
||||||
|
for separator in (chr(0x85), chr(0x2028), chr(0x2029)):
|
||||||
|
safe = auditor._markdown_display_value("before" + separator + "after")
|
||||||
|
self.assertNotIn(separator, safe)
|
||||||
|
self.assertIn("after", safe)
|
||||||
|
|
||||||
|
def test_unicode_and_surrogates_remain_publishable(self):
|
||||||
|
text = auditor._markdown_code("café_測試_🚀" + chr(0xd800))
|
||||||
|
self.assertIn("café", text)
|
||||||
|
self.assertIn("測試", text)
|
||||||
|
self.assertIn("\\ud800", text)
|
||||||
|
text.encode("utf-8")
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
unittest.main()
|
unittest.main()
|
||||||
|
|||||||
Reference in New Issue
Block a user