fix(s3): judge the BucketOwnerEnforced exception against the bucket owner

crates/fakecloud-s3/src/service/config/mod.rs

@@ -386,6 +386,24 @@ impl S3Service {
         }
     }
 
+    /// The canonical user the bucket's ACL is owned by, which is who "bucket
+    /// owner full control" means. Usually the caller, but not always: a bucket
+    /// persisted by one account is hydrated into the configured default account
+    /// on restart while keeping its own stored `acl_owner_id`.
+    ///
+    /// `None` when the bucket is not in memory. That is not an error here, and
+    /// callers may fall back to the caller's own id: every consumer of this pairs
+    /// it with [`S3Service::bucket_owner_enforced`], which returns `false` for
+    /// exactly the same two misses, so the fallback value is never the one an
+    /// answer turns on -- and the missing bucket is reported by the operation
+    /// itself.
+    pub(super) fn bucket_acl_owner_id(&self, account_id: &str, bucket: &str) -> Option<String> {
+        let accts = self.state.read();
+        let state = accts.get(account_id)?;
+        let b = state.buckets.get(bucket)?;
+        Some(b.acl_owner_id.clone())
+    }
+
     /// Returns true when the bucket has
     /// `ObjectOwnership=BucketOwnerEnforced` set in its
     /// `OwnershipControls` configuration. Under that mode AWS

crates/fakecloud-s3/src/service/mod.rs

@@ -249,15 +249,9 @@ impl S3Service {
         bucket: &str,
         headers: &HeaderMap,
     ) -> Result<WriteAclHeaders, AwsServiceError> {
-        // A present-but-blank value is treated as absent, the same rule
-        // `has_grant_headers` applies to the `x-amz-grant-*` family and for the
-        // same reason: it is what a client sends for an unset config field, and
-        // testing presence alone turned that into a hard 400 on every
-        // ACL-accepting write.
         let canned = headers
             .get("x-amz-acl")
             .and_then(|v| v.to_str().ok())
-            .filter(|s| !s.trim().is_empty())
             .map(|s| s.to_string());
         if let Some(acl) = canned.as_deref() {
             validate_object_canned_acl(acl)?;
@@ -274,11 +268,28 @@ impl S3Service {
                 "Specifying both Canned ACLs and Header Grants is not allowed",
             ));
         }
+        // The bucket's ACL owner, which is who "bucket owner full control" means
+        // -- not the caller. They coincide for a bucket this account created, and
+        // diverge for one persisted by another account and hydrated into the
+        // default account on restart, where comparing against the caller accepted
+        // a grant to somebody who is not the owner and refused the grant to the
+        // owner that AWS accepts.
+        //
+        // Looked up only when the request actually names an ACL: the overwhelming
+        // majority of object writes carry none, and this takes the global read
+        // lock. Resolved once and used for both the grants below and the exception
+        // check, so the two cannot derive the owner differently.
+        let bucket_owner = (canned.is_some() || grants.is_some())
+            .then(|| self.bucket_acl_owner_id(account_id, bucket))
+            .flatten();
+        let owner_id = bucket_owner.as_deref().unwrap_or(account_id);
+
         // The grants the request resolves to, which is what the public-ACL check
-        // below has to judge.
+        // below has to judge. Resolved against the bucket's owner, which is what
+        // the write paths store them under.
         let requested = match (&grants, canned.as_deref()) {
             (Some(g), _) => Some(g.clone()),
-            (None, Some(acl)) => Some(canned_acl_grants_for_object(acl, account_id)),
+            (None, Some(acl)) => Some(canned_acl_grants_for_object(acl, owner_id)),
             (None, None) => None,
         };
 
