Hey !
(I hope this isn’t a duplicate, but didn’t find much discussion about this neither in issues nor here)
Isn’t defaulting to an empty string for MEDIA_ROOT dangerous ?
Out of the box, if you just add a “FileField” to your model and don’t define MEDIA_ROOT in you settings, files are uploaded to the current working directory (which in most cases would be your source directory). Also, if you serve media filesin such a setup, you’ll be leaking your source code.
In amateur deployments (writable source dir, hardcoded config, etc.) that could easily become very serious. Usually Django is super helpful in helping devs no to shoot themsleves in the foot.
I would suggest:
- to default MEDIA_ROOT to None instead of “”, so that functions like django.views.static.serve fail
- to add a check MEDIA_ROOT != current directory, in the same way there is a check that MEDIA_ROOT != STATIC_ROOT (not sure if we could go further - is there a way to determine whether a MEDIA_ROOT is safe ?)
IMO these would be reasonably backwards compatible, anyone using media files should definitely have set they MEDIA_ROOT setting.
Let me know what you think ! Happy to open an issue if there seem to be some consensus.
Note: see CRITICAL: fallback to MEDIA_ROOT should be clearly documented to prevent leaking the source folder · Issue #65 · kimetrica/django-binary-database-files · GitHub which is where I stumbled upon this (TLDR: didn’t know that app also serves files from MEDIA_ROOT, which I hadn’t configured, then realised i was leaking my source files)