refactor: serve non-resident buckets with the global CORS policy

The pre-authentication CORS lookup stays resident-only, so client-supplied
path segments still cause no metadata I/O and no cache growth. The
fail-closed states for startup, load failures, and the internal namespace
are gone: CORS is a browser response policy rather than an authorization
boundary, and failing closed only denied browser clients CORS headers while
bucket metadata was still loading. A bucket whose stored CORS document does
not parse still gets no CORS headers. This removes the loadFailed bookkeeping
and the unused GetCorsConfig, HasAllowedOrigin, and generic Update path for
CORS; tests use the CORS-specific writer.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PvgysXDmhPBBimCReYtA8q
Signed-off-by: Feng Ruohang <rh@vonng.com>

cmd/api-router.go

@@ -461,15 +461,15 @@ func registerAPIRouter(router *mux.Router) {
 		router.Methods(http.MethodPut).
 			HandlerFunc(s3APIMiddleware(api.PutBucketACLHandler)).
 			Queries("acl", "")
-		// GetBucketCors - this is a dummy call.
+		// GetBucketCors
 		router.Methods(http.MethodGet).
 			HandlerFunc(s3APIMiddleware(api.GetBucketCorsHandler)).
 			Queries("cors", "")
-		// PutBucketCors - this is a dummy call.
+		// PutBucketCors
 		router.Methods(http.MethodPut).
 			HandlerFunc(s3APIMiddleware(api.PutBucketCorsHandler)).
 			Queries("cors", "")
-		// DeleteBucketCors - this is a dummy call.
+		// DeleteBucketCors
 		router.Methods(http.MethodDelete).
 			HandlerFunc(s3APIMiddleware(api.DeleteBucketCorsHandler)).
 			Queries("cors", "")
@@ -787,15 +787,11 @@ func corsHandler(handler http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		if r.Header.Get("Origin") != "" {
 			if bucket, _ := request2BucketObjectName(r); bucket != "" && globalBucketMetadataSys != nil {
-				// Resident-only lookup: this runs pre-auth for every
-				// Origin-bearing request using a client-supplied path segment as
-				// the bucket name. It must never load or cache metadata for
-				// arbitrary names (see GetResidentCorsConfig). GetResidentCorsConfig
-				// is the single decision point: it returns errInvalidArgument for
-				// the internal .minio.sys namespace (fail closed), a config for a
-				// resident bucket, errBucketMetadataNotInitialized for a real but
-				// unloaded bucket (fail closed), and errConfigNotFound otherwise
-				// (fall back to the global policy below).
+				// Resident-only lookup: this runs before authentication with a
+				// client-supplied path segment as the bucket name, so it must
+				// never load or cache metadata. A bucket with a stored CORS
+				// document that failed to parse gets no CORS headers; any other
+				// non-resident name falls back to the global policy below.
 				cfg, _, err := globalBucketMetadataSys.GetResidentCorsConfig(bucket)
 				if err == nil && cfg != nil {
 					if applyBucketCors(w, r, cfg) {

cmd/bucket-cors-middleware_test.go

@@ -261,6 +261,11 @@ func TestBucketCorsMetadataErrorFailsClosed(t *testing.T) {
 	oldMetadataSys := globalBucketMetadataSys
 	setObjectLayer(nil)
 	globalBucketMetadataSys = NewBucketMetadataSys()
+	// A resident bucket whose stored CORS document does not parse must not be
+	// answered with the global policy: it has a configuration we cannot honor.
+	meta := newBucketMetadata("cors-metadata-error")
+	meta.corsConfigErr = fmt.Errorf("invalid bucket CORS configuration")
+	globalBucketMetadataSys.Set("cors-metadata-error", meta)
 	defer func() {
 		setObjectLayer(oldObjectAPI)
 		globalBucketMetadataSys = oldMetadataSys
@@ -593,15 +598,15 @@ func testBucketCorsUnknownBucketDoesNotGrowMetadata(obj ObjectLayer, _ string, _
 	}
 }
 
-func TestBucketCorsStartupMissFailsClosedWithoutIO(t *testing.T) {
+func TestBucketCorsStartupMissUsesGlobalFallbackWithoutIO(t *testing.T) {
 	ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{
 		t:          t,
-		objAPITest: testBucketCorsStartupMissFailsClosedWithoutIO,
+		objAPITest: testBucketCorsStartupMissUsesGlobalFallbackWithoutIO,
 		endpoints:  []string{"GetBucketCors"},
 	})
 }
 
-func testBucketCorsStartupMissFailsClosedWithoutIO(obj ObjectLayer, _ string, _ string, _ http.Handler, _ auth.Credentials, t *testing.T) {
+func testBucketCorsStartupMissUsesGlobalFallbackWithoutIO(obj ObjectLayer, _ string, _ string, _ http.Handler, _ auth.Credentials, t *testing.T) {
 	oldObjectAPI := newObjectLayerFn()
 	oldMetadataSys := globalBucketMetadataSys
 	counting := &corsLookupCountingObjectLayer{ObjectLayer: obj}
@@ -625,8 +630,8 @@ func testBucketCorsStartupMissFailsClosedWithoutIO(obj ObjectLayer, _ string, _
 	if !innerCalled || rec.Code != http.StatusNoContent {
 		t.Fatalf("startup miss did not reach inner handler: called=%v status=%d", innerCalled, rec.Code)
 	}
-	if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "" {
-		t.Fatalf("startup miss used permissive global CORS: %q", got)
+	if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "https://app.example.com" {
+		t.Fatalf("startup miss did not fall back to the global policy: %q", got)
 	}
 	if got := counting.getObjectNInfoCalls.Load(); got != 0 {
 		t.Fatalf("startup miss performed %d metadata reads", got)
@@ -635,106 +640,3 @@ func testBucketCorsStartupMissFailsClosedWithoutIO(obj ObjectLayer, _ string, _
 		t.Fatalf("startup miss grew metadataMap to %d", got)
 	}
 }
-
-// markBucketMetadataLoadFailed records a bucket as one whose metadata failed to
-// load at startup while the subsystem is Initialized, modeling the degraded
-// state where a real bucket is not resident. Returns a restore function.
-func markBucketMetadataLoadFailed(t *testing.T, bucket string) func() {
-	t.Helper()
-	sys := globalBucketMetadataSys
-	if sys == nil {
-		t.Fatal("globalBucketMetadataSys is nil")
-	}
-	sys.Lock()
-	_, had := sys.loadFailed[bucket]
-	sys.loadFailed[bucket] = struct{}{}
-	sys.Unlock()
-	return func() {
-		sys.Lock()
-		if !had {
-			delete(sys.loadFailed, bucket)
-		}
-		sys.Unlock()
-	}
-}
-
-// TestBucketCorsLoadFailedBucketFailsClosed guards P1: a real bucket whose
-// metadata could not be loaded at startup (present in loadFailed, subsystem
-// Initialized) must NOT be answered with the permissive global CORS policy. We
-// cannot rule out a restrictive per-bucket config for it, so it must fail
-// closed — without a synchronous disk read.
-func TestBucketCorsLoadFailedBucketFailsClosed(t *testing.T) {
-	ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{
-		t:          t,
-		objAPITest: testBucketCorsLoadFailedBucketFailsClosed,
-		endpoints:  []string{"GetBucketCors"},
-	})
-}
-
-func testBucketCorsLoadFailedBucketFailsClosed(obj ObjectLayer, _ string, _ string, _ http.Handler, _ auth.Credentials, t *testing.T) {
-	restoreInit := markBucketMetadataInitialized(t)
-	defer restoreInit()
-	restoreFail := markBucketMetadataLoadFailed(t, "strict-cors-bucket")
-	defer restoreFail()
-
-	oldObjectAPI := newObjectLayerFn()
-	counting := &corsLookupCountingObjectLayer{ObjectLayer: obj}
-	setObjectLayer(counting)
-	defer setObjectLayer(oldObjectAPI)
-
-	wrapped := corsHandler(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
-		w.WriteHeader(http.StatusNotFound)
-	}))
-
-	for _, method := range []string{http.MethodGet, http.MethodOptions} {
-		rec := httptest.NewRecorder()
-		req := httptest.NewRequest(method, getGetObjectURL("", "strict-cors-bucket", "object"), nil)
-		req.Header.Set("Origin", "https://app.example.com")
-		if method == http.MethodOptions {
-			req.Header.Set("Access-Control-Request-Method", http.MethodGet)
-		}
-		wrapped.ServeHTTP(rec, req)
-
-		if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "" {
-			t.Fatalf("%s: load-failed bucket fell back to global allow-origin %q", method, got)
-		}
-		if got := rec.Header().Get("Access-Control-Allow-Credentials"); got != "" {
-			t.Fatalf("%s: load-failed bucket fell back to global credentials %q", method, got)
-		}
-	}
-	if got := counting.getObjectNInfoCalls.Load(); got != 0 {
-		t.Fatalf("load-failed CORS lookup performed %d synchronous bucket metadata reads", got)
-	}
-}
-
-// TestBucketCorsInternalBucketFailsClosed guards P2: an Origin-bearing request
-// whose first path segment is the reserved .minio.sys namespace must preserve
-// GetConfig's errInvalidArgument semantics and fail closed, not fall back to
-// the permissive global CORS policy.
-func TestBucketCorsInternalBucketFailsClosed(t *testing.T) {
-	ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{
-		t:          t,
-		objAPITest: testBucketCorsInternalBucketFailsClosed,
-		endpoints:  []string{"GetBucketCors"},
-	})
-}
-
-func testBucketCorsInternalBucketFailsClosed(_ ObjectLayer, _ string, _ string, _ http.Handler, _ auth.Credentials, t *testing.T) {
-	restoreInit := markBucketMetadataInitialized(t)
-	defer restoreInit()
-
-	wrapped := corsHandler(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
-		w.WriteHeader(http.StatusNotFound)
-	}))
-	rec := httptest.NewRecorder()
-	req := httptest.NewRequest(http.MethodGet, getGetObjectURL("", minioMetaBucket, "object"), nil)
-	req.Header.Set("Origin", "https://app.example.com")
-	wrapped.ServeHTTP(rec, req)
-
-	if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "" {
-		t.Fatalf("internal bucket fell back to global allow-origin %q", got)
-	}
-	if got := rec.Header().Get("Access-Control-Allow-Credentials"); got != "" {
-		t.Fatalf("internal bucket fell back to global credentials %q", got)
-	}
-}

cmd/bucket-cors-site-replication_test.go

@@ -85,7 +85,7 @@ func testPeerBucketCorsReplicationOrdering(_ ObjectLayer, _ string, bucket strin
 	if !meta.CorsConfigUpdatedAt.Equal(putAt) {
 		t.Fatalf("peer PUT timestamp = %v, want source time %v", meta.CorsConfigUpdatedAt, putAt)
 	}
-	cfg, cfgAt, err := globalBucketMetadataSys.GetCorsConfig(bucket)
+	cfg, cfgAt, err := globalBucketMetadataSys.GetResidentCorsConfig(bucket)
 	if err != nil {
 		t.Fatalf("peer PUT stored raw XML but no parsed config: %v", err)
 	}
@@ -165,10 +165,10 @@ func TestSiteReplicationMetaInfoPreservesCorsTombstone(t *testing.T) {
 
 func testSiteReplicationMetaInfoPreservesCorsTombstone(obj ObjectLayer, _ string, bucket string, _ http.Handler, _ auth.Credentials, t *testing.T) {
 	ctx := t.Context()
-	if _, err := globalBucketMetadataSys.Update(ctx, bucket, bucketCorsConfig, []byte(testSiteReplicationCORSDoc)); err != nil {
+	if _, err := updateLocalBucketCORSMetadata(ctx, obj, bucket, []byte(testSiteReplicationCORSDoc)); err != nil {
 		t.Fatal(err)
 	}
-	deleteAt, err := globalBucketMetadataSys.Delete(ctx, bucket, bucketCorsConfig)
+	deleteAt, err := updateLocalBucketCORSMetadata(ctx, obj, bucket, nil)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -918,8 +918,8 @@ func testLegacyInvalidCorsMetadataCanBeDeleted(obj ObjectLayer, _ string, bucket
 		t.Fatalf("legacy CORS state = (%#v, %v), want fail-closed parse error", loaded.corsConfig, loaded.corsConfigErr)
 	}
 	globalBucketMetadataSys.Set(bucket, loaded)
-	if _, gotAt, err := globalBucketMetadataSys.GetCorsConfig(bucket); err == nil || !gotAt.Equal(legacyAt) {
-		t.Fatalf("GetCorsConfig = timestamp %v, error %v; want legacy timestamp and error", gotAt, err)
+	if _, gotAt, err := globalBucketMetadataSys.GetResidentCorsConfig(bucket); err == nil || !gotAt.Equal(legacyAt) {
+		t.Fatalf("GetResidentCorsConfig = timestamp %v, error %v; want legacy timestamp and error", gotAt, err)
 	}
 	if _, gotAt, err := globalBucketMetadataSys.GetCorsConfigXML(bucket); err == nil || !gotAt.Equal(legacyAt) {
 		t.Fatalf("GetCorsConfigXML = timestamp %v, error %v; want legacy timestamp and error", gotAt, err)

cmd/bucket-metadata-sys.go

@@ -51,14 +51,6 @@ type BucketMetadataSys struct {
 	initialized bool
 	group       *singleflight.Group
 	metadataMap map[string]BucketMetadata
-	// loadFailed tracks real buckets whose metadata could not be loaded at
-	// startup (concurrentLoad) or during a refresh. Such buckets are NOT
-	// resident in metadataMap even though the subsystem is Initialized, so a
-	// plain map miss cannot distinguish "not a bucket" from "known bucket whose
-	// config we could not read". Callers that must fail closed for a real but
-	// unreadable bucket (e.g. per-bucket CORS) consult this set. It is bounded
-	// by the number of load failures and is empty in normal operation.
-	loadFailed map[string]struct{}
 }
 
 // Count returns number of bucket metadata map entries.
@@ -75,7 +67,6 @@ func (sys *BucketMetadataSys) Remove(buckets ...string) {
 	for _, bucket := range buckets {
 		sys.group.Forget(bucket)
 		delete(sys.metadataMap, bucket)
-		delete(sys.loadFailed, bucket)
 		globalBucketMonitor.DeleteBucket(bucket)
 	}
 	sys.Unlock()
@@ -93,11 +84,6 @@ func (sys *BucketMetadataSys) RemoveStaleBuckets(diskBuckets set.StringSet) {
 		delete(sys.metadataMap, bucket)
 		globalBucketMonitor.DeleteBucket(bucket)
 	}
-	for bucket := range sys.loadFailed {
-		if !diskBuckets.Contains(bucket) {
-			delete(sys.loadFailed, bucket)
-		}
-	}
 }
 
 // Set - sets a new metadata in-memory.
@@ -109,7 +95,6 @@ func (sys *BucketMetadataSys) Set(bucket string, meta BucketMetadata) {
 	if !isMinioMetaBucketName(bucket) {
 		sys.Lock()
 		sys.metadataMap[bucket] = meta
-		delete(sys.loadFailed, bucket)
 		sys.Unlock()
 	}
 }
@@ -163,9 +148,6 @@ func (sys *BucketMetadataSys) updateAndParse(ctx context.Context, bucket string,
 		case bucketTaggingConfig:
 			meta.TaggingConfigXML = configData
 			meta.TaggingConfigUpdatedAt = updatedAt
-		case bucketCorsConfig:
-			meta.CorsConfigXML = configData
-			meta.CorsConfigUpdatedAt = updatedAt
 		case bucketQuotaConfigFile:
 			meta.QuotaConfigJSON = configData
 			meta.QuotaConfigUpdatedAt = updatedAt
@@ -415,12 +397,22 @@ func (sys *BucketMetadataSys) GetSSEConfig(bucket string) (*bucketsse.BucketSSEC
 	return meta.sseConfig, meta.EncryptionConfigUpdatedAt, nil
 }
 
-// GetCorsConfig returns the CORS configuration for the given bucket.
-// The returned object must not be modified.
-func (sys *BucketMetadataSys) GetCorsConfig(bucket string) (*cors.Config, time.Time, error) {
-	meta, _, err := sys.GetConfig(GlobalContext, bucket)
-	if err != nil {
-		return nil, time.Time{}, err
+// GetResidentCorsConfig returns the CORS configuration of a bucket whose
+// metadata is already resident in memory. It runs before authentication for
+// every Origin-bearing request with a client-supplied path segment, so it
+// never loads or caches metadata. A name that is not resident, including any
+// real bucket while startup loading is still in progress, reports
+// errConfigNotFound and the caller applies the global CORS policy exactly as
+// releases without per-bucket CORS did.
+func (sys *BucketMetadataSys) GetResidentCorsConfig(bucket string) (*cors.Config, time.Time, error) {
+	if isReservedOrInvalidBucket(bucket, true) {
+		return nil, time.Time{}, errConfigNotFound
+	}
+	sys.RLock()
+	meta, ok := sys.metadataMap[bucket]
+	sys.RUnlock()
+	if !ok {
+		return nil, time.Time{}, errConfigNotFound
 	}
 	if meta.corsConfigErr != nil {
 		return nil, meta.CorsConfigUpdatedAt, meta.corsConfigErr
@@ -431,65 +423,6 @@ func (sys *BucketMetadataSys) GetCorsConfig(bucket string) (*cors.Config, time.T
 	return meta.corsConfig, meta.CorsConfigUpdatedAt, nil
 }
 
-// GetResidentCorsConfig returns the CORS configuration for the given bucket
-// using only bucket metadata that is already resident in memory. Unlike
-// GetCorsConfig it never loads metadata from disk and never caches a new
-// entry.
-//
-// The per-request CORS middleware runs before authentication, for every
-// Origin-bearing request, using the validated first path segment as the bucket
-// name.
-// Routing that through GetCorsConfig (which loads and caches) let an
-// unauthenticated client grow metadataMap without bound and trigger an
-// erasure metadata probe for every distinct, attacker-controlled,
-// non-existent name it sent with an Origin header (e.g. /minio/... , /api/... ,
-// or random buckets). Every bucket that can carry a CORS document is made
-// resident when the document is written (Set) and when metadata is loaded at
-// startup (Init/concurrentLoad), so a resident-only read is complete for real
-// buckets while costing only an in-memory map lookup for everything else.
-//
-// While bucket metadata is still loading (not yet Initialized) a non-resident
-// bucket returns errBucketMetadataNotInitialized so the caller fails closed
-// rather than answering with the permissive global policy for a bucket whose
-// restrictive CORS document may simply not be loaded yet.
-func (sys *BucketMetadataSys) GetResidentCorsConfig(bucket string) (*cors.Config, time.Time, error) {
-	if isMinioMetaBucketName(bucket) {
-		// Preserve GetConfig's semantics for the internal namespace: this is
-		// not a real bucket, and returning a non-errConfigNotFound error makes
-		// the CORS middleware fail closed rather than answer for .minio.sys
-		// with the permissive global policy.
-		return nil, time.Time{}, errInvalidArgument
-	}
-	if isReservedOrInvalidBucket(bucket, true) {
-		return nil, time.Time{}, errConfigNotFound
-	}
-	sys.RLock()
-	meta, ok := sys.metadataMap[bucket]
-	_, failed := sys.loadFailed[bucket]
-	initialized := sys.initialized
-	sys.RUnlock()
-	if ok {
-		if meta.corsConfigErr != nil {
-			return nil, meta.CorsConfigUpdatedAt, meta.corsConfigErr
-		}
-		if meta.corsConfig == nil {
-			return nil, time.Time{}, errConfigNotFound
-		}
-		return meta.corsConfig, meta.CorsConfigUpdatedAt, nil
-	}
-	// Not resident. Two cases must not be conflated:
-	//   - metadata is still loading (!initialized), or this is a real bucket
-	//     whose metadata failed to load: we cannot rule out a restrictive CORS
-	//     config, so fail closed rather than answer with the global policy.
-	//   - a fully initialized subsystem with no record of the name: it is not a
-	//     bucket that can carry CORS, so fall back to the global policy without
-	//     loading or caching metadata for an arbitrary, client-supplied name.
-	if !initialized || failed {
-		return nil, time.Time{}, errBucketMetadataNotInitialized
-	}
-	return nil, time.Time{}, errConfigNotFound
-}
-
 // GetCorsConfigXML returns the raw stored CORS configuration XML for the
 // given bucket, preserving the document exactly as it was PUT (including
 // the S3 xmlns and any unmodeled elements).
@@ -687,13 +620,8 @@ func (sys *BucketMetadataSys) concurrentLoad(ctx context.Context, buckets []stri
 	sys.Lock()
 	for i, meta := range bucketMetas {
 		if errs[i] != nil {
-			// Real bucket whose metadata could not be loaded: record it so
-			// consumers that must fail closed (per-bucket CORS) can tell it
-			// apart from a name that is not a bucket at all.
-			sys.loadFailed[buckets[i]] = struct{}{}
 			continue
 		}
-		delete(sys.loadFailed, buckets[i])
 		sys.metadataMap[buckets[i]] = meta
 	}
 	sys.Unlock()
@@ -743,9 +671,6 @@ func (sys *BucketMetadataSys) refreshBucketsMetadataLoop(ctx context.Context) {
 				meta, err := loadBucketMetadata(ctx, sys.objAPI, bucket)
 				if err != nil {
 					internalLogIf(ctx, err, logger.WarningKind)
-					sys.Lock()
-					sys.loadFailed[bucket] = struct{}{}
-					sys.Unlock()
 					wait() // wait to proceed to next entry.
 					continue
 				}
@@ -756,8 +681,6 @@ func (sys *BucketMetadataSys) refreshBucketsMetadataLoop(ctx context.Context) {
 					updated = true
 					sys.metadataMap[bucket] = meta
 				}
-				// A successful (re)load clears any earlier load failure.
-				delete(sys.loadFailed, bucket)
 				sys.Unlock()
 
 				if updated {
@@ -805,7 +728,6 @@ func (sys *BucketMetadataSys) init(ctx context.Context, buckets []string) {
 func (sys *BucketMetadataSys) Reset() {
 	sys.Lock()
 	clear(sys.metadataMap)
-	clear(sys.loadFailed)
 	sys.Unlock()
 }
 
@@ -813,7 +735,6 @@ func (sys *BucketMetadataSys) Reset() {
 func NewBucketMetadataSys() *BucketMetadataSys {
 	return &BucketMetadataSys{
 		metadataMap: make(map[string]BucketMetadata),
-		loadFailed:  make(map[string]struct{}),
 		group:       &singleflight.Group{},
 	}
 }

cmd/site-replication-status-accounting_test.go

@@ -69,12 +69,14 @@ func testSiteReplicationStatusAccountsPerSiteAndSurvivesMalformedConfig(obj Obje
 		bucketSSEConfig:        sseXML,
 		bucketQuotaConfigFile:  quotaJSON,
 		bucketPolicyConfig:     policyJSON,
-		bucketCorsConfig:       []byte(testSiteReplicationCORSDoc),
 	} {
 		if _, err := globalBucketMetadataSys.Update(ctx, localBucket, configFile, data); err != nil {
 			t.Fatalf("%s: update %s: %v", instanceType, configFile, err)
 		}
 	}
+	if _, err := updateLocalBucketCORSMetadata(ctx, obj, localBucket, []byte(testSiteReplicationCORSDoc)); err != nil {
+		t.Fatalf("%s: update %s: %v", instanceType, bucketCorsConfig, err)
+	}
 
 	encode := func(data []byte) *string {
 		encoded := base64.StdEncoding.EncodeToString(data)