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: 5 additions & 0 deletions .jules/sentinel.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,8 @@
**Learning:** `slowapi` decorators (like `@limiter.limit`) require the decorated FastAPI endpoint function to accept a `request: Request` argument, even if the endpoint logic itself doesn't use the request object, because `slowapi` extracts the client IP and other metadata from it to track rate limits.
**Prevention:** Always verify that every endpoint in a FastAPI application, including simple informational endpoints, is protected by appropriate rate limits and ensures `request: Request` is included in the signature when applying `@limiter.limit`.


## 2025-05-11 - Zip Extraction Path Traversal and Sensitive File Bypass
**Vulnerability:** The ZIP extraction logic in `app/routers/upload.py` implemented custom, inline path traversal and sensitive file checks that differed from the application's standard file upload validation. This could potentially allow bypasses if edge cases were missed. Additionally, the standard normalizer (`normalize_uploaded_filename`) can be misleading if its return value is ignored.
**Learning:** Security validation logic must be unified across all upload paths. Implementing separate inline checks for ZIP members creates an inconsistent security boundary. Furthermore, when using a normalizer, it is critical to use its *return value* as the finalized path. Calling a normalizer but continuing to use the raw unsanitized input path completely bypasses the intended security sanitizations.
**Prevention:** Unify validation logic by always calling the core `normalize_uploaded_filename()` utility on extracted ZIP members, and explicitly using its return value (`safe_filename = normalize_uploaded_filename(...)`) for all subsequent path operations to enforce a single, robust security boundary.
Comment on lines +36 to +39
22 changes: 5 additions & 17 deletions app/routers/upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,30 +140,18 @@ def upload_project(
detail=f"Too many files in zip archive (max {MAX_ZIP_FILES})",
)

safe_filename = member.filename.lstrip("/\\")
target_path = os.path.join(tmp_dir_real, safe_filename)
extracted_path = os.path.realpath(target_path)

try:
safe_filename = normalize_uploaded_filename(member.filename)
Comment on lines 143 to +144
target_path = os.path.join(tmp_dir_real, safe_filename)
extracted_path = os.path.realpath(target_path)

if (
not extracted_path.startswith(tmp_dir_real + os.sep)
and extracted_path != tmp_dir_real
):
raise ValueError("Path traversal detected")

sensitive_names = {
".env",
".git",
".ssh",
".aws",
".config",
}
for part in safe_filename.replace("\\", "/").split("/"):
if part in sensitive_names:
raise ValueError(
f"Sensitive hidden file or directory not allowed: {part}"
)
except ValueError:
except (HTTPException, ValueError):
# Let's not expose the exact reason to avoid information disclosure, just standard Unsafe zip file.
raise HTTPException(
status_code=400,
Expand Down
Loading