fix: skip ACL params when bucketObjectsACL is false

[ghost]

Summary

Fixes uploads failing with AccessControlListNotSupported on S3 buckets
with BucketOwnerEnforced policy. Since April 2023, AWS S3 creates all new
buckets with ACLs disabled by default, but lib/storage/s3.js always sent
ACL headers in every request with no way to disable them.

When bucketObjectsACL: false is passed, ACL headers are now skipped
entirely in copyIn, enable, and disable calls.

Closes #5366

What are the specific steps to test this change?

1. Create an AWS S3 bucket with ObjectOwnership: BucketOwnerEnforced (default since April 2023)
2. Configure uploadfs with bucketObjectsACL: false and disabledBucketObjectsACL: false
3. Try uploading a file — it should succeed without ACL errors
4. Verify existing users without these options still get public-read default behavior

What kind of change does this PR introduce?

• Bug fix

Make sure the PR fulfills these requirements:

• It includes the existing issue ID being resolved (S3: uploads fail with AccessControlListNotSupported on buckets with BucketOwnerEnforced #5366)
• The changelog is updated
• Related documentation has been updated
• Related tests have been updated

Other information:

Note: I have not updated the changelog or tests yet —
happy to do so if the maintainers approve this approach first.

[boutell]

This eliminates the ability to enable or disable files entirely, or makes it only pretend to do those things, which would not meet the need.

A better approach would be to look at what's done in the local and azure storage backends, both of which handle disabling access by hashing the filename in a secure way.

Ideally that approach should be made available globally with reusable logic.

[boutell]

Thanks for contributing!

Unfortunately this eliminates the ability to enable or disable files entirely, or makes it only pretend to do those things, which would not meet the need.

A better approach would be to look at what's done in the local and azure storage backends, both of which handle disabling access by hashing the filename in a secure way.

Ideally that approach should be made available globally with reusable logic.

[boutell]

(Submissions must also pass the test suite.)

[ghost]

Hi @boutell — I've refactored the approach based on your feedback:

disable() and enable() now use filename hashing via utils.getDisabledPath() when disabledFileKey is set — same pattern as the local and azure backends
Old ACL-based path kept as fallback for backward compatibility
copyIn now also skips ACL param when bucketObjectsACL: false

Please let me know if this approach looks good and I'll update the changelog and tests too!

[boutell]

Sounds promising. We'll check it out in a week and a half or so, I am on vacation. Thank you! THOMAS BOUTELL | CHIEF EXECUTIVE OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

…

On Sun, May 17, 2026, 3:48 AM Anjali ray ***@***.***> wrote: *anjali-rayy* left a comment (apostrophecms/apostrophe#5411) <#5411 (comment)> Hi @boutell <https://github.com/boutell> — I've refactored the approach based on your feedback: disable() and enable() now use filename hashing via utils.getDisabledPath() when disabledFileKey is set — same pattern as the local and azure backends Old ACL-based path kept as fallback for backward compatibility copyIn now also skips ACL param when bucketObjectsACL: false Please let me know if this approach looks good and I'll update the changelog and tests too! — Reply to this email directly, view it on GitHub <#5411 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAH27NPCJEDA36KFJTLRL343CZWJAVCNFSM6AAAAACY4ZDHDOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DINRXG44TKNZXGA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[ghost]

Hi @boutell — I've updated the PR with the filename hashing approach using utils.getDisabledPath() for disable/enable, same as the local and azure backends, with ACL fallback for backward compatibility.
Changelog and tests will be added once you confirm this looks good.
Enjoy your vacation!

[boutell]

We'll need test coverage showing the tests work with or without this option. Also a minor nitpick about code formatting.

We're also going to need to think about a migration path, but this is fine for new projects as written, so I won't hold you up for that.

[boutell]

Please follow the convention (no single-line statements in if, use {})