Merge pull request 'fix(tradein/security): photo-upload — чанковое чтение с капом 10MB + Caddy request_body 12MB (#2233)' (#2258) from fix/tradein-photo-upload-stream-cap into main
All checks were successful
Deploy Trade-In / changes (push) Successful in 12s
Deploy Trade-In / build-browser (push) Successful in 38s
Deploy Trade-In / build-frontend (push) Successful in 40s
Deploy Trade-In / test (push) Successful in 1m48s
Deploy Trade-In / build-backend (push) Successful in 58s
Deploy Trade-In / deploy (push) Successful in 56s

This commit is contained in:
lekss361 2026-07-03 06:15:57 +00:00
commit 910bfed5f1
3 changed files with 81 additions and 3 deletions

View file

@ -518,11 +518,19 @@ async def upload_photo(
status_code=409, detail=f"photo limit reached ({_MAX_PHOTOS_PER_ESTIMATE})" status_code=409, detail=f"photo limit reached ({_MAX_PHOTOS_PER_ESTIMATE})"
) )
content = await file.read() # #2233: читаем тело чанками с жёстким капом, а не await file.read() целиком —
# иначе multi-GB аплоад буферизуется в RAM и OOM-killed backend (mem_limit 768m, #2214).
# 413 бросается СРАЗУ при превышении, остаток тела запроса НЕ читается.
chunks: list[bytes] = []
total = 0
while chunk := await file.read(64 * 1024):
total += len(chunk)
if total > _MAX_PHOTO_BYTES:
raise HTTPException(status_code=413, detail="file too large (max 10 MB)")
chunks.append(chunk)
content = b"".join(chunks)
if not content: if not content:
raise HTTPException(status_code=400, detail="empty file") raise HTTPException(status_code=400, detail="empty file")
if len(content) > _MAX_PHOTO_BYTES:
raise HTTPException(status_code=413, detail="file too large (max 10 MB)")
# Sanitize: re-encode through Pillow to drop EXIF, kill polyglot payloads, # Sanitize: re-encode through Pillow to drop EXIF, kill polyglot payloads,
# cap dimensions. Closes finding #6 from 2026-05-24 audit. # cap dimensions. Closes finding #6 from 2026-05-24 audit.

View file

@ -491,6 +491,69 @@ def test_upload_photo_owner_can_upload(trade_in_app: FastAPI, monkeypatch) -> No
assert resp.status_code == 200 assert resp.status_code == 200
class _InfiniteUploadFile:
"""UploadFile stub whose async read(size) yields endless 64KB chunks (#2233).
Simulates a multi-GB stream. Counts read() calls so the test can assert the
handler stops reading right after the cap is exceeded instead of draining the
whole body into RAM.
"""
content_type = "image/png"
filename = "huge.png"
def __init__(self) -> None:
self.read_calls = 0
async def read(self, size: int = -1) -> bytes:
self.read_calls += 1
return b"\x00" * (64 * 1024)
async def test_upload_photo_streams_over_cap_returns_413_without_full_read() -> None:
"""#2233: a stream larger than 10 MB → 413 raised early, body NOT fully read.
Acceptance: read() is called only ~(10MB/64KB)+1 times (161), proving the loop
bails on the first chunk that pushes total past _MAX_PHOTO_BYTES rather than
buffering an unbounded upload (which would OOM the 768m-capped container, #2214).
"""
from uuid import UUID
from fastapi import HTTPException
import app.core.auth as auth_mod
from app.api.v1.trade_in import _MAX_PHOTO_BYTES, upload_photo
auth_mod.get_role = lambda _u: "pilot" # type: ignore[assignment]
# guard SELECT → owner; count SELECT → 0. INSERT must never be reached.
db = MagicMock()
guard_result = MagicMock()
guard_result.fetchone.return_value = SimpleNamespace(created_by="kopylov")
count_result = MagicMock()
count_result.scalar_one.return_value = 0
db.execute.side_effect = [guard_result, count_result]
upload = _InfiniteUploadFile()
with pytest.raises(HTTPException) as exc_info:
await upload_photo(
estimate_id=UUID(_ESTIMATE_ID),
db=db,
file=upload, # type: ignore[arg-type]
x_authenticated_user="kopylov",
)
assert exc_info.value.status_code == 413
expected_reads = _MAX_PHOTO_BYTES // (64 * 1024) + 1 # 161
assert upload.read_calls == expected_reads
# sanity: bounded, nowhere near infinite
assert upload.read_calls < 200
# INSERT (3rd execute) never fired — nothing persisted
assert db.execute.call_count == 2
db.commit.assert_not_called()
# ── Derived analytics routes (representative: /houses, /imv-benchmark) ──────── # ── Derived analytics routes (representative: /houses, /imv-benchmark) ────────

View file

@ -15,6 +15,13 @@
handle_path /trade-in/api/* { handle_path /trade-in/api/* {
# handle_path вырезает префикс /trade-in перед прокси → # handle_path вырезает префикс /trade-in перед прокси →
# FastAPI получает /api/v1/... # FastAPI получает /api/v1/...
#
# #2233: первый рубеж против OOM на фото-аплоаде — режем тело >12MB на
# уровне Caddy (backend капит на 10MB чанково, см. upload_photo). Scoped
# ТОЛЬКО на /trade-in/api/* — не трогает фронт/остальной сайт.
request_body {
max_size 12MB
}
reverse_proxy tradein-backend:8000 reverse_proxy tradein-backend:8000
} }