feat(api): allow text/html attachments - #9552
Open
Semih702 wants to merge 1 commit into
Open
Conversation
text/html and application/xhtml+xml were missing from ATTACHMENT_MIME_TYPES, so uploading an .html file to a work item failed with "Invalid file type." HTML reports (Playwright, Jest, Lighthouse, JaCoCo) are a common thing to attach, and the workaround so far has been to zip the file or rename it to .txt, which loses the extension. Both types are already members of SCRIPT_CAPABLE_MIME_TYPES, so every asset download path already pins them to Content-Disposition: attachment (makeplane#9312 / GHSA-ch8j-vr4r-qf6h). The browser downloads them rather than rendering them, which is the same protection image/svg+xml has had since that fix. Closes makeplane#9543
Contributor
📝 WalkthroughWalkthroughThe attachment allowlist now accepts ChangesHTML attachment support
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
text/htmlandapplication/xhtml+xmlwere missing fromATTACHMENT_MIME_TYPES, so uploading an.htmlfile to a work item fails with400 {"error": "Invalid file type."}. HTML reports (Playwright, Jest, Lighthouse, JaCoCo), exported emails and BI exports are common things to attach to a work item, and the workaround today is to zip the file or rename it to.txt, which loses the extension.This adds both types to the allowlist. No other change is needed.
On the security side: the obvious objection here is stored XSS, but that path is already closed. Both types are already members of
SCRIPT_CAPABLE_MIME_TYPES, and since #9312 / GHSA-ch8j-vr4r-qf6h every asset download path pins those types toContent-Disposition: attachment:GenericAssetEndpoint.get—apps/api/plane/api/views/asset.pyStaticFileAssetEndpoint.get—apps/api/plane/app/views/asset/v2.pyEntityAssetEndpoint.get—apps/api/plane/space/views/asset.pyThe two work item attachment download endpoints (
IssueAttachmentV2Endpoint.getand its/api/v1sibling) hardcodedisposition="attachment"regardless of type. I went through everyS3Storage.generate_presigned_urlcall site in the repo — all of them either hardcodeattachmentor derive it fromSCRIPT_CAPABLE_MIME_TYPES, none fall through to theinlinedefault. So an uploaded HTML file gets downloaded, never rendered on the app's origin.The precedent is
image/svg+xml, which is equally script-capable, already uploadable, and protected by exactly this mechanism.Type of Change
Screenshots and Media (if applicable)
Not applicable — no UI change. The upload dropzone has no client-side
acceptfilter, so the server allowlist is the only gate.Test Scenarios
Added
apps/api/plane/tests/contract/app/test_issue_attachment_mime_types_app.pycoveringIssueAttachmentV2Endpoint:text/htmlandapplication/xhtml+xmlreturns 200 and stores the type (fails onpreviewwithout this change)disposition="attachment"SCRIPT_CAPABLE_MIME_TYPES— this is the guard that keeps the allowlist entry safe, so it fails loudly if someone ever drops them from thereRun via
docker-compose-test.yml:Full API suite: 523 passed with this branch, against a 516-passed baseline on
preview(7 new tests, nothing else changed).ruff checkandruff formatare clean on both files.I did leave
ACCEPTED_ATTACHMENT_MIME_TYPESinpackages/editoralone — that list gates editor drag-and-drop, which is a separate surface from work item attachments, and I didn't want to change its behaviour in this PR. Happy to add it there too if you'd prefer them kept in sync.References
Closes #9543
Related: #9312 / GHSA-ch8j-vr4r-qf6h