fix: keep load-failed buckets fail-closed in the CORS lookup

Restore the load-failure set that the cleanup removed. A presigned URL is authenticated by its signature, so for such requests the bucket's CORS document is the only origin boundary a browser enforces; a real bucket whose metadata failed to load must therefore not be answered with the global policy, and without this bit it is indistinguishable from a name that is not a bucket. Two helpers own the set's lifecycle; the resident-only lookup and the removal of the internal-namespace special case stay. 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>
2026-09-05 18:16:16 +03:00 · 2026-09-02 19:51:54 +08:00
parent ec2979ca48
commit 3f9c79e919
4 changed files with 111 additions and 10 deletions
@@ -790,9 +790,10 @@ func corsHandler(handler http.Handler) http.Handler {
 				// 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. While startup loading is still
-				// running, and for a bucket whose stored CORS document failed to
-				// parse, the request gets no CORS headers; any other non-resident
-				// name falls back to the global policy below.
+				// running, for a real bucket whose metadata failed to load, and
+				// for a bucket whose stored CORS document failed to parse, the
+				// request 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) {
@@ -640,3 +640,74 @@ 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)
+	}
+}
@@ -51,8 +51,19 @@ type BucketMetadataSys struct {
 	initialized bool
 	group       *singleflight.Group
 	metadataMap map[string]BucketMetadata
+	// loadFailed records real buckets whose metadata could not be loaded at
+	// startup or during a refresh. They are absent from metadataMap even though
+	// the subsystem is initialized, and without this bit a resident-only lookup
+	// could not tell them apart from a name that is not a bucket at all. The
+	// set is bounded by the number of failed loads and empty in normal operation.
+	loadFailed map[string]struct{}
 }

