Skip to content

Fix duplicate conditions in S3 presigned POST policy generation #8541

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

Open
ebati wants to merge 1 commit into
base: preview
Choose a base branch
from
+4 −9
Open

Fix duplicate conditions in S3 presigned POST policy generation #8541

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 4 additions & 9 deletions apps/api/plane/settings/storage.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,21 +62,16 @@ def generate_presigned_post(self, object_name, file_type, file_size, expiration=
"""Generate a presigned URL to upload an S3 object"""
if expiration is None:
expiration = self.signed_url_expiration
fields = {"Content-Type": file_type}
fields = {
"Content-Type": file_type,
"bucket": self.aws_storage_bucket_name,
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

According to the boto3 documentation and the PR description, bucket should not be included in the Fields parameter. The boto3 library automatically handles the bucket when it's passed as the Bucket parameter to generate_presigned_post(). Including it in fields will still cause duplicate bucket conditions in the generated policy. This line should be removed, leaving only the Content-Type field.

Suggested change
"bucket": self.aws_storage_bucket_name,

Copilot uses AI. Check for mistakes.
Copy link
Author

@ebati ebati Jan 14, 2026

Choose a reason for hiding this comment

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

Documentation only mentions conditions and not fields for bucket.

}
Copy link

@cursor cursor bot Jan 14, 2026

Choose a reason for hiding this comment

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

Bucket incorrectly added to fields instead of being removed

High Severity

The PR description states the intent is to remove manually-set bucket and key to let boto3 handle them. However, the code now adds "bucket": self.aws_storage_bucket_name to the fields dictionary. The bucket is not a valid form field in S3 POST requests—it's determined by the URL endpoint. boto3 handles bucket automatically by adding it to policy conditions. This extra form field could cause upload failures, particularly on stricter S3-compatible services like Garage that this PR is meant to support.

Fix in Cursor Fix in Web

Comment on lines +65 to +68
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 14, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for generate_presigned_post usage and tests in the codebase
rg -n 'generate_presigned_post' --type=py -A 10 -B 2

Repository: makeplane/plane

Length of output: 17121

🌐 Web query:

boto3 s3 generate_presigned_post fields parameter bucket documentation

💡 Result:

Short answer: The boto3 S3 client method generate_presigned_post(Bucket, Key, Fields=None, Conditions=None, ExpiresIn=3600) requires Bucket and Key. The Fields parameter is a dict of pre-filled form fields you want included in the POST (examples accepted: acl, Cache-Control, Content-Type, Content-Disposition, Content-Encoding, Expires, success_action_redirect / success_action_status, and x-amz-meta-*). Bucket- and Key-related conditions/fields are handled for you and should not be included in Fields or in Conditions; if you include an element in Fields you must also include a matching Condition. See the official docs. [1][2]

Sources

  • Boto3 generate_presigned_post documentation. [1]
  • Botocore / boto3 S3 client reference (generate_presigned_post parameters and notes). [2]

Remove bucket from Fields dictionary.

Per AWS boto3 documentation, bucket and key are handled automatically by boto3 and should not be included in the Fields parameter. Including bucket in Fields violates AWS S3 POST requirements and contradicts the PR objective to remove duplicates. Additionally, if an element is included in Fields, a matching condition must be present—bucket is not in the Conditions array, creating an inconsistency.

🤖 Prompt for AI Agents 
In `@apps/api/plane/settings/storage.py` around lines 65 - 68, Remove the "bucket"
entry from the Fields dictionary used to build the S3 POST parameters: in the
method that constructs the fields dict (the variable named fields in
settings/storage.py), delete the "bucket": self.aws_storage_bucket_name line so
bucket is no longer included in Fields; rely on boto3 to handle bucket/key
automatically and ensure Conditions remains consistent with the remaining
Fields.


conditions = [
{"bucket": self.aws_storage_bucket_name},
["content-length-range", 1, file_size],
{"Content-Type": file_type},
]

# Add condition for the object name (key)
if object_name.startswith("${filename}"):
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:
# Generate a presigned URL for the S3 object
Expand Down
Loading