Fix Trust Center review findings (XSS, header injection, IP, gated link)

A max-effort self-review of the Trust Center surfaced several issues, fixed here:

- Stored XSS via custom CSS: clean_css() did a single regex pass, defeated by
  split-token recreation (e.g. `</sty</stylele>` -> `</style>`). Custom CSS is
  now served from a dedicated text/css endpoint (/trust/custom.css) via <link>,
  so there is no HTML context to break out of; clean_css() also iterates to a
  fixed point as defence in depth. Drops the now-unused safe_css template filter.
- HTTP 500 on the public request endpoint: the client-controlled X-Forwarded-For
  was stored into a GenericIPAddressField unvalidated, crashing on the Postgres
  inet column. _client_ip() now validates via ipaddress and returns None on bad
  input.
- Content-Disposition header injection: the download filename was interpolated
  unescaped; a double quote broke the header and CRLF caused a 500. Both public
  and gated downloads now use a sanitized, RFC 5987-encoded disposition.
- Gated link outliving takedown: the gated download re-checks the document is
  still published(), so unpublishing/archiving revokes outstanding approved links.
- Approval now warns when the download-link email could not be sent instead of
  always reporting success.
- Removes the unused "trust_request" throttle rate and its misleading comment.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: claude

core/settings.py

@@ -215,12 +215,11 @@ def _detect_version():
     "DEFAULT_RENDERER_CLASSES": [
         "context.api.renderers.StandardJSONRenderer",
     ],
-    # Throttling is opt-in per view (public Trust Center endpoints only); the
-    # authenticated app is not throttled. Rates are consumed by AnonRateThrottle
-    # (public reads) and the "trust_request" ScopedRateThrottle (gated requests).
+    # Throttling is opt-in per view (public Trust Center read endpoints only,
+    # via AnonRateThrottle); the authenticated app is not throttled. The public
+    # gated-request form is a Django view rate-limited separately in its view.
     "DEFAULT_THROTTLE_RATES": {
         "anon": "120/hour",
-        "trust_request": "5/hour",
     },
 }
 

trust_center/admin_views.py

@@ -302,8 +302,13 @@ def post(self, request, pk):
         if target == DocumentRequestState.APPROVED:
             token = obj.issue_download_link(dj_settings.TRUST_CENTER_DOWNLOAD_TTL)
             url = reverse("trust_center:gated-download", kwargs={"token": token})
-            send_gated_link_email(obj, url)
-            messages.success(request, _("Request approved and a download link was emailed."))
+            if send_gated_link_email(obj, url):
+                messages.success(request, _("Request approved and a download link was emailed."))
+            else:
+                messages.warning(
+                    request,
+                    _("Request approved, but the download link email could not be sent."),
+                )
         else:
             messages.success(request, _("Request updated."))
         return redirect("trust_center_manage:request-detail", pk=pk)

trust_center/sanitizers.py

@@ -371,10 +371,21 @@ def clean_html(markup: str) -> str:
 
 
 def clean_css(css: str) -> str:
-    """Strip style-tag breakout and active-content constructs from custom CSS."""
+    """Strip active-content / breakout constructs from custom CSS.
+
+    Applied to a fixed point so a split-token payload (e.g. ``<scr<script>ipt>``)
+    cannot reconstruct a stripped token after a single pass. Custom CSS is served
+    from a dedicated ``text/css`` endpoint (not inlined in a ``<style>``), so this
+    is defence in depth rather than the sole boundary.
+    """
     if not css:
         return ""
-    return _CSS_DANGEROUS.sub("", css)
+    previous = None
+    out = css
+    while previous != out:
+        previous = out
+        out = _CSS_DANGEROUS.sub("", out)
+    return out
 
 
 # --- Logo rendering ---------------------------------------------------------

trust_center/templates/trust_center/public_base.html

@@ -125,7 +125,7 @@
     .tc-doc-icon { color: var(--accent); font-size: 1.15rem; flex-shrink: 0; }
     .badge-soft { background: var(--accent-soft); color: var(--accent); font-weight: 600; }
   </style>
-  {% if tc.custom_css %}<style>{{ tc.custom_css|safe_css }}</style>{% endif %}
+  {% if tc.custom_css %}<link rel="stylesheet" href="{% url 'trust_center:custom-css' %}">{% endif %}
   {% block extra_head %}{% endblock %}
 </head>
 <body>