+// noteLoadFailure and clearLoadFailure maintain loadFailed; both expect the
+// caller to hold sys.Lock.
+func (sys *BucketMetadataSys) noteLoadFailure(bucket string)  { sys.loadFailed[bucket] = struct{}{} }
+func (sys *BucketMetadataSys) clearLoadFailure(bucket string) { delete(sys.loadFailed, bucket) }
+
 // Count returns number of bucket metadata map entries.
 func (sys *BucketMetadataSys) Count() int {
 	sys.RLock()
@@ -67,6 +78,7 @@ func (sys *BucketMetadataSys) Remove(buckets ...string) {
 	for _, bucket := range buckets {
 		sys.group.Forget(bucket)
 		delete(sys.metadataMap, bucket)
+		sys.clearLoadFailure(bucket)
 		globalBucketMonitor.DeleteBucket(bucket)
 	}
 	sys.Unlock()
@@ -84,6 +96,11 @@ func (sys *BucketMetadataSys) RemoveStaleBuckets(diskBuckets set.StringSet) {
 		delete(sys.metadataMap, bucket)
 		globalBucketMonitor.DeleteBucket(bucket)
 	}
+	for bucket := range sys.loadFailed {
+		if !diskBuckets.Contains(bucket) {
+			sys.clearLoadFailure(bucket)
+		}
+	}
 }

 // Set - sets a new metadata in-memory.
@@ -95,6 +112,7 @@ func (sys *BucketMetadataSys) Set(bucket string, meta BucketMetadata) {
 	if !isMinioMetaBucketName(bucket) {
 		sys.Lock()
 		sys.metadataMap[bucket] = meta
+		sys.clearLoadFailure(bucket)
 		sys.Unlock()
 	}
 }
@@ -400,21 +418,24 @@ func (sys *BucketMetadataSys) GetSSEConfig(bucket string) (*bucketsse.BucketSSEC
 // 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. Until startup loading has completed, a
-// non-resident name may still be a bucket with a restrictive document, so it
-// reports errBucketMetadataNotInitialized and gets no CORS answer. After
-// that, a non-resident name reports errConfigNotFound and the caller applies
-// the global CORS policy exactly as releases without per-bucket CORS did.
+// never loads or caches metadata. A non-resident name gets no CORS answer
+// (errBucketMetadataNotInitialized) while startup loading is still running,
+// and afterwards when it is a real bucket whose metadata failed to load: a
+// presigned URL is authenticated on its own, so the bucket's CORS document is
+// the only origin boundary a browser enforces for it. Any other non-resident
+// name 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]
+	_, failed := sys.loadFailed[bucket]
 	initialized := sys.initialized
 	sys.RUnlock()
 	if !ok {
-		if !initialized {
+		if !initialized || failed {
 			return nil, time.Time{}, errBucketMetadataNotInitialized
 		}
 		return nil, time.Time{}, errConfigNotFound
@@ -625,8 +646,10 @@ func (sys *BucketMetadataSys) concurrentLoad(ctx context.Context, buckets []stri
 	sys.Lock()
 	for i, meta := range bucketMetas {
 		if errs[i] != nil {
+			sys.noteLoadFailure(buckets[i])
 			continue
 		}
+		sys.clearLoadFailure(buckets[i])
 		sys.metadataMap[buckets[i]] = meta
 	}
 	sys.Unlock()
@@ -676,6 +699,9 @@ 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.noteLoadFailure(bucket)
+					sys.Unlock()
 					wait() // wait to proceed to next entry.
 					continue
 				}
@@ -686,6 +712,7 @@ func (sys *BucketMetadataSys) refreshBucketsMetadataLoop(ctx context.Context) {
 					updated = true
 					sys.metadataMap[bucket] = meta
 				}
+				sys.clearLoadFailure(bucket)
 				sys.Unlock()

 				if updated {
@@ -733,6 +760,7 @@ func (sys *BucketMetadataSys) init(ctx context.Context, buckets []string) {
 func (sys *BucketMetadataSys) Reset() {
 	sys.Lock()
 	clear(sys.metadataMap)
+	clear(sys.loadFailed)
 	sys.Unlock()
 }

@@ -740,6 +768,7 @@ func (sys *BucketMetadataSys) Reset() {
 func NewBucketMetadataSys() *BucketMetadataSys {
 	return &BucketMetadataSys{
 		metadataMap: make(map[string]BucketMetadata),
+		loadFailed:  make(map[string]struct{}),
 		group:       &singleflight.Group{},
 	}
 }
@@ -41,7 +41,7 @@ The first Silo community release was cut from upstream history that already cont
 | `CVE-2026-34986` | `68e0ba997` | Upgrades `go-jose` to `v4.1.4`. |
 | `CVE-2026-39883` | `1869bd30b`, `e4fa06394` | Updates OpenTelemetry dependencies. |
 | Upstream Go security fixes | [Go 1.26.5](https://go.dev/doc/devel/release#go1.26.5) | Bumps the required toolchain to Go 1.26.5, which includes security fixes to `crypto/tls` and `os`. |
-| Toolchain and dependency refresh | [Go 1.27.0](https://go.dev/doc/devel/release#go1.27) via [`43f4bb7ed`](https://github.com/pgsty/silo/commit/43f4bb7ed), [`edc8be6ed`](https://github.com/pgsty/silo/commit/edc8be6ed), [`4d6e1ea8e`](https://github.com/pgsty/silo/commit/4d6e1ea8e) | Moves the toolchain to Go 1.27.0 and refreshes the dependency stack (upstream `minio-go` v7.3.1 pre-release, etcd client v3.7.1, `jwx` v3.0.13, `klauspost/compress` v1.19.2; the `silo-go` fork is no longer used). `govulncheck` reports no reachable vulnerability at `6586fbfd0`. |
+| Toolchain and dependency refresh | [Go 1.27.0](https://go.dev/doc/devel/release#go1.27) via [`43f4bb7ed`](https://github.com/pgsty/silo/commit/43f4bb7ed), [`edc8be6ed`](https://github.com/pgsty/silo/commit/edc8be6ed), [`4d6e1ea8e`](https://github.com/pgsty/silo/commit/4d6e1ea8e) | Moves the toolchain to Go 1.27.0 and refreshes the dependency stack (etcd client v3.7.1, `jwx` v3.0.13, `klauspost/compress` v1.19.2). The pre-release cleanup then returns to upstream `minio-go` (v7.3.1 pre-release) and retires the `silo-go` fork; `govulncheck` reports no reachable vulnerability on the release candidate. |
 | [GO-2026-6061](https://pkg.go.dev/vuln/GO-2026-6061) / [GHSA-hrxh-6v49-42gf](https://github.com/advisories/GHSA-hrxh-6v49-42gf) | gRPC `v1.82.1` | Updates gRPC to the first fixed version for vulnerabilities in the xDS RBAC authorization engine and HTTP/2 transport server. |
 | [GO-2026-5970](https://pkg.go.dev/vuln/GO-2026-5970) / `CVE-2026-56852` | `x/text` `v0.39.0` | Updates `x/text` to the first fixed version for an infinite loop on invalid input. |