mirrors/seaweedfs
1
0
Fork 0
mirror of https://github.com/seaweedfs/seaweedfs.git synced 2026-08-22 07:06:51 +00:00
Code Issues Packages Projects Releases Wiki Activity

s3api: address object lock delete review comments

Browse Source 
- Remove versioningConfigured guard: object lock protections must apply to all buckets
- Combine identical switch cases for clarity
- Change default case to fail-closed (true) for safety
- Add test case for suspended versioning with specific versionId
- Update test expectations to reflect fail-closed default

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Chris Lu
2026-02-16 02:00:30 -08:00
co-authored by Copilot
parent 5e040ba03d
commit 3bd990d2ea
2 changed files with 25 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files
weed/s3api/s3api_object_handlers_delete.go
+16 -20
View File
@@ -28,14 +28,12 @@ func objectLockVersionToCheckForDelete(versioningState, requestedVersionID strin
}
switch versioningState {
case s3_constants.VersioningEnabled:
case s3_constants.VersioningEnabled, "":
return "", true
case s3_constants.VersioningSuspended:
return "null", true
case "":
return "", true
default:
return "", false
return "", true
}
}
@@ -253,22 +251,20 @@ func (s3a *S3ApiServer) DeleteMultipleObjectsHandler(w http.ResponseWriter, r *h
continue
}
// Check object lock permissions before deletion
if versioningConfigured {
lockCheckVersionID, shouldCheckObjectLock := objectLockVersionToCheckForDelete(versioningState, object.VersionId)
if shouldCheckObjectLock {
// Validate governance bypass for this specific object
governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, object.Key)
if err := s3a.enforceObjectLockProtections(r, bucket, object.Key, lockCheckVersionID, governanceBypassAllowed); err != nil {
glog.V(2).Infof("DeleteMultipleObjectsHandler: object lock check failed for %s/%s (version: %s): %v", bucket, object.Key, lockCheckVersionID, err)
deleteErrors = append(deleteErrors, DeleteError{
Code: s3err.GetAPIError(s3err.ErrAccessDenied).Code,
Message: s3err.GetAPIError(s3err.ErrAccessDenied).Description,
Key: object.Key,
VersionId: object.VersionId,
})
continue
}
// Check object lock permissions before deletion (applies to all buckets: versioned or non-versioned)
lockCheckVersionID, shouldCheckObjectLock := objectLockVersionToCheckForDelete(versioningState, object.VersionId)
if shouldCheckObjectLock {
// Validate governance bypass for this specific object
governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, object.Key)
if err := s3a.enforceObjectLockProtections(r, bucket, object.Key, lockCheckVersionID, governanceBypassAllowed); err != nil {
glog.V(2).Infof("DeleteMultipleObjectsHandler: object lock check failed for %s/%s (version: %s): %v", bucket, object.Key, lockCheckVersionID, err)
deleteErrors = append(deleteErrors, DeleteError{
Code: s3err.GetAPIError(s3err.ErrAccessDenied).Code,
Message: s3err.GetAPIError(s3err.ErrAccessDenied).Description,
Key: object.Key,
VersionId: object.VersionId,
})
continue
}
}
weed/s3api/s3api_object_handlers_delete_test.go
+9 -2
View File
@@ -44,11 +44,18 @@ func TestObjectLockVersionToCheckForDelete(t *testing.T) {
expectedShouldCheck: true,
},
{
name: "unknown versioning state without version id is skipped",
name: "unknown versioning state without version id is checked (fail-closed for safety)",
versioningState: "UnexpectedState",
requestedVersionID: "",
expectedVersionID: "",
expectedShouldCheck: false,
expectedShouldCheck: true,
},
{
name: "suspended versioning with specific version id checks that version",
versioningState: s3_constants.VersioningSuspended,
requestedVersionID: "abc123",
expectedVersionID: "abc123",
expectedShouldCheck: true,
},
}