s3tables: confine caller-supplied metadataLocation to the table's namespace

CreateTable/RegisterTable/UpdateTable (and the view equivalents) validate
the location only for bucket and traversal, so a tenant can register a
table whose location resolves into another namespace's directory and
DeleteTable will purge it. Require the location to live under the
caller's namespace subtree, the boundary per-namespace delegation uses.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
Chris LuandDevin committed 2026-10-12 02:41:28 -07:00
1 parent 73949daa98
commit 6f93f87446
4 files changed
+40 -28

No files matched your search

+3 -3
View File
@@ -100,7 +100,7 @@ func (h *S3TablesHandler) handleCreateTable(w http.ResponseWriter, r *http.Reque
return err return err
} }
if err := ValidateMetadataLocation(req.MetadataLocation, bucketName); err != nil { if err := ValidateMetadataLocation(req.MetadataLocation, bucketName, namespaceName); err != nil {
h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error()) h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error())
return err return err
} }
@@ -233,7 +233,7 @@ func (h *S3TablesHandler) handleRegisterTable(w http.ResponseWriter, r *http.Req
} }
bucketName, namespaceName, tableName := target.bucketName, target.namespaceName, target.tableName bucketName, namespaceName, tableName := target.bucketName, target.namespaceName, target.tableName
if err := ValidateMetadataLocation(req.MetadataLocation, bucketName); err != nil { if err := ValidateMetadataLocation(req.MetadataLocation, bucketName, namespaceName); err != nil {
h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error()) h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error())
return err return err
} }
@@ -1494,7 +1494,7 @@ func (h *S3TablesHandler) handleUpdateTable(w http.ResponseWriter, r *http.Reque
return err return err
} }
if err := ValidateMetadataLocation(req.MetadataLocation, bucketName); err != nil { if err := ValidateMetadataLocation(req.MetadataLocation, bucketName, namespaceName); err != nil {
h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error()) h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error())
return err return err
} }
+2 -2
View File
@@ -121,7 +121,7 @@ func (h *S3TablesHandler) handleCreateView(w http.ResponseWriter, r *http.Reques
return err return err
} }
if err := ValidateMetadataLocation(req.MetadataLocation, bucketName); err != nil { if err := ValidateMetadataLocation(req.MetadataLocation, bucketName, namespaceName); err != nil {
h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error()) h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error())
return err return err
} }
@@ -410,7 +410,7 @@ func (h *S3TablesHandler) handleUpdateView(w http.ResponseWriter, r *http.Reques
return err return err
} }
if err := ValidateMetadataLocation(req.MetadataLocation, bucketName); err != nil { if err := ValidateMetadataLocation(req.MetadataLocation, bucketName, namespaceName); err != nil {
h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error()) h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error())
return err return err
} }
+10 -5
View File
@@ -127,11 +127,13 @@ func TableDataDirFromMetadataLocation(metadataLocation string) string {
} }
// ValidateMetadataLocation checks that an s3:// metadata location stays within // ValidateMetadataLocation checks that an s3:// metadata location stays within
// the authorized table bucket and rejects traversal segments that path.Join // the authorized table bucket and inside the caller's own namespace subtree,
// would collapse to escape the bucket directory. Empty locations are allowed // the boundary per-tenant policies are delegated on. Without the namespace
// (the catalog derives one). A non-empty location must include a table path so // check a caller could register a location inside another tenant's data
// its metadata directory is table-specific, not shared at the bucket level. // directory and DeleteTable would purge it. Empty locations are allowed (the
func ValidateMetadataLocation(metadataLocation, bucketName string) error { // catalog derives one). A non-empty location must include a table path so its
// metadata directory is table-specific, not shared at the bucket level.
func ValidateMetadataLocation(metadataLocation, bucketName, namespace string) error {
if metadataLocation == "" { if metadataLocation == "" {
return nil return nil
} }
@@ -155,6 +157,9 @@ func ValidateMetadataLocation(metadataLocation, bucketName string) error {
if !hasSegment { if !hasSegment {
return fmt.Errorf("metadata location must include a table path") return fmt.Errorf("metadata location must include a table path")
} }
if !strings.HasPrefix(tablePath, namespace+"/") {
return fmt.Errorf("metadata location must be within namespace %s", namespace)
}
return nil return nil
} }
+25 -18
View File
@@ -4,29 +4,36 @@ import "testing"
func TestValidateMetadataLocation(t *testing.T) { func TestValidateMetadataLocation(t *testing.T) {
tests := []struct { tests := []struct {
name string name string
loc string loc string
bucket string bucket string
wantErr bool namespace string
wantErr bool
}{ }{
{"empty allowed", "", "bkt", false}, {"empty allowed", "", "bkt", "ns", false},
{"same bucket", "s3://bkt/ns/t/metadata/v1.metadata.json", "bkt", false}, {"own namespace", "s3://bkt/ns/t/metadata/v1.metadata.json", "bkt", "ns", false},
{"cross bucket rejected", "s3://other/ns/t/metadata/v1.metadata.json", "bkt", true}, {"own namespace arbitrary layout", "s3://bkt/ns/warehouse/t/metadata/v1.metadata.json", "bkt", "ns", false},
{"bucket only rejected", "s3://bkt", "bkt", true}, {"cross bucket rejected", "s3://other/ns/t/metadata/v1.metadata.json", "bkt", "ns", true},
{"bucket with trailing slash rejected", "s3://bkt/", "bkt", true}, {"cross namespace rejected", "s3://bkt/nsa/t/metadata/v1.metadata.json", "bkt", "ns", true},
{"slash-only path rejected", "s3://bkt///", "bkt", true}, {"cross namespace data dir rejected", "s3://bkt/nsa/vtab/data/metadata/v1.metadata.json", "bkt", "ns", true},
{"dotdot in path rejected", "s3://bkt/../victim/metadata/v1.metadata.json", "bkt", true}, {"namespace prefix rejected", "s3://bkt/nsa/vtab/metadata/v1.metadata.json", "bkt", "ns", true},
{"dotdot mid path rejected", "s3://bkt/ns/../../victim/metadata/v1.metadata.json", "bkt", true}, {"outside namespace tree rejected", "s3://bkt/shared/t/metadata/v1.metadata.json", "bkt", "ns", true},
{"dot segment rejected", "s3://bkt/ns/./t/metadata/v1.metadata.json", "bkt", true}, {"namespace dir without table path rejected", "s3://bkt/ns", "bkt", "ns", true},
{"backslash segment rejected", "s3://bkt/ns/\\t/metadata/v1.metadata.json", "bkt", true}, {"bucket only rejected", "s3://bkt", "bkt", "ns", true},
{"non-s3 scheme rejected", "file:///bkt/ns/t", "bkt", true}, {"bucket with trailing slash rejected", "s3://bkt/", "bkt", "ns", true},
{"empty location string", "s3://", "bkt", true}, {"slash-only path rejected", "s3://bkt///", "bkt", "ns", true},
{"dotdot in path rejected", "s3://bkt/../victim/metadata/v1.metadata.json", "bkt", "ns", true},
{"dotdot mid path rejected", "s3://bkt/ns/../../victim/metadata/v1.metadata.json", "bkt", "ns", true},
{"dot segment rejected", "s3://bkt/ns/./t/metadata/v1.metadata.json", "bkt", "ns", true},
{"backslash segment rejected", "s3://bkt/ns/\\t/metadata/v1.metadata.json", "bkt", "ns", true},
{"non-s3 scheme rejected", "file:///bkt/ns/t", "bkt", "ns", true},
{"empty location string", "s3://", "bkt", "ns", true},
} }
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
err := ValidateMetadataLocation(tt.loc, tt.bucket) err := ValidateMetadataLocation(tt.loc, tt.bucket, tt.namespace)
if (err != nil) != tt.wantErr { if (err != nil) != tt.wantErr {
t.Fatalf("ValidateMetadataLocation(%q, %q) err = %v, wantErr = %v", tt.loc, tt.bucket, err, tt.wantErr) t.Fatalf("ValidateMetadataLocation(%q, %q, %q) err = %v, wantErr = %v", tt.loc, tt.bucket, tt.namespace, err, tt.wantErr)
} }
}) })
} }