Skip to content

Escape search UI content - #7

Merged
ruthdelalucha merged 1 commit into
mainfrom
codex/escape-search-html
Mar 28, 2026
Merged

ruthdelalucha merged 1 commit into
mainfrom
codex/escape-search-html

Conversation

@endvater

Copy link
Copy Markdown
Owner

Summary

  • escape user-provided and upstream yente values before rendering the /search HTML UI
  • escape the raw JSON preview so match data cannot inject markup into the page
  • add a regression test covering the reproduced script-injection payload

Testing

  • python3 -m pytest sanctions/tests -q
  • python3 -m ruff check sanctions/src/main.py sanctions/tests/test_main.py
  • reproduced q="><script>alert(1)</script> no longer appears raw in the HTML response

@jsg-claude jsg-claude left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #7 Review — Escape search UI content

Kurz: ✅ Wichtiger Security-Fix, sauber umgesetzt. Approve & Merge.


Was der PR fixt

Die /search HTML-Seite hat User-Input und yente-Daten ungefiltert in den HTML-Output gerendert — klassisches XSS-Risiko. Wer ?q="><script>alert(1)</script> eingibt, hätte JavaScript ausgeführt.

Der Fix ist vollständig:

Stelle Escaped
q Query-Parameter im ✅
Match-Name und ID in den Karten ✅
Property-Keys und -Values (birthPlace, notes etc.) ✅
Dataset-Badge Labels ✅
Fehlermeldungen ✅
Raw JSON im
-Block
✅

Umsetzung

html.escape(str(value), quote=True) ist der richtige Ansatz — quote=True escapt zusätzlich " und ', was für Attribute (value="...") zwingend nötig ist.

Die zentrale escape_text()-Hilfsfunktion vermeidet Wiederholungen und macht spätere Erweiterungen sicherer.


Test

Der Regression-Test prüft alle drei Vektoren explizit:

  • Script-Injection im Query-Parameter
  • HTML-Tags im Entitäts-Namen
  • Event-Handler in Property-Werten

Und verifiziert nicht nur dass der Raw-Payload fehlt, sondern auch dass die escaped Version vorhanden ist. Das ist der wichtige Unterschied.


Einzige Anmerkung

Der JSON-Block in <pre> wird jetzt escaped — das ist korrekt, da <pre> kein automatisches Escaping macht. Wer den JSON-Block für Clipboard-Copy nutzt, bekommt den escaped String. Das ist ein akzeptabler Trade-off für die Sicherheit; für eine spätere Version könnte man den Raw-JSON per JSON.stringify in ein <script>-Tag oder via data--Attribut übergeben. Für jetzt: ausreichend.

@ruthdelalucha
ruthdelalucha merged commit 8d144d3 into main Mar 28, 2026
3 checks passed

@ruthdelalucha ruthdelalucha left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: XSS Fix ✅ CRITICAL

APPROVED - Kritischer Security-Fix!

Schwachstelle behoben:

  • XSS via Query-Parameter: q="><script>alert(1)</script> wurde unescaped in HTML gerendert
  • XSS via yente-Daten: Match-Daten konnten JavaScript injizieren
  • JSON Preview: Raw JSON konnte HTML markup enthalten

Fixes:

  • ✅ html.escape() für alle User-Input- und Upstream-Daten
  • ✅ quote=True für Attribute-Werte (z.B. value="{safe_query}")
  • ✅ Sichere Rendering von: Query, Name, ID, Properties, Fehlermeldungen, JSON

Regression Test:

  • ✅ Expliziter Test mit XSS-Payload
  • ✅ Prüft auf escaped Entities (&lt;, &gt;, &quot;)
  • ✅ Verifiziert, dass raw HTML nicht durchschlägt

Bewertung:

Sprint-1-kritisch - UI ohne XSS-Schutz ist nicht produktionsbereit.

LGTM 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants