Address code review nitpicks

- Remove unused extractObjectTags placeholder function (engine.go)
- Add clarifying comment about s3:ExistingObjectTag/<key> evaluation
- Consolidate duplicate tag-based examples in README
- Factor out tagsToEntry helper to package level in tests

weed/s3api/policy_engine/README_POLICY_ENGINE.md

@@ -163,6 +163,33 @@ You can control access based on object tags using `s3:ExistingObjectTag/<tag-key
 
 This allows anonymous access only to objects that have a tag `status=public`.
 
+**Deny access to confidential objects:**
+
+```json
+{
+  "Version": "2012-10-17",
+  "Statement": [
+    {
+      "Effect": "Allow",
+      "Principal": "*",
+      "Action": "s3:GetObject",
+      "Resource": "arn:aws:s3:::my-bucket/*"
+    },
+    {
+      "Effect": "Deny",
+      "Principal": "*",
+      "Action": "s3:GetObject",
+      "Resource": "arn:aws:s3:::my-bucket/*",
+      "Condition": {
+        "StringEquals": {
+          "s3:ExistingObjectTag/classification": ["confidential", "secret"]
+        }
+      }
+    }
+  ]
+}
+```
+
 **Supported Operations for Tag-Based Conditions:**
 
 Tag-based conditions (`s3:ExistingObjectTag/<key>`) are evaluated for the following operations:
@@ -247,56 +274,6 @@ Note: For these conditions to be evaluated, the object must exist and the policy
 }
 ```
 
-### Tag-Based Access Control
-
-Allow public read only for objects tagged as public:
-
-```json
-{
-  "Version": "2012-10-17",
-  "Statement": [
-    {
-      "Effect": "Allow",
-      "Principal": "*",
-      "Action": "s3:GetObject",
-      "Resource": "arn:aws:s3:::my-bucket/*",
-      "Condition": {
-        "StringEquals": {
-          "s3:ExistingObjectTag/visibility": ["public"]
-        }
-      }
-    }
-  ]
-}
-```
-
-Deny access to confidential objects:
-
-```json
-{
-  "Version": "2012-10-17",
-  "Statement": [
-    {
-      "Effect": "Allow",
-      "Principal": "*",
-      "Action": "s3:GetObject",
-      "Resource": "arn:aws:s3:::my-bucket/*"
-    },
-    {
-      "Effect": "Deny",
-      "Principal": "*",
-      "Action": "s3:GetObject",
-      "Resource": "arn:aws:s3:::my-bucket/*",
-      "Condition": {
-        "StringEquals": {
-          "s3:ExistingObjectTag/classification": ["confidential", "secret"]
-        }
-      }
-    }
-  ]
-}
-```
-
 ## Integration
 
 ### For Existing SeaweedFS Users

weed/s3api/policy_engine/engine.go

@@ -209,10 +209,8 @@ func ExtractConditionValuesFromRequest(r *http.Request) map[string][]string {
 		values["aws:Referer"] = []string{referer}
 	}
 
-	// S3 object-level conditions
-	if r.Method == "GET" || r.Method == "HEAD" {
-		values["s3:ExistingObjectTag"] = extractObjectTags(r)
-	}
+	// Note: s3:ExistingObjectTag/<key> conditions are evaluated using objectEntry
+	// passed to EvaluatePolicy, not extracted from the request.
 
 	// S3 bucket-level conditions
 	if delimiter := r.URL.Query().Get("delimiter"); delimiter != "" {
@@ -251,13 +249,6 @@ func ExtractConditionValuesFromRequest(r *http.Request) map[string][]string {
 	return values
 }
 
-// extractObjectTags extracts object tags from request (placeholder implementation)
-func extractObjectTags(r *http.Request) []string {
-	// This would need to be implemented based on how object tags are stored
-	// For now, return empty slice
-	return []string{}
-}
-
 // BuildResourceArn builds an ARN for the given bucket and object
 func BuildResourceArn(bucketName, objectName string) string {
 	if objectName == "" {

weed/s3api/policy_engine/engine_test.go

@@ -9,6 +9,19 @@ import (
 	"github.com/seaweedfs/seaweedfs/weed/s3api/s3err"
 )
 
+// tagsToEntry converts a map of tag key-value pairs to the entry.Extended format
+// used for s3:ExistingObjectTag/<key> condition evaluation
+func tagsToEntry(tags map[string]string) map[string][]byte {
+	if tags == nil {
+		return nil
+	}
+	entry := make(map[string][]byte)
+	for k, v := range tags {
+		entry[s3_constants.AmzObjectTaggingPrefix+k] = []byte(v)
+	}
+	return entry
+}
+
 func TestPolicyEngine(t *testing.T) {
 	engine := NewPolicyEngine()
 
@@ -743,18 +756,6 @@ func TestExistingObjectTagCondition(t *testing.T) {
 		t.Fatalf("Failed to set bucket policy: %v", err)
 	}
 
-	// Helper to convert tags to entry.Extended format
-	tagsToEntry := func(tags map[string]string) map[string][]byte {
-		if tags == nil {
-			return nil
-		}
-		entry := make(map[string][]byte)
-		for k, v := range tags {
-			entry[s3_constants.AmzObjectTaggingPrefix+k] = []byte(v)
-		}
-		return entry
-	}
-
 	tests := []struct {
 		name       string
 		objectTags map[string]string
@@ -837,15 +838,6 @@ func TestExistingObjectTagConditionMultipleTags(t *testing.T) {
 		t.Fatalf("Failed to set bucket policy: %v", err)
 	}
 
-	// Helper to convert tags to entry.Extended format
-	tagsToEntry := func(tags map[string]string) map[string][]byte {
-		entry := make(map[string][]byte)
-		for k, v := range tags {
-			entry[s3_constants.AmzObjectTaggingPrefix+k] = []byte(v)
-		}
-		return entry
-	}
-
 	tests := []struct {
 		name       string
 		objectTags map[string]string
@@ -928,18 +920,6 @@ func TestExistingObjectTagDenyPolicy(t *testing.T) {
 		t.Fatalf("Failed to set bucket policy: %v", err)
 	}
 
-	// Helper to convert tags to entry.Extended format
-	tagsToEntry := func(tags map[string]string) map[string][]byte {
-		if tags == nil {
-			return nil
-		}
-		entry := make(map[string][]byte)
-		for k, v := range tags {
-			entry[s3_constants.AmzObjectTaggingPrefix+k] = []byte(v)
-		}
-		return entry
-	}
-
 	tests := []struct {
 		name       string
 		objectTags map[string]string