Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 26 additions & 1 deletion apps/api/plane/settings/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,34 @@ def __init__(self, request=None):
)

def generate_presigned_post(self, object_name, file_type, file_size, expiration=None):
"""Generate a presigned URL to upload an S3 object"""
if expiration is None:
expiration = self.signed_url_expiration

# Check if we are using Cloudflare R2 (which doesn't support S3 Presigned POST nicely)
is_r2 = self.aws_s3_endpoint_url and "r2.cloudflarestorage" in self.aws_s3_endpoint_url
if is_r2:
try:
response_url = self.s3_client.generate_presigned_url(
ClientMethod="put_object",
Params={
"Bucket": self.aws_storage_bucket_name,
"Key": object_name,
"ContentType": file_type,
"ContentLength": file_size,
},
ExpiresIn=expiration,
)
return {
"url": response_url,
"fields": {
"_method": "PUT",
"Content-Type": file_type
}
}
Comment on lines 65 to +89
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Preserve file-size enforcement for R2 uploads.

The POST path enforces file_size with a signed content-length-range, but the R2 PUT branch ignores file_size, so a client can request an upload for an allowed size and PUT a larger object. Add a post-upload head_object validation before marking the asset uploaded, deleting/rejecting objects over the limit, or use an upload mechanism that can enforce content length server-side.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/api/plane/settings/storage.py` around lines 65 - 88, The R2 PUT branch
in generate_presigned_post returns a presigned PUT URL but ignores the requested
file_size, allowing oversized uploads; to fix this, after the client signals
upload completion (implement or update the upload-complete handler that
currently marks assets uploaded), call
s3_client.head_object(Bucket=self.aws_storage_bucket_name, Key=object_name) to
read ContentLength, compare it to the file_size passed to
generate_presigned_post, and if it exceeds the limit call
s3_client.delete_object(Bucket=..., Key=...) and raise/return an error (and log)
so the asset is rejected; ensure you catch and handle S3 errors and keep the
existing behavior for non-R2 paths that use content-length-range.

except ClientError as e:
print(f"Error generating presigned PUT URL for R2: {e}")
return None

fields = {"Content-Type": file_type}

conditions = [
Expand Down
40 changes: 40 additions & 0 deletions apps/api/plane/tests/unit/settings/test_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,46 @@ def test_generate_presigned_post_uses_custom_expiration(self, mock_boto3):
call_kwargs = mock_s3_client.generate_presigned_post.call_args[1]
assert call_kwargs["ExpiresIn"] == 60

@patch.dict(
os.environ,
{
"AWS_ACCESS_KEY_ID": "test-key",
"AWS_SECRET_ACCESS_KEY": "test-secret",
"AWS_S3_BUCKET_NAME": "test-bucket",
"AWS_REGION": "us-east-1",
"AWS_S3_ENDPOINT_URL": "https://test.r2.cloudflarestorage.com",
},
clear=True,
)
@patch("plane.settings.storage.boto3")
def test_generate_presigned_post_with_cloudflare_r2(self, mock_boto3):
"""Test that Cloudflare R2 endpoints generate presigned PUT URLs instead of POST"""
# Mock the boto3 client and its response
mock_s3_client = Mock()
mock_s3_client.generate_presigned_url.return_value = "https://r2-test-url.com"
mock_boto3.client.return_value = mock_s3_client

# Create S3Storage instance
storage = S3Storage()

# Call generate_presigned_post
result = storage.generate_presigned_post("test-object", "image/png", 1024)

# Assert that the boto3 method generate_presigned_url was called with put_object
mock_s3_client.generate_presigned_url.assert_called_once()
call_kwargs = mock_s3_client.generate_presigned_url.call_args[1]
assert call_kwargs["ClientMethod"] == "put_object"
assert call_kwargs["Params"]["Bucket"] == "test-bucket"
assert call_kwargs["Params"]["Key"] == "test-object"
assert call_kwargs["Params"]["ContentType"] == "image/png"
assert call_kwargs["Params"]["ContentLength"] == 1024

# Verify the returned object structure
assert result["url"] == "https://r2-test-url.com"
assert result["fields"]["_method"] == "PUT"
assert result["fields"]["Content-Type"] == "image/png"


@patch.dict(
os.environ,
{
Expand Down
22 changes: 17 additions & 5 deletions apps/web/core/services/file-upload.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import type { AxiosRequestConfig } from "axios";
import axios from "axios";
import { CancelToken, isCancel } from "axios";
// services
import { APIService } from "@/services/api.service";

Expand All @@ -21,18 +21,30 @@ export class FileUploadService extends APIService {
data: FormData,
uploadProgressHandler?: AxiosRequestConfig["onUploadProgress"]
): Promise<void> {
this.cancelSource = axios.CancelToken.source();
return this.post(url, data, {
this.cancelSource = CancelToken.source();

const isPut = data.has("_method") && data.get("_method") === "PUT";
const requestMethod = isPut ? "put" : "post";
// For PUT (Cloudflare R2), we send the raw File blob, not FormData. The helper appended it as 'file'
const requestData = isPut ? data.get("file") : data;

if (isPut && !(requestData instanceof Blob)) {
return Promise.reject(new Error("Invalid or missing file data for upload."));
}

const contentType = isPut ? data.get("Content-Type") || "application/octet-stream" : "multipart/form-data";

return this[requestMethod](url, requestData, {
headers: {
"Content-Type": "multipart/form-data",
"Content-Type": contentType as string,
Comment thread
coderabbitai[bot] marked this conversation as resolved.
},
cancelToken: this.cancelSource.token,
withCredentials: false,
onUploadProgress: uploadProgressHandler,
})
.then((response) => response?.data)
.catch((error) => {
if (axios.isCancel(error)) {
if (isCancel(error)) {
console.log(error.message);
} else {
throw error?.response?.data;
Expand Down