fix tests

4 files changed, 38 insertions, 14 deletions

diff --git a/weed/s3api/s3api_object_handlers_delete.go b/weed/s3api/s3api_object_handlers_delete.go
index f06749f1d..72f484429 100644
--- a/weed/s3api/s3api_object_handlers_delete.go
+++ b/weed/s3api/s3api_object_handlers_delete.go
@@ -53,7 +53,7 @@ func (s3a *S3ApiServer) DeleteObjectHandler(w http.ResponseWriter, r *http.Reque
 		// Handle versioned delete
 		if versionId != "" {
 			// Check object lock permissions before deleting specific version
-			bypassGovernance := r.Header.Get("x-amz-bypass-governance-retention") == "true"
+			bypassGovernance := s3a.validateGovernanceBypass(r, bucket, object)
 			if err := s3a.checkObjectLockPermissions(r, bucket, object, versionId, bypassGovernance); err != nil {
 				glog.V(2).Infof("DeleteObjectHandler: object lock check failed for %s/%s: %v", bucket, object, err)
 				s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied)
@@ -71,7 +71,16 @@ func (s3a *S3ApiServer) DeleteObjectHandler(w http.ResponseWriter, r *http.Reque
 			// Set version ID in response header
 			w.Header().Set("x-amz-version-id", versionId)
 		} else {
-			// Create delete marker (logical delete) - this is NOT blocked by object lock retention
+			// Check object lock permissions before creating delete marker
+			// AWS S3 behavior: delete operations fail if latest version has retention protection
+			bypassGovernance := s3a.validateGovernanceBypass(r, bucket, object)
+			if err := s3a.checkObjectLockPermissions(r, bucket, object, "", bypassGovernance); err != nil {
+				glog.V(2).Infof("DeleteObjectHandler: object lock check failed for %s/%s: %v", bucket, object, err)
+				s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied)
+				return
+			}
+
+			// Create delete marker (logical delete)
 			deleteMarkerVersionId, err := s3a.createDeleteMarker(bucket, object)
 			if err != nil {
 				glog.Errorf("Failed to create delete marker: %v", err)
@@ -85,6 +94,14 @@ func (s3a *S3ApiServer) DeleteObjectHandler(w http.ResponseWriter, r *http.Reque
 		}
 	} else {
 		// Handle regular delete (non-versioned)
+		// Check object lock permissions before deleting object
+		bypassGovernance := s3a.validateGovernanceBypass(r, bucket, object)
+		if err := s3a.checkObjectLockPermissions(r, bucket, object, "", bypassGovernance); err != nil {
+			glog.V(2).Infof("DeleteObjectHandler: object lock check failed for %s/%s: %v", bucket, object, err)
+			s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied)
+			return
+		}
+
 		target := util.FullPath(fmt.Sprintf("%s/%s%s", s3a.option.BucketsPath, bucket, object))
 		dir, name := target.DirAndName()
 
@@ -191,9 +208,6 @@ func (s3a *S3ApiServer) DeleteMultipleObjectsHandler(w http.ResponseWriter, r *h
 		auditLog = s3err.GetAccessLog(r, http.StatusNoContent, s3err.ErrNone)
 	}
 
-	// Check for bypass governance retention header
-	bypassGovernance := r.Header.Get("x-amz-bypass-governance-retention") == "true"
-
 	// Check if versioning is enabled for the bucket (needed for object lock checks)
 	versioningEnabled, err := s3a.isVersioningEnabled(bucket)
 	if err != nil {
@@ -216,6 +230,8 @@ func (s3a *S3ApiServer) DeleteMultipleObjectsHandler(w http.ResponseWriter, r *h
 
 			// Check object lock permissions before deletion (only for versioned buckets)
 			if versioningEnabled {
+				// Validate governance bypass for this specific object
+				bypassGovernance := s3a.validateGovernanceBypass(r, bucket, object.Key)
 				if err := s3a.checkObjectLockPermissions(r, bucket, object.Key, object.VersionId, bypassGovernance); err != nil {
 					glog.V(2).Infof("DeleteMultipleObjectsHandler: object lock check failed for %s/%s (version: %s): %v", bucket, object.Key, object.VersionId, err)
 					deleteErrors = append(deleteErrors, DeleteError{
diff --git a/weed/s3api/s3api_object_handlers_put.go b/weed/s3api/s3api_object_handlers_put.go
index 8142113d8..408479f8b 100644
--- a/weed/s3api/s3api_object_handlers_put.go
+++ b/weed/s3api/s3api_object_handlers_put.go
@@ -119,7 +119,7 @@ func (s3a *S3ApiServer) PutObjectHandler(w http.ResponseWriter, r *http.Request)
 		// For non-versioned buckets, check if existing object has object lock protections
 		// that would prevent overwrite (PUT operations overwrite existing objects in non-versioned buckets)
 		if !versioningEnabled {
-			bypassGovernance := r.Header.Get("x-amz-bypass-governance-retention") == "true"
+			bypassGovernance := s3a.validateGovernanceBypass(r, bucket, object)
 			if err := s3a.checkObjectLockPermissions(r, bucket, object, "", bypassGovernance); err != nil {
 				glog.V(2).Infof("PutObjectHandler: object lock permissions check failed for %s/%s: %v", bucket, object, err)
 				s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied)
diff --git a/weed/s3api/s3api_object_handlers_retention.go b/weed/s3api/s3api_object_handlers_retention.go
index 0414ebbad..fb6d6e737 100644
--- a/weed/s3api/s3api_object_handlers_retention.go
+++ b/weed/s3api/s3api_object_handlers_retention.go
@@ -25,8 +25,8 @@ func (s3a *S3ApiServer) PutObjectRetentionHandler(w http.ResponseWriter, r *http
 	// Get version ID from query parameters
 	versionId := r.URL.Query().Get("versionId")
 
-	// Check for bypass governance retention header
-	bypassGovernance := r.Header.Get("x-amz-bypass-governance-retention") == "true"
+	// Validate governance bypass permission
+	bypassGovernance := s3a.validateGovernanceBypass(r, bucket, object)
 
 	// Parse retention configuration from request body
 	retention, err := parseObjectRetention(r)
diff --git a/weed/s3api/s3api_object_retention.go b/weed/s3api/s3api_object_retention.go
index b3b7635cc..273b6b641 100644
--- a/weed/s3api/s3api_object_retention.go
+++ b/weed/s3api/s3api_object_retention.go
@@ -627,6 +627,18 @@ func (s3a *S3ApiServer) checkGovernanceBypassPermission(request *http.Request, b
 	return false
 }
 
+// validateGovernanceBypass checks if the user has requested governance bypass via header
+// and validates they have the required s3:BypassGovernanceRetention permission.
+// This helper method consolidates the repetitive governance bypass validation logic
+// used across multiple handlers (DELETE, PUT, etc.).
+func (s3a *S3ApiServer) validateGovernanceBypass(r *http.Request, bucket, object string) bool {
+	// Check if governance bypass header is present
+	bypassRequested := r.Header.Get("x-amz-bypass-governance-retention") == "true"
+
+	// Only allow bypass if both header is present AND user has permission
+	return bypassRequested && s3a.checkGovernanceBypassPermission(r, bucket, object)
+}
+
 // checkObjectLockPermissions checks if an object can be deleted or modified
 func (s3a *S3ApiServer) checkObjectLockPermissions(request *http.Request, bucket, object, versionId string, bypassGovernance bool) error {
 	// Get the object entry to check both retention and legal hold
@@ -682,12 +694,8 @@ func (s3a *S3ApiServer) checkObjectLockPermissions(request *http.Request, bucket
 			if !bypassGovernance {
 				return ErrGovernanceModeActive
 			}
-
-			// If bypass is requested, check if user has permission
-			if !s3a.checkGovernanceBypassPermission(request, bucket, object) {
-				glog.V(2).Infof("User does not have s3:BypassGovernanceRetention permission for %s/%s", bucket, object)
-				return ErrGovernanceBypassNotPermitted
-			}
+			// Note: bypassGovernance parameter is already validated by validateGovernanceBypass()
+			// which checks both header presence and IAM permissions, so we trust it here
 		}
 	}