diff --git a/tests/test_build_review_report.py b/tests/test_build_review_report.py index 7fa47f5..089be48 100644 --- a/tests/test_build_review_report.py +++ b/tests/test_build_review_report.py @@ -12,6 +12,10 @@ EM_DASH = chr(0x2014) ZERO_WIDTH_SPACE = chr(0x200B) INJECTION = "" +# Breaks out of an HTML attribute without relying on angle brackets. +ATTRIBUTE_BREAKER = '" onmouseover="alert(1)' +ESCAPED_ATTRIBUTE_BREAKER = "" onmouseover="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}

" 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'
' 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'

' in page + + +def test_render_html_escapes_change_summary_files_changed(): + hostile_count = '' + payload = _payload( + change_summary={ + "files_changed": hostile_count, + "additions": 12, + "deletions": 3, + } + ) + + page = render_html(payload, PACKET_PATH) + + assert "" + "<img src=x onerror="alert(1)">" + ) in page + + +def test_ordinary_severity_status_and_counts_render_unchanged(): + page = render_html(_payload(decision=APPROVED_DECISION), PACKET_PATH) + + assert '

' in page + assert '

' in page + assert "Changed files1" in page diff --git a/triage_core/build_review_report.py b/triage_core/build_review_report.py index 52d584a..360031a 100644 --- a/triage_core/build_review_report.py +++ b/triage_core/build_review_report.py @@ -185,7 +185,7 @@ def render_html( for item in payload["changed_files"] ) or 'No changed files.' findings = "".join( - f'

' + f'
' f"

{_escape(item['severity'].upper())} " f"{_escape(item['code'])}

" f"

{_escape(item['title'])}

" @@ -219,7 +219,7 @@ def render_html( ) else: decision_view = ( - f'

' + f'

' f"{_escape(decision['status'].upper())}
" f"Reviewer: {_escape(decision['reviewer'])}
" f"Time: {_escape(decision['decided_at'])}
" @@ -293,7 +293,7 @@ def render_html(

Recommendation{_escape(payload["recommendation"])}
-
Changed files{summary["files_changed"]}
+
Changed files{_escape(summary["files_changed"])}
Decision{_escape(decision["status"])}