From 5ec79e652f30be462fb3ea28128a6498d4f4a7bb Mon Sep 17 00:00:00 2001
From: Keith Smith <keithsmith@darksingularity.org>
Date: Fri, 28 Aug 2026 19:08:25 -0600
Subject: [PATCH] Play video attachments inline, with an expand option (#65)

A video file previously rendered as a generic downloadable file card,
same as any other attachment. The file-serve endpoint forces
Content-Disposition: attachment for every upload as an XSS mitigation
(a same-origin-served .html/.svg executing script), which also meant a
<video> tag pointed at it couldn't play -- the browser would just try
to download it.

Carve out a strict, server-side allowlist (video/mp4, video/webm,
video/ogg -- deliberately not "every video/* type") that skips the
forced download, the same reasoning MessageImage's own endpoint
already relies on: these are content types a browser only ever
interprets as media, never as something that could execute script.
Anything else, including other video formats like .mov, still forces
a download exactly as before.

On the frontend, a video attachment with one of those content types
renders as an inline <video controls> instead of the generic file
card, with a hover-revealed expand button that calls the browser's
native Fullscreen API on the video element directly rather than
building a second lightbox component.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---
 backend/app/routers/rooms.py            | 16 ++++++++-
 backend/app/storage.py                  | 13 +++++++
 backend/tests/test_files.py             | 37 +++++++++++++++++++
 frontend/src/components/MessageList.css | 38 ++++++++++++++++++++
 frontend/src/components/MessageList.tsx | 48 ++++++++++++++++++++++++-
 5 files changed, 150 insertions(+), 2 deletions(-)

diff --git a/backend/app/routers/rooms.py b/backend/app/routers/rooms.py
index 4e98b23..05f8b39 100644
--- a/backend/app/routers/rooms.py
+++ b/backend/app/routers/rooms.py
@@ -89,6 +89,7 @@ from app.services.webhook_service import (
 from app.services.ssrf import UnsafeUrlError
 from app.storage import (
     ALLOWED_IMAGE_CONTENT_TYPES,
+    INLINE_SAFE_VIDEO_CONTENT_TYPES,
     UPLOADS_DIR,
     InvalidImageError,
     UploadTooLargeError,
@@ -548,12 +549,25 @@ async def get_room_file_endpoint(
     message_file = await db.get(MessageFile, file_id)
     if message_file is None or message_file.room_id != room_id:
         raise HTTPException(status_code=404, detail="File not found")
+    # #65: a browser-playable video is served inline (no filename=) so a
+    # <video> tag can actually play it instead of triggering a download --
+    # gated to a strict allowlist (INLINE_SAFE_VIDEO_CONTENT_TYPES), the
+    # same reasoning MessageImage's own endpoint already relies on: these
+    # are content types a browser only ever interprets as media, never as
+    # something that could execute script, so the attachment-disposition
+    # mitigation below doesn't need to apply to them.
+    if message_file.content_type in INLINE_SAFE_VIDEO_CONTENT_TYPES:
+        return FileResponse(
+            UPLOADS_DIR / message_file.storage_filename,
+            media_type=message_file.content_type,
+            headers={"Cache-Control": "private, max-age=31536000, immutable"},
+        )
     # `filename=` makes Starlette set Content-Disposition: attachment,
     # forcing a download instead of an inline render regardless of
     # content-type -- the mitigation for a same-origin-served, user-
     # uploaded file (e.g. .html/.svg) executing script in this app's own
     # origin if opened directly. No content-type allowlist needed on top
-    # of this; see backend/README.md.
+    # of this beyond the video carve-out above; see backend/README.md.
     return FileResponse(
         UPLOADS_DIR / message_file.storage_filename,
         media_type=message_file.content_type,
diff --git a/backend/app/storage.py b/backend/app/storage.py
index e85d217..73812e2 100644
--- a/backend/app/storage.py
+++ b/backend/app/storage.py
@@ -25,6 +25,19 @@ ALLOWED_IMAGE_CONTENT_TYPES: dict[str, tuple[str, str]] = {
     "image/webp": (".webp", "WEBP"),
 }
 
+# #65: browser-natively-playable video formats -- used to decide whether a
+# stored MessageFile gets served inline (a <video> tag can actually play
+# it) or forced to download like every other non-image attachment (see
+# rooms.py's file-serve endpoint). Deliberately a strict allowlist, not
+# "every video/* type": .mov (video/quicktime) has spotty <video> support
+# outside Safari, and more importantly this is the one thing standing
+# between "serve with the browser trusting our declared Content-Type" and
+# reopening the same-origin-script-execution risk Content-Disposition:
+# attachment exists to close off for arbitrary uploads -- it must only
+# ever contain types a <video> tag renders as media, never as something
+# that could execute script.
+INLINE_SAFE_VIDEO_CONTENT_TYPES = frozenset({"video/mp4", "video/webm", "video/ogg"})
+
 
 class UploadTooLargeError(Exception):
     pass
diff --git a/backend/tests/test_files.py b/backend/tests/test_files.py
index 455c461..30e4fdd 100644
--- a/backend/tests/test_files.py
+++ b/backend/tests/test_files.py
@@ -99,6 +99,43 @@ async def test_serve_file_forces_download(client, db_session):
     assert "notes.txt" in disposition
 
 
+async def test_serve_allowlisted_video_inline(client, db_session):
+    # #65: a <video> tag can't play something the browser is forced to
+    # download instead -- browser-playable video types are the one carve-
+    # out from test_serve_file_forces_download's rule above.
+    await register_and_login(client, db_session, username=_unique("alice"))
+    room = (await client.post("/api/rooms", json={"name": _unique("general")})).json()
+    upload = await client.post(
+        f"/api/rooms/{room['id']}/files",
+        files={"file": ("clip.mp4", b"not a real mp4", "video/mp4")},
+    )
+    file_id = upload.json()["id"]
+
+    resp = await client.get(f"/api/rooms/{room['id']}/files/{file_id}")
+    assert resp.status_code == 200
+    assert "content-disposition" not in resp.headers
+    assert resp.headers["content-type"] == "video/mp4"
+
+
+async def test_serve_non_allowlisted_video_still_forces_download(client, db_session):
+    # video/quicktime (.mov) has spotty <video> support outside Safari, and
+    # more importantly this proves the carve-out is a strict allowlist, not
+    # "every video/* content type" -- the security-relevant boundary from
+    # test_serve_file_forces_download must still hold for anything not on
+    # INLINE_SAFE_VIDEO_CONTENT_TYPES.
+    await register_and_login(client, db_session, username=_unique("alice"))
+    room = (await client.post("/api/rooms", json={"name": _unique("general")})).json()
+    upload = await client.post(
+        f"/api/rooms/{room['id']}/files",
+        files={"file": ("clip.mov", b"not a real mov", "video/quicktime")},
+    )
+    file_id = upload.json()["id"]
+
+    resp = await client.get(f"/api/rooms/{room['id']}/files/{file_id}")
+    assert resp.status_code == 200
+    assert "attachment" in resp.headers["content-disposition"]
+
+
 def _register_ws(ws_client, username: str) -> dict:
     from app.schemas.user import UserCreate
     from app.services.auth_service import register_user
diff --git a/frontend/src/components/MessageList.css b/frontend/src/components/MessageList.css
index d850c9d..01cd062 100644
--- a/frontend/src/components/MessageList.css
+++ b/frontend/src/components/MessageList.css
@@ -62,6 +62,44 @@
   margin-bottom: 4px;
 }
 
+.message-video-wrap {
+  position: relative;
+  display: inline-block;
+  max-width: min(320px, 100%);
+  margin-bottom: 4px;
+}
+
+.message-video {
+  display: block;
+  width: 100%;
+  max-height: 240px;
+  border-radius: var(--radius);
+  border: 1px solid var(--ds-border);
+  background: var(--ds-void);
+}
+
+.message-video-expand {
+  position: absolute;
+  top: 6px;
+  right: 6px;
+  display: flex;
+  align-items: center;
+  justify-content: center;
+  padding: 5px;
+  background: rgba(0, 0, 0, 0.6);
+  border: none;
+  border-radius: 6px;
+  color: white;
+  cursor: pointer;
+  opacity: 0;
+  transition: opacity 0.15s ease;
+}
+
+.message-video-wrap:hover .message-video-expand,
+.message-video-expand:focus-visible {
+  opacity: 1;
+}
+
 .message-file-attachment {
   display: inline-flex;
   align-items: center;
diff --git a/frontend/src/components/MessageList.tsx b/frontend/src/components/MessageList.tsx
index aeb14dd..90dbb2a 100644
--- a/frontend/src/components/MessageList.tsx
+++ b/frontend/src/components/MessageList.tsx
@@ -60,6 +60,49 @@ function FileAttachmentCard({ file, roomId, onPreview }: FileAttachmentCardProps
   )
 }
 
+// #65: kept in sync with backend/app/storage.py's INLINE_SAFE_VIDEO_
+// CONTENT_TYPES -- the server only ever serves these particular content
+// types without a forced download, so a <video> tag pointed at anything
+// else would just show a broken player instead of playing (or, worse,
+// trigger a download the moment the browser tries to fetch it).
+const PLAYABLE_VIDEO_CONTENT_TYPES = new Set(['video/mp4', 'video/webm', 'video/ogg'])
+
+interface VideoAttachmentProps {
+  file: MessageFileInfo
+  roomId: string
+}
+
+// Plays inline via the browser's own <video controls> (no custom overlay
+// needed for play/pause/volume/seek) -- the one thing it doesn't give a
+// small inline player is an obvious way to go bigger, so this adds an
+// explicit expand button on top calling the standard Fullscreen API
+// directly on the video element, rather than building a whole second
+// lightbox component just to re-embed the same <video>.
+function VideoAttachment({ file, roomId }: VideoAttachmentProps) {
+  const videoRef = useRef<HTMLVideoElement>(null)
+  return (
+    <div className="message-video-wrap">
+      <video ref={videoRef} src={getRoomFileUrl(roomId, file.id)} controls className="message-video" />
+      <button
+        type="button"
+        className="message-video-expand"
+        onClick={() => videoRef.current?.requestFullscreen()}
+        aria-label="Expand video"
+      >
+        <svg width="14" height="14" viewBox="0 0 20 20" fill="none" aria-hidden="true">
+          <path
+            d="M7 3H3v4M13 3h4v4M3 13v4h4M17 13v4h-4"
+            stroke="currentColor"
+            strokeWidth="1.6"
+            strokeLinecap="round"
+            strokeLinejoin="round"
+          />
+        </svg>
+      </button>
+    </div>
+  )
+}
+
 interface MessageListProps {
   roomId: string
   messages: (Message | ChatMessageEnvelope)[]
@@ -215,7 +258,10 @@ export function MessageList({
                       onClick={() => setLightboxSrc(getRoomImageUrl(roomId, msg.image_id!))}
                     />
                   )}
-                  {msg.file && (
+                  {msg.file && PLAYABLE_VIDEO_CONTENT_TYPES.has(msg.file.content_type) && (
+                    <VideoAttachment file={msg.file} roomId={roomId} />
+                  )}
+                  {msg.file && !PLAYABLE_VIDEO_CONTENT_TYPES.has(msg.file.content_type) && (
                     <FileAttachmentCard
                       file={msg.file}
                       roomId={roomId}