Private
Public Access
Add admin-configurable upload size limits
The 8MB image/file/avatar cap is now a site setting (UploadSettings, single-row table like SmtpSettings) editable from the Admin Settings tab, instead of a hardcoded constant. All three upload endpoints read the live value and interpolate it into their 413 messages. A new GET /api/uploads/limit endpoint (open to any authenticated user, unlike the admin-only settings endpoints) lets the composer reject an oversized file client-side before it ever hits the network, though the server still enforces the same cap independently. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
+29
-1
@@ -1,4 +1,4 @@
|
||||
# KeepItTalking backend (Phase 1 + 2 + 4 + 5 + 6 + 7 + 8, image uploads, file attachments, emoji & reactions, user profiles, site invites & email, password reset)
|
||||
# KeepItTalking backend (Phase 1 + 2 + 4 + 5 + 6 + 7 + 8, image uploads, file attachments, admin-configurable upload size limits, 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
|
||||
@@ -398,6 +398,34 @@ share them; `ImageTooLargeError` was likewise renamed to
|
||||
|
||||
Same orphaned-upload disk-space caveat as images applies here too.
|
||||
|
||||
## Upload size limits
|
||||
|
||||
The 8 MB image/file/avatar cap is no longer hardcoded — it's an
|
||||
admin-configurable site setting (`UploadSettings`, single-row table, same
|
||||
"fetch-or-create" convention as `SmtpSettings`), editable from the Admin
|
||||
portal's Settings tab. Unlike `SmtpSettings` (where "no row yet" means
|
||||
"unconfigured, skip"), a missing row here still needs a usable value, so
|
||||
`upload_settings_service.get_upload_settings` creates it with the 8 MB
|
||||
default (`storage.DEFAULT_MAX_UPLOAD_BYTES`) on first read instead of
|
||||
returning `None`.
|
||||
|
||||
- `GET`/`PUT /api/admin/settings/uploads` (site-admin only) — read/update
|
||||
the cap. Bounds-checked to 1–500 MB (`UploadSettingsUpdate`) to guard
|
||||
against a fat-fingered 0 or an unbounded figure that could exhaust disk.
|
||||
- `GET /api/uploads/limit` — unlike the admin endpoints, this one is open
|
||||
to any authenticated user (same `Depends(get_current_user)`-only pattern
|
||||
as `/api/push/vapid-public-key`), since every room member needs to know
|
||||
the cap, not just admins. The frontend composer fetches it once per
|
||||
mount and rejects an oversized file client-side before ever hitting the
|
||||
network; the server still enforces the same value independently via
|
||||
`read_capped(file, cap=...)`, so the client-side check is a UX nicety,
|
||||
not the actual security boundary.
|
||||
- All three upload endpoints (room image, room file, avatar) now call
|
||||
`get_upload_settings(db)` and pass the live value into `read_capped`
|
||||
instead of relying on a module-level constant; their 413 error messages
|
||||
interpolate the actual configured limit (`upload_settings_service.
|
||||
format_mb`) rather than a hardcoded "8 MB" string.
|
||||
|
||||
## Emoji & reactions
|
||||
|
||||
An emoji picker in the frontend composer is purely client-side (a static
|
||||
|
||||
@@ -0,0 +1,37 @@
|
||||
"""upload settings
|
||||
|
||||
Revision ID: 880f080648de
|
||||
Revises: 5b1a142dc271
|
||||
Create Date: 2026-08-15 20:35:19.357542
|
||||
|
||||
"""
|
||||
from typing import Sequence, Union
|
||||
|
||||
from alembic import op
|
||||
import sqlalchemy as sa
|
||||
|
||||
|
||||
# revision identifiers, used by Alembic.
|
||||
revision: str = '880f080648de'
|
||||
down_revision: Union[str, Sequence[str], None] = '5b1a142dc271'
|
||||
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('upload_settings',
|
||||
sa.Column('id', sa.Uuid(), nullable=False),
|
||||
sa.Column('max_upload_bytes', sa.BigInteger(), nullable=False),
|
||||
sa.Column('updated_at', sa.DateTime(timezone=True), server_default=sa.text('now()'), nullable=False),
|
||||
sa.PrimaryKeyConstraint('id')
|
||||
)
|
||||
# ### end Alembic commands ###
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
"""Downgrade schema."""
|
||||
# ### commands auto generated by Alembic - please adjust! ###
|
||||
op.drop_table('upload_settings')
|
||||
# ### end Alembic commands ###
|
||||
+2
-1
@@ -11,7 +11,7 @@ from redis.asyncio import Redis
|
||||
from starlette.middleware.sessions import SessionMiddleware
|
||||
|
||||
from app.config import settings
|
||||
from app.routers import admin, auth, bots, health, push, rooms, signup, users, webhooks
|
||||
from app.routers import admin, auth, bots, health, push, rooms, signup, uploads, users, webhooks
|
||||
from app.ws.broadcaster import RoomBroadcaster
|
||||
from app.ws.chat import router as ws_router
|
||||
from app.ws.connection_manager import ConnectionManager
|
||||
@@ -76,6 +76,7 @@ def create_app() -> FastAPI:
|
||||
app.include_router(rooms.router)
|
||||
app.include_router(users.router)
|
||||
app.include_router(push.router)
|
||||
app.include_router(uploads.router)
|
||||
app.include_router(admin.router)
|
||||
app.include_router(bots.router)
|
||||
app.include_router(webhooks.router)
|
||||
|
||||
@@ -13,6 +13,7 @@ from app.models.push_subscription import PushSubscription
|
||||
from app.models.room import Room
|
||||
from app.models.site_invite import SiteInvite
|
||||
from app.models.smtp_settings import SmtpSettings
|
||||
from app.models.upload_settings import UploadSettings
|
||||
from app.models.user import User
|
||||
from app.models.webhook_incoming import WebhookIncoming
|
||||
|
||||
@@ -30,6 +31,7 @@ __all__ = [
|
||||
"PasswordReset",
|
||||
"SiteInvite",
|
||||
"SmtpSettings",
|
||||
"UploadSettings",
|
||||
"PushSubscription",
|
||||
"AdminAuditLog",
|
||||
"ApiToken",
|
||||
|
||||
@@ -0,0 +1,21 @@
|
||||
import uuid
|
||||
from datetime import datetime
|
||||
|
||||
from sqlalchemy import BigInteger, DateTime, func
|
||||
from sqlalchemy.orm import Mapped, mapped_column
|
||||
|
||||
from app.models.base import Base
|
||||
|
||||
|
||||
class UploadSettings(Base):
|
||||
"""A single-row table (enforced in the service layer, not the schema --
|
||||
same convention as SmtpSettings) holding the site-wide max upload size
|
||||
for images and file attachments, set through the Admin UI."""
|
||||
|
||||
__tablename__ = "upload_settings"
|
||||
|
||||
id: Mapped[uuid.UUID] = mapped_column(primary_key=True, default=uuid.uuid4)
|
||||
max_upload_bytes: Mapped[int] = mapped_column(BigInteger, nullable=False)
|
||||
updated_at: Mapped[datetime] = mapped_column(
|
||||
DateTime(timezone=True), server_default=func.now(), onupdate=func.now(), nullable=False
|
||||
)
|
||||
@@ -16,6 +16,7 @@ from app.schemas.admin import (
|
||||
)
|
||||
from app.schemas.site_invite import SiteInviteCreate, SiteInviteRead
|
||||
from app.schemas.smtp_settings import SmtpSettingsRead, SmtpSettingsUpdate
|
||||
from app.schemas.upload_settings import UploadSettingsRead, UploadSettingsUpdate
|
||||
from app.schemas.webhook import EventSubscriptionAdminRead, WebhookIncomingAdminRead
|
||||
from app.services.admin_service import (
|
||||
CannotActOnSelfError,
|
||||
@@ -40,6 +41,7 @@ from app.services.site_invite_service import (
|
||||
revoke_site_invite,
|
||||
)
|
||||
from app.services.smtp_settings_service import get_smtp_settings, upsert_smtp_settings
|
||||
from app.services.upload_settings_service import get_upload_settings, update_upload_settings
|
||||
from app.services.webhook_service import (
|
||||
list_all_event_subscriptions_admin,
|
||||
list_all_incoming_webhooks_admin,
|
||||
@@ -379,3 +381,24 @@ async def test_smtp_settings_endpoint(
|
||||
raise HTTPException(status_code=400, detail="SMTP is not configured yet")
|
||||
except Exception as exc:
|
||||
raise HTTPException(status_code=502, detail=f"Failed to send test email: {exc}")
|
||||
|
||||
|
||||
@router.get("/settings/uploads", response_model=UploadSettingsRead)
|
||||
async def get_upload_settings_endpoint(
|
||||
current_user: User = Depends(get_current_user),
|
||||
db: AsyncSession = Depends(get_db),
|
||||
):
|
||||
require_site_admin(current_user)
|
||||
cfg = await get_upload_settings(db)
|
||||
return UploadSettingsRead(max_upload_bytes=cfg.max_upload_bytes, updated_at=cfg.updated_at)
|
||||
|
||||
|
||||
@router.put("/settings/uploads", response_model=UploadSettingsRead)
|
||||
async def update_upload_settings_endpoint(
|
||||
data: UploadSettingsUpdate,
|
||||
current_user: User = Depends(get_current_user),
|
||||
db: AsyncSession = Depends(get_db),
|
||||
):
|
||||
require_site_admin(current_user)
|
||||
cfg = await update_upload_settings(db, max_upload_bytes=data.max_upload_bytes)
|
||||
return UploadSettingsRead(max_upload_bytes=cfg.max_upload_bytes, updated_at=cfg.updated_at)
|
||||
|
||||
@@ -20,6 +20,7 @@ from app.services.password_service import (
|
||||
request_password_reset,
|
||||
validate_reset_token,
|
||||
)
|
||||
from app.services.upload_settings_service import format_mb, get_upload_settings
|
||||
from app.storage import (
|
||||
ALLOWED_IMAGE_CONTENT_TYPES,
|
||||
InvalidImageError,
|
||||
@@ -89,10 +90,14 @@ async def upload_avatar(
|
||||
if file.content_type not in ALLOWED_IMAGE_CONTENT_TYPES:
|
||||
raise HTTPException(status_code=400, detail="Unsupported image type")
|
||||
|
||||
upload_settings = await get_upload_settings(db)
|
||||
try:
|
||||
data = await read_capped(file)
|
||||
data = await read_capped(file, cap=upload_settings.max_upload_bytes)
|
||||
except UploadTooLargeError:
|
||||
raise HTTPException(status_code=413, detail="Image exceeds 8 MB limit")
|
||||
raise HTTPException(
|
||||
status_code=413,
|
||||
detail=f"Image exceeds {format_mb(upload_settings.max_upload_bytes)} limit",
|
||||
)
|
||||
|
||||
try:
|
||||
data, ext = process_image(
|
||||
|
||||
@@ -35,6 +35,7 @@ from app.schemas.webhook import (
|
||||
WebhookIncomingRead,
|
||||
)
|
||||
from app.services.message_service import get_reactions_for_messages, list_recent_messages
|
||||
from app.services.upload_settings_service import format_mb, get_upload_settings
|
||||
from app.services.room_service import (
|
||||
AlreadyMemberError,
|
||||
CannotRemoveOwnerError,
|
||||
@@ -73,7 +74,6 @@ 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,
|
||||
InvalidImageError,
|
||||
UploadTooLargeError,
|
||||
@@ -337,10 +337,14 @@ async def upload_room_image_endpoint(
|
||||
if file.content_type not in ALLOWED_IMAGE_CONTENT_TYPES:
|
||||
raise HTTPException(status_code=400, detail="Unsupported image type")
|
||||
|
||||
upload_settings = await get_upload_settings(db)
|
||||
try:
|
||||
data = await read_capped(file)
|
||||
data = await read_capped(file, cap=upload_settings.max_upload_bytes)
|
||||
except UploadTooLargeError:
|
||||
raise HTTPException(status_code=413, detail="Image exceeds 8 MB limit")
|
||||
raise HTTPException(
|
||||
status_code=413,
|
||||
detail=f"Image exceeds {format_mb(upload_settings.max_upload_bytes)} limit",
|
||||
)
|
||||
|
||||
try:
|
||||
data, ext = process_image(data, file.content_type)
|
||||
@@ -388,10 +392,14 @@ async def upload_room_file_endpoint(
|
||||
):
|
||||
await require_room_member(room_id, current_user, db)
|
||||
|
||||
upload_settings = await get_upload_settings(db)
|
||||
try:
|
||||
data = await read_capped(file, cap=MAX_FILE_BYTES)
|
||||
data = await read_capped(file, cap=upload_settings.max_upload_bytes)
|
||||
except UploadTooLargeError:
|
||||
raise HTTPException(status_code=413, detail="File exceeds 8 MB limit")
|
||||
raise HTTPException(
|
||||
status_code=413,
|
||||
detail=f"File exceeds {format_mb(upload_settings.max_upload_bytes)} limit",
|
||||
)
|
||||
|
||||
original_filename = file.filename or "file"
|
||||
ext = pathlib.Path(original_filename).suffix
|
||||
|
||||
@@ -0,0 +1,22 @@
|
||||
from fastapi import APIRouter, Depends
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from app.database import get_db
|
||||
from app.dependencies import get_current_user
|
||||
from app.models import User
|
||||
from app.schemas.upload_settings import UploadSettingsRead
|
||||
from app.services.upload_settings_service import get_upload_settings
|
||||
|
||||
router = APIRouter(prefix="/api/uploads", tags=["uploads"])
|
||||
|
||||
|
||||
@router.get("/limit", response_model=UploadSettingsRead)
|
||||
async def get_upload_limit_endpoint(
|
||||
current_user: User = Depends(get_current_user),
|
||||
db: AsyncSession = Depends(get_db),
|
||||
):
|
||||
"""Any authenticated user (not just site admins -- unlike
|
||||
/api/admin/settings/uploads) can read the current cap, so the composer
|
||||
can validate client-side before uploading."""
|
||||
cfg = await get_upload_settings(db)
|
||||
return UploadSettingsRead(max_upload_bytes=cfg.max_upload_bytes, updated_at=cfg.updated_at)
|
||||
@@ -0,0 +1,16 @@
|
||||
from datetime import datetime
|
||||
|
||||
from pydantic import BaseModel, Field
|
||||
|
||||
|
||||
class UploadSettingsRead(BaseModel):
|
||||
max_upload_bytes: int
|
||||
updated_at: datetime
|
||||
|
||||
|
||||
class UploadSettingsUpdate(BaseModel):
|
||||
# Guardrails against a fat-fingered 0/negative value or an unbounded
|
||||
# figure that could exhaust disk -- 1 MB to 500 MB is generous enough
|
||||
# for any real attachment while still being a sane range to type into
|
||||
# a number input.
|
||||
max_upload_bytes: int = Field(ge=1024 * 1024, le=500 * 1024 * 1024)
|
||||
@@ -0,0 +1,33 @@
|
||||
from sqlalchemy import select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from app.models import UploadSettings
|
||||
from app.storage import DEFAULT_MAX_UPLOAD_BYTES
|
||||
|
||||
|
||||
async def get_upload_settings(db: AsyncSession) -> UploadSettings:
|
||||
"""Unlike SmtpSettings (where "no row yet" means "unconfigured, skip"),
|
||||
an upload cap must always resolve to a usable value -- so this creates
|
||||
the row with the default on first read instead of returning None."""
|
||||
result = await db.execute(select(UploadSettings).limit(1))
|
||||
settings_row = result.scalar_one_or_none()
|
||||
if settings_row is None:
|
||||
settings_row = UploadSettings(max_upload_bytes=DEFAULT_MAX_UPLOAD_BYTES)
|
||||
db.add(settings_row)
|
||||
await db.commit()
|
||||
await db.refresh(settings_row)
|
||||
return settings_row
|
||||
|
||||
|
||||
async def update_upload_settings(db: AsyncSession, *, max_upload_bytes: int) -> UploadSettings:
|
||||
settings_row = await get_upload_settings(db)
|
||||
settings_row.max_upload_bytes = max_upload_bytes
|
||||
await db.commit()
|
||||
await db.refresh(settings_row)
|
||||
return settings_row
|
||||
|
||||
|
||||
def format_mb(num_bytes: int) -> str:
|
||||
"""Used in 413 error messages, which need to reflect the live
|
||||
admin-configured cap rather than a hardcoded "8 MB"."""
|
||||
return f"{num_bytes / (1024 * 1024):g} MB"
|
||||
@@ -10,11 +10,10 @@ from PIL import Image, UnidentifiedImageError
|
||||
# production layout with zero new config.
|
||||
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
|
||||
# Seed value for the admin-configurable UploadSettings row (see
|
||||
# app/services/upload_settings_service.py) -- also the fallback `read_capped`
|
||||
# default for call sites that don't look up the live setting.
|
||||
DEFAULT_MAX_UPLOAD_BYTES = 8 * 1024 * 1024
|
||||
_READ_CHUNK_BYTES = 1024 * 1024
|
||||
_MAX_DIMENSION = 2000
|
||||
|
||||
@@ -35,7 +34,7 @@ class InvalidImageError(Exception):
|
||||
pass
|
||||
|
||||
|
||||
async def read_capped(file, cap: int = MAX_IMAGE_BYTES) -> bytes:
|
||||
async def read_capped(file, cap: int = DEFAULT_MAX_UPLOAD_BYTES) -> bytes:
|
||||
"""Reads an UploadFile-like object in chunks, raising as soon as `cap`
|
||||
is exceeded rather than after buffering the whole (potentially huge)
|
||||
body first."""
|
||||
|
||||
@@ -0,0 +1,113 @@
|
||||
import uuid
|
||||
|
||||
from sqlalchemy import select
|
||||
|
||||
from app.models import UploadSettings, User
|
||||
from tests.conftest import register_and_login
|
||||
|
||||
|
||||
async def _get_settings_row(db_session) -> UploadSettings:
|
||||
result = await db_session.execute(select(UploadSettings))
|
||||
return result.scalar_one()
|
||||
|
||||
|
||||
async def _make_admin(db_session, user_id: str) -> None:
|
||||
user = await db_session.get(User, uuid.UUID(user_id))
|
||||
user.is_site_admin = True
|
||||
await db_session.commit()
|
||||
|
||||
|
||||
async def test_upload_settings_require_admin(client, db_session):
|
||||
await register_and_login(client, db_session, username="alice")
|
||||
resp = await client.get("/api/admin/settings/uploads")
|
||||
assert resp.status_code == 403
|
||||
|
||||
resp = await client.put("/api/admin/settings/uploads", json={"max_upload_bytes": 1024 * 1024})
|
||||
assert resp.status_code == 403
|
||||
|
||||
|
||||
async def test_upload_settings_get_defaults_to_8mb(client, db_session):
|
||||
admin = await register_and_login(client, db_session, username="admin1")
|
||||
await _make_admin(db_session, admin["id"])
|
||||
|
||||
resp = await client.get("/api/admin/settings/uploads")
|
||||
assert resp.status_code == 200
|
||||
assert resp.json()["max_upload_bytes"] == 8 * 1024 * 1024
|
||||
|
||||
|
||||
async def test_upload_settings_update(client, db_session):
|
||||
admin = await register_and_login(client, db_session, username="admin1")
|
||||
await _make_admin(db_session, admin["id"])
|
||||
|
||||
resp = await client.put("/api/admin/settings/uploads", json={"max_upload_bytes": 20 * 1024 * 1024})
|
||||
assert resp.status_code == 200
|
||||
assert resp.json()["max_upload_bytes"] == 20 * 1024 * 1024
|
||||
|
||||
resp = await client.get("/api/admin/settings/uploads")
|
||||
assert resp.json()["max_upload_bytes"] == 20 * 1024 * 1024
|
||||
|
||||
row = await _get_settings_row(db_session)
|
||||
assert row.max_upload_bytes == 20 * 1024 * 1024
|
||||
|
||||
|
||||
async def test_upload_settings_rejects_out_of_range(client, db_session):
|
||||
admin = await register_and_login(client, db_session, username="admin1")
|
||||
await _make_admin(db_session, admin["id"])
|
||||
|
||||
resp = await client.put("/api/admin/settings/uploads", json={"max_upload_bytes": 0})
|
||||
assert resp.status_code == 422
|
||||
|
||||
resp = await client.put(
|
||||
"/api/admin/settings/uploads", json={"max_upload_bytes": 1000 * 1024 * 1024}
|
||||
)
|
||||
assert resp.status_code == 422
|
||||
|
||||
|
||||
async def test_upload_limit_endpoint_accessible_to_any_authenticated_user(client, db_session):
|
||||
admin = await register_and_login(client, db_session, username="admin1")
|
||||
await _make_admin(db_session, admin["id"])
|
||||
await client.put("/api/admin/settings/uploads", json={"max_upload_bytes": 5 * 1024 * 1024})
|
||||
|
||||
await register_and_login(client, db_session, username="regular")
|
||||
resp = await client.get("/api/uploads/limit")
|
||||
assert resp.status_code == 200
|
||||
assert resp.json()["max_upload_bytes"] == 5 * 1024 * 1024
|
||||
|
||||
|
||||
async def test_lowering_limit_enforced_on_file_upload(client, db_session):
|
||||
admin = await register_and_login(client, db_session, username="admin1")
|
||||
await _make_admin(db_session, admin["id"])
|
||||
await client.put("/api/admin/settings/uploads", json={"max_upload_bytes": 1024 * 1024})
|
||||
|
||||
room = (await client.post("/api/rooms", json={"name": "general"})).json()
|
||||
|
||||
resp = await client.post(
|
||||
f"/api/rooms/{room['id']}/files",
|
||||
files={"file": ("big.bin", b"0" * (2 * 1024 * 1024), "application/octet-stream")},
|
||||
)
|
||||
assert resp.status_code == 413
|
||||
assert "1 MB" in resp.json()["detail"]
|
||||
|
||||
resp = await client.post(
|
||||
f"/api/rooms/{room['id']}/files",
|
||||
files={"file": ("small.bin", b"0" * (512 * 1024), "application/octet-stream")},
|
||||
)
|
||||
assert resp.status_code == 201
|
||||
|
||||
|
||||
async def test_raising_limit_allows_larger_image_upload(client, db_session):
|
||||
admin = await register_and_login(client, db_session, username="admin1")
|
||||
await _make_admin(db_session, admin["id"])
|
||||
await client.put("/api/admin/settings/uploads", json={"max_upload_bytes": 20 * 1024 * 1024})
|
||||
|
||||
room = (await client.post("/api/rooms", json={"name": "general"})).json()
|
||||
|
||||
# 9MB would be rejected under the old hardcoded 8MB cap.
|
||||
oversized_for_old_cap = b"0" * (9 * 1024 * 1024)
|
||||
resp = await client.post(
|
||||
f"/api/rooms/{room['id']}/images",
|
||||
files={"file": ("big.bin", oversized_for_old_cap, "image/png")},
|
||||
)
|
||||
# Not a valid PNG, but it must get past the size check to prove the
|
||||
# raised limit took effect -- 400 (invalid image), not 413.
|
||||
assert resp.status_code == 400
|
||||
Reference in New Issue
Block a user