From 0d3d12be3c7b92a7dd576063f1418c7b236740b5 Mon Sep 17 00:00:00 2001 From: Rahul Goel Date: Mon, 3 Aug 2026 18:51:37 -0400 Subject: [PATCH 1/3] GH-49949: [C++][S3] Don't call HeadBucket when creating directories --- cpp/src/arrow/filesystem/s3fs.cc | 12 ++++--- cpp/src/arrow/filesystem/s3fs_test.cc | 51 +++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/cpp/src/arrow/filesystem/s3fs.cc b/cpp/src/arrow/filesystem/s3fs.cc index 1c6763a4aee9..f65b1d8cf5b5 100644 --- a/cpp/src/arrow/filesystem/s3fs.cc +++ b/cpp/src/arrow/filesystem/s3fs.cc @@ -3238,10 +3238,14 @@ Status S3FileSystem::CreateDir(const std::string& s, bool recursive) { FileInfo file_info; if (recursive) { - // Ensure bucket exists - ARROW_ASSIGN_OR_RAISE(bool bucket_exists, impl_->BucketExists(path.bucket)); - if (!bucket_exists) { - RETURN_NOT_OK(impl_->CreateBucket(path.bucket)); + // Only probe the bucket if we could create it: HeadBucket is denied for + // credentials scoped to a prefix inside the bucket (GH-49949). A missing + // bucket then surfaces as an error from the directory write that follows. + if (options().allow_bucket_creation) { + ARROW_ASSIGN_OR_RAISE(bool bucket_exists, impl_->BucketExists(path.bucket)); + if (!bucket_exists) { + RETURN_NOT_OK(impl_->CreateBucket(path.bucket)); + } } auto key_i = path.key_parts.begin(); diff --git a/cpp/src/arrow/filesystem/s3fs_test.cc b/cpp/src/arrow/filesystem/s3fs_test.cc index 114701b6d446..ac426a93f8de 100644 --- a/cpp/src/arrow/filesystem/s3fs_test.cc +++ b/cpp/src/arrow/filesystem/s3fs_test.cc @@ -48,6 +48,7 @@ #include #include #include +#include #include #include @@ -1240,6 +1241,52 @@ TEST_F(TestS3FS, CreateDir) { FileType::Directory); } +TEST_F(TestS3FS, CreateDirPrefixScopedCredentials) { + // Grant anonymous access to the "allowed/" prefix only. HeadBucket needs a + // bucket-wide grant, so it is denied for these credentials. + const std::string policy = R"({ + "Version": "2012-10-17", + "Statement": [{ + "Effect": "Allow", + "Principal": {"AWS": ["*"]}, + "Action": ["s3:GetObject", "s3:PutObject"], + "Resource": ["arn:aws:s3:::bucket/allowed", "arn:aws:s3:::bucket/allowed/*"] + }, { + "Effect": "Allow", + "Principal": {"AWS": ["*"]}, + "Action": ["s3:ListBucket"], + "Resource": ["arn:aws:s3:::bucket"], + "Condition": {"StringLike": {"s3:prefix": ["allowed/*"]}} + }] + })"; + { + Aws::S3::Model::PutBucketPolicyRequest req; + req.SetBucket(ToAwsString("bucket")); + req.SetBody(std::make_shared(policy)); + ASSERT_OK(OutcomeToStatus("PutBucketPolicy", client_->PutBucketPolicy(req))); + } + + S3Options options; + options.ConfigureAnonymousCredentials(); + options.scheme = minio_->scheme(); + options.endpoint_override = minio_->connect_string(); + options.retry_strategy = std::make_shared(); + if (enable_tls_) { + options.tls_ca_file_path = minio_->ca_file_path(); + } + ASSERT_OK_AND_ASSIGN(auto fs, S3FileSystem::Make(options)); + + // The bucket itself is not readable... + ASSERT_RAISES(IOError, fs->GetFileInfo("bucket")); + // ...but a directory below the granted prefix can still be created. + ASSERT_OK(fs->CreateDir("bucket/allowed/newdir", /*recursive=*/true)); + AssertObjectContents(client_.get(), "bucket", "allowed/", ""); + AssertObjectContents(client_.get(), "bucket", "allowed/newdir/", ""); + + // Outside of the granted prefix the write itself is denied + ASSERT_RAISES(IOError, fs->CreateDir("bucket/denied/newdir", /*recursive=*/true)); +} + TEST_F(TestS3FS, DeleteFile) { // Bucket ASSERT_RAISES(IOError, fs_->DeleteFile("bucket")); @@ -1722,6 +1769,10 @@ TEST_F(TestS3FS, NoCreateDeleteBucket) { ASSERT_THAT(maybe_create_dir.message(), ::testing::HasSubstr("Bucket 'test-no-create' not found")); + // Creating a directory inside a nonexistent bucket fails when writing the + // directory entry, since the bucket is not probed beforehand + ASSERT_RAISES(IOError, fs_->CreateDir("test-no-create/newdir", /*recursive=*/true)); + auto maybe_delete_dir = fs_->DeleteDir("test-no-delete"); ASSERT_RAISES(IOError, maybe_delete_dir); ASSERT_THAT(maybe_delete_dir.message(), From cd86f570ae19894f507e1c70e3a4ab925318b6cb Mon Sep 17 00:00:00 2001 From: Rahul Goel Date: Mon, 3 Aug 2026 19:46:05 -0400 Subject: [PATCH 2/3] Guard PutBucketPolicyRequest include and test on MinGW --- cpp/src/arrow/filesystem/s3fs_test.cc | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/filesystem/s3fs_test.cc b/cpp/src/arrow/filesystem/s3fs_test.cc index ac426a93f8de..9bafd88f03a5 100644 --- a/cpp/src/arrow/filesystem/s3fs_test.cc +++ b/cpp/src/arrow/filesystem/s3fs_test.cc @@ -48,8 +48,13 @@ #include #include #include -#include #include +// PutBucketPolicyRequest.h marks an inline method definition with AWS_S3_API, +// which expands to __declspec(dllimport) under USE_IMPORT_EXPORT. GCC rejects +// that outright, while MSVC only warns, so the header is unusable on MinGW. +#ifndef __MINGW32__ +# include +#endif #include #include "arrow/filesystem/filesystem.h" @@ -1241,6 +1246,8 @@ TEST_F(TestS3FS, CreateDir) { FileType::Directory); } +// Needs PutBucketPolicyRequest, which does not compile on MinGW (see above). +#ifndef __MINGW32__ TEST_F(TestS3FS, CreateDirPrefixScopedCredentials) { // Grant anonymous access to the "allowed/" prefix only. HeadBucket needs a // bucket-wide grant, so it is denied for these credentials. @@ -1286,6 +1293,7 @@ TEST_F(TestS3FS, CreateDirPrefixScopedCredentials) { // Outside of the granted prefix the write itself is denied ASSERT_RAISES(IOError, fs->CreateDir("bucket/denied/newdir", /*recursive=*/true)); } +#endif TEST_F(TestS3FS, DeleteFile) { // Bucket From 4d3838050556e328ffb1fe9363f6b48e5ea1a1a6 Mon Sep 17 00:00:00 2001 From: Rahul Goel Date: Mon, 3 Aug 2026 19:47:51 -0400 Subject: [PATCH 3/3] make mingw happy --- cpp/src/arrow/filesystem/s3fs_test.cc | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/cpp/src/arrow/filesystem/s3fs_test.cc b/cpp/src/arrow/filesystem/s3fs_test.cc index 9bafd88f03a5..6e621f13251f 100644 --- a/cpp/src/arrow/filesystem/s3fs_test.cc +++ b/cpp/src/arrow/filesystem/s3fs_test.cc @@ -49,9 +49,8 @@ #include #include #include -// PutBucketPolicyRequest.h marks an inline method definition with AWS_S3_API, -// which expands to __declspec(dllimport) under USE_IMPORT_EXPORT. GCC rejects -// that outright, while MSVC only warns, so the header is unusable on MinGW. +// AWS_S3_API on an inline definition in this header expands to +// __declspec(dllimport), which GCC rejects and MSVC merely warns about. #ifndef __MINGW32__ # include #endif @@ -1246,7 +1245,7 @@ TEST_F(TestS3FS, CreateDir) { FileType::Directory); } -// Needs PutBucketPolicyRequest, which does not compile on MinGW (see above). +// PutBucketPolicyRequest.h is not included on MinGW. #ifndef __MINGW32__ TEST_F(TestS3FS, CreateDirPrefixScopedCredentials) { // Grant anonymous access to the "allowed/" prefix only. HeadBucket needs a