Harden build-review HTML interpolation escaping

tests/test_build_review_report.py

@@ -12,6 +12,10 @@
 EM_DASH = chr(0x2014)
 ZERO_WIDTH_SPACE = chr(0x200B)
 INJECTION = "<script>alert('xss')</script>"
+# Breaks out of an HTML attribute without relying on angle brackets.
+ATTRIBUTE_BREAKER = '" onmouseover="alert(1)'
+ESCAPED_ATTRIBUTE_BREAKER = "&quot; onmouseover=&quot;alert(1)"
+RAW_ATTRIBUTE_ESCAPE = '" onmouseover="'
 PACKET_PATH = Path("packets/review-42.json")
 
 APPROVED_DECISION = {
@@ -242,3 +246,61 @@ def test_completed_decision_without_note_renders_em_dash():
 
     assert f"- Note: {EM_DASH}" in render_markdown(payload, PACKET_PATH)
     assert f"Note: {EM_DASH}</p>" in render_html(payload, PACKET_PATH)
+
+
+def test_render_html_escapes_finding_severity_in_class_attribute():
+    payload = _payload(
+        findings=[
+            {
+                "code": "RISK",
+                "severity": ATTRIBUTE_BREAKER,
+                "title": "Hostile severity",
+                "detail": "Severity reaches a class attribute.",
+                "files": [],
+            }
+        ]
+    )
+
+    page = render_html(payload, PACKET_PATH)
+
+    assert RAW_ATTRIBUTE_ESCAPE not in page
+    assert "onmouseover=" not in page.replace(ESCAPED_ATTRIBUTE_BREAKER, "")
+    assert f'<article class="finding {ESCAPED_ATTRIBUTE_BREAKER}">' in page
+
+
+def test_render_html_escapes_completed_decision_status_in_class_attribute():
+    payload = _payload(decision={**APPROVED_DECISION, "status": ATTRIBUTE_BREAKER})
+
+    page = render_html(payload, PACKET_PATH)
+
+    assert RAW_ATTRIBUTE_ESCAPE not in page
+    assert "onmouseover=" not in page.replace(ESCAPED_ATTRIBUTE_BREAKER, "")
+    assert f'<p class="decision {ESCAPED_ATTRIBUTE_BREAKER}">' in page
+
+
+def test_render_html_escapes_change_summary_files_changed():
+    hostile_count = '<img src=x onerror="alert(1)">'
+    payload = _payload(
+        change_summary={
+            "files_changed": hostile_count,
+            "additions": 12,
+            "deletions": 3,
+        }
+    )
+
+    page = render_html(payload, PACKET_PATH)
+
+    assert "<img" not in page
+    assert 'onerror="alert(1)"' not in page
+    assert (
+        "Changed files<strong>"
+        "&lt;img src=x onerror=&quot;alert(1)&quot;&gt;</strong>"
+    ) in page
+
+
+def test_ordinary_severity_status_and_counts_render_unchanged():
+    page = render_html(_payload(decision=APPROVED_DECISION), PACKET_PATH)
+
+    assert '<article class="finding low">' in page
+    assert '<p class="decision approved">' in page
+    assert "Changed files<strong>1</strong>" in page

triage_core/build_review_report.py

@@ -185,7 +185,7 @@ def render_html(
         for item in payload["changed_files"]
     ) or '<tr><td colspan="4">No changed files.</td></tr>'
     findings = "".join(
-        f'<article class="finding {item["severity"]}">'
+        f'<article class="finding {_escape(item["severity"])}">'
         f"<p><strong>{_escape(item['severity'].upper())}</strong> "
         f"<code>{_escape(item['code'])}</code></p>"
         f"<h3>{_escape(item['title'])}</h3>"
@@ -219,7 +219,7 @@ def render_html(
         )
     else:
         decision_view = (
-            f'<p class="decision {decision["status"]}">'
+            f'<p class="decision {_escape(decision["status"])}">'
             f"<strong>{_escape(decision['status'].upper())}</strong><br>"
             f"Reviewer: {_escape(decision['reviewer'])}<br>"
             f"Time: {_escape(decision['decided_at'])}<br>"
@@ -293,7 +293,7 @@ def render_html(
     </header>
     <section class="metrics" aria-label="Review summary">
       <div class="metric">Recommendation<strong>{_escape(payload["recommendation"])}</strong></div>
-      <div class="metric">Changed files<strong>{summary["files_changed"]}</strong></div>
+      <div class="metric">Changed files<strong>{_escape(summary["files_changed"])}</strong></div>
       <div class="metric">Decision<strong>{_escape(decision["status"])}</strong></div>
     </section>
     <section>