From d00a2a87074f5cc2dd13f54fcf861d2dda318b35 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Thu, 30 Oct 2025 11:00:31 -0700 Subject: [PATCH 1/4] Fix S3 bucket policy ARN validation to accept AWS ARNs and simplified formats (#7409) * Fix S3 bucket policy ARN validation to accept AWS ARNs and simplified formats Fixes #7252 The bucket policy validation was rejecting valid AWS-style ARNs and simplified resource formats, causing validation failures with the error 'resource X does not match bucket X' even when they were identical strings. Changes: - Updated validateResourceForBucket() to accept three formats: 1. AWS-style ARNs: arn:aws:s3:::bucket-name[/*|/path] 2. SeaweedFS ARNs: arn:seaweed:s3:::bucket-name[/*|/path] 3. Simplified formats: bucket-name[/*|/path] - Added comprehensive test coverage for all three formats - Added specific test cases from issue #7252 to prevent regression This ensures compatibility with standard AWS S3 bucket policies while maintaining support for SeaweedFS-specific ARN format. * Refactor validateResourceForBucket to reduce code duplication Simplified the validation logic by stripping ARN prefixes first, then performing validation on the remaining resource path. This reduces code duplication and improves maintainability while maintaining identical functionality. Addresses review feedback from Gemini Code Assist. * Use strings.CutPrefix for cleaner ARN prefix stripping Replace strings.HasPrefix checks with strings.CutPrefix for more idiomatic Go code. This function is available in Go 1.20+ and provides cleaner conditional logic with the combined check and extraction. Addresses review feedback from Gemini Code Assist. --- weed/s3api/s3_bucket_policy_simple_test.go | 177 ++++++++++++++++++++- weed/s3api/s3api_bucket_policy_handlers.go | 44 +++-- 2 files changed, 204 insertions(+), 17 deletions(-) diff --git a/weed/s3api/s3_bucket_policy_simple_test.go b/weed/s3api/s3_bucket_policy_simple_test.go index 025b44900..5188779ff 100644 --- a/weed/s3api/s3_bucket_policy_simple_test.go +++ b/weed/s3api/s3_bucket_policy_simple_test.go @@ -143,42 +143,106 @@ func TestBucketResourceValidation(t *testing.T) { bucket string valid bool }{ + // SeaweedFS ARN format { - name: "Exact bucket ARN", + name: "Exact bucket ARN (SeaweedFS)", resource: "arn:seaweed:s3:::test-bucket", bucket: "test-bucket", valid: true, }, { - name: "Bucket wildcard ARN", + name: "Bucket wildcard ARN (SeaweedFS)", resource: "arn:seaweed:s3:::test-bucket/*", bucket: "test-bucket", valid: true, }, { - name: "Specific object ARN", + name: "Specific object ARN (SeaweedFS)", resource: "arn:seaweed:s3:::test-bucket/path/to/object.txt", bucket: "test-bucket", valid: true, }, + // AWS ARN format (compatibility) { - name: "Different bucket ARN", + name: "Exact bucket ARN (AWS)", + resource: "arn:aws:s3:::test-bucket", + bucket: "test-bucket", + valid: true, + }, + { + name: "Bucket wildcard ARN (AWS)", + resource: "arn:aws:s3:::test-bucket/*", + bucket: "test-bucket", + valid: true, + }, + { + name: "Specific object ARN (AWS)", + resource: "arn:aws:s3:::test-bucket/path/to/object.txt", + bucket: "test-bucket", + valid: true, + }, + // Simplified format (without ARN prefix) + { + name: "Simplified bucket name", + resource: "test-bucket", + bucket: "test-bucket", + valid: true, + }, + { + name: "Simplified bucket wildcard", + resource: "test-bucket/*", + bucket: "test-bucket", + valid: true, + }, + { + name: "Simplified specific object", + resource: "test-bucket/path/to/object.txt", + bucket: "test-bucket", + valid: true, + }, + // Invalid cases + { + name: "Different bucket ARN (SeaweedFS)", resource: "arn:seaweed:s3:::other-bucket/*", bucket: "test-bucket", valid: false, }, { - name: "Global S3 wildcard", + name: "Different bucket ARN (AWS)", + resource: "arn:aws:s3:::other-bucket/*", + bucket: "test-bucket", + valid: false, + }, + { + name: "Different bucket simplified", + resource: "other-bucket/*", + bucket: "test-bucket", + valid: false, + }, + { + name: "Global S3 wildcard (SeaweedFS)", resource: "arn:seaweed:s3:::*", bucket: "test-bucket", valid: false, }, + { + name: "Global S3 wildcard (AWS)", + resource: "arn:aws:s3:::*", + bucket: "test-bucket", + valid: false, + }, { name: "Invalid ARN format", resource: "invalid-arn", bucket: "test-bucket", valid: false, }, + { + name: "Bucket name prefix match but different bucket", + resource: "test-bucket-different/*", + bucket: "test-bucket", + valid: false, + }, } for _, tt := range tests { @@ -226,3 +290,106 @@ func marshalPolicy(t *testing.T, policyDoc *policy.PolicyDocument) []byte { require.NoError(t, err) return data } + +// TestIssue7252Examples tests the specific examples from GitHub issue #7252 +func TestIssue7252Examples(t *testing.T) { + s3Server := &S3ApiServer{} + + tests := []struct { + name string + policy *policy.PolicyDocument + bucket string + expectedValid bool + description string + }{ + { + name: "Issue #7252 - Standard ARN with wildcard", + policy: &policy.PolicyDocument{ + Version: "2012-10-17", + Statement: []policy.Statement{ + { + Effect: "Allow", + Principal: map[string]interface{}{ + "AWS": "*", + }, + Action: []string{"s3:GetObject"}, + Resource: []string{"arn:aws:s3:::main-bucket/*"}, + }, + }, + }, + bucket: "main-bucket", + expectedValid: true, + description: "AWS ARN format should be accepted", + }, + { + name: "Issue #7252 - Simplified resource with wildcard", + policy: &policy.PolicyDocument{ + Version: "2012-10-17", + Statement: []policy.Statement{ + { + Effect: "Allow", + Principal: map[string]interface{}{ + "AWS": "*", + }, + Action: []string{"s3:GetObject"}, + Resource: []string{"main-bucket/*"}, + }, + }, + }, + bucket: "main-bucket", + expectedValid: true, + description: "Simplified format with wildcard should be accepted", + }, + { + name: "Issue #7252 - Resource as exact bucket name", + policy: &policy.PolicyDocument{ + Version: "2012-10-17", + Statement: []policy.Statement{ + { + Effect: "Allow", + Principal: map[string]interface{}{ + "AWS": "*", + }, + Action: []string{"s3:GetObject"}, + Resource: []string{"main-bucket"}, + }, + }, + }, + bucket: "main-bucket", + expectedValid: true, + description: "Exact bucket name should be accepted", + }, + { + name: "Public read policy with AWS ARN", + policy: &policy.PolicyDocument{ + Version: "2012-10-17", + Statement: []policy.Statement{ + { + Sid: "PublicReadGetObject", + Effect: "Allow", + Principal: map[string]interface{}{ + "AWS": "*", + }, + Action: []string{"s3:GetObject"}, + Resource: []string{"arn:aws:s3:::my-public-bucket/*"}, + }, + }, + }, + bucket: "my-public-bucket", + expectedValid: true, + description: "Standard public read policy with AWS ARN should work", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := s3Server.validateBucketPolicy(tt.policy, tt.bucket) + + if tt.expectedValid { + assert.NoError(t, err, "Policy should be valid: %s", tt.description) + } else { + assert.Error(t, err, "Policy should be invalid: %s", tt.description) + } + }) + } +} diff --git a/weed/s3api/s3api_bucket_policy_handlers.go b/weed/s3api/s3api_bucket_policy_handlers.go index e079eb53e..4a83f0da4 100644 --- a/weed/s3api/s3api_bucket_policy_handlers.go +++ b/weed/s3api/s3api_bucket_policy_handlers.go @@ -274,18 +274,38 @@ func (s3a *S3ApiServer) validateBucketPolicy(policyDoc *policy.PolicyDocument, b // validateResourceForBucket checks if a resource ARN is valid for the given bucket func (s3a *S3ApiServer) validateResourceForBucket(resource, bucket string) bool { - // Expected formats: - // arn:seaweed:s3:::bucket-name - // arn:seaweed:s3:::bucket-name/* - // arn:seaweed:s3:::bucket-name/path/to/object - - expectedBucketArn := fmt.Sprintf("arn:seaweed:s3:::%s", bucket) - expectedBucketWildcard := fmt.Sprintf("arn:seaweed:s3:::%s/*", bucket) - expectedBucketPath := fmt.Sprintf("arn:seaweed:s3:::%s/", bucket) - - return resource == expectedBucketArn || - resource == expectedBucketWildcard || - strings.HasPrefix(resource, expectedBucketPath) + // Accepted formats for S3 bucket policies: + // AWS-style ARNs: + // arn:aws:s3:::bucket-name + // arn:aws:s3:::bucket-name/* + // arn:aws:s3:::bucket-name/path/to/object + // SeaweedFS ARNs: + // arn:seaweed:s3:::bucket-name + // arn:seaweed:s3:::bucket-name/* + // arn:seaweed:s3:::bucket-name/path/to/object + // Simplified formats (for convenience): + // bucket-name + // bucket-name/* + // bucket-name/path/to/object + + var resourcePath string + const awsPrefix = "arn:aws:s3:::" + const seaweedPrefix = "arn:seaweed:s3:::" + + // Strip the optional ARN prefix to get the resource path + if path, ok := strings.CutPrefix(resource, awsPrefix); ok { + resourcePath = path + } else if path, ok := strings.CutPrefix(resource, seaweedPrefix); ok { + resourcePath = path + } else { + resourcePath = resource + } + + // After stripping the optional ARN prefix, the resource path must + // either match the bucket name exactly, or be a path within the bucket. + return resourcePath == bucket || + resourcePath == bucket+"/*" || + strings.HasPrefix(resourcePath, bucket+"/") } // IAM integration functions From db35159a41e0f098c25fed2ea06255ae85503fc5 Mon Sep 17 00:00:00 2001 From: Guilherme Moreira Rodrigues <30627541+guimoreirar@users.noreply.github.com> Date: Thu, 30 Oct 2025 18:31:54 -0300 Subject: [PATCH 2/4] [Helm Chart] add missing apiVersion and kind in PVC templates for better compatibility with GitOps tools (#7408) * fix: add missing apiVersion and kind in PVC templates * fix: correct PVC template condition in SeaweedFS filer StatefulSet --- .../templates/filer/filer-statefulset.yaml | 14 ++++++++++---- .../templates/master/master-statefulset.yaml | 8 ++++++-- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/k8s/charts/seaweedfs/templates/filer/filer-statefulset.yaml b/k8s/charts/seaweedfs/templates/filer/filer-statefulset.yaml index 5c1a0950b..5aeccfa02 100644 --- a/k8s/charts/seaweedfs/templates/filer/filer-statefulset.yaml +++ b/k8s/charts/seaweedfs/templates/filer/filer-statefulset.yaml @@ -392,10 +392,12 @@ spec: nodeSelector: {{ tpl .Values.filer.nodeSelector . | indent 8 | trim }} {{- end }} - {{- if and (.Values.filer.enablePVC) (eq .Values.filer.data.type "persistentVolumeClaim") }} + {{- if and (.Values.filer.enablePVC) (not .Values.filer.data) }} # DEPRECATION: Deprecate in favor of filer.data section below volumeClaimTemplates: - - metadata: + - apiVersion: v1 + kind: PersistentVolumeClaim + metadata: name: data-filer spec: accessModes: @@ -411,7 +413,9 @@ spec: {{- if $pvc_exists }} volumeClaimTemplates: {{- if eq .Values.filer.data.type "persistentVolumeClaim" }} - - metadata: + - apiVersion: v1 + kind: PersistentVolumeClaim + metadata: name: data-filer {{- with .Values.filer.data.annotations }} annotations: @@ -425,7 +429,9 @@ spec: storage: {{ .Values.filer.data.size }} {{- end }} {{- if eq .Values.filer.logs.type "persistentVolumeClaim" }} - - metadata: + - apiVersion: v1 + kind: PersistentVolumeClaim + metadata: name: seaweedfs-filer-log-volume {{- with .Values.filer.logs.annotations }} annotations: diff --git a/k8s/charts/seaweedfs/templates/master/master-statefulset.yaml b/k8s/charts/seaweedfs/templates/master/master-statefulset.yaml index 01387fc91..704a33b80 100644 --- a/k8s/charts/seaweedfs/templates/master/master-statefulset.yaml +++ b/k8s/charts/seaweedfs/templates/master/master-statefulset.yaml @@ -327,7 +327,9 @@ spec: {{- if $pvc_exists }} volumeClaimTemplates: {{- if eq .Values.master.data.type "persistentVolumeClaim"}} - - metadata: + - apiVersion: v1 + kind: PersistentVolumeClaim + metadata: name: data-{{ .Release.Namespace }} {{- with .Values.master.data.annotations }} annotations: @@ -341,7 +343,9 @@ spec: storage: {{ .Values.master.data.size }} {{- end }} {{- if eq .Values.master.logs.type "persistentVolumeClaim"}} - - metadata: + - apiVersion: v1 + kind: PersistentVolumeClaim + metadata: name: seaweedfs-master-log-volume {{- with .Values.master.logs.annotations }} annotations: From a6da3eb77092b9744bde2394e83809cf6cfc33cb Mon Sep 17 00:00:00 2001 From: chrislu Date: Thu, 30 Oct 2025 08:58:54 -0700 Subject: [PATCH 3/4] server can start when no network for local dev --- weed/util/network.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/weed/util/network.go b/weed/util/network.go index 69559b5f0..b801ec682 100644 --- a/weed/util/network.go +++ b/weed/util/network.go @@ -44,7 +44,11 @@ func selectIpV4(netInterfaces []net.Interface, isIpV4 bool) string { } } else { if ipNet.IP.To16() != nil { - return ipNet.IP.String() + // Filter out link-local IPv6 addresses (fe80::/10) + // They require zone identifiers and are not suitable for server binding + if !ipNet.IP.IsLinkLocalUnicast() { + return ipNet.IP.String() + } } } } From f5a57a6463428a7c07179ff445a7853112bee6a0 Mon Sep 17 00:00:00 2001 From: chrislu Date: Thu, 30 Oct 2025 09:11:30 -0700 Subject: [PATCH 4/4] fixed superfluous response.WriteHeader call" warning --- weed/server/common.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/weed/server/common.go b/weed/server/common.go index 49dd78ce0..930695f4b 100644 --- a/weed/server/common.go +++ b/weed/server/common.go @@ -369,8 +369,7 @@ func ProcessRangeRequest(r *http.Request, w http.ResponseWriter, totalSize int64 err = writeFn(bufferedWriter) if err != nil { glog.Errorf("ProcessRangeRequest range[0]: %+v err: %v", w.Header(), err) - w.Header().Del("Content-Length") - http.Error(w, err.Error(), http.StatusInternalServerError) + // Cannot call http.Error() here because WriteHeader was already called return fmt.Errorf("ProcessRangeRequest range[0]: %w", err) } return nil @@ -424,7 +423,7 @@ func ProcessRangeRequest(r *http.Request, w http.ResponseWriter, totalSize int64 w.WriteHeader(http.StatusPartialContent) if _, err := io.CopyN(bufferedWriter, sendContent, sendSize); err != nil { glog.Errorf("ProcessRangeRequest err: %v", err) - http.Error(w, "Internal Error", http.StatusInternalServerError) + // Cannot call http.Error() here because WriteHeader was already called return fmt.Errorf("ProcessRangeRequest err: %w", err) } return nil