diff --git a/artifacts/views.py b/artifacts/views.py index e7650f9..3e47865 100644 --- a/artifacts/views.py +++ b/artifacts/views.py @@ -11,6 +11,7 @@ from pathlib import Path from urllib.parse import urlencode import markdown +import nh3 import openpyxl from django.conf import settings from django.contrib import messages @@ -536,7 +537,18 @@ def serve_artifact_file(request, artifact): raise Http404("Artifact file not found") content_type, _ = mimetypes.guess_type(path.name) - return FileResponse(path.open("rb"), content_type=content_type or "application/octet-stream") + response = FileResponse(path.open("rb"), content_type=content_type or "application/octet-stream") + # The sanitized kinds legitimately contain zero scripts, so a + # script-blocking CSP is both safe and strong here — and because it's a + # response header it also neutralizes any script in artifacts stored + # *before* create-time sanitization existed. Not applied to html_full + # (poster's own scripts by contract) or json/csv (their viewers need JS; + # their user data is separately escaped). + if artifact.kind in SANITIZED_KINDS: + response["Content-Security-Policy"] = ( + "default-src 'self'; script-src 'none'; object-src 'none'; base-uri 'none'" + ) + return response def create_text_artifact(data, request, user): @@ -553,26 +565,24 @@ def create_text_artifact(data, request, user): target_dir.mkdir(parents=True, exist_ok=True) if kind == Artifact.HTML_FULL: + # Deliberately verbatim: html_full is the "poster owns everything, + # including risk" kind. Not sanitized. See issue #014. rendered = content elif kind == Artifact.MARKDOWN: - rendered = wrap_html( - markdown.markdown( - content, - extensions=["extra", "sane_lists", "codehilite"], - extension_configs={"codehilite": {"guess_lang": False}}, - ), - title, - theme, - request, + md_html = markdown.markdown( + content, + extensions=["extra", "sane_lists", "codehilite"], + extension_configs={"codehilite": {"guess_lang": False}}, ) + rendered = wrap_html(sanitize_html(md_html), title, theme, request) elif kind == Artifact.DECK: - rendered = wrap_deck(content, title, request) + rendered = wrap_deck(sanitize_html(content), title, request) elif kind == Artifact.JSON: rendered = wrap_json_viewer(content, title, request) elif kind == Artifact.CSV: rendered = wrap_csv_viewer(content, title, request) - else: - rendered = wrap_html(content, title, theme, request) + else: # html_fragment + rendered = wrap_html(sanitize_html(content), title, theme, request) sidecar_suffix = SIDECAR_SUFFIXES.get(kind) if sidecar_suffix: @@ -728,6 +738,47 @@ def create_binary_artifact(data, request, user): return artifact +# Allowlist for sanitizing the "themed" text kinds (markdown, deck, +# html_fragment) — kinds where Daybook, not the poster, owns presentation, so +# they must never be a script-execution vector (issue #014). html_full is the +# deliberate exception (verbatim, poster owns everything) and is NOT run +# through this. The list keeps ordinary formatting + tables + code-highlight +# markup and drops everything else (script, on* handlers, style, unknown tags). +SANITIZE_ALLOWED_TAGS = { + "a", "abbr", "b", "blockquote", "br", "caption", "code", "col", "colgroup", + "dd", "del", "details", "div", "dl", "dt", "em", "figcaption", "figure", + "h1", "h2", "h3", "h4", "h5", "h6", "hr", "i", "img", "ins", "kbd", "li", + "mark", "ol", "p", "pre", "s", "section", "small", "span", "strong", "sub", + "summary", "sup", "table", "tbody", "td", "tfoot", "th", "thead", "tr", + "u", "ul", +} +SANITIZE_ALLOWED_ATTRS = { + "*": {"class", "id", "title"}, + "a": {"href", "title"}, # nh3 adds rel="noopener noreferrer" itself + "img": {"src", "alt", "title", "width", "height"}, + "td": {"colspan", "rowspan"}, + "th": {"colspan", "rowspan", "scope"}, + "col": {"span"}, + "colgroup": {"span"}, +} +# Kinds whose stored HTML is sanitized at create time and served with a +# script-blocking CSP (they legitimately contain no JS of their own). +SANITIZED_KINDS = {Artifact.MARKDOWN, Artifact.DECK, Artifact.HTML_FRAGMENT} + + +def sanitize_html(body): + """Allowlist-sanitize author-influenced HTML. nh3 parses to a DOM and keeps + only known-safe nodes (never a blocklist), and re-serializes well-formed + HTML as a side effect. javascript:/data: URLs are dropped by restricting + url_schemes; relative URLs (e.g. /assets/ images) pass through.""" + return nh3.clean( + body, + tags=SANITIZE_ALLOWED_TAGS, + attributes=SANITIZE_ALLOWED_ATTRS, + url_schemes={"http", "https", "mailto"}, + ) + + def wrap_html(body, title, theme, request): stylesheet = request.build_absolute_uri(f"/static/themes/{theme}.css") pygments_stylesheet = request.build_absolute_uri("/static/themes/pygments.css") diff --git a/docs/api.md b/docs/api.md index 362300e..b412aba 100644 --- a/docs/api.md +++ b/docs/api.md @@ -28,10 +28,10 @@ Currently supported kinds: | Kind | Upload type | Rendered behavior | Raw support | |---|---|---|---| -| `html_fragment` | JSON text | Wrapped with a Daybook theme; tables styled automatically | No | -| `html_full` | JSON text | Stored as complete HTML, verbatim — no theme or styling injected | Yes | -| `markdown` | JSON text | Rendered to HTML (code blocks syntax-highlighted, tables styled) and original `.md` preserved | Yes | -| `deck` | JSON text | Wrapped as a simple HTML slide/deck page | No | +| `html_fragment` | JSON text | Wrapped with a Daybook theme; tables styled automatically; **HTML sanitized** | No | +| `html_full` | JSON text | Stored as complete HTML, verbatim — no theme, styling, or sanitization | Yes | +| `markdown` | JSON text | Rendered to HTML (code blocks syntax-highlighted, tables styled), **sanitized**, original `.md` preserved | Yes | +| `deck` | JSON text | Wrapped as a simple HTML slide/deck page; **HTML sanitized** | No | | `image` | Multipart file | Browser image preview | No | | `pdf` | Multipart file | Browser PDF preview | No | | `binary` | Multipart file | Stored and downloadable | No | @@ -42,6 +42,22 @@ Currently supported kinds: though the value itself is JSON/CSV text — i.e. `{"kind": "json", "content": "{\"a\": 1}"}`, not a nested JSON object in the request body. +### HTML sanitization + +The "themed" kinds — `markdown`, `html_fragment`, `deck` — have their +rendered HTML run through an allowlist sanitizer (nh3) before storage. +Ordinary formatting, tables, links, images (including relative +`/assets/` URLs), and code-highlight markup are preserved; `\n\n\n\n[x](javascript:alert(1))", + ).data["id"] + html = self._stored_html(aid).lower() + self.assertNotIn("", html) + self.assertIn('class="codehilite"', html) + self.assertIn('def', html) + self.assertIn("/assets/ab12cd34ef56", html) # relative image URL survives + + def test_html_full_is_not_sanitized(self): + aid = self.create_artifact( + kind="html_full", + content="", + ).data["id"] + # html_full is the deliberate verbatim escape hatch — its script stays. + self.assertIn("", self._stored_html(aid)) + + def test_sanitized_kinds_served_with_script_blocking_csp(self): + aid = self.create_artifact(kind="markdown", content="# hi").data["id"] + self.client.force_login(self.user) + response = self.client.get(reverse("artifact", args=[aid])) + self.assertIn("script-src 'none'", response.headers.get("Content-Security-Policy", "")) + + def test_html_full_served_without_restrictive_csp(self): + aid = self.create_artifact( + kind="html_full", content="x" + ).data["id"] + self.client.force_login(self.user) + response = self.client.get(reverse("artifact", args=[aid])) + self.assertIsNone(response.headers.get("Content-Security-Policy")) +