Backport 239a6b4#14224
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive file validation framework to enhance security during uploads. It implements a new FileValidationUploadHandler that utilizes libmagic for early content-type sniffing and adds a dedicated ZIP validation utility to protect against path traversal and ZIP bombs. Additionally, the documents API has been hardened by restricting allowed HTTP methods and ensuring that file-related metadata is read-only. Feedback suggests including the files field in the read_only_fields of the DocumentSerializer to fully align with the intended lockdown and adding a null check for file_name in the upload handler to prevent potential TypeError exceptions.
| # name and extension determine where the document file lives on disk | ||
| # and how it is served. POST/PUT are blocked at the http_method_names | ||
| # layer; making them read-only locks them down on PATCH too. | ||
| read_only_fields = ("name", "extension") |
There was a problem hiding this comment.
The PR description states that files is locked down, but it is not included in read_only_fields. If files is a field in the serializer, it remains mutable via PATCH requests. Please add files to read_only_fields to ensure it is locked down as intended.
| read_only_fields = ("name", "extension") | |
| read_only_fields = ("name", "extension", "files") |
| self.magic_mimetype_map = self.validation_config.get("magic_mimetype_map", {}) | ||
| self.magic_description_map = self.validation_config.get("magic_description_map", {}) | ||
|
|
||
| self.extension = Path(self.file_name).suffix.replace(".", "").lower() |
There was a problem hiding this comment.
Accessing self.file_name directly in Path() can raise a TypeError if file_name is None. It is safer to provide a default empty string.
| self.extension = Path(self.file_name).suffix.replace(".", "").lower() | |
| self.extension = Path(self.file_name or "").suffix.replace(".", "").lower() |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 5.0.x #14224 +/- ##
========================================
Coverage ? 74.29%
========================================
Files ? 940
Lines ? 56288
Branches ? 7641
========================================
Hits ? 41820
Misses ? 12791
Partials ? 1677 🚀 New features to boost your workflow:
|
1 participant
No description provided.