shacker / django-todo

A multi-user, multi-group todo/ticketing system for Django projects. Includes CSV import and integrated mail tracking.
http://django-todo.org
BSD 3-Clause "New" or "Revised" License
819 stars 285 forks source link

if "*" in TODO_LIMIT_FILE_ATTACHMENTS, allow all #79

Closed james1293 closed 5 years ago

james1293 commented 5 years ago

What does everyone think about this? I am not a security expert, so feel free to shoot down the idea.

shacker commented 5 years ago

I didn't see an Issue filed to go with this. So there is currently no way to allow any file attachment type? I have to say I would NEVER do that on my site - people could attach viruses of all kinds, then those attachments appear on the web page, to be downloaded by any allowed visitor. That feels like an invitation to trouble. No, I think we should continue to make it explicit. This feels like too wide of an open door.

james1293 commented 5 years ago

Understood. Side question on GitHub best practices - should I have created an issue in addition to this PR? I didn't want to introduce unnecessary clutter, but I'm happy to follow standard procedures :-)

shacker commented 5 years ago

Not sure about etiquette, but the most common workflow is to start something as an issue, discuss it with participants, then do a pull request. But for things that are small or have an obvious solution, it's also common to post an issue and a PR at the same time.