-
Notifications
You must be signed in to change notification settings - Fork 583
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Restrict upload of binary or unknown file types by default #1507
Conversation
Reviewer's Guide by SourceryThis PR implements security improvements by restricting the upload of binary and unknown file types by default. The implementation adds a default validator for 'application/octet-stream' MIME type in the FILE_VALIDATORS settings and provides a way to customize validation through configuration settings. Class diagram for file validation changesclassDiagram
class FilerConfig {
+FILE_VALIDATORS: dict
}
class ValidationSettings {
+FILER_REMOVE_FILE_VALIDATORS: list
+FILER_ADD_FILE_VALIDATORS: dict
}
FilerConfig --> ValidationSettings
note for FilerConfig "FILE_VALIDATORS now includes 'application/octet-stream' with a default deny validator."
note for ValidationSettings "Settings can be customized to add or remove file validators."
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @fsbraun - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a migration guide section to the documentation to help users transition from the previous behavior, especially for cases where they need to explicitly enable binary file uploads.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟡 Testing: 2 issues found
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1507 +/- ##
==========================================
+ Coverage 76.59% 76.73% +0.14%
==========================================
Files 75 77 +2
Lines 3546 3624 +78
Branches 568 519 -49
==========================================
+ Hits 2716 2781 +65
- Misses 667 677 +10
- Partials 163 166 +3 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
@sourcery-ai review |
I'm sorry, I don't understand the command Please use |
Description
This PR changes the default settings for file validations to deny upload of binary or unknown file types. Uploading unknown file types is a security risk which will have to be enabled first by setting:
The docs now include an example of how to run binary file uploads by a virus checker (ClamAV in the example)
It bumps the version to 3.3 since it will require an explicit update of settings for some use cases.
Related resources
Checklist
master
Slack to find a “pr review buddy” who is going to review my pull request.