@@ -297,15 +308,14 @@ impl S3Service {
         // "bucket owner full control".
         let asks_for_owner_full_control = match (&grants, canned.as_deref()) {
             // "an equivalent form of this ACL": explicit grants that give the
-            // owner full control and nobody anything.
-            (Some(g), _) => {
-                !g.is_empty()
-                    && g.iter().all(|grant| {
-                        grant.permission == "FULL_CONTROL"
-                            && grant.grantee_type == "CanonicalUser"
-                            && grant.grantee_id.as_deref() == Some(account_id)
-                    })
-            }
+            // owner full control and nobody anything. `resolved_grant_headers`
+            // never yields an empty vector (it errors instead), so every grant
+            // here is a real clause from the request.
+            (Some(g), _) => g.iter().all(|grant| {
+                grant.permission == "FULL_CONTROL"
+                    && grant.grantee_type == "CanonicalUser"
+                    && grant.grantee_id.as_deref() == Some(owner_id)
+            }),
             (None, Some(acl)) => acl == "bucket-owner-full-control",
             (None, None) => false,
         };

crates/fakecloud-s3/src/service/tests.rs

@@ -3885,26 +3885,45 @@ fn bucket_owner_full_control_is_accepted_when_ownership_disables_acls() {
     // Assert what it resolves to, not that the function echoed back the header
     // it was handed: the whole point is that the owner ends up with full
     // control and nobody else appears.
+    // Resolved against the bucket's own ACL owner, which is what the write paths
+    // pass and therefore what ends up stored.
     let grants = allowed
-        .grants_for("123456789012")
+        .grants_for("owner")
         .expect("the canned value resolves to grants");
     assert_eq!(grants.len(), 1, "{grants:?}");
     assert_eq!(grants[0].permission, "FULL_CONTROL");
     assert_eq!(grants[0].grantee_type, "CanonicalUser");
-    assert_eq!(grants[0].grantee_id.as_deref(), Some("123456789012"));
+    assert_eq!(grants[0].grantee_id.as_deref(), Some("owner"));
 
     // The exception is about what the ACL GRANTS, not how it is spelled: AWS
-    // accepts "an equivalent form of this ACL" too, so an explicit
-    // full-control grant to the owner is accepted just like the canned value.
+    // accepts "an equivalent form of this ACL" too, so an explicit full-control
+    // grant to the BUCKET OWNER is accepted just like the canned value.
+    //
+    // `seed_bucket` owns this bucket as "owner" while the caller is
+    // 123456789012, which is the case that distinguishes the two. They coincide
+    // for a bucket the caller created; they diverge for one persisted by another
+    // account and hydrated into the default account on restart.
     svc.resolve_write_acl_headers("123456789012", "bofc", &{
         let mut h = HeaderMap::new();
-        h.insert(
-            "x-amz-grant-full-control",
-            "id=123456789012".parse().unwrap(),
-        );
+        h.insert("x-amz-grant-full-control", "id=owner".parse().unwrap());
         h
     })
-    .expect("an owner-only grant is the equivalent form and must be accepted");
+    .expect("full control to the bucket owner is the equivalent form");
+
+    // ...and a grant to anyone else is not, even the caller. Such a request
+    // would leave an object on a BucketOwnerEnforced bucket whose ACL gives the
+    // bucket owner nothing, which is the state that setting exists to prevent.
+    let err = svc
+        .resolve_write_acl_headers("123456789012", "bofc", &{
+            let mut h = HeaderMap::new();
+            h.insert(
+                "x-amz-grant-full-control",
+                "id=123456789012".parse().unwrap(),
+            );
+            h
+        })
+        .expect_err("full control to a non-owner is not the equivalent form");
+    assert_eq!(err.code(), "AccessControlListNotSupported", "{err:?}");
 
     // Every other ACL is refused. The model's wording is narrow -- such a
     // bucket "only accept[s] PUT requests that don't specify an ACL or PUT
@@ -3988,28 +4007,31 @@ async fn put_object_honors_the_bucket_owner_enforced_rules_end_to_end() {
 }
 
 #[tokio::test]
-async fn a_blank_acl_header_is_treated_as_no_acl() {
-    // A present-but-blank header is what a client sends for an unset config
-    // field; the `x-amz-grant-*` family already treats it as absent, and
-    // 400-ing on the canned one made that inconsistent.
+async fn a_blank_acl_header_is_rejected_on_every_acl_setting_path() {
+    // `""` is not a member of `ObjectCannedACL`, and every path that accepts a
+    // canned ACL answers 400 for it. Treating it as "no ACL asked for" on the
+    // object-write paths alone made one operation disagree with four; the
+    // `x-amz-grant-*` family's blank rule is a different case, since an empty
+    // value there names no grantee rather than naming an invalid ACL.
     let svc = make_service();
     seed_bucket(&svc, "blank-acl");
 
-    let resolved = svc
+    let err = svc
         .resolve_write_acl_headers("123456789012", "blank-acl", &{
             let mut h = HeaderMap::new();
             h.insert("x-amz-acl", "".parse().unwrap());
             h
         })
-        .expect("a blank canned ACL asks for nothing, so it is not an error");
-    assert_eq!(resolved.canned, None);
-    assert!(resolved.grants_for("123456789012").is_none());
+        .expect_err("a blank canned ACL is not a canned ACL");
+    assert_eq!(err.code(), "InvalidArgument", "{err:?}");
 
     let mut req = make_request(Method::PUT, "/blank-acl/k", &[], b"body");
     req.headers.insert("x-amz-acl", "".parse().unwrap());
-    svc.put_object("123456789012", &req, "blank-acl", "k")
-        .await
-        .expect("a blank canned ACL must not fail the write");
+    let err = match svc.put_object("123456789012", &req, "blank-acl", "k").await {
+        Ok(_) => panic!("a blank canned ACL must not be accepted"),
+        Err(e) => e,
+    };
+    assert_eq!(err.code(), "InvalidArgument", "{err:?}");
 }
 
 #[test]

website/content/docs/services/s3.md

@@ -26,7 +26,7 @@ fakecloud implements **107 of 107** S3 operations at 100% Smithy conformance.
 - **Object Lambda** — `WriteGetObjectResponse` actually stores the transformed body + metadata against the original request token; the next `GetObject` on the access point returns the transformed payload.
 - **Public Access Block** — `IgnorePublicAcls` is enforced on `GetObject`; public-read ACL grants are ignored when the bucket-level block is set. `BlockPublicAcls` is enforced when the ACL is written, on the object-write paths as well as `PutObjectAcl` / `PutBucketAcl`: `put-object --acl public-read` against a bucket with `BlockPublicAcls` is `AccessDenied`, as on AWS, rather than quietly storing the `AllUsers` grant the block exists to prevent.
 - **Anonymous access** — unsigned `GET`/`HEAD` requests (`GET /bucket/key` with no SigV4) reach S3 directly, so you can serve static assets / CDN origins straight from a bucket. In the default (no-IAM) mode they're served permissively; under `--iam soft|strict` an anonymous request is authorized only when a bucket policy grants the action to `Principal:"*"` or a public-read ACL (`AllUsers`) is set, denied otherwise.
-- **ACL ownership modes** — `BucketOwnerEnforced` disables ACLs entirely (all ACL writes rejected, reads return owner-only).
+- **ACL ownership modes** — `BucketOwnerEnforced` disables ACL WRITES: `PutBucketAcl` and `PutObjectAcl` are rejected outright. Reads are not reinterpreted: `GetBucketAcl` / `GetObjectAcl` return whatever grants are stored, so an ACL set before the mode was turned on is still reported (AWS reports owner-only there). The object-write paths keep the one exception AWS documents, described under Bucket and object ACLs above: a request specifying no ACL, or bucket-owner-full-control (canned or as the equivalent explicit grant to the bucket owner), is accepted.
 
 ## Protocol