Media URL traversal check (integrations 0.2.1) + image alt text (ui 0.5.0)

CHANGELOG.md

@@ -7,6 +7,55 @@ Changelog ist die Upgrade-Anleitung für die Tools.
 
 ## [Unreleased]
 
+### basicbar-integrations (→ wird `integrations/v0.2.1`)
+
+**Sicherheitsfix:** `clean_media_url` prüfte den rohen String, sodass
+Pfade wie `/media/%2e%2e/api/whoami/` oder `/media/a\..\b` durchkamen —
+Browser lösen die enthaltenen `..`-Segmente nach dem Decodieren aber
+trotzdem auf, sodass ein admin-verfasstes `<img>` einen same-origin GET
+auf beliebige Pfade auslösen konnte (basicbar#6). Geprüft wird jetzt der
+decodierte, normalisierte Pfad; einfach- und doppelt-encodierte
+`..`-Segmente sowie Backslashes werden abgelehnt.
+
+**Fix Runde 2 (basicbar#6):** drei weitere Bypasses, die der WHATWG-URL-
+Parser aber ein reiner Substring-Check nicht sieht, sind jetzt ebenfalls
+abgedeckt: rohe oder `%09`/`%0a`/`%0d`-codierte C0-/DEL-Steuerzeichen
+(Browser entfernen sie überall im URL vor dem Auflösen, sodass z. B.
+`/media/.&#9;./api/` sonst als `/api/` aufgelöst würde); ein `?`/`#`
+(roh oder codiert), das ein `..` davor verbirgt (`/media/..?x` →
+`/?x`); und `url[:300]`-Truncation *nach* der Validierung, die einen
+validierten langen Pfad nachträglich wieder auf eine Traversal kürzen
+konnte — über 300 Zeichen wird jetzt komplett abgelehnt statt gekürzt.
+
+Migration: keine — reiner Bugfix, die öffentliche Signatur von
+`clean_media_url`/`clean_html` ändert sich nicht.
+
+### @basicbar/ui (→ wird `ui/v0.5.0`)
+
+**Alt-Text für Bilder in `RichTextEditor`** (basicbar#7, WCAG 1.1.1): direkt
+nach einem erfolgreichen Bild-Upload fragt der Editor per `window.prompt`
+nach einer Beschreibung (`t("Image description (alt text)")`) — leere
+Eingabe oder Abbrechen setzen beide `alt=""` (bewusst dekoratives Bild), das
+Bild wird in jedem Fall eingefügt. Neuer Toolbar-Button
+`t("Image description")` (nur sichtbar, wenn `onUploadImage` gesetzt ist),
+deaktiviert, solange kein Bild markiert ist; bei markiertem Bild öffnet er
+denselben Prompt vorausgefüllt mit dem aktuellen Alt-Text und übernimmt
+Änderungen per `updateAttributes("image", { alt })` — Abbrechen lässt den
+bestehenden Alt-Text unangetastet. `ToolbarButton` hat dafür eine neue
+optionale `disabled`-Prop (reduzierte Deckkraft, `aria-disabled`).
+
+Nebenbei behoben: `useEditor` setzt jetzt `shouldRerenderOnTransaction: true`
+— TipTap 3 rendert standardmäßig nicht mehr bei reinen Selektionsänderungen
+neu, sodass sämtliche Toolbar-Buttons (Fett/Kursiv/Überschriften/Link und
+jetzt auch der neue Bildbeschreibungs-Button), die ihren aktiven/deaktivierten
+Zustand aus `editor.isActive(...)` lesen, nach einem Klick ohne Dokument-
+änderung (z. B. Bild ab-/anwählen) den alten Stand zeigten, bis die nächste
+Bearbeitung ein Re-Render auslöste.
+
+Migration: keine für Tools ohne `onUploadImage` — additiv. Tools mit
+Bild-Upload ergänzen die neuen Übersetzungs-Keys
+`"Image description (alt text)"` und `"Image description"`.
+
 ### @basicbar/ui (→ wird `ui/v0.4.0`)
 
 **`RichTextEditor` + `RichText`** (modulierbar#5), aus AbstimmBAR verschoben:

packages/django/basicbar-integrations/basicbar_integrations/html_sanitize.py

@@ -10,6 +10,9 @@
 point anywhere (rel="noopener" is forced); images must come from the app's
 own /media/ storage.
 """
+import posixpath
+from urllib.parse import unquote
+
 import nh3
 
 ALLOWED_TAGS = {
@@ -20,13 +23,50 @@
 ALLOWED_URL_SCHEMES = {"http", "https", "mailto"}
 
 
+_CONTROL_CHARS = frozenset(chr(c) for c in range(0x20)) | {"\x7f"}  # C0 + DEL
+
+
+def _has_control_chars(value):
+    return any(c in _CONTROL_CHARS for c in value)
+
+
 def clean_media_url(url):
     """Accept only the app's own media storage: a relative ``/media/…`` path
-    without traversal — anything else becomes ""."""
-    url = (url or "").strip()
-    if url.startswith("/media/") and ".." not in url and not url.startswith("//"):
-        return url[:300]
-    return ""
+    whose decoded, normalised form stays inside ``/media/`` — anything else
+    becomes "". Percent-encoded dot segments (``%2e%2e``), double encoding
+    and backslashes are rejected (basicbar#6), as are C0/DEL control
+    characters (e.g. a raw or ``%09``-encoded tab — browsers strip these
+    anywhere in a URL before resolving it, so ``normpath`` must never see
+    them), a ``?``/``#`` query or fragment (raw or encoded — a trailing
+    ``..`` hidden after one is still resolved by the browser; query strings
+    on media URLs aren't supported), and anything over 300 characters
+    (truncating *after* validation could cut a long, validated path back
+    down to a traversal)."""
+    url = url or ""
+    if len(url) > 300:
+        return ""
+    if (
+        not url.startswith("/media/")
+        or url.startswith("//")
+        or "\\" in url
+        or "?" in url
+        or "#" in url
+        or _has_control_chars(url)
+    ):
+        return ""
+    decoded = unquote(url)
+    if (
+        "\\" in decoded
+        or "%" in decoded
+        or "?" in decoded
+        or "#" in decoded
+        or _has_control_chars(decoded)
+    ):
+        return ""
+    normalised = posixpath.normpath(decoded)
+    if normalised != "/media" and not normalised.startswith("/media/"):
+        return ""
+    return url
 
 
 def _attribute_filter(tag, attr, value):

packages/django/basicbar-integrations/basicbar_integrations/tests/test_html_sanitize.py

@@ -42,3 +42,56 @@ def test_clean_media_url_accepts_only_local_media(self):
         self.assertEqual(clean_media_url("https://evil/x.png"), "")
         self.assertEqual(clean_media_url("/media/../etc/passwd"), "")
         self.assertEqual(clean_media_url("//evil/x.png"), "")
+
+    def test_clean_media_url_rejects_encoded_path_traversal(self):
+        for url in (
+            "/media/%2e%2e/api/whoami/",
+            "/media/.%2E/x",
+            "/media/%2E%2E/x",
+            "/media/%252e%252e/x",
+            "/media/..%2fapi",
+            "/media/a\\..\\b",
+            "/media/a/../../api",
+            "//evil.example/media/x",
+            "https://evil.example/media/x",
+        ):
+            self.assertEqual(clean_media_url(url), "", url)
+
+    def test_clean_media_url_keeps_valid_paths(self):
+        self.assertEqual(clean_media_url("/media/rich/abc.png"), "/media/rich/abc.png")
+        self.assertEqual(
+            clean_media_url("/media/products/Kuche2.jpeg"), "/media/products/Kuche2.jpeg"
+        )
+        self.assertEqual(clean_media_url("/media/rich/a%20b.png"), "/media/rich/a%20b.png")
+        self.assertEqual(clean_media_url("/media/rich/gr%C3%BC%C3%9F.png"), "/media/rich/gr%C3%BC%C3%9F.png")
+
+    def test_clean_html_drops_img_src_with_encoded_traversal(self):
+        self.assertNotIn("api", clean_html('<img src="/media/%2e%2e/api/">'))
+
+    def test_clean_media_url_rejects_control_characters(self):
+        for url in (
+            "/media/.\t./api/",
+            "/media/.\n./api/",
+            "/media/%2e\r%2e/api/",
+            "/media/%09",
+        ):
+            self.assertEqual(clean_media_url(url), "", repr(url))
+
+    def test_clean_media_url_rejects_query_or_fragment(self):
+        for url in (
+            "/media/..?x",
+            "/media/.%2e?x",
+            "/media/%2e%2e#f",
+            "/media/rich/a.png?v=1",
+        ):
+            self.assertEqual(clean_media_url(url), "", url)
+
+    def test_clean_media_url_rejects_over_length_url_instead_of_truncating(self):
+        # Validated-then-truncated would cut this back down to ".../../..zzz"
+        # which normalises outside /media/ — must be rejected outright.
+        url = "/media/" + "a" * 287 + "/../..zzz"
+        self.assertEqual(len(url), 303)
+        self.assertEqual(clean_media_url(url), "")
+
+    def test_clean_html_drops_img_src_with_encoded_control_char(self):
+        self.assertNotIn("api", clean_html('<img src="/media/.&#9;./api/whoami/">'))

packages/django/basicbar-integrations/pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "basicbar-integrations"
-version = "0.2.0"
+version = "0.2.1"
 description = "Optionale Integrationen der virtUOS -bar-Tools: LibreTranslate, LiteLLM, Capabilities-Endpoint"
 readme = "README.md"
 requires-python = ">=3.12"

packages/ui/README.md

@@ -108,6 +108,16 @@ URL als String; scheitert der Upload, zeigt der Editor
 />
 ```
 
+**Bildbeschreibung (Alt-Text, WCAG 1.1.1):** direkt nach einem erfolgreichen
+Upload fragt der Editor per `window.prompt` nach einer Beschreibung
+(`t("Image description (alt text)")`) — leere Eingabe oder Abbrechen setzen
+beide `alt=""` (bewusst dekoratives Bild), das Bild wird in jedem Fall
+eingefügt. Ein eigener Toolbar-Button (`t("Image description")`, nur
+sichtbar, wenn `onUploadImage` gesetzt ist) ist deaktiviert, solange kein
+Bild markiert ist; bei markiertem Bild öffnet er denselben Prompt,
+vorausgefüllt mit dem aktuellen Alt-Text, und übernimmt die Änderung —
+Abbrechen lässt den bestehenden Alt-Text unangetastet.
+
 **Bilder aus eingefügtem HTML werden gefiltert, nicht nur Datei-Paste/-Drop:**
 Fügt man Rich-HTML aus einer Webseite ein (Browser-Copy&Paste, nicht als
 Datei), landet es über ProseMirrors HTML-Parser im Dokument — ein
@@ -160,4 +170,5 @@ statt ein zweites, redundantes `ariaLabel` zu brauchen:
 siehe `initI18n`): `"Bold"`, `"Italic"`, `"Heading (large)"`,
 `"Heading (small)"`, `"Bulleted list"`, `"Numbered list"`, `"Link"`,
 `"Enter URL"`, `"Insert image (or drag and drop)"`,
-`"Image upload failed"`.
+`"Image upload failed"`, `"Image description (alt text)"`,
+`"Image description"`.

packages/ui/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@basicbar/ui",
-  "version": "0.4.0",
+  "version": "0.5.0",
   "description": "Design-System-Basis der virtUOS -bar-Tools: Tailwind-Preset, Basis-Styles, Theme (Dark Mode), i18n-Bootstrap, contentLang, RichTextEditor/RichText (TipTap)",
   "license": "Apache-2.0",
   "author": "Universität Osnabrück (virtUOS)",

packages/ui/src/RichTextEditor.tsx

@@ -18,6 +18,7 @@ import { EditorContent, useEditor, type Editor } from "@tiptap/react";
 import StarterKit from "@tiptap/starter-kit";
 import {
   Bold as BoldIcon,
+  Captions,
   Heading2,
   Heading3,
   ImagePlus,
@@ -62,11 +63,13 @@ export interface RichTextEditorProps {
 
 function ToolbarButton({
   active,
+  disabled,
   label,
   onClick,
   children,
 }: {
   active?: boolean;
+  disabled?: boolean;
   label: string;
   onClick: () => void;
   children: React.ReactNode;
@@ -77,10 +80,17 @@ function ToolbarButton({
       title={label}
       aria-label={label}
       aria-pressed={active}
+      // aria-disabled (not native disabled) keeps the button in the tab order,
+      // so keyboard users can discover it before selecting an image.
+      aria-disabled={disabled || undefined}
       onMouseDown={(event) => event.preventDefault()}
-      onClick={onClick}
+      onClick={disabled ? undefined : onClick}
       className={`rounded px-2 py-1 text-sm ${
-        active ? "bg-brand-100 dark:bg-brand-900 text-brand-800 dark:text-brand-200" : "text-slate-600 dark:text-slate-300 hover:bg-slate-100 dark:hover:bg-slate-800"
+        disabled
+          ? "cursor-not-allowed text-slate-400 opacity-50 dark:text-slate-600"
+          : active
+            ? "bg-brand-100 dark:bg-brand-900 text-brand-800 dark:text-brand-200"
+            : "text-slate-600 dark:text-slate-300 hover:bg-slate-100 dark:hover:bg-slate-800"
       }`}
     >
       {children}
@@ -108,13 +118,25 @@ export function RichTextEditor({
       window.alert(`${t("Image upload failed")}${detail}`);
       return;
     }
+    // Ask for alt text once the upload has actually succeeded (WCAG 1.1.1);
+    // empty input or Cancel both mean "no description" — the image is
+    // inserted either way, just as a (possibly) decorative one.
+    const alt = (window.prompt(t("Image description (alt text)"), "") ?? "").trim();
     const chain = editor.chain().focus();
-    if (pos !== undefined) chain.insertContentAt(pos, { type: "image", attrs: { src: url } });
-    else chain.setImage({ src: url });
+    if (pos !== undefined) chain.insertContentAt(pos, { type: "image", attrs: { src: url, alt } });
+    else chain.setImage({ src: url, alt });
     chain.run();
   }
 
   const editor = useEditor({
+    // @tiptap/react v3 no longer re-renders on every transaction by default
+    // (perf choice — see `useEditorState`); we do want that, though: the
+    // toolbar reads `editor.isActive(...)` directly in the render body for
+    // every button (Bold/Italic/headings/link and now "Image description"),
+    // so a selection-only change (e.g. clicking into/out of an image, no
+    // document change) must still re-render this component or those buttons
+    // go stale until the next actual edit.
+    shouldRerenderOnTransaction: true,
     extensions: [
       StarterKit.configure({
         heading: { levels: [2, 3] },
@@ -240,6 +262,14 @@ export function RichTextEditor({
     editor.chain().focus().extendMarkRange("link").setLink({ href: url }).run();
   }
 
+  function setImageAlt() {
+    if (!editor || !editor.isActive("image")) return;
+    const previous = (editor.getAttributes("image").alt as string) ?? "";
+    const alt = window.prompt(t("Image description (alt text)"), previous);
+    if (alt === null) return; // cancelled: leave the current alt untouched
+    editor.chain().focus().updateAttributes("image", { alt: alt.trim() }).run();
+  }
+
   return (
     <div className="rounded-lg border border-slate-300 dark:border-slate-700 focus-within:ring-2 focus-within:ring-brand-600 focus-within:ring-offset-2 focus-within:ring-offset-white dark:focus-within:ring-offset-slate-950">
       <div className="flex flex-nowrap gap-1 overflow-x-auto border-b border-slate-200 dark:border-slate-800 px-2 py-1">
@@ -311,6 +341,15 @@ export function RichTextEditor({
             />
           </label>
         )}
+        {onUploadImage && (
+          <ToolbarButton
+            label={t("Image description")}
+            disabled={!editor.isActive("image")}
+            onClick={setImageAlt}
+          >
+            <Captions aria-hidden className="h-4 w-4" />
+          </ToolbarButton>
+        )}
       </div>
       <EditorContent editor={editor} />
     </div>