fix(s3): honor Tags in CreateBucketConfiguration

crates/fakecloud-core/src/dispatch.rs

@@ -486,19 +486,10 @@ pub async fn dispatch(
     // ran against the wire path, so this rewrite is signature-safe.
     let wire_path = parts.uri.path();
     let path = if detected.service == "s3" {
-        if let Some(bucket) = host_info.as_ref().and_then(|h| h.bucket.as_deref()) {
-            let prefix_with_slash = format!("/{bucket}/");
-            let is_bucket_root = wire_path.trim_end_matches('/') == format!("/{bucket}");
-            if wire_path.starts_with(&prefix_with_slash) || is_bucket_root {
-                wire_path.to_string()
-            } else if wire_path == "/" || wire_path.is_empty() {
-                format!("/{bucket}")
-            } else {
-                format!("/{bucket}{wire_path}")
-            }
-        } else {
-            wire_path.to_string()
-        }
+        s3_routing_path(
+            wire_path,
+            host_info.as_ref().and_then(|h| h.bucket.as_deref()),
+        )
     } else {
         wire_path.to_string()
     };
@@ -1086,6 +1077,39 @@ impl DispatchConfig {
     }
 }
 
+/// The path an S3 request is routed on: for virtual-hosted-style the bucket
+/// comes from the Host header and is prepended, unless the client already put
+/// it in the path (`PUT /<bucket>` / `PUT /<bucket>/key` against a
+/// virtual-hosted host). Path-style requests pass through unchanged.
+///
+/// Shared with [`streaming_route`] on purpose. The streaming gate has to agree
+/// with routing about where the bucket ends and the key begins; when the two
+/// derived it separately they drifted, and a request the router read as a
+/// bucket-level operation was dispatched unbuffered, so the handler and IAM
+/// enforcement saw an empty body.
+///
+/// Known limitation: under virtual-hosted addressing the whole path is the
+/// key on real S3, so an object key that begins with `<bucket>/` (or a key
+/// that IS the bucket name) is not representable here — the passthrough reads
+/// it as a client that put the bucket in the path, and the prefix is dropped.
+/// Nothing in a single request distinguishes the two, so this resolves the
+/// ambiguity in favor of the mixed-addressing client. Keys shaped like their
+/// own bucket name are the cost.
+fn s3_routing_path(wire_path: &str, host_bucket: Option<&str>) -> String {
+    let Some(bucket) = host_bucket else {
+        return wire_path.to_string();
+    };
+    let prefix_with_slash = format!("/{bucket}/");
+    let is_bucket_root = wire_path.trim_end_matches('/') == format!("/{bucket}");
+    if wire_path.starts_with(&prefix_with_slash) || is_bucket_root {
+        wire_path.to_string()
+    } else if wire_path == "/" || wire_path.is_empty() {
+        format!("/{bucket}")
+    } else {
+        format!("/{bucket}{wire_path}")
+    }
+}
+
 /// Extract the 12-digit account ID segment from an AWS ARN.
 ///
 /// ARNs follow `arn:<partition>:<service>:<region>:<account>:<resource>`.
