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
}
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())
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
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())
return err
}
@@ -1494,7 +1494,7 @@ func (h *S3TablesHandler) handleUpdateTable(w http.ResponseWriter, r *http.Reque
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())
return err
}
+2 -2
View File
@@ -121,7 +121,7 @@ func (h *S3TablesHandler) handleCreateView(w http.ResponseWriter, r *http.Reques
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())
return err
}
@@ -410,7 +410,7 @@ func (h *S3TablesHandler) handleUpdateView(w http.ResponseWriter, r *http.Reques
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())
return err
}
+10 -5
View File
@@ -127,11 +127,13 @@ func TableDataDirFromMetadataLocation(metadataLocation string) string {
}
// ValidateMetadataLocation checks that an s3:// metadata location stays within
// the authorized table bucket and rejects traversal segments that path.Join
// would collapse to escape the bucket directory. Empty locations are allowed
// (the 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 string) error {
// the authorized table bucket and inside the caller's own namespace subtree,
// the boundary per-tenant policies are delegated on. Without the namespace
// check a caller could register a location inside another tenant's data
// directory and DeleteTable would purge it. Empty locations are allowed (the
// 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 == "" {
return nil
}
@@ -155,6 +157,9 @@ func ValidateMetadataLocation(metadataLocation, bucketName string) error {
if !hasSegment {
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
}
+25 -18
View File
@@ -4,29 +4,36 @@ import "testing"
func TestValidateMetadataLocation(t *testing.T) {
tests := []struct {
name string
loc string
bucket string
wantErr bool
name string
loc string
bucket string
namespace string
wantErr bool
}{
{"empty allowed", "", "bkt", false},
{"same bucket", "s3://bkt/ns/t/metadata/v1.metadata.json", "bkt", false},
{"cross bucket rejected", "s3://other/ns/t/metadata/v1.metadata.json", "bkt", true},
{"bucket only rejected", "s3://bkt", "bkt", true},
{"bucket with trailing slash rejected", "s3://bkt/", "bkt", true},
{"slash-only path rejected", "s3://bkt///", "bkt", true},
{"dotdot in path rejected", "s3://bkt/../victim/metadata/v1.metadata.json", "bkt", true},
{"dotdot mid path rejected", "s3://bkt/ns/../../victim/metadata/v1.metadata.json", "bkt", true},
{"dot segment rejected", "s3://bkt/ns/./t/metadata/v1.metadata.json", "bkt", true},
{"backslash segment rejected", "s3://bkt/ns/\\t/metadata/v1.metadata.json", "bkt", true},
{"non-s3 scheme rejected", "file:///bkt/ns/t", "bkt", true},
{"empty location string", "s3://", "bkt", true},
{"empty allowed", "", "bkt", "ns", false},
{"own namespace", "s3://bkt/ns/t/metadata/v1.metadata.json", "bkt", "ns", false},
{"own namespace arbitrary layout", "s3://bkt/ns/warehouse/t/metadata/v1.metadata.json", "bkt", "ns", false},
{"cross bucket rejected", "s3://other/ns/t/metadata/v1.metadata.json", "bkt", "ns", true},
{"cross namespace rejected", "s3://bkt/nsa/t/metadata/v1.metadata.json", "bkt", "ns", true},
{"cross namespace data dir rejected", "s3://bkt/nsa/vtab/data/metadata/v1.metadata.json", "bkt", "ns", true},
{"namespace prefix rejected", "s3://bkt/nsa/vtab/metadata/v1.metadata.json", "bkt", "ns", true},
{"outside namespace tree rejected", "s3://bkt/shared/t/metadata/v1.metadata.json", "bkt", "ns", true},
{"namespace dir without table path rejected", "s3://bkt/ns", "bkt", "ns", true},
{"bucket only rejected", "s3://bkt", "bkt", "ns", true},
{"bucket with trailing slash rejected", "s3://bkt/", "bkt", "ns", 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 {
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 {
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)
}
})
}