From c383d9399a93d7372089c395c9d9490ff8c578df Mon Sep 17 00:00:00 2001 From: Anshul Jain Date: Sat, 11 Jul 2026 18:24:38 +0530 Subject: [PATCH 1/5] fix: make image pixel-limit validation thread-safe Replace process-wide Image.MAX_IMAGE_PIXELS mutation with request-local dimension validation. The shared mutable global is not thread-safe when concurrent uploads run in a thread pool: overlapping requests could weaken decompression-bomb protection if one request's limit overlaps with another's concurrent validation window. Changes: - Add MAX_IMAGE_PIXELS to config (100M pixels default, configurable) - Check dimensions locally in _ingest_image without mutating Pillow globals - Provide clear error message when pixel limit is exceeded - Keep verify() call for corruption detection Concurrency safety ensured: each request checks dimensions independently against the config value before any Pillow decompression occurs. Fixes #289 --- backend/src/find_api/core/config.py | 1 + backend/src/find_api/routers/upload.py | 21 ++++++++++++++------- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/backend/src/find_api/core/config.py b/backend/src/find_api/core/config.py index 2301799d..598a1092 100644 --- a/backend/src/find_api/core/config.py +++ b/backend/src/find_api/core/config.py @@ -64,6 +64,7 @@ class Settings(BaseSettings): MAX_BULK_FILES: int = 200 MAX_BULK_TOTAL_SIZE_MB: int = 500 MAX_BULK_COMPRESSION_RATIO: int = 100 + MAX_IMAGE_PIXELS: int = 100_000_000 WORKER_TIMEOUT: int = 600 # Trashed assets older than this many days are eligible for permanent # auto-purge (via POST /trash/purge). 0 disables age-based purging. diff --git a/backend/src/find_api/routers/upload.py b/backend/src/find_api/routers/upload.py index 760f0ba2..1e3fce58 100644 --- a/backend/src/find_api/routers/upload.py +++ b/backend/src/find_api/routers/upload.py @@ -228,16 +228,23 @@ def _ingest_image( if not detected_type.startswith("image/"): raise HTTPException(400, f"File {filename} is not an image") - # Verify image content and protect against decompression bombs + # Verify image content and protect against decompression bombs using + # request-local dimension validation instead of mutating process-wide + # Image.MAX_IMAGE_PIXELS, which is not thread-safe when concurrent + # uploads run in a thread pool (overlapping requests could weaken + # decompression-bomb protection). This check is done request-locally. try: - # Set a reasonable limit for image pixels (e.g., 100MP) - Image.MAX_IMAGE_PIXELS = 100_000_000 with Image.open(io.BytesIO(file_data)) as img: + width, height = img.size + pixel_count = width * height + if pixel_count > settings.MAX_IMAGE_PIXELS: + raise HTTPException( + 400, + f"File {filename} exceeds pixel limit ({pixel_count:,} > {settings.MAX_IMAGE_PIXELS:,})", + ) img.verify() - # Re-open to check dimensions (verify() consumes the file pointer) - # This is still lazy and doesn't decode pixels. - with Image.open(io.BytesIO(file_data)) as img2: - _ = img2.size + except HTTPException: + raise except Exception: raise HTTPException(400, f"File {filename} is corrupted or not a valid image") From d2329a22c120eea043c94f391f95fae4453b0c18 Mon Sep 17 00:00:00 2001 From: Anshul Jain Date: Sun, 12 Jul 2026 11:46:37 +0530 Subject: [PATCH 2/5] Add pixel validation tests: normal, oversized, concurrent Add three test methods to TestPixelLimitValidation: - test_pixel_limit_normal_image(): Verifies normal-sized images pass validation - test_pixel_limit_oversized_image(): Confirms oversized images are rejected when exceeding the configured limit - test_pixel_limit_concurrent_validation(): Ensures concurrent uploads validate independently without global mutation These tests verify the thread-safe pixel validation approach addresses the maintainer's feedback about Pillow's built-in ceiling. Signed-off-by: Anshul Jain --- backend/tests/test_upload.py | 64 ++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/backend/tests/test_upload.py b/backend/tests/test_upload.py index 741aa5fa..e54ef4ff 100644 --- a/backend/tests/test_upload.py +++ b/backend/tests/test_upload.py @@ -1,3 +1,4 @@ +import concurrent.futures import io import os import zipfile @@ -114,6 +115,69 @@ def test_missing_files_returns_422(self, client): assert response.status_code == 422 +class TestPixelLimitValidation: + """Image pixel-limit validation (thread-safe, Pillow ceiling-aware).""" + + def test_pixel_limit_normal_image(self, client): + """Normal image within pixel limit should succeed.""" + img = Image.new("RGB", (100, 100), color="red") + buf = io.BytesIO() + img.save(buf, format="PNG") + buf.seek(0) + + response = client.post( + "/api/upload", + files=[("files", ("normal.png", buf.getvalue(), "image/png"))], + ) + assert response.status_code == 200 + assert response.json()["results"][0]["status"] == "uploaded" + + def test_pixel_limit_oversized_image(self, client): + """Image exceeding MAX_IMAGE_PIXELS should be rejected.""" + from find_api.settings import settings + + with patch("find_api.routers.upload.settings.MAX_IMAGE_PIXELS", 100): + img = Image.new("RGB", (50, 50), color="blue") + buf = io.BytesIO() + img.save(buf, format="PNG") + buf.seek(0) + + response = client.post( + "/api/upload", + files=[("files", ("oversized.png", buf.getvalue(), "image/png"))], + ) + assert response.status_code == 400 + assert "exceeds pixel limit" in response.json()["detail"].lower() + + def test_pixel_limit_concurrent_validation(self, client): + """Concurrent uploads should each validate independently without global mutation.""" + import concurrent.futures + + def upload_image(size): + img = Image.new("RGB", (size, size), color="green") + buf = io.BytesIO() + img.save(buf, format="PNG") + buf.seek(0) + return client.post( + "/api/upload", + files=[ + ( + "files", + (f"image_{size}.png", buf.getvalue(), "image/png"), + ) + ], + ) + + with concurrent.futures.ThreadPoolExecutor(max_workers=3) as executor: + futures = [executor.submit(upload_image, size) for size in [10, 50, 100]] + results = [f.result() for f in concurrent.futures.as_completed(futures)] + + assert all(r.status_code == 200 for r in results) + assert all( + r.json()["results"][0]["status"] == "uploaded" for r in results + ) + + class TestBulkUpload: """Bulk ZIP upload behavior.""" From 4694731aa17d87726bd4fdc714c64f696218aa86 Mon Sep 17 00:00:00 2001 From: Abhash Chakraborty <80592559+Abhash-Chakraborty@users.noreply.github.com> Date: Mon, 13 Jul 2026 01:31:55 +0530 Subject: [PATCH 3/5] fix: align upload pixel limits with Pillow safety --- backend/src/find_api/core/config.py | 19 ++++++++++++++++++- backend/src/find_api/routers/upload.py | 2 ++ backend/tests/test_upload.py | 15 +++++++++------ 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/backend/src/find_api/core/config.py b/backend/src/find_api/core/config.py index 598a1092..4e36140a 100644 --- a/backend/src/find_api/core/config.py +++ b/backend/src/find_api/core/config.py @@ -4,10 +4,15 @@ import os from typing import Literal, Optional + +from PIL import Image from pydantic import field_validator, model_validator from pydantic_settings import BaseSettings, SettingsConfigDict +PILLOW_MAX_IMAGE_PIXELS = Image.MAX_IMAGE_PIXELS or 89_478_485 + + class Settings(BaseSettings): """Application settings""" @@ -64,7 +69,7 @@ class Settings(BaseSettings): MAX_BULK_FILES: int = 200 MAX_BULK_TOTAL_SIZE_MB: int = 500 MAX_BULK_COMPRESSION_RATIO: int = 100 - MAX_IMAGE_PIXELS: int = 100_000_000 + MAX_IMAGE_PIXELS: int = PILLOW_MAX_IMAGE_PIXELS WORKER_TIMEOUT: int = 600 # Trashed assets older than this many days are eligible for permanent # auto-purge (via POST /trash/purge). 0 disables age-based purging. @@ -99,6 +104,7 @@ class Settings(BaseSettings): "ML_MAX_LOADED_MODELS", "SESSION_TTL_HOURS", "INVITE_TTL_HOURS", + "MAX_IMAGE_PIXELS", ) @classmethod def validate_positive_int(cls, value: int, info): @@ -107,6 +113,17 @@ def validate_positive_int(cls, value: int, info): raise ValueError(f"{info.field_name} must be greater than 0") return value + @field_validator("MAX_IMAGE_PIXELS") + @classmethod + def validate_image_pixel_ceiling(cls, value: int): + """Keep the request-local cap at or below Pillow's process ceiling.""" + if value > PILLOW_MAX_IMAGE_PIXELS: + raise ValueError( + "MAX_IMAGE_PIXELS cannot exceed Pillow's built-in safety ceiling " + f"of {PILLOW_MAX_IMAGE_PIXELS} pixels" + ) + return value + @model_validator(mode="after") def validate_remote_ml_config(self): """Require remote ML settings when remote mode is enabled.""" diff --git a/backend/src/find_api/routers/upload.py b/backend/src/find_api/routers/upload.py index 1e3fce58..0087a445 100644 --- a/backend/src/find_api/routers/upload.py +++ b/backend/src/find_api/routers/upload.py @@ -245,6 +245,8 @@ def _ingest_image( img.verify() except HTTPException: raise + except Image.DecompressionBombError: + raise HTTPException(400, f"File {filename} exceeds the safe pixel limit") except Exception: raise HTTPException(400, f"File {filename} is corrupted or not a valid image") diff --git a/backend/tests/test_upload.py b/backend/tests/test_upload.py index e54ef4ff..55a393d8 100644 --- a/backend/tests/test_upload.py +++ b/backend/tests/test_upload.py @@ -4,7 +4,9 @@ import zipfile from unittest.mock import patch +import pytest from PIL import Image +from find_api.core.config import PILLOW_MAX_IMAGE_PIXELS, Settings from find_api.models.media import Media @@ -134,8 +136,6 @@ def test_pixel_limit_normal_image(self, client): def test_pixel_limit_oversized_image(self, client): """Image exceeding MAX_IMAGE_PIXELS should be rejected.""" - from find_api.settings import settings - with patch("find_api.routers.upload.settings.MAX_IMAGE_PIXELS", 100): img = Image.new("RGB", (50, 50), color="blue") buf = io.BytesIO() @@ -151,7 +151,7 @@ def test_pixel_limit_oversized_image(self, client): def test_pixel_limit_concurrent_validation(self, client): """Concurrent uploads should each validate independently without global mutation.""" - import concurrent.futures + pillow_limit = Image.MAX_IMAGE_PIXELS def upload_image(size): img = Image.new("RGB", (size, size), color="green") @@ -173,9 +173,12 @@ def upload_image(size): results = [f.result() for f in concurrent.futures.as_completed(futures)] assert all(r.status_code == 200 for r in results) - assert all( - r.json()["results"][0]["status"] == "uploaded" for r in results - ) + assert all(r.json()["results"][0]["status"] == "uploaded" for r in results) + assert Image.MAX_IMAGE_PIXELS == pillow_limit + + def test_config_rejects_limit_above_pillow_ceiling(self): + with pytest.raises(ValueError, match="Pillow's built-in safety ceiling"): + Settings(MAX_IMAGE_PIXELS=PILLOW_MAX_IMAGE_PIXELS + 1) class TestBulkUpload: From 9d2504f00101cc045a479c2f51fc4cda6daa64ae Mon Sep 17 00:00:00 2001 From: Abhash Chakraborty <80592559+Abhash-Chakraborty@users.noreply.github.com> Date: Mon, 13 Jul 2026 01:46:43 +0530 Subject: [PATCH 4/5] test: isolate concurrent pixel validation --- backend/src/find_api/routers/upload.py | 38 +++++++++++++------------- backend/tests/test_upload.py | 22 ++++++--------- 2 files changed, 27 insertions(+), 33 deletions(-) diff --git a/backend/src/find_api/routers/upload.py b/backend/src/find_api/routers/upload.py index 0087a445..25038eda 100644 --- a/backend/src/find_api/routers/upload.py +++ b/backend/src/find_api/routers/upload.py @@ -214,25 +214,8 @@ def _get_zip_member_basename(member_name: str) -> str: return member_name.replace("\\", "/").split("/")[-1] -def _ingest_image( - *, - filename: str, - content_type: Optional[str], - file_data: bytes, - db: Session, - uploader_user_id: Optional[int] = None, -) -> dict: - """Create or reuse a media record from raw image bytes""" - detected_type = content_type or mimetypes.guess_type(filename)[0] or "" - - if not detected_type.startswith("image/"): - raise HTTPException(400, f"File {filename} is not an image") - - # Verify image content and protect against decompression bombs using - # request-local dimension validation instead of mutating process-wide - # Image.MAX_IMAGE_PIXELS, which is not thread-safe when concurrent - # uploads run in a thread pool (overlapping requests could weaken - # decompression-bomb protection). This check is done request-locally. +def _verify_image_content(filename: str, file_data: bytes) -> None: + """Validate image bytes without mutating Pillow process-wide state.""" try: with Image.open(io.BytesIO(file_data)) as img: width, height = img.size @@ -250,6 +233,23 @@ def _ingest_image( except Exception: raise HTTPException(400, f"File {filename} is corrupted or not a valid image") + +def _ingest_image( + *, + filename: str, + content_type: Optional[str], + file_data: bytes, + db: Session, + uploader_user_id: Optional[int] = None, +) -> dict: + """Create or reuse a media record from raw image bytes""" + detected_type = content_type or mimetypes.guess_type(filename)[0] or "" + + if not detected_type.startswith("image/"): + raise HTTPException(400, f"File {filename} is not an image") + + _verify_image_content(filename, file_data) + file_size = len(file_data) max_size = settings.MAX_UPLOAD_SIZE_MB * 1024 * 1024 diff --git a/backend/tests/test_upload.py b/backend/tests/test_upload.py index 55a393d8..a7349acc 100644 --- a/backend/tests/test_upload.py +++ b/backend/tests/test_upload.py @@ -8,6 +8,7 @@ from PIL import Image from find_api.core.config import PILLOW_MAX_IMAGE_PIXELS, Settings from find_api.models.media import Media +from find_api.routers.upload import _verify_image_content def get_valid_image_bytes(): @@ -153,27 +154,20 @@ def test_pixel_limit_concurrent_validation(self, client): """Concurrent uploads should each validate independently without global mutation.""" pillow_limit = Image.MAX_IMAGE_PIXELS - def upload_image(size): + def validate_image(size): img = Image.new("RGB", (size, size), color="green") buf = io.BytesIO() img.save(buf, format="PNG") - buf.seek(0) - return client.post( - "/api/upload", - files=[ - ( - "files", - (f"image_{size}.png", buf.getvalue(), "image/png"), - ) - ], - ) + _verify_image_content(f"image_{size}.png", buf.getvalue()) + return size with concurrent.futures.ThreadPoolExecutor(max_workers=3) as executor: - futures = [executor.submit(upload_image, size) for size in [10, 50, 100]] + futures = [ + executor.submit(validate_image, size) for size in [10, 50, 100] + ] results = [f.result() for f in concurrent.futures.as_completed(futures)] - assert all(r.status_code == 200 for r in results) - assert all(r.json()["results"][0]["status"] == "uploaded" for r in results) + assert sorted(results) == [10, 50, 100] assert Image.MAX_IMAGE_PIXELS == pillow_limit def test_config_rejects_limit_above_pillow_ceiling(self): From 54de6f9c0fa3111a0e06ebe3d97f32c13fcff1c0 Mon Sep 17 00:00:00 2001 From: Abhash Chakraborty <80592559+Abhash-Chakraborty@users.noreply.github.com> Date: Mon, 13 Jul 2026 01:47:11 +0530 Subject: [PATCH 5/5] style: format pixel validation changes --- backend/tests/test_upload.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/backend/tests/test_upload.py b/backend/tests/test_upload.py index a7349acc..44448ac9 100644 --- a/backend/tests/test_upload.py +++ b/backend/tests/test_upload.py @@ -162,9 +162,7 @@ def validate_image(size): return size with concurrent.futures.ThreadPoolExecutor(max_workers=3) as executor: - futures = [ - executor.submit(validate_image, size) for size in [10, 50, 100] - ] + futures = [executor.submit(validate_image, size) for size in [10, 50, 100]] results = [f.result() for f in concurrent.futures.as_completed(futures)] assert sorted(results) == [10, 50, 100]