trust_center/templatetags/trust_center_tags.py

@@ -2,7 +2,6 @@
 from django.utils.safestring import mark_safe
 
 from trust_center.sanitizers import (
-    clean_css,
     clean_html,
     clean_svg,
     logo_href,
@@ -39,9 +38,3 @@ def safe_svg(markup):
 def safe_html(markup):
     """Sanitize admin-authored rich text (Jodit) for safe public rendering."""
     return mark_safe(clean_html(markup or ""))  # noqa: S308 - sanitized by clean_html
-
-
-@register.filter(name="safe_css")
-def safe_css(css):
-    """Sanitize operator-supplied custom CSS for inline <style> injection."""
-    return mark_safe(clean_css(css or ""))  # noqa: S308 - sanitized by clean_css

trust_center/tests/test_branding.py

@@ -115,11 +115,15 @@ def test_landing_shows_document_type_icon():
 # --- Custom CSS -------------------------------------------------------------
 
 
-def test_custom_css_injected_and_sanitized():
-    _enable(custom_css="body { background: rebeccapurple; } </style><script>alert(1)</script>")
-    content = Client().get("/trust/").content
-    assert b"rebeccapurple" in content  # custom CSS present
-    assert b"<script>alert(1)" not in content  # breakout neutralized
+def test_custom_css_served_from_stylesheet_endpoint():
+    _enable(custom_css="body { background: rebeccapurple; }")
+    page = Client().get("/trust/").content.decode()
+    assert "/trust/custom.css" in page  # linked, not inlined
+    assert "<style>body { background: rebeccapurple" not in page
+    css = Client().get("/trust/custom.css")
+    assert css.status_code == 200
+    assert css["Content-Type"].startswith("text/css")
+    assert b"rebeccapurple" in css.content
 
 
 def test_clean_css_strips_dangerous_constructs():
@@ -133,6 +137,43 @@ def test_clean_css_strips_dangerous_constructs():
     assert "a{}" in out  # benign CSS preserved
 
 
+def test_clean_css_defeats_split_token_recreation():
+    out = clean_css("</sty</stylele><scr<scriptipt>x</scr<scriptipt>@imp@importort")
+    lowered = out.lower()
+    assert "</style" not in lowered
+    assert "<script" not in lowered
+    assert "@import" not in lowered
+
+
+def test_client_ip_validates_x_forwarded_for():
+    from django.test import RequestFactory
+
+    from trust_center.views import _client_ip
+
+    rf = RequestFactory()
+    assert _client_ip(rf.get("/", HTTP_X_FORWARDED_FOR="not-an-ip")) is None
+    assert _client_ip(rf.get("/", HTTP_X_FORWARDED_FOR="203.0.113.7")) == "203.0.113.7"
+
+
+def test_public_download_filename_header_is_safe():
+    from trust_center.tests.factories import TrustCenterDocumentFactory, publish
+
+    _enable()
+    doc = publish(TrustCenterDocumentFactory(file_name='evil".pdf'))
+    resp = Client().get(f"/trust/documents/{doc.id}/download/")
+    assert resp.status_code == 200
+    assert 'filename="evil.pdf"' in resp["Content-Disposition"]  # quote stripped
+
+
+def test_public_download_filename_crlf_does_not_500():
+    from trust_center.tests.factories import TrustCenterDocumentFactory, publish
+
+    _enable()
+    doc = publish(TrustCenterDocumentFactory(file_name="a\r\nb.pdf"))
+    resp = Client().get(f"/trust/documents/{doc.id}/download/")
+    assert resp.status_code == 200
+
+
 # --- CSS upload -------------------------------------------------------------
 
 

trust_center/tests/test_gated.py

@@ -145,6 +145,18 @@ def test_download_rejects_expired_token():
     assert resp.status_code == 410
 
 
+def test_download_blocked_after_document_unpublished():
+    _enable()
+    req = DocumentRequestFactory(workflow_state=DocumentRequestState.APPROVED)
+    publish(req.document)
+    token = req.make_download_token()
+    assert Client().get(f"/trust/documents/download/{token}/").status_code == 200
+    # Taking the document down revokes outstanding approved links too.
+    req.document.workflow_state = "draft"
+    req.document.save()
+    assert Client().get(f"/trust/documents/download/{token}/").status_code == 404
+
+
 def test_download_rejected_after_revoke():
     _enable()
     req = DocumentRequestFactory(workflow_state=DocumentRequestState.APPROVED)

trust_center/urls.py

@@ -6,6 +6,7 @@
 
 urlpatterns = [
     path("", views.TrustCenterLandingView.as_view(), name="landing"),
+    path("custom.css", views.TrustCenterCustomCSSView.as_view(), name="custom-css"),
     path(
         "documents/<uuid:pk>/download/",
         views.TrustCenterPublicDocumentDownloadView.as_view(),

trust_center/views.py

@@ -5,6 +5,9 @@
 global ``TrustCenterSettings.is_published`` kill switch 404s everything when off.
 """
 
+import ipaddress
+from urllib.parse import quote
+
 from django.conf import settings
 from django.core import signing
 from django.core.cache import cache
@@ -28,13 +31,30 @@
     TrustCenterSettings,
     TrustCenterSubprocessor,
 )
+from trust_center.sanitizers import clean_css
 
 
 def _client_ip(request):
-    xff = request.META.get("HTTP_X_FORWARDED_FOR")
-    if xff:
-        return xff.split(",")[0].strip()
-    return request.META.get("REMOTE_ADDR")
+    """Return a valid client IP string, or None.
+
+    X-Forwarded-For is client-controlled, so the value is validated before it
+    reaches the GenericIPAddressField (an ``inet`` column on PostgreSQL rejects
+    a malformed value with a hard error).
+    """
+    xff = request.META.get("HTTP_X_FORWARDED_FOR", "")
+    candidate = (xff.split(",")[0].strip() if xff else "") or request.META.get("REMOTE_ADDR")
+    if not candidate:
+        return None
+    try:
+        return str(ipaddress.ip_address(candidate))
+    except ValueError:
+        return None
+
+
+def _attachment_disposition(filename):
+    """Build a safe Content-Disposition value (no header injection / breakout)."""
+    safe = "".join(ch for ch in (filename or "") if ch not in '"\\\r\n').strip() or "document"
+    return f"attachment; filename=\"{safe}\"; filename*=UTF-8''{quote(safe)}"
 
 
 def _rate_ok(key, limit, window):
@@ -84,11 +104,22 @@ def get(self, request, pk):
         resp = HttpResponse(
             data, content_type=doc.content_type or "application/octet-stream"
         )
-        filename = doc.effective_file_name or "document"
-        resp["Content-Disposition"] = f'attachment; filename="{filename}"'
+        resp["Content-Disposition"] = _attachment_disposition(doc.effective_file_name)
         return resp
 
 
+class TrustCenterCustomCSSView(View):
+    """Serve the operator-supplied custom CSS as a standalone stylesheet.
+
+    Served with ``text/css`` (not inlined in a <style> element) so there is no
+    HTML context to break out of; ``clean_css`` is applied as defence in depth.
+    """
+
+    def get(self, request):
+        css = clean_css(TrustCenterSettings.get().custom_css or "")
+        return HttpResponse(css, content_type="text/css; charset=utf-8")
+
+
 class DocumentRequestCreateView(FormView):
     """Public request form for a GATED document (no authentication)."""
 
@@ -183,6 +214,11 @@ def get(self, request, token):
         if req is None or not req.is_granted:
             raise Http404()
 
+        # The document must still be published (an admin unpublishing or
+        # archiving it revokes outstanding approved links too).
+        if not TrustCenterDocument.objects.published().filter(pk=req.document_id).exists():
+            raise Http404()
+
         data = req.document.get_file_bytes()
         if not data:
             raise Http404()
@@ -193,6 +229,5 @@ def get(self, request, token):
         resp = HttpResponse(
             data, content_type=req.document.content_type or "application/octet-stream"
         )
-        filename = req.document.effective_file_name or "document"
-        resp["Content-Disposition"] = f'attachment; filename="{filename}"'
+        resp["Content-Disposition"] = _attachment_disposition(req.document.effective_file_name)
         return resp