fix: enforce FILE_SIZE_LIMIT on published Space asset upload#9242
Open
sriramveeraghanta wants to merge 2 commits into
Open
fix: enforce FILE_SIZE_LIMIT on published Space asset upload#9242sriramveeraghanta wants to merge 2 commits into
sriramveeraghanta wants to merge 2 commits into
Conversation
The public Space asset upload endpoint
(POST /api/public/assets/v2/anchor/{anchor}/) trusted the client-supplied
`size` value end-to-end: it was stored on the FileAsset and passed straight
to generate_presigned_post(), which uses it as the S3/MinIO policy bound
(["content-length-range", 1, file_size]). This let an authenticated user
obtain a signed upload policy exceeding the instance's FILE_SIZE_LIMIT.
Cap the value with `size_limit = min(size, settings.FILE_SIZE_LIMIT)` and use
it consistently for the stored asset metadata and the presigned POST policy,
matching every other asset upload endpoint.
sriramveeraghanta
requested review from
dheeru0198 and
pablohashescobar
as code owners
Contributor
📝 WalkthroughWalkthroughIn ChangesUpload Size Cap in EntityAssetEndpoint
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
No React Doctor issues found. 🎉 Reviewed by React Doctor for commit |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/plane/space/views/asset.py`:
- Around line 83-85: The size_limit calculation at line 85 only enforces an
upper bound but allows non-positive size values to pass through. Add validation
before the size_limit assignment to check if size is less than or equal to 0,
and if so, return a 400 Bad Request error response. Then update the size_limit
calculation to clamp the value to the range [1, FILE_SIZE_LIMIT] using max(1,
min(size, settings.FILE_SIZE_LIMIT)) to enforce both the lower and upper bounds,
ensuring only valid asset sizes are used for signing uploads.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d91f263a-6928-41d5-8251-51e7db161ffa
📒 Files selected for processing (1)
apps/api/plane/space/views/asset.py
Address review feedback: reject malformed (non-integer) `size` with 400 and clamp the value to [1, FILE_SIZE_LIMIT] via max(1, min(...)) so the presigned content-length-range is always valid and no non-positive size is persisted.
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
1 participant
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
The published Space asset upload endpoint
POST /api/public/assets/v2/anchor/{anchor}/(apps/api/plane/space/views/asset.py) trusted the client-suppliedsizevalue end-to-end. It readsize = int(request.data.get("size", settings.FILE_SIZE_LIMIT)), stored that value on theFileAsset, and passed it directly tostorage.generate_presigned_post(..., file_size=size), which uses it as the upper bound of the signed S3/MinIO policy (["content-length-range", 1, file_size]).Because the policy is signed by the backend, an authenticated user could intercept the asset-metadata request, set
sizeabove the instance limit, and obtain a presigned POST policy permitting an oversized upload — bypassing the configuredFILE_SIZE_LIMITand enabling storage abuse / resource exhaustion through published issue-comment uploads.This was the only asset upload path missing the cap that every other endpoint (
app/views/asset/v2.py,api/views/asset.py,app/views/issue/attachment.py) already applies.Fix: cap the client-provided size with
size_limit = min(size, settings.FILE_SIZE_LIMIT)and usesize_limitconsistently for the stored asset metadata (attributes+size) and thegenerate_presigned_post()call, so the signed policy can never exceed the instance limit.Reported via responsible disclosure to security@plane.so.
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
FILE_SIZE_LIMIT=5242880(5 MB), callPOST /api/public/assets/v2/anchor/{anchor}/withsizeset to10485760(10 MB). Decode the returned presigned policy and confirmcontent-length-rangeupper bound is5242880, not the requested10485760.FileAsset.sizeandattributes.sizeequal the capped value (≤FILE_SIZE_LIMIT), not the client-supplied value.PATCHmark uploaded).References
Summary by CodeRabbit
Release Notes