Escape user-supplied titles reaching fast_html attributes (#888)

[vjpixel]

Description

Brings this up to date with develop (it had been conflicted since April) and adds the tests it was missing.

The issue it closes, #888, has been rewritten: the original text described sanitising a CommentForm that does not exist in this repository. The concern behind it turned out to be real and reachable, and worse than the issue claimed — a stored XSS, now documented with the exact lines.

Resolves (Issues)

Closes #888

The vector, for the reviewer

fast_html escapes nothing:

>>> render(img(src="x", title='" onerror="alert(1)'))
'``<img src="x" title="" onerror="alert(1)">``'

title is a plain user-supplied form field, it goes straight into that attribute in as_html(), and the modal templates render the result with | safe. Upload a marker titled " onerror="alert(1) and it runs in the browser of anyone who opens that marker's modal.

What the merge changed about this PR

develop has since deleted Sound.as_html_thumbnail and Exhibit.as_html_thumbnail, so the escaping this PR originally added to them no longer has anywhere to live — those hunks take develop's deletion. The one surviving conflict was in Marker.as_html, where develop introduced a cache-busted src; both sides are kept.

Net diff against current develop is therefore small and entirely the fix: the escape import, escape() at the three as_html attribute sites, and the | safe dropped from the blog excerpt.

Tests added

The original PR had none, which is how this bug comes back. src/core/tests/test_html_escaping.py:

• a title of " onerror="alert(1) cannot break out of the attribute, on Marker, Object and Sound;
• a <script> tag in a title is neutralised;
• and a test asserting that fast_html still escapes nothing — so if that ever changes, someone finds out deliberately rather than discovering that the call-site escaping had quietly become redundant.

Removing the three escape() calls again makes 4 of the 5 fail.

Worth a follow-up, not done here

Escaping at each call site relies on every future as_html remembering to do it, and fast_html will keep not escaping. A single choke point returning SafeString would be sturdier — but that is a refactor, and this PR is a security fix that should merge on its own.

Verification: full suite pytest src/core src/users src/blog passes — 277 passed. ruff format --diff src/ clean, ruff check --extend-select I at 93 findings matching develop with zero I001, no pending migrations.

---

🤖 Generated with Claude Code

https://claude.ai/code/session_01HXUw7kyELDu8ycGQxtaFbb

---

Generated by Claude Code

[vjpixel]

Self-review

Verdict: ✅

fast_html does not escape attribute values or text content — verified directly:

>>> render(span('<script>alert(1)</script>'))
'<span><script>alert(1)</script></span>'
>>> render(img(title='<script>alert(1)</script>', src='x'))
'<img title="<script>alert(1)</script>" src="x">'

Combined with | safe on every as_html()/as_html_thumbnail() consumer in jinja2, this was a real attribute-injection / inline-script XSS waiting on someone with a malicious title.

Scope:

• Sound.as_html/as_html_thumbnail, Marker.as_html, Object.as_html, Exhibit.as_html_thumbnail — all user-controlled strings (title, name, slug, username) wrapped in escape().
• post.excerpt was rendered with | safe in post_preview.jinja2 despite being a plain TextField — | safe removed.
• post.formatted_body left alone — already sanitized by ProseEditor(sanitize=True).
• used_in_html_string() and other internal compositions left alone — no user input.

Verified: escape() produces <script>...</script> in both attribute and text positions, defusing the injection.

Unique: No duplicate PR.

---

Generated by Claude Code