Skip to content

functional: do not add an unsigned x-amz-acl to presigned PUTs - #1724

Merged
harshavardhana merged 1 commit into
minio:masterfrom
harshavardhana:fix-presigned-put-unsigned-acl
Sep 27, 2026
Merged

harshavardhana merged 1 commit into
minio:masterfrom
harshavardhana:fix-presigned-put-unsigned-acl

Conversation

@harshavardhana

@harshavardhana harshavardhana commented Sep 27, 2026 •

Copy link
Copy Markdown
Member

TestArgs.writeObject() adds x-amz-acl: bucket-owner-full-control to the request it sends to a presigned PUT URL. The header
is added after signing, so it is not in X-Amz-SignedHeaders.

AWS S3 refuses any x-amz-* header that the signature does not cover, with 403 AccessDenied ("There were headers present in
the request which were not signed") and a HeadersNotSigned element. MinIO AIStor now enforces the same rule (miniohq/aistor
#7691), and getPresignedObjectUrl() [PUT] fails against it:

<Code>AccessDenied</Code><Message>There were headers present in the request which were not signed</Message>
... <HeadersNotSigned>x-amz-acl</HeadersNotSigned>

writeObject() has one caller, the presigned PUT test, so the "objects created anonymously" reason in the removed comment no
longer applies. This removes the header.

Summary by CodeRabbit

  • Bug Fixes
    • PUT requests no longer include the bucket-owner-full-control ACL header. The request URL, body, and handling of unsuccessful responses remain unchanged.

writeObject() is only used by the presigned PUT test, and it added x-amz-acl after the URL was signed. S3 refuses any
x-amz-* header the signature does not cover (403 AccessDenied, HeadersNotSigned), so the test fails against AWS and
against MinIO AIStor servers that enforce the same rule.
@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 3b3b0273-9715-4b07-b5e2-4353f0e1b756

📥 Commits

Reviewing files that changed from the base of the PR and between 2841d7a and b7488ed.

📒 Files selected for processing (1)
  • functional/TestArgs.java
💤 Files with no reviewable changes (1)
  • functional/TestArgs.java

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

The PUT request in writeObject no longer sets the x-amz-acl: bucket-owner-full-control header. The request URL, body, and unsuccessful-response handling remain unchanged.

Changes

PUT request header

Layer / File(s) Summary
Remove ACL header
functional/TestArgs.java
writeObject no longer adds the bucket-owner-full-control ACL header to PUT requests. It still sends the request to the supplied URL with the provided data.

Priority: ➖ Normal

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to b7488

The header change addresses the reported presigned PUT rejection. No actionable merge-blocking risk was established.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: removing the unsigned x-amz-acl header from presigned PUT requests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

A rabbit watched the headers go,
One less tag for PUT to show.
The URL and data still arrive,
The request keeps its simple drive.
Soft paws tap a quiet cheer,
No ACL header travels here.

Comment @coderabbitai help to get the list of available commands.

@harshavardhana
harshavardhana merged commit a2bad3e into minio:master Sep 27, 2026
13 checks passed
@harshavardhana
harshavardhana deleted the fix-presigned-put-unsigned-acl branch September 27, 2026 05:54
@harshavardhana

Copy link
Copy Markdown
Member Author

Merging this as this is blocking a CVE PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant