Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion conf/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion infra/compose/.env.example
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -146,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).
Expand Down
41 changes: 35 additions & 6 deletions openrag/api/dependencies/files.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import os
from pathlib import Path
from typing import Any

Expand All @@ -12,6 +13,21 @@
FORBIDDEN_CHARS_IN_FILE_ID = set("/")


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
Comment thread
coderabbitai[bot] marked this conversation as resolved.


async def validate_file_id(file_id: str):
return core_validators.validate_file_id(file_id, FORBIDDEN_CHARS_IN_FILE_ID)

Expand Down Expand Up @@ -58,11 +74,24 @@ 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)
max_bytes = _max_upload_size_bytes()
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_bytes > 0 and total > max_bytes:
raise ValidationError(
f"File exceeds the maximum allowed size of {max_bytes // (1024 * 1024)} MB.",
status_code=413,
)
Comment thread
Ahmath-Gadji marked this conversation as resolved.
await buffer.write(chunk)
except ValidationError:
# Remove the partially written file before propagating.
file_path.unlink(missing_ok=True)
raise

return file_path
6 changes: 6 additions & 0 deletions openrag/api/routers/admin/indexing.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
6 changes: 6 additions & 0 deletions openrag/api/routers/admin/tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down
35 changes: 33 additions & 2 deletions openrag/core/indexing/parsers/docx_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down Expand Up @@ -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
Comment thread
Ahmath-Gadji marked this conversation as resolved.
raw = zf.read(name)
try:
with Image.open(BytesIO(raw)) as im:
im = ensure_png_compatible_mode(im)
Expand All @@ -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 []
Expand Down
12 changes: 12 additions & 0 deletions openrag/core/indexing/parsers/pptx_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 3 additions & 1 deletion openrag/services/websearch/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
25 changes: 22 additions & 3 deletions openrag/services/workers/parsers/marker_workers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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"
Expand Down
25 changes: 25 additions & 0 deletions tests/unit/api/dependencies/test_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -30,6 +31,30 @@ 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", lambda: 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()


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:
Expand Down
58 changes: 58 additions & 0 deletions tests/unit/api/routers/admin/test_indexing_upload_errors.py
Original file line number Diff line number Diff line change
@@ -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
Loading
Loading