fix(iam): reject incomplete policy loads throughout authorization

Signed-off-by: Feng Ruohang <rh@vonng.com>

cmd/admin-handlers-users.go

@@ -622,7 +622,11 @@ func (a adminAPIHandlers) TemporaryAccountInfo(w http.ResponseWriter, r *http.Re
 			policiesNames = policySet.ToSlice()
 		}
 
-		stsAccountPolicy = globalIAMSys.GetCombinedPolicy(policiesNames...)
+		stsAccountPolicy, err = globalIAMSys.GetCombinedPolicy(policiesNames...)
+		if err != nil {
+			writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL)
+			return
+		}
 	}
 
 	policyJSON, err := json.MarshalIndent(stsAccountPolicy, "", " ")
@@ -987,7 +991,11 @@ func (a adminAPIHandlers) InfoServiceAccount(w http.ResponseWriter, r *http.Requ
 			writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL)
 			return
 		}
-		svcAccountPolicy = globalIAMSys.GetCombinedPolicy(policiesNames...)
+		svcAccountPolicy, err = globalIAMSys.GetCombinedPolicy(policiesNames...)
+		if err != nil {
+			writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL)
+			return
+		}
 	}
 
 	policyJSON, err := json.MarshalIndent(svcAccountPolicy, "", " ")
@@ -1466,6 +1474,7 @@ func (a adminAPIHandlers) AccountInfoHandler(w http.ResponseWriter, r *http.Requ
 	roleArn := policy.Args{Claims: cred.Claims}.GetRoleArn()
 	policySetFromClaims, hasPolicyClaim := policy.GetPoliciesFromClaims(cred.Claims, iamPolicyClaimNameOpenID())
 	var effectivePolicy policy.Policy
+	var policyErr error
 
 	var buf []byte
 	switch {
@@ -1489,10 +1498,10 @@ func (a adminAPIHandlers) AccountInfoHandler(w http.ResponseWriter, r *http.Requ
 			return
 		}
 		policySlice := newMappedPolicy(policy).toSlice()
-		effectivePolicy = globalIAMSys.GetCombinedPolicy(policySlice...)
+		effectivePolicy, policyErr = globalIAMSys.GetCombinedPolicy(policySlice...)
 
 	case hasPolicyClaim:
-		effectivePolicy = globalIAMSys.GetCombinedPolicy(policySetFromClaims.ToSlice()...)
+		effectivePolicy, policyErr = globalIAMSys.GetCombinedPolicy(policySetFromClaims.ToSlice()...)
 
 	default:
 		policies, err := globalIAMSys.PolicyDBGet(accountName, cred.Groups...)
@@ -1501,7 +1510,11 @@ func (a adminAPIHandlers) AccountInfoHandler(w http.ResponseWriter, r *http.Requ
 			writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL)
 			return
 		}
-		effectivePolicy = globalIAMSys.GetCombinedPolicy(policies...)
+		effectivePolicy, policyErr = globalIAMSys.GetCombinedPolicy(policies...)
+	}
+	if policyErr != nil {
+		writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, policyErr), r.URL)
+		return
 	}
 
 	buf, err = json.MarshalIndent(effectivePolicy, "", " ")
@@ -2189,7 +2202,11 @@ func (a adminAPIHandlers) InfoAccessKey(w http.ResponseWriter, r *http.Request)
 			writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL)
 			return
 		}
-		svcAccountPolicy = globalIAMSys.GetCombinedPolicy(policiesNames...)
+		svcAccountPolicy, err = globalIAMSys.GetCombinedPolicy(policiesNames...)
+		if err != nil {
+			writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL)
+			return
+		}
 	}
 
 	policyJSON, err := json.MarshalIndent(svcAccountPolicy, "", " ")
