From 5a3eaedbf42363bf73b6263a759fde9a40738c8b Mon Sep 17 00:00:00 2001
From: Artem Muterko <artem@sopho.tech>
Date: Wed, 3 Jun 2026 00:26:42 +0200
Subject: [PATCH] object: clobber bucket policy on modify instead of merging

ModifyBucketPolicy merged the caller's statement into the policy fetched
from the bucket, matching by SID. Besides a missing match flag that
appended a duplicate when a SID matched, this preserved any pre-existing
statements on the bucket and reapplied them on every reconcile.

Overwrite the policy with the provided statements instead, so a managed
bucket always ends up with exactly the intended policy and cannot retain
unexpected statements.

Signed-off-by: Artem Muterko <artem@sopho.tech>
(cherry picked from commit 321911b96cd3fcf6639b9e543a05144d66b52a32)
---
 pkg/operator/ceph/object/policy.go      | 17 ++------
 pkg/operator/ceph/object/policy_test.go | 53 +++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 14 deletions(-)
 create mode 100644 pkg/operator/ceph/object/policy_test.go

diff --git a/pkg/operator/ceph/object/policy.go b/pkg/operator/ceph/object/policy.go
index 0f278b414..48299cb44 100644
--- a/pkg/operator/ceph/object/policy.go
+++ b/pkg/operator/ceph/object/policy.go
@@ -194,21 +194,10 @@ func (s *S3Agent) GetBucketPolicy(bucket string) (*BucketPolicy, error) {
 	return policy, nil
 }
 
-// ModifyBucketPolicy new and old statement SIDs and overwrites on a match.
-// This allows users to Get, modify, and Replace existing statements as well as
-// add new ones.
+// ModifyBucketPolicy replaces the policy's statements with the given ones,
+// rather than merging them into the existing statements by SID.
 func (bp *BucketPolicy) ModifyBucketPolicy(ps ...PolicyStatement) *BucketPolicy {
-	for _, newP := range ps {
-		var match bool
-		for j, oldP := range bp.Statement {
-			if newP.Sid == oldP.Sid {
-				bp.Statement[j] = newP
-			}
-		}
-		if !match {
-			bp.Statement = append(bp.Statement, newP)
-		}
-	}
+	bp.Statement = append([]PolicyStatement{}, ps...)
 	return bp
 }
 
diff --git a/pkg/operator/ceph/object/policy_test.go b/pkg/operator/ceph/object/policy_test.go
new file mode 100644
index 000000000..5d78d092e
--- /dev/null
+++ b/pkg/operator/ceph/object/policy_test.go
@@ -0,0 +1,53 @@
+/*
+Copyright 2026 The Rook Authors. All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+	http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package object
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestModifyBucketPolicy(t *testing.T) {
+	t.Run("does not duplicate an existing SID", func(t *testing.T) {
+		bp := NewBucketPolicy(*NewPolicyStatement().WithSID("s1").Allows().Actions(GetObject))
+		bp.ModifyBucketPolicy(*NewPolicyStatement().WithSID("s1").Denies().Actions(PutObject))
+		assert.Len(t, bp.Statement, 1)
+		assert.Equal(t, effectDeny, bp.Statement[0].Effect)
+	})
+
+	t.Run("replaces existing statements instead of merging", func(t *testing.T) {
+		bp := NewBucketPolicy(
+			*NewPolicyStatement().WithSID("s1").Allows().Actions(GetObject),
+			*NewPolicyStatement().WithSID("s2").Allows().Actions(GetObject),
+		)
+		bp.ModifyBucketPolicy(*NewPolicyStatement().WithSID("s3").Denies().Actions(PutObject))
+		assert.Len(t, bp.Statement, 1)
+		assert.Equal(t, "s3", bp.Statement[0].Sid)
+	})
+
+	t.Run("keeps all passed statements in order", func(t *testing.T) {
+		bp := NewBucketPolicy(*NewPolicyStatement().WithSID("old").Allows().Actions(GetObject))
+		bp.ModifyBucketPolicy(
+			*NewPolicyStatement().WithSID("a").Allows().Actions(GetObject),
+			*NewPolicyStatement().WithSID("b").Denies().Actions(PutObject),
+		)
+		assert.Len(t, bp.Statement, 2)
+		assert.Equal(t, "a", bp.Statement[0].Sid)
+		assert.Equal(t, "b", bp.Statement[1].Sid)
+	})
+}