From 7c4dbf2cd97c4f4d043d101c70bb77edcbad947b Mon Sep 17 00:00:00 2001
From: Johan Lundberg <lundberg@sunet.se>
Date: Thu, 4 Jun 2026 11:06:08 +0200
Subject: [PATCH] fix(security): escape error text in OIDC error pages

OIDC error responses interpolated parse-error/exception and error_description
text straight into HTML. idpyoidc currently emits canned messages, but this is
the same reflected-XSS class as the validation-error fix; relying on upstream
not to echo input is fragile.

Add a shared _error_page() helper that HTML-escapes the message and route all
six dynamic error responses through it.

Refs: porchlight-8iw

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 src/porchlight/oidc/endpoints.py      | 22 ++++++++++++++++------
 tests/test_oidc/test_authorization.py | 10 ++++++++++
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/src/porchlight/oidc/endpoints.py b/src/porchlight/oidc/endpoints.py
index 8bfca41..12af079 100644
--- a/src/porchlight/oidc/endpoints.py
+++ b/src/porchlight/oidc/endpoints.py
@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+import html
 import json
 from types import SimpleNamespace
 from urllib.parse import urlencode
@@ -14,6 +15,15 @@ from porchlight.oidc.claims import PorchlightUserInfo, user_to_claims
 
 router = APIRouter(tags=["oidc"])
 
+
+def _error_page(message: object, status_code: int = 400, title: str = "Error") -> HTMLResponse:
+    """Render an error page, escaping the (possibly request-derived) message."""
+    return HTMLResponse(
+        f"<h1>{html.escape(title)}</h1><p>{html.escape(str(message))}</p>",
+        status_code=status_code,
+    )
+
+
 SCOPE_DESCRIPTIONS: dict[str, str] = {
     "openid": "Sign you in (required)",
     "profile": "Your name and profile information",
@@ -59,11 +69,11 @@ async def authorization(request: Request) -> Response:
     try:
         parsed = endpoint.parse_request(query_params)
     except Exception as exc:
-        return HTMLResponse(f"<h1>Invalid Request</h1><p>{exc}</p>", status_code=400)
+        return _error_page(exc, title="Invalid Request")
 
     if "error" in parsed:
         error_desc = parsed.get("error_description", parsed["error"])
-        return HTMLResponse(f"<h1>Error</h1><p>{error_desc}</p>", status_code=400)
+        return _error_page(error_desc)
 
     # Check if user is authenticated
     userid = request.session.get("userid")
@@ -95,11 +105,11 @@ async def authorization_complete(request: Request) -> Response:
     try:
         parsed = endpoint.parse_request(auth_request_params)
     except Exception as exc:
-        return HTMLResponse(f"<h1>Error</h1><p>{exc}</p>", status_code=400)
+        return _error_page(exc)
 
     if "error" in parsed:
         error_desc = parsed.get("error_description", parsed["error"])
-        return HTMLResponse(f"<h1>Error</h1><p>{error_desc}</p>", status_code=400)
+        return _error_page(error_desc)
 
     return await _check_consent_or_complete(
         request, oidc_server, endpoint, parsed, userid, username, auth_request_params
@@ -172,7 +182,7 @@ async def _complete_authorization(  # noqa: PLR0913
     if "error" in result.get("response_args", {}):
         response_args = result["response_args"]
         error_desc = response_args.get("error_description", response_args["error"])
-        return HTMLResponse(f"<h1>Error</h1><p>{error_desc}</p>", status_code=400)
+        return _error_page(error_desc)
 
     # Build redirect URL
     response_args = result.get("response_args", {})
@@ -361,6 +371,6 @@ async def consent_submit(request: Request) -> Response:
         if "error" in parsed:
             raise ValueError(parsed.get("error_description", parsed["error"]))
     except Exception as exc:
-        return HTMLResponse(f"<h1>Error</h1><p>{exc}</p>", status_code=400)
+        return _error_page(exc)
 
     return await _complete_authorization(request, oidc_server, endpoint, parsed, userid, username)
diff --git a/tests/test_oidc/test_authorization.py b/tests/test_oidc/test_authorization.py
index 5994134..de3c70b 100644
--- a/tests/test_oidc/test_authorization.py
+++ b/tests/test_oidc/test_authorization.py
@@ -2,6 +2,16 @@ import secrets
 
 from httpx import AsyncClient
 
+from porchlight.oidc.endpoints import _error_page
+
+
+def test_error_page_escapes_html() -> None:
+    # OIDC error pages must not interpolate request-derived text as raw HTML.
+    resp = _error_page("<script>alert(1)</script>")
+    body = resp.body.decode()
+    assert "<script>" not in body
+    assert "&lt;script&gt;alert(1)&lt;/script&gt;" in body
+
 
 def _register_test_client(
     client: AsyncClient,