diff --git a/weed/s3api/policy_conversion.go b/weed/s3api/policy_conversion.go index d47564642..726bc0200 100644 --- a/weed/s3api/policy_conversion.go +++ b/weed/s3api/policy_conversion.go @@ -101,12 +101,20 @@ func convertPrincipal(principal interface{}) (*policy_engine.StringOrStringSlice switch p := principal.(type) { case string: + if p == "" { + return nil, fmt.Errorf("principal string cannot be empty") + } result := policy_engine.NewStringOrStringSlice(p) return &result, nil case []string: if len(p) == 0 { return nil, nil } + for _, s := range p { + if s == "" { + return nil, fmt.Errorf("principal string in slice cannot be empty") + } + } result := policy_engine.NewStringOrStringSlice(p...) return &result, nil case []interface{}: @@ -117,6 +125,9 @@ func convertPrincipal(principal interface{}) (*policy_engine.StringOrStringSlice if err != nil { return nil, fmt.Errorf("failed to convert principal array item: %w", err) } + if str == "" { + return nil, fmt.Errorf("principal string in slice cannot be empty") + } strs = append(strs, str) } } diff --git a/weed/s3api/policy_conversion_test.go b/weed/s3api/policy_conversion_test.go index 90468861d..e7a77126f 100644 --- a/weed/s3api/policy_conversion_test.go +++ b/weed/s3api/policy_conversion_test.go @@ -438,6 +438,59 @@ func TestConvertPrincipalUnsupportedTypes(t *testing.T) { } } +func TestConvertPrincipalEmptyStrings(t *testing.T) { + // Test that empty string principals are rejected for security + testCases := []struct { + name string + principal interface{} + wantError string + }{ + { + name: "Empty string principal", + principal: "", + wantError: "principal string cannot be empty", + }, + { + name: "Empty string in array", + principal: []string{"arn:aws:iam::123456789012:user/Alice", "", "arn:aws:iam::123456789012:user/Bob"}, + wantError: "principal string in slice cannot be empty", + }, + { + name: "Empty string in interface array", + principal: []interface{}{"arn:aws:iam::123456789012:user/Alice", ""}, + wantError: "principal string in slice cannot be empty", + }, + { + name: "Empty string in AWS map", + principal: map[string]interface{}{ + "AWS": "", + }, + wantError: "principal string cannot be empty", + }, + { + name: "Empty string in AWS map array", + principal: map[string]interface{}{ + "AWS": []string{"arn:aws:iam::123456789012:user/Alice", ""}, + }, + wantError: "principal string in slice cannot be empty", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result, err := convertPrincipal(tc.principal) + if err == nil { + t.Error("Expected error for empty principal string") + } else if !strings.Contains(err.Error(), tc.wantError) { + t.Errorf("Expected error containing %q, got: %v", tc.wantError, err) + } + if result != nil { + t.Error("Expected nil result for empty principal string") + } + }) + } +} + func TestConvertStatementWithUnsupportedFields(t *testing.T) { // Test that errors are returned for unsupported fields // These fields are critical for policy semantics and ignoring them would be a security risk