Skip to content

Add MP4 as default allowed attachment type #18170

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jan 4, 2022
Merged

Add MP4 as default allowed attachment type #18170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c2baa66
Add MP4 as default allowed attachment type
Jan 3, 2022
f766636
Merge branch 'main' into add-mp4
Jan 3, 2022
08a6ad7
Merge branch 'main' into add-mp4
zeripath Jan 3, 2022
ecd26b5
Merge branch 'main' into add-mp4
lunny Jan 4, 2022
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
2 changes: 1 addition & 1 deletion custom/conf/app.example.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1617,7 +1617,7 @@ PATH =
;ENABLED = true
;;
;; Comma-separated list of allowed file extensions (`.zip`), mime types (`text/plain`) or wildcard type (`image/*`, `audio/*`, `video/*`). Empty value or `*/*` allows all types.
;ALLOWED_TYPES = .docx,.gif,.gz,.jpeg,.jpg,.log,.pdf,.png,.pptx,.txt,.xlsx,.zip
;ALLOWED_TYPES = .docx,.gif,.gz,.jpeg,.jpg,.mp4,.log,.pdf,.png,.pptx,.txt,.xlsx,.zip
;;
;; Max size of each file. Defaults to 4MB
;MAX_SIZE = 4
Expand Down
2 changes: 1 addition & 1 deletion docs/content/doc/advanced/config-cheat-sheet.en-us.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,7 @@ Default templates for project boards:
## Issue and pull request attachments (`attachment`)

- `ENABLED`: **true**: Whether issue and pull request attachments are enabled.
- `ALLOWED_TYPES`: **.docx,.gif,.gz,.jpeg,.jpg,.log,.pdf,.png,.pptx,.txt,.xlsx,.zip**: Comma-separated list of allowed file extensions (`.zip`), mime types (`text/plain`) or wildcard type (`image/*`, `audio/*`, `video/*`). Empty value or `*/*` allows all types.
- `ALLOWED_TYPES`: **.docx,.gif,.gz,.jpeg,.jpg,mp4,.log,.pdf,.png,.pptx,.txt,.xlsx,.zip**: Comma-separated list of allowed file extensions (`.zip`), mime types (`text/plain`) or wildcard type (`image/*`, `audio/*`, `video/*`). Empty value or `*/*` allows all types.
- `MAX_SIZE`: **4**: Maximum size (MB).
- `MAX_FILES`: **5**: Maximum number of attachments that can be uploaded at once.
- `STORAGE_TYPE`: **local**: Storage type for attachments, `local` for local disk or `minio` for s3 compatible object storage service, default is `local` or other name defined with `[storage.xxx]`
Expand Down
2 changes: 1 addition & 1 deletion modules/setting/attachment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func newAttachmentService() {

Attachment.Storage = getStorage("attachments", storageType, sec)

Attachment.AllowedTypes = sec.Key("ALLOWED_TYPES").MustString(".docx,.gif,.gz,.jpeg,.jpg,.log,.pdf,.png,.pptx,.txt,.xlsx,.zip")
Attachment.AllowedTypes = sec.Key("ALLOWED_TYPES").MustString(".docx,.gif,.gz,.jpeg,.jpg,.mp4,.log,.pdf,.png,.pptx,.txt,.xlsx,.zip")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it is kinda unrelated, but why are svg, md, and csv missing?
Those three are also fairly popular formats with basically no security implications.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

md, and csv

Both looks good to me to be added...

svg

I haven't looked all that deep into the code that serves the actual file, but svg allows for inline script(and we don't want to serve malicious files) also golang/go#15888

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^ Related to svg #1095

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I didn't know that you can embed JS inside svgs.
I mean, it makes sense that it depends on the renderer whether it's actually interpreted or not.

On the other hand, we also allow docx files that can contain macros.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, it makes sense that it depends on the renderer whether it's actually interpreted or not.

Yeah, if I know that it will be downloaded and not executed, then we can include it safely.

On the other hand, we also allow docx files that can contain macros.

Yeah, but docx aren't executed immediately within the browsers. And nowadays the popular software open most docx in a safe mode.

Attachment.MaxSize = sec.Key("MAX_SIZE").MustInt64(4)
Attachment.MaxFiles = sec.Key("MAX_FILES").MustInt(5)
Attachment.Enabled = sec.Key("ENABLED").MustBool(true)
Expand Down