Merge pull request #347 from christianlouis/copilot/add-request-size-limits
This commit is contained in:
@@ -21,6 +21,12 @@ MAX_UPLOAD_SIZE=1073741824
|
|||||||
# Default: None (no splitting). Example: 104857600 for 100MB chunks
|
# Default: None (no splitting). Example: 104857600 for 100MB chunks
|
||||||
# MAX_SINGLE_FILE_SIZE=104857600
|
# MAX_SINGLE_FILE_SIZE=104857600
|
||||||
|
|
||||||
|
# **Request Body Size Limit** (Security - see SECURITY_AUDIT.md)
|
||||||
|
# Maximum request body size in bytes for non-file-upload requests (JSON, form data, etc.).
|
||||||
|
# Default: 1MB (1048576 bytes). File uploads are governed by MAX_UPLOAD_SIZE above.
|
||||||
|
# Prevents memory exhaustion from oversized JSON/form payloads.
|
||||||
|
# MAX_REQUEST_BODY_SIZE=1048576
|
||||||
|
|
||||||
# **Security Headers** (see SECURITY_AUDIT.md and docs/DeploymentGuide.md)
|
# **Security Headers** (see SECURITY_AUDIT.md and docs/DeploymentGuide.md)
|
||||||
# Disabled by default since most deployments use a reverse proxy (Traefik, Nginx, etc.)
|
# Disabled by default since most deployments use a reverse proxy (Traefik, Nginx, etc.)
|
||||||
# that already adds these headers. Set to true only if deploying directly without a reverse proxy.
|
# that already adds these headers. Set to true only if deploying directly without a reverse proxy.
|
||||||
|
|||||||
+3
-1
@@ -274,6 +274,8 @@ ftp = ftplib.FTP() # nosec B321 - Plaintext FTP intentional when configured
|
|||||||
- ✅ Unique filenames with UUID to prevent conflicts and overwrites
|
- ✅ Unique filenames with UUID to prevent conflicts and overwrites
|
||||||
- ✅ File upload size limits with configurable maximum (default: 1GB)
|
- ✅ File upload size limits with configurable maximum (default: 1GB)
|
||||||
- ✅ Optional file splitting for large PDFs (when max_single_file_size is configured)
|
- ✅ Optional file splitting for large PDFs (when max_single_file_size is configured)
|
||||||
|
- ✅ Request body size limits via `RequestSizeLimitMiddleware` (non-upload: 1MB default; uploads: governed by MAX_UPLOAD_SIZE)
|
||||||
|
- ✅ Streaming file reads in upload endpoint to prevent memory exhaustion
|
||||||
- ⏳ **TODO:** Implement rate limiting on API endpoints
|
- ⏳ **TODO:** Implement rate limiting on API endpoints
|
||||||
- ⏳ **TODO:** Add CSRF protection for state-changing operations
|
- ⏳ **TODO:** Add CSRF protection for state-changing operations
|
||||||
- ⏳ **TODO:** Add comprehensive input sanitization for all user inputs ([#172](https://github.com/christianlouis/DocuElevate/issues/172))
|
- ⏳ **TODO:** Add comprehensive input sanitization for all user inputs ([#172](https://github.com/christianlouis/DocuElevate/issues/172))
|
||||||
@@ -302,7 +304,7 @@ ftp = ftplib.FTP() # nosec B321 - Plaintext FTP intentional when configured
|
|||||||
1. ~~**Enable CodeQL scanning**~~ ✅ Already implemented - Two CodeQL workflows active
|
1. ~~**Enable CodeQL scanning**~~ ✅ Already implemented - Two CodeQL workflows active
|
||||||
2. **Implement rate limiting** - Prevent abuse and DoS attacks (consider slowapi or fastapi-limiter)
|
2. **Implement rate limiting** - Prevent abuse and DoS attacks (consider slowapi or fastapi-limiter)
|
||||||
3. **Add comprehensive input validation** - Prevent injection attacks ([#172](https://github.com/christianlouis/DocuElevate/issues/172))
|
3. **Add comprehensive input validation** - Prevent injection attacks ([#172](https://github.com/christianlouis/DocuElevate/issues/172))
|
||||||
4. **Add request size limits** - Prevent memory exhaustion from large uploads ([#173](https://github.com/christianlouis/DocuElevate/issues/173))
|
4. ~~**Add request size limits**~~ ✅ Implemented - `RequestSizeLimitMiddleware` enforces `MAX_REQUEST_BODY_SIZE` (default 1 MB) for non-file requests and `MAX_UPLOAD_SIZE` (default 1 GB) for multipart uploads; file uploads also use streaming reads to bound memory usage ([#173](https://github.com/christianlouis/DocuElevate/issues/173))
|
||||||
5. **Implement CSRF protection** - Protect state-changing operations
|
5. **Implement CSRF protection** - Protect state-changing operations
|
||||||
|
|
||||||
### Medium Priority
|
### Medium Priority
|
||||||
|
|||||||
+122
-34
@@ -45,7 +45,8 @@ def list_files_api(
|
|||||||
page: int = Query(1, ge=1, description="Page number"),
|
page: int = Query(1, ge=1, description="Page number"),
|
||||||
per_page: int = Query(50, ge=1, le=200, description="Items per page"),
|
per_page: int = Query(50, ge=1, le=200, description="Items per page"),
|
||||||
sort_by: str = Query(
|
sort_by: str = Query(
|
||||||
"created_at", description="Sort field: id, original_filename, file_size, mime_type, created_at, status"
|
"created_at",
|
||||||
|
description="Sort field: id, original_filename, file_size, mime_type, created_at, status",
|
||||||
),
|
),
|
||||||
sort_order: str = Query("desc", description="Sort order: asc or desc"),
|
sort_order: str = Query("desc", description="Sort order: asc or desc"),
|
||||||
search: Optional[str] = Query(None, description="Search in filename"),
|
search: Optional[str] = Query(None, description="Search in filename"),
|
||||||
@@ -128,7 +129,13 @@ def list_files_api(
|
|||||||
"mime_type": f.mime_type,
|
"mime_type": f.mime_type,
|
||||||
"created_at": f.created_at.isoformat() if f.created_at else None,
|
"created_at": f.created_at.isoformat() if f.created_at else None,
|
||||||
"processing_status": statuses.get(
|
"processing_status": statuses.get(
|
||||||
f.id, {"status": "pending", "last_step": None, "has_errors": False, "total_steps": 0}
|
f.id,
|
||||||
|
{
|
||||||
|
"status": "pending",
|
||||||
|
"last_step": None,
|
||||||
|
"has_errors": False,
|
||||||
|
"total_steps": 0,
|
||||||
|
},
|
||||||
),
|
),
|
||||||
}
|
}
|
||||||
)
|
)
|
||||||
@@ -138,7 +145,12 @@ def list_files_api(
|
|||||||
|
|
||||||
return {
|
return {
|
||||||
"files": result,
|
"files": result,
|
||||||
"pagination": {"page": page, "per_page": per_page, "total_items": total_items, "total_pages": total_pages},
|
"pagination": {
|
||||||
|
"page": page,
|
||||||
|
"per_page": per_page,
|
||||||
|
"total_items": total_items,
|
||||||
|
"total_pages": total_pages,
|
||||||
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
@@ -187,7 +199,7 @@ def get_file_details(request: Request, file_id: int, db: DbSession):
|
|||||||
processing_status = _get_file_processing_status(db, file_id)
|
processing_status = _get_file_processing_status(db, file_id)
|
||||||
|
|
||||||
# Check if files exist on disk
|
# Check if files exist on disk
|
||||||
files_on_disk = {"original": os.path.exists(file_record.local_filename) if file_record.local_filename else False}
|
files_on_disk = {"original": (os.path.exists(file_record.local_filename) if file_record.local_filename else False)}
|
||||||
|
|
||||||
return {
|
return {
|
||||||
"file": {
|
"file": {
|
||||||
@@ -197,7 +209,7 @@ def get_file_details(request: Request, file_id: int, db: DbSession):
|
|||||||
"local_filename": file_record.local_filename,
|
"local_filename": file_record.local_filename,
|
||||||
"file_size": file_record.file_size,
|
"file_size": file_record.file_size,
|
||||||
"mime_type": file_record.mime_type,
|
"mime_type": file_record.mime_type,
|
||||||
"created_at": file_record.created_at.isoformat() if file_record.created_at else None,
|
"created_at": (file_record.created_at.isoformat() if file_record.created_at else None),
|
||||||
},
|
},
|
||||||
"processing_status": processing_status,
|
"processing_status": processing_status,
|
||||||
"logs": log_list,
|
"logs": log_list,
|
||||||
@@ -230,7 +242,10 @@ def delete_file_record(request: Request, file_id: int, db: DbSession):
|
|||||||
db.delete(file_record)
|
db.delete(file_record)
|
||||||
db.commit()
|
db.commit()
|
||||||
|
|
||||||
return {"status": "success", "message": f"File record {file_id} deleted successfully"}
|
return {
|
||||||
|
"status": "success",
|
||||||
|
"message": f"File record {file_id} deleted successfully",
|
||||||
|
}
|
||||||
|
|
||||||
except HTTPException:
|
except HTTPException:
|
||||||
raise
|
raise
|
||||||
@@ -318,7 +333,11 @@ def bulk_reprocess_files(request: Request, file_ids: List[int], db: DbSession):
|
|||||||
task = process_document.delay(file_record.local_filename, file_id=file_record.id)
|
task = process_document.delay(file_record.local_filename, file_id=file_record.id)
|
||||||
task_ids.append(task.id)
|
task_ids.append(task.id)
|
||||||
processed_files.append(
|
processed_files.append(
|
||||||
{"file_id": file_record.id, "filename": file_record.original_filename, "task_id": task.id}
|
{
|
||||||
|
"file_id": file_record.id,
|
||||||
|
"filename": file_record.original_filename,
|
||||||
|
"task_id": task.id,
|
||||||
|
}
|
||||||
)
|
)
|
||||||
|
|
||||||
logger.info(
|
logger.info(
|
||||||
@@ -328,7 +347,13 @@ def bulk_reprocess_files(request: Request, file_ids: List[int], db: DbSession):
|
|||||||
|
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.exception(f"Error reprocessing file {file_record.id}: {str(e)}")
|
logger.exception(f"Error reprocessing file {file_record.id}: {str(e)}")
|
||||||
errors.append({"file_id": file_record.id, "filename": file_record.original_filename, "error": str(e)})
|
errors.append(
|
||||||
|
{
|
||||||
|
"file_id": file_record.id,
|
||||||
|
"filename": file_record.original_filename,
|
||||||
|
"error": str(e),
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
return {
|
return {
|
||||||
"status": "success" if processed_files else "error",
|
"status": "success" if processed_files else "error",
|
||||||
@@ -366,11 +391,16 @@ def reprocess_single_file(request: Request, file_id: int, db: DbSession):
|
|||||||
|
|
||||||
# Check if local file exists
|
# Check if local file exists
|
||||||
if not file_record.local_filename or not os.path.exists(file_record.local_filename):
|
if not file_record.local_filename or not os.path.exists(file_record.local_filename):
|
||||||
raise HTTPException(status_code=400, detail="Local file not found on disk. Cannot reprocess.")
|
raise HTTPException(
|
||||||
|
status_code=400,
|
||||||
|
detail="Local file not found on disk. Cannot reprocess.",
|
||||||
|
)
|
||||||
|
|
||||||
# Queue the file for processing, passing file_id to skip duplicate check
|
# Queue the file for processing, passing file_id to skip duplicate check
|
||||||
task = process_document.delay(
|
task = process_document.delay(
|
||||||
file_record.local_filename, original_filename=file_record.original_filename, file_id=file_record.id
|
file_record.local_filename,
|
||||||
|
original_filename=file_record.original_filename,
|
||||||
|
file_id=file_record.id,
|
||||||
)
|
)
|
||||||
|
|
||||||
logger.info(
|
logger.info(
|
||||||
@@ -425,12 +455,16 @@ def reprocess_with_cloud_ocr(request: Request, file_id: int, db: DbSession):
|
|||||||
logger.info(f"Using local file for Cloud OCR reprocessing: {source_file}")
|
logger.info(f"Using local file for Cloud OCR reprocessing: {source_file}")
|
||||||
else:
|
else:
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
status_code=400, detail="Neither original nor local file found on disk. Cannot reprocess."
|
status_code=400,
|
||||||
|
detail="Neither original nor local file found on disk. Cannot reprocess.",
|
||||||
)
|
)
|
||||||
|
|
||||||
# Queue the file for processing with force_cloud_ocr=True
|
# Queue the file for processing with force_cloud_ocr=True
|
||||||
task = process_document.delay(
|
task = process_document.delay(
|
||||||
source_file, original_filename=file_record.original_filename, file_id=file_record.id, force_cloud_ocr=True
|
source_file,
|
||||||
|
original_filename=file_record.original_filename,
|
||||||
|
file_id=file_record.id,
|
||||||
|
force_cloud_ocr=True,
|
||||||
)
|
)
|
||||||
|
|
||||||
logger.info(
|
logger.info(
|
||||||
@@ -510,10 +544,14 @@ def _retry_pipeline_step(file_record: FileRecord, step_name: str, db: Session) -
|
|||||||
|
|
||||||
logger.info(f"Found file for process_document retry at: {file_record.local_filename!r}")
|
logger.info(f"Found file for process_document retry at: {file_record.local_filename!r}")
|
||||||
task = process_document.delay(
|
task = process_document.delay(
|
||||||
file_record.local_filename, original_filename=file_record.original_filename, file_id=file_id
|
file_record.local_filename,
|
||||||
|
original_filename=file_record.original_filename,
|
||||||
|
file_id=file_id,
|
||||||
)
|
)
|
||||||
elif step_name == "process_with_azure_document_intelligence":
|
elif step_name == "process_with_azure_document_intelligence":
|
||||||
from app.tasks.process_with_azure_document_intelligence import process_with_azure_document_intelligence
|
from app.tasks.process_with_azure_document_intelligence import (
|
||||||
|
process_with_azure_document_intelligence,
|
||||||
|
)
|
||||||
|
|
||||||
# OCR needs the file in workdir/tmp
|
# OCR needs the file in workdir/tmp
|
||||||
logger.info(
|
logger.info(
|
||||||
@@ -542,7 +580,10 @@ def _retry_pipeline_step(file_record: FileRecord, step_name: str, db: Session) -
|
|||||||
)
|
)
|
||||||
if not file_record.local_filename:
|
if not file_record.local_filename:
|
||||||
logger.error(f"Metadata extraction retry failed for file {file_id}: local_filename is None")
|
logger.error(f"Metadata extraction retry failed for file {file_id}: local_filename is None")
|
||||||
raise HTTPException(status_code=400, detail="Local file path is None. Cannot retry metadata extraction.")
|
raise HTTPException(
|
||||||
|
status_code=400,
|
||||||
|
detail="Local file path is None. Cannot retry metadata extraction.",
|
||||||
|
)
|
||||||
|
|
||||||
exists = os.path.exists(file_record.local_filename)
|
exists = os.path.exists(file_record.local_filename)
|
||||||
logger.info(f"Checking local_filename: {file_record.local_filename!r}, exists={exists}")
|
logger.info(f"Checking local_filename: {file_record.local_filename!r}, exists={exists}")
|
||||||
@@ -556,7 +597,9 @@ def _retry_pipeline_step(file_record: FileRecord, step_name: str, db: Session) -
|
|||||||
filename = os.path.basename(file_record.local_filename)
|
filename = os.path.basename(file_record.local_filename)
|
||||||
task = extract_metadata_with_gpt.delay(filename, extracted_text, file_id)
|
task = extract_metadata_with_gpt.delay(filename, extracted_text, file_id)
|
||||||
elif step_name == "embed_metadata_into_pdf":
|
elif step_name == "embed_metadata_into_pdf":
|
||||||
from app.tasks.extract_metadata_with_gpt import extract_metadata_with_gpt as extract_metadata_task
|
from app.tasks.extract_metadata_with_gpt import (
|
||||||
|
extract_metadata_with_gpt as extract_metadata_task,
|
||||||
|
)
|
||||||
|
|
||||||
# Retrying embed requires re-running metadata extraction first, because
|
# Retrying embed requires re-running metadata extraction first, because
|
||||||
# embed_metadata_into_pdf needs the actual metadata dict (not empty).
|
# embed_metadata_into_pdf needs the actual metadata dict (not empty).
|
||||||
@@ -644,7 +687,8 @@ def retry_subtask(
|
|||||||
file_id: int,
|
file_id: int,
|
||||||
db: DbSession,
|
db: DbSession,
|
||||||
subtask_name: str = Query(
|
subtask_name: str = Query(
|
||||||
..., description="Name of the subtask to retry (e.g., 'upload_to_dropbox', 'extract_metadata_with_gpt')"
|
...,
|
||||||
|
description="Name of the subtask to retry (e.g., 'upload_to_dropbox', 'extract_metadata_with_gpt')",
|
||||||
),
|
),
|
||||||
):
|
):
|
||||||
"""
|
"""
|
||||||
@@ -822,7 +866,10 @@ def get_file_preview(
|
|||||||
if not file_path:
|
if not file_path:
|
||||||
raise HTTPException(status_code=404, detail="Processed file not found")
|
raise HTTPException(status_code=404, detail="Processed file not found")
|
||||||
else:
|
else:
|
||||||
raise HTTPException(status_code=400, detail="Invalid version parameter. Use 'original' or 'processed'")
|
raise HTTPException(
|
||||||
|
status_code=400,
|
||||||
|
detail="Invalid version parameter. Use 'original' or 'processed'",
|
||||||
|
)
|
||||||
|
|
||||||
# Return the file
|
# Return the file
|
||||||
return FileResponse(
|
return FileResponse(
|
||||||
@@ -894,7 +941,10 @@ def download_file(
|
|||||||
if not file_path:
|
if not file_path:
|
||||||
raise HTTPException(status_code=404, detail="Processed file not found")
|
raise HTTPException(status_code=404, detail="Processed file not found")
|
||||||
else:
|
else:
|
||||||
raise HTTPException(status_code=400, detail="Invalid version parameter. Use 'original' or 'processed'")
|
raise HTTPException(
|
||||||
|
status_code=400,
|
||||||
|
detail="Invalid version parameter. Use 'original' or 'processed'",
|
||||||
|
)
|
||||||
|
|
||||||
# Return the file with attachment disposition to trigger download
|
# Return the file with attachment disposition to trigger download
|
||||||
return FileResponse(
|
return FileResponse(
|
||||||
@@ -916,6 +966,21 @@ async def ui_upload(request: Request, file: UploadFile = File(...)):
|
|||||||
"""Endpoint to accept a user-uploaded file and enqueue it for processing."""
|
"""Endpoint to accept a user-uploaded file and enqueue it for processing."""
|
||||||
workdir = settings.workdir
|
workdir = settings.workdir
|
||||||
|
|
||||||
|
# Early size check: reject before reading the body if Content-Length is known
|
||||||
|
max_size = settings.max_upload_size
|
||||||
|
content_length_header = request.headers.get("content-length")
|
||||||
|
if content_length_header is not None:
|
||||||
|
try:
|
||||||
|
declared_size = int(content_length_header)
|
||||||
|
if declared_size > max_size:
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=413,
|
||||||
|
detail=f"File too large: declared size {declared_size} bytes exceeds maximum "
|
||||||
|
f"{max_size} bytes. See SECURITY_AUDIT.md for configuration details.",
|
||||||
|
)
|
||||||
|
except ValueError:
|
||||||
|
pass # Malformed header; proceed and check actual size after reading
|
||||||
|
|
||||||
# Extract just the filename without any path components to prevent path traversal
|
# Extract just the filename without any path components to prevent path traversal
|
||||||
# First, use basename to remove any directory components
|
# First, use basename to remove any directory components
|
||||||
base_filename = os.path.basename(file.filename)
|
base_filename = os.path.basename(file.filename)
|
||||||
@@ -934,27 +999,37 @@ async def ui_upload(request: Request, file: UploadFile = File(...)):
|
|||||||
# Store both the safe original name and the unique name
|
# Store both the safe original name and the unique name
|
||||||
target_path = os.path.join(workdir, target_filename)
|
target_path = os.path.join(workdir, target_filename)
|
||||||
|
|
||||||
|
# Read file in chunks to avoid loading the entire body into memory at once,
|
||||||
|
# enforcing the size limit during the read so memory usage stays bounded.
|
||||||
try:
|
try:
|
||||||
|
written_size = 0
|
||||||
with open(target_path, "wb") as f:
|
with open(target_path, "wb") as f:
|
||||||
content = await file.read()
|
chunk_size = 65536 # 64 KB chunks
|
||||||
f.write(content)
|
while True:
|
||||||
|
chunk = await file.read(chunk_size)
|
||||||
|
if not chunk:
|
||||||
|
break
|
||||||
|
written_size += len(chunk)
|
||||||
|
if written_size > max_size:
|
||||||
|
# Exceeded limit mid-stream; clean up and reject
|
||||||
|
f.close()
|
||||||
|
os.remove(target_path)
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=413,
|
||||||
|
detail=f"File too large: exceeded {max_size} bytes during upload. "
|
||||||
|
f"See SECURITY_AUDIT.md for configuration details.",
|
||||||
|
)
|
||||||
|
f.write(chunk)
|
||||||
|
except HTTPException:
|
||||||
|
raise
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
|
if os.path.exists(target_path):
|
||||||
|
os.remove(target_path)
|
||||||
raise HTTPException(status_code=500, detail=f"Failed to save file: {e}")
|
raise HTTPException(status_code=500, detail=f"Failed to save file: {e}")
|
||||||
|
|
||||||
# Log the mapping between original and safe filename
|
# Log the mapping between original and safe filename
|
||||||
logger.info(f"Saved uploaded file '{safe_filename}' as '{target_filename}'")
|
logger.info(f"Saved uploaded file '{safe_filename}' as '{target_filename}'")
|
||||||
|
file_size = written_size
|
||||||
# Check file size against configured maximum
|
|
||||||
file_size = os.path.getsize(target_path)
|
|
||||||
max_size = settings.max_upload_size
|
|
||||||
if file_size > max_size:
|
|
||||||
# Remove the file if it's too large
|
|
||||||
os.remove(target_path)
|
|
||||||
raise HTTPException(
|
|
||||||
status_code=413,
|
|
||||||
detail=f"File too large: {file_size} bytes (max {max_size} bytes). "
|
|
||||||
f"See SECURITY_AUDIT.md for configuration details.",
|
|
||||||
)
|
|
||||||
|
|
||||||
# Same set of allowed file types as in the IMAP task
|
# Same set of allowed file types as in the IMAP task
|
||||||
ALLOWED_MIME_TYPES = {
|
ALLOWED_MIME_TYPES = {
|
||||||
@@ -1044,7 +1119,20 @@ async def ui_upload(request: Request, file: UploadFile = File(...)):
|
|||||||
logger.info(f"Enqueued image for PDF conversion: {target_path}")
|
logger.info(f"Enqueued image for PDF conversion: {target_path}")
|
||||||
elif mime_type in ALLOWED_MIME_TYPES or any(
|
elif mime_type in ALLOWED_MIME_TYPES or any(
|
||||||
file_ext.endswith(ext)
|
file_ext.endswith(ext)
|
||||||
for ext in [".doc", ".docx", ".xls", ".xlsx", ".ppt", ".pptx", ".odt", ".ods", ".odp", ".rtf", ".txt", ".csv"]
|
for ext in [
|
||||||
|
".doc",
|
||||||
|
".docx",
|
||||||
|
".xls",
|
||||||
|
".xlsx",
|
||||||
|
".ppt",
|
||||||
|
".pptx",
|
||||||
|
".odt",
|
||||||
|
".ods",
|
||||||
|
".odp",
|
||||||
|
".rtf",
|
||||||
|
".txt",
|
||||||
|
".csv",
|
||||||
|
]
|
||||||
):
|
):
|
||||||
# If it's an office document, convert to PDF first
|
# If it's an office document, convert to PDF first
|
||||||
task = convert_to_pdf.delay(target_path, original_filename=safe_filename)
|
task = convert_to_pdf.delay(target_path, original_filename=safe_filename)
|
||||||
|
|||||||
+20
-6
@@ -152,15 +152,18 @@ class Settings(BaseSettings):
|
|||||||
|
|
||||||
# Batch processing settings
|
# Batch processing settings
|
||||||
processall_throttle_threshold: int = Field(
|
processall_throttle_threshold: int = Field(
|
||||||
default=20, description="Number of files above which throttling is applied in /processall endpoint"
|
default=20,
|
||||||
|
description="Number of files above which throttling is applied in /processall endpoint",
|
||||||
)
|
)
|
||||||
processall_throttle_delay: int = Field(
|
processall_throttle_delay: int = Field(
|
||||||
default=3, description="Delay in seconds between each task submission when throttling in /processall"
|
default=3,
|
||||||
|
description="Delay in seconds between each task submission when throttling in /processall",
|
||||||
)
|
)
|
||||||
|
|
||||||
# Notification settings
|
# Notification settings
|
||||||
notification_urls: Union[List[str], str] = Field(
|
notification_urls: Union[List[str], str] = Field(
|
||||||
default_factory=list, description="List of Apprise notification URLs (e.g., discord://, telegram://, etc.)"
|
default_factory=list,
|
||||||
|
description="List of Apprise notification URLs (e.g., discord://, telegram://, etc.)",
|
||||||
)
|
)
|
||||||
notify_on_task_failure: bool = Field(default=True, description="Send notifications when Celery tasks fail")
|
notify_on_task_failure: bool = Field(default=True, description="Send notifications when Celery tasks fail")
|
||||||
notify_on_credential_failure: bool = Field(
|
notify_on_credential_failure: bool = Field(
|
||||||
@@ -169,7 +172,8 @@ class Settings(BaseSettings):
|
|||||||
notify_on_startup: bool = Field(default=True, description="Send notifications when application starts")
|
notify_on_startup: bool = Field(default=True, description="Send notifications when application starts")
|
||||||
notify_on_shutdown: bool = Field(default=False, description="Send notifications when application shuts down")
|
notify_on_shutdown: bool = Field(default=False, description="Send notifications when application shuts down")
|
||||||
notify_on_file_processed: bool = Field(
|
notify_on_file_processed: bool = Field(
|
||||||
default=True, description="Send notifications when files are successfully processed"
|
default=True,
|
||||||
|
description="Send notifications when files are successfully processed",
|
||||||
)
|
)
|
||||||
|
|
||||||
# File upload size limits (for security - see SECURITY_AUDIT.md)
|
# File upload size limits (for security - see SECURITY_AUDIT.md)
|
||||||
@@ -184,6 +188,14 @@ class Settings(BaseSettings):
|
|||||||
" it will be split into smaller chunks for processing. Default: None (no splitting)."
|
" it will be split into smaller chunks for processing. Default: None (no splitting)."
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
|
max_request_body_size: int = Field(
|
||||||
|
default=1048576, # 1MB in bytes (1024 * 1024)
|
||||||
|
description=(
|
||||||
|
"Maximum request body size in bytes for non-file-upload requests. Default: 1MB."
|
||||||
|
" Prevents memory exhaustion attacks via oversized JSON/form payloads."
|
||||||
|
" File uploads are governed by MAX_UPLOAD_SIZE instead."
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
# Deduplication settings - prevents processing of duplicate files
|
# Deduplication settings - prevents processing of duplicate files
|
||||||
enable_deduplication: bool = Field(
|
enable_deduplication: bool = Field(
|
||||||
@@ -240,12 +252,14 @@ class Settings(BaseSettings):
|
|||||||
# X-Frame-Options - Prevents clickjacking
|
# X-Frame-Options - Prevents clickjacking
|
||||||
security_header_x_frame_options_enabled: bool = Field(default=True, description="Enable X-Frame-Options header.")
|
security_header_x_frame_options_enabled: bool = Field(default=True, description="Enable X-Frame-Options header.")
|
||||||
security_header_x_frame_options_value: str = Field(
|
security_header_x_frame_options_value: str = Field(
|
||||||
default="DENY", description="X-Frame-Options header value. Options: DENY, SAMEORIGIN, or ALLOW-FROM uri"
|
default="DENY",
|
||||||
|
description="X-Frame-Options header value. Options: DENY, SAMEORIGIN, or ALLOW-FROM uri",
|
||||||
)
|
)
|
||||||
|
|
||||||
# X-Content-Type-Options - Prevents MIME sniffing
|
# X-Content-Type-Options - Prevents MIME sniffing
|
||||||
security_header_x_content_type_options_enabled: bool = Field(
|
security_header_x_content_type_options_enabled: bool = Field(
|
||||||
default=True, description="Enable X-Content-Type-Options header (always set to 'nosniff')."
|
default=True,
|
||||||
|
description="Enable X-Content-Type-Options header (always set to 'nosniff').",
|
||||||
)
|
)
|
||||||
|
|
||||||
# Audit Logging Configuration (see SECURITY_AUDIT.md – Infrastructure Security)
|
# Audit Logging Configuration (see SECURITY_AUDIT.md – Infrastructure Security)
|
||||||
|
|||||||
+16
-3
@@ -20,6 +20,7 @@ from app.config import settings
|
|||||||
from app.database import init_db
|
from app.database import init_db
|
||||||
from app.middleware.audit_log import AuditLogMiddleware
|
from app.middleware.audit_log import AuditLogMiddleware
|
||||||
from app.middleware.rate_limit import create_limiter, get_rate_limit_exceeded_handler
|
from app.middleware.rate_limit import create_limiter, get_rate_limit_exceeded_handler
|
||||||
|
from app.middleware.request_size_limit import RequestSizeLimitMiddleware
|
||||||
from app.middleware.security_headers import SecurityHeadersMiddleware
|
from app.middleware.security_headers import SecurityHeadersMiddleware
|
||||||
from app.utils.config_validator import check_all_configs
|
from app.utils.config_validator import check_all_configs
|
||||||
from app.utils.notification import init_apprise, notify_shutdown, notify_startup
|
from app.utils.notification import init_apprise, notify_shutdown, notify_startup
|
||||||
@@ -116,6 +117,12 @@ app.add_exception_handler(RateLimitExceeded, get_rate_limit_exceeded_handler())
|
|||||||
# Set to False if reverse proxy (Traefik, Nginx) handles security headers
|
# Set to False if reverse proxy (Traefik, Nginx) handles security headers
|
||||||
app.add_middleware(SecurityHeadersMiddleware, config=settings)
|
app.add_middleware(SecurityHeadersMiddleware, config=settings)
|
||||||
|
|
||||||
|
# 2) Request Size Limit Middleware - enforces body size limits before reading
|
||||||
|
# MAX_REQUEST_BODY_SIZE: limit for non-file requests (default 1 MB)
|
||||||
|
# MAX_UPLOAD_SIZE: limit for multipart/form-data uploads (default 1 GB)
|
||||||
|
# See SECURITY_AUDIT.md – Code Security section
|
||||||
|
app.add_middleware(RequestSizeLimitMiddleware, config=settings)
|
||||||
|
|
||||||
# 2) Audit Logging Middleware - logs all requests with sensitive data masking
|
# 2) Audit Logging Middleware - logs all requests with sensitive data masking
|
||||||
# Configure via AUDIT_LOGGING_ENABLED environment variable
|
# Configure via AUDIT_LOGGING_ENABLED environment variable
|
||||||
# See SECURITY_AUDIT.md – Infrastructure Security section
|
# See SECURITY_AUDIT.md – Infrastructure Security section
|
||||||
@@ -128,7 +135,10 @@ app.add_middleware(SessionMiddleware, secret_key=SESSION_SECRET)
|
|||||||
app.add_middleware(ProxyHeadersMiddleware, trusted_hosts="*")
|
app.add_middleware(ProxyHeadersMiddleware, trusted_hosts="*")
|
||||||
|
|
||||||
# 5) Restrict valid hosts to prevent Host header attacks
|
# 5) Restrict valid hosts to prevent Host header attacks
|
||||||
app.add_middleware(TrustedHostMiddleware, allowed_hosts=[settings.external_hostname, "localhost", "127.0.0.1"])
|
app.add_middleware(
|
||||||
|
TrustedHostMiddleware,
|
||||||
|
allowed_hosts=[settings.external_hostname, "localhost", "127.0.0.1"],
|
||||||
|
)
|
||||||
|
|
||||||
# Mount the static files directory
|
# Mount the static files directory
|
||||||
static_dir = pathlib.Path(__file__).parents[1] / "frontend" / "static"
|
static_dir = pathlib.Path(__file__).parents[1] / "frontend" / "static"
|
||||||
@@ -174,13 +184,16 @@ async def custom_500_handler(request: Request, exc: Exception):
|
|||||||
# For API routes, return JSON instead of HTML
|
# For API routes, return JSON instead of HTML
|
||||||
if request.url.path.startswith("/api/"):
|
if request.url.path.startswith("/api/"):
|
||||||
return JSONResponse(
|
return JSONResponse(
|
||||||
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, content={"detail": "Internal server error"}
|
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||||
|
content={"detail": "Internal server error"},
|
||||||
)
|
)
|
||||||
|
|
||||||
# Serve the 500 template for non-API routes
|
# Serve the 500 template for non-API routes
|
||||||
templates = Jinja2Templates(directory=str(static_dir.parent / "templates"))
|
templates = Jinja2Templates(directory=str(static_dir.parent / "templates"))
|
||||||
return templates.TemplateResponse(
|
return templates.TemplateResponse(
|
||||||
"500.html", {"request": request, "exc": exc}, status_code=status.HTTP_500_INTERNAL_SERVER_ERROR
|
"500.html",
|
||||||
|
{"request": request, "exc": exc},
|
||||||
|
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,114 @@
|
|||||||
|
#!/usr/bin/env python3
|
||||||
|
|
||||||
|
"""
|
||||||
|
Request Size Limit Middleware for DocuElevate.
|
||||||
|
|
||||||
|
This middleware enforces configurable size limits on incoming HTTP request bodies
|
||||||
|
to prevent memory exhaustion and Denial-of-Service (DoS) attacks.
|
||||||
|
|
||||||
|
Two independent limits are enforced:
|
||||||
|
- ``MAX_REQUEST_BODY_SIZE``: applied to all non-multipart requests (JSON, form data, etc.).
|
||||||
|
Default: 1 MB. Configurable via the ``MAX_REQUEST_BODY_SIZE`` environment variable.
|
||||||
|
- ``MAX_UPLOAD_SIZE``: applied to multipart/form-data (file upload) requests.
|
||||||
|
Default: 1 GB. Configurable via the ``MAX_UPLOAD_SIZE`` environment variable.
|
||||||
|
|
||||||
|
When a request exceeds the applicable limit the middleware immediately returns
|
||||||
|
``HTTP 413 Request Entity Too Large`` without reading the full body, which keeps
|
||||||
|
memory usage bounded.
|
||||||
|
|
||||||
|
See SECURITY_AUDIT.md – Code Security section for background.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import logging
|
||||||
|
|
||||||
|
from fastapi import Request
|
||||||
|
from fastapi.responses import JSONResponse
|
||||||
|
from starlette.middleware.base import BaseHTTPMiddleware
|
||||||
|
|
||||||
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
|
||||||
|
class RequestSizeLimitMiddleware(BaseHTTPMiddleware):
|
||||||
|
"""
|
||||||
|
Middleware that rejects requests whose body exceeds a configured size limit.
|
||||||
|
|
||||||
|
File-upload requests (``Content-Type: multipart/form-data``) are checked
|
||||||
|
against ``config.max_upload_size``; all other requests are checked against
|
||||||
|
``config.max_request_body_size``.
|
||||||
|
|
||||||
|
The check is performed on the ``Content-Length`` header before the body is
|
||||||
|
read, so oversized requests are rejected without buffering the payload into
|
||||||
|
memory. If the client omits the ``Content-Length`` header the request is
|
||||||
|
passed through to the normal handler (where endpoint-level checks still
|
||||||
|
apply for file uploads).
|
||||||
|
"""
|
||||||
|
|
||||||
|
def __init__(self, app, config):
|
||||||
|
"""
|
||||||
|
Initialize the middleware.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
app: The ASGI application to wrap.
|
||||||
|
config: Application settings object with ``max_request_body_size``
|
||||||
|
and ``max_upload_size`` attributes.
|
||||||
|
"""
|
||||||
|
super().__init__(app)
|
||||||
|
self.max_body_size = config.max_request_body_size
|
||||||
|
self.max_upload_size = config.max_upload_size
|
||||||
|
logger.info(
|
||||||
|
f"Request size limit middleware enabled – "
|
||||||
|
f"body limit: {self.max_body_size} bytes, "
|
||||||
|
f"upload limit: {self.max_upload_size} bytes"
|
||||||
|
)
|
||||||
|
|
||||||
|
async def dispatch(self, request: Request, call_next):
|
||||||
|
"""
|
||||||
|
Check the ``Content-Length`` header and reject oversized requests early.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
request: Incoming HTTP request.
|
||||||
|
call_next: Next middleware or route handler.
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
HTTP 413 response if the request is too large, otherwise the
|
||||||
|
downstream response.
|
||||||
|
"""
|
||||||
|
content_length_header = request.headers.get("content-length")
|
||||||
|
if content_length_header is not None:
|
||||||
|
try:
|
||||||
|
content_length = int(content_length_header)
|
||||||
|
except ValueError:
|
||||||
|
# Malformed header – let downstream handle it
|
||||||
|
return await call_next(request)
|
||||||
|
|
||||||
|
content_type = request.headers.get("content-type", "")
|
||||||
|
is_multipart = "multipart/form-data" in content_type
|
||||||
|
|
||||||
|
if is_multipart:
|
||||||
|
limit = self.max_upload_size
|
||||||
|
limit_description = "file upload"
|
||||||
|
config_var = "MAX_UPLOAD_SIZE"
|
||||||
|
else:
|
||||||
|
limit = self.max_body_size
|
||||||
|
limit_description = "request body"
|
||||||
|
config_var = "MAX_REQUEST_BODY_SIZE"
|
||||||
|
|
||||||
|
if content_length > limit:
|
||||||
|
logger.warning(
|
||||||
|
f"Rejected oversized {limit_description}: "
|
||||||
|
f"{content_length} bytes > {limit} bytes limit "
|
||||||
|
f"(configure with {config_var})"
|
||||||
|
)
|
||||||
|
return JSONResponse(
|
||||||
|
status_code=413,
|
||||||
|
content={
|
||||||
|
"detail": (
|
||||||
|
f"Request body too large: {content_length} bytes "
|
||||||
|
f"(maximum allowed: {limit} bytes). "
|
||||||
|
f"Adjust the {config_var} environment variable to change this limit. "
|
||||||
|
f"See SECURITY_AUDIT.md for details."
|
||||||
|
)
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
return await call_next(request)
|
||||||
@@ -39,12 +39,14 @@ Control how the `/processall` endpoint handles large batches of files to prevent
|
|||||||
|---------------------------|--------------------------------------------------------------------------------------------------------------|---------------|
|
|---------------------------|--------------------------------------------------------------------------------------------------------------|---------------|
|
||||||
| `MAX_UPLOAD_SIZE` | Maximum file upload size in bytes. Files exceeding this limit are rejected. | `1073741824` (1GB) |
|
| `MAX_UPLOAD_SIZE` | Maximum file upload size in bytes. Files exceeding this limit are rejected. | `1073741824` (1GB) |
|
||||||
| `MAX_SINGLE_FILE_SIZE` | Optional: Maximum size for a single file chunk in bytes. Files exceeding this are split into smaller parts. | `None` (no splitting) |
|
| `MAX_SINGLE_FILE_SIZE` | Optional: Maximum size for a single file chunk in bytes. Files exceeding this are split into smaller parts. | `None` (no splitting) |
|
||||||
|
| `MAX_REQUEST_BODY_SIZE` | Maximum request body size in bytes for non-file-upload requests (JSON, form data, etc.). File uploads use `MAX_UPLOAD_SIZE` instead. | `1048576` (1MB) |
|
||||||
|
|
||||||
**Configuration Examples:**
|
**Configuration Examples:**
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
# Default: Allow up to 1GB uploads, no splitting
|
# Default: Allow up to 1GB uploads, no splitting, 1MB JSON/form body limit
|
||||||
MAX_UPLOAD_SIZE=1073741824
|
MAX_UPLOAD_SIZE=1073741824
|
||||||
|
MAX_REQUEST_BODY_SIZE=1048576
|
||||||
|
|
||||||
# Conservative: 100MB max, split files over 50MB
|
# Conservative: 100MB max, split files over 50MB
|
||||||
MAX_UPLOAD_SIZE=104857600
|
MAX_UPLOAD_SIZE=104857600
|
||||||
|
|||||||
+92
-35
@@ -32,7 +32,10 @@ def mock_celery_tasks():
|
|||||||
mock_process_task.delay.return_value = mock_task
|
mock_process_task.delay.return_value = mock_task
|
||||||
mock_convert_task.delay.return_value = mock_task
|
mock_convert_task.delay.return_value = mock_task
|
||||||
|
|
||||||
yield {"process_document": mock_process_task.delay, "convert_to_pdf": mock_convert_task.delay}
|
yield {
|
||||||
|
"process_document": mock_process_task.delay,
|
||||||
|
"convert_to_pdf": mock_convert_task.delay,
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.integration
|
@pytest.mark.integration
|
||||||
@@ -67,7 +70,8 @@ class TestValidFileUploads:
|
|||||||
"""Test uploading a valid text file."""
|
"""Test uploading a valid text file."""
|
||||||
text_content = b"This is a test text file.\nWith multiple lines."
|
text_content = b"This is a test text file.\nWith multiple lines."
|
||||||
response = client.post(
|
response = client.post(
|
||||||
"/api/ui-upload", files={"file": ("document.txt", io.BytesIO(text_content), "text/plain")}
|
"/api/ui-upload",
|
||||||
|
files={"file": ("document.txt", io.BytesIO(text_content), "text/plain")},
|
||||||
)
|
)
|
||||||
|
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
@@ -87,7 +91,10 @@ class TestValidFileUploads:
|
|||||||
b"\xff\xd9"
|
b"\xff\xd9"
|
||||||
)
|
)
|
||||||
|
|
||||||
response = client.post("/api/ui-upload", files={"file": ("image.jpg", io.BytesIO(jpeg_content), "image/jpeg")})
|
response = client.post(
|
||||||
|
"/api/ui-upload",
|
||||||
|
files={"file": ("image.jpg", io.BytesIO(jpeg_content), "image/jpeg")},
|
||||||
|
)
|
||||||
|
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
data = response.json()
|
data = response.json()
|
||||||
@@ -106,7 +113,8 @@ class TestValidFileUploads:
|
|||||||
)
|
)
|
||||||
|
|
||||||
response = client.post(
|
response = client.post(
|
||||||
"/api/ui-upload", files={"file": ("screenshot.png", io.BytesIO(png_content), "image/png")}
|
"/api/ui-upload",
|
||||||
|
files={"file": ("screenshot.png", io.BytesIO(png_content), "image/png")},
|
||||||
)
|
)
|
||||||
|
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
@@ -143,7 +151,10 @@ class TestValidFileUploads:
|
|||||||
"""Test uploading a CSV file."""
|
"""Test uploading a CSV file."""
|
||||||
csv_content = b"name,age,city\nJohn,30,NYC\nJane,25,LA\n"
|
csv_content = b"name,age,city\nJohn,30,NYC\nJane,25,LA\n"
|
||||||
|
|
||||||
response = client.post("/api/ui-upload", files={"file": ("data.csv", io.BytesIO(csv_content), "text/csv")})
|
response = client.post(
|
||||||
|
"/api/ui-upload",
|
||||||
|
files={"file": ("data.csv", io.BytesIO(csv_content), "text/csv")},
|
||||||
|
)
|
||||||
|
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
data = response.json()
|
data = response.json()
|
||||||
@@ -159,27 +170,34 @@ class TestInvalidFileUploads:
|
|||||||
"""Test that files exceeding MAX_UPLOAD_SIZE are rejected."""
|
"""Test that files exceeding MAX_UPLOAD_SIZE are rejected."""
|
||||||
from app.config import settings
|
from app.config import settings
|
||||||
|
|
||||||
# Create a large file content (mock it to avoid memory issues)
|
# Temporarily lower the upload limit so a tiny file exceeds it,
|
||||||
large_content = b"x" * 1024 # 1KB for testing
|
# avoiding the need to allocate a real 1 GB payload in memory.
|
||||||
|
small_limit = 100 # 100 bytes
|
||||||
with patch("os.path.getsize") as mock_getsize:
|
small_content = b"x" * (small_limit + 1)
|
||||||
# Mock the file size to be over the configured limit
|
|
||||||
mock_getsize.return_value = settings.max_upload_size + 1
|
|
||||||
|
|
||||||
|
with patch.object(settings, "max_upload_size", small_limit):
|
||||||
response = client.post(
|
response = client.post(
|
||||||
"/api/ui-upload", files={"file": ("huge.pdf", io.BytesIO(large_content), "application/pdf")}
|
"/api/ui-upload",
|
||||||
|
files={"file": ("huge.pdf", io.BytesIO(small_content), "application/pdf")},
|
||||||
)
|
)
|
||||||
|
|
||||||
assert response.status_code == 413 # Request Entity Too Large
|
assert response.status_code == 413 # Request Entity Too Large
|
||||||
assert "too large" in response.json()["detail"].lower()
|
assert "too large" in response.json()["detail"].lower()
|
||||||
assert "SECURITY_AUDIT.md" in response.json()["detail"]
|
assert "SECURITY_AUDIT.md" in response.json()["detail"]
|
||||||
|
|
||||||
def test_upload_executable_file(self, client: TestClient, mock_celery_tasks):
|
def test_upload_executable_file(self, client: TestClient, mock_celery_tasks):
|
||||||
"""Test that executable files are handled (attempted conversion)."""
|
"""Test that executable files are handled (attempted conversion)."""
|
||||||
exe_content = b"MZ\x90\x00" # PE header
|
exe_content = b"MZ\x90\x00" # PE header
|
||||||
|
|
||||||
response = client.post(
|
response = client.post(
|
||||||
"/api/ui-upload", files={"file": ("program.exe", io.BytesIO(exe_content), "application/x-msdownload")}
|
"/api/ui-upload",
|
||||||
|
files={
|
||||||
|
"file": (
|
||||||
|
"program.exe",
|
||||||
|
io.BytesIO(exe_content),
|
||||||
|
"application/x-msdownload",
|
||||||
|
)
|
||||||
|
},
|
||||||
)
|
)
|
||||||
|
|
||||||
# Per the code, unsupported types get a warning but are still processed
|
# Per the code, unsupported types get a warning but are still processed
|
||||||
@@ -189,7 +207,10 @@ class TestInvalidFileUploads:
|
|||||||
|
|
||||||
def test_upload_empty_file(self, client: TestClient, mock_celery_tasks):
|
def test_upload_empty_file(self, client: TestClient, mock_celery_tasks):
|
||||||
"""Test uploading an empty file."""
|
"""Test uploading an empty file."""
|
||||||
response = client.post("/api/ui-upload", files={"file": ("empty.txt", io.BytesIO(b""), "text/plain")})
|
response = client.post(
|
||||||
|
"/api/ui-upload",
|
||||||
|
files={"file": ("empty.txt", io.BytesIO(b""), "text/plain")},
|
||||||
|
)
|
||||||
|
|
||||||
# Empty files are accepted and queued for processing
|
# Empty files are accepted and queued for processing
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
@@ -216,7 +237,8 @@ class TestUploadSecurity:
|
|||||||
pdf_content = b"%PDF-1.4\n%EOF"
|
pdf_content = b"%PDF-1.4\n%EOF"
|
||||||
|
|
||||||
response = client.post(
|
response = client.post(
|
||||||
"/api/ui-upload", files={"file": (malicious_filename, io.BytesIO(pdf_content), "application/pdf")}
|
"/api/ui-upload",
|
||||||
|
files={"file": (malicious_filename, io.BytesIO(pdf_content), "application/pdf")},
|
||||||
)
|
)
|
||||||
|
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
@@ -233,7 +255,8 @@ class TestUploadSecurity:
|
|||||||
pdf_content = b"%PDF-1.4\n%EOF"
|
pdf_content = b"%PDF-1.4\n%EOF"
|
||||||
|
|
||||||
response = client.post(
|
response = client.post(
|
||||||
"/api/ui-upload", files={"file": (malicious_filename, io.BytesIO(pdf_content), "application/pdf")}
|
"/api/ui-upload",
|
||||||
|
files={"file": (malicious_filename, io.BytesIO(pdf_content), "application/pdf")},
|
||||||
)
|
)
|
||||||
|
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
@@ -249,7 +272,8 @@ class TestUploadSecurity:
|
|||||||
pdf_content = b"%PDF-1.4\n%EOF"
|
pdf_content = b"%PDF-1.4\n%EOF"
|
||||||
|
|
||||||
response = client.post(
|
response = client.post(
|
||||||
"/api/ui-upload", files={"file": (special_filename, io.BytesIO(pdf_content), "application/pdf")}
|
"/api/ui-upload",
|
||||||
|
files={"file": (special_filename, io.BytesIO(pdf_content), "application/pdf")},
|
||||||
)
|
)
|
||||||
|
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
@@ -272,7 +296,8 @@ class TestUploadSecurity:
|
|||||||
pdf_content = b"%PDF-1.4\n%EOF"
|
pdf_content = b"%PDF-1.4\n%EOF"
|
||||||
|
|
||||||
response = client.post(
|
response = client.post(
|
||||||
"/api/ui-upload", files={"file": (malicious_filename, io.BytesIO(pdf_content), "application/pdf")}
|
"/api/ui-upload",
|
||||||
|
files={"file": (malicious_filename, io.BytesIO(pdf_content), "application/pdf")},
|
||||||
)
|
)
|
||||||
|
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
@@ -297,7 +322,8 @@ class TestUploadSecurity:
|
|||||||
pdf_content = b"%PDF-1.4\n%EOF"
|
pdf_content = b"%PDF-1.4\n%EOF"
|
||||||
|
|
||||||
response = client.post(
|
response = client.post(
|
||||||
"/api/ui-upload", files={"file": (malicious_filename, io.BytesIO(pdf_content), "application/pdf")}
|
"/api/ui-upload",
|
||||||
|
files={"file": (malicious_filename, io.BytesIO(pdf_content), "application/pdf")},
|
||||||
)
|
)
|
||||||
|
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
@@ -323,7 +349,8 @@ class TestUploadErrorHandling:
|
|||||||
pdf_content = b"%PDF-1.4\n%EOF"
|
pdf_content = b"%PDF-1.4\n%EOF"
|
||||||
|
|
||||||
response = client.post(
|
response = client.post(
|
||||||
"/api/ui-upload", files={"file": ("test.pdf", io.BytesIO(pdf_content), "application/pdf")}
|
"/api/ui-upload",
|
||||||
|
files={"file": ("test.pdf", io.BytesIO(pdf_content), "application/pdf")},
|
||||||
)
|
)
|
||||||
|
|
||||||
assert response.status_code == 500
|
assert response.status_code == 500
|
||||||
@@ -338,7 +365,10 @@ class TestUploadErrorHandling:
|
|||||||
# The endpoint should still handle the error gracefully
|
# The endpoint should still handle the error gracefully
|
||||||
# In this case, the exception will propagate
|
# In this case, the exception will propagate
|
||||||
with pytest.raises(Exception):
|
with pytest.raises(Exception):
|
||||||
client.post("/api/ui-upload", files={"file": ("test.pdf", io.BytesIO(pdf_content), "application/pdf")})
|
client.post(
|
||||||
|
"/api/ui-upload",
|
||||||
|
files={"file": ("test.pdf", io.BytesIO(pdf_content), "application/pdf")},
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.integration
|
@pytest.mark.integration
|
||||||
@@ -351,11 +381,13 @@ class TestUploadFilenameHandling:
|
|||||||
|
|
||||||
# Upload same file twice
|
# Upload same file twice
|
||||||
response1 = client.post(
|
response1 = client.post(
|
||||||
"/api/ui-upload", files={"file": ("same.pdf", io.BytesIO(pdf_content), "application/pdf")}
|
"/api/ui-upload",
|
||||||
|
files={"file": ("same.pdf", io.BytesIO(pdf_content), "application/pdf")},
|
||||||
)
|
)
|
||||||
|
|
||||||
response2 = client.post(
|
response2 = client.post(
|
||||||
"/api/ui-upload", files={"file": ("same.pdf", io.BytesIO(pdf_content), "application/pdf")}
|
"/api/ui-upload",
|
||||||
|
files={"file": ("same.pdf", io.BytesIO(pdf_content), "application/pdf")},
|
||||||
)
|
)
|
||||||
|
|
||||||
assert response1.status_code == 200
|
assert response1.status_code == 200
|
||||||
@@ -375,7 +407,8 @@ class TestUploadFilenameHandling:
|
|||||||
content = b"Some content"
|
content = b"Some content"
|
||||||
|
|
||||||
response = client.post(
|
response = client.post(
|
||||||
"/api/ui-upload", files={"file": ("NOEXTENSION", io.BytesIO(content), "application/octet-stream")}
|
"/api/ui-upload",
|
||||||
|
files={"file": ("NOEXTENSION", io.BytesIO(content), "application/octet-stream")},
|
||||||
)
|
)
|
||||||
|
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
@@ -394,7 +427,8 @@ class TestUploadMimeTypeDetection:
|
|||||||
pdf_content = b"%PDF-1.4\n%EOF"
|
pdf_content = b"%PDF-1.4\n%EOF"
|
||||||
|
|
||||||
response = client.post(
|
response = client.post(
|
||||||
"/api/ui-upload", files={"file": ("doc.pdf", io.BytesIO(pdf_content), "application/octet-stream")}
|
"/api/ui-upload",
|
||||||
|
files={"file": ("doc.pdf", io.BytesIO(pdf_content), "application/octet-stream")},
|
||||||
)
|
)
|
||||||
|
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
@@ -407,7 +441,14 @@ class TestUploadMimeTypeDetection:
|
|||||||
image_content = b"\x00\x01\x02\x03"
|
image_content = b"\x00\x01\x02\x03"
|
||||||
|
|
||||||
response = client.post(
|
response = client.post(
|
||||||
"/api/ui-upload", files={"file": ("photo.jpg", io.BytesIO(image_content), "application/octet-stream")}
|
"/api/ui-upload",
|
||||||
|
files={
|
||||||
|
"file": (
|
||||||
|
"photo.jpg",
|
||||||
|
io.BytesIO(image_content),
|
||||||
|
"application/octet-stream",
|
||||||
|
)
|
||||||
|
},
|
||||||
)
|
)
|
||||||
|
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
@@ -436,7 +477,10 @@ class TestFileSplitting:
|
|||||||
# Mock should_split_file to return True
|
# Mock should_split_file to return True
|
||||||
with patch("app.utils.file_splitting.should_split_file", return_value=True):
|
with patch("app.utils.file_splitting.should_split_file", return_value=True):
|
||||||
with open(sample_pdf_path, "rb") as f:
|
with open(sample_pdf_path, "rb") as f:
|
||||||
response = client.post("/api/ui-upload", files={"file": ("large.pdf", f, "application/pdf")})
|
response = client.post(
|
||||||
|
"/api/ui-upload",
|
||||||
|
files={"file": ("large.pdf", f, "application/pdf")},
|
||||||
|
)
|
||||||
|
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
data = response.json()
|
data = response.json()
|
||||||
@@ -459,7 +503,10 @@ class TestFileSplitting:
|
|||||||
# Ensure max_single_file_size is None (default)
|
# Ensure max_single_file_size is None (default)
|
||||||
with patch.object(settings, "max_single_file_size", None):
|
with patch.object(settings, "max_single_file_size", None):
|
||||||
with open(sample_pdf_path, "rb") as f:
|
with open(sample_pdf_path, "rb") as f:
|
||||||
response = client.post("/api/ui-upload", files={"file": ("document.pdf", f, "application/pdf")})
|
response = client.post(
|
||||||
|
"/api/ui-upload",
|
||||||
|
files={"file": ("document.pdf", f, "application/pdf")},
|
||||||
|
)
|
||||||
|
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
data = response.json()
|
data = response.json()
|
||||||
@@ -479,7 +526,10 @@ class TestFileSplitting:
|
|||||||
# Configure a very large limit
|
# Configure a very large limit
|
||||||
with patch.object(settings, "max_single_file_size", 1000000000): # 1GB limit
|
with patch.object(settings, "max_single_file_size", 1000000000): # 1GB limit
|
||||||
with open(sample_pdf_path, "rb") as f:
|
with open(sample_pdf_path, "rb") as f:
|
||||||
response = client.post("/api/ui-upload", files={"file": ("small.pdf", f, "application/pdf")})
|
response = client.post(
|
||||||
|
"/api/ui-upload",
|
||||||
|
files={"file": ("small.pdf", f, "application/pdf")},
|
||||||
|
)
|
||||||
|
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
data = response.json()
|
data = response.json()
|
||||||
@@ -498,9 +548,15 @@ class TestFileSplitting:
|
|||||||
with patch.object(settings, "max_single_file_size", 100): # Small limit
|
with patch.object(settings, "max_single_file_size", 100): # Small limit
|
||||||
with patch("app.utils.file_splitting.should_split_file", return_value=True):
|
with patch("app.utils.file_splitting.should_split_file", return_value=True):
|
||||||
# Mock split_pdf_by_size to raise an exception
|
# Mock split_pdf_by_size to raise an exception
|
||||||
with patch("app.utils.file_splitting.split_pdf_by_size", side_effect=Exception("Split failed")):
|
with patch(
|
||||||
|
"app.utils.file_splitting.split_pdf_by_size",
|
||||||
|
side_effect=Exception("Split failed"),
|
||||||
|
):
|
||||||
with open(sample_pdf_path, "rb") as f:
|
with open(sample_pdf_path, "rb") as f:
|
||||||
response = client.post("/api/ui-upload", files={"file": ("document.pdf", f, "application/pdf")})
|
response = client.post(
|
||||||
|
"/api/ui-upload",
|
||||||
|
files={"file": ("document.pdf", f, "application/pdf")},
|
||||||
|
)
|
||||||
|
|
||||||
# Should still succeed, falling back to processing the whole file
|
# Should still succeed, falling back to processing the whole file
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
@@ -526,7 +582,8 @@ class TestFileSplitting:
|
|||||||
)
|
)
|
||||||
|
|
||||||
response = client.post(
|
response = client.post(
|
||||||
"/api/ui-upload", files={"file": ("image.png", io.BytesIO(image_content), "image/png")}
|
"/api/ui-upload",
|
||||||
|
files={"file": ("image.png", io.BytesIO(image_content), "image/png")},
|
||||||
)
|
)
|
||||||
|
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
|
|||||||
@@ -0,0 +1,172 @@
|
|||||||
|
"""
|
||||||
|
Tests for the RequestSizeLimitMiddleware.
|
||||||
|
|
||||||
|
Validates that:
|
||||||
|
- Non-file requests exceeding MAX_REQUEST_BODY_SIZE are rejected with HTTP 413
|
||||||
|
- Multipart/form-data uploads exceeding MAX_UPLOAD_SIZE are rejected with HTTP 413
|
||||||
|
- Requests within the limits pass through normally
|
||||||
|
- Missing Content-Length header does not cause false rejections
|
||||||
|
"""
|
||||||
|
|
||||||
|
import io
|
||||||
|
from unittest.mock import patch
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
from fastapi.testclient import TestClient
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.unit
|
||||||
|
class TestRequestSizeLimitMiddleware:
|
||||||
|
"""Unit tests for the RequestSizeLimitMiddleware dispatch logic."""
|
||||||
|
|
||||||
|
def test_middleware_rejects_oversized_json_body(self, client: TestClient):
|
||||||
|
"""Non-file request with Content-Length exceeding MAX_REQUEST_BODY_SIZE is rejected."""
|
||||||
|
from app.config import settings
|
||||||
|
|
||||||
|
oversized = settings.max_request_body_size + 1
|
||||||
|
response = client.post(
|
||||||
|
"/api/process-url",
|
||||||
|
content=b"x" * 10, # actual body doesn't matter; header is checked first
|
||||||
|
headers={
|
||||||
|
"Content-Length": str(oversized),
|
||||||
|
"Content-Type": "application/json",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
assert response.status_code == 413
|
||||||
|
detail = response.json()["detail"]
|
||||||
|
assert "MAX_REQUEST_BODY_SIZE" in detail
|
||||||
|
|
||||||
|
def test_middleware_allows_request_within_json_limit(self, client: TestClient):
|
||||||
|
"""Non-file request with Content-Length within limit is not rejected by middleware."""
|
||||||
|
from app.config import settings
|
||||||
|
|
||||||
|
# Send a body within limit; the endpoint may return 4xx for its own reasons,
|
||||||
|
# but the middleware must NOT return 413.
|
||||||
|
small = settings.max_request_body_size - 1
|
||||||
|
response = client.post(
|
||||||
|
"/api/process-url",
|
||||||
|
content=b"{}",
|
||||||
|
headers={"Content-Length": str(small), "Content-Type": "application/json"},
|
||||||
|
)
|
||||||
|
# The endpoint may return 400/422 (bad JSON or auth), but NOT 413 from middleware
|
||||||
|
assert response.status_code != 413
|
||||||
|
|
||||||
|
def test_middleware_rejects_oversized_multipart_upload(self, client: TestClient):
|
||||||
|
"""Multipart upload with Content-Length exceeding MAX_UPLOAD_SIZE is rejected."""
|
||||||
|
from app.config import settings
|
||||||
|
|
||||||
|
oversized = settings.max_upload_size + 1
|
||||||
|
response = client.post(
|
||||||
|
"/api/ui-upload",
|
||||||
|
content=b"x" * 10,
|
||||||
|
headers={
|
||||||
|
"Content-Length": str(oversized),
|
||||||
|
"Content-Type": "multipart/form-data; boundary=boundary",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
assert response.status_code == 413
|
||||||
|
detail = response.json()["detail"]
|
||||||
|
assert "MAX_UPLOAD_SIZE" in detail
|
||||||
|
|
||||||
|
def test_middleware_allows_multipart_within_upload_limit(self, client: TestClient):
|
||||||
|
"""Multipart upload with Content-Length within MAX_UPLOAD_SIZE passes middleware."""
|
||||||
|
from app.config import settings
|
||||||
|
|
||||||
|
# A Content-Length within the upload limit should NOT be rejected by the middleware.
|
||||||
|
# The endpoint itself will reject because the body is not a real multipart payload.
|
||||||
|
within_limit = min(1024, settings.max_upload_size - 1)
|
||||||
|
response = client.post(
|
||||||
|
"/api/ui-upload",
|
||||||
|
content=b"x" * 10,
|
||||||
|
headers={
|
||||||
|
"Content-Length": str(within_limit),
|
||||||
|
"Content-Type": "multipart/form-data; boundary=boundary",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
# Not rejected by middleware (may be 400/422 from endpoint)
|
||||||
|
assert response.status_code != 413
|
||||||
|
|
||||||
|
def test_middleware_allows_request_without_content_length(self, client: TestClient):
|
||||||
|
"""Requests without Content-Length header pass through middleware (no false rejection)."""
|
||||||
|
# Remove Content-Length header entirely; middleware must not reject
|
||||||
|
response = client.get("/api/files")
|
||||||
|
# May get 200/401/403 but not 413
|
||||||
|
assert response.status_code != 413
|
||||||
|
|
||||||
|
def test_middleware_error_message_contains_limit_and_config_hint(self, client: TestClient):
|
||||||
|
"""413 response body contains limit details and config variable name."""
|
||||||
|
from app.config import settings
|
||||||
|
|
||||||
|
oversized = settings.max_request_body_size + 1
|
||||||
|
response = client.post(
|
||||||
|
"/api/process-url",
|
||||||
|
content=b"{}",
|
||||||
|
headers={
|
||||||
|
"Content-Length": str(oversized),
|
||||||
|
"Content-Type": "application/json",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
assert response.status_code == 413
|
||||||
|
detail = response.json()["detail"]
|
||||||
|
assert str(settings.max_request_body_size) in detail
|
||||||
|
assert "SECURITY_AUDIT.md" in detail
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.integration
|
||||||
|
class TestFileUploadSizeLimitStreaming:
|
||||||
|
"""Integration tests for streaming size enforcement in the ui-upload endpoint."""
|
||||||
|
|
||||||
|
@pytest.fixture(autouse=True)
|
||||||
|
def mock_celery(self):
|
||||||
|
with (
|
||||||
|
patch("app.api.files.process_document") as mock_proc,
|
||||||
|
patch("app.api.files.convert_to_pdf") as mock_conv,
|
||||||
|
):
|
||||||
|
from unittest.mock import MagicMock
|
||||||
|
|
||||||
|
task = MagicMock()
|
||||||
|
task.id = "test-task-id"
|
||||||
|
mock_proc.delay.return_value = task
|
||||||
|
mock_conv.delay.return_value = task
|
||||||
|
yield
|
||||||
|
|
||||||
|
def test_upload_rejected_when_content_length_declared_too_large(self, client: TestClient):
|
||||||
|
"""Upload is rejected early via Content-Length check before reading data."""
|
||||||
|
from app.config import settings
|
||||||
|
|
||||||
|
oversized = settings.max_upload_size + 1
|
||||||
|
pdf_data = b"%PDF-1.4\n%EOF"
|
||||||
|
response = client.post(
|
||||||
|
"/api/ui-upload",
|
||||||
|
content=pdf_data,
|
||||||
|
headers={
|
||||||
|
"Content-Length": str(oversized),
|
||||||
|
"Content-Type": "multipart/form-data; boundary=boundary",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
assert response.status_code == 413
|
||||||
|
|
||||||
|
def test_upload_rejected_mid_stream_when_data_exceeds_limit(self, client: TestClient):
|
||||||
|
"""Upload is rejected mid-stream when actual data exceeds max_upload_size."""
|
||||||
|
from app.config import settings
|
||||||
|
|
||||||
|
# Temporarily reduce max_upload_size to a tiny value for this test
|
||||||
|
small_limit = 100 # 100 bytes
|
||||||
|
with patch.object(settings, "max_upload_size", small_limit):
|
||||||
|
large_content = b"x" * (small_limit + 1)
|
||||||
|
response = client.post(
|
||||||
|
"/api/ui-upload",
|
||||||
|
files={"file": ("big.pdf", io.BytesIO(large_content), "application/pdf")},
|
||||||
|
)
|
||||||
|
assert response.status_code == 413
|
||||||
|
assert "too large" in response.json()["detail"].lower()
|
||||||
|
|
||||||
|
def test_upload_succeeds_within_size_limit(self, client: TestClient):
|
||||||
|
"""Small, valid file upload completes successfully within size limits."""
|
||||||
|
pdf_content = b"%PDF-1.4\n1 0 obj\n<</Type /Catalog>>\nendobj\n%%EOF"
|
||||||
|
response = client.post(
|
||||||
|
"/api/ui-upload",
|
||||||
|
files={"file": ("small.pdf", io.BytesIO(pdf_content), "application/pdf")},
|
||||||
|
)
|
||||||
|
assert response.status_code == 200
|
||||||
|
assert response.json()["status"] == "queued"
|
||||||
Reference in New Issue
Block a user