From 8eee6b2a0e639a68667bc6638037a978d1f64fc0 Mon Sep 17 00:00:00 2001
From: Chris Lu <chris.lu@gmail.com>
Date: Wed, 28 Jan 2026 17:02:59 -0800
Subject: [PATCH] s3tables: Fix bucket policy error handling in permission
 checks

Replace error-swallowing pattern where all errors from getExtendedAttribute
were ignored for bucket policy reads. Now properly distinguish between:

- ErrAttributeNotFound: Policy not found is expected; continue with empty policy
- Other errors: Return internal server error and stop processing

Applied fix to all bucket policy reads in:
- handleDeleteTableBucketPolicy (line 220)
- handleTagResource (line 313)
- handleUntagResource (line 405)
- handleListTagsForResource (line 488)
- And additional occurrences in closures

This prevents silent failures and ensures policy-related errors are surfaced
to callers rather than being silently ignored.
---
 weed/s3api/s3tables/handler_policy.go | 49 +++++++++++++++++++++++----
 1 file changed, 42 insertions(+), 7 deletions(-)

diff --git a/weed/s3api/s3tables/handler_policy.go b/weed/s3api/s3tables/handler_policy.go
index 5d822a68b..93fb3673f 100644
--- a/weed/s3api/s3tables/handler_policy.go
+++ b/weed/s3api/s3tables/handler_policy.go
@@ -218,7 +218,12 @@ func (h *S3TablesHandler) handleDeleteTableBucketPolicy(w http.ResponseWriter, r
 
 		// Fetch bucket policy if it exists
 		policyData, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy)
-		if err == nil {
+		if err != nil {
+			if !errors.Is(err, ErrAttributeNotFound) {
+				return fmt.Errorf("failed to read bucket policy: %w", err)
+			}
+			// Policy not found is not an error; bucketPolicy remains empty
+		} else {
 			bucketPolicy = string(policyData)
 		}
 
@@ -306,7 +311,12 @@ func (h *S3TablesHandler) handlePutTablePolicy(w http.ResponseWriter, r *http.Re
 
 		// Fetch bucket policy if it exists
 		policyData, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy)
-		if err == nil {
+		if err != nil {
+			if !errors.Is(err, ErrAttributeNotFound) {
+				return fmt.Errorf("failed to read bucket policy: %w", err)
+			}
+			// Policy not found is not an error; bucketPolicy remains empty
+		} else {
 			bucketPolicy = string(policyData)
 		}
 
@@ -398,7 +408,12 @@ func (h *S3TablesHandler) handleGetTablePolicy(w http.ResponseWriter, r *http.Re
 
 		// Fetch bucket policy if it exists
 		policyData, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy)
-		if err == nil {
+		if err != nil {
+			if !errors.Is(err, ErrAttributeNotFound) {
+				return fmt.Errorf("failed to read bucket policy: %w", err)
+			}
+			// Policy not found is not an error; bucketPolicy remains empty
+		} else {
 			bucketPolicy = string(policyData)
 		}
 
@@ -481,7 +496,12 @@ func (h *S3TablesHandler) handleDeleteTablePolicy(w http.ResponseWriter, r *http
 
 		// Fetch bucket policy if it exists
 		policyData, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy)
-		if err == nil {
+		if err != nil {
+			if !errors.Is(err, ErrAttributeNotFound) {
+				return fmt.Errorf("failed to read bucket policy: %w", err)
+			}
+			// Policy not found is not an error; bucketPolicy remains empty
+		} else {
 			bucketPolicy = string(policyData)
 		}
 
@@ -560,7 +580,12 @@ func (h *S3TablesHandler) handleTagResource(w http.ResponseWriter, r *http.Reque
 		if bucketName != "" {
 			bucketPath := getTableBucketPath(bucketName)
 			policyData, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy)
-			if err == nil {
+			if err != nil {
+				if !errors.Is(err, ErrAttributeNotFound) {
+					return fmt.Errorf("failed to read bucket policy: %w", err)
+				}
+				// Policy not found is not an error; bucketPolicy remains empty
+			} else {
 				bucketPolicy = string(policyData)
 			}
 		}
@@ -659,7 +684,12 @@ func (h *S3TablesHandler) handleListTagsForResource(w http.ResponseWriter, r *ht
 		if bucketName != "" {
 			bucketPath := getTableBucketPath(bucketName)
 			policyData, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy)
-			if err == nil {
+			if err != nil {
+				if !errors.Is(err, ErrAttributeNotFound) {
+					return fmt.Errorf("failed to read bucket policy: %w", err)
+				}
+				// Policy not found is not an error; bucketPolicy remains empty
+			} else {
 				bucketPolicy = string(policyData)
 			}
 		}
@@ -746,7 +776,12 @@ func (h *S3TablesHandler) handleUntagResource(w http.ResponseWriter, r *http.Req
 		if bucketName != "" {
 			bucketPath := getTableBucketPath(bucketName)
 			policyData, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy)
-			if err == nil {
+			if err != nil {
+				if !errors.Is(err, ErrAttributeNotFound) {
+					return fmt.Errorf("failed to read bucket policy: %w", err)
+				}
+				// Policy not found is not an error; bucketPolicy remains empty
+			} else {
 				bucketPolicy = string(policyData)
 			}
 		}