Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
8 changes: 6 additions & 2 deletions apps/api/plane/settings/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,13 @@ def generate_presigned_post(self, object_name, file_type, file_size, expiration=
expiration = self.signed_url_expiration
fields = {"Content-Type": file_type}

# boto3.generate_presigned_post auto-injects {"bucket": ...} and
# {"key": ...} conditions when Bucket=/Key= are passed below; do NOT
# also add them by hand here. AWS S3 silently accepts the resulting
# over-sized policy, but stricter S3-compatible backends enforce a
# hard cap on the `policy` form field (some as low as 1024 bytes)
# and reject the upload with `MaxMessageLengthExceeded`.
conditions = [
{"bucket": self.aws_storage_bucket_name},
["content-length-range", 1, file_size],
{"Content-Type": file_type},
]
Expand All @@ -79,7 +84,6 @@ def generate_presigned_post(self, object_name, file_type, file_size, expiration=
conditions.append(["starts-with", "$key", object_name[: -len("${filename}")]])
else:
fields["key"] = object_name
conditions.append({"key": object_name})

# Generate the presigned POST URL
try:
Expand Down
41 changes: 41 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,47 @@ 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",
},
clear=True,
)
@patch("plane.settings.storage.boto3")
def test_generate_presigned_post_does_not_duplicate_bucket_or_key(self, mock_boto3):
"""boto3 auto-injects {bucket}/{key} when Bucket=/Key= are passed.

Adding them again by hand pads the policy by ~80 bytes — a no-op on
AWS S3, but enough to overflow stricter S3-compatible backends that
cap the `policy` field at 1024 bytes and reject the upload with
`MaxMessageLengthExceeded`.

This is the regression test for that fix.
"""
mock_s3_client = Mock()
mock_s3_client.generate_presigned_post.return_value = {
"url": "https://test-url.com",
"fields": {},
}
mock_boto3.client.return_value = mock_s3_client
storage = S3Storage()

storage.generate_presigned_post("test-object", "image/png", 1024)

kw = mock_s3_client.generate_presigned_post.call_args[1]
# Bucket+Key must be passed as named args (boto3 derives conditions from them).
assert kw["Bucket"] == "test-bucket"
assert kw["Key"] == "test-object"
# Conditions must NOT contain hand-rolled {"bucket": ...} or {"key": ...}.
for cond in kw["Conditions"]:
if isinstance(cond, dict):
assert "bucket" not in cond, f"hand-rolled bucket condition: {cond}"
assert "key" not in cond, f"hand-rolled key condition: {cond}"

@patch.dict(
os.environ,
{
Expand Down