Description of the Bug
In backend/app/routes/documents.py, the validate_upload function creates a temporary file at the start of validation:
temp_path = str(upload_dir / f"{uuid.uuid4()}{ext}")
with open(temp_path, "wb") as temp_file:
shutil.copyfileobj(file.file, temp_file)
If validation fails after the file is written but before the success path returns it, the temp file should be cleaned up. The cleanup code does handle common failure paths, but there are edge cases:
- If
shutil.copyfileobj() succeeds but Path(temp_path).stat().st_size raises a filesystem error, the temp file is leaked
- If an unexpected exception occurs between file creation and the validation check, the
finally block does not clean up (it's intentionally a no-op since caller manages the file)
- The
try/finally in the calling function (upload_document) only cleans up on error if the temp path was successfully returned, but on some exception paths the temp_path is never returned and never cleaned up
This causes disk space accumulation over time, especially with repeated failed uploads.
Steps to Reproduce
- Trigger a filesystem error while
Path(temp_path).stat().st_size is executing (e.g., by removing the directory concurrently)
- The exception propagates without cleaning up the temp file
- The temp file remains on disk permanently
Expected Behavior
Temp files should be cleaned up on ALL failure paths, not just the expected ones. A robust pattern is:
temp_path = None
try:
temp_path = str(upload_dir / f"{uuid.uuid4()}{ext}")
# ... validation logic ...
return temp_path
except Exception:
if temp_path and Path(temp_path).exists():
Path(temp_path).unlink()
raise
Affected File
backend/app/routes/documents.py (lines ~107-149)
Suggested Fix
Add a comprehensive cleanup in the except block to delete the temp file on any failure. Only the caller should receive the path on success.
GSSoC '26
Description of the Bug
In
backend/app/routes/documents.py, thevalidate_uploadfunction creates a temporary file at the start of validation:If validation fails after the file is written but before the success path returns it, the temp file should be cleaned up. The cleanup code does handle common failure paths, but there are edge cases:
shutil.copyfileobj()succeeds butPath(temp_path).stat().st_sizeraises a filesystem error, the temp file is leakedfinallyblock does not clean up (it's intentionally a no-op since caller manages the file)try/finallyin the calling function (upload_document) only cleans up on error if the temp path was successfully returned, but on some exception paths the temp_path is never returned and never cleaned upThis causes disk space accumulation over time, especially with repeated failed uploads.
Steps to Reproduce
Path(temp_path).stat().st_sizeis executing (e.g., by removing the directory concurrently)Expected Behavior
Temp files should be cleaned up on ALL failure paths, not just the expected ones. A robust pattern is:
Affected File
backend/app/routes/documents.py(lines ~107-149)Suggested Fix
Add a comprehensive cleanup in the
exceptblock to delete the temp file on any failure. Only the caller should receive the path on success.GSSoC '26