mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-10-13 09:38:36 +02:00
s3tables: check data-path ancestors for catalog markers on DeleteTable
The decoupled-delete guard only inspects the resolved data path itself, but the catalog marker lives on a table's name path, so a location that resolves inside another table's directory (its marker-less data subdirectory) slips through and the recursive purge destroys the other table's data. Walk the data path's ancestors within the bucket and refuse when any carries a table or view catalog entry. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
6f93f87446
commit
85f04b54fd
2 files changed
+69
No files matched your search
@@ -3,6 +3,7 @@ package s3tables
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"path"
|
||||
"testing"
|
||||
|
||||
"github.com/seaweedfs/seaweedfs/weed/s3api/s3tables/s3tablestest"
|
||||
@@ -120,3 +121,50 @@ func TestDeleteTableRefusesLiveSiblingDataPath(t *testing.T) {
|
||||
assert.NotNil(t, fs.Get(GetTablePath(renameTestBucket, "ns", "t"), "data"),
|
||||
"the targeted table's data must survive")
|
||||
}
|
||||
|
||||
// A location inside another table's directory carries no catalog marker of its
|
||||
// own — the marker lives on the name path. Dropping a table whose
|
||||
// MetadataLocation resolves into nsa/vtab/data must be refused by the ancestor
|
||||
// check, or the victim's marker-less data subdirectory is purged wholesale.
|
||||
func TestDeleteTableRefusesNestedSiblingDataPath(t *testing.T) {
|
||||
fs, m := startRenameManager(t)
|
||||
|
||||
nsaMeta, _ := json.Marshal(namespaceMetadata{Namespace: []string{"nsa"}, OwnerAccountID: DefaultAccountID})
|
||||
fs.Put(GetTableBucketPath(renameTestBucket), "nsa", map[string][]byte{ExtendedKeyMetadata: nsaMeta})
|
||||
|
||||
victimMeta, _ := json.Marshal(tableMetadataInternal{
|
||||
Name: "vtab",
|
||||
Namespace: "nsa",
|
||||
Format: "ICEBERG",
|
||||
OwnerAccountID: DefaultAccountID,
|
||||
MetadataLocation: "s3://" + renameTestBucket + "/nsa/vtab/metadata/v1.metadata.json",
|
||||
})
|
||||
victimTablePath := GetTablePath(renameTestBucket, "nsa", "vtab")
|
||||
fs.Put(GetNamespacePath(renameTestBucket, "nsa"), "vtab", map[string][]byte{
|
||||
ExtendedKeyMetadata: victimMeta,
|
||||
ExtendedKeyEntryType: []byte(EntryTypeTable),
|
||||
})
|
||||
fs.Put(victimTablePath, "data", nil)
|
||||
fs.Put(path.Join(victimTablePath, "data"), "important-0001.parquet", nil)
|
||||
|
||||
evilMeta, _ := json.Marshal(tableMetadataInternal{
|
||||
Name: "evil",
|
||||
Namespace: "attackerns2",
|
||||
Format: "ICEBERG",
|
||||
OwnerAccountID: DefaultAccountID,
|
||||
MetadataLocation: "s3://" + renameTestBucket + "/nsa/vtab/data/metadata/v1.metadata.json",
|
||||
})
|
||||
nsMeta, _ := json.Marshal(namespaceMetadata{Namespace: []string{"attackerns2"}, OwnerAccountID: DefaultAccountID})
|
||||
fs.Put(GetTableBucketPath(renameTestBucket), "attackerns2", map[string][]byte{ExtendedKeyMetadata: nsMeta})
|
||||
fs.Put(GetNamespacePath(renameTestBucket, "attackerns2"), "evil", map[string][]byte{
|
||||
ExtendedKeyMetadata: evilMeta,
|
||||
ExtendedKeyEntryType: []byte(EntryTypeTable),
|
||||
})
|
||||
|
||||
require.Error(t, runDeleteTable(t, m, fs, "attackerns2", "evil"))
|
||||
|
||||
assert.NotNil(t, fs.Get(path.Join(victimTablePath, "data"), "important-0001.parquet"),
|
||||
"data inside another table's directory must survive")
|
||||
assert.NotNil(t, fs.Get(GetNamespacePath(renameTestBucket, "nsa"), "vtab"),
|
||||
"the victim table's catalog entry must survive")
|
||||
}
|
||||
@@ -8,6 +8,7 @@ import (
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"path"
|
||||
"strconv"
|
||||
"strings"
|
||||
"time"
|
||||
@@ -996,6 +997,26 @@ func (h *S3TablesHandler) handleDeleteTable(w http.ResponseWriter, r *http.Reque
|
||||
// still a live catalog entry. Only this table's authorization was
|
||||
// checked; a decoupled location has had its catalog attributes
|
||||
// stripped, so a surviving marker means the path belongs elsewhere.
|
||||
// The marker lives on the entry's name path, so ancestors of the
|
||||
// data path are checked as well: a location inside another table's
|
||||
// directory resolves above any marker the data path itself would
|
||||
// carry.
|
||||
for dir := path.Dir(dataPath); dir != GetTableBucketPath(bucketName) && strings.HasPrefix(dir, GetTableBucketPath(bucketName)+"/"); dir = path.Dir(dir) {
|
||||
ancestor, lookupErr := h.lookupEntry(r.Context(), client, dir)
|
||||
if lookupErr != nil {
|
||||
if errors.Is(lookupErr, filer_pb.ErrNotFound) {
|
||||
continue
|
||||
}
|
||||
return fmt.Errorf("refusing to delete table %s: cannot read data path %q: %w", tableName, dataPath, lookupErr)
|
||||
}
|
||||
if _, ok := ancestor.Extended[ExtendedKeyEntryType]; ok {
|
||||
return fmt.Errorf("refusing to delete table %s: data path %q is inside another catalog entry", tableName, dataPath)
|
||||
}
|
||||
var ownerMeta tableMetadataInternal
|
||||
if data := ancestor.Extended[ExtendedKeyMetadata]; json.Unmarshal(data, &ownerMeta) == nil && ownerMeta.Name != "" {
|
||||
return fmt.Errorf("refusing to delete table %s: data path %q is inside another catalog entry", tableName, dataPath)
|
||||
}
|
||||
}
|
||||
switch _, markerErr := h.getExtendedAttribute(r.Context(), client, dataPath, ExtendedKeyMetadata); {
|
||||
case markerErr == nil:
|
||||
return fmt.Errorf("refusing to delete table %s: data path %q is another catalog entry", tableName, dataPath)
|
||||
|
||||
Reference in new issue
Block a user