functional: do not add an unsigned x-amz-acl to presigned PUTs - #1724
Conversation
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.
|
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 configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe PUT request in ChangesPUT request header
Priority: ➖ Normal Estimated code review effort: 1 (Trivial) | ~3 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The header change addresses the reported presigned PUT rejection. No actionable merge-blocking risk was established. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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. A rabbit watched the headers go, Comment |
|
Merging this as this is blocking a CVE PR |
TestArgs.writeObject()addsx-amz-acl: bucket-owner-full-controlto the request it sends to a presigned PUT URL. The headeris 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 403AccessDenied("There were headers present inthe request which were not signed") and a
HeadersNotSignedelement. MinIO AIStor now enforces the same rule (miniohq/aistor#7691), and
getPresignedObjectUrl() [PUT]fails against it:writeObject()has one caller, the presigned PUT test, so the "objects created anonymously" reason in the removed comment nolonger applies. This removes the header.
Summary by CodeRabbit
bucket-owner-full-controlACL header. The request URL, body, and handling of unsuccessful responses remain unchanged.