@@ -2949,7 +2966,15 @@ func canManageRootCredentials(cred auth.Credentials) bool {
 func isAllowedServiceAccountAction(cred auth.Credentials, args policy.Args) bool {
 	if args.DenyOnly && !args.IsOwner && !cred.IsTemp() && !cred.IsServiceAccount() && newGlobalAuthZPluginFn() == nil {
 		policies, err := globalIAMSys.PolicyDBGet(args.AccountName, args.Groups...)
-		return err == nil && globalIAMSys.GetCombinedPolicy(policies...).IsAllowed(args)
+		if err != nil {
+			return false
+		}
+		combined, err := globalIAMSys.GetCombinedPolicy(policies...)
+		if err != nil {
+			iamLogIf(GlobalContext, err)
+			return false
+		}
+		return combined.IsAllowed(args)
 	}
 	return globalIAMSys.IsAllowed(args)
 }

cmd/iam-policy-merge_test.go

@@ -0,0 +1,171 @@
+// Copyright (c) 2026 PGSTY
+// SPDX-License-Identifier: AGPL-3.0-or-later
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+package cmd
+
+import (
+	"context"
+	"errors"
+	"net/http"
+	"testing"
+
+	"github.com/pgsty/silo-pkg/v3/policy"
+)
+
+func TestIAMPolicyMergeFailureDeniesAuthorization(t *testing.T) {
+	ctx, sys, _ := prepareIAMRevisionFixture(t)
+	const names = "merge-allow,merge-deny"
+	batchAuthzPolicy(ctx, t, sys, "merge-allow", `{"Effect":"Allow","Action":["s3:GetObject"],"Resource":["arn:aws:s3:::merge-bucket/*"]}`)
+	batchAuthzPolicy(ctx, t, sys, "merge-deny", `{"Effect":"Deny","Action":["s3:GetObject"],"Resource":["arn:aws:s3:::merge-bucket/protected"]},
+{"Effect":"Deny","Action":["admin:ListServiceAccounts"]}`)
+	cred := iamAdminUser(ctx, t, sys, "merge-user", names)
+	store := sys.store
+	original := store.IAMStorageAPI
+	t.Cleanup(func() { store.IAMStorageAPI = original })
+	evict := func() {
+		cache := store.lock()
+		delete(cache.iamPolicyDocsMap, "merge-deny")
+		store.unlock()
+	}
+	args := policy.Args{AccountName: cred.AccessKey, Action: policy.GetObjectAction, BucketName: "merge-bucket", ObjectName: "protected"}
+	checks := []struct {
+		name    string
+		allowed func() bool
+	}{
+		{"regular", func() bool { return sys.IsAllowed(args) }},
+		{"sts", func() bool { return sys.IsAllowedSTS(args, cred.AccessKey) }},
+		{"service-account", func() bool {
+			saArgs := args
+			saArgs.Claims = map[string]any{parentClaim: cred.AccessKey, iamPolicyClaimNameSA(): inheritedPolicyType}
+			return sys.IsAllowedServiceAccount(saArgs, cred.AccessKey)
+		}},
+		{"deny-only", func() bool {
+			return sys.doesPolicyAllow(names, policy.Args{DenyOnly: true, Action: policy.ListServiceAccountsAdminAction})
+		}},
+		{"self-service", func() bool {
+			return isAllowedServiceAccountAction(cred, policy.Args{AccountName: cred.AccessKey, DenyOnly: true, Action: policy.ListServiceAccountsAdminAction})
+		}},
+	}
+	for _, fault := range []struct {
+		name string
+		err  error
+	}{
+		{"quorum", errErasureReadQuorum},
+		{"timeout", context.DeadlineExceeded},
+		{"malformed", errors.New("invalid policy document")},
+	} {
+		t.Run(fault.name, func(t *testing.T) {
+			evict()
+			store.IAMStorageAPI = &batchPolicyOutage{IAMStorageAPI: original, failed: "merge-deny", err: fault.err}
+			for _, check := range checks {
+				t.Run(check.name, func(t *testing.T) {
+					if check.allowed() {
+						t.Fatal("policy load failure discarded Deny and granted authorization")
+					}
+				})
+			}
+		})
+	}
+	// A successful retry must enforce the loaded Deny and keep its unaffected
+	// Allow. Neither an outage nor a deny may permanently poison the cache.
+	store.IAMStorageAPI = original
+	for _, check := range checks {
+		if check.allowed() {
+			t.Errorf("%s discarded Deny after recovery", check.name)
+		}
+	}
+	args.ObjectName = "unprotected"
+	if !sys.IsAllowed(args) {
+		t.Fatal("unaffected permission did not recover")
+	}
+}
+
+func TestIAMPolicyMergeLoadResult(t *testing.T) {
+	ctx, sys, _ := prepareIAMRevisionFixture(t)
+	batchAuthzPolicy(ctx, t, sys, "merge-allow", `{"Effect":"Allow","Action":["s3:GetObject"],"Resource":["arn:aws:s3:::merge-bucket/*"]}`)
+	store := sys.store
+	original := store.IAMStorageAPI
+	t.Cleanup(func() { store.IAMStorageAPI = original })
+	for _, tc := range []struct {
+		name    string
+		loadErr error
+		wantErr bool
+	}{
+		{"confirmed-absent", errNoSuchPolicy, false},
+		{"unavailable", errErasureReadQuorum, true},
+		{"empty-success", nil, true},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			store.IAMStorageAPI = &batchPolicyOutage{IAMStorageAPI: original, failed: "merge-missing", err: tc.loadErr}
+			names, combined, err := store.MergePolicies("merge-allow,merge-missing")
+			if tc.wantErr {
+				if err == nil || names != "" || len(combined.Statements) != 0 {
+					t.Fatalf("incomplete policy selection returned a usable result: names=%q statements=%d err=%v", names, len(combined.Statements), err)
+				}
+				if tc.loadErr != nil && !errors.Is(err, tc.loadErr) {
+					t.Fatalf("lost policy load error: %v", err)
+				}
+				if _, err := sys.CurrentPolicies("merge-allow,merge-missing"); err == nil {
+					t.Fatal("STS policy selection hid a load failure")
+				}
+				if _, err := sys.GetCombinedPolicy("merge-allow", "merge-missing"); err == nil {
+					t.Fatal("combined policy hid a load failure")
+				}
+			} else {
+				mustIAM(t, err)
+				if names != "merge-allow" || !combined.IsAllowed(policy.Args{Action: policy.GetObjectAction, BucketName: "merge-bucket", ObjectName: "object"}) {
+					t.Fatal("confirmed absence changed the remaining policy")
+				}
+			}
+		})
+	}
+	store.IAMStorageAPI = original
+	// A regular user's self inspection without any policy must still work.
+	cred := iamAdminUser(ctx, t, sys, "merge-no-policy", "")
+	if !isAllowedServiceAccountAction(cred, policy.Args{AccountName: cred.AccessKey, DenyOnly: true, Action: policy.ListServiceAccountsAdminAction}) {
+		t.Fatal("a valid empty policy set was mistaken for an unavailable policy")
+	}
+}
+
+func TestIAMPolicyMergeInfoFailure(t *testing.T) {
+	ctx, sys := iamAdminFixture(t)
+	batchAuthzPolicy(ctx, t, sys, "merge-info-allow", `{"Effect":"Allow","Action":["s3:GetObject"],"Resource":["arn:aws:s3:::merge-bucket/*"]}`)
+	batchAuthzPolicy(ctx, t, sys, "merge-info-deny", `{"Effect":"Deny","Action":["s3:GetObject"],"Resource":["arn:aws:s3:::merge-bucket/protected"]}`)
+	parent := iamAdminUser(ctx, t, sys, "merge-info-user", "merge-info-allow,merge-info-deny")
+	svc := iamAdminServiceAccount(ctx, t, sys, parent.AccessKey, "merge-info-service", "")
+	store := sys.store
+	original := store.IAMStorageAPI
+	t.Cleanup(func() { store.IAMStorageAPI = original })
+	cache := store.lock()
+	delete(cache.iamPolicyDocsMap, "merge-info-deny")
+	store.unlock()
+	store.IAMStorageAPI = &batchPolicyOutage{IAMStorageAPI: original, failed: "merge-info-deny", err: errErasureReadQuorum}
+	api := adminAPIHandlers{}
+	for _, tc := range []struct {
+		name    string
+		handler http.HandlerFunc
+	}{
+		{"info-service-account", api.InfoServiceAccount},
+		{"info-access-key", api.InfoAccessKey},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			rec := iamAdminCall(t, globalActiveCred, http.MethodGet, "/minio/admin/v3/"+tc.name+"?accessKey="+svc.AccessKey, nil, map[string]string{"accessKey": svc.AccessKey}, tc.handler)
+			if rec.Code < http.StatusInternalServerError {
+				t.Fatalf("incomplete effective policy returned as success: %d %s", rec.Code, rec.Body.String())
+			}
+		})
+	}
+}