@@ -1124,16 +1148,23 @@ fn streaming_route(
     // presigned URL (X-Amz-Credential .../s3/...) OR a SigV2 presigned
     // URL (AWSAccessKeyId + Signature + Expires query parameters).
     if method == http::Method::PUT {
-        let after = path.trim_start_matches('/');
-        // Path-style PutObject is `PUT /<bucket>/<key>` (path contains a
-        // slash); virtual-hosted-style is `PUT /<key>` with the bucket
-        // in the Host header. For virtual-hosted, accept any non-empty
-        // path so the key flows through the streaming dispatch — the
-        // Host parser already routed this request to S3.
-        let virtual_hosted_s3 = protocol::parse_routing_host_from_headers(headers)
-            .filter(|h| h.service == "s3" && h.bucket.is_some())
-            .is_some();
-        if after.is_empty() || (!virtual_hosted_s3 && !after.contains('/')) {
+        // An object upload needs a bucket AND a key. Resolve the request the
+        // way routing will (bucket from the Host header for virtual-hosted,
+        // straight from the path otherwise), then split it the way the request
+        // builder builds `path_segments` -- on '/', dropping empty segments.
+        // Anything that leaves no key is a bucket-level operation:
+        // `PUT /<bucket>`, `PUT /<bucket>/` (the form the AWS SDKs actually
+        // send for CreateBucket), `PUT /<bucket>//`, and the virtual-hosted
+        // equivalents. Streaming those left the body unbuffered, so everything
+        // that runs before the handler -- IAM enforcement above all -- saw an
+        // empty body and could not read `CreateBucketConfiguration` (no
+        // `aws:RequestTag` from a create-time tag set, for one).
+        let host_bucket = protocol::parse_routing_host_from_headers(headers)
+            .filter(|h| h.service == "s3")
+            .and_then(|h| h.bucket);
+        let routed = s3_routing_path(path, host_bucket.as_deref());
+        let has_key = routed.split('/').filter(|seg| !seg.is_empty()).count() >= 2;
+        if !has_key {
             return None;
         }
         let header_s3 = headers
@@ -1997,6 +2028,53 @@ mod tests {
         );
     }
 
+    #[test]
+    fn streaming_route_path_style_create_bucket_with_trailing_slash_skipped() {
+        // The AWS SDKs send CreateBucket as `PUT /<bucket>/`. The trailing
+        // slash must not make it look like an object upload: the body carries
+        // `CreateBucketConfiguration` (location constraint, tag set) and has
+        // to be buffered before IAM enforcement reads it.
+        let headers = s3_sigv4_headers();
+        assert_eq!(
+            streaming_route(&http::Method::PUT, "/my-bucket/", &headers, &HashMap::new(),),
+            None,
+        );
+    }
+
+    #[test]
+    fn streaming_route_path_style_doubled_slash_skipped() {
+        // Routing filters empty path segments, so `PUT /<bucket>//` is a
+        // bucket-level operation, not an object upload with a "/" key. The
+        // streaming gate has to agree, or the body goes unbuffered for a
+        // request the handler reads as CreateBucket / PutBucketTagging.
+        let headers = s3_sigv4_headers();
+        assert_eq!(
+            streaming_route(
+                &http::Method::PUT,
+                "/my-bucket//",
+                &headers,
+                &HashMap::new()
+            ),
+            None,
+        );
+    }
+
+    #[test]
+    fn streaming_route_path_style_key_with_trailing_slash_streams() {
+        // A real object key that ends in '/' (a directory marker) still
+        // streams -- the gate rejects an absent key, not a trailing slash.
+        let headers = s3_sigv4_headers();
+        assert_eq!(
+            streaming_route(
+                &http::Method::PUT,
+                "/my-bucket/folder/",
+                &headers,
+                &HashMap::new(),
+            ),
+            Some(("s3", "")),
+        );
+    }
+
     #[test]
     fn streaming_route_virtual_hosted_s3_put_object() {
         let mut headers = s3_sigv4_headers();
@@ -2016,6 +2094,39 @@ mod tests {
         );
     }
 
+    #[test]
+    fn streaming_route_virtual_hosted_bucket_in_path_skipped() {
+        // `Host: my-bucket.s3...` with the bucket ALSO in the path is the
+        // router's bucket-root shape (dispatch collapses it to CreateBucket),
+        // so it must not stream -- with or without the trailing slash.
+        let mut headers = s3_sigv4_headers();
+        headers.insert(
+            "host",
+            "my-bucket.s3.us-east-1.amazonaws.com".parse().unwrap(),
+        );
+        assert_eq!(
+            streaming_route(&http::Method::PUT, "/my-bucket", &headers, &HashMap::new()),
+            None,
+        );
+        assert_eq!(
+            streaming_route(&http::Method::PUT, "/my-bucket/", &headers, &HashMap::new()),
+            None,
+        );
+        // ...but a path the router reads as a key under that bucket still
+        // streams. (Per the `s3_routing_path` limitation, a virtual-hosted key
+        // that genuinely begins with `my-bucket/` is indistinguishable from
+        // this and is routed the same way.)
+        assert_eq!(
+            streaming_route(
+                &http::Method::PUT,
+                "/my-bucket/key.txt",
+                &headers,
+                &HashMap::new(),
+            ),
+            Some(("s3", "")),
+        );
+    }
+
     #[test]
     fn streaming_route_virtual_hosted_s3_root_skipped() {
         // `PUT /` against a virtual-hosted Host = CreateBucket, which

crates/fakecloud-e2e/tests/iam_enforcement.rs

@@ -411,6 +411,146 @@ async fn sns_publish_allowed_on_specific_topic() {
 // S3 tests
 // ======================================================================
 
+/// Real S3 requires `s3:TagResource` on top of `s3:CreateBucket` to create a
+/// bucket carrying `CreateBucketConfiguration.Tags` (issue #2553). A grant of
+/// `s3:CreateBucket` alone still creates untagged buckets, but a tagged create
+/// is denied.
+#[tokio::test]
+async fn s3_create_bucket_with_tags_needs_tag_resource() {
+    let server = start_strict().await;
+    let (akid, secret) = bootstrap_user(&server, "s3taguser").await;
+    attach_inline_policy(
+        &server,
+        "s3taguser",
+        "create-only",
+        r#"{"Version":"2012-10-17","Statement":[
+            {"Effect":"Allow","Action":"s3:CreateBucket","Resource":"*"}
+        ]}"#,
+    )
+    .await;
+
+    let cfg = sdk_config_with(&server, &akid, &secret).await;
+    let s3 = aws_sdk_s3::Client::new(&cfg);
+
+    // Untagged create is allowed by s3:CreateBucket alone.
+    s3.create_bucket()
+        .bucket("tagperm-untagged")
+        .send()
+        .await
+        .unwrap();
+
+    // The same grant must not authorize a tagged create.
+    let err = s3
+        .create_bucket()
+        .bucket("tagperm-denied")
+        .create_bucket_configuration(
+            aws_sdk_s3::types::CreateBucketConfiguration::builder()
+                .tags(
+                    aws_sdk_s3::types::Tag::builder()
+                        .key("team")
+                        .value("a")
+                        .build()
+                        .unwrap(),
+                )
+                .build(),
+        )
+        .send()
+        .await
+        .unwrap_err();
+    let msg = format!("{err:?}");
+    assert!(
+        msg.contains("AccessDenied"),
+        "expected AccessDenied for tagged create, got {msg}"
+    );
+
+    // Adding s3:TagResource unblocks it, and the bucket comes out tagged.
+    attach_inline_policy(
+        &server,
+        "s3taguser",
+        "create-and-tag",
+        r#"{"Version":"2012-10-17","Statement":[
+            {"Effect":"Allow","Action":["s3:CreateBucket","s3:TagResource","s3:GetBucketTagging"],"Resource":"*"}
+        ]}"#,
+    )
+    .await;
+    s3.create_bucket()
+        .bucket("tagperm-allowed")
+        .create_bucket_configuration(
+            aws_sdk_s3::types::CreateBucketConfiguration::builder()
+                .tags(
+                    aws_sdk_s3::types::Tag::builder()
+                        .key("team")
+                        .value("a")
+                        .build()
+                        .unwrap(),
+                )
+                .build(),
+        )
+        .send()
+        .await
+        .unwrap();
+    let tags = s3
+        .get_bucket_tagging()
+        .bucket("tagperm-allowed")
+        .send()
+        .await
+        .unwrap();
+    assert!(tags
+        .tag_set()
+        .iter()
+        .any(|t| t.key() == "team" && t.value() == "a"));
+}
+
+/// The create-time tag set feeds `aws:RequestTag/*`, so a policy conditioned
+/// on the tag value actually decides the create.
+#[tokio::test]
+async fn s3_create_bucket_tags_drive_request_tag_condition() {
+    let server = start_strict().await;
+    let (akid, secret) = bootstrap_user(&server, "s3reqtaguser").await;
+    attach_inline_policy(
+        &server,
+        "s3reqtaguser",
+        "team-a-only",
+        r#"{"Version":"2012-10-17","Statement":[
+            {"Effect":"Allow","Action":["s3:CreateBucket","s3:TagResource"],"Resource":"*",
+             "Condition":{"StringEquals":{"aws:RequestTag/team":"a"}}}
+        ]}"#,
+    )
+    .await;
+
+    let cfg = sdk_config_with(&server, &akid, &secret).await;
+    let s3 = aws_sdk_s3::Client::new(&cfg);
+    let create_with_team = |bucket: &'static str, team: &'static str| {
+        let s3 = s3.clone();
+        async move {
+            s3.create_bucket()
+                .bucket(bucket)
+                .create_bucket_configuration(
+                    aws_sdk_s3::types::CreateBucketConfiguration::builder()
+                        .tags(
+                            aws_sdk_s3::types::Tag::builder()
+                                .key("team")
+                                .value(team)
+                                .build()
+                                .unwrap(),
+                        )
+                        .build(),
+                )
+                .send()
+                .await
+        }
+    };
+
+    create_with_team("reqtag-allowed", "a").await.unwrap();
+
+    let err = create_with_team("reqtag-denied", "b").await.unwrap_err();
+    let msg = format!("{err:?}");
+    assert!(
+        msg.contains("AccessDenied"),
+        "expected AccessDenied for the non-matching tag value, got {msg}"
+    );
+}
+
 #[tokio::test]
 async fn s3_get_object_resource_scoped() {
     let server = start_strict().await;