mirrors/seaweedfs
1
0
Fork 0
mirror of https://github.com/seaweedfs/seaweedfs.git synced 2026-09-03 14:47:04 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix object tagging writing back to the wrong object for nested keys (#10338)

Browse Source 
Put/DeleteObjectTagging set the update directory to the bucket root for a
null version, so a tag change on allowed/protected.txt landed on
protected.txt at the bucket root instead. A principal scoped to one
nested key could overwrite a different object sharing the basename, the
same class of scope bypass fixed for PutObjectAcl. Share the object's
parent-directory resolver across both paths.
This commit is contained in:
Chris Lu
2026-07-15 02:31:51 -07:00
committed by GitHub
parent 8f29d0a91e
commit 0ad83d5061
3 changed files with 22 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files
weed/s3api/s3api_object_handlers_acl.go
+6 -6
View File
@@ -269,7 +269,7 @@ func (s3a *S3ApiServer) PutObjectAclHandler(w http.ResponseWriter, r *http.Reque
return
}
updateDirectory := s3a.objectAclUpdateDirectory(bucket, object, versioningConfigured, versionId, entry)
updateDirectory := s3a.objectMetadataUpdateDirectory(bucket, object, versioningConfigured, versionId, entry)
// Update the object with new ACL metadata
err = s3a.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
@@ -294,11 +294,11 @@ func (s3a *S3ApiServer) PutObjectAclHandler(w http.ResponseWriter, r *http.Reque
writeSuccessResponseEmpty(w, r)
}
// objectAclUpdateDirectory returns the filer directory holding the entry a PutObjectAcl
// update must target. A regular object lives under its full key's parent directory, so a
// nested key must not fall back to the bucket root, which would rewrite a different object
// sharing the same basename.
func (s3a *S3ApiServer) objectAclUpdateDirectory(bucket, object string, versioningConfigured bool, versionId string, entry *filer_pb.Entry) string {
// objectMetadataUpdateDirectory returns the filer directory holding the entry an in-place
// metadata update (ACL or tags) must target. A regular object lives under its full key's
// parent directory, so a nested key must not fall back to the bucket root, which would
// rewrite a different object sharing the same basename.
func (s3a *S3ApiServer) objectMetadataUpdateDirectory(bucket, object string, versioningConfigured bool, versionId string, entry *filer_pb.Entry) string {
if versioningConfigured {
if versionId != "" && versionId != "null" {
return s3a.bucketDir(bucket) + "/" + object + s3_constants.VersionsFolder
weed/s3api/s3api_object_handlers_acl_test.go
+8 -6
View File
@@ -7,10 +7,11 @@ import (
"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
)
// TestObjectAclUpdateDirectory guards against a PutObjectAcl scope bypass: a nested object
// key must resolve to its own parent directory, not the bucket root, otherwise updating the
// ACL of allowed/protected.txt would rewrite a different protected.txt at the bucket root.
func TestObjectAclUpdateDirectory(t *testing.T) {
// TestObjectMetadataUpdateDirectory guards against a metadata-update scope bypass shared by
// PutObjectAcl and Put/DeleteObjectTagging: a nested object key must resolve to its own
// parent directory, not the bucket root, otherwise updating the ACL or tags of
// allowed/protected.txt would rewrite a different protected.txt at the bucket root.
func TestObjectMetadataUpdateDirectory(t *testing.T) {
s3a := &S3ApiServer{option: &S3ApiServerOption{BucketsPath: "/buckets"}}
const bucket = "target-bucket"
bucketDir := "/buckets/target-bucket"
@@ -31,6 +32,7 @@ func TestObjectAclUpdateDirectory(t *testing.T) {
{"non-versioned root key", "protected.txt", false, "", &filer_pb.Entry{}, bucketDir},
{"non-versioned deep key", "a/b/c/protected.txt", false, "", &filer_pb.Entry{}, bucketDir + "/a/b/c"},
{"null version nested key", "allowed/protected.txt", true, "", versioned("null"), bucketDir + "/allowed"},
{"explicit null version nested key", "allowed/protected.txt", true, "null", versioned("null"), bucketDir + "/allowed"},
{"empty version nested key", "allowed/protected.txt", true, "", &filer_pb.Entry{}, bucketDir + "/allowed"},
{"specific version nested key", "allowed/protected.txt", true, "v1", versioned("v1"), bucketDir + "/allowed/protected.txt" + s3_constants.VersionsFolder},
{"latest versioned nested key", "allowed/protected.txt", true, "", versioned("v1"), bucketDir + "/allowed/protected.txt" + s3_constants.VersionsFolder},
@@ -38,9 +40,9 @@ func TestObjectAclUpdateDirectory(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := s3a.objectAclUpdateDirectory(bucket, tt.object, tt.versioningConfigured, tt.versionId, tt.entry)
got := s3a.objectMetadataUpdateDirectory(bucket, tt.object, tt.versioningConfigured, tt.versionId, tt.entry)
if got != tt.want {
t.Errorf("objectAclUpdateDirectory(%q) = %q, want %q", tt.object, got, tt.want)
t.Errorf("objectMetadataUpdateDirectory(%q) = %q, want %q", tt.object, got, tt.want)
}
})
}
weed/s3api/s3api_object_handlers_tagging.go
+8 -58
View File
@@ -199,35 +199,10 @@ func (s3a *S3ApiServer) PutObjectTaggingHandler(w http.ResponseWriter, r *http.R
return
}
// For versioned objects, determine the correct directory based on the version
var updateDirectory string
if versionId != "" {
// Specific version requested
if versionId == "null" {
// Null version (pre-versioning object) - stored as regular file
updateDirectory = s3a.bucketDir(bucket)
} else {
// Versioned object - stored in .versions directory
updateDirectory = s3a.bucketDir(bucket) + "/" + object + s3_constants.VersionsFolder
}
} else {
// Latest version in versioned bucket - could be null version or versioned object
// Extract version ID from the entry to determine where it's stored
var actualVersionId string
if entry.Extended != nil {
if versionIdBytes, exists := entry.Extended[s3_constants.ExtVersionIdKey]; exists {
actualVersionId = string(versionIdBytes)
}
}
if actualVersionId == "null" || actualVersionId == "" {
// Null version (pre-versioning object) - stored as regular file
updateDirectory = s3a.bucketDir(bucket)
} else {
// Versioned object - stored in .versions directory
updateDirectory = s3a.bucketDir(bucket) + "/" + object + s3_constants.VersionsFolder
}
}
// For versioned objects, resolve the directory holding the entry to update. A null
// version lives under its full key's parent directory, so a nested key must not fall
// back to the bucket root and rewrite a different object sharing the same basename.
updateDirectory := s3a.objectMetadataUpdateDirectory(bucket, object, versioningConfigured, versionId, entry)
// Remove old tags and add new ones
for k := range entry.Extended {
@@ -343,35 +318,10 @@ func (s3a *S3ApiServer) DeleteObjectTaggingHandler(w http.ResponseWriter, r *htt
return
}
// For versioned objects, determine the correct directory based on the version
var updateDirectory string
if versionId != "" {
// Specific version requested
if versionId == "null" {
// Null version (pre-versioning object) - stored as regular file
updateDirectory = s3a.bucketDir(bucket)
} else {
// Versioned object - stored in .versions directory
updateDirectory = s3a.bucketDir(bucket) + "/" + object + s3_constants.VersionsFolder
}
} else {
// Latest version in versioned bucket - could be null version or versioned object
// Extract version ID from the entry to determine where it's stored
var actualVersionId string
if entry.Extended != nil {
if versionIdBytes, exists := entry.Extended[s3_constants.ExtVersionIdKey]; exists {
actualVersionId = string(versionIdBytes)
}
}
if actualVersionId == "null" || actualVersionId == "" {
// Null version (pre-versioning object) - stored as regular file
updateDirectory = s3a.bucketDir(bucket)
} else {
// Versioned object - stored in .versions directory
updateDirectory = s3a.bucketDir(bucket) + "/" + object + s3_constants.VersionsFolder
}
}
// For versioned objects, resolve the directory holding the entry to update. A null
// version lives under its full key's parent directory, so a nested key must not fall
// back to the bucket root and rewrite a different object sharing the same basename.
updateDirectory := s3a.objectMetadataUpdateDirectory(bucket, object, versioningConfigured, versionId, entry)
// Remove all tags
hasDeletion := false