cmd/iam-store.go

@@ -1706,48 +1706,64 @@ func filterPolicies(cache *iamCache, policyName string, bucketName string) (stri
 	return strings.Join(policies, ","), policy.MergePolicies(toMerge...)
 }
 
-// MergePolicies - accepts a comma separated list of policy names as a string
-// and returns only policies that currently exist in MinIO. It includes hot loading
-// of policies if not in the memory
-func (store *IAMStoreSys) MergePolicies(policyName string) (string, policy.Policy) {
-	var policies []string
-	var missingPolicies []string
-	var toMerge []policy.Policy
-
+// MergePolicies loads and combines the named policies. Only confirmed absent
+// policies may be omitted: a failed read could hide an explicit Deny.
+func (store *IAMStoreSys) MergePolicies(policyName string) (string, policy.Policy, error) {
+	names := newMappedPolicy(policyName).toSlice()
+	allCached := true
 	cache := store.rlock()
-	for _, policy := range newMappedPolicy(policyName).toSlice() {
-		if policy == "" {
+	for _, name := range names {
+		if name == "" {
 			continue
 		}
-		p, found := cache.iamPolicyDocsMap[policy]
-		if !found {
-			missingPolicies = append(missingPolicies, policy)
-			continue
+		if _, found := cache.iamPolicyDocsMap[name]; !found {
+			allCached = false
+			break
 		}
-		policies = append(policies, policy)
-		toMerge = append(toMerge, p.Policy)
+	}
+	if allCached {
+		names, combined := filterPolicies(cache, policyName, "")
+		store.runlock()
+		return names, combined, nil
 	}
 	store.runlock()
 
-	if len(missingPolicies) > 0 {
-		m := make(map[string]PolicyDoc)
-		for _, policy := range missingPolicies {
+	// Re-read every document under the writer lock so a concurrent edit or
+	// deletion cannot be overwritten by a stale lazy load. Publish the loaded
+	// documents only when the whole selection succeeds.
+	cache = store.lock()
+	defer store.unlock()
+	var policies []string
+	var toMerge []policy.Policy
+	loaded := make(map[string]PolicyDoc)
+	for _, name := range names {
+		if name == "" {
+			continue
+		}
+		p, found := cache.iamPolicyDocsMap[name]
+		if !found {
 			ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
-			_ = store.loadPolicyDoc(ctx, policy, m)
+			err := store.loadPolicyDoc(ctx, name, loaded)
 			cancel()
+			if errors.Is(err, errNoSuchPolicy) {
+				continue
+			}
+			if err != nil {
+				return "", policy.Policy{}, fmt.Errorf("load IAM policy %q: %w", name, err)
+			}
+			p, found = loaded[name]
+			if !found {
+				return "", policy.Policy{}, fmt.Errorf("load IAM policy %q returned no document", name)
+			}
 		}
-
-		cache := store.lock()
-		maps.Copy(cache.iamPolicyDocsMap, m)
-		store.unlock()
-
-		for policy, p := range m {
-			policies = append(policies, policy)
-			toMerge = append(toMerge, p.Policy)
-		}
+		policies = append(policies, name)
+		toMerge = append(toMerge, p.Policy)
 	}
-
-	return strings.Join(policies, ","), policy.MergePolicies(toMerge...)
+	if len(loaded) > 0 {
+		maps.Copy(cache.iamPolicyDocsMap, loaded)
+		cache.updatedAt = time.Now()
+	}
+	return strings.Join(policies, ","), policy.MergePolicies(toMerge...), nil
 }
 
 // GetBucketUsers - returns users (not STS or service accounts) that have access

cmd/iam.go

@@ -763,13 +763,13 @@ func (sys *IAMSys) DeleteUser(ctx context.Context, accessKey string, notifyPeers
 // CurrentPolicies - returns comma separated policy string, from
 // an input policy after validating if there are any current
 // policies which exist on MinIO corresponding to the input.
-func (sys *IAMSys) CurrentPolicies(policyName string) string {
+func (sys *IAMSys) CurrentPolicies(policyName string) (string, error) {
 	if !sys.Initialized() {
-		return ""
+		return "", errServerNotInitialized
 	}
 
-	policies, _ := sys.store.MergePolicies(policyName)
-	return policies
+	policies, _, err := sys.store.MergePolicies(policyName)
+	return policies, err
 }
 
 func (sys *IAMSys) notifyForUser(ctx context.Context, accessKey string, isTemp bool) {
@@ -2304,7 +2304,11 @@ func (sys *IAMSys) IsAllowedServiceAccount(args policy.Args, parentUser string)
 	var combinedPolicy policy.Policy
 	// Policies were found, evaluate all of them.
 	if !isOwnerDerived {
-		availablePoliciesStr, c := sys.store.MergePolicies(strings.Join(svcPolicies, ","))
+		availablePoliciesStr, c, err := sys.store.MergePolicies(strings.Join(svcPolicies, ","))
+		if err != nil {
+			iamLogIf(GlobalContext, err)
+			return false
+		}
 		if availablePoliciesStr == "" {
 			return false
 		}
@@ -2397,7 +2401,11 @@ func (sys *IAMSys) IsAllowedSTS(args policy.Args, parentUser string) bool {
 	var combinedPolicy policy.Policy
 	// Policies were found, evaluate all of them.
 	if !isOwnerDerived {
-		availablePoliciesStr, c := sys.store.MergePolicies(strings.Join(policies, ","))
+		availablePoliciesStr, c, err := sys.store.MergePolicies(strings.Join(policies, ","))
+		if err != nil {
+			iamLogIf(GlobalContext, err)
+			return false
+		}
 		if availablePoliciesStr == "" {
 			// all policies presented in the claim should exist
 			iamLogIf(GlobalContext, fmt.Errorf("expected policy (%s) missing from the JWT claim %s, rejecting the request", policies, iamPolicyClaimNameOpenID()))
@@ -2527,16 +2535,21 @@ func isAllowedBySessionPolicy(args policy.Args) (hasSessionPolicy bool, isAllowe
 }
 
 // GetCombinedPolicy returns a combined policy combining all policies
-func (sys *IAMSys) GetCombinedPolicy(policies ...string) policy.Policy {
-	_, policy := sys.store.MergePolicies(strings.Join(policies, ","))
-	return policy
+func (sys *IAMSys) GetCombinedPolicy(policies ...string) (policy.Policy, error) {
+	_, combined, err := sys.store.MergePolicies(strings.Join(policies, ","))
+	return combined, err
 }
 
 // doesPolicyAllow - checks if the given policy allows the passed action with given args. This is rarely needed.
 // Notice there is no account name involved, so this is a dangerous function.
 func (sys *IAMSys) doesPolicyAllow(policy string, args policy.Args) bool {
 	// Policies were found, evaluate all of them.
-	return sys.GetCombinedPolicy(policy).IsAllowed(args)
+	combined, err := sys.GetCombinedPolicy(policy)
+	if err != nil {
+		iamLogIf(GlobalContext, err)
+		return false
+	}
+	return combined.IsAllowed(args)
 }
 
 // IsAllowed - checks given policy args is allowed to continue the Rest API.
@@ -2590,7 +2603,12 @@ func (sys *IAMSys) isAllowedLocally(args policy.Args) bool {
 	}
 
 	// Policies were found, evaluate all of them.
-	return sys.GetCombinedPolicy(policies...).IsAllowed(args)
+	combined, err := sys.GetCombinedPolicy(policies...)
+	if err != nil {
+		iamLogIf(GlobalContext, err)
+		return false
+	}
+	return combined.IsAllowed(args)
 }
 
 // SetUsersSysType - sets the users system type, regular or LDAP.

cmd/sts-handlers.go

@@ -797,7 +797,12 @@ func (sts *stsAPIHandlers) AssumeRoleWithSSO(w http.ResponseWriter, r *http.Requ
 		policySet, ok := policy.GetPoliciesFromClaims(claims, iamPolicyClaimNameOpenID())
 		policies := strings.Join(policySet.ToSlice(), ",")
 		if ok {
-			policyName = globalIAMSys.CurrentPolicies(policies)
+			current, err := globalIAMSys.CurrentPolicies(policies)
+			if err != nil {
+				writeSTSErrorResponse(ctx, w, ErrSTSInternalError, err)
+				return
+			}
+			policyName = current
 		}
 
 		if newGlobalAuthZPluginFn() == nil {
@@ -881,7 +886,12 @@ func (sts *stsAPIHandlers) AssumeRoleWithSSO(w http.ResponseWriter, r *http.Requ
 			}
 			if newGlobalAuthZPluginFn() == nil {
 				// if authZ is not set - we expect the policies to be present.
-				if globalIAMSys.CurrentPolicies(p) == "" {
+				current, err := globalIAMSys.CurrentPolicies(p)
+				if err != nil {
+					writeSTSErrorResponse(ctx, w, ErrSTSInternalError, err)
+					return
+				}
+				if current == "" {
 					writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue,
 						fmt.Errorf("None of the given policies (`%s`) are defined, credentials will not be generated", p))
 					return
@@ -1366,7 +1376,12 @@ func (sts *stsAPIHandlers) AssumeRoleWithCustomToken(w http.ResponseWriter, r *h
 	}
 
 	if newGlobalAuthZPluginFn() == nil { // if authZ is not set - we expect the policyname to be present.
-		if globalIAMSys.CurrentPolicies(policyName) == "" {
+		current, err := globalIAMSys.CurrentPolicies(policyName)
+		if err != nil {
+			writeSTSErrorResponse(ctx, w, ErrSTSInternalError, err)
+			return
+		}
+		if current == "" {
 			writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue,
 				fmt.Errorf("None of the given policies (`%s`) are defined, credentials will not be generated", policyName))
 			return