From 48e69f276abc1943e2f28915e41b899743376863 Mon Sep 17 00:00:00 2001 From: andyne13 Date: Mon, 29 Jun 2026 22:42:29 +0200 Subject: [PATCH 1/5] fix(security): bound upload size to prevent disk/memory exhaustion Backport of 8ca8561e (v1.1.x hardening, M8 family) lost in the hexagonal refactor. save_file_to_disk streamed uploads with no byte cap; the per-user quota limits file count, not bytes, so one request could write an arbitrarily large file and exhaust disk/RAM. Enforce a configurable max (MAX_UPLOAD_SIZE_MB, default 1024) during streaming, returning 413 and removing the partial file when exceeded. --- infra/compose/.env.example | 3 +++ openrag/api/dependencies/files.py | 30 ++++++++++++++++++----- tests/unit/api/dependencies/test_files.py | 15 ++++++++++++ 3 files changed, 42 insertions(+), 6 deletions(-) diff --git a/infra/compose/.env.example b/infra/compose/.env.example index 5fdc317fa..11fe10fa8 100644 --- a/infra/compose/.env.example +++ b/infra/compose/.env.example @@ -43,6 +43,9 @@ VLM_MODEL= # SAVE_UPLOADED_FILES=true # usefull for chainlit (chat interface) source viewing +# Maximum accepted upload size in MB (0 or negative = unlimited). Default 1024. +# MAX_UPLOAD_SIZE_MB=1024 + # Set to true, it will mount chainlit chat ui to the fastapi app (Default: true) ## WITH_CHAINLIT_UI=true diff --git a/openrag/api/dependencies/files.py b/openrag/api/dependencies/files.py index ab97d0023..5f7c2624d 100644 --- a/openrag/api/dependencies/files.py +++ b/openrag/api/dependencies/files.py @@ -1,3 +1,4 @@ +import os from pathlib import Path from typing import Any @@ -11,6 +12,11 @@ FORBIDDEN_CHARS_IN_FILE_ID = set("/") +# Maximum accepted upload size. Streamed writes are bounded so a single request +# cannot exhaust disk/RAM (the per-user quota limits file count, not bytes). +# 0 or negative disables the limit. Override with MAX_UPLOAD_SIZE_MB. +MAX_UPLOAD_SIZE_BYTES = int(os.getenv("MAX_UPLOAD_SIZE_MB", "1024")) * 1024 * 1024 + async def validate_file_id(file_id: str): return core_validators.validate_file_id(file_id, FORBIDDEN_CHARS_IN_FILE_ID) @@ -58,11 +64,23 @@ async def save_file_to_disk( except ValueError: raise ValidationError("Uploaded filename resolves outside destination directory.", status_code=400) - async with aiofiles.open(file_path, "wb") as buffer: - while True: - chunk = await file.read(chunk_size) - if not chunk: - break - await buffer.write(chunk) + total = 0 + try: + async with aiofiles.open(file_path, "wb") as buffer: + while True: + chunk = await file.read(chunk_size) + if not chunk: + break + total += len(chunk) + if MAX_UPLOAD_SIZE_BYTES > 0 and total > MAX_UPLOAD_SIZE_BYTES: + raise ValidationError( + f"File exceeds the maximum allowed size of {MAX_UPLOAD_SIZE_BYTES // (1024 * 1024)} MB.", + status_code=413, + ) + await buffer.write(chunk) + except ValidationError: + # Remove the partially written file before propagating. + file_path.unlink(missing_ok=True) + raise return file_path diff --git a/tests/unit/api/dependencies/test_files.py b/tests/unit/api/dependencies/test_files.py index 1d930337a..86afe14df 100644 --- a/tests/unit/api/dependencies/test_files.py +++ b/tests/unit/api/dependencies/test_files.py @@ -30,6 +30,21 @@ async def test_save_file_to_disk_writes_content(tmp_path: Path): assert saved_content == content +@pytest.mark.asyncio +async def test_save_file_to_disk_rejects_oversize_upload(tmp_path: Path, monkeypatch): + # Cap at ~8 bytes; a larger upload must be rejected (413) and not left on disk. + monkeypatch.setattr("api.dependencies.files.MAX_UPLOAD_SIZE_BYTES", 8) + upload = UploadFile(file=io.BytesIO(b"x" * 100), filename="big.bin") + dest_dir = tmp_path / "uploads" + + with pytest.raises(ValidationError) as exc: + await save_file_to_disk(file=upload, dest_dir=dest_dir, chunk_size=4) + + assert exc.value.status_code == 413 + # The partially written file must have been cleaned up. + assert not (dest_dir / "big.bin").exists() + + @pytest.mark.asyncio async def test_save_file_to_disk_with_random_prefix(tmp_path, monkeypatch): def fake_make_unique_filename(filename: str) -> str: From ce5842c57a206573d34fea5fae02068dcd80eb26 Mon Sep 17 00:00:00 2001 From: andyne13 Date: Mon, 29 Jun 2026 22:42:29 +0200 Subject: [PATCH 2/5] fix(security): bound parser fan-out/page counts; fix DOCX memory bomb Backport of 221f8ed8 (M8). The hexagonal refactor dropped the parser-bomb caps and reintroduced the attacker-controlled allocation the original fix removed: DocxParser._extract_embedded_images built [None]*max_order where max_order is parsed from the untrusted word/media/imageN filename -> a small crafted DOCX could OOM the indexer. - DOCX: cap embedded media entries iterated and per-entry decompressed size; skip non-positive indices; only materialise the positional array when the max index is within the cap, else fall back to a compact ordered list. - PPTX: cap slides walked and pictures decoded into memory. - PDF (marker): cap pages processed per file (_MAX_PDF_PAGES). EML fan-out/recursion caps were already carried over (_MAX_EML_ATTACHMENTS + _build_eml depth bound), so are unchanged. Caps follow the existing module-level constant pattern used by the EML parser. --- openrag/core/indexing/parsers/docx_parser.py | 35 +++++++++++++++++-- openrag/core/indexing/parsers/pptx_parser.py | 12 +++++++ .../workers/parsers/marker_workers.py | 25 +++++++++++-- .../core/indexing/parsers/test_docx_parser.py | 17 +++++++++ 4 files changed, 84 insertions(+), 5 deletions(-) diff --git a/openrag/core/indexing/parsers/docx_parser.py b/openrag/core/indexing/parsers/docx_parser.py index a1702eb7c..f70e66eb3 100644 --- a/openrag/core/indexing/parsers/docx_parser.py +++ b/openrag/core/indexing/parsers/docx_parser.py @@ -48,6 +48,13 @@ # loader used (``components/indexer/loaders/docx.py``). _MARKITDOWN_IMAGE_PLACEHOLDER = re.compile(r"!\[.*?\]\(data:image/[^)]*\)") +# Parser-bomb caps (mirrors the EML loader's module-level limits): bound how many +# embedded media entries we iterate and how large any single (decompressed) entry +# may be, so a crafted DOCX can't exhaust memory via thousands of parts, one huge +# image, or an attacker-controlled positional index in the media filename. +_MAX_ARCHIVE_ENTRIES = 2000 +_MAX_ARCHIVE_ENTRY_BYTES = 100 * 1024 * 1024 + def _image_ref(index: int) -> str: """Synthetic markdown image ref used as a placeholder for embedded DOCX images.""" @@ -133,13 +140,30 @@ def _extract_embedded_images(path: str) -> list[bytes | None]: media = [n for n in zf.namelist() if n.startswith("word/media/")] if not media: return [] + if len(media) > _MAX_ARCHIVE_ENTRIES: + logger.warning( + "Capping DOCX embedded media: %d entries found, processing first %d", + len(media), + _MAX_ARCHIVE_ENTRIES, + ) + media = media[:_MAX_ARCHIVE_ENTRIES] ordered: dict[int, bytes | None] = {} for name in media: - raw = zf.read(name) try: order_num = int(name.split("media/image")[1].split(".")[0]) except (IndexError, ValueError): continue + # order_num comes from the untrusted filename; a non-positive + # value would misalign placeholders or index wrongly. Skip it. + if order_num < 1: + logger.warning("Skipping DOCX media with non-positive index: %s", name) + continue + # Bound per-entry decompressed size (read from the zip header, + # before decompressing) so one huge embedded image can't OOM us. + if zf.getinfo(name).file_size > _MAX_ARCHIVE_ENTRY_BYTES: + logger.warning("Skipping oversized DOCX media %s (%d bytes)", name, zf.getinfo(name).file_size) + continue + raw = zf.read(name) try: with Image.open(BytesIO(raw)) as im: im = ensure_png_compatible_mode(im) @@ -149,8 +173,15 @@ def _extract_embedded_images(path: str) -> list[bytes | None]: ordered[order_num] = None if not ordered: return [] + # Reorder by document position. ``max_order`` derives from the + # (untrusted) filename, so only materialise the positional array + # when it's within the entry cap; otherwise fall back to a compact + # ordered list to avoid an attacker-controlled huge allocation + # (``[None] * max_order`` was a memory bomb). max_order = max(ordered) - return [ordered.get(i + 1) for i in range(max_order)] + if max_order <= _MAX_ARCHIVE_ENTRIES: + return [ordered.get(i + 1) for i in range(max_order)] + return [ordered[k] for k in sorted(ordered)] except zipfile.BadZipFile: logger.warning("DOCX is not a valid zip archive; skipping image extraction") return [] diff --git a/openrag/core/indexing/parsers/pptx_parser.py b/openrag/core/indexing/parsers/pptx_parser.py index 5663c0512..8b8249028 100644 --- a/openrag/core/indexing/parsers/pptx_parser.py +++ b/openrag/core/indexing/parsers/pptx_parser.py @@ -31,6 +31,12 @@ logger = logging.getLogger(__name__) +# Parser-bomb caps (mirrors the EML loader's module-level limits): bound how many +# slides we walk and how many pictures we decode into memory so a crafted PPTX +# can't exhaust CPU/memory during ingestion. +_MAX_SLIDES = 2000 +_MAX_IMAGES = 2000 + def _image_ref(index: int) -> str: """Synthetic markdown image ref used as a placeholder for slide pictures.""" @@ -83,11 +89,17 @@ def _convert(self, path: str) -> tuple[int, list[tuple[int, str]], list[ImageBlo images: list[ImageBlock] = [] for slide_num, slide in enumerate(presentation.slides, start=1): + if slide_num > _MAX_SLIDES: + logger.warning("Capping PPTX slide processing at %d slides", _MAX_SLIDES) + break md = "" title = slide.shapes.title for shape in slide.shapes: if self._is_picture(shape): + # Bound pictures decoded into memory across the whole deck. + if len(images) >= _MAX_IMAGES: + continue try: with Image.open(BytesIO(shape.image.blob)) as im: im = ensure_png_compatible_mode(im) diff --git a/openrag/services/workers/parsers/marker_workers.py b/openrag/services/workers/parsers/marker_workers.py index 085800450..8d5559bf3 100644 --- a/openrag/services/workers/parsers/marker_workers.py +++ b/openrag/services/workers/parsers/marker_workers.py @@ -42,6 +42,12 @@ def _marker_num_gpus(config) -> float: return requested_gpus if torch.cuda.is_available() else 0 +# Parser-bomb cap: never process more than this many pages from one PDF, so a +# crafted high-page-count file can't exhaust CPU/memory during ingestion. +# 0 or negative disables the cap. +_MAX_PDF_PAGES = 2000 + + @ray.remote class MarkerWorker: def __init__(self): @@ -286,15 +292,28 @@ async def attempt(_i: int): async def process_pdf(self, file_path: str): chunk_size = self.config.loader.marker_chunk_size + total_pages = self._get_page_count(file_path) + capped = _MAX_PDF_PAGES > 0 and total_pages > _MAX_PDF_PAGES + if capped: + self.logger.warning( + f"PDF has {total_pages} pages; processing only the first {_MAX_PDF_PAGES} (max page cap)" + ) + page_count = min(total_pages, _MAX_PDF_PAGES) if _MAX_PDF_PAGES > 0 else total_pages + if chunk_size <= 0: - return await self._process_chunk(file_path, page_range=None, label="(all pages)") + # When capped, restrict to the first page_count pages instead of all. + page_range = list(range(page_count)) if capped else None + label = f"(first {page_count}p)" if capped else "(all pages)" + return await self._process_chunk(file_path, page_range=page_range, label=label) - page_count = self._get_page_count(file_path) chunks = self._create_chunks(page_count, chunk_size) if len(chunks) == 1: page_range, label = chunks[0] - return await self._process_chunk(file_path, page_range=None, label=label) + # When capped, the single chunk only covers the first page_count pages, + # so pass that explicit range — page_range=None would process the whole + # file and bypass the cap. Uncapped, None means "all pages". + return await self._process_chunk(file_path, page_range=(page_range if capped else None), label=label) self.logger.info( f"Splitting {page_count}-page PDF into {len(chunks)} chunks of ~{chunk_size} pages for parallel processing" diff --git a/tests/unit/core/indexing/parsers/test_docx_parser.py b/tests/unit/core/indexing/parsers/test_docx_parser.py index 3c6b22458..4890e40f7 100644 --- a/tests/unit/core/indexing/parsers/test_docx_parser.py +++ b/tests/unit/core/indexing/parsers/test_docx_parser.py @@ -82,6 +82,23 @@ def test_invalid_zip_returns_empty(self): tmp.flush() assert DocxParser._extract_embedded_images(tmp.name) == [] + def test_huge_positional_index_does_not_allocate(self): + # Security regression: order_num comes from the (untrusted) filename. A + # crafted entry like image999999999.png must NOT trigger a + # ``[None] * 999999999`` allocation (memory bomb). Above the entry cap we + # fall back to a compact ordered list, so the result stays tiny. + docx = _fake_docx({"image999999999.png": _png_bytes("red")}) + result = DocxParser._extract_embedded_images(str(docx)) + assert len(result) == 1 + assert result[0] is not None + + def test_non_positive_index_skipped(self): + # image0 (order 0) is non-positive and must be dropped; image1 is kept. + docx = _fake_docx({"image0.png": _png_bytes("blue"), "image1.png": _png_bytes("red")}) + result = DocxParser._extract_embedded_images(str(docx)) + assert len(result) == 1 + assert result[0] is not None + class TestRewritePlaceholdersAndBuildBlocks: """The parser→caption contract: synthetic refs + ImageBlock metadata.""" From eca396d8fa3debcd863f557d6e77eca83dbac2fe Mon Sep 17 00:00:00 2001 From: andyne13 Date: Mon, 29 Jun 2026 22:54:40 +0200 Subject: [PATCH 3/5] fix(security): default logs to INFO and drop user query from web-search log Backport of 1bfca310 (M10). DEBUG was the default level and the web-search zero-results warning logged the raw query text (survives an INFO default and persists to the long-lived JSON sink), leaking potentially sensitive request content. - conf/config.yaml + infra/compose/.env.example: default log level INFO. - websearch/service.py: drop the query string from the zero-results warning. The pre-refactor pipeline.py temporal-filter warning that also logged the query has no hexagonal equivalent (that retry path no longer logs the query), so needs no change. Search endpoints already log query_len, not the text. --- conf/config.yaml | 5 ++++- infra/compose/.env.example | 3 ++- openrag/services/websearch/service.py | 4 +++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/conf/config.yaml b/conf/config.yaml index 3994cd522..168f39017 100644 --- a/conf/config.yaml +++ b/conf/config.yaml @@ -115,8 +115,11 @@ map_reduce: # --- Logging --- # Env: LOG_LEVEL +# INFO by default: DEBUG logs (and the long-lived JSON sink) persist user +# queries and other potentially sensitive request data. Raise to DEBUG only +# for short-lived troubleshooting. verbose: - level: DEBUG + level: INFO # --- Server --- # Env: PREFERRED_URL_SCHEME diff --git a/infra/compose/.env.example b/infra/compose/.env.example index 11fe10fa8..7bfa8a170 100644 --- a/infra/compose/.env.example +++ b/infra/compose/.env.example @@ -149,7 +149,8 @@ API_BASE_URL='http://X.X.X.X:APP_PORT' # Base URL of your FastAPI backe # OPENRAG_MCP_MAX_DOWNLOAD_BYTES=104857600 # 100 MiB # LOGGING -LOG_LEVEL=DEBUG # See possible values https://loguru.readthedocs.io/en/stable/api/logger.html +# INFO by default; DEBUG persists user queries and request data to logs. +LOG_LEVEL=INFO # See possible values https://loguru.readthedocs.io/en/stable/api/logger.html # SERVER # Set the preferred URL scheme for generated URLs (e.g., task_status_url). diff --git a/openrag/services/websearch/service.py b/openrag/services/websearch/service.py index d6af5c5b7..44c8c552b 100644 --- a/openrag/services/websearch/service.py +++ b/openrag/services/websearch/service.py @@ -23,7 +23,9 @@ async def search(self, query: str) -> list[WebResult]: try: results = await self.provider.search(query) if not results: - logger.warning("Web search returned zero results", query=query) + # Don't log the query text (user PII); the empty-result signal + # is enough for diagnostics. + logger.warning("Web search returned zero results") return results if self.content_fetcher: From bcd79fca32c65baed02cb566bfa3ed8e9b11371c Mon Sep 17 00:00:00 2001 From: Ahmath-Gadji Date: Tue, 30 Jun 2026 07:43:08 +0000 Subject: [PATCH 4/5] fix(security): resolve upload-size cap at call time so .env is honored MAX_UPLOAD_SIZE_BYTES was evaluated at import, but api.main imports the admin routers (and thus this module) before it calls load_config()/ load_dotenv(). A MAX_UPLOAD_SIZE_MB set in .env was therefore ignored on local starts and the process silently used the 1024 MB default. Read the value lazily inside save_file_to_disk via _max_upload_size_bytes(). --- openrag/api/dependencies/files.py | 23 +++++++++++++++++------ tests/unit/api/dependencies/test_files.py | 12 +++++++++++- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/openrag/api/dependencies/files.py b/openrag/api/dependencies/files.py index 5f7c2624d..487256505 100644 --- a/openrag/api/dependencies/files.py +++ b/openrag/api/dependencies/files.py @@ -12,10 +12,20 @@ FORBIDDEN_CHARS_IN_FILE_ID = set("/") -# Maximum accepted upload size. Streamed writes are bounded so a single request -# cannot exhaust disk/RAM (the per-user quota limits file count, not bytes). -# 0 or negative disables the limit. Override with MAX_UPLOAD_SIZE_MB. -MAX_UPLOAD_SIZE_BYTES = int(os.getenv("MAX_UPLOAD_SIZE_MB", "1024")) * 1024 * 1024 + +def _max_upload_size_bytes() -> int: + """Maximum accepted upload size in bytes, resolved at call time. + + Streamed writes are bounded so a single request cannot exhaust disk/RAM + (the per-user quota limits file count, not bytes). 0 or negative disables + the limit. Override with ``MAX_UPLOAD_SIZE_MB``. + + Read lazily rather than at import: ``api.main`` imports the admin routers + (and therefore this module) before it calls ``load_config()`` / + ``load_dotenv()``, so evaluating at import would ignore a + ``MAX_UPLOAD_SIZE_MB`` set in ``.env`` and silently use the default. + """ + return int(os.getenv("MAX_UPLOAD_SIZE_MB", "1024")) * 1024 * 1024 async def validate_file_id(file_id: str): @@ -64,6 +74,7 @@ async def save_file_to_disk( except ValueError: raise ValidationError("Uploaded filename resolves outside destination directory.", status_code=400) + max_bytes = _max_upload_size_bytes() total = 0 try: async with aiofiles.open(file_path, "wb") as buffer: @@ -72,9 +83,9 @@ async def save_file_to_disk( if not chunk: break total += len(chunk) - if MAX_UPLOAD_SIZE_BYTES > 0 and total > MAX_UPLOAD_SIZE_BYTES: + if max_bytes > 0 and total > max_bytes: raise ValidationError( - f"File exceeds the maximum allowed size of {MAX_UPLOAD_SIZE_BYTES // (1024 * 1024)} MB.", + f"File exceeds the maximum allowed size of {max_bytes // (1024 * 1024)} MB.", status_code=413, ) await buffer.write(chunk) diff --git a/tests/unit/api/dependencies/test_files.py b/tests/unit/api/dependencies/test_files.py index 86afe14df..ab3af5763 100644 --- a/tests/unit/api/dependencies/test_files.py +++ b/tests/unit/api/dependencies/test_files.py @@ -2,6 +2,7 @@ from pathlib import Path import pytest +from api.dependencies import files as files_dep from api.dependencies.files import save_file_to_disk from core.utils.exceptions import ValidationError from core.utils.filename import extract_temporal_fields, sanitize_filename @@ -33,7 +34,7 @@ async def test_save_file_to_disk_writes_content(tmp_path: Path): @pytest.mark.asyncio async def test_save_file_to_disk_rejects_oversize_upload(tmp_path: Path, monkeypatch): # Cap at ~8 bytes; a larger upload must be rejected (413) and not left on disk. - monkeypatch.setattr("api.dependencies.files.MAX_UPLOAD_SIZE_BYTES", 8) + monkeypatch.setattr("api.dependencies.files._max_upload_size_bytes", lambda: 8) upload = UploadFile(file=io.BytesIO(b"x" * 100), filename="big.bin") dest_dir = tmp_path / "uploads" @@ -45,6 +46,15 @@ async def test_save_file_to_disk_rejects_oversize_upload(tmp_path: Path, monkeyp assert not (dest_dir / "big.bin").exists() +def test_max_upload_size_reads_env_at_call_time(monkeypatch): + # The cap must be resolved per call: api.main imports this module before + # load_dotenv() runs, so reading at import would ignore MAX_UPLOAD_SIZE_MB. + monkeypatch.setenv("MAX_UPLOAD_SIZE_MB", "5") + assert files_dep._max_upload_size_bytes() == 5 * 1024 * 1024 + monkeypatch.setenv("MAX_UPLOAD_SIZE_MB", "0") + assert files_dep._max_upload_size_bytes() == 0 + + @pytest.mark.asyncio async def test_save_file_to_disk_with_random_prefix(tmp_path, monkeypatch): def fake_make_unique_filename(filename: str) -> str: From 697aabe80025924c863a97dc496664cf2c519a4e Mon Sep 17 00:00:00 2001 From: Ahmath-Gadji Date: Tue, 30 Jun 2026 07:47:57 +0000 Subject: [PATCH 5/5] fix(security): surface upload-rejection status instead of masking as 500 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit save_file_to_disk raises ValidationError (413 for oversize uploads, 400 for bad filenames), but add_file and execute_tool wrapped it in a broad 'except Exception' that re-raised HTTP 500 — so the new payload-too-large contract looked like a server failure. Let OpenRAGError propagate to the registered handler, which maps it to its declared status. Adds a route-level regression test asserting 413 (not 500) for an oversize upload. --- openrag/api/routers/admin/indexing.py | 6 ++ openrag/api/routers/admin/tools.py | 6 ++ .../admin/test_indexing_upload_errors.py | 58 +++++++++++++++++++ 3 files changed, 70 insertions(+) create mode 100644 tests/unit/api/routers/admin/test_indexing_upload_errors.py diff --git a/openrag/api/routers/admin/indexing.py b/openrag/api/routers/admin/indexing.py index ec06a41a3..d2d35cb3a 100644 --- a/openrag/api/routers/admin/indexing.py +++ b/openrag/api/routers/admin/indexing.py @@ -30,6 +30,7 @@ validate_metadata, ) from api.routers.admin.task_logs import collect_task_logs +from core.utils.exceptions import OpenRAGError from core.utils.filename import sanitize_filename from core.utils.log_tail import app_log_file from core.utils.logging import get_logger @@ -138,6 +139,11 @@ async def add_file( file.filename = sanitize_filename(file.filename) try: file_path = await save_file_to_disk(file, Path(config.paths.data_dir), with_random_prefix=True) + except OpenRAGError: + # Domain errors (e.g. 413 too-large, 400 bad filename) carry their own + # HTTP status; let the OpenRAGError handler map them instead of masking + # the upload rejection as a 500. + raise except Exception as e: # Log the full error server-side; return a generic message so we don't # leak filesystem paths or internals to the client. diff --git a/openrag/api/routers/admin/tools.py b/openrag/api/routers/admin/tools.py index 257123120..c68912a05 100644 --- a/openrag/api/routers/admin/tools.py +++ b/openrag/api/routers/admin/tools.py @@ -18,6 +18,7 @@ validate_metadata, ) from api.schemas.admin.tools import ToolInfo +from core.utils.exceptions import OpenRAGError from core.utils.logging import get_logger from di.providers import get_config, get_conversion_service from fastapi import APIRouter, Depends, Form, HTTPException, UploadFile, status @@ -121,6 +122,11 @@ async def execute_tool( except HTTPException: raise + except OpenRAGError: + # Domain errors (e.g. 413 too-large, 400 bad filename from + # save_file_to_disk) carry their own HTTP status; let the OpenRAGError + # handler map them instead of masking the rejection as a 500. + raise except TimeoutError: logger.warning("Tool execution timed out.", extra={"filename": file.filename}) raise HTTPException( diff --git a/tests/unit/api/routers/admin/test_indexing_upload_errors.py b/tests/unit/api/routers/admin/test_indexing_upload_errors.py new file mode 100644 index 000000000..6c421d1e8 --- /dev/null +++ b/tests/unit/api/routers/admin/test_indexing_upload_errors.py @@ -0,0 +1,58 @@ +"""The add-file route must surface upload-rejection status codes. + +``save_file_to_disk`` raises ``ValidationError`` (e.g. 413 for an oversize +upload) which is an ``OpenRAGError``. Regression guard: the route's broad +``except Exception`` must not mask that as a 500 — the OpenRAGError handler +should map it to its real status. +""" + +import io +from types import SimpleNamespace + +import httpx +import pytest +from api.dependencies.auth import check_user_file_quota, require_partition_editor +from api.dependencies.files import validate_file_format, validate_file_id, validate_metadata +from api.error_handlers import register_error_handlers +from api.routers.admin.indexing import router as indexer_router +from di.providers import get_config, get_indexing_service +from fastapi import FastAPI, UploadFile + + +class _FakeIndexingService: + async def file_exists(self, file_id: str, partition: str) -> bool: + return False + + +def _empty_metadata() -> dict: + return {} + + +def _build_app(tmp_path, monkeypatch, content: bytes) -> FastAPI: + # Cap at ~8 bytes so a small upload trips the limit. + monkeypatch.setattr("api.dependencies.files._max_upload_size_bytes", lambda: 8) + + app = FastAPI() + register_error_handlers(app) + app.include_router(indexer_router, prefix="/indexer") + + cfg = SimpleNamespace(paths=SimpleNamespace(data_dir=str(tmp_path / "data"))) + + app.dependency_overrides[validate_file_id] = lambda: "f1" + app.dependency_overrides[validate_file_format] = lambda: UploadFile(file=io.BytesIO(content), filename="big.bin") + app.dependency_overrides[validate_metadata] = _empty_metadata + app.dependency_overrides[require_partition_editor] = lambda: {"id": 1, "is_admin": True} + app.dependency_overrides[check_user_file_quota] = lambda: None + app.dependency_overrides[get_config] = lambda: cfg + app.dependency_overrides[get_indexing_service] = lambda: _FakeIndexingService() + return app + + +@pytest.mark.asyncio +async def test_add_file_oversize_returns_413_not_500(tmp_path, monkeypatch): + app = _build_app(tmp_path, monkeypatch, content=b"x" * 100) + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: + resp = await client.post("/indexer/partition/p1/file/f1", data={"_": "1"}) + + assert resp.status_code == 413