From efbed39e255465cdc4f3323a1bea4f0c1455b03b Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Thu, 2 Apr 2026 11:51:54 -0700 Subject: [PATCH] S3: map canned ACL to file permissions and add configurable default file mode (#8886) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * S3: map canned ACL to file permissions and add configurable default file mode S3 uploads were hardcoded to 0660 regardless of ACL headers. Now the X-Amz-Acl header maps to Unix file permissions per-object: - public-read, authenticated-read, bucket-owner-read → 0644 - public-read-write → 0666 - private, bucket-owner-full-control → 0660 Also adds -defaultFileMode / -s3.defaultFileMode flag to set a server-wide default when no ACL header is present. Closes #8874 * Address review feedback for S3 file mode feature - Extract hardcoded 0660 to defaultFileMode constant - Change parseDefaultFileMode to return error instead of calling Fatalf - Add -s3.defaultFileMode flag to filer.go and mini.go (was missing) - Add doc comment to S3Options about updating all four flag sites - Add TestResolveFileMode with 10 test cases covering ACL mapping, server default, and priority ordering --- weed/command/filer.go | 1 + weed/command/mini.go | 1 + weed/command/s3.go | 23 ++++++++++++ weed/command/server.go | 1 + weed/s3api/s3api_object_handlers_put.go | 25 ++++++++++++- weed/s3api/s3api_object_handlers_put_test.go | 38 ++++++++++++++++++++ weed/s3api/s3api_server.go | 1 + 7 files changed, 89 insertions(+), 1 deletion(-) diff --git a/weed/command/filer.go b/weed/command/filer.go index d6a990ebd..edfd8afef 100644 --- a/weed/command/filer.go +++ b/weed/command/filer.go @@ -146,6 +146,7 @@ func init() { filerS3Options.iamReadOnly = cmdFiler.Flag.Bool("s3.iam.readOnly", true, "disable IAM write operations on this server") filerS3Options.portIceberg = cmdFiler.Flag.Int("s3.port.iceberg", 8181, "Iceberg REST Catalog server listen port (0 to disable)") filerS3Options.externalUrl = cmdFiler.Flag.String("s3.externalUrl", "", "the external URL clients use to connect (e.g. https://api.example.com:9000). Used for S3 signature verification behind a reverse proxy. Falls back to S3_EXTERNAL_URL env var.") + filerS3Options.defaultFileMode = cmdFiler.Flag.String("s3.defaultFileMode", "", "default file mode for S3 uploaded objects, e.g. 0660, 0644, 0666") // start webdav on filer filerStartWebDav = cmdFiler.Flag.Bool("webdav", false, "whether to start webdav gateway") diff --git a/weed/command/mini.go b/weed/command/mini.go index b5a3bd367..38257bdff 100644 --- a/weed/command/mini.go +++ b/weed/command/mini.go @@ -250,6 +250,7 @@ func initMiniS3Flags() { miniS3Options.auditLogConfig = cmdMini.Flag.String("s3.auditLogConfig", "", "path to the audit log config file") miniS3Options.allowDeleteBucketNotEmpty = miniS3AllowDeleteBucketNotEmpty miniS3Options.externalUrl = cmdMini.Flag.String("s3.externalUrl", "", "the external URL clients use to connect (e.g. https://api.example.com:9000). Used for S3 signature verification behind a reverse proxy. Falls back to S3_EXTERNAL_URL env var.") + miniS3Options.defaultFileMode = cmdMini.Flag.String("s3.defaultFileMode", "", "default file mode for S3 uploaded objects, e.g. 0660, 0644, 0666") // In mini mode, S3 uses the shared debug server started at line 681, not its own separate debug server miniS3Options.debug = new(bool) // explicitly false miniS3Options.debugPort = cmdMini.Flag.Int("s3.debug.port", 6060, "http port for debugging (unused in mini mode)") diff --git a/weed/command/s3.go b/weed/command/s3.go index 081e84f13..dc2191bcf 100644 --- a/weed/command/s3.go +++ b/weed/command/s3.go @@ -10,6 +10,7 @@ import ( "net/http" "os" "runtime" + "strconv" "strings" "time" @@ -36,6 +37,9 @@ var ( s3StandaloneOptions S3Options ) +// S3Options holds CLI flags for the S3 gateway. +// Flags are registered in multiple commands: s3.go (standalone), server.go, filer.go, and mini.go. +// When adding a new field, update all four flag registration sites. type S3Options struct { filer *string bindIp *string @@ -68,6 +72,7 @@ type S3Options struct { debugPort *int cipher *bool externalUrl *string + defaultFileMode *string } func init() { @@ -103,6 +108,7 @@ func init() { s3StandaloneOptions.debugPort = cmdS3.Flag.Int("debug.port", 6060, "http port for debugging") s3StandaloneOptions.cipher = cmdS3.Flag.Bool("encryptVolumeData", false, "encrypt data on volume servers") s3StandaloneOptions.externalUrl = cmdS3.Flag.String("externalUrl", "", "the external URL clients use to connect (e.g. https://api.example.com:9000). Used for S3 signature verification behind a reverse proxy. Falls back to S3_EXTERNAL_URL env var.") + s3StandaloneOptions.defaultFileMode = cmdS3.Flag.String("defaultFileMode", "", "default file mode for S3 uploaded objects, e.g. 0660, 0644, 0666") } var cmdS3 = &Command{ @@ -232,6 +238,17 @@ func (s3opt *S3Options) resolveExternalUrl() string { return os.Getenv("S3_EXTERNAL_URL") } +func (s3opt *S3Options) parseDefaultFileMode() (uint32, error) { + if s3opt.defaultFileMode == nil || *s3opt.defaultFileMode == "" { + return 0, nil + } + mode, err := strconv.ParseUint(*s3opt.defaultFileMode, 8, 32) + if err != nil { + return 0, fmt.Errorf("invalid defaultFileMode %q: %v", *s3opt.defaultFileMode, err) + } + return uint32(mode), nil +} + func (s3opt *S3Options) startS3Server() bool { filerAddresses := pb.ServerAddresses(*s3opt.filer).ToAddresses() @@ -298,6 +315,11 @@ func (s3opt *S3Options) startS3Server() bool { *s3opt.bindIp = "0.0.0.0" } + defaultFileMode, fileModeErr := s3opt.parseDefaultFileMode() + if fileModeErr != nil { + glog.Fatalf("S3 API Server startup error: %v", fileModeErr) + } + s3ApiServer, s3ApiServer_err = s3api.NewS3ApiServer(router, &s3api.S3ApiServerOption{ Filers: filerAddresses, Masters: masterAddresses, @@ -320,6 +342,7 @@ func (s3opt *S3Options) startS3Server() bool { BindIp: *s3opt.bindIp, GrpcPort: *s3opt.portGrpc, ExternalUrl: s3opt.resolveExternalUrl(), + DefaultFileMode: defaultFileMode, }) if s3ApiServer_err != nil { glog.Fatalf("S3 API Server startup error: %v", s3ApiServer_err) diff --git a/weed/command/server.go b/weed/command/server.go index a2759845d..deb1c5e71 100644 --- a/weed/command/server.go +++ b/weed/command/server.go @@ -180,6 +180,7 @@ func init() { s3Options.iamReadOnly = cmdServer.Flag.Bool("s3.iam.readOnly", true, "disable IAM write operations on this server") s3Options.cipher = cmdServer.Flag.Bool("s3.encryptVolumeData", false, "encrypt data on volume servers for S3 uploads") s3Options.externalUrl = cmdServer.Flag.String("s3.externalUrl", "", "the external URL clients use to connect (e.g. https://api.example.com:9000). Used for S3 signature verification behind a reverse proxy. Falls back to S3_EXTERNAL_URL env var.") + s3Options.defaultFileMode = cmdServer.Flag.String("s3.defaultFileMode", "", "default file mode for S3 uploaded objects, e.g. 0660, 0644, 0666") sftpOptions.port = cmdServer.Flag.Int("sftp.port", 2022, "SFTP server listen port") sftpOptions.sshPrivateKey = cmdServer.Flag.String("sftp.sshPrivateKey", "", "path to the SSH private key file for host authentication") diff --git a/weed/s3api/s3api_object_handlers_put.go b/weed/s3api/s3api_object_handlers_put.go index 5419a4852..805d63133 100644 --- a/weed/s3api/s3api_object_handlers_put.go +++ b/weed/s3api/s3api_object_handlers_put.go @@ -603,13 +603,14 @@ func (s3a *S3ApiServer) putToFiler(r *http.Request, filePath string, dataReader } // Create entry + fileMode := s3a.resolveFileMode(r) entry := &filer_pb.Entry{ Name: path.Base(filePath), IsDirectory: false, Attributes: &filer_pb.FuseAttributes{ Crtime: now.Unix(), Mtime: now.Unix(), - FileMode: 0660, + FileMode: fileMode, Uid: 0, Gid: 0, Mime: mimeType, @@ -815,6 +816,28 @@ func (s3a *S3ApiServer) putToFiler(r *http.Request, filePath string, dataReader return etag, s3err.ErrNone, responseMetadata } +const defaultFileMode = uint32(0660) + +// resolveFileMode determines the file permission mode for an S3 upload. +// Priority: per-object X-Amz-Acl header > server default > defaultFileMode. +func (s3a *S3ApiServer) resolveFileMode(r *http.Request) uint32 { + if cannedAcl := r.Header.Get(s3_constants.AmzCannedAcl); cannedAcl != "" { + switch cannedAcl { + case s3_constants.CannedAclPublicRead, s3_constants.CannedAclAuthenticatedRead, + s3_constants.CannedAclBucketOwnerRead: + return 0644 + case s3_constants.CannedAclPublicReadWrite: + return 0666 + case s3_constants.CannedAclPrivate, s3_constants.CannedAclBucketOwnerFullControl: + return defaultFileMode + } + } + if s3a.option.DefaultFileMode != 0 { + return s3a.option.DefaultFileMode + } + return defaultFileMode +} + func setEtag(w http.ResponseWriter, etag string) { if etag != "" { if strings.HasPrefix(etag, "\"") { diff --git a/weed/s3api/s3api_object_handlers_put_test.go b/weed/s3api/s3api_object_handlers_put_test.go index eaad6ab6d..a5646bff7 100644 --- a/weed/s3api/s3api_object_handlers_put_test.go +++ b/weed/s3api/s3api_object_handlers_put_test.go @@ -301,3 +301,41 @@ func TestWithObjectWriteLockSerializesConcurrentPreconditions(t *testing.T) { t.Fatalf("expected %d precondition failures, got %d", workers-1, preconditionFailedCount) } } + +func TestResolveFileMode(t *testing.T) { + tests := []struct { + name string + acl string + defaultFileMode uint32 + expected uint32 + }{ + {"no acl, no default", "", 0, 0660}, + {"no acl, with default", "", 0644, 0644}, + {"private", s3_constants.CannedAclPrivate, 0, 0660}, + {"private overrides default", s3_constants.CannedAclPrivate, 0644, 0660}, + {"public-read", s3_constants.CannedAclPublicRead, 0, 0644}, + {"public-read overrides default", s3_constants.CannedAclPublicRead, 0666, 0644}, + {"public-read-write", s3_constants.CannedAclPublicReadWrite, 0, 0666}, + {"authenticated-read", s3_constants.CannedAclAuthenticatedRead, 0, 0644}, + {"bucket-owner-read", s3_constants.CannedAclBucketOwnerRead, 0, 0644}, + {"bucket-owner-full-control", s3_constants.CannedAclBucketOwnerFullControl, 0, 0660}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s3a := &S3ApiServer{ + option: &S3ApiServerOption{ + DefaultFileMode: tt.defaultFileMode, + }, + } + req := httptest.NewRequest(http.MethodPut, "/bucket/object", nil) + if tt.acl != "" { + req.Header.Set(s3_constants.AmzCannedAcl, tt.acl) + } + got := s3a.resolveFileMode(req) + if got != tt.expected { + t.Errorf("resolveFileMode() = %04o, want %04o", got, tt.expected) + } + }) + } +} diff --git a/weed/s3api/s3api_server.go b/weed/s3api/s3api_server.go index 08e376aec..35f3684d3 100644 --- a/weed/s3api/s3api_server.go +++ b/weed/s3api/s3api_server.go @@ -60,6 +60,7 @@ type S3ApiServerOption struct { BindIp string GrpcPort int ExternalUrl string // external URL clients use, for signature verification behind a reverse proxy + DefaultFileMode uint32 // default file permission mode for S3 uploads (e.g. 0660, 0644) } type S3ApiServer struct {