From 5700aa5e91374f9d76f40b455e6c9a4488125ca5 Mon Sep 17 00:00:00 2001 From: Wleeaf Date: Sat, 24 Oct 2026 16:58:59 +0300 Subject: [PATCH] s3: evaluate bucket CORS for path-style OPTIONS on the bucket root (#12028) * s3: evaluate bucket CORS for path-style OPTIONS on the bucket root A path-style preflight for /bucket (no trailing slash) missed the bucket OPTIONS route, whose template is /{bucket}/, and was answered by the global OPTIONS handler from the global AllowedOrigins instead of the bucket CORS configuration. Register the bucket OPTIONS route without a path matcher, and keep OPTIONS for the health probe paths on the service-level handler ahead of each path-style router. Fixes #12025 * test: drop the issue reference from the bucket-root preflight test comment --------- Co-authored-by: Wleeaf <179125857+wleeaf@users.noreply.github.com> Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- weed/s3api/s3api_server.go | 22 +++++-- weed/s3api/s3api_server_routing_test.go | 87 +++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 5 deletions(-) diff --git a/weed/s3api/s3api_server.go b/weed/s3api/s3api_server.go index c10e311e3d..dc9cb139a2 100644 --- a/weed/s3api/s3api_server.go +++ b/weed/s3api/s3api_server.go @@ -947,6 +947,17 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) { // Object path pattern with (?s) flag to match newlines in object keys const objectPath = "/{object:(?s).+}" + // CORS preflight for service-level requests, answered from the global + // AllowedOrigins rather than a bucket's CORS configuration + serviceOptionsHandler := func(w http.ResponseWriter, r *http.Request) { + if s3a.handleCORSOriginValidation(w, r) { + writeSuccessResponseEmpty(w, r) + } + } + // A path-style bucket router would take the health probe paths as bucket + // names, so their preflight is registered ahead of each one + const healthProbePath = "/{probe:status|healthz|readyz}" + var routers []*mux.Router if s3a.option.DomainName != "" { domainNames := strings.Split(s3a.option.DomainName, ",") @@ -954,6 +965,7 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) { // Register path-style domains for _, domain := range pathStyleDomains { + apiRouter.Host(domain).Methods(http.MethodOptions).Path(healthProbePath).HandlerFunc(serviceOptionsHandler) routers = append(routers, apiRouter.Host(domain).PathPrefix("/{bucket}").Subrouter()) apiRouter.Host(domain). Methods(http.MethodGet). @@ -971,6 +983,7 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) { // arrive via an IP, an unlisted hostname, or a reverse proxy that rewrites // the Host header still match bucket routes. Host-specific routers above // take precedence because they were registered first. + apiRouter.Methods(http.MethodOptions).Path(healthProbePath).HandlerFunc(serviceOptionsHandler) routers = append(routers, apiRouter.PathPrefix("/{bucket}").Subrouter()) // Get CORS middleware instance with caching @@ -986,8 +999,9 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) { bucket.Use(corsMiddleware.Handler) // Bucket-specific OPTIONS handler for CORS preflight requests - // Use PathPrefix to catch all bucket-level preflight routes including /bucket/object - bucket.PathPrefix("/").Methods(http.MethodOptions).HandlerFunc(corsMiddleware.HandleOptionsRequest) + // No path matcher, so it also catches the path-style bucket root (/bucket), + // which PathPrefix("/") would miss, as well as /bucket/ and /bucket/object + bucket.Methods(http.MethodOptions).HandlerFunc(corsMiddleware.HandleOptionsRequest) // each case should follow the next rule: // - requesting object with query must precede any other methods @@ -1222,9 +1236,7 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) { return } - if s3a.handleCORSOriginValidation(w, r) { - writeSuccessResponseEmpty(w, r) - } + serviceOptionsHandler(w, r) }) // STS API endpoint for AssumeRoleWithWebIdentity diff --git a/weed/s3api/s3api_server_routing_test.go b/weed/s3api/s3api_server_routing_test.go index 9459fe588f..9316174924 100644 --- a/weed/s3api/s3api_server_routing_test.go +++ b/weed/s3api/s3api_server_routing_test.go @@ -17,6 +17,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/iam/policy" "github.com/seaweedfs/seaweedfs/weed/iam/sts" "github.com/seaweedfs/seaweedfs/weed/pb/iam_pb" + "github.com/seaweedfs/seaweedfs/weed/s3api/cors" "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" "github.com/seaweedfs/seaweedfs/weed/util" "github.com/stretchr/testify/assert" @@ -442,3 +443,89 @@ func TestRouting_BucketRouteMatchesAnyHost(t *testing.T) { }) } } + +// TestRouting_BucketRootPreflightUsesBucketCORS is a regression test: a +// path-style preflight for the bucket root without a trailing +// slash must be evaluated against the bucket CORS rules like /bucket/, not +// answered from the global AllowedOrigins. Service-level paths keep the +// global answer, and virtual-host routing is unchanged. +func TestRouting_BucketRootPreflightUsesBucketCORS(t *testing.T) { + const ( + bucket = "cors-bucket" + allowedOrigin = "https://app.example.com" + foreignOrigin = "https://untrusted.example" + ) + pathStyleBucketPaths := []string{"/" + bucket, "/" + bucket + "/", "/" + bucket + "?list-type=2", "/" + bucket + "/key"} + + cases := []struct { + name string + domainName string + host string + bucketPaths []string // evaluated against the bucket CORS rules + servicePaths []string // answered from the global AllowedOrigins + }{ + { + name: "path-style", + host: "10.0.0.5:8333", + bucketPaths: pathStyleBucketPaths, + servicePaths: []string{"/", "/status", "/healthz", "/readyz"}, + }, + { + name: "path-style domain", + domainName: "s3.example.com,api.s3.example.com", + host: "api.s3.example.com", + bucketPaths: pathStyleBucketPaths, + servicePaths: []string{"/status", "/healthz", "/readyz"}, + }, + { + name: "virtual-host domain", + domainName: "s3.example.com,api.s3.example.com", + host: bucket + ".s3.example.com", + bucketPaths: []string{"/", "/?list-type=2", "/key", "/status"}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + s3a := setupRoutingTestServer(t) + s3a.option.DomainName = tc.domainName + s3a.option.AllowedOrigins = []string{"*"} + s3a.bucketConfigCache = NewBucketConfigCache(time.Minute) + s3a.bucketConfigCache.Set(bucket, &BucketConfig{ + Name: bucket, + CORS: &cors.CORSConfiguration{CORSRules: []cors.CORSRule{{ + AllowedOrigins: []string{allowedOrigin}, + AllowedMethods: []string{http.MethodGet, http.MethodPut}, + AllowedHeaders: []string{"*"}, + }}}, + }) + router := mux.NewRouter() + s3a.registerRouter(router) + + preflight := func(path, origin string) *httptest.ResponseRecorder { + req := httptest.NewRequest(http.MethodOptions, "http://"+tc.host+path, nil) + req.Header.Set("Origin", origin) + req.Header.Set("Access-Control-Request-Method", http.MethodPut) + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + return rr + } + + for _, path := range tc.bucketPaths { + rr := preflight(path, foreignOrigin) + assert.Equal(t, http.StatusForbidden, rr.Code, "foreign origin on %s", path) + assert.Empty(t, rr.Header().Get("Access-Control-Allow-Origin"), "foreign origin on %s", path) + + rr = preflight(path, allowedOrigin) + assert.Equal(t, http.StatusOK, rr.Code, "allowed origin on %s", path) + assert.Equal(t, allowedOrigin, rr.Header().Get("Access-Control-Allow-Origin"), "allowed origin on %s", path) + } + + for _, path := range tc.servicePaths { + rr := preflight(path, foreignOrigin) + assert.Equal(t, http.StatusOK, rr.Code, "service-level %s", path) + assert.Equal(t, "*", rr.Header().Get("Access-Control-Allow-Origin"), "service-level %s", path) + } + }) + } +}