From c78d7454b64dec824add26481c3af5e20106deb8 Mon Sep 17 00:00:00 2001 From: Keith Smith Date: Sat, 15 Aug 2026 08:02:55 -0600 Subject: [PATCH] Add generic file attachments to chat messages Messages can now carry an arbitrary file (MessageFile), parallel to the existing MessageImage feature rather than a refactor of it. Files serve with Content-Disposition: attachment to force a download and prevent an uploaded HTML/SVG from executing same-origin. No content-type allowlist, same 8MB cap as images for now (a separate size-limit redesign is tracked as its own issue). Co-Authored-By: Claude Sonnet 5 --- backend/README.md | 44 +++- .../5b1a142dc271_message_file_attachments.py | 54 +++++ backend/app/models/__init__.py | 2 + backend/app/models/message.py | 8 +- backend/app/models/message_file.py | 28 +++ backend/app/routers/auth.py | 14 +- backend/app/routers/rooms.py | 86 +++++++- backend/app/schemas/message.py | 10 + backend/app/schemas/message_file.py | 10 + backend/app/services/message_events.py | 28 ++- backend/app/services/message_service.py | 7 +- backend/app/storage.py | 12 +- backend/app/ws/chat.py | 23 +- backend/tests/test_files.py | 198 ++++++++++++++++++ frontend/src/api/rooms.ts | 44 +++- frontend/src/components/Composer.css | 32 +++ frontend/src/components/Composer.tsx | 61 ++++-- frontend/src/components/MessageList.css | 43 ++++ frontend/src/components/MessageList.tsx | 29 ++- frontend/src/types.ts | 9 + frontend/src/ws/useChatSocket.ts | 10 +- 21 files changed, 697 insertions(+), 55 deletions(-) create mode 100644 backend/alembic/versions/5b1a142dc271_message_file_attachments.py create mode 100644 backend/app/models/message_file.py create mode 100644 backend/app/schemas/message_file.py create mode 100644 backend/tests/test_files.py diff --git a/backend/README.md b/backend/README.md index d03c3a7..9cdb513 100644 --- a/backend/README.md +++ b/backend/README.md @@ -1,4 +1,4 @@ -# KeepItTalking backend (Phase 1 + 2 + 4 + 5 + 6 + 7 + 8, image uploads, emoji & reactions, user profiles, site invites & email, password reset) +# KeepItTalking backend (Phase 1 + 2 + 4 + 5 + 6 + 7 + 8, image uploads, file attachments, emoji & reactions, user profiles, site invites & email, password reset) FastAPI + SQLAlchemy 2.0 (async) + PostgreSQL + Redis. Implements auth, room CRUD (open and private), room roles (owner/admin/member) and direct @@ -6,8 +6,9 @@ membership management, a WebSocket chat endpoint that fans out across multiple app-server instances via Redis pub/sub, Web Push notifications for offline room members, a site-admin portal (user/room/bot management + an audit log), a bot/extension layer (scoped API tokens, live bot WebSocket -access, incoming and outgoing webhooks, message editing), image uploads in -chat messages, emoji reactions on messages, self-service user profiles +access, incoming and outgoing webhooks, message editing), image uploads and +generic file attachments in chat messages, emoji reactions on messages, +self-service user profiles (display name, avatar), self-service password change and a token-based forgot-password flow, and admin-issued email invites for new accounts plus email notifications when a user is added to a room. See @@ -364,6 +365,39 @@ cleanup job for this yet. Not a security issue, since serving still goes through the same room-membership gate as everything else; just an eventual disk-space housekeeping item. +## File attachments + +A message can also carry a generic file attachment (`Message.file_id`, +nullable, alongside the pre-existing `content` and `image_id`) — a +parallel `MessageFile` model/table, not a generalization of `MessageImage`, +so the working image feature stayed untouched. `app/storage.py`'s +`save_image`/`delete_image` were content-agnostic already (no Pillow +usage) and were renamed to `save_file`/`delete_file` now that both features +share them; `ImageTooLargeError` was likewise renamed to +`UploadTooLargeError`. + +- `POST /api/rooms/{room_id}/files` (room-member gated, multipart) — same + 8 MB cap as images (`MAX_FILE_BYTES`, currently an alias of + `MAX_IMAGE_BYTES`; a real, independently-configurable size-limit redesign + is a separate later task — see the open file/image size-limit issue), but + **no content-type allowlist** — arbitrary file types are the point of this + endpoint, unlike `/images`. +- `GET /api/rooms/{room_id}/files/{file_id}` (room-member gated) — 404s if + the file doesn't belong to that room, otherwise streams it via + `FileResponse(..., filename=...)`. Passing `filename=` makes Starlette set + `Content-Disposition: attachment`, which forces a download in the browser + regardless of content-type — the deliberate mitigation against a + user-uploaded `.html`/`.svg` executing script same-origin (session-cookie + theft) if opened directly. This is why there's no content-type blocklist + on top of it: forcing a download already neutralizes that whole class of + risk. +- The WS `"message"` handler and `message_events.py` push-body/broadcast + logic mirror the image path exactly (an optional `file_id`, validated + against the room; push body says "`{username} sent a file`" for a + file-only message). + +Same orphaned-upload disk-space caveat as images applies here too. + ## Emoji & reactions An emoji picker in the frontend composer is purely client-side (a static @@ -402,11 +436,11 @@ scoped out. everywhere it's displayed. - `POST /api/auth/me/avatar` / `DELETE /api/auth/me/avatar` — reuse `app/storage.py`'s upload primitives (`read_capped`, `process_image`, - `save_image`) from image uploads, but call `process_image(..., square=True, + `save_file`) from image uploads, but call `process_image(..., square=True, max_dimension=512)` — a new option that center-crops before downscaling, since avatars need a fixed square shape at a much smaller size than a message image. Unlike message images (which never delete), the previous - avatar file **is deleted** on replace/remove (`storage.delete_image`) — + avatar file **is deleted** on replace/remove (`storage.delete_file`) — safe to do here because it's strictly one file per user, no accumulation risk to accept the way an orphaned message-image upload has. - `GET /api/users/{user_id}/avatar` (`app/routers/users.py`, new router) — diff --git a/backend/alembic/versions/5b1a142dc271_message_file_attachments.py b/backend/alembic/versions/5b1a142dc271_message_file_attachments.py new file mode 100644 index 0000000..2d3235d --- /dev/null +++ b/backend/alembic/versions/5b1a142dc271_message_file_attachments.py @@ -0,0 +1,54 @@ +"""message file attachments + +Revision ID: 5b1a142dc271 +Revises: 456cd78ca571 +Create Date: 2026-08-14 21:53:05.055721 + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision: str = '5b1a142dc271' +down_revision: Union[str, Sequence[str], None] = '456cd78ca571' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + """Upgrade schema.""" + # ### commands auto generated by Alembic - please adjust! ### + op.create_table('message_files', + sa.Column('id', sa.Uuid(), nullable=False), + sa.Column('room_id', sa.Uuid(), nullable=False), + sa.Column('uploaded_by', sa.Uuid(), nullable=False), + sa.Column('storage_filename', sa.String(length=64), nullable=False), + sa.Column('original_filename', sa.String(length=255), nullable=False), + sa.Column('content_type', sa.String(length=150), nullable=False), + sa.Column('size_bytes', sa.BigInteger(), nullable=False), + sa.Column('created_at', sa.DateTime(timezone=True), server_default=sa.text('now()'), nullable=False), + sa.ForeignKeyConstraint(['room_id'], ['rooms.id'], ), + sa.ForeignKeyConstraint(['uploaded_by'], ['users.id'], ), + sa.PrimaryKeyConstraint('id') + ) + op.create_index(op.f('ix_message_files_room_id'), 'message_files', ['room_id'], unique=False) + op.add_column('messages', sa.Column('file_id', sa.Uuid(), nullable=True)) + op.create_foreign_key('messages_file_id_fkey', 'messages', 'message_files', ['file_id'], ['id']) + op.drop_constraint(op.f('messages_content_or_image_required'), 'messages', type_='check') + op.create_check_constraint('messages_content_or_attachment_required', 'messages', 'content IS NOT NULL OR image_id IS NOT NULL OR file_id IS NOT NULL') + # ### end Alembic commands ### + + +def downgrade() -> None: + """Downgrade schema.""" + # ### commands auto generated by Alembic - please adjust! ### + op.drop_constraint('messages_content_or_attachment_required', 'messages', type_='check') + op.create_check_constraint(op.f('messages_content_or_image_required'), 'messages', 'content IS NOT NULL OR image_id IS NOT NULL') + op.drop_constraint('messages_file_id_fkey', 'messages', type_='foreignkey') + op.drop_column('messages', 'file_id') + op.drop_index(op.f('ix_message_files_room_id'), table_name='message_files') + op.drop_table('message_files') + # ### end Alembic commands ### diff --git a/backend/app/models/__init__.py b/backend/app/models/__init__.py index 8719b89..a719560 100644 --- a/backend/app/models/__init__.py +++ b/backend/app/models/__init__.py @@ -5,6 +5,7 @@ from app.models.event_subscription import EventSubscription from app.models.invite import InviteStatus from app.models.membership import RoomMembership, RoomRole from app.models.message import Message +from app.models.message_file import MessageFile from app.models.message_image import MessageImage from app.models.message_reaction import MessageReaction from app.models.password_reset import PasswordReset @@ -22,6 +23,7 @@ __all__ = [ "RoomMembership", "RoomRole", "Message", + "MessageFile", "MessageImage", "MessageReaction", "InviteStatus", diff --git a/backend/app/models/message.py b/backend/app/models/message.py index fad67ba..82024d5 100644 --- a/backend/app/models/message.py +++ b/backend/app/models/message.py @@ -11,8 +11,8 @@ class Message(Base): __tablename__ = "messages" __table_args__ = ( CheckConstraint( - "content IS NOT NULL OR image_id IS NOT NULL", - name="messages_content_or_image_required", + "content IS NOT NULL OR image_id IS NOT NULL OR file_id IS NOT NULL", + name="messages_content_or_attachment_required", ), ) @@ -21,9 +21,10 @@ class Message(Base): user_id: Mapped[uuid.UUID] = mapped_column(ForeignKey("users.id"), nullable=False) # Nullable since Phase "image uploads": a message can be an image with # no caption. The CheckConstraint above still requires at least one of - # content/image_id. + # content/image_id/file_id. content: Mapped[str | None] = mapped_column(Text) image_id: Mapped[uuid.UUID | None] = mapped_column(ForeignKey("message_images.id")) + file_id: Mapped[uuid.UUID | None] = mapped_column(ForeignKey("message_files.id")) created_at: Mapped[datetime] = mapped_column( DateTime(timezone=True), server_default=func.now(), index=True, nullable=False ) @@ -32,3 +33,4 @@ class Message(Base): user = relationship("User") image = relationship("MessageImage") + file = relationship("MessageFile") diff --git a/backend/app/models/message_file.py b/backend/app/models/message_file.py new file mode 100644 index 0000000..6f825a7 --- /dev/null +++ b/backend/app/models/message_file.py @@ -0,0 +1,28 @@ +import uuid +from datetime import datetime + +from sqlalchemy import BigInteger, DateTime, ForeignKey, String, func +from sqlalchemy.orm import Mapped, mapped_column, relationship + +from app.models.base import Base + + +class MessageFile(Base): + __tablename__ = "message_files" + + id: Mapped[uuid.UUID] = mapped_column(primary_key=True, default=uuid.uuid4) + room_id: Mapped[uuid.UUID] = mapped_column(ForeignKey("rooms.id"), index=True, nullable=False) + uploaded_by: Mapped[uuid.UUID] = mapped_column(ForeignKey("users.id"), nullable=False) + storage_filename: Mapped[str] = mapped_column(String(64), nullable=False) + original_filename: Mapped[str] = mapped_column(String(255), nullable=False) + # Wider than MessageImage's content_type column -- generic MIME strings + # (e.g. the Office Open XML types) run 60-80 chars, unlike images' + # four known short values. + content_type: Mapped[str] = mapped_column(String(150), nullable=False) + size_bytes: Mapped[int] = mapped_column(BigInteger, nullable=False) + created_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), server_default=func.now(), nullable=False + ) + + room = relationship("Room") + uploader = relationship("User") diff --git a/backend/app/routers/auth.py b/backend/app/routers/auth.py index e6b2c3e..b634b44 100644 --- a/backend/app/routers/auth.py +++ b/backend/app/routers/auth.py @@ -22,12 +22,12 @@ from app.services.password_service import ( ) from app.storage import ( ALLOWED_IMAGE_CONTENT_TYPES, - ImageTooLargeError, InvalidImageError, - delete_image, + UploadTooLargeError, + delete_file, process_image, read_capped, - save_image, + save_file, ) AVATAR_MAX_DIMENSION = 512 @@ -91,7 +91,7 @@ async def upload_avatar( try: data = await read_capped(file) - except ImageTooLargeError: + except UploadTooLargeError: raise HTTPException(status_code=413, detail="Image exceeds 8 MB limit") try: @@ -102,14 +102,14 @@ async def upload_avatar( raise HTTPException(status_code=400, detail="File is not a valid image") previous_filename = current_user.avatar_filename - storage_filename = save_image(data, ext) + storage_filename = save_file(data, ext) current_user.avatar_filename = storage_filename current_user.avatar_content_type = file.content_type await db.commit() await db.refresh(current_user) if previous_filename: - delete_image(previous_filename) + delete_file(previous_filename) return current_user @@ -126,7 +126,7 @@ async def remove_avatar( await db.refresh(current_user) if previous_filename: - delete_image(previous_filename) + delete_file(previous_filename) return current_user diff --git a/backend/app/routers/rooms.py b/backend/app/routers/rooms.py index b0fcf25..cb24872 100644 --- a/backend/app/routers/rooms.py +++ b/backend/app/routers/rooms.py @@ -1,3 +1,4 @@ +import pathlib import uuid from fastapi import APIRouter, Depends, File, HTTPException, Query, Request, UploadFile @@ -11,8 +12,9 @@ from app.dependencies import ( require_room_role, require_scope, ) -from app.models import MessageImage, RoomRole, User -from app.schemas.message import MessageRead +from app.models import MessageFile, MessageImage, RoomRole, User +from app.schemas.message import MessageFileInfo, MessageRead +from app.schemas.message_file import MessageFileCreated from app.schemas.message_image import MessageImageCreated from app.schemas.room import ( MyRoomItem, @@ -71,12 +73,13 @@ from app.services.webhook_service import ( from app.services.ssrf import UnsafeWebhookUrlError from app.storage import ( ALLOWED_IMAGE_CONTENT_TYPES, + MAX_FILE_BYTES, UPLOADS_DIR, - ImageTooLargeError, InvalidImageError, + UploadTooLargeError, process_image, read_capped, - save_image, + save_file, ) router = APIRouter(prefix="/api/rooms", tags=["rooms"]) @@ -284,6 +287,15 @@ async def transfer_ownership_endpoint( ) +def _to_message_file_info(file: MessageFile) -> MessageFileInfo: + return MessageFileInfo( + id=file.id, + filename=file.original_filename, + size_bytes=file.size_bytes, + content_type=file.content_type, + ) + + @router.get("/{room_id}/messages", response_model=list[MessageRead]) async def get_room_messages_endpoint( room_id: uuid.UUID, @@ -304,6 +316,7 @@ async def get_room_messages_endpoint( username=m.user.username, content=m.content, image_id=m.image_id, + file=_to_message_file_info(m.file) if m.file else None, reactions=reactions_by_message.get(m.id, []), created_at=m.created_at, edited_at=m.edited_at, @@ -326,7 +339,7 @@ async def upload_room_image_endpoint( try: data = await read_capped(file) - except ImageTooLargeError: + except UploadTooLargeError: raise HTTPException(status_code=413, detail="Image exceeds 8 MB limit") try: @@ -334,7 +347,7 @@ async def upload_room_image_endpoint( except InvalidImageError: raise HTTPException(status_code=400, detail="File is not a valid image") - storage_filename = save_image(data, ext) + storage_filename = save_file(data, ext) image = MessageImage( room_id=room_id, uploaded_by=current_user.id, @@ -366,6 +379,67 @@ async def get_room_image_endpoint( ) +@router.post("/{room_id}/files", response_model=MessageFileCreated, status_code=201) +async def upload_room_file_endpoint( + room_id: uuid.UUID, + file: UploadFile = File(...), + current_user: User = Depends(get_current_user), + db: AsyncSession = Depends(get_db), +): + await require_room_member(room_id, current_user, db) + + try: + data = await read_capped(file, cap=MAX_FILE_BYTES) + except UploadTooLargeError: + raise HTTPException(status_code=413, detail="File exceeds 8 MB limit") + + original_filename = file.filename or "file" + ext = pathlib.Path(original_filename).suffix + storage_filename = save_file(data, ext) + message_file = MessageFile( + room_id=room_id, + uploaded_by=current_user.id, + storage_filename=storage_filename, + original_filename=original_filename, + content_type=file.content_type or "application/octet-stream", + size_bytes=len(data), + ) + db.add(message_file) + await db.commit() + await db.refresh(message_file) + return MessageFileCreated( + id=message_file.id, + filename=message_file.original_filename, + size_bytes=message_file.size_bytes, + content_type=message_file.content_type, + ) + + +@router.get("/{room_id}/files/{file_id}") +async def get_room_file_endpoint( + room_id: uuid.UUID, + file_id: uuid.UUID, + current_user: User = Depends(get_current_user), + db: AsyncSession = Depends(get_db), +): + await require_room_member(room_id, current_user, db) + 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") + # `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. + return FileResponse( + UPLOADS_DIR / message_file.storage_filename, + media_type=message_file.content_type, + filename=message_file.original_filename, + headers={"Cache-Control": "private, max-age=31536000, immutable"}, + ) + + @router.post("/{room_id}/members", response_model=RoomMemberRead, status_code=201) async def add_member_endpoint( room_id: uuid.UUID, diff --git a/backend/app/schemas/message.py b/backend/app/schemas/message.py index 6b1fa3f..7914d58 100644 --- a/backend/app/schemas/message.py +++ b/backend/app/schemas/message.py @@ -10,6 +10,15 @@ class ReactionSummary(BaseModel): user_ids: list[str] +class MessageFileInfo(BaseModel): + model_config = ConfigDict(from_attributes=True) + + id: uuid.UUID + filename: str + size_bytes: int + content_type: str + + class MessageRead(BaseModel): model_config = ConfigDict(from_attributes=True) @@ -19,6 +28,7 @@ class MessageRead(BaseModel): username: str content: str | None image_id: uuid.UUID | None + file: MessageFileInfo | None reactions: list[ReactionSummary] created_at: datetime edited_at: datetime | None diff --git a/backend/app/schemas/message_file.py b/backend/app/schemas/message_file.py new file mode 100644 index 0000000..fbea6af --- /dev/null +++ b/backend/app/schemas/message_file.py @@ -0,0 +1,10 @@ +import uuid + +from pydantic import BaseModel + + +class MessageFileCreated(BaseModel): + id: uuid.UUID + filename: str + size_bytes: int + content_type: str diff --git a/backend/app/services/message_events.py b/backend/app/services/message_events.py index aff3e75..74f2806 100644 --- a/backend/app/services/message_events.py +++ b/backend/app/services/message_events.py @@ -3,7 +3,7 @@ import uuid from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession -from app.models import Message, Room, RoomMembership, User +from app.models import Message, MessageFile, Room, RoomMembership, User from app.schemas.message import ReactionSummary from app.services.push_service import send_push_to_user from app.services.webhook_service import dispatch_event @@ -27,11 +27,12 @@ async def _notify_offline_members( return room = await db.get(Room, room_id) - body = ( - f"{sender.username}: {message.content}"[:120] - if message.content - else f"{sender.username} sent an image" - ) + if message.content: + body = f"{sender.username}: {message.content}"[:120] + elif message.file_id: + body = f"{sender.username} sent a file" + else: + body = f"{sender.username} sent an image" payload = { "title": f"#{room.name}" if room else "New message", "body": body, @@ -41,7 +42,17 @@ async def _notify_offline_members( await send_push_to_user(db, user_id, payload) -def _message_payload(message: Message, username: str) -> dict: +async def _message_payload(db: AsyncSession, message: Message, username: str) -> dict: + file_payload = None + if message.file_id: + message_file = await db.get(MessageFile, message.file_id) + if message_file: + file_payload = { + "id": str(message_file.id), + "filename": message_file.original_filename, + "size_bytes": message_file.size_bytes, + "content_type": message_file.content_type, + } return { "type": "message", "id": str(message.id), @@ -50,6 +61,7 @@ def _message_payload(message: Message, username: str) -> dict: "username": username, "content": message.content, "image_id": str(message.image_id) if message.image_id else None, + "file": file_payload, "reactions": [], "created_at": message.created_at.isoformat(), "edited_at": message.edited_at.isoformat() if message.edited_at else None, @@ -67,7 +79,7 @@ async def broadcast_new_message( """The full side-effect sequence for a newly created message, shared by the WS "message" handler and the incoming-webhook receiver so both trigger identical fan-out/push/event behavior.""" - payload = _message_payload(message, sender.username) + payload = await _message_payload(db, message, sender.username) await broadcaster.publish(room_id, payload) await _notify_offline_members(db, presence, room_id, sender, message) await dispatch_event(db, "message.created", room_id, payload) diff --git a/backend/app/services/message_service.py b/backend/app/services/message_service.py index c55804a..13548e8 100644 --- a/backend/app/services/message_service.py +++ b/backend/app/services/message_service.py @@ -24,8 +24,11 @@ async def create_message( user_id: uuid.UUID, content: str | None = None, image_id: uuid.UUID | None = None, + file_id: uuid.UUID | None = None, ) -> Message: - message = Message(room_id=room_id, user_id=user_id, content=content, image_id=image_id) + message = Message( + room_id=room_id, user_id=user_id, content=content, image_id=image_id, file_id=file_id + ) db.add(message) await db.commit() await db.refresh(message) @@ -54,7 +57,7 @@ async def list_recent_messages( result = await db.execute( select(Message) .where(Message.room_id == room_id) - .options(selectinload(Message.user)) + .options(selectinload(Message.user), selectinload(Message.file)) .order_by(Message.created_at.desc()) .limit(limit) ) diff --git a/backend/app/storage.py b/backend/app/storage.py index 49e7256..ab73338 100644 --- a/backend/app/storage.py +++ b/backend/app/storage.py @@ -11,6 +11,10 @@ from PIL import Image, UnidentifiedImageError UPLOADS_DIR = pathlib.Path(__file__).resolve().parent.parent.parent / "uploads" MAX_IMAGE_BYTES = 8 * 1024 * 1024 +# Separate named constant (same value for now) so a later size-limit +# redesign for generic file attachments doesn't have to touch image +# behavior. +MAX_FILE_BYTES = MAX_IMAGE_BYTES _READ_CHUNK_BYTES = 1024 * 1024 _MAX_DIMENSION = 2000 @@ -23,7 +27,7 @@ ALLOWED_IMAGE_CONTENT_TYPES: dict[str, tuple[str, str]] = { } -class ImageTooLargeError(Exception): +class UploadTooLargeError(Exception): pass @@ -43,7 +47,7 @@ async def read_capped(file, cap: int = MAX_IMAGE_BYTES) -> bytes: break total += len(chunk) if total > cap: - raise ImageTooLargeError() + raise UploadTooLargeError() chunks.append(chunk) return b"".join(chunks) @@ -89,14 +93,14 @@ def process_image( return out.getvalue(), ext -def save_image(data: bytes, ext: str) -> str: +def save_file(data: bytes, ext: str) -> str: UPLOADS_DIR.mkdir(parents=True, exist_ok=True) storage_filename = f"{uuid.uuid4()}{ext}" (UPLOADS_DIR / storage_filename).write_bytes(data) return storage_filename -def delete_image(storage_filename: str) -> None: +def delete_file(storage_filename: str) -> None: """Best-effort delete -- a missing file (already gone, or never written) is not an error.""" (UPLOADS_DIR / storage_filename).unlink(missing_ok=True) diff --git a/backend/app/ws/chat.py b/backend/app/ws/chat.py index ad1140d..a68989f 100644 --- a/backend/app/ws/chat.py +++ b/backend/app/ws/chat.py @@ -6,7 +6,7 @@ from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession from app.database import get_db -from app.models import ApiToken, Message, MessageImage, RoomMembership, User +from app.models import ApiToken, Message, MessageFile, MessageImage, RoomMembership, User from app.services.bot_service import resolve_token from app.services.message_events import ( broadcast_message_update, @@ -31,6 +31,7 @@ class ClientEnvelope(BaseModel): room_id: uuid.UUID | None = None content: str | None = None image_id: uuid.UUID | None = None + file_id: uuid.UUID | None = None message_id: uuid.UUID | None = None emoji: str | None = None @@ -110,9 +111,16 @@ async def chat_endpoint(websocket: WebSocket, db: AsyncSession = Depends(get_db) joined_rooms.discard(envelope.room_id) elif envelope.type == "message": - if envelope.room_id is None or (not envelope.content and envelope.image_id is None): + if envelope.room_id is None or ( + not envelope.content + and envelope.image_id is None + and envelope.file_id is None + ): await websocket.send_json( - {"type": "error", "detail": "room_id and content or image_id required"} + { + "type": "error", + "detail": "room_id and content or image_id or file_id required", + } ) continue if _missing_scope(api_token, "write:messages"): @@ -134,8 +142,15 @@ async def chat_endpoint(websocket: WebSocket, db: AsyncSession = Depends(get_db) await websocket.send_json({"type": "error", "detail": "Invalid image"}) continue image_id = image.id + file_id = None + if envelope.file_id is not None: + message_file = await db.get(MessageFile, envelope.file_id) + if message_file is None or message_file.room_id != envelope.room_id: + await websocket.send_json({"type": "error", "detail": "Invalid file"}) + continue + file_id = message_file.id message = await create_message( - db, envelope.room_id, user.id, envelope.content, image_id + db, envelope.room_id, user.id, envelope.content, image_id, file_id ) await broadcast_new_message(db, broadcaster, presence, envelope.room_id, message, user) diff --git a/backend/tests/test_files.py b/backend/tests/test_files.py new file mode 100644 index 0000000..455c461 --- /dev/null +++ b/backend/tests/test_files.py @@ -0,0 +1,198 @@ +import uuid + +from tests.conftest import register_and_login + + +def _unique(prefix: str) -> str: + return f"{prefix}-{uuid.uuid4().hex[:8]}" + + +async def test_upload_file_succeeds(client, db_session): + await register_and_login(client, db_session, username=_unique("alice")) + room = (await client.post("/api/rooms", json={"name": _unique("general")})).json() + + resp = await client.post( + f"/api/rooms/{room['id']}/files", + files={"file": ("report.pdf", b"%PDF-1.4 not a real pdf", "application/pdf")}, + ) + assert resp.status_code == 201, resp.text + body = resp.json() + assert "id" in body + assert body["filename"] == "report.pdf" + assert body["size_bytes"] == len(b"%PDF-1.4 not a real pdf") + assert body["content_type"] == "application/pdf" + + +async def test_upload_any_content_type_accepted(client, db_session): + await register_and_login(client, db_session, username=_unique("alice")) + room = (await client.post("/api/rooms", json={"name": _unique("general")})).json() + + # Unlike /images, /files has no content-type allowlist -- arbitrary + # files are the point of this endpoint. + resp = await client.post( + f"/api/rooms/{room['id']}/files", + files={"file": ("archive.zip", b"PK\x03\x04 not a real zip", "application/zip")}, + ) + assert resp.status_code == 201, resp.text + + +async def test_upload_oversized_file_rejected(client, db_session): + await register_and_login(client, db_session, username=_unique("alice")) + room = (await client.post("/api/rooms", json={"name": _unique("general")})).json() + + oversized = b"0" * (9 * 1024 * 1024) + resp = await client.post( + f"/api/rooms/{room['id']}/files", + files={"file": ("huge.bin", oversized, "application/octet-stream")}, + ) + assert resp.status_code == 413 + + +async def test_serve_file_requires_room_membership(client, db_session): + 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": ("notes.txt", b"hello", "text/plain")}, + ) + file_id = upload.json()["id"] + + ok = await client.get(f"/api/rooms/{room['id']}/files/{file_id}") + assert ok.status_code == 200 + + await register_and_login(client, db_session, username=_unique("bob")) + resp = await client.get(f"/api/rooms/{room['id']}/files/{file_id}") + assert resp.status_code == 403 + + +async def test_serve_file_404s_for_wrong_room(client, db_session): + await register_and_login(client, db_session, username=_unique("alice")) + room_a = (await client.post("/api/rooms", json={"name": _unique("room-a")})).json() + room_b = (await client.post("/api/rooms", json={"name": _unique("room-b")})).json() + upload = await client.post( + f"/api/rooms/{room_a['id']}/files", + files={"file": ("notes.txt", b"hello", "text/plain")}, + ) + file_id = upload.json()["id"] + + resp = await client.get(f"/api/rooms/{room_b['id']}/files/{file_id}") + assert resp.status_code == 404 + + +async def test_serve_file_forces_download(client, db_session): + # The security-relevant assertion for this feature: a user-uploaded + # file must never render inline (e.g. an uploaded .html executing + # script same-origin) -- Content-Disposition: attachment forces a + # download in the browser regardless of content-type. + 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": ("notes.txt", b"hello", "text/plain")}, + ) + file_id = upload.json()["id"] + + resp = await client.get(f"/api/rooms/{room['id']}/files/{file_id}") + assert resp.status_code == 200 + disposition = resp.headers["content-disposition"] + assert "attachment" in disposition + assert "notes.txt" in disposition + + +def _register_ws(ws_client, username: str) -> dict: + from app.schemas.user import UserCreate + from app.services.auth_service import register_user + + async def _seed(): + async with ws_client.session_factory() as session: + await register_user( + session, + UserCreate(username=username, email=f"{username}@example.com", password="password123"), + ) + + ws_client.portal.call(_seed) + resp = ws_client.post( + "/api/auth/login", json={"username_or_email": username, "password": "password123"} + ) + assert resp.status_code == 200, resp.text + return resp.json() + + +def test_ws_file_only_message_roundtrips(ws_client): + _register_ws(ws_client, _unique("alice")) + room = ws_client.post("/api/rooms", json={"name": _unique("general")}).json() + + upload = ws_client.post( + f"/api/rooms/{room['id']}/files", + files={"file": ("notes.txt", b"hello", "text/plain")}, + ) + file_id = upload.json()["id"] + + with ws_client.websocket_connect("/ws/chat") as ws: + ws.send_json({"type": "join", "room_id": room["id"]}) + assert ws.receive_json()["type"] == "joined" + + ws.send_json({"type": "message", "room_id": room["id"], "file_id": file_id}) + message = ws.receive_json() + assert message["type"] == "message" + assert message["content"] is None + assert message["file"]["id"] == file_id + assert message["file"]["filename"] == "notes.txt" + assert message["file"]["size_bytes"] == len(b"hello") + + history = ws_client.get(f"/api/rooms/{room['id']}/messages").json() + persisted = next(m for m in history if m["id"] == message["id"]) + assert persisted["content"] is None + assert persisted["file"]["id"] == file_id + assert persisted["file"]["filename"] == "notes.txt" + + +def test_ws_message_requires_content_or_image_or_file(ws_client): + _register_ws(ws_client, _unique("alice")) + room = ws_client.post("/api/rooms", json={"name": _unique("general")}).json() + + with ws_client.websocket_connect("/ws/chat") as ws: + ws.send_json({"type": "join", "room_id": room["id"]}) + assert ws.receive_json()["type"] == "joined" + + ws.send_json({"type": "message", "room_id": room["id"]}) + resp = ws.receive_json() + assert resp["type"] == "error" + + +def test_push_body_says_sent_a_file_for_file_only_message(ws_client, monkeypatch): + calls = [] + monkeypatch.setattr("app.services.push_service.webpush", lambda **kw: calls.append(kw)) + + alice = _register_ws(ws_client, _unique("alice")) + room = ws_client.post("/api/rooms", json={"name": _unique("general")}).json() + + _register_ws(ws_client, _unique("bob")) + ws_client.post(f"/api/rooms/{room['id']}/join") + ws_client.post( + "/api/push/subscribe", + json={ + "endpoint": f"https://push.example.com/ep-{_unique('bob')}", + "keys": {"p256dh": "p256dh-bob", "auth": "auth-bob"}, + }, + ) + + ws_client.post( + "/api/auth/login", json={"username_or_email": alice["username"], "password": "password123"} + ) + upload = ws_client.post( + f"/api/rooms/{room['id']}/files", + files={"file": ("notes.txt", b"hello", "text/plain")}, + ) + file_id = upload.json()["id"] + + with ws_client.websocket_connect("/ws/chat") as ws: + ws.send_json({"type": "join", "room_id": room["id"]}) + assert ws.receive_json()["type"] == "joined" + ws.send_json({"type": "message", "room_id": room["id"], "file_id": file_id}) + assert ws.receive_json()["type"] == "message" + ws.send_json({"type": "join", "room_id": room["id"]}) + assert ws.receive_json()["type"] == "joined" + + assert len(calls) == 1 + assert "sent a file" in calls[0]["data"] diff --git a/frontend/src/api/rooms.ts b/frontend/src/api/rooms.ts index dbbd400..264c46f 100644 --- a/frontend/src/api/rooms.ts +++ b/frontend/src/api/rooms.ts @@ -1,5 +1,13 @@ import { apiFetch, ApiError, NetworkError } from './client' -import type { Message, MyRoomItem, Room, RoomListItem, RoomMember, RoomRole } from '../types' +import type { + Message, + MessageFileInfo, + MyRoomItem, + Room, + RoomListItem, + RoomMember, + RoomRole, +} from '../types' export function listRooms(): Promise { return apiFetch('/api/rooms') @@ -114,3 +122,37 @@ export async function uploadRoomImage(roomId: string, file: File): Promise<{ id: export function getRoomImageUrl(roomId: string, imageId: string): string { return `/api/rooms/${roomId}/images/${imageId}` } + +// Not apiFetch, same multipart-boundary reason as uploadRoomImage. +export async function uploadRoomFile(roomId: string, file: File): Promise { + const formData = new FormData() + formData.append('file', file) + + let response: Response + try { + response = await fetch(`/api/rooms/${roomId}/files`, { + method: 'POST', + credentials: 'include', + body: formData, + }) + } catch { + throw new NetworkError() + } + + if (!response.ok) { + let detail = response.statusText + try { + const body = await response.json() + detail = body.detail ?? detail + } catch { + // response had no JSON body + } + throw new ApiError(response.status, detail) + } + + return (await response.json()) as MessageFileInfo +} + +export function getRoomFileUrl(roomId: string, fileId: string): string { + return `/api/rooms/${roomId}/files/${fileId}` +} diff --git a/frontend/src/components/Composer.css b/frontend/src/components/Composer.css index c4fc33b..63abfb3 100644 --- a/frontend/src/components/Composer.css +++ b/frontend/src/components/Composer.css @@ -130,6 +130,38 @@ object-fit: cover; } +.composer-attachment-file { + display: flex; + align-items: center; + gap: 6px; + background: var(--ds-surface-2); + border: 1px solid var(--ds-border); + border-radius: var(--radius); + padding: 6px 28px 6px 10px; + color: var(--ds-text); + max-width: 260px; +} + +.composer-attachment-file svg { + flex: none; + color: var(--ds-muted); +} + +.composer-attachment-filename { + font-size: 0.82rem; + font-weight: 600; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; +} + +.composer-attachment-size { + font-size: 0.72rem; + color: var(--ds-muted); + font-family: var(--mono); + flex: none; +} + .composer-attachment-remove { position: absolute; top: -6px; diff --git a/frontend/src/components/Composer.tsx b/frontend/src/components/Composer.tsx index 0e867ce..9ed330e 100644 --- a/frontend/src/components/Composer.tsx +++ b/frontend/src/components/Composer.tsx @@ -1,6 +1,6 @@ import { useRef, useState, type ChangeEvent, type KeyboardEvent } from 'react' import { useOnlineStatus } from '../hooks/useOnlineStatus' -import { uploadRoomImage } from '../api/rooms' +import { uploadRoomFile, uploadRoomImage } from '../api/rooms' import { EmojiPicker } from './EmojiPicker' import './Composer.css' @@ -8,12 +8,21 @@ interface ComposerProps { roomId: string roomName: string disabled?: boolean - onSend: (content: string, imageId?: string) => void + onSend: (content: string, imageId?: string, fileId?: string) => void +} + +function formatFileSize(bytes: number): string { + if (bytes < 1024) return `${bytes} B` + if (bytes < 1024 * 1024) return `${(bytes / 1024).toFixed(1)} KB` + return `${(bytes / (1024 * 1024)).toFixed(1)} MB` } export function Composer({ roomId, roomName, disabled, onSend }: ComposerProps) { const [value, setValue] = useState('') const [pendingImage, setPendingImage] = useState<{ id: string; previewUrl: string } | null>(null) + const [pendingFile, setPendingFile] = useState<{ id: string; filename: string; size: number } | null>( + null, + ) const [uploading, setUploading] = useState(false) const [uploadError, setUploadError] = useState(null) const [emojiPickerOpen, setEmojiPickerOpen] = useState(false) @@ -30,10 +39,11 @@ export function Composer({ roomId, roomName, disabled, onSend }: ComposerProps) function handleSend() { const trimmed = value.trim() - if (!trimmed && !pendingImage) return - onSend(trimmed, pendingImage?.id) + if (!trimmed && !pendingImage && !pendingFile) return + onSend(trimmed, pendingImage?.id, pendingFile?.id) setValue('') removePendingImage() + setPendingFile(null) requestAnimationFrame(autoGrow) } @@ -52,11 +62,16 @@ export function Composer({ roomId, roomName, disabled, onSend }: ComposerProps) setUploadError(null) setUploading(true) try { - const { id } = await uploadRoomImage(roomId, file) - setPendingImage((prev) => { - if (prev) URL.revokeObjectURL(prev.previewUrl) - return { id, previewUrl: URL.createObjectURL(file) } - }) + if (file.type.startsWith('image/')) { + const { id } = await uploadRoomImage(roomId, file) + setPendingImage((prev) => { + if (prev) URL.revokeObjectURL(prev.previewUrl) + return { id, previewUrl: URL.createObjectURL(file) } + }) + } else { + const uploaded = await uploadRoomFile(roomId, file) + setPendingFile({ id: uploaded.id, filename: uploaded.filename, size: uploaded.size_bytes }) + } } catch (err) { setUploadError(err instanceof Error ? err.message : 'Upload failed') } finally { @@ -104,12 +119,34 @@ export function Composer({ roomId, roomName, disabled, onSend }: ComposerProps) )} + {pendingFile && ( +
+ + {pendingFile.filename} + {formatFileSize(pendingFile.size)} + +
+ )} {uploadError &&
{uploadError}
}
@@ -118,7 +155,7 @@ export function Composer({ roomId, roomName, disabled, onSend }: ComposerProps) className="composer-attach" onClick={() => fileInputRef.current?.click()} disabled={disabled || uploading} - aria-label="Attach an image" + aria-label="Attach a file" > {uploading ? ( + {msg.file.filename} + {formatFileSize(msg.file.size_bytes)} + + + )} {msg.content && (
diff --git a/frontend/src/types.ts b/frontend/src/types.ts index 757ef8b..de6bd1c 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -52,6 +52,13 @@ export interface ReactionSummary { user_ids: string[] } +export interface MessageFileInfo { + id: string + filename: string + size_bytes: number + content_type: string +} + export interface Message { id: string room_id: string @@ -59,6 +66,7 @@ export interface Message { username: string content: string | null image_id: string | null + file: MessageFileInfo | null reactions: ReactionSummary[] created_at: string edited_at: string | null @@ -72,6 +80,7 @@ export interface ChatMessageEnvelope { username: string content: string | null image_id: string | null + file: MessageFileInfo | null reactions: ReactionSummary[] created_at: string edited_at: string | null diff --git a/frontend/src/ws/useChatSocket.ts b/frontend/src/ws/useChatSocket.ts index ac7dc80..2817662 100644 --- a/frontend/src/ws/useChatSocket.ts +++ b/frontend/src/ws/useChatSocket.ts @@ -82,11 +82,17 @@ export function useChatSocket({ roomId, onMessage, onUnauthenticated }: UseChatS } }, [roomId]) - const send = useCallback((content: string, imageId?: string) => { + const send = useCallback((content: string, imageId?: string, fileId?: string) => { const ws = socketRef.current if (!ws || ws.readyState !== WebSocket.OPEN) return ws.send( - JSON.stringify({ type: 'message', room_id: roomId, content: content || null, image_id: imageId ?? null }), + JSON.stringify({ + type: 'message', + room_id: roomId, + content: content || null, + image_id: imageId ?? null, + file_id: fileId ?? null, + }), ) }, [roomId])