feat: honor predefinedAcl and acl on bucket creation - #2309
Open
gaul wants to merge 1 commit into
Open
Conversation
buckets.insert accepts a predefinedAcl query parameter and an acl list
in the request body, and the real API applies either to the new bucket.
The handler decoded only name, versioning and defaultEventBasedHold, so
both were accepted and silently dropped: a client asking for a
publicRead bucket got a private one and no error, and had to follow up
with a separate call to /b/{bucket}/acl to get the access it had
already requested.
Nothing else was missing -- BucketAttrs already carries ACL, the backend
already stores it, and the standalone bucket ACL endpoints already read
and write it. Only the create path ignored the request.
Decode acl into the existing aclRule type, which already unmarshals the
entity and role field names, and expand predefinedAcl for the values
that name a public or authenticated scope. private and projectPrivate
map to no rules, matching what the emulator already produces for a
bucket created without an ACL, so the default is unchanged. The
project-scoped roles the real API adds are omitted because the emulator
does not model projects.
Reject a request carrying both predefinedAcl and acl, and one naming an
unknown predefinedAcl, rather than silently preferring one.
Objects already had this: upload.go reads predefinedAcl on every upload
path and maps it through getObjectACL. Buckets were the gap.
gaul
added a commit
to gaul/s3proxy
that referenced
this pull request
Jul 31, 2026
createContainer issued buckets.insert and then, for a public-read container, a separate bucket ACL request whose failure it discarded. A caller asking for a public bucket could therefore be handed a private one and told it succeeded -- and would be, on any project with uniform bucket-level access, which rejects bucket ACLs outright and is the configuration Google now recommends. buckets.insert accepts a predefined ACL, so name it there and let the one request settle both. fake-gcs-server accepts the parameter and drops it, so only the real endpoint takes that path; every other endpoint keeps the second request, which the emulator does implement. The test is the endpoint rather than whether it is local, because an emulator is usually reached by container hostname rather than at localhost, and reading a hostname as the real service is the direction that loses the ACL silently. Dropping the swallowed exception is the substance of this: on the emulator path a failed ACL is now reported rather than passed off as a public bucket. The catch claimed the emulator did not support the operation, which it does -- the full fake-gcs-server lane passes without it. The atomic branch cannot run in CI, since no lane speaks to Google. It was exercised by hand against an emulator carrying fsouza/fake-gcs-server#2309, which teaches buckets.insert the parameter; once that is released the split collapses to a single unconditional path that every lane exercises. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
gaul
added a commit
to gaul/s3proxy
that referenced
this pull request
Jul 31, 2026
handleCopyBlob copied the object and then, for any canned ACL other than private, issued PutObjectAcl against the copy. A failure there left an object the caller had asked to be public readable by nobody, behind a copy that had already answered 200 -- and the ACL request is the more likely of the two to fail, since a bucket with uniform access rejects it outright. CopyOptions had no blobAccess to carry, which is what the comment above that call blamed on jclouds. 71a2427 forked the blobstore API, so the type is this project's own now: give it a blobAccess defaulting to private, exactly as PutOptions has, and let each store apply it to the request that creates the object. S3 names a canned ACL on CopyObject, GCS a predefined ACL on the rewrite, and the nio2 stores hand it to the put that already sets permissions on the temporary file before renaming it into place. Azure and Swift grant read at the container and have no per-blob ACL to set: their setBlobAccess throws, which is what answered NotImplemented before. Refuse the copy the same way rather than accept the request and quietly produce a private object. The test is skipped on those two for that reason, and on GCS for an unrelated one: the copy does carry destinationPredefinedAcl, confirmed on the wire, but fake-gcs-server drops it and gives the destination the source's ACL. The lane passes against an emulator carrying fsouza/fake-gcs-server#2309 and its companion for objects. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
buckets.insert accepts a predefinedAcl query parameter and an acl list in the request body, and the real API applies either to the new bucket. The handler decoded only name, versioning and defaultEventBasedHold, so both were accepted and silently dropped: a client asking for a publicRead bucket got a private one and no error, and had to follow up with a separate call to /b/{bucket}/acl to get the access it had already requested.
Nothing else was missing -- BucketAttrs already carries ACL, the backend already stores it, and the standalone bucket ACL endpoints already read and write it. Only the create path ignored the request.
Decode acl into the existing aclRule type, which already unmarshals the entity and role field names, and expand predefinedAcl for the values that name a public or authenticated scope. private and projectPrivate map to no rules, matching what the emulator already produces for a bucket created without an ACL, so the default is unchanged. The project-scoped roles the real API adds are omitted because the emulator does not model projects.
Reject a request carrying both predefinedAcl and acl, and one naming an unknown predefinedAcl, rather than silently preferring one.
Objects already had this: upload.go reads predefinedAcl on every upload path and maps it through getObjectACL. Buckets were the gap.