Skip to content

fix(server): stop sanitize_input from double-escaping web-UI text - #785

Closed
Giorgio-Germani wants to merge 1 commit into
DRYTRIX:mainfrom
Giorgio-Germani:fix/sanitize-input-double-escape
Closed

Giorgio-Germani wants to merge 1 commit into
DRYTRIX:mainfrom
Giorgio-Germani:fix/sanitize-input-double-escape

Conversation

@Giorgio-Germani

Copy link
Copy Markdown

Problem

Every text written through a web-UI form that passes sanitize_input() is stored HTML-escaped in the DB: bleach.clean(value, tags=[], strip=True) removes tags but entity-escapes the surviving text (& → &amp;, < → &lt;). Jinja then auto-escapes again on output, so stored data was effectively double-escaped.

API-v1 writes (desktop, mobile, browser extension) never call sanitize_input(), which is why only web-created entries were affected — and raw API consumers displayed them with literal &amp; (e.g. task "Error Handling & Security Improvements", notes "Referenzen & Über uns …"). We hit this with real data on our self-hosted instance and traced it back to this function.

Fix

sanitize_input() now decodes entities once after stripping tags:

sanitized = html_lib.unescape(bleach.clean(value, tags=[], strip=True))
  • Tags are still removed (<script>alert(1)</script>hello → alert(1)hello — inert plain text; bleach keeps text between stripped tags).
  • XSS safety stays where it belongs: output encoding (Jinja auto-escape; the markdown and nl2br template filters escape internally).
  • Audited all 40 |safe occurrences in app/templates/: every user-text path goes through | markdown | safe or | nl2br | safe, both escape internally — no template relied on pre-escaped storage.

Also included

  • tests/test_validation_sanitize.py — 6 regression tests (plain & preserved, tags stripped, entities typed as text decode to inert plain text, truncation intact).
  • scripts/fix_double_escaped_entities.py — optional one-off cleanup for existing rows: idempotent (html.unescape only where an entity pattern matches), dry-run by default, --apply to write. Covers the 7 models whose fields flow through sanitize_input() (tasks, time entries, clients, projects, issues, users, stock items). We ran it against our instance after taking a pg_dump backup: 5 rows fixed, re-run confirms no-op.

Testing

  • pytest tests/test_validation_sanitize.py (new, 6 pass)
  • task/route suites touching sanitized fields still pass

bleach.clean(tags=[], strip=True) entity-escapes surviving text
("&" -> "&amp;"), so every web-UI write through sanitize_input stored
double-escaped data (Jinja re-escapes on output). API writes were
unaffected, which is why only some entries were broken.

- decode entities once after stripping tags; XSS safety stays with
  output encoding (Jinja auto-escape, markdown/nl2br filters)
- verified no template relies on pre-escaped storage (no |safe on
  plain text fields; markdown/nl2br escape internally)
- add scripts/fix_double_escaped_entities.py: idempotent one-off
  cleanup for tasks, time_entries, clients, projects, issues, users,
  stock items (dry-run by default, --apply to write)
- unit tests covering the regression
@Giorgio-Germani
Giorgio-Germani deleted the fix/sanitize-input-double-escape branch October 16, 2026 14:47
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.

1 participant