diff --git a/crates/fakecloud-e2e/tests/s3_persistence.rs b/crates/fakecloud-e2e/tests/s3_persistence.rs
index 4ffff317d..fb6330302 100644
--- a/crates/fakecloud-e2e/tests/s3_persistence.rs
+++ b/crates/fakecloud-e2e/tests/s3_persistence.rs
@@ -4,9 +4,10 @@ use std::time::Duration;
use aws_sdk_s3::primitives::ByteStream;
use aws_sdk_s3::types::{
- BucketVersioningStatus, CompletedMultipartUpload, CompletedPart, CorsConfiguration, CorsRule,
- CreateBucketConfiguration, ObjectLockConfiguration, ObjectLockEnabled, ObjectLockLegalHold,
- ObjectLockLegalHoldStatus, ServerSideEncryption, ServerSideEncryptionByDefault,
+ BucketCannedAcl, BucketVersioningStatus, CompletedMultipartUpload, CompletedPart,
+ CorsConfiguration, CorsRule, CreateBucketConfiguration, ObjectCannedAcl,
+ ObjectLockConfiguration, ObjectLockEnabled, ObjectLockLegalHold, ObjectLockLegalHoldStatus,
+ ObjectOwnership, ServerSideEncryption, ServerSideEncryptionByDefault,
ServerSideEncryptionConfiguration, ServerSideEncryptionRule, StorageClass, Tag, Tagging,
VersioningConfiguration,
};
@@ -1265,6 +1266,206 @@ async fn persistence_body_cache_small_and_large_objects() {
}
}
+#[tokio::test]
+async fn persistence_create_time_acl_object_lock_and_ownership_round_trip() {
+ // The canned ACL, object-lock enablement, and object-ownership rule a
+ // bucket is CREATED with live in subresources, not in BucketMeta, so
+ // without an explicit write at create time they were all gone after a
+ // restart -- object-lock retention silently stopped being enforced, and a
+ // public-read bucket came back owner-only.
+ let tmp = tempfile::tempdir().unwrap();
+ let mut server = TestServer::start_persistent(tmp.path()).await;
+ let client = server.s3_client().await;
+
+ client
+ .create_bucket()
+ .bucket("create-subres")
+ .acl(BucketCannedAcl::PublicRead)
+ .object_lock_enabled_for_bucket(true)
+ .object_ownership(ObjectOwnership::ObjectWriter)
+ .send()
+ .await
+ .unwrap();
+
+ server.restart().await;
+ let client = server.s3_client().await;
+
+ let acl = client
+ .get_bucket_acl()
+ .bucket("create-subres")
+ .send()
+ .await
+ .unwrap();
+ assert!(
+ acl.grants().iter().any(|g| {
+ g.grantee()
+ .and_then(|gr| gr.uri())
+ .is_some_and(|uri| uri.contains("AllUsers"))
+ }),
+ "create-time public-read grant lost across restart: {:?}",
+ acl.grants()
+ );
+
+ let lock = client
+ .get_object_lock_configuration()
+ .bucket("create-subres")
+ .send()
+ .await;
+ assert!(
+ lock.is_ok(),
+ "create-time object lock lost across restart: {:?}",
+ lock.err()
+ );
+
+ let ownership = client
+ .get_bucket_ownership_controls()
+ .bucket("create-subres")
+ .send()
+ .await
+ .expect("create-time ownership controls lost across restart");
+ assert_eq!(
+ ownership
+ .ownership_controls()
+ .and_then(|oc| oc.rules().first())
+ .map(|r| r.object_ownership()),
+ Some(&ObjectOwnership::ObjectWriter),
+ );
+}
+
+#[tokio::test]
+async fn persistence_plain_create_clears_orphan_create_time_subresources() {
+ // A create that failed partway leaves sidecars in a directory the loader
+ // skips (no meta.toml). A later plain create of that name must not adopt
+ // them.
+ let tmp = tempfile::tempdir().unwrap();
+ let mut server = TestServer::start_persistent(tmp.path()).await;
+ let _client = server.s3_client().await;
+
+ let orphan_dir = tmp.path().join("s3").join("buckets").join("orphan-subres");
+ std::fs::create_dir_all(&orphan_dir).unwrap();
+ std::fs::write(
+ orphan_dir.join("ownership.toml"),
+ "BucketOwnerEnforced",
+ )
+ .unwrap();
+ std::fs::write(
+ orphan_dir.join("object_lock.toml"),
+ "Enabled",
+ )
+ .unwrap();
+ // Subresources the create path never writes must be cleared too -- the
+ // whole directory is stale, not just the three create-time sidecars.
+ std::fs::write(
+ orphan_dir.join("policy.toml"),
+ r#"{"Version":"2012-10-17","Statement":[{"Effect":"Deny","Principal":"*","Action":"s3:GetObject","Resource":"arn:aws:s3:::orphan-subres/*"}]}"#,
+ )
+ .unwrap();
+ std::fs::write(orphan_dir.join("versioning.toml"), "Enabled").unwrap();
+
+ server.restart().await;
+ let client = server.s3_client().await;
+ client
+ .create_bucket()
+ .bucket("orphan-subres")
+ .send()
+ .await
+ .unwrap();
+
+ server.restart().await;
+ let client = server.s3_client().await;
+
+ let ownership = client
+ .get_bucket_ownership_controls()
+ .bucket("orphan-subres")
+ .send()
+ .await;
+ assert!(
+ ownership.is_err(),
+ "plain create inherited an orphan ownership rule"
+ );
+ let lock = client
+ .get_object_lock_configuration()
+ .bucket("orphan-subres")
+ .send()
+ .await;
+ assert!(
+ lock.is_err(),
+ "plain create inherited an orphan object-lock configuration"
+ );
+ let policy = client
+ .get_bucket_policy()
+ .bucket("orphan-subres")
+ .send()
+ .await;
+ assert!(policy.is_err(), "plain create inherited an orphan policy");
+ let versioning = client
+ .get_bucket_versioning()
+ .bucket("orphan-subres")
+ .send()
+ .await
+ .unwrap();
+ assert!(
+ versioning.status().is_none(),
+ "plain create inherited orphan versioning state: {:?}",
+ versioning.status()
+ );
+}
+
+#[tokio::test]
+async fn persistence_create_bucket_grant_headers_round_trip() {
+ // CreateBucket honors the x-amz-grant-* headers as well as the canned
+ // x-amz-acl, and the resulting ACL is an explicit one, so it gets an
+ // acl.toml and survives a restart.
+ let tmp = tempfile::tempdir().unwrap();
+ let mut server = TestServer::start_persistent(tmp.path()).await;
+ let client = server.s3_client().await;
+
+ client
+ .create_bucket()
+ .bucket("grant-hdr")
+ .grant_read("uri=http://acs.amazonaws.com/groups/global/AllUsers")
+ .send()
+ .await
+ .unwrap();
+
+ let before = client
+ .get_bucket_acl()
+ .bucket("grant-hdr")
+ .send()
+ .await
+ .unwrap();
+ assert!(
+ before.grants().iter().any(|g| {
+ g.permission().map(|p| p.as_str()) == Some("READ")
+ && g.grantee()
+ .and_then(|gr| gr.uri())
+ .is_some_and(|uri| uri.contains("AllUsers"))
+ }),
+ "grant header ignored at create: {:?}",
+ before.grants()
+ );
+
+ server.restart().await;
+ let client = server.s3_client().await;
+
+ let after = client
+ .get_bucket_acl()
+ .bucket("grant-hdr")
+ .send()
+ .await
+ .unwrap();
+ assert!(
+ after.grants().iter().any(|g| {
+ g.permission().map(|p| p.as_str()) == Some("READ")
+ && g.grantee()
+ .and_then(|gr| gr.uri())
+ .is_some_and(|uri| uri.contains("AllUsers"))
+ }),
+ "create-time grant headers lost across restart: {:?}",
+ after.grants()
+ );
+}
+
#[tokio::test]
async fn persistence_create_bucket_tags_survive_restart() {
// Tags supplied through CreateBucketConfiguration go to the same Tags
@@ -1347,3 +1548,223 @@ async fn persistence_untagged_create_clears_an_orphan_tag_file() {
let msg = format!("{err:?}");
assert!(msg.contains("NoSuchTagSet"), "unexpected error: {msg}");
}
+
+#[tokio::test]
+async fn persistence_create_after_reset_reuses_the_name() {
+ // `/_fakecloud/reset/s3` clears in-memory state and deliberately leaves the
+ // store alone, so every bucket ever created still has a readable meta.toml
+ // on disk afterwards. Re-creating those names has to keep working -- gating
+ // the "unloaded state" refusal on files existing would brick all of them.
+ let tmp = tempfile::tempdir().unwrap();
+ let server = TestServer::start_persistent(tmp.path()).await;
+ let client = server.s3_client().await;
+
+ client
+ .create_bucket()
+ .bucket("reset-reuse")
+ .send()
+ .await
+ .unwrap();
+ client
+ .put_object()
+ .bucket("reset-reuse")
+ .key("k.txt")
+ .body(ByteStream::from_static(b"v"))
+ .send()
+ .await
+ .unwrap();
+ // The bucket needs a stored configuration for the assertion below to mean
+ // anything: without a tag set there is nothing that could be adopted.
+ client
+ .put_bucket_tagging()
+ .bucket("reset-reuse")
+ .tagging(
+ Tagging::builder()
+ .tag_set(Tag::builder().key("era").value("before").build().unwrap())
+ .build()
+ .unwrap(),
+ )
+ .send()
+ .await
+ .unwrap();
+
+ let status = reqwest::Client::new()
+ .post(format!("{}/_fakecloud/reset/s3", server.endpoint()))
+ .send()
+ .await
+ .expect("reset should respond")
+ .status();
+ assert!(status.is_success(), "reset failed: {status}");
+
+ // The name is free again as far as the running server is concerned.
+ client
+ .create_bucket()
+ .bucket("reset-reuse")
+ .send()
+ .await
+ .expect("re-creating a bucket after a reset must succeed");
+
+ // The bucket's stored CONFIGURATION does not carry over: the create clears
+ // every sidecar for the name, so the tag set from before the reset is gone
+ // rather than being restored on the next load.
+ let mut server = server;
+ server.restart().await;
+ let client = server.s3_client().await;
+ let tags = client
+ .get_bucket_tagging()
+ .bucket("reset-reuse")
+ .send()
+ .await;
+ assert!(
+ tags.is_err(),
+ "the pre-reset tag set was adopted by the re-created bucket"
+ );
+}
+
+#[tokio::test]
+async fn persistence_recreating_a_load_skipped_bucket_does_not_destroy_its_objects() {
+ // The loader skips a bucket it cannot fully read (a corrupt object meta, a
+ // missing part body) and logs a warning -- the data is still on disk and
+ // recoverable by fixing the one bad file. Such a bucket is absent from
+ // ListBuckets, so its name looks free: re-creating it must not be what
+ // destroys the objects, which is why the create clears only the sidecars.
+ let tmp = tempfile::tempdir().unwrap();
+ let mut server = TestServer::start_persistent(tmp.path()).await;
+ let client = server.s3_client().await;
+
+ client
+ .create_bucket()
+ .bucket("skipped")
+ .send()
+ .await
+ .unwrap();
+ for key in ["keep.txt", "corrupt.txt"] {
+ client
+ .put_object()
+ .bucket("skipped")
+ .key(key)
+ .body(ByteStream::from_static(b"precious"))
+ .send()
+ .await
+ .unwrap();
+ }
+
+ let objects_dir = tmp
+ .path()
+ .join("s3")
+ .join("buckets")
+ .join("skipped")
+ .join("objects");
+ let corrupt_meta = objects_dir.join("corrupt.txt").join("null.toml");
+ assert!(corrupt_meta.exists(), "expected {corrupt_meta:?} to exist");
+ std::fs::write(&corrupt_meta, "not valid toml = = =").unwrap();
+
+ server.restart().await;
+ let client = server.s3_client().await;
+
+ let list = client.list_buckets().send().await.unwrap();
+ assert!(
+ !list.buckets().iter().any(|b| b.name() == Some("skipped")),
+ "expected the unreadable bucket to be skipped on load"
+ );
+
+ client
+ .create_bucket()
+ .bucket("skipped")
+ .send()
+ .await
+ .unwrap();
+ assert!(
+ objects_dir.join("keep.txt").join("null.bin").exists(),
+ "re-creating a load-skipped bucket destroyed its objects"
+ );
+
+ // Repairing the one bad file brings the whole bucket back.
+ std::fs::remove_dir_all(objects_dir.join("corrupt.txt")).unwrap();
+ server.restart().await;
+ let client = server.s3_client().await;
+ let body = client
+ .get_object()
+ .bucket("skipped")
+ .key("keep.txt")
+ .send()
+ .await
+ .expect("the repaired bucket should load")
+ .body
+ .collect()
+ .await
+ .unwrap()
+ .into_bytes();
+ assert_eq!(&body[..], b"precious");
+}
+
+#[tokio::test]
+async fn persistence_put_object_acl_survives_restart() {
+ // PutObjectAcl writes the object's meta sidecar, and that snapshot copies
+ // acl_grants -- so snapshotting before applying the new grants persists the
+ // OLD ACL and leaves the new one memory-only, which only a restart reveals.
+ let tmp = tempfile::tempdir().unwrap();
+ let mut server = TestServer::start_persistent(tmp.path()).await;
+ let client = server.s3_client().await;
+
+ client
+ .create_bucket()
+ .bucket("obj-acl-persist")
+ .send()
+ .await
+ .unwrap();
+ client
+ .put_object()
+ .bucket("obj-acl-persist")
+ .key("k.txt")
+ .body(ByteStream::from_static(b"v"))
+ .send()
+ .await
+ .unwrap();
+
+ client
+ .put_object_acl()
+ .bucket("obj-acl-persist")
+ .key("k.txt")
+ .acl(ObjectCannedAcl::PublicRead)
+ .send()
+ .await
+ .unwrap();
+
+ let before = client
+ .get_object_acl()
+ .bucket("obj-acl-persist")
+ .key("k.txt")
+ .send()
+ .await
+ .unwrap();
+ assert!(
+ before.grants().iter().any(|g| {
+ g.grantee()
+ .and_then(|gr| gr.uri())
+ .is_some_and(|u| u.contains("AllUsers"))
+ }),
+ "public-read not applied: {:?}",
+ before.grants()
+ );
+
+ server.restart().await;
+ let client = server.s3_client().await;
+
+ let after = client
+ .get_object_acl()
+ .bucket("obj-acl-persist")
+ .key("k.txt")
+ .send()
+ .await
+ .unwrap();
+ assert!(
+ after.grants().iter().any(|g| {
+ g.grantee()
+ .and_then(|gr| gr.uri())
+ .is_some_and(|u| u.contains("AllUsers"))
+ }),
+ "object ACL reverted across restart: {:?}",
+ after.grants()
+ );
+}
diff --git a/crates/fakecloud-persistence/src/lib.rs b/crates/fakecloud-persistence/src/lib.rs
index 692de1870..f7d26a43a 100644
--- a/crates/fakecloud-persistence/src/lib.rs
+++ b/crates/fakecloud-persistence/src/lib.rs
@@ -11,6 +11,6 @@ pub use s3::{
AclGrantSnapshot, AclSnapshot, AnnotationSnapshot, BodyRef, BodySource, BucketMeta,
BucketSnapshot, BucketSubresource, InventorySnapshot, LoadedMpu, LoadedObject, LoadedPart,
MemoryS3Store, MpuInit, ObjectMeta, S3State as S3StateSnapshot, S3Store, StoreError,
- StoreResult, TagsSnapshot, UploadPartMeta,
+ StoreResult, TagsSnapshot, UploadPartMeta, ALL_SUBRESOURCES,
};
pub use snapshot::{DiskSnapshotStore, MemorySnapshotStore, SnapshotHook, SnapshotStore};
diff --git a/crates/fakecloud-persistence/src/s3.rs b/crates/fakecloud-persistence/src/s3.rs
index 87b6392b2..01a0d3869 100644
--- a/crates/fakecloud-persistence/src/s3.rs
+++ b/crates/fakecloud-persistence/src/s3.rs
@@ -982,11 +982,29 @@ impl S3Store for DiskS3Store {
fn delete_object(&self, bucket: &str, key: &str, version: Option<&str>) -> StoreResult<()> {
let (dir, bin_path, toml_path) = self.object_paths(bucket, key, version);
- for p in [&bin_path, &toml_path] {
- match std::fs::remove_file(p) {
- Ok(_) => {}
- Err(e) if e.kind() == std::io::ErrorKind::NotFound => {}
- Err(e) => return Err(e.into()),
+ // Sidecar first, body second. `load` iterates `*.toml` and treats a
+ // sidecar whose `.bin` is gone as a hard error that costs the whole
+ // bucket, while an orphan `.bin` is never looked at. So if this is
+ // interrupted -- a signal, or the second `remove_file` failing -- the
+ // residue left behind has to be the body, not the sidecar.
+ // The sidecar is the durable record, so its removal is the one that has
+ // to succeed. Once it is gone the object is deleted as far as every
+ // reader is concerned, and failing the call over a body that could not
+ // be unlinked would report a delete that did happen as an error --
+ // while still leaving the caller no way to reclaim the bytes.
+ match std::fs::remove_file(&toml_path) {
+ Ok(_) => {}
+ Err(e) if e.kind() == std::io::ErrorKind::NotFound => {}
+ Err(e) => return Err(e.into()),
+ }
+ if let Err(e) = std::fs::remove_file(&bin_path) {
+ if e.kind() != std::io::ErrorKind::NotFound {
+ tracing::warn!(
+ path = %bin_path.display(),
+ error = %e,
+ "deleted object's body could not be removed; it is orphaned until the \
+ bucket is deleted",
+ );
}
}
Self::cleanup_empty(&dir);
@@ -1504,6 +1522,56 @@ mod disk_tests {
assert_eq!(obj.meta.tags.get("x").map(String::as_str), Some("y"));
}
+ #[test]
+ fn delete_object_removes_the_sidecar_before_the_body() {
+ // `load` iterates `*.toml` and treats a sidecar whose `.bin` is gone as
+ // a hard error for the whole bucket, while an orphan `.bin` is never
+ // read. So the sidecar has to be the file the delete attempts first.
+ //
+ // Forces the SIDECAR removal to fail, by making that path a non-empty
+ // directory `remove_file` cannot take. Only a sidecar-first delete
+ // reports the failure with the body still on disk; a body-first one
+ // would have removed the body before reaching it.
+ let tmp = TempDir::new().unwrap();
+ let store = new_store(&tmp);
+ store
+ .put_bucket_meta(
+ "b",
+ &BucketMeta {
+ name: "b".to_string(),
+ ..Default::default()
+ },
+ )
+ .unwrap();
+ store
+ .put_object(
+ "b",
+ "k.txt",
+ None,
+ BodySource::Bytes(Bytes::from_static(b"v")),
+ &ObjectMeta {
+ key: "k.txt".to_string(),
+ size: 1,
+ ..Default::default()
+ },
+ )
+ .unwrap();
+
+ let (_dir, bin, toml) = store.object_paths("b", "k.txt", None);
+ std::fs::remove_file(&toml).unwrap();
+ std::fs::create_dir(&toml).unwrap();
+ std::fs::write(toml.join("blocker"), b"x").unwrap();
+
+ assert!(
+ store.delete_object("b", "k.txt", None).is_err(),
+ "a sidecar that cannot be removed must fail the delete: it is the durable record"
+ );
+ assert!(
+ bin.exists(),
+ "the body must still be there, which is only true if the sidecar was attempted first"
+ );
+ }
+
#[test]
fn delete_object_cleans_up_files_and_cache() {
let tmp = TempDir::new().unwrap();
diff --git a/crates/fakecloud-s3/src/persistence.rs b/crates/fakecloud-s3/src/persistence.rs
index f4c023a52..40a4306aa 100644
--- a/crates/fakecloud-s3/src/persistence.rs
+++ b/crates/fakecloud-s3/src/persistence.rs
@@ -112,6 +112,24 @@ pub fn upload_part_meta_snapshot(p: &UploadPart) -> UploadPartMeta {
}
}
+/// Whether a stored grant names someone it could ever match.
+///
+/// An older build read an email grantee's absent `` as an empty string, so
+/// a legacy `acl.toml` can hold a grant that matches nobody. Dropping those at
+/// load keeps memory, disk and `GetBucketAcl` telling the same story -- hiding
+/// them only when rendering would make the response disagree with the ACL the
+/// bucket actually has, and a read-modify-write would then persist the omission.
+fn acl_grant_names_a_grantee(g: &AclGrantSnapshot) -> bool {
+ // Trimmed, like the request-side checks: a stored grantee of whitespace
+ // names nobody just as surely as an empty one.
+ let named = |v: &Option| v.as_deref().is_some_and(|s| !s.trim().is_empty());
+ match g.grantee_type.as_str() {
+ "Group" => named(&g.grantee_uri),
+ "AmazonCustomerByEmail" => named(&g.grantee_display_name),
+ _ => named(&g.grantee_id),
+ }
+}
+
fn acl_grant_from_snapshot(g: &AclGrantSnapshot) -> AclGrant {
AclGrant {
grantee_type: g.grantee_type.clone(),
@@ -247,7 +265,13 @@ pub fn s3_bucket_from_snapshot(
grantee_uri: None,
permission: "FULL_CONTROL".to_string(),
};
- let has_acl_sidecar = subresources.contains_key("acl.toml");
+ // An `acl.toml` written by an older build could be empty (its writer used
+ // `unwrap_or_default()` on serialization failure). Treat that as no sidecar
+ // at all, so the bucket keeps the default owner grant instead of loading
+ // with no grants and silently stripping the owner's FULL_CONTROL.
+ let has_acl_sidecar = subresources
+ .get("acl.toml")
+ .is_some_and(|text| !text.trim().is_empty());
let mut b = S3Bucket {
name: name.to_string(),
creation_date: meta.creation_date,
@@ -330,7 +354,30 @@ pub fn s3_bucket_from_snapshot(
if !snap.owner_id.is_empty() {
b.acl_owner_id = snap.owner_id;
}
- b.acl_grants = snap.grants.iter().map(acl_grant_from_snapshot).collect();
+ // Drop grants that name nobody, so the loaded ACL is one the
+ // server would also accept back from a client.
+ let usable: Vec = snap
+ .grants
+ .iter()
+ .filter(|g| acl_grant_names_a_grantee(g))
+ .map(acl_grant_from_snapshot)
+ .collect();
+ // ...but never leave the bucket with no grants at all. A
+ // sidecar whose every grant is unusable is indistinguishable
+ // from one that was never written, so fall back to the same
+ // default owner grant a bucket with no `acl.toml` gets, rather
+ // than loading a bucket whose owner has lost FULL_CONTROL.
+ b.acl_grants = if usable.is_empty() {
+ vec![AclGrant {
+ grantee_type: "CanonicalUser".to_string(),
+ grantee_id: Some(b.acl_owner_id.clone()),
+ grantee_display_name: Some(b.acl_owner_id.clone()),
+ grantee_uri: None,
+ permission: "FULL_CONTROL".to_string(),
+ }]
+ } else {
+ usable
+ };
}
"inventory.toml" => {
if text.trim().is_empty() {
@@ -345,19 +392,58 @@ pub fn s3_bucket_from_snapshot(
if text.trim().is_empty() {
continue;
}
- b.analytics_configs = toml::from_str(&text).unwrap_or_default();
+ // Neither `unwrap_or_default` nor `?`. Defaulting would load the
+ // bucket with no configurations and no word, and the next Put
+ // would overwrite the file and make the loss permanent; failing
+ // would hide every object in the bucket behind an unparseable
+ // reporting config. Skip the sidecar, keep the bucket, and say so,
+ // so the file is still there to look at.
+ match toml::from_str(&text) {
+ Ok(parsed) => b.analytics_configs = parsed,
+ Err(e) => tracing::warn!(
+ bucket = %name,
+ error = %e,
+ "ignoring unreadable analytics.toml; its configurations are not loaded",
+ ),
+ }
}
"intelligent_tiering.toml" => {
if text.trim().is_empty() {
continue;
}
- b.intelligent_tiering_configs = toml::from_str(&text).unwrap_or_default();
+ // Neither `unwrap_or_default` nor `?`. Defaulting would load the
+ // bucket with no configurations and no word, and the next Put
+ // would overwrite the file and make the loss permanent; failing
+ // would hide every object in the bucket behind an unparseable
+ // reporting config. Skip the sidecar, keep the bucket, and say so,
+ // so the file is still there to look at.
+ match toml::from_str(&text) {
+ Ok(parsed) => b.intelligent_tiering_configs = parsed,
+ Err(e) => tracing::warn!(
+ bucket = %name,
+ error = %e,
+ "ignoring unreadable intelligent_tiering.toml; its configurations are not loaded",
+ ),
+ }
}
"metrics.toml" => {
if text.trim().is_empty() {
continue;
}
- b.metrics_configs = toml::from_str(&text).unwrap_or_default();
+ // Neither `unwrap_or_default` nor `?`. Defaulting would load the
+ // bucket with no configurations and no word, and the next Put
+ // would overwrite the file and make the loss permanent; failing
+ // would hide every object in the bucket behind an unparseable
+ // reporting config. Skip the sidecar, keep the bucket, and say so,
+ // so the file is still there to look at.
+ match toml::from_str(&text) {
+ Ok(parsed) => b.metrics_configs = parsed,
+ Err(e) => tracing::warn!(
+ bucket = %name,
+ error = %e,
+ "ignoring unreadable metrics.toml; its configurations are not loaded",
+ ),
+ }
}
"request_payment.toml" => {
b.request_payment = Some(text);
@@ -381,11 +467,41 @@ pub fn hydrate_s3_state(
snapshot: S3StateSnapshot,
account_id: &str,
region: &str,
+) -> Result {
+ hydrate_s3_state_reporting(snapshot, account_id, region, &mut |_, _| {})
+}
+
+/// Hydrate a loaded snapshot, reporting each bucket that cannot be used instead
+/// of failing the whole load.
+///
+/// The store already isolates a bucket whose objects it cannot read: it warns,
+/// records the refusal and carries on, so one bad file costs one bucket rather
+/// than the server. Sidecars are parsed here, a layer later, and a single
+/// malformed `acl.toml` or `tags.toml` used to abort startup -- every other
+/// bucket inaccessible because of one file. Such a bucket is skipped and handed
+/// to `refused` so it is reported exactly like a store-level refusal: absent
+/// from memory, its name refused by CreateBucket, and clearable by DeleteBucket.
+pub fn hydrate_s3_state_reporting(
+ snapshot: S3StateSnapshot,
+ account_id: &str,
+ region: &str,
+ refused: &mut dyn FnMut(&str, &str),
) -> Result {
let mut state = S3State::new(account_id, region);
for (name, snap) in snapshot.buckets {
- let bucket = s3_bucket_from_snapshot(&name, snap, region)?;
- state.buckets.insert(name, bucket);
+ match s3_bucket_from_snapshot(&name, snap, region) {
+ Ok(bucket) => {
+ state.buckets.insert(name, bucket);
+ }
+ Err(e) => {
+ tracing::warn!(
+ bucket = %name,
+ error = %e,
+ "skipping S3 bucket whose stored configuration could not be read",
+ );
+ refused(&name, &e);
+ }
+ }
}
Ok(state)
}
diff --git a/crates/fakecloud-s3/src/service/acl.rs b/crates/fakecloud-s3/src/service/acl.rs
index 6e6667979..6d1c5b4c5 100644
--- a/crates/fakecloud-s3/src/service/acl.rs
+++ b/crates/fakecloud-s3/src/service/acl.rs
@@ -6,8 +6,7 @@ use fakecloud_core::service::{AwsRequest, AwsResponse, AwsServiceError};
use crate::persistence::object_meta_snapshot;
use super::{
- build_acl_xml, canned_acl_grants_for_object, no_such_key, parse_acl_xml, parse_grant_headers,
- s3_xml, S3Service,
+ build_acl_xml, canned_acl_grants_for_object, no_such_key, parse_acl_xml, s3_xml, S3Service,
};
impl S3Service {
@@ -43,6 +42,12 @@ impl S3Service {
.get("x-amz-acl")
.and_then(|v| v.to_str().ok())
.map(|s| s.to_string());
+ // Validated before the key is resolved, as S3 does: a bad canned value
+ // is an InvalidArgument whether or not the key exists.
+ if let Some(acl) = canned.as_deref() {
+ super::validate_object_canned_acl(acl)?;
+ }
+ super::reject_conflicting_acl_sources(canned.as_deref(), &req.headers, &req.body)?;
if self.bucket_owner_enforced(account_id, bucket) {
return Err(AwsServiceError::aws_error(
@@ -70,19 +75,35 @@ impl S3Service {
let proposed_grants = if let Some(acl) = &canned {
canned_acl_grants_for_object(acl, &owner_id)
} else {
- let has_grant_headers = req.headers.keys().any(|k| {
- let name = k.as_str();
- name.starts_with("x-amz-grant-")
- });
- if has_grant_headers {
- parse_grant_headers(&req.headers)
+ if super::has_grant_headers(&req.headers) {
+ super::resolved_grant_headers(&req.headers)?
} else {
+ // No canned header, no grant header, no body names no ACL at
+ // all. Re-persisting the object's current grants would answer
+ // 200 for a request that asked for nothing, which the caller
+ // cannot tell from an applied change -- the same rule
+ // PutBucketAcl applies.
let body_str = std::str::from_utf8(&req.body).unwrap_or("");
- if !body_str.is_empty() {
- parse_acl_xml(body_str)?
- } else {
- obj.acl_grants.clone()
+ if body_str.trim().is_empty() {
+ return Err(AwsServiceError::aws_error(
+ StatusCode::BAD_REQUEST,
+ "MalformedACLError",
+ "The XML you provided was not well-formed or did not validate against our published schema",
+ ));
+ }
+ // A body that is not an AccessControlPolicy at all (JSON, a
+ // misspelled root) parses to zero grants, and taking that as
+ // "remove every grant" strips the object's owner FULL_CONTROL
+ // and writes the empty list into its meta. Same rule the bucket
+ // path applies.
+ if !body_str.contains(",
+ ) -> Result<(), AwsServiceError> {
+ let Some(text) = payload else {
+ return Ok(());
+ };
+ self.store
+ .put_bucket_subresource(bucket, kind, text)
+ .map_err(super::persistence_error)
+ }
+
pub(super) fn list_buckets(
&self,
account_id: &str,
@@ -222,12 +240,93 @@ impl S3Service {
let create_tags = create_bucket_configuration_tags(body_str);
validate_tags(&create_tags)?;
- // Parse ACL from header
- let acl = req
+ // Parse the ACL the create asks for. Either a canned `x-amz-acl` or the
+ // `x-amz-grant-*` headers, never both -- S3 rejects the combination
+ // rather than picking a winner. Whichever is used, the result is an ACL
+ // the caller chose, so it needs an `acl.toml`; the default private
+ // grant is what the loader reconstructs on its own and needs no
+ // sidecar.
+ let acl_header = req.headers.get("x-amz-acl").and_then(|v| v.to_str().ok());
+ let grant_headers_present = has_grant_headers(&req.headers);
+ if acl_header.is_some() && grant_headers_present {
+ return Err(AwsServiceError::aws_error(
+ StatusCode::BAD_REQUEST,
+ "InvalidRequest",
+ "Specifying both Canned ACLs and Header Grants is not allowed",
+ ));
+ }
+ if let Some(acl) = acl_header {
+ if !BUCKET_CANNED_ACLS.contains(&acl) {
+ return Err(AwsServiceError::aws_error(
+ StatusCode::BAD_REQUEST,
+ "InvalidArgument",
+ format!("Invalid x-amz-acl value: {acl}"),
+ ));
+ }
+ }
+ let header_grants = if grant_headers_present {
+ resolved_grant_headers(&req.headers)?
+ } else {
+ Vec::new()
+ };
+
+ // BucketOwnerEnforced turns ACLs off for the bucket, so S3 refuses a
+ // create that also asks for one. Without this the bucket would come out
+ // publicly readable via a canned `public-read` while `PutBucketAcl` can
+ // no longer edit that ACL -- and this create now persists it, so the
+ // state would survive restarts instead of evaporating.
+ //
+ // A canned ACL is judged by the grants it resolves to (`private` grants
+ // nothing beyond the owner, so S3 accepts it), while ANY `x-amz-grant-*`
+ // header conflicts on presence alone, as on S3 -- the caller is asking
+ // for an ACL on a bucket that has none.
+ let ownership_header = req
.headers
- .get("x-amz-acl")
- .and_then(|v| v.to_str().ok())
- .unwrap_or("private");
+ .get("x-amz-object-ownership")
+ .and_then(|v| v.to_str().ok());
+ if let Some(ownership) = ownership_header {
+ if !OBJECT_OWNERSHIP_VALUES.contains(&ownership) {
+ return Err(AwsServiceError::aws_error(
+ StatusCode::BAD_REQUEST,
+ "InvalidArgument",
+ format!("Invalid x-amz-object-ownership value: {ownership}"),
+ ));
+ }
+ }
+ // Compared exactly, not case-insensitively: `bucket_owner_enforced()`
+ // matches the stored XML case-sensitively, so accepting a differently
+ // cased value here would refuse an ACL at create that the very next
+ // PutBucketAcl would then happily set.
+ let ownership_enforced = ownership_header == Some("BucketOwnerEnforced");
+ let owner_only = |grants: &[crate::state::AclGrant]| {
+ grants.iter().all(|g| {
+ g.permission == "FULL_CONTROL"
+ && g.grantee_type == "CanonicalUser"
+ && g.grantee_id.as_deref() == Some(req.account_id.as_str())
+ })
+ };
+ let acl_requests_grants = grant_headers_present
+ || acl_header.is_some_and(|a| {
+ // `aws-exec-read` grants READ to the EC2 service's canonical
+ // user. That grantee is not modeled, so the resolved grants
+ // look owner-only -- but the request still asks for an ACL
+ // reaching outside the owner, which is what conflicts.
+ a == "aws-exec-read" || !owner_only(&canned_acl_grants(a, &req.account_id))
+ });
+ if ownership_enforced && acl_requests_grants {
+ return Err(AwsServiceError::aws_error(
+ StatusCode::BAD_REQUEST,
+ "InvalidBucketAclWithObjectOwnership",
+ "Bucket cannot have ACLs set with ObjectOwnership's BucketOwnerEnforced setting",
+ ));
+ }
+ // `aws-exec-read` resolves to owner-only here (its READ grant to the EC2
+ // service's canonical user is not modeled), which is exactly what the
+ // loader reconstructs without a sidecar -- so it gets no `acl.toml`
+ // rather than a stored record claiming an explicit ACL it does not have.
+ let acl_header_present =
+ grant_headers_present || acl_header.is_some_and(|a| a != "aws-exec-read");
+ let acl = acl_header.unwrap_or("private");
let mut accts = self.state.write();
// Check global uniqueness across all accounts before creating
@@ -248,7 +347,7 @@ impl S3Service {
if let Some(existing) = state.buckets.get(bucket) {
// In us-east-1, re-creating same bucket in same region is idempotent
// (returns 200). The re-create is a no-op on the existing bucket: it
- // re-applies none of the create-time settings — not the canned ACL,
+ // re-applies none of the create-time settings -- not the canned ACL,
// not object lock, not object ownership, and not the
// `CreateBucketConfiguration` tag set. Applying only the tags here
// would make them the one create-time setting that mutates a bucket
@@ -279,7 +378,11 @@ impl S3Service {
.unwrap_or(false);
let mut b = S3Bucket::new(bucket, &requested_region, &req.account_id);
- b.acl_grants = canned_acl_grants(acl, &req.account_id);
+ b.acl_grants = if grant_headers_present {
+ header_grants
+ } else {
+ canned_acl_grants(acl, &req.account_id)
+ };
if object_lock_enabled {
b.versioning = Some("Enabled".to_string());
b.object_lock_config = Some(
@@ -292,11 +395,7 @@ impl S3Service {
}
// Handle x-amz-object-ownership header
- if let Some(ownership) = req
- .headers
- .get("x-amz-object-ownership")
- .and_then(|v| v.to_str().ok())
- {
+ if let Some(ownership) = ownership_header {
b.ownership_controls = Some(format!(
"\
\
@@ -315,36 +414,92 @@ impl S3Service {
};
let meta = bucket_meta_snapshot(&b);
- // Persist before committing the bucket to memory, tags first: bucket
- // tags live in their own subresource rather than in the bucket meta, so
- // they need an explicit write to survive a restart, and writing them
- // ahead of `meta.toml` keeps a failed create from leaving anything
- // usable behind. The loader skips a bucket directory with no
- // `meta.toml`, and the in-memory insert is last, so a store error at
- // either step surfaces as an error with no half-created bucket — the
- // same guarantee the `InvalidTag` path gives.
- match tags_snapshot {
- Some(snap) => {
- let payload = toml::to_string(&snap).unwrap_or_default();
- self.store
- .put_bucket_subresource(bucket, BucketSubresource::Tags, &payload)
- .map_err(super::persistence_error)?;
- }
- // An untagged create clears the file rather than leaving it alone:
- // a create that failed after its tag write (or died between the two
- // writes) leaves a `tags.toml` in a directory with no `meta.toml`,
- // which the loader skips — but a later untagged create of the same
- // name would adopt that never-committed tag set on the next restart.
- // Deleting is a no-op when the file is absent, which is the normal
- // case.
- None => self
- .store
- .delete_bucket_subresource(bucket, BucketSubresource::Tags)
- .map_err(super::persistence_error)?,
- }
+ // Persist the create-time subresources before `meta.toml` and before the
+ // in-memory insert. None of these three live in `BucketMeta`, and the
+ // loader restores `object_lock_config` / `ownership_controls` as `None`
+ // and falls back to the default owner grant when `acl.toml` is absent --
+ // so without an explicit write, a bucket created with `x-amz-acl`,
+ // object lock, or an ownership rule came back after a restart with none
+ // of them (object-lock retention silently stopped being enforced).
+ //
+ // Each one is written only when the create set it; the clear above is
+ // what guarantees nothing stale is left for the ones it did not.
+ let acl_payload = if acl_header_present {
+ let snap = AclSnapshot {
+ owner_id: b.acl_owner_id.clone(),
+ grants: b.acl_grants.iter().map(AclGrantSnapshot::from).collect(),
+ };
+ // Never fall back to an empty document here: the loader takes the
+ // mere presence of `acl.toml` to mean "this bucket has an explicit
+ // ACL" and skips the default owner grant, so an empty file would
+ // restore the bucket with no grants at all.
+ Some(toml::to_string(&snap).map_err(|e| {
+ AwsServiceError::aws_error(
+ StatusCode::INTERNAL_SERVER_ERROR,
+ "InternalError",
+ format!("failed to serialize bucket ACL: {e}"),
+ )
+ })?)
+ } else {
+ None
+ };
+ let tags_payload = match tags_snapshot {
+ // Never fall back to an empty document: a blank `tags.toml` is not
+ // what "no tags" means on disk, and the sweep below is what clears
+ // a stale one.
+ Some(snap) => Some(toml::to_string(&snap).map_err(|e| {
+ AwsServiceError::aws_error(
+ StatusCode::INTERNAL_SERVER_ERROR,
+ "InternalError",
+ format!("failed to serialize bucket tags: {e}"),
+ )
+ })?),
+ None => None,
+ };
+ // The meta goes first, and nothing is destroyed until it lands: a
+ // create that fails here has changed nothing on disk, where clearing
+ // the old sidecars first would have thrown away the configuration of
+ // whatever bucket this name belonged to for a create that never
+ // happened.
self.store
.put_bucket_meta(bucket, &meta)
.map_err(super::persistence_error)?;
+ // Clear every stored subresource for this name before writing this
+ // bucket's own, now that the create is committed. A create or delete that stopped partway -- or a
+ // `/_fakecloud/reset`, which clears memory and leaves the store alone --
+ // can leave sidecars behind, and a later create would otherwise be
+ // restored carrying the old bucket's `policy.toml`, `acl.toml` and the
+ // rest. Each delete tolerates a missing file, which is the normal case.
+ //
+ // Scoped to the sidecars on purpose. `objects/` is NOT touched: the
+ // loader skips a bucket whose objects it cannot read, so that bucket is
+ // absent from memory while its data sits intact on disk, and clearing
+ // the directory here would make re-creating the name the thing that
+ // destroys it. Whether a create should adopt or discard a stale object
+ // tree is a separate question from this one, and this is not the change
+ // that answers it.
+ for kind in fakecloud_persistence::ALL_SUBRESOURCES {
+ self.store
+ .delete_bucket_subresource(bucket, *kind)
+ .map_err(super::persistence_error)?;
+ }
+
+ self.put_bucket_subresource_if_set(
+ bucket,
+ BucketSubresource::Tags,
+ tags_payload.as_deref(),
+ )?;
+ self.put_bucket_subresource_if_set(bucket, BucketSubresource::Acl, acl_payload.as_deref())?;
+ self.put_bucket_subresource_if_set(
+ bucket,
+ BucketSubresource::ObjectLock,
+ b.object_lock_config.as_deref(),
+ )?;
+ self.put_bucket_subresource_if_set(
+ bucket,
+ BucketSubresource::Ownership,
+ b.ownership_controls.as_deref(),
+ )?;
state.buckets.insert(bucket.to_string(), b);
let mut headers = HeaderMap::new();
diff --git a/crates/fakecloud-s3/src/service/config/bucket_inventory.rs b/crates/fakecloud-s3/src/service/config/bucket_inventory.rs
index a4af4de19..89b5cfdea 100644
--- a/crates/fakecloud-s3/src/service/config/bucket_inventory.rs
+++ b/crates/fakecloud-s3/src/service/config/bucket_inventory.rs
@@ -25,7 +25,7 @@ impl S3Service {
let snap = InventorySnapshot {
configs: b.inventory_configs.clone(),
};
- toml::to_string(&snap).unwrap_or_default()
+ crate::service::toml_or_internal_error(&snap)?
};
self.store
.put_bucket_subresource(bucket, BucketSubresource::Inventory, &payload)
@@ -114,7 +114,7 @@ impl S3Service {
let snap = InventorySnapshot {
configs: b.inventory_configs.clone(),
};
- let payload = toml::to_string(&snap).unwrap_or_default();
+ let payload = crate::service::toml_or_internal_error(&snap)?;
self.store
.put_bucket_subresource(bucket, BucketSubresource::Inventory, &payload)
.map_err(crate::service::persistence_error)?;
diff --git a/crates/fakecloud-s3/src/service/config/mod.rs b/crates/fakecloud-s3/src/service/config/mod.rs
index b612af564..368b96e35 100644
--- a/crates/fakecloud-s3/src/service/config/mod.rs
+++ b/crates/fakecloud-s3/src/service/config/mod.rs
@@ -10,9 +10,10 @@ use crate::inventory;
use crate::persistence::{bucket_meta_snapshot, object_meta_snapshot};
use super::{
- build_acl_xml, canned_acl_grants, empty_response, extract_xml_value, no_such_bucket,
- normalize_notification_ids, normalize_replication_xml, parse_acl_xml, parse_tagging_xml,
- s3_xml, validate_lifecycle_xml, validate_tags, xml_escape, S3Service,
+ build_acl_xml, canned_acl_grants, empty_response, extract_xml_value, has_grant_headers,
+ no_such_bucket, normalize_notification_ids, normalize_replication_xml, parse_acl_xml,
+ parse_tagging_xml, reject_conflicting_acl_sources, resolved_grant_headers, s3_xml,
+ validate_lifecycle_xml, validate_tags, xml_escape, S3Service, BUCKET_CANNED_ACLS,
};
/// Decoded `PublicAccessBlockConfiguration` flags read by the request
@@ -240,12 +241,28 @@ impl S3Service {
req: &AwsRequest,
bucket: &str,
) -> Result {
- // Check for canned ACL header
+ // An unrecognized canned value used to fall through `canned_acl_grants`'
+ // catch-all to owner-only, silently stripping every public grant -- and
+ // that wipe is now written to `acl.toml`, so it outlives the process.
+ //
+ // Checked before the bucket is resolved, matching PutObjectAcl and S3
+ // itself: a bad header is an InvalidArgument whether or not the target
+ // exists.
let canned = req
.headers
.get("x-amz-acl")
.and_then(|v| v.to_str().ok())
.map(|s| s.to_string());
+ if let Some(acl) = canned.as_deref() {
+ if !BUCKET_CANNED_ACLS.contains(&acl) {
+ return Err(AwsServiceError::aws_error(
+ StatusCode::BAD_REQUEST,
+ "InvalidArgument",
+ format!("Invalid x-amz-acl value: {acl}"),
+ ));
+ }
+ }
+ reject_conflicting_acl_sources(canned.as_deref(), &req.headers, &req.body)?;
// BucketOwnerEnforced disables ACLs on this bucket entirely;
// any ACL-mutating call rejects with
@@ -267,12 +284,41 @@ impl S3Service {
.get_mut(bucket)
.ok_or_else(|| no_such_bucket(bucket))?;
+ // Canned header, else the `x-amz-grant-*` headers, else the XML body --
+ // the precedence PutObjectAcl uses. Without the grant-header arm a
+ // `put-bucket-acl --grant-read uri=...` request fell through to an
+ // empty body and wiped the bucket's ACL to nothing.
let proposed_grants = if let Some(acl) = &canned {
canned_acl_grants(acl, &b.acl_owner_id.clone())
+ } else if has_grant_headers(&req.headers) {
+ resolved_grant_headers(&req.headers)?
} else {
let body_str = std::str::from_utf8(&req.body).unwrap_or("");
parse_acl_xml(body_str)?
};
+ // A request that names no ACL at all is malformed rather than "remove
+ // every grant". An explicit `` IS the documented
+ // way to drop all grants, so zero grants on its own is not an error --
+ // but a body that is not an AccessControlPolicy at all (whitespace,
+ // JSON, a misspelled root) parses to zero grants too, and taking that
+ // as "drop everything" wipes the bucket's ACL durably.
+ if canned.is_none() && !has_grant_headers(&req.headers) {
+ let body_str = std::str::from_utf8(&req.body).unwrap_or("");
+ if body_str.trim().is_empty() {
+ return Err(AwsServiceError::aws_error(
+ StatusCode::BAD_REQUEST,
+ "MalformedACLError",
+ "The XML you provided was not well-formed or did not validate against our published schema",
+ ));
+ }
+ if !body_str.contains("") {
+ let after = &rest[start + "".len()..];
+ let Some(end) = after.find("") else {
+ break;
+ };
+ let value = after[..end].trim();
+ if !crate::service::OBJECT_OWNERSHIP_VALUES.contains(&value) {
+ return Err(AwsServiceError::aws_error(
+ StatusCode::BAD_REQUEST,
+ "InvalidArgument",
+ format!("Invalid ObjectOwnership value: {value}"),
+ ));
+ }
+ seen += 1;
+ rest = &after[end..];
+ }
+ let single = seen == 1;
+ let value = extract_xml_value(&body_str, "ObjectOwnership").unwrap_or_default();
+ // `bucket_owner_enforced` looks for the literal anywhere in the stored
+ // document, so it must appear exactly when it is the rule's value --
+ // never in a second rule or a comment.
+ let enforced_mentions = body_str.matches("BucketOwnerEnforced").count();
+ let expected = usize::from(value == "BucketOwnerEnforced");
+ if !single || enforced_mentions != expected {
+ return Err(AwsServiceError::aws_error(
+ StatusCode::BAD_REQUEST,
+ "MalformedXML",
+ "OwnershipControls must carry exactly one Rule with a valid ObjectOwnership",
+ ));
+ }
let mut accts = self.state.write();
let state = accts.get_or_create(account_id);
let b = state
.buckets
.get_mut(bucket)
.ok_or_else(|| no_such_bucket(bucket))?;
- b.ownership_controls = Some(body_str.clone());
+ // Persist before mutating memory, as the ACL paths do: a store failure
+ // would otherwise leave memory ahead of disk, and a restart would
+ // silently revert what the caller was told had failed.
self.store
.put_bucket_subresource(bucket, BucketSubresource::Ownership, &body_str)
.map_err(crate::service::persistence_error)?;
+ b.ownership_controls = Some(body_str.clone());
Ok(empty_response(StatusCode::OK))
}
diff --git a/crates/fakecloud-s3/src/service/config/tagging.rs b/crates/fakecloud-s3/src/service/config/tagging.rs
index 0d89b7f77..2ab61ee28 100644
--- a/crates/fakecloud-s3/src/service/config/tagging.rs
+++ b/crates/fakecloud-s3/src/service/config/tagging.rs
@@ -62,7 +62,7 @@ impl S3Service {
let snap = TagsSnapshot {
tags: b.tags.clone(),
};
- let payload = toml::to_string(&snap).unwrap_or_default();
+ let payload = crate::service::toml_or_internal_error(&snap)?;
self.store
.put_bucket_subresource(bucket, BucketSubresource::Tags, &payload)
.map_err(crate::service::persistence_error)?;
diff --git a/crates/fakecloud-s3/src/service/mod.rs b/crates/fakecloud-s3/src/service/mod.rs
index 850c2426e..3a8ccbb23 100644
--- a/crates/fakecloud-s3/src/service/mod.rs
+++ b/crates/fakecloud-s3/src/service/mod.rs
@@ -142,6 +142,72 @@ pub struct S3Service {
pub(crate) credential_resolver: Option>,
}
+/// Serialize a persistence snapshot, turning a failure into a 500 rather than an
+/// empty document.
+///
+/// An empty file is worse than no file: the loader either skips it (losing the
+/// configuration with no word, which the next Put then makes permanent) or, for
+/// `acl.toml`, takes its presence to mean an explicit ACL and drops the default
+/// owner grant.
+pub(crate) fn toml_or_internal_error(
+ value: &T,
+) -> Result {
+ toml::to_string(value).map_err(|e| {
+ AwsServiceError::aws_error(
+ StatusCode::INTERNAL_SERVER_ERROR,
+ "InternalError",
+ format!("failed to serialize persisted state: {e}"),
+ )
+ })
+}
+
+/// Reject a request that names an ACL more than one way.
+///
+/// A canned header, the `x-amz-grant-*` headers and an `AccessControlPolicy`
+/// body are mutually exclusive on S3, and letting one win silently discards what
+/// the others asked for -- durably, since the result is persisted. Shared so the
+/// bucket and object paths cannot drift on which pairs they reject.
+pub(crate) fn reject_conflicting_acl_sources(
+ canned: Option<&str>,
+ headers: &HeaderMap,
+ body: &[u8],
+) -> Result<(), AwsServiceError> {
+ let grants = has_grant_headers(headers);
+ let has_body = !body_is_blank(body);
+ let conflict = match (canned.is_some(), grants, has_body) {
+ (true, true, _) => Some("Specifying both Canned ACLs and Header Grants is not allowed"),
+ (true, _, true) => {
+ Some("Specifying both a Canned ACL and an AccessControlPolicy body is not allowed")
+ }
+ (_, true, true) => {
+ Some("Specifying both Header Grants and an AccessControlPolicy body is not allowed")
+ }
+ _ => None,
+ };
+ match conflict {
+ Some(message) => Err(AwsServiceError::aws_error(
+ StatusCode::BAD_REQUEST,
+ "InvalidRequest",
+ message,
+ )),
+ None => Ok(()),
+ }
+}
+
+/// Whether a request body carries nothing an ACL could be read from.
+///
+/// The mutual-exclusion checks and the "names no ACL at all" check have to agree
+/// on this: testing raw emptiness in one and trimmed emptiness in the other made
+/// a canned ACL plus a stray newline -- one ACL, named once -- fail as though it
+/// named two.
+pub(crate) fn body_is_blank(body: &[u8]) -> bool {
+ // A body that does not decode is one no ACL can be read from, which is
+ // exactly how the parse sites treat it (`from_utf8(..).unwrap_or("")`).
+ // Calling it present would reject a canned ACL for conflicting with a body
+ // the next line discards.
+ std::str::from_utf8(body).map_or(true, |s| s.trim().is_empty())
+}
+
/// Map a [`StoreError`] from the persistence layer to a 500 InternalError
/// response. Invoked at every mutation site when the write-through persistence
/// call fails: the in-memory mutation has already happened, but we surface the
@@ -2262,6 +2328,13 @@ pub(crate) fn build_acl_xml(owner_id: &str, grants: &[AclGrant], _account_id: &s
{}",
xml_escape(uri),
)
+ } else if g.grantee_type == "AmazonCustomerByEmail" {
+ let email = g.grantee_display_name.as_deref().unwrap_or("");
+ format!(
+ "\
+ {}",
+ xml_escape(email),
+ )
} else {
let id = g.grantee_id.as_deref().unwrap_or("");
format!(
@@ -2286,6 +2359,71 @@ pub(crate) fn build_acl_xml(owner_id: &str, grants: &[AclGrant], _account_id: &s
)
}
+/// The canned ACLs an object accepts, per `com.amazonaws.s3#ObjectCannedACL`.
+/// `log-delivery-write` is bucket-only and is not among them.
+pub(crate) const OBJECT_CANNED_ACLS: [&str; 7] = [
+ "private",
+ "public-read",
+ "public-read-write",
+ "authenticated-read",
+ "aws-exec-read",
+ "bucket-owner-read",
+ "bucket-owner-full-control",
+];
+
+/// Reject a canned ACL an object does not accept. An unrecognized value used to
+/// fall through `canned_acl_grants`' catch-all to owner-only, silently wiping
+/// the object's grants with a 200.
+pub(crate) fn validate_object_canned_acl(acl: &str) -> Result<(), AwsServiceError> {
+ if OBJECT_CANNED_ACLS.contains(&acl) {
+ return Ok(());
+ }
+ Err(AwsServiceError::aws_error(
+ StatusCode::BAD_REQUEST,
+ "InvalidArgument",
+ format!("Invalid x-amz-acl value: {acl}"),
+ ))
+}
+
+/// The values `x-amz-object-ownership` accepts, per
+/// `com.amazonaws.s3#ObjectOwnership`. Matched case-sensitively, as S3 does.
+pub(crate) const OBJECT_OWNERSHIP_VALUES: [&str; 3] = [
+ "BucketOwnerPreferred",
+ "ObjectWriter",
+ "BucketOwnerEnforced",
+];
+
+/// The canned ACLs a bucket accepts. `com.amazonaws.s3#BucketCannedACL` lists
+/// only the first four; `log-delivery-write` is bucket-scoped in the S3
+/// canned-ACL table (it is how a server-access-logging target is set up), so
+/// the header takes it even though the modeled enum omits it.
+///
+/// `aws-exec-read` is bucket-scoped too (CloudFormation's
+/// `AWS::S3::Bucket` `AccessControl: AwsExecRead` maps to it), and AWS answers
+/// 200, so refusing it would turn a working call into a hard failure. Its grant
+/// is a READ to the EC2 service's canonical user, which this build cannot
+/// reproduce, so it resolves to owner-only -- `create_bucket` special-cases it
+/// when checking the BucketOwnerEnforced conflict, since the request does ask
+/// for an ACL reaching past the owner.
+///
+/// `bucket-owner-read` and `bucket-owner-full-control` are object-scoped, and
+/// the canned-ACL table says S3 IGNORES them when they are given on a bucket
+/// rather than refusing the call -- so they are accepted here and resolve to
+/// the owner's FULL_CONTROL, which is what ignoring them produces. Rejecting
+/// them would have turned a working call into a hard failure on a claim about
+/// live S3 this build cannot check. A value S3 does not define at all is still
+/// refused, since that is a typo silently wiping the bucket's grants.
+pub(crate) const BUCKET_CANNED_ACLS: [&str; 8] = [
+ "private",
+ "public-read",
+ "public-read-write",
+ "authenticated-read",
+ "aws-exec-read",
+ "log-delivery-write",
+ "bucket-owner-read",
+ "bucket-owner-full-control",
+];
+
pub(crate) fn canned_acl_grants(acl: &str, owner_id: &str) -> Vec {
let owner_grant = AclGrant {
grantee_type: "CanonicalUser".to_string(),
@@ -2335,6 +2473,27 @@ pub(crate) fn canned_acl_grants(acl: &str, owner_id: &str) -> Vec {
permission: "READ".to_string(),
},
],
+ // Grants the log-delivery group what it needs to write access logs into
+ // the bucket, which is the entire point of this canned ACL.
+ "log-delivery-write" => vec![
+ owner_grant,
+ AclGrant {
+ grantee_type: "Group".to_string(),
+ grantee_id: None,
+ grantee_display_name: None,
+ grantee_uri: Some("http://acs.amazonaws.com/groups/s3/LogDelivery".to_string()),
+ permission: "WRITE".to_string(),
+ },
+ AclGrant {
+ grantee_type: "Group".to_string(),
+ grantee_id: None,
+ grantee_display_name: None,
+ grantee_uri: Some("http://acs.amazonaws.com/groups/s3/LogDelivery".to_string()),
+ permission: "READ_ACP".to_string(),
+ },
+ ],
+ // `bucket-owner-read` / `bucket-owner-full-control` on an object owned
+ // by the caller come out as the owner's FULL_CONTROL.
"bucket-owner-full-control" => vec![owner_grant],
_ => vec![owner_grant],
}
@@ -2345,24 +2504,110 @@ pub(crate) fn canned_acl_grants_for_object(acl: &str, owner_id: &str) -> Vec usize {
+ let mut clauses = 0;
+ for (header, _) in &GRANT_HEADER_PERMISSIONS {
+ // `get_all`, not `get`: a repeated header carries more than one value,
+ // and counting only the first would let the extras be dropped silently
+ // -- the very thing the count exists to catch.
+ for value in headers.get_all(*header) {
+ match value.to_str() {
+ Ok(value) => {
+ clauses += value
+ .split(',')
+ .filter(|part| !part.trim().is_empty())
+ .count();
+ }
+ // Header values are opaque bytes. `parse_grant_headers` cannot
+ // read this one either, so counting it as zero would let the
+ // counts agree and the grant disappear -- count it as a clause
+ // that did not resolve.
+ Err(_) => clauses += 1,
+ }
+ }
+ }
+ clauses
+}
+
+/// Whether the request carries any of the recognized `x-amz-grant-*` headers.
+/// Matched by exact name, not by prefix, so an unknown `x-amz-grant-…` header
+/// is ignored rather than turning a valid create into an error.
+pub(crate) fn has_grant_headers(headers: &HeaderMap) -> bool {
+ // A present-but-blank value is what a client sends for an unset config
+ // field, and S3 treats it as absent. Testing presence alone turned that
+ // into a hard rejection on every ACL-setting operation.
+ GRANT_HEADER_PERMISSIONS.iter().any(|(header, _)| {
+ headers
+ .get_all(*header)
+ .iter()
+ .any(|v| v.to_str().map(|s| !s.trim().is_empty()).unwrap_or(true))
+ })
+}
+
+/// Resolve the `x-amz-grant-*` headers into grants, refusing the request when
+/// any grantee clause does not resolve into one.
+///
+/// [`parse_grant_headers`] skips a clause it cannot make a grantee of -- a key
+/// with an empty value, or one it does not recognize -- so a caller that STORES
+/// the result as the authoritative ACL would persist a permission set nobody
+/// asked for, and a request where nothing resolves would store an ACL with not
+/// even an owner entry. Every ACL-setting path goes through here so they all
+/// agree. (`emailAddress=` IS resolved, into an AmazonCustomerByEmail grantee.)
+pub(crate) fn resolved_grant_headers(
+ headers: &HeaderMap,
+) -> Result, AwsServiceError> {
+ let grants = parse_grant_headers(headers);
+ if grants.is_empty() || grants.len() != grant_header_clause_count(headers) {
+ return Err(AwsServiceError::aws_error(
+ StatusCode::BAD_REQUEST,
+ "InvalidArgument",
+ "Argument format not recognized: grantees must be specified as id= or uri=",
+ ));
+ }
+ Ok(grants)
+}
+
pub(crate) fn parse_grant_headers(headers: &HeaderMap) -> Vec {
let mut grants = Vec::new();
- let header_permission_map = [
- ("x-amz-grant-read", "READ"),
- ("x-amz-grant-write", "WRITE"),
- ("x-amz-grant-read-acp", "READ_ACP"),
- ("x-amz-grant-write-acp", "WRITE_ACP"),
- ("x-amz-grant-full-control", "FULL_CONTROL"),
- ];
+ let header_permission_map = GRANT_HEADER_PERMISSIONS;
for (header, permission) in &header_permission_map {
- if let Some(value) = headers.get(*header).and_then(|v| v.to_str().ok()) {
+ for value in headers.get_all(*header) {
+ let Ok(value) = value.to_str() else { continue };
// Parse "id=xxx" or "uri=xxx" or "emailAddress=xxx"
for part in value.split(',') {
let part = part.trim();
if let Some((key, val)) = part.split_once('=') {
- let val = val.trim().trim_matches('"');
+ // Quotes first, then trim: trimming first leaves `" "` as
+ // two spaces, which the emptiness checks below would accept
+ // as a grantee naming somebody.
+ let val = val.trim().trim_matches('"').trim();
let key = key.trim().to_lowercase();
+ // `id=` / `uri=` with nothing after it names no grantee.
+ // Emitting a grant with an empty id would store an ACL
+ // entry that can never match anyone; leaving it out lets
+ // the caller's clause count catch the request instead.
+ if val.is_empty() {
+ continue;
+ }
match key.as_str() {
"id" => {
grants.push(AclGrant {
@@ -2373,6 +2618,34 @@ pub(crate) fn parse_grant_headers(headers: &HeaderMap) -> Vec {
permission: permission.to_string(),
});
}
+ // S3 resolves an email to the account's canonical id.
+ // There is no directory to resolve against here, so the
+ // grant keeps the address as an AmazonCustomerByEmail
+ // grantee -- a real ACL grantee type -- rather than
+ // being dropped (which would silently lose a grant) or
+ // refused (which would fail a call AWS accepts).
+ //
+ // It is recorded, not enforced: every authorization path
+ // matches a Group URI or a canonical id, so this grant
+ // gives the named address no access. Say so rather than
+ // letting it look like working authorization.
+ "emailaddress" => {
+ tracing::warn!(
+ target: "fakecloud::s3",
+ email = %val,
+ permission = %permission,
+ "storing an email-addressed ACL grant that is not enforced: \
+ email grantees are not resolved to a canonical user here, so \
+ this grant conveys no access",
+ );
+ grants.push(AclGrant {
+ grantee_type: "AmazonCustomerByEmail".to_string(),
+ grantee_id: None,
+ grantee_display_name: Some(val.to_string()),
+ grantee_uri: None,
+ permission: permission.to_string(),
+ });
+ }
"uri" | "url" => {
grants.push(AclGrant {
grantee_type: "Group".to_string(),
@@ -2422,7 +2695,21 @@ pub(crate) fn parse_acl_xml(xml: &str) -> Result, AwsServiceError>
// Determine grantee type
if grant_body.contains("xsi:type=\"Group\"") || grant_body.contains("") {
- let uri = extract_xml_value(grant_body, "URI").unwrap_or_default();
+ // Trimmed: `\n http://...\n` is what a formatted
+ // policy carries, and the padding would otherwise be stored as
+ // part of the group URI and never match.
+ let uri = extract_xml_value(grant_body, "URI")
+ .map(|u| u.trim().to_string())
+ .unwrap_or_default();
+ // Same rule as an empty : a grant naming no group can never
+ // match anyone, and it would be stored as the bucket's ACL.
+ if uri.is_empty() {
+ return Err(AwsServiceError::aws_error(
+ StatusCode::BAD_REQUEST,
+ "MalformedACLError",
+ "The XML you provided was not well-formed or did not validate against our published schema",
+ ));
+ }
grants.push(AclGrant {
grantee_type: "Group".to_string(),
grantee_id: None,
@@ -2430,10 +2717,60 @@ pub(crate) fn parse_acl_xml(xml: &str) -> Result, AwsServiceError>
grantee_uri: Some(uri),
permission,
});
+ } else if grant_body.contains("AmazonCustomerByEmail")
+ || grant_body.contains("")
+ {
+ // S3 allows exactly one grantee identifier. A Grantee carrying
+ // both an ID and an EmailAddress used to silently drop the
+ // canonical id in favor of the (unenforced) email grant, and the
+ // wrong one was the persisted one.
+ if grant_body.contains("") {
+ return Err(AwsServiceError::aws_error(
+ StatusCode::BAD_REQUEST,
+ "MalformedACLError",
+ "The XML you provided was not well-formed or did not validate against our published schema",
+ ));
+ }
+ // An email grantee carries , never . Reading it
+ // as a CanonicalUser would store a grant with an empty id --
+ // an entry that can never match anyone, which is exactly what
+ // the header path refuses. This is reachable by feeding
+ // GetBucketAcl output straight back into PutBucketAcl.
+ let email = extract_xml_value(grant_body, "EmailAddress")
+ .map(|e| e.trim().to_string())
+ .unwrap_or_default();
+ if email.is_empty() {
+ return Err(AwsServiceError::aws_error(
+ StatusCode::BAD_REQUEST,
+ "MalformedACLError",
+ "The XML you provided was not well-formed or did not validate against our published schema",
+ ));
+ }
+ grants.push(AclGrant {
+ grantee_type: "AmazonCustomerByEmail".to_string(),
+ grantee_id: None,
+ grantee_display_name: Some(email),
+ grantee_uri: None,
+ permission,
+ });
} else {
- let id = extract_xml_value(grant_body, "ID").unwrap_or_default();
- let display =
- extract_xml_value(grant_body, "DisplayName").unwrap_or_else(|| id.clone());
+ // Trimmed for the same reason: a pretty-printed canonical id
+ // would otherwise be stored with its padding and never match
+ // the owner in any authorization comparison.
+ let id = extract_xml_value(grant_body, "ID")
+ .map(|i| i.trim().to_string())
+ .unwrap_or_default();
+ if id.is_empty() {
+ return Err(AwsServiceError::aws_error(
+ StatusCode::BAD_REQUEST,
+ "MalformedACLError",
+ "The XML you provided was not well-formed or did not validate against our published schema",
+ ));
+ }
+ let display = extract_xml_value(grant_body, "DisplayName")
+ .map(|d| d.trim().to_string())
+ .filter(|d| !d.is_empty())
+ .unwrap_or_else(|| id.clone());
grants.push(AclGrant {
grantee_type: "CanonicalUser".to_string(),
grantee_id: Some(id),
diff --git a/crates/fakecloud-s3/src/service/multipart.rs b/crates/fakecloud-s3/src/service/multipart.rs
index dbdf866ad..364d5fb55 100644
--- a/crates/fakecloud-s3/src/service/multipart.rs
+++ b/crates/fakecloud-s3/src/service/multipart.rs
@@ -13,8 +13,8 @@ use md5::{Digest, Md5};
use super::{
canned_acl_grants, compute_md5, extract_user_metadata, no_such_bucket, no_such_key,
- no_such_upload, parse_complete_multipart_xml, parse_grant_headers, parse_url_encoded_tags,
- precondition_failed, resolve_object, s3_xml, xml_escape, S3Service,
+ no_such_upload, parse_complete_multipart_xml, parse_url_encoded_tags, precondition_failed,
+ resolve_object, resolved_grant_headers, s3_xml, xml_escape, S3Service,
};
/// Build the `CompleteMultipartUploadResult` XML response for an object that
@@ -100,10 +100,19 @@ impl S3Service {
.get("x-amz-acl")
.and_then(|v| v.to_str().ok())
.map(|s| s.to_string());
- let has_grant_headers = req
- .headers
- .keys()
- .any(|k| k.as_str().starts_with("x-amz-grant-"));
+ let has_grant_headers = super::has_grant_headers(&req.headers);
+ // Every sibling ACL-setting path rejects an ACL on a bucket whose
+ // ownership disables them; this one used to accept it and carry the
+ // grants into the completed object, which now persists them.
+ if (acl_header.is_some() || has_grant_headers)
+ && self.bucket_owner_enforced(account_id, bucket)
+ {
+ return Err(AwsServiceError::aws_error(
+ StatusCode::BAD_REQUEST,
+ "AccessControlListNotSupported",
+ "The bucket does not allow ACLs",
+ ));
+ }
if acl_header.is_some() && has_grant_headers {
return Err(AwsServiceError::aws_error(
@@ -128,9 +137,10 @@ impl S3Service {
.ok_or_else(|| no_such_bucket(bucket))?;
let acl_grants = if has_grant_headers {
- parse_grant_headers(&req.headers)
+ resolved_grant_headers(&req.headers)?
} else {
let acl = acl_header.as_deref().unwrap_or("private");
+ super::validate_object_canned_acl(acl)?;
canned_acl_grants(acl, &b.acl_owner_id)
};
diff --git a/crates/fakecloud-s3/src/service/objects/mod.rs b/crates/fakecloud-s3/src/service/objects/mod.rs
index 9a3b57687..7682b2adb 100644
--- a/crates/fakecloud-s3/src/service/objects/mod.rs
+++ b/crates/fakecloud-s3/src/service/objects/mod.rs
@@ -15,8 +15,8 @@ use super::{
check_object_lock_for_overwrite, compute_checksum, deliver_notifications, etag_matches,
extract_user_metadata, extract_xml_value, is_frozen, is_valid_storage_class,
make_delete_marker, no_such_bucket, no_such_key, parse_delete_objects_quiet,
- parse_delete_objects_xml, parse_grant_headers, parse_range_header, parse_url_encoded_tags,
- precondition_failed, replicate_through_store, resolve_object, s3_xml, url_encode_s3_key,
+ parse_delete_objects_xml, parse_range_header, parse_url_encoded_tags, precondition_failed,
+ replicate_through_store, resolve_object, resolved_grant_headers, s3_xml, url_encode_s3_key,
xml_escape, RangeResult, S3Service,
};
diff --git a/crates/fakecloud-s3/src/service/objects/write.rs b/crates/fakecloud-s3/src/service/objects/write.rs
index 221328470..94f8dea08 100644
--- a/crates/fakecloud-s3/src/service/objects/write.rs
+++ b/crates/fakecloud-s3/src/service/objects/write.rs
@@ -48,11 +48,17 @@ impl S3Service {
.map(|s| s.to_string());
// Check for grant headers alongside canned ACL
- let has_grant_headers = req.headers.keys().any(|k| {
- let name = k.as_str();
- name.starts_with("x-amz-grant-")
- });
+ let has_grant_headers = super::super::has_grant_headers(&req.headers);
+ // Validated here, before `take_body_stream` spools the payload to disk:
+ // returning after the spool leaks the file, since nothing unlinks it on
+ // the error paths.
+ if let Some(acl) = acl_header.as_deref() {
+ super::super::validate_object_canned_acl(acl)?;
+ }
+ if has_grant_headers {
+ resolved_grant_headers(&req.headers)?;
+ }
if acl_header.is_some() && has_grant_headers {
return Err(AwsServiceError::aws_error(
StatusCode::BAD_REQUEST,
@@ -301,7 +307,9 @@ impl S3Service {
// Build ACL grants for object
let acl_grants = if has_grant_headers {
- parse_grant_headers(&req.headers)
+ // Already validated before the body was spooled; this cannot fail
+ // here, but resolving again keeps one source for the grants.
+ resolved_grant_headers(&req.headers)?
} else if let Some(ref acl) = acl_header {
canned_acl_grants_for_object(acl, &acl_owner_id)
} else {
diff --git a/crates/fakecloud-s3/src/service/tests.rs b/crates/fakecloud-s3/src/service/tests.rs
index 4cae69419..6b9f5369d 100644
--- a/crates/fakecloud-s3/src/service/tests.rs
+++ b/crates/fakecloud-s3/src/service/tests.rs
@@ -3473,6 +3473,748 @@ fn create_bucket_idempotent_same_region_us_east_1() {
assert_eq!(resp.status, StatusCode::OK);
}
+#[test]
+fn create_bucket_honors_grant_headers() {
+ let svc = make_service();
+ let mut req = make_request(Method::PUT, "/granted", &[], b"");
+ req.headers.insert(
+ "x-amz-grant-read",
+ "uri=http://acs.amazonaws.com/groups/global/AllUsers"
+ .parse()
+ .unwrap(),
+ );
+ svc.create_bucket("123456789012", &req, "granted").unwrap();
+
+ let get = make_request(Method::GET, "/granted", &[("acl", "")], b"");
+ let resp = svc.get_bucket_acl("123456789012", &get, "granted").unwrap();
+ let body = std::str::from_utf8(resp.body.expect_bytes()).unwrap();
+ assert!(body.contains("AllUsers"), "grant header ignored: {body}");
+ assert!(body.contains("READ"), "{body}");
+}
+
+#[test]
+fn create_bucket_rejects_canned_acl_and_grant_headers_together() {
+ let svc = make_service();
+ let mut req = make_request(Method::PUT, "/both-acl", &[], b"");
+ req.headers
+ .insert("x-amz-acl", "public-read".parse().unwrap());
+ req.headers.insert(
+ "x-amz-grant-read",
+ "uri=http://acs.amazonaws.com/groups/global/AllUsers"
+ .parse()
+ .unwrap(),
+ );
+ assert_aws_err(
+ svc.create_bucket("123456789012", &req, "both-acl"),
+ "InvalidRequest",
+ );
+ assert_aws_err(svc.head_bucket("123456789012", "both-acl"), "NotFound");
+}
+
+#[test]
+fn create_bucket_keeps_an_email_grantee_instead_of_dropping_or_refusing_it() {
+ // S3 resolves `emailAddress=` against its own directory and answers 200.
+ // There is nothing to resolve against here, so the grant keeps the address
+ // as an AmazonCustomerByEmail grantee: dropping it would silently lose a
+ // grant, and refusing would fail a call AWS accepts.
+ let svc = make_service();
+ let mut req = make_request(Method::PUT, "/email-grant", &[], b"");
+ req.headers.insert(
+ "x-amz-grant-full-control",
+ "emailAddress=someone@example.com".parse().unwrap(),
+ );
+ svc.create_bucket("123456789012", &req, "email-grant")
+ .unwrap();
+
+ let get = make_request(Method::GET, "/email-grant", &[("acl", "")], b"");
+ let resp = svc
+ .get_bucket_acl("123456789012", &get, "email-grant")
+ .unwrap();
+ let body = std::str::from_utf8(resp.body.expect_bytes()).unwrap();
+ assert!(
+ body.contains("AmazonCustomerByEmail")
+ && body.contains("someone@example.com"),
+ "email grantee not preserved: {body}"
+ );
+}
+
+#[test]
+fn create_bucket_rejects_an_unparseable_grantee_rather_than_writing_an_empty_acl() {
+ // A clause naming no known grantee key parses to nothing. Storing that as
+ // the bucket's ACL would leave it with no grants at all -- not even the
+ // owner -- which the loader honors as an explicit empty ACL.
+ let svc = make_service();
+ for value in ["nonsense=whoever", "justtext"] {
+ let mut req = make_request(Method::PUT, "/bad-grantee", &[], b"");
+ req.headers
+ .insert("x-amz-grant-full-control", value.parse().unwrap());
+ assert_aws_err(
+ svc.create_bucket("123456789012", &req, "bad-grantee"),
+ "InvalidArgument",
+ );
+ }
+ assert_aws_err(svc.head_bucket("123456789012", "bad-grantee"), "NotFound");
+}
+
+#[test]
+fn create_bucket_rejects_an_acl_alongside_bucket_owner_enforced() {
+ // Real S3 answers InvalidBucketAclWithObjectOwnership: BucketOwnerEnforced
+ // disables ACLs, so a create cannot also ask for one. Allowing it would
+ // leave a publicly readable bucket whose ACL PutBucketAcl refuses to edit.
+ let svc = make_service();
+ let mut req = make_request(Method::PUT, "/owner-enforced", &[], b"");
+ req.headers
+ .insert("x-amz-acl", "public-read".parse().unwrap());
+ req.headers.insert(
+ "x-amz-object-ownership",
+ "BucketOwnerEnforced".parse().unwrap(),
+ );
+ assert_aws_err(
+ svc.create_bucket("123456789012", &req, "owner-enforced"),
+ "InvalidBucketAclWithObjectOwnership",
+ );
+
+ // `private` grants nothing beyond the owner, so S3 accepts it alongside
+ // BucketOwnerEnforced -- and so do we.
+ let mut ok_req = make_request(Method::PUT, "/owner-enforced-ok", &[], b"");
+ ok_req
+ .headers
+ .insert("x-amz-acl", "private".parse().unwrap());
+ ok_req.headers.insert(
+ "x-amz-object-ownership",
+ "BucketOwnerEnforced".parse().unwrap(),
+ );
+ svc.create_bucket("123456789012", &ok_req, "owner-enforced-ok")
+ .unwrap();
+
+ // A grant header conflicts on presence alone, even one naming the owner:
+ // the caller is asking for an ACL on a bucket that has none.
+ let mut grant_req = make_request(Method::PUT, "/owner-enforced-grant", &[], b"");
+ grant_req.headers.insert(
+ "x-amz-grant-full-control",
+ "id=123456789012".parse().unwrap(),
+ );
+ grant_req.headers.insert(
+ "x-amz-object-ownership",
+ "BucketOwnerEnforced".parse().unwrap(),
+ );
+ assert_aws_err(
+ svc.create_bucket("123456789012", &grant_req, "owner-enforced-grant"),
+ "InvalidBucketAclWithObjectOwnership",
+ );
+}
+
+#[test]
+fn create_bucket_rejects_a_canned_acl_that_is_not_a_bucket_canned_acl() {
+ // `com.amazonaws.s3#BucketCannedACL` has exactly four members;
+ // `bucket-owner-full-control` is an OBJECT canned ACL, and a typo is not an
+ // ACL at all. Both used to fall through to owner-only grants silently, and
+ // this branch would then persist that as the bucket's explicit ACL.
+ let svc = make_service();
+ for value in ["pubic-read", "not-an-acl"] {
+ let mut req = make_request(Method::PUT, "/bad-canned", &[], b"");
+ req.headers.insert("x-amz-acl", value.parse().unwrap());
+ assert_aws_err(
+ svc.create_bucket("123456789012", &req, "bad-canned"),
+ "InvalidArgument",
+ );
+ }
+ assert_aws_err(svc.head_bucket("123456789012", "bad-canned"), "NotFound");
+
+ // The object-scoped canned ACLs are IGNORED on a bucket rather than
+ // refused, so they are accepted and resolve to the owner's FULL_CONTROL.
+ let mut owner_acl = make_request(Method::PUT, "/owner-canned", &[], b"");
+ owner_acl
+ .headers
+ .insert("x-amz-acl", "bucket-owner-full-control".parse().unwrap());
+ svc.create_bucket("123456789012", &owner_acl, "owner-canned")
+ .unwrap();
+}
+
+#[test]
+fn create_bucket_accepts_log_delivery_write_with_real_grants() {
+ // log-delivery-write is bucket-scoped in S3's canned-ACL table even though
+ // `com.amazonaws.s3#BucketCannedACL` omits it, and it has to actually grant
+ // the log-delivery group write access -- that is the whole point of using
+ // it on a logging target.
+ let svc = make_service();
+ let mut req = make_request(Method::PUT, "/canned-logdelivery", &[], b"");
+ req.headers
+ .insert("x-amz-acl", "log-delivery-write".parse().unwrap());
+ svc.create_bucket("123456789012", &req, "canned-logdelivery")
+ .unwrap();
+
+ let get = make_request(Method::GET, "/canned-logdelivery", &[("acl", "")], b"");
+ let resp = svc
+ .get_bucket_acl("123456789012", &get, "canned-logdelivery")
+ .unwrap();
+ let body = std::str::from_utf8(resp.body.expect_bytes()).unwrap();
+ assert!(body.contains("s3/LogDelivery"), "{body}");
+ assert!(body.contains("WRITE"), "{body}");
+}
+
+#[test]
+fn create_bucket_accepts_aws_exec_read_but_treats_it_as_granting_past_the_owner() {
+ // aws-exec-read is bucket-scoped and AWS answers 200 (CloudFormation's
+ // AccessControl: AwsExecRead maps to it), so refusing it would break a
+ // working call. Its READ grant to the EC2 service's canonical user is not
+ // modeled, so the stored ACL is owner-only -- but the request still asks for
+ // an ACL reaching past the owner, so it conflicts with BucketOwnerEnforced.
+ let svc = make_service();
+ let mut req = make_request(Method::PUT, "/aws-exec", &[], b"");
+ req.headers
+ .insert("x-amz-acl", "aws-exec-read".parse().unwrap());
+ svc.create_bucket("123456789012", &req, "aws-exec").unwrap();
+
+ let mut enforced = make_request(Method::PUT, "/aws-exec-enforced", &[], b"");
+ enforced
+ .headers
+ .insert("x-amz-acl", "aws-exec-read".parse().unwrap());
+ enforced.headers.insert(
+ "x-amz-object-ownership",
+ "BucketOwnerEnforced".parse().unwrap(),
+ );
+ assert_aws_err(
+ svc.create_bucket("123456789012", &enforced, "aws-exec-enforced"),
+ "InvalidBucketAclWithObjectOwnership",
+ );
+}
+
+#[test]
+fn put_object_acl_rejects_a_body_that_is_not_an_access_control_policy() {
+ // A non-XML body parses to zero grants; treating that as "remove every
+ // grant" strips the object's owner FULL_CONTROL and writes the empty list
+ // into its meta.
+ let svc = make_service();
+ seed_bucket(&svc, "obj-junk");
+ seed_object(&svc, "obj-junk", "k.txt", b"body");
+
+ let json = make_request(Method::PUT, "/obj-junk/k.txt", &[("acl", "")], b"{}");
+ assert_aws_err(
+ svc.put_object_acl("123456789012", &json, "obj-junk", "k.txt"),
+ "MalformedXML",
+ );
+
+ let blank = make_request(Method::PUT, "/obj-junk/k.txt", &[("acl", "")], b" ");
+ assert_aws_err(
+ svc.put_object_acl("123456789012", &blank, "obj-junk", "k.txt"),
+ "MalformedACLError",
+ );
+}
+
+#[test]
+fn put_object_acl_rejects_a_canned_acl_with_grants_or_a_body() {
+ // Letting the canned value win discarded the grants the caller asked for,
+ // and the result is written into the object meta.
+ let svc = make_service();
+ seed_bucket(&svc, "obj-both");
+ seed_object(&svc, "obj-both", "k.txt", b"body");
+
+ let mut with_grant = make_request(Method::PUT, "/obj-both/k.txt", &[("acl", "")], b"");
+ with_grant
+ .headers
+ .insert("x-amz-acl", "private".parse().unwrap());
+ with_grant.headers.insert(
+ "x-amz-grant-read",
+ "uri=http://acs.amazonaws.com/groups/global/AllUsers"
+ .parse()
+ .unwrap(),
+ );
+ assert_aws_err(
+ svc.put_object_acl("123456789012", &with_grant, "obj-both", "k.txt"),
+ "InvalidRequest",
+ );
+
+ let mut with_body = make_request(
+ Method::PUT,
+ "/obj-both/k.txt",
+ &[("acl", "")],
+ b"123456789012",
+ );
+ with_body
+ .headers
+ .insert("x-amz-acl", "private".parse().unwrap());
+ assert_aws_err(
+ svc.put_object_acl("123456789012", &with_body, "obj-both", "k.txt"),
+ "InvalidRequest",
+ );
+}
+
+#[test]
+fn create_multipart_upload_rejects_an_acl_when_ownership_disables_them() {
+ // Every sibling ACL-setting path refuses this; multipart used to accept it
+ // and carry the grants into the completed object, which is now persisted.
+ let svc = make_service();
+ seed_bucket(&svc, "mpu-owner");
+ {
+ let mut mas = svc.state.write();
+ let state = mas.default_mut();
+ let b = state.buckets.get_mut("mpu-owner").unwrap();
+ b.ownership_controls = Some(
+ "BucketOwnerEnforced\
+ "
+ .to_string(),
+ );
+ }
+
+ let mut canned = make_request(Method::POST, "/mpu-owner/k.txt", &[("uploads", "")], b"");
+ canned
+ .headers
+ .insert("x-amz-acl", "public-read".parse().unwrap());
+ assert_aws_err(
+ svc.create_multipart_upload("123456789012", &canned, "mpu-owner", "k.txt"),
+ "AccessControlListNotSupported",
+ );
+
+ let mut granted = make_request(Method::POST, "/mpu-owner/k.txt", &[("uploads", "")], b"");
+ granted.headers.insert(
+ "x-amz-grant-read",
+ "uri=http://acs.amazonaws.com/groups/global/AllUsers"
+ .parse()
+ .unwrap(),
+ );
+ assert_aws_err(
+ svc.create_multipart_upload("123456789012", &granted, "mpu-owner", "k.txt"),
+ "AccessControlListNotSupported",
+ );
+}
+
+#[test]
+fn put_bucket_ownership_controls_rejects_an_unknown_value() {
+ // An unrecognized value used to be stored verbatim and reloaded as an
+ // ownership rule meaning nothing, silently returning the bucket to
+ // ACLs-enabled.
+ let svc = make_service();
+ seed_bucket(&svc, "own-bad");
+ let req = make_request(
+ Method::PUT,
+ "/own-bad",
+ &[("ownershipControls", "")],
+ b"Whatever",
+ );
+ assert_aws_err(
+ svc.put_bucket_ownership_controls("123456789012", &req, "own-bad"),
+ "InvalidArgument",
+ );
+}
+
+#[test]
+fn put_object_acl_updates_the_versioned_copy_too() {
+ // A versioned bucket keeps a second copy of the current version in
+ // `object_versions`. Leaving it stale let a later delete re-derive the
+ // current object from the old grants, silently reverting a change that had
+ // already been persisted. The ids have to be real: with None on both sides
+ // the version guard matches anything and the test proves nothing.
+ let svc = make_service();
+ seed_bucket(&svc, "ver-acl");
+ seed_object(&svc, "ver-acl", "k.txt", b"body");
+ {
+ let mut mas = svc.state.write();
+ let state = mas.default_mut();
+ let b = state.buckets.get_mut("ver-acl").unwrap();
+ b.versioning = Some("Enabled".to_string());
+ let mut current = b.objects.get("k.txt").unwrap().clone();
+ current.version_id = Some("v2".to_string());
+ let mut older = current.clone();
+ older.version_id = Some("v1".to_string());
+ b.objects.insert("k.txt".to_string(), current.clone());
+ b.object_versions
+ .insert("k.txt".to_string(), vec![older, current]);
+ }
+
+ let mut req = make_request(Method::PUT, "/ver-acl/k.txt", &[("acl", "")], b"");
+ req.headers
+ .insert("x-amz-acl", "public-read".parse().unwrap());
+ svc.put_object_acl("123456789012", &req, "ver-acl", "k.txt")
+ .unwrap();
+
+ let mas = svc.state.read();
+ let state = mas.get("123456789012").unwrap();
+ let b = state.buckets.get("ver-acl").unwrap();
+ let versions = b.object_versions.get("k.txt").unwrap();
+ let public = |o: &crate::state::S3Object| {
+ o.acl_grants.iter().any(|g| {
+ g.grantee_uri
+ .as_deref()
+ .is_some_and(|u| u.contains("AllUsers"))
+ })
+ };
+ let current = versions
+ .iter()
+ .find(|v| v.version_id.as_deref() == Some("v2"));
+ assert!(
+ public(current.expect("v2 present")),
+ "the current version's copy kept the old grants"
+ );
+ let older = versions
+ .iter()
+ .find(|v| v.version_id.as_deref() == Some("v1"));
+ assert!(
+ !public(older.expect("v1 present")),
+ "an older version must not be touched"
+ );
+}
+
+#[test]
+fn put_object_acl_rejects_an_unresolvable_grantee() {
+ // The object paths share parse_grant_headers, which drops what it cannot
+ // resolve. Without the shared guard they stored an object ACL with not even
+ // the owner's FULL_CONTROL.
+ let svc = make_service();
+ seed_bucket(&svc, "obj-acl");
+ seed_object(&svc, "obj-acl", "k.txt", b"body");
+
+ let mut req = make_request(Method::PUT, "/obj-acl/k.txt", &[("acl", "")], b"");
+ req.headers.insert(
+ "x-amz-grant-full-control",
+ "nonsense=alice".parse().unwrap(),
+ );
+ assert_aws_err(
+ svc.put_object_acl("123456789012", &req, "obj-acl", "k.txt"),
+ "InvalidArgument",
+ );
+}
+
+#[test]
+fn create_bucket_rejects_a_grant_header_with_an_empty_grantee() {
+ // `id=` with nothing after it names no grantee. Storing a grant with an
+ // empty canonical id would put an entry in the bucket's ACL that can never
+ // match anyone.
+ let svc = make_service();
+ for value in ["id=", "uri=", "id= "] {
+ let mut req = make_request(Method::PUT, "/empty-grantee", &[], b"");
+ req.headers
+ .insert("x-amz-grant-full-control", value.parse().unwrap());
+ assert_aws_err(
+ svc.create_bucket("123456789012", &req, "empty-grantee"),
+ "InvalidArgument",
+ );
+ }
+ assert_aws_err(svc.head_bucket("123456789012", "empty-grantee"), "NotFound");
+}
+
+#[test]
+fn put_bucket_acl_rejects_a_body_that_is_not_an_access_control_policy() {
+ // parse_acl_xml returns no grants for a body with no and no
+ // 123456789012",
+ );
+ svc.put_bucket_acl("123456789012", &empty_list, "pba-junk")
+ .unwrap();
+}
+
+#[test]
+fn put_bucket_acl_rejects_a_canned_acl_alongside_a_policy_body() {
+ // Letting the canned value win would answer 200 while dropping everything
+ // the body granted, and this branch persists the result.
+ let svc = make_service();
+ let create = make_request(Method::PUT, "/pba-canned-body", &[], b"");
+ svc.create_bucket("123456789012", &create, "pba-canned-body")
+ .unwrap();
+
+ let mut req = make_request(
+ Method::PUT,
+ "/pba-canned-body",
+ &[("acl", "")],
+ b"123456789012http://acs.amazonaws.com/groups/global/AllUsersREAD",
+ );
+ req.headers.insert("x-amz-acl", "private".parse().unwrap());
+ assert_aws_err(
+ svc.put_bucket_acl("123456789012", &req, "pba-canned-body"),
+ "InvalidRequest",
+ );
+}
+
+#[test]
+fn bucket_acl_email_grantee_round_trips_through_the_policy_body() {
+ // GetBucketAcl now emits AmazonCustomerByEmail, so its output can be fed
+ // straight back into PutBucketAcl. Reading that grantee as a CanonicalUser
+ // would store an empty-id grant -- an entry matching nobody, which the
+ // header path refuses.
+ let svc = make_service();
+ let mut create = make_request(Method::PUT, "/email-rt", &[], b"");
+ create.headers.insert(
+ "x-amz-grant-read",
+ "emailAddress=someone@example.com".parse().unwrap(),
+ );
+ svc.create_bucket("123456789012", &create, "email-rt")
+ .unwrap();
+
+ let get = make_request(Method::GET, "/email-rt", &[("acl", "")], b"");
+ let first = svc
+ .get_bucket_acl("123456789012", &get, "email-rt")
+ .unwrap();
+ let xml = std::str::from_utf8(first.body.expect_bytes())
+ .unwrap()
+ .to_string();
+
+ let put = make_request(Method::PUT, "/email-rt", &[("acl", "")], xml.as_bytes());
+ svc.put_bucket_acl("123456789012", &put, "email-rt")
+ .unwrap();
+
+ let second = svc
+ .get_bucket_acl("123456789012", &get, "email-rt")
+ .unwrap();
+ let body = std::str::from_utf8(second.body.expect_bytes()).unwrap();
+ assert!(
+ body.contains("someone@example.com"),
+ "email grantee lost on the body round trip: {body}"
+ );
+ // Assert against the stored ACL, not the rendering: the grant has to BE an
+ // email grantee, not merely render without an empty .
+ let mas = svc.state.read();
+ let state = mas.get("123456789012").unwrap();
+ let stored = &state.buckets.get("email-rt").unwrap().acl_grants;
+ assert!(
+ stored.iter().any(|g| {
+ g.grantee_type == "AmazonCustomerByEmail"
+ && g.grantee_display_name.as_deref() == Some("someone@example.com")
+ }),
+ "the round trip did not store an email grantee: {stored:?}"
+ );
+ assert!(
+ stored.iter().all(|g| g.grantee_id.as_deref() != Some("")),
+ "stored a grant naming nobody: {stored:?}"
+ );
+}
+
+#[test]
+fn put_bucket_acl_rejects_a_grantee_naming_nobody() {
+ let svc = make_service();
+ let create = make_request(Method::PUT, "/pba-empty-id", &[], b"");
+ svc.create_bucket("123456789012", &create, "pba-empty-id")
+ .unwrap();
+
+ let req = make_request(
+ Method::PUT,
+ "/pba-empty-id",
+ &[("acl", "")],
+ b"123456789012FULL_CONTROL",
+ );
+ assert_aws_err(
+ svc.put_bucket_acl("123456789012", &req, "pba-empty-id"),
+ "MalformedACLError",
+ );
+}
+
+#[test]
+fn put_bucket_acl_rejects_grant_headers_alongside_a_policy_body() {
+ let svc = make_service();
+ let create = make_request(Method::PUT, "/pba-both", &[], b"");
+ svc.create_bucket("123456789012", &create, "pba-both")
+ .unwrap();
+
+ let mut req = make_request(
+ Method::PUT,
+ "/pba-both",
+ &[("acl", "")],
+ b"123456789012123456789012FULL_CONTROL",
+ );
+ req.headers.insert(
+ "x-amz-grant-read",
+ "uri=http://acs.amazonaws.com/groups/global/AllUsers"
+ .parse()
+ .unwrap(),
+ );
+ assert_aws_err(
+ svc.put_bucket_acl("123456789012", &req, "pba-both"),
+ "InvalidRequest",
+ );
+}
+
+#[test]
+fn create_bucket_rejects_an_unknown_object_ownership_value() {
+ // An unrecognized value used to be stored verbatim, and this branch
+ // persists it -- so the bucket would come back after a restart with an
+ // OwnershipControls rule that means nothing.
+ let svc = make_service();
+ let mut req = make_request(Method::PUT, "/bad-ownership", &[], b"");
+ req.headers
+ .insert("x-amz-object-ownership", "Whatever".parse().unwrap());
+ assert_aws_err(
+ svc.create_bucket("123456789012", &req, "bad-ownership"),
+ "InvalidArgument",
+ );
+
+ // Casing is significant: `bucket_owner_enforced` matches the stored XML
+ // case-sensitively, so a differently cased value must not be accepted here
+ // and then read back as "ACLs enabled" by the next call.
+ let mut cased = make_request(Method::PUT, "/cased-ownership", &[], b"");
+ cased.headers.insert(
+ "x-amz-object-ownership",
+ "bucketownerenforced".parse().unwrap(),
+ );
+ assert_aws_err(
+ svc.create_bucket("123456789012", &cased, "cased-ownership"),
+ "InvalidArgument",
+ );
+}
+
+#[test]
+fn put_bucket_acl_rejects_an_unknown_canned_value_instead_of_wiping_grants() {
+ // The catch-all in `canned_acl_grants` turned an unrecognized canned ACL
+ // into owner-only, silently stripping every public grant -- and that wipe
+ // is now persisted.
+ let svc = make_service();
+ let create = make_request(Method::PUT, "/pba-canned", &[], b"");
+ svc.create_bucket("123456789012", &create, "pba-canned")
+ .unwrap();
+
+ let mut req = make_request(Method::PUT, "/pba-canned", &[("acl", "")], b"");
+ req.headers
+ .insert("x-amz-acl", "pubic-read".parse().unwrap());
+ assert_aws_err(
+ svc.put_bucket_acl("123456789012", &req, "pba-canned"),
+ "InvalidArgument",
+ );
+}
+
+#[test]
+fn put_bucket_acl_honors_grant_headers() {
+ let svc = make_service();
+ let create = make_request(Method::PUT, "/pba-grant", &[], b"");
+ svc.create_bucket("123456789012", &create, "pba-grant")
+ .unwrap();
+
+ let mut req = make_request(Method::PUT, "/pba-grant", &[("acl", "")], b"");
+ req.headers.insert(
+ "x-amz-grant-read",
+ "uri=http://acs.amazonaws.com/groups/global/AllUsers"
+ .parse()
+ .unwrap(),
+ );
+ svc.put_bucket_acl("123456789012", &req, "pba-grant")
+ .unwrap();
+
+ let get = make_request(Method::GET, "/pba-grant", &[("acl", "")], b"");
+ let resp = svc
+ .get_bucket_acl("123456789012", &get, "pba-grant")
+ .unwrap();
+ let body = std::str::from_utf8(resp.body.expect_bytes()).unwrap();
+ assert!(body.contains("AllUsers"), "grant headers ignored: {body}");
+}
+
+#[test]
+fn put_bucket_acl_rejects_a_request_that_names_no_acl() {
+ // No canned header, no grant header, no body: malformed, not "drop every
+ // grant". Without this the bucket's ACL was wiped to nothing, persisted.
+ let svc = make_service();
+ let create = make_request(Method::PUT, "/pba-empty", &[], b"");
+ svc.create_bucket("123456789012", &create, "pba-empty")
+ .unwrap();
+
+ let req = make_request(Method::PUT, "/pba-empty", &[("acl", "")], b"");
+ assert_aws_err(
+ svc.put_bucket_acl("123456789012", &req, "pba-empty"),
+ "MalformedACLError",
+ );
+
+ // The grants the bucket had are untouched.
+ let get = make_request(Method::GET, "/pba-empty", &[("acl", "")], b"");
+ let resp = svc
+ .get_bucket_acl("123456789012", &get, "pba-empty")
+ .unwrap();
+ let body = std::str::from_utf8(resp.body.expect_bytes()).unwrap();
+ assert!(body.contains("FULL_CONTROL"), "{body}");
+}
+
+#[test]
+fn create_bucket_rejects_an_empty_grant_header() {
+ // An empty or blank header value resolves to no grants at all. Storing that
+ // would leave the bucket with not even an owner grant, permanently.
+ // A blank value is what a client sends for an unset config field, and S3
+ // treats the header as absent rather than failing the call.
+ let svc = make_service();
+ for (name, value) in [("blank-empty", ""), ("blank-space", " ")] {
+ let path = format!("/{name}");
+ let mut req = make_request(Method::PUT, &path, &[], b"");
+ req.headers
+ .insert("x-amz-grant-read", value.parse().unwrap());
+ svc.create_bucket("123456789012", &req, name)
+ .unwrap_or_else(|e| panic!("blank grant header should be ignored: {e:?}"));
+ }
+
+ // A value that carries something but names no grantee is still refused: a
+ // bare separator, or a grantee form this build cannot resolve.
+ for value in [",", "nonsense=x"] {
+ let mut bad = make_request(Method::PUT, "/blank-grant", &[], b"");
+ bad.headers
+ .insert("x-amz-grant-read", value.parse().unwrap());
+ assert_aws_err(
+ svc.create_bucket("123456789012", &bad, "blank-grant"),
+ "InvalidArgument",
+ );
+ }
+ assert_aws_err(svc.head_bucket("123456789012", "blank-grant"), "NotFound");
+}
+
+#[test]
+fn create_bucket_rejects_partially_unresolvable_grant_headers() {
+ // A mixed request is the dangerous case: the resolvable clause would be
+ // stored as the bucket's authoritative ACL while the unresolvable one is
+ // dropped, so GetBucketAcl would report a permission set nobody asked for.
+ let svc = make_service();
+ let mut req = make_request(Method::PUT, "/mixed-grant", &[], b"");
+ req.headers.insert(
+ "x-amz-grant-full-control",
+ "nonsense=alice".parse().unwrap(),
+ );
+ req.headers.insert(
+ "x-amz-grant-read",
+ "uri=http://acs.amazonaws.com/groups/global/AllUsers"
+ .parse()
+ .unwrap(),
+ );
+ assert_aws_err(
+ svc.create_bucket("123456789012", &req, "mixed-grant"),
+ "InvalidArgument",
+ );
+ assert_aws_err(svc.head_bucket("123456789012", "mixed-grant"), "NotFound");
+}
+
+#[test]
+fn create_bucket_ignores_an_unrecognized_grant_header() {
+ // Only the five real `x-amz-grant-*` headers count. A lookalike must not
+ // turn a valid create into an error, nor collide with a canned ACL.
+ let svc = make_service();
+ let mut req = make_request(Method::PUT, "/odd-grant", &[], b"");
+ req.headers
+ .insert("x-amz-acl", "public-read".parse().unwrap());
+ req.headers
+ .insert("x-amz-grant-nonsense", "id=whoever".parse().unwrap());
+ svc.create_bucket("123456789012", &req, "odd-grant")
+ .unwrap();
+
+ let get = make_request(Method::GET, "/odd-grant", &[("acl", "")], b"");
+ let resp = svc
+ .get_bucket_acl("123456789012", &get, "odd-grant")
+ .unwrap();
+ let body = std::str::from_utf8(resp.body.expect_bytes()).unwrap();
+ assert!(body.contains("AllUsers"), "canned ACL not applied: {body}");
+}
+
#[test]
fn create_bucket_stores_tags_from_create_bucket_configuration() {
let svc = make_service();
diff --git a/website/content/docs/services/s3.md b/website/content/docs/services/s3.md
index 10b3a2189..efa446855 100644
--- a/website/content/docs/services/s3.md
+++ b/website/content/docs/services/s3.md
@@ -18,7 +18,8 @@ fakecloud implements **107 of 107** S3 operations at 100% Smithy conformance.
- **Bucket subresources** — policy, CORS, lifecycle, logging, website, public access block, object lock, replication, ownership, inventory, encryption, accelerate, request payment, tagging
- **Bucket tags at create time** — `CreateBucket` honors the `Tags` tag set inside `CreateBucketConfiguration`, so a bucket comes out of `CreateBucket` already tagged and `GetBucketTagging` returns the set without a follow-up `PutBucketTagging`. This is the path the AWS Terraform provider (6.x) uses for `aws_s3_bucket`'s `tags`. Duplicate keys and `aws:`-prefixed keys are rejected with `InvalidTag` and no bucket is created, and the tags persist across a restart like any other bucket subresource. Under `--iam`, a tagged create is authorized as `s3:CreateBucket` **and** `s3:TagResource`, as on AWS — a grant of `s3:CreateBucket` alone still creates untagged buckets — and the tag set populates `aws:RequestTag/` / `aws:TagKeys` so policies can condition on it.
- **CORS** — `PutBucketCors` rules drive real browser CORS: the `OPTIONS` preflight is matched on origin, `Access-Control-Request-Method` and every header in `Access-Control-Request-Headers` before returning `Access-Control-Allow-Origin/-Methods/-Headers` and `Access-Control-Max-Age`, and an actual request carrying `Origin` gets `Access-Control-Allow-Origin` plus `Access-Control-Expose-Headers`. Actual requests are matched on method as well as origin, so a rule allowing only `GET` hands no allow-origin to a `DELETE`. Multipart operations are CORS-evaluated like any other request, so browser multipart upload works. Errors from S3 itself are evaluated too, so a 404 `NoSuchKey` fetched by an allowed origin carries `Access-Control-Allow-Origin` and reaches the caller as a real 404 rather than an opaque CORS failure. Denials raised before the request reaches S3 (SigV4 or IAM, under `--verify-sigv4` / `--iam`) are not CORS-decorated, so under auth enforcement a rejected browser request still surfaces as an opaque failure. `Vary: Origin, Access-Control-Request-Headers, Access-Control-Request-Method` keeps a cache from reusing one origin's response for a different origin. **Every** preflight response carries it, since a preflight's outcome always turns on `Origin`: `200` when the rules allow it, `403 AccessForbidden` when they do not, and `400 BadRequest` ("Insufficient information. Origin request header needed.") when the request omits `Origin` altogether. On the actual-request path it goes on every response evaluated against a bucket's CORS config, successes and errors alike, while a request without `Origin` is not CORS-evaluated and is served normally with neither header, as on S3. A concrete allowed origin also gets `Access-Control-Allow-Credentials: true`, so `credentials: 'include'` works. Note that an actual request declares no headers, so it can match an earlier, broader rule than the preflight did: if a `*` rule precedes the concrete one, the actual response carries `Access-Control-Allow-Origin: *` and no credentials, and the browser blocks a credentialed request that just passed preflight. That is S3's behavior too — order the concrete rule first. `AllowedOrigin` and `AllowedHeader` each take one `*` anywhere in the value (`https://*.example.com`, `x-amz-*`). `ExposeHeader` takes no wildcard at all, as on S3. A rule missing `AllowedMethods` or `AllowedOrigins`, carrying more than one wildcard in a value, or setting a non-numeric `MaxAgeSeconds`, is rejected at write time rather than silently matching nothing. CloudFormation's `CorsConfiguration` goes through the same validation, so a template cannot deploy a bucket whose preflights all fail.
-- **Object Lock** — legal hold, retention modes
+- **Bucket and object ACLs** — a canned `x-amz-acl`, the `x-amz-grant-*` headers and an `AccessControlPolicy` body are mutually exclusive, as on S3: naming an ACL two ways is `InvalidRequest` rather than one of them silently winning. Canned values are checked against the set S3 accepts for the target (bucket ACLs take `private`, `public-read`, `public-read-write`, `authenticated-read`, `aws-exec-read` and `log-delivery-write`; `bucket-owner-read` and `bucket-owner-full-control` are object-only), and a grant naming no resolvable grantee is rejected instead of being stored as an entry that matches nobody. `emailAddress=` grantees are kept as `AmazonCustomerByEmail` so they round-trip, but they are not resolved to a canonical user and so convey no access. A bucket whose `ObjectOwnership` is `BucketOwnerEnforced` rejects every ACL-setting call, `CreateMultipartUpload` included.
+- **Object Lock** — legal hold, retention modes. A bucket created with `x-amz-bucket-object-lock-enabled` keeps its lock configuration (and its canned ACL and object-ownership rule) across a restart in persistent mode, so retention stays enforced.
- **Website hosting** — index/error documents, redirect rules
- **Access Points** — full control plane (`CreateAccessPoint`, `GetAccessPoint`, `DeleteAccessPoint`, `ListAccessPoints`) via the `s3-control` host prefix; data plane traffic to `s3-accesspoint.` resolves the alias to its underlying bucket so standard S3 operations work unchanged.
- **S3 Select** — real `SelectObjectContent` over CSV/JSON via EventStream framing (`Records`, `Stats`, `End